vfs_glusterfs changes to get rid of mutex use.

Jeremy Allison jra at samba.org
Wed Jan 21 13:06:29 MST 2015


On Wed, Jan 21, 2015 at 11:04:22AM -0800, Jeremy Allison wrote:
> On Mon, Jan 19, 2015 at 11:17:08PM -0500, Ira Cooper wrote:
> > This patch removes all of the posix mutexes used in the previous GlusterFS
> > AIO patch in favor of using a standard POSIX pipe.  This also simplifies
> > the actual code, and makes our fork safety issues more apparent.
> > 
> > As part of this changeset, there is a basic set of "rules" around fork use
> > and VFS modules.  I expect they will be wordsmithed, but the basic ideas
> > are sound.
> > 
> > 1. A child process should never interfere with a parent process.
> > 2. If you call into the VFS after it has been initialized in a child
> > process, the behavior is "undefined" and could be as mean as someone
> > putting "abort" in every single call.  Calling it in the parent is totally
> > legal.
> > 
> > These rules also echo the similar conventions we use around "tevent" which
> > also apply so VFS modules can use tevent.   You don't reuse tevent contexts
> > in the child process :).
> > 
> > Feel free to correct me, this patch is based on those assumptions, as were
> > previous patches.
> 
> OK, mostly LGTM but I'd suggest one (minor) change.
> 
> Right now you're creating a pipe, then having the
> gluster threads call:
> 
>         sts = sys_write (write_fd, &req, sizeof(struct tevent_req *));
>         if (sts < 0) {
>                 DEBUG(0,("\nWrite to pipe failed (%s)", strerror(errno)));
>         }
> 
> to write (on a 64-bit box) 8 bytes of pointer
> data into it. And on the read side:
> 
>         sts = sys_read(read_fd, &req, sizeof(struct tevent_req *));
>         if (sts < 0) {
>                 DEBUG(0,("\nRead from pipe failed (%s)", strerror(errno)));
>         }
> 
>         if (req) {
>                 tevent_req_done(req);
>         }
> 
> However sys_read()/sys_write() are signal-safe, but don't
> guarentee the entire data being written in one chunk.
> 
> For that you should use write_data()/read_data() which
> are blocking calls that will not return unless theres
> an IO error, or the complete amount of data is written/read.
> 
> Now I know this is under the catagory of "this can't happen"
> bugs :-), but in order to ensure complete paranoia safety
> I'd feel happier if the sys_read/sys_write calls were
> replaced by read_data/write_data calls.

Oh, my review crossed in the post with the push :-).

Feel free to add this as an additional fix !

Cheers,

	Jeremy.


More information about the samba-technical mailing list