| Message ID | 1408637375-11343-4-git-send-email-hch@lst.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Aug 21, 2014 at 9:09 AM, Christoph Hellwig <hch@lst.de> wrote: > Expedite layout recall processing by forcing a layout commit when > we see busy segments. Without it the layout recall might have to wait > until the VM decided to start writeback for the file, which can introduce > long delays. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > fs/nfs/callback_proc.c | 16 +++++++++++----- > fs/nfs/pnfs.c | 3 +++ > 2 files changed, 14 insertions(+), 5 deletions(-) > > diff --git a/fs/nfs/callback_proc.c b/fs/nfs/callback_proc.c > index 41db525..bf017b0 100644 > --- a/fs/nfs/callback_proc.c > +++ b/fs/nfs/callback_proc.c > @@ -164,6 +164,7 @@ static u32 initiate_file_draining(struct nfs_client *clp, > struct inode *ino; > struct pnfs_layout_hdr *lo; > u32 rv = NFS4ERR_NOMATCHING_LAYOUT; > + bool need_commit = false; > LIST_HEAD(free_me_list); > > lo = get_layout_by_fh(clp, &args->cbl_fh, &args->cbl_stateid); > @@ -172,16 +173,21 @@ static u32 initiate_file_draining(struct nfs_client *clp, > > ino = lo->plh_inode; > spin_lock(&ino->i_lock); > - if (test_bit(NFS_LAYOUT_BULK_RECALL, &lo->plh_flags) || > - pnfs_mark_matching_lsegs_invalid(lo, &free_me_list, > - &args->cbl_range)) > + if (test_bit(NFS_LAYOUT_BULK_RECALL, &lo->plh_flags)) { > rv = NFS4ERR_DELAY; > - else > - rv = NFS4ERR_NOMATCHING_LAYOUT; > + } else if (pnfs_mark_matching_lsegs_invalid(lo, &free_me_list, > + &args->cbl_range)) { > + need_commit = true; > + rv = NFS4ERR_DELAY; > + } > + > pnfs_set_layout_stateid(lo, &args->cbl_stateid, true); > spin_unlock(&ino->i_lock); > pnfs_free_lseg_list(&free_me_list); > pnfs_put_layout_hdr(lo); > + > + if (need_commit) > + pnfs_layoutcommit_inode(ino, false); Why wouldn't it make more sense to call pnfs_layoutcommit_inode() unconditionally before the call to pnfs_mark_matching_lsegs_invalid()? > iput(ino); > out: > return rv; > diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c > index 6e0fa71..242e73f 100644 > --- a/fs/nfs/pnfs.c > +++ b/fs/nfs/pnfs.c > @@ -604,6 +604,9 @@ pnfs_layout_free_bulk_destroy_list(struct list_head *layout_list, > spin_unlock(&inode->i_lock); > pnfs_free_lseg_list(&lseg_list); > pnfs_put_layout_hdr(lo); > + > + if (ret) > + pnfs_layoutcommit_inode(inode, false); Ditto. The test for 'ret' here is particularly confusing given that it can get set in a previous iteration of the loop. > iput(inode); > } > return ret; > -- > 1.9.1 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Sep 08, 2014 at 05:37:42PM -0700, Trond Myklebust wrote: > Why wouldn't it make more sense to call pnfs_layoutcommit_inode() > unconditionally before the call to pnfs_mark_matching_lsegs_invalid()? It would minimally reduce the latency, but otherwise be not very different. The downside is that we'll now need two i_lock roundtrips per recall. But if this is your preference I can easily fix it up. -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Sep 8, 2014 at 10:49 PM, Christoph Hellwig <hch@lst.de> wrote: > On Mon, Sep 08, 2014 at 05:37:42PM -0700, Trond Myklebust wrote: >> Why wouldn't it make more sense to call pnfs_layoutcommit_inode() >> unconditionally before the call to pnfs_mark_matching_lsegs_invalid()? > > It would minimally reduce the latency, but otherwise be not very different. > The downside is that we'll now need two i_lock roundtrips per recall. > > But if this is your preference I can easily fix it up. Please do. It would make the code a little easier on the eye.
diff --git a/fs/nfs/callback_proc.c b/fs/nfs/callback_proc.c index 41db525..bf017b0 100644 --- a/fs/nfs/callback_proc.c +++ b/fs/nfs/callback_proc.c @@ -164,6 +164,7 @@ static u32 initiate_file_draining(struct nfs_client *clp, struct inode *ino; struct pnfs_layout_hdr *lo; u32 rv = NFS4ERR_NOMATCHING_LAYOUT; + bool need_commit = false; LIST_HEAD(free_me_list); lo = get_layout_by_fh(clp, &args->cbl_fh, &args->cbl_stateid); @@ -172,16 +173,21 @@ static u32 initiate_file_draining(struct nfs_client *clp, ino = lo->plh_inode; spin_lock(&ino->i_lock); - if (test_bit(NFS_LAYOUT_BULK_RECALL, &lo->plh_flags) || - pnfs_mark_matching_lsegs_invalid(lo, &free_me_list, - &args->cbl_range)) + if (test_bit(NFS_LAYOUT_BULK_RECALL, &lo->plh_flags)) { rv = NFS4ERR_DELAY; - else - rv = NFS4ERR_NOMATCHING_LAYOUT; + } else if (pnfs_mark_matching_lsegs_invalid(lo, &free_me_list, + &args->cbl_range)) { + need_commit = true; + rv = NFS4ERR_DELAY; + } + pnfs_set_layout_stateid(lo, &args->cbl_stateid, true); spin_unlock(&ino->i_lock); pnfs_free_lseg_list(&free_me_list); pnfs_put_layout_hdr(lo); + + if (need_commit) + pnfs_layoutcommit_inode(ino, false); iput(ino); out: return rv; diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c index 6e0fa71..242e73f 100644 --- a/fs/nfs/pnfs.c +++ b/fs/nfs/pnfs.c @@ -604,6 +604,9 @@ pnfs_layout_free_bulk_destroy_list(struct list_head *layout_list, spin_unlock(&inode->i_lock); pnfs_free_lseg_list(&lseg_list); pnfs_put_layout_hdr(lo); + + if (ret) + pnfs_layoutcommit_inode(inode, false); iput(inode); } return ret;
Expedite layout recall processing by forcing a layout commit when we see busy segments. Without it the layout recall might have to wait until the VM decided to start writeback for the file, which can introduce long delays. Signed-off-by: Christoph Hellwig <hch@lst.de> --- fs/nfs/callback_proc.c | 16 +++++++++++----- fs/nfs/pnfs.c | 3 +++ 2 files changed, 14 insertions(+), 5 deletions(-)