On Fri, 2010-01-22 at 11:30 -0800, Batsakis, Alexandros wrote:
>
> > -----Original Message-----
> > From: Myklebust, Trond
> > Sent: Friday, January 22, 2010 10:47 AM
> > To: Batsakis, Alexandros
> > Cc: linux-nfs@xxxxxxxxxxxxxxx; Myklebust, Trond
> > Subject: Re: [PATCH 1/1] nfs: fix race between renewd, umount, and the
> > state manager in V4.1
> >
> > On Thu, 2010-01-21 at 20:51 -0800, Alexandros Batsakis wrote:
> >
> > > struct rpc_cred *nfs4_get_setclientid_cred(struct nfs_client *clp)
> > > @@ -878,7 +888,11 @@ void nfs4_schedule_state_manager(struct
> > nfs_client *clp)
> > > if (test_and_set_bit(NFS4CLNT_MANAGER_RUNNING, &clp->cl_state) !=
> > 0)
> > > return;
> > > __module_get(THIS_MODULE);
> > > - atomic_inc(&clp->cl_count);
> > > + if (!atomic_inc_not_zero(&clp->cl_count)) {
> > > + nfs4_clear_state_manager_bit(clp);
> > > + module_put(THIS_MODULE);
> > > + return;
> > > + }
> >
> > The use of atomic_inc_not_zero() should not be necessary here. Anybody
> > who is calling this function without holding a reference to clp is
> > doing
> > something fundamentally wrong.
>
> Agreed, but how can we enforce this (especially with asynchronous RPC) ?
> For example what will happen if _during_ umount (after nfs_put_client()), an NFS call that was sleeping e.g. waiting for the server to come up, reaches the server and the response causes the state manager to start ?
> Any ideas ?
What would you expect to see running during umount? Nobody can be
holding a reference to the vfsmount at that point, so there should be no
active processes around to make NFS calls.
Trond
--
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
