FIXME: pnfs: BUG in layout_commit logic
Subsystems (2)
PATCH 1/1 FIXME: pnfs: BUG in layout_commit logic
When segments are used with the pnfs-objects driver (which supports them) I get a crash in layout commit do to unbalanced lseg reference counting. (under reference)
With this patch taken from the blocks layout part of the patchset this problem goes away. But looking at the code it looks scary to me (I just don't understand it fully).
Fred please review this code, to see if you like what it does.
I will need a fix for the v3.0 Kernel
SOB: Boaz
--- fs/nfs/pnfs.c | 21 ++++++++++++++++++--- fs/nfs/pnfs.h | 1 + 2 files changed, 19 insertions(+), 3 deletions(-)
fs/nfs/pnfs.c +18 -3
@@ -1246,10 +1246,19 @@ pnfs_try_to_read_data(struct nfs_read_data *rdata,static struct pnfs_layout_segment *pnfs_list_write_lseg (struct inode *inode) {struct pnfs_layout_segment *lseg, *rv =NULL ; +loff_t max_pos =0 ; + +list_for_each_entry (lseg, &NFS_I (inode)->layout->plh_segs, pls_list) { +if (lseg->pls_range.iomode ==IOMODE_RW ) { +if (max_pos < lseg->pls_end_pos) + max_pos = lseg->pls_end_pos; +if (test_and_clear_bit + (NFS_LSEG_LAYOUTCOMMIT , &lseg->pls_flags)) + rv = lseg; + } + } + rv->pls_end_pos = max_pos; -list_for_each_entry (lseg, &NFS_I (inode)->layout->plh_segs, pls_list) -if (lseg->pls_range.iomode ==IOMODE_RW ) - rv = lseg;return rv; }
@@ -1258,21 +1267,27 @@ pnfs_set_layoutcommit(struct nfs_write_data *wdata) {struct nfs_inode *nfsi =NFS_I (wdata->inode);loff_t end_pos = wdata->mds_offset + wdata->res.count; +loff_t isize =i_size_read (wdata->inode);bool mark_as_dirty = false;spin_lock (&nfsi->vfs_inode.i_lock);if (!test_and_set_bit (NFS_INO_LAYOUTCOMMIT , &nfsi->flags)) {/* references matched in nfs4_layoutcommit_release */ get_lseg (wdata->lseg); +set_bit (NFS_LSEG_LAYOUTCOMMIT , &wdata->lseg->pls_flags); wdata->lseg->pls_lc_cred =get_rpccred (wdata->args.context->state->owner->so_cred); mark_as_dirty = true;dprintk ("%s: Set layoutcommit for inode %lu " , __func__, wdata->inode->i_ino); } +if (end_pos > isize) + end_pos = isize;if (end_pos > wdata->lseg->pls_end_pos) wdata->lseg->pls_end_pos = end_pos;spin_unlock (&nfsi->vfs_inode.i_lock); +dprintk ("%s: lseg %p end_pos %llu\n" , + __func__, wdata->lseg, wdata->lseg->pls_end_pos);/* if pnfs_layoutcommit_inode() runs between inode locks, the next one * will be a noop because NFS_INO_LAYOUTCOMMIT will not be set */
fs/nfs/pnfs.h +1 -0
@@ -36,6 +36,7 @@enum {NFS_LSEG_VALID =0 ,/* cleared when lseg is recalled/returned */ NFS_LSEG_ROC ,/* roc bit received from server */ +NFS_LSEG_LAYOUTCOMMIT ,/* layoutcommit bit set for layoutcommit */ };struct pnfs_layout_segment {
Signature · 1 line
1.7.3.4
Quoted 9 lines · Boaz Harrosh
When segments are used with the pnfs-objects driver (which supports them) I get a crash in layout commit do to unbalanced lseg reference counting. (under reference)
With this patch taken from the blocks layout part of the patchset this problem goes away. But looking at the code it looks scary to me (I just don't understand it fully).
Quoted 5 lines
Fred please review this code, to see if you like what it does.
I will need a fix for the v3.0 Kernel
SOB: Boaz
<snip>
Benny, Fred, Hi.
This patch is from the pnfsblock branch:
[0e5ef56b] pnfs: let layoutcommit code handle multiple segments
by: Peng Tao<[email protected]>
Except the above patch has one obvious bug where segments are not found (See below)
Fred I'm not sure I like this patch it looks wrong to me. But it does solve my immediate problem (Needed for this Kernel). Have you had a chance to look at this code and think about what we should do?
Thanks
Boaz
---
SQUASHME: Fix BUG in: [0e5ef56b] pnfs: let layoutcommit code handle multiple segments
The patch: [0e5ef56b] pnfs: let layoutcommit code handle multiple segments Broke pnfs_find_lseg because now with the new break, the smaller then last layouts are never found.
(Though I agree that searching in reverse might be an optimization since the
last one is usually what we need. (Perhaps reverse the offset test))
---
fs/nfs/pnfs.c +1 -1
@@ -905,7 +905,7 @@ pnfs_find_lseg(struct pnfs_layout_hdr *lo,dprintk ("%s:Begin\n" , __func__);assert_spin_locked (&lo->plh_inode->i_lock); -list_for_each_entry_reverse (lseg, &lo->plh_segs, pls_list) { +list_for_each_entry (lseg, &lo->plh_segs, pls_list) {if (test_bit (NFS_LSEG_VALID , &lseg->pls_flags) &&is_matching_lseg (&lseg->pls_range, range)) { ret =get_lseg (lseg);