On 05/24/2010 10:04 PM, Boaz Harrosh wrote:
> From: Tao Guo <guotao@xxxxxxxxxxxx>
>
> This is for a bug introduced to 2.6.34. In 2.6.32 and 2.6.33 we call
> layoutcommit in nfs_sync_mapping_wait(), but in 2.6.34 we use
> sync_inode() to sync inode's data, so the layoutcommit code is gone.
>
> BTW: In current code, layoutcommit_ctx will increase refcount of
> nfs_inode's ctx, so if layoutcommit_ctx is not NULL, we could not
> reach nfs4_close_context ... --> __nfs_close().
> So pnfs_layoutcommit_inode() in __nfs_close() will not be called in
> whatever situation.
> Why we have to use nfs_inode's ctx as layoutcommit_ctx, since we
> only need its rpc_creds actually?
>
> Boaz:
> Without this patch none-files layouts which return NFS_FILE_SYNC
> from writes, will eventually hang on uncommitted inodes.
>
> This patch reveals a bad design pattern from the pnfs-client. On
> none-files layouts nfs_post_op_update_inode_force_wcc or friends
> are never called and i_size_write() is never preformed on client.
> What happens is that the layoutcommit eventually updates the server.
> In Linux the sever would update i_size_attr and mtime. It would then
> be detected by the client and i_size is fetched from server. This is
> not nice client behaviour, and is not done so for files and none-pnfs
> IO. Sigh!
> I think also files-lo might suffer from this in some cases, which means
> if commit is not preformed (It's just an hint) then writeout will have
commit => layout_commit, oops
> i_size corruption.
>
> Signed-off-by: Tao Guo <guotao@xxxxxxxxxxxx>
> Signed-off-by: Boaz Harrosh <bharrosh@xxxxxxxxxxx>
> ---
> fs/nfs/write.c | 8 +++++++-
> 1 files changed, 7 insertions(+), 1 deletions(-)
>
> diff --git a/fs/nfs/write.c b/fs/nfs/write.c
> index dad8da3..9424203 100644
> --- a/fs/nfs/write.c
> +++ b/fs/nfs/write.c
> @@ -1558,6 +1558,7 @@ int nfs_write_inode(struct inode *inode, struct writeback_control *wbc)
> */
> int nfs_wb_all(struct inode *inode)
> {
> + int ret;
> struct writeback_control wbc = {
> .sync_mode = WB_SYNC_ALL,
> .nr_to_write = LONG_MAX,
> @@ -1565,7 +1566,12 @@ int nfs_wb_all(struct inode *inode)
> .range_end = LLONG_MAX,
> };
>
> - return sync_inode(inode, &wbc);
> + ret = sync_inode(inode, &wbc);
> +#ifdef CONFIG_NFS_V4_1
> + if (!ret && do_layoutcommit(NFS_I(inode)))
> + ret = pnfs_layoutcommit_inode(inode, 1);
> +#endif
> + return ret;
> }
>
> int nfs_wb_page_cancel(struct inode *inode, struct page *page)
--
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
