Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755439Ab3HFJYU (ORCPT ); Tue, 6 Aug 2013 05:24:20 -0400 Received: from mx1.redhat.com ([209.132.183.28]:11628 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755217Ab3HFJYS convert rfc822-to-8bit (ORCPT ); Tue, 6 Aug 2013 05:24:18 -0400 Date: Tue, 6 Aug 2013 05:24:06 -0400 From: Jeff Layton To: "Myklebust, Trond" Cc: Nix , Toralf =?ISO-8859-1?B?RvZyc3Rlcg==?= , Oleg Nesterov , NFS list , Linux Kernel Mailing List , "dhowells@redhat.com" Subject: Re: [3.10.4] NFS locking panic, plus persisting NFS shutdown panic from 3.9.* Message-ID: <20130806052406.40f7852f@tlielax.poochiereds.net> In-Reply-To: <1375755693.7337.42.camel@leira.trondhjem.org> References: <8761vlv4z9.fsf@spindle.srvr.nix> <20130805084436.69ee4415@corrin.poochiereds.net> <87siyotcri.fsf@spindle.srvr.nix> <20130805110427.509db424@tlielax.poochiereds.net> <20130805111106.73d6ab90@tlielax.poochiereds.net> <87zjswrvaq.fsf@spindle.srvr.nix> <1375719301.7337.14.camel@leira.trondhjem.org> <20130805133739.21654ecb@tlielax.poochiereds.net> <1375726682.7337.29.camel@leira.trondhjem.org> <20130805143311.4ae59067@tlielax.poochiereds.net> <1375755693.7337.42.camel@leira.trondhjem.org> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4315 Lines: 98 On Tue, 6 Aug 2013 02:21:35 +0000 "Myklebust, Trond" wrote: > On Mon, 2013-08-05 at 14:33 -0400, Jeff Layton wrote: > > On Mon, 5 Aug 2013 18:18:03 +0000 > > "Myklebust, Trond" wrote: > > > > > On Mon, 2013-08-05 at 13:37 -0400, Jeff Layton wrote: > > > > On Mon, 5 Aug 2013 16:15:01 +0000 > > > > "Myklebust, Trond" wrote: > > > > > > > > > From 3c50ba80105464a28d456d9a1e0f1d81d4af92a8 Mon Sep 17 00:00:00 2001 > > > > > From: Trond Myklebust > > > > > Date: Mon, 5 Aug 2013 12:06:12 -0400 > > > > > Subject: [PATCH] LOCKD: Don't call utsname()->nodename from > > > > > nlmclnt_setlockargs > > > > > MIME-Version: 1.0 > > > > > Content-Type: text/plain; charset=UTF-8 > > > > > Content-Transfer-Encoding: 8bit > > > > > > > > > > Firstly, nlmclnt_setlockargs can be called from a reclaimer thread, in > > > > > which case we're in entirely the wrong namespace. > > > > > Secondly, commit 8aac62706adaaf0fab02c4327761561c8bda9448 (move > > > > > exit_task_namespaces() outside of exit_notify()) now means that > > > > > exit_task_work() is called after exit_task_namespaces(), which > > > > > triggers an Oops when we're freeing up the locks. > > > > > > > > > > Signed-off-by: Trond Myklebust > > > > > Cc: Toralf F?rster > > > > > Cc: Oleg Nesterov > > > > > Cc: Nix > > > > > Cc: Jeff Layton > > > > > --- > > > > > fs/lockd/clntproc.c | 5 +++-- > > > > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > > > > > > > diff --git a/fs/lockd/clntproc.c b/fs/lockd/clntproc.c > > > > > index 9760ecb..acd3947 100644 > > > > > --- a/fs/lockd/clntproc.c > > > > > +++ b/fs/lockd/clntproc.c > > > > > @@ -125,14 +125,15 @@ static void nlmclnt_setlockargs(struct nlm_rqst *req, struct file_lock *fl) > > > > > { > > > > > struct nlm_args *argp = &req->a_args; > > > > > struct nlm_lock *lock = &argp->lock; > > > > > + char *nodename = req->a_host->h_rpcclnt->cl_nodename; > > > > > > > > > > nlmclnt_next_cookie(&argp->cookie); > > > > > memcpy(&lock->fh, NFS_FH(file_inode(fl->fl_file)), sizeof(struct nfs_fh)); > > > > > - lock->caller = utsname()->nodename; > > > > > + lock->caller = nodename; > > > > > lock->oh.data = req->a_owner; > > > > > lock->oh.len = snprintf(req->a_owner, sizeof(req->a_owner), "%u@%s", > > > > > (unsigned int)fl->fl_u.nfs_fl.owner->pid, > > > > > - utsname()->nodename); > > > > > + nodename); > > > > > lock->svid = fl->fl_u.nfs_fl.owner->pid; > > > > > lock->fl.fl_start = fl->fl_start; > > > > > lock->fl.fl_end = fl->fl_end; > > > > > > > > Looks good to me... > > > > > > > > Reviewed-by: Jeff Layton > > > > > > > > Trond, any thoughts on the other oops that Nix posted? The issue there > > > > seems to be that we're trying to do the pathwalk to the rpcbind unix > > > > socket from exit_task_work(), but that's happening after we've already > > > > called exit_fs(). > > > > > > > > The trivial answer seems to be to simply call exit_task_work() before > > > > exit_fs() there, but it seems like we ought to be doing the upcall to > > > > rpcbind in a mount namespace from which we know we can reach the > > > > socket... > > > > > > Isn't it enough to just do the same thing as we did for gss proxy? i.e. > > > set the RPC_CLNT_CREATE_NO_IDLE_TIMEOUT flag. > > > > > > See attachment. > > > > Yeah, that looks like a reasonable thing to do... > > > > OTOH, Is there any other way for a unix socket to end up disconnected > > other than if we were to close it? Maybe if rpcbind stopped, the socket > > unlinked and recreated and then started again? > > > > If so then you still could potentially end up in this situation even if > > you didn't autoclose it. > > True. How about something like the following instead. Note the change to > the original patch... Looks good to me. Acked-by: Jeff Layton -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/