Hi Ben,
Thanks for continuing to work on this.
> On Aug 18, 2016, at 11:55, Benjamin Coddington <bcodding@xxxxxxxxxx> wrote:
>
> Block/SCSI layout write completion may add committable extents to the
> extent tree before updating the layout's last-written byte under the inode
> lock. If a sync happens before this value is updated, then
> prepare_layoutcommit may find and encode these extents which would produce
> a LAYOUTCOMMIT request whose encoded extents are larger than the request's
> loca_length.
>
> Fix this by holding the i_lock while marking extents writeable and updating
> the value of the last written byte. Then extending the i_lock over
> prepare_layoutcommit in pnfs_layoutcommit_inode() ensures we won't find
> extents starting beyond the current last written byte.
>
> Signed-off-by: Benjamin Coddington <bcodding@xxxxxxxxxx>
> ---
> fs/nfs/blocklayout/blocklayout.c | 3 +++
> fs/nfs/pnfs.c | 4 +---
> 2 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/fs/nfs/blocklayout/blocklayout.c b/fs/nfs/blocklayout/blocklayout.c
> index 17a42e4eb872..36923b55f4f8 100644
> --- a/fs/nfs/blocklayout/blocklayout.c
> +++ b/fs/nfs/blocklayout/blocklayout.c
> @@ -343,8 +343,11 @@ static void bl_write_cleanup(struct work_struct *work)
> u64 end = (hdr->args.offset + hdr->args.count +
> PAGE_SIZE - 1) & (loff_t)PAGE_MASK;
>
> + spin_lock(&hdr->inode->i_lock);
> + bl->bl_layout.plh_lwb = hdr->args.offset + hdr->res.count;
> ext_tree_mark_written(bl, start >> SECTOR_SHIFT,
> (end - start) >> SECTOR_SHIFT);
> + spin_unlock(&hdr->inode->i_lock);
> }
>
> pnfs_ld_write_done(hdr);
> diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
> index 4110c1dc8f68..978df68c498c 100644
> --- a/fs/nfs/pnfs.c
> +++ b/fs/nfs/pnfs.c
> @@ -2386,7 +2386,6 @@ pnfs_layoutcommit_inode(struct inode *inode, bool sync)
> end_pos = nfsi->layout->plh_lwb;
>
> nfs4_stateid_copy(&data->args.stateid, &nfsi->layout->plh_stateid);
> - spin_unlock(&inode->i_lock);
>
> data->args.inode = inode;
> data->cred = get_rpccred(nfsi->layout->plh_lc_cred);
> @@ -2403,14 +2402,13 @@ pnfs_layoutcommit_inode(struct inode *inode, bool sync)
> status = ld->prepare_layoutcommit(&data->args);
Doesn’t ext_tree_prepare_commit() contain a GFP_NOFS allocation that could sleep?
> if (status) {
> put_rpccred(data->cred);
> - spin_lock(&inode->i_lock);
> set_bit(NFS_INO_LAYOUTCOMMIT, &nfsi->flags);
> if (end_pos > nfsi->layout->plh_lwb)
> nfsi->layout->plh_lwb = end_pos;
> goto out_unlock;
> }
> }
> -
> + spin_unlock(&inode->i_lock);
>
> status = nfs4_proc_layoutcommit(data, sync);
> out:
> --
> 2.5.5
>
--
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
