On Fri, 2012-03-16 at 09:48 -0400, Fred Isaman wrote:
> On Thu, Mar 15, 2012 at 7:59 PM, Trond Myklebust
> <Trond.Myklebust@xxxxxxxxxx> wrote:
> > Move more pnfs-isms out of the generic commit code.
> > @@ -781,9 +784,15 @@ static u32 select_bucket_index(struct nfs4_filelayout_segment *fl, u32 j)
> > * If this will make the bucket empty, it will need to put the lseg reference.
> > * Note inode lock is held, so we can't do the put here.
> > */
>
> The whole comment above should probably be removed.
Will do!
> > +/**
> > + * nfs_request_add_commit_list - add request to a commit list
> > + * @req: pointer to a struct nfs_page
> > + * @head: commit list head
> > + *
> > + * This sets the PG_CLEAN bit, updates the inode global count of
> > + * number of outstanding requests requiring a commit as well as
> > + * the MM page stats.
> > + *
> > + * The caller must _not_ hold the inode->i_lock.
> > */
> > -static void
> > -nfs_mark_request_commit(struct nfs_page *req, struct pnfs_layout_segment *lseg)
> > +void
> > +nfs_request_add_commit_list(struct nfs_page *req, struct list_head *head)
> > {
> > struct inode *inode = req->wb_context->dentry->d_inode;
> > - struct nfs_inode *nfsi = NFS_I(inode);
> > - struct list_head *clist;
> >
> > - clist = pnfs_choose_commit_list(req, lseg);
> > - spin_lock(&inode->i_lock);
> > set_bit(PG_CLEAN, &(req)->wb_flags);
>
> shouldn't this stay inside the spinlock?
>
> Fred
The set_bit? We should always be holding the PG_BUSY lock any time that
we test or modify the other wb_flags (maybe I should put that in the
comment above), and so changing them outside the inode->i_lock should be
safe.
--
Trond Myklebust
Linux NFS client maintainer
NetApp
Trond.Myklebust@xxxxxxxxxx
www.netapp.com
��.n��������+%������w��{.n�����{��w��� jg��� �����ݢj����G���� ���j:+v���w�m������w�������h�����٥
