lkml.org 
[lkml]   [2016]   [Jun]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v2] nfsd: Always lock state exclusively.
    On Tue, Jun 14, 2016 at 10:19:49PM -0400, Oleg Drokin wrote:
    > On Jun 14, 2016, at 2:46 PM, J . Bruce Fields wrote:
    > > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
    > > index fa5fb5aa4847..41b59854c40f 100644
    > > --- a/fs/nfsd/nfs4state.c
    > > +++ b/fs/nfsd/nfs4state.c
    > > @@ -3480,13 +3480,15 @@ alloc_init_open_stateowner(unsigned int strhashval, struct nfsd4_open *open,
    > > }
    > >
    > > static struct nfs4_ol_stateid *
    > > -init_open_stateid(struct nfs4_ol_stateid *stp, struct nfs4_file *fp,
    > > - struct nfsd4_open *open)
    > > +init_open_stateid(struct nfs4_file *fp, struct nfsd4_open *open)
    > > {
    > >
    > > struct nfs4_openowner *oo = open->op_openowner;
    > > struct nfs4_ol_stateid *retstp = NULL;
    > > + struct nfs4_ol_stateid *stp;
    > >
    > > + stp = open->op_stp;
    > > + open->op_stp = NULL;
    > > /* We are moving these outside of the spinlocks to avoid the warnings */
    > > mutex_init(&stp->st_mutex);
    > > mutex_lock(&stp->st_mutex);
    > > @@ -3512,9 +3514,12 @@ init_open_stateid(struct nfs4_ol_stateid *stp, struct nfs4_file *fp,
    > > out_unlock:
    > > spin_unlock(&fp->fi_lock);
    > > spin_unlock(&oo->oo_owner.so_client->cl_lock);
    > > - if (retstp)
    > > - mutex_lock(&retstp->st_mutex);
    > > - return retstp;
    > > + if (retstp) {
    > > + nfs4_put_stid(&stp->st_stid);
    >
    > So as I am trying to integrate this into my patchset,
    > do we really need this?
    > We don't if we took the other path and left this one
    > hanging off the struct nfsd4_open (why do we need to
    > assign it NULL before the search?) I imagine then
    > we'd save some free/realloc churn as well?

    Yes, good idea.

    > I assume struct nfsd4_open cannot be shared between threads?

    Right.

    > Otherwise we have bigger problems at hand like mutex init on a locked
    > mutex from another thread and stuff.
    >
    > I'll try this theory I guess.

    Sounds good!

    --b.

    \
     
     \ /
      Last update: 2016-06-15 16:21    [W:2.919 / U:0.572 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site