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