On Mon, Nov 10, 2008 at 03:21:55PM -0500, Trond Myklebust wrote: > On Mon, 2008-11-10 at 15:17 -0500, J. Bruce Fields wrote: > > On Mon, Nov 10, 2008 at 03:11:38PM -0500, Trond Myklebust wrote: > > > On Mon, 2008-11-10 at 15:07 -0500, J. Bruce Fields wrote: > > > > > > > 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); > > > > > > Which means that if the kernel calls rpc_close_pipes() before gssd has > > > managed to close, then you _NEVER_ call ops->release_pipe()... > > > > So, I take "before gssd has managed to close" to mean that gssd is still > > holding the file open. Thus the statement > > > > last_close = rpci->nreaders != 0 || rpci->nwriters != 0; > > > > evaluates to true; either nreaders or nwriters must be nonzero. > > > > (And note the open and close code that modifes nreaders and nwriters is > > all serialized with this code by the i_mutex.) > > > > --b. > > Exactly... That is a very common situation that happens pretty much > every time you unmount. The kernel closes the pipe on its side, and > removes the dentry; it doesn't wait for gssd to close the pipe. Yep. And when we're in that situation, last_close is true, so if (last_close && ops->release_pipe) ops->release_pipe(inode) does ensure that calls to the ->open_pipe() and ->release_pipe() methods balance, as originally claimed. Maybe it'd be clearer to call that variable "still_open", or something? still_open = rpci->nreaders != 0 || rpci->nwriters != 0; ... if (still_open && 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
