diff mbox

[07/19] pnfs: add flag to force read-modify-write in ->write_begin

Message ID 1408637375-11343-8-git-send-email-hch@lst.de (mailing list archive)
State New, archived
Headers show

Commit Message

Christoph Hellwig Aug. 21, 2014, 4:09 p.m. UTC
Like all block based filesystems, the pNFS block layout driver can't read
or write at a byte granularity and thus has to perform read-modify-write
cycles on writes smaller than this granularity.

Add a flag so that the core NFS code always reads a whole page when
starting a smaller write, so that we can do it in the place where the VFS
expects it instead of doing in very deadlock prone way in the writeback
handler.

Note that in theory we could do less than page size reads here for disks
that have a smaller sector size which are server by a server with a smaller
pnfs block size.  But so far that doesn't seem like a worthwhile
optimization.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/nfs/file.c | 9 +++++++++
 fs/nfs/pnfs.h | 1 +
 2 files changed, 10 insertions(+)

Comments

Trond Myklebust Sept. 9, 2014, 3:50 a.m. UTC | #1
On Thu, Aug 21, 2014 at 9:09 AM, Christoph Hellwig <hch@lst.de> wrote:
> Like all block based filesystems, the pNFS block layout driver can't read
> or write at a byte granularity and thus has to perform read-modify-write
> cycles on writes smaller than this granularity.
>
> Add a flag so that the core NFS code always reads a whole page when
> starting a smaller write, so that we can do it in the place where the VFS
> expects it instead of doing in very deadlock prone way in the writeback
> handler.
>
> Note that in theory we could do less than page size reads here for disks
> that have a smaller sector size which are server by a server with a smaller
> pnfs block size.  But so far that doesn't seem like a worthwhile
> optimization.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/nfs/file.c | 9 +++++++++
>  fs/nfs/pnfs.h | 1 +
>  2 files changed, 10 insertions(+)
>
> diff --git a/fs/nfs/file.c b/fs/nfs/file.c
> index 524dd80..58566e9 100644
> --- a/fs/nfs/file.c
> +++ b/fs/nfs/file.c
> @@ -36,6 +36,7 @@
>  #include "internal.h"
>  #include "iostat.h"
>  #include "fscache.h"
> +#include "pnfs.h"
>
>  #include "nfstrace.h"
>
> @@ -323,10 +324,18 @@ nfs_file_fsync(struct file *file, loff_t start, loff_t end, int datasync)
>  static int nfs_want_read_modify_write(struct file *file, struct page *page,
>                         loff_t pos, unsigned len)
>  {
> +       struct nfs_server *server = NFS_SERVER(file->f_mapping->host);
>         unsigned int pglen = nfs_page_length(page);
>         unsigned int offset = pos & (PAGE_CACHE_SIZE - 1);
>         unsigned int end = offset + len;
>
> +       if (server->pnfs_curr_ld &&
> +           (server->pnfs_curr_ld->flags & PNFS_READ_WHOLE_PAGE)) {
> +               if (!PageUptodate(page))
> +                       return 1;
> +               return 0;
> +       }
> +
>         if ((file->f_mode & FMODE_READ) &&      /* open for read? */
>             !PageUptodate(page) &&              /* Uptodate? */
>             !PagePrivate(page) &&               /* i/o request already? */
> diff --git a/fs/nfs/pnfs.h b/fs/nfs/pnfs.h
> index 16cd14f..302b279 100644
> --- a/fs/nfs/pnfs.h
> +++ b/fs/nfs/pnfs.h
> @@ -72,6 +72,7 @@ enum layoutdriver_policy_flags {
>         /* Should the pNFS client commit and return the layout upon a setattr */
>         PNFS_LAYOUTRET_ON_SETATTR       = 1 << 0,
>         PNFS_LAYOUTRET_ON_ERROR         = 1 << 1,
> +       PNFS_READ_WHOLE_PAGE            = 1 << 2,
>  };
>
>  struct nfs4_deviceid_node;
>

This doesn't look as if it will compile without IS_ENABLED(CONFIG_NFS_V4).
Christoph Hellwig Sept. 9, 2014, 5:53 a.m. UTC | #2
On Mon, Sep 08, 2014 at 08:50:13PM -0700, Trond Myklebust wrote:
> This doesn't look as if it will compile without IS_ENABLED(CONFIG_NFS_V4).

Indeed it doesn't.  I've added an inline wrapper similar to the other
PNFS layoutdriver checks.

--
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
Trond Myklebust Sept. 9, 2014, 2:41 p.m. UTC | #3
On Mon, Sep 8, 2014 at 10:53 PM, Christoph Hellwig <hch@lst.de> wrote:
> On Mon, Sep 08, 2014 at 08:50:13PM -0700, Trond Myklebust wrote:
>> This doesn't look as if it will compile without IS_ENABLED(CONFIG_NFS_V4).
>
> Indeed it doesn't.  I've added an inline wrapper similar to the other
> PNFS layoutdriver checks.
>

Cool. Thanks!
diff mbox

Patch

diff --git a/fs/nfs/file.c b/fs/nfs/file.c
index 524dd80..58566e9 100644
--- a/fs/nfs/file.c
+++ b/fs/nfs/file.c
@@ -36,6 +36,7 @@ 
 #include "internal.h"
 #include "iostat.h"
 #include "fscache.h"
+#include "pnfs.h"
 
 #include "nfstrace.h"
 
@@ -323,10 +324,18 @@  nfs_file_fsync(struct file *file, loff_t start, loff_t end, int datasync)
 static int nfs_want_read_modify_write(struct file *file, struct page *page,
 			loff_t pos, unsigned len)
 {
+	struct nfs_server *server = NFS_SERVER(file->f_mapping->host);
 	unsigned int pglen = nfs_page_length(page);
 	unsigned int offset = pos & (PAGE_CACHE_SIZE - 1);
 	unsigned int end = offset + len;
 
+	if (server->pnfs_curr_ld &&
+	    (server->pnfs_curr_ld->flags & PNFS_READ_WHOLE_PAGE)) {
+		if (!PageUptodate(page))
+			return 1;
+		return 0;
+	}
+
 	if ((file->f_mode & FMODE_READ) &&	/* open for read? */
 	    !PageUptodate(page) &&		/* Uptodate? */
 	    !PagePrivate(page) &&		/* i/o request already? */
diff --git a/fs/nfs/pnfs.h b/fs/nfs/pnfs.h
index 16cd14f..302b279 100644
--- a/fs/nfs/pnfs.h
+++ b/fs/nfs/pnfs.h
@@ -72,6 +72,7 @@  enum layoutdriver_policy_flags {
 	/* Should the pNFS client commit and return the layout upon a setattr */
 	PNFS_LAYOUTRET_ON_SETATTR	= 1 << 0,
 	PNFS_LAYOUTRET_ON_ERROR		= 1 << 1,
+	PNFS_READ_WHOLE_PAGE		= 1 << 2,
 };
 
 struct nfs4_deviceid_node;