All of lore.kernel.org
 help / color / mirror / Atom feed
From: Boaz Harrosh <bharrosh@panasas.com>
To: Tao Guo <glorioustao@gmail.com>, Andy Adamson <andros@netapp.com>,
	Benny Halevy <bhalevy@panasas.com>,
	NFS list <linux-nfs@vger.kernel.org>
Subject: [PATCH] pnfs: call layoutcommit after flushing inode's data to disk.
Date: Mon, 24 May 2010 22:04:25 +0300	[thread overview]
Message-ID: <4BFACDB9.7030008@panasas.com> (raw)

From: Tao Guo <guotao@nrchpc.ac.cn>

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
  i_size  corruption.

Signed-off-by: Tao Guo <guotao@nrchpc.ac.cn>
Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
---
 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)
-- 
1.6.6.1


             reply	other threads:[~2010-05-24 19:04 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-05-24 19:04 Boaz Harrosh [this message]
2010-05-24 19:06 ` [PATCH] pnfs: call layoutcommit after flushing inode's data to disk Boaz Harrosh
2010-05-25  6:55   ` Benny Halevy
  -- strict thread matches above, loose matches on Subject: below --
2010-05-20  3:28 Tao Guo
2010-05-20  4:42 ` Tao Guo
     [not found]   ` <AANLkTikc6JXz1ubKJHxs0UJq-x80Mafsz4SKgkBPZgnf-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2010-05-20 14:17     ` Andy Adamson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4BFACDB9.7030008@panasas.com \
    --to=bharrosh@panasas.com \
    --cc=andros@netapp.com \
    --cc=bhalevy@panasas.com \
    --cc=glorioustao@gmail.com \
    --cc=linux-nfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.