On 01/05/2015 04:20 PM, Trond Myklebust wrote: > > On Jan 5, 2015 12:59 PM, "Anna Schumaker" <Anna.Schumaker@xxxxxxxxxx <mailto:Anna.Schumaker@xxxxxxxxxx>> wrote: >> >> Hi again, >> >> On 12/24/2014 02:12 AM, Tom Haynes wrote: >> > From: Peng Tao <tao.peng@xxxxxxxxxxxxxxx <mailto:tao.peng@xxxxxxxxxxxxxxx>> >> > >> > Signed-off-by: Peng Tao <tao.peng@xxxxxxxxxxxxxxx <mailto:tao.peng@xxxxxxxxxxxxxxx>> >> > Signed-off-by: Tom Haynes <Thomas.Haynes@xxxxxxxxxxxxxxx <mailto:Thomas.Haynes@xxxxxxxxxxxxxxx>> >> > --- >> > fs/nfs/pnfs.c | 8 +++----- >> > 1 file changed, 3 insertions(+), 5 deletions(-) >> > >> > diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c >> > index 2d25670..fa00b56 100644 >> > --- a/fs/nfs/pnfs.c >> > +++ b/fs/nfs/pnfs.c >> > @@ -1288,7 +1288,6 @@ pnfs_update_layout(struct inode *ino, >> > struct nfs_client *clp = server->nfs_client; >> > struct pnfs_layout_hdr *lo; >> > struct pnfs_layout_segment *lseg = NULL; >> > - bool first; >> > >> > if (!pnfs_enabled_sb(NFS_SERVER(ino))) >> > goto out; >> > @@ -1321,16 +1320,15 @@ pnfs_update_layout(struct inode *ino, >> > if (pnfs_layoutgets_blocked(lo, 0)) >> > goto out_unlock; >> > atomic_inc(&lo->plh_outstanding); >> > - >> > - first = list_empty(&lo->plh_layouts) ? true : false; >> > spin_unlock(&ino->i_lock); >> > >> > - if (first) { >> > + if (list_empty(&lo->plh_layouts)) { >> > /* The lo must be on the clp list if there is any >> > * chance of a CB_LAYOUTRECALL(FILE) coming in. >> > */ >> > spin_lock(&clp->cl_lock); >> > - list_add_tail(&lo->plh_layouts, &server->layouts); >> > + if (list_empty(&lo->plh_layouts)) >> > + list_add_tail(&lo->plh_layouts, &server->layouts); >> > spin_unlock(&clp->cl_lock); >> > } >> >> Do we really need to call list_empty() twice? Would there be a serious performance drawback if we removed the outer layer if condition and then always call list_empty() under the cl_lock? > > What is the problem with that? It avoids unnecessary contention on a per-server global lock. I was thinking about the case where the plh_layouts list becomes empty after the outer if. I took a closer look at the code and that only happens when the layout is being freed, so it shouldn't be an issue. Anna > > Please keep it, > >> >> > >> > >> > -- 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
