On Mon, Nov 10, 2008 at 03:01:19PM -0500, Trond Myklebust wrote: > On Mon, 2008-11-10 at 14:49 -0500, J. Bruce Fields wrote: > > On Mon, Nov 10, 2008 at 02:11:37PM -0500, Trond Myklebust wrote: > > > On Sun, 2008-11-09 at 16:04 -0500, J. Bruce Fields wrote: > > > > I can't see any reason we particularly need to call this until the last > > > > gssd closes the pipe. > > > > > > There's a very good reason: if we call rpc_close_pipes() then it is > > > because the kernel listener is shutting down. At that point, we want to > > > return EPIPE for all future read() or write() attempts by gssd. > > > > Sure. This doesn't interfere with that--we're still calling > > release_pipe() itself (which is the function that sets rpci->ops to > > NULL), it's only the ->release_pipe() op that we're not calling. > > > > > > Also, this allows to guarantee that open_pipe and release_pipe are > > > > called strictly in pairs; open_pipe on the first open, release_pipe on > > > > the last close. That'll make it very easy for the gss code to keep > > > > track of which pipes gssd is using. > > > > > > ...unless the gss code is no longer running. > > > > I'm not following you. > > > > Note the patch calls ->release_pipe() at close_pipe() time if there are > > still opens (and then subsequent closes become no-ops). So it does > > guarantee that there's always a matching call. > > No it doesn't. Now that rpci->ops has been set to NULL by > rpc_close_pipes(), exactly how are you going to call > rpci->ops->release_pipe() at a later time? Again, I'm dealing with that case by calling release_pipe() from rpc_close_pipes(), just as the current code does--the only change being to do that only when there are still opens: last_close = rpci->nreaders != 0 || rpci->nwriters != 0; rpci->nreaders = 0; ... rpci->nwriters = 0; if (last_close && ops->release_pipe) ops->release_pipe(inode); --b. -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
