All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: Christoph Hellwig <hch@lst.de>
Cc: "J. Bruce Fields" <bfields@fieldses.org>,
	linux-fsdevel@vger.kernel.org, linux-nfs@vger.kernel.org,
	Jeff Layton <jlayton@primarydata.com>,
	xfs@oss.sgi.com
Subject: Re: [PATCH 16/18] xfs: do not allocate blocks when converting unwritten extents
Date: Wed, 7 Jan 2015 10:21:03 +1100	[thread overview]
Message-ID: <20150106232103.GF31508@dastard> (raw)
In-Reply-To: <1420561721-9150-17-git-send-email-hch@lst.de>

On Tue, Jan 06, 2015 at 05:28:39PM +0100, Christoph Hellwig wrote:
> Current xfs_bmapi_write always allocates blocks when it encounters a
> hole.  But for unwritten extent conversions we do not have the proper
> transaction reservations to do that, and should error out instead.
> 
> Currently this doesn't matter too much because the writeback path
> ensures that all blocks are properly allocated, but the pNFS block
> server code will accept unwritten extent conversions from clients,
> and in case of recovery from a crashed server we might get conversion
> requests for blocks whose allocation transaction hasn't made it to
> disk before the crash.  Also in general it is a good idea to be
> defensive here, especially for client initiated requests.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/libxfs/xfs_bmap.c | 15 +++++++++++++++
>  fs/xfs/xfs_iomap.c       | 17 ++++++-----------
>  2 files changed, 21 insertions(+), 11 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index b5eb474..be08671 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -4580,6 +4580,20 @@ xfs_bmapi_write(
>  		 * that we found, if any.
>  		 */
>  		if (inhole || wasdelay) {
> +			if ((flags & (XFS_BMAPI_CONVERT|XFS_BMAPI_PREALLOC)) ==
> +					XFS_BMAPI_CONVERT) {
> +				xfs_filblks_t count;
> +
> +				if (eof)
> +					bma.got.br_startoff = end;
> +
> +				count = XFS_FILBLKS_MIN(len,
> +						bma.got.br_startoff - bno);
> +				bno += count;
> +				len -= count;
> +				goto next;
> +			}

Please add a comment to the code explaining why this check is needed.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

WARNING: multiple messages have this Message-ID (diff)
From: Dave Chinner <david@fromorbit.com>
To: Christoph Hellwig <hch@lst.de>
Cc: "J. Bruce Fields" <bfields@fieldses.org>,
	Jeff Layton <jlayton@primarydata.com>,
	linux-nfs@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	xfs@oss.sgi.com
Subject: Re: [PATCH 16/18] xfs: do not allocate blocks when converting unwritten extents
Date: Wed, 7 Jan 2015 10:21:03 +1100	[thread overview]
Message-ID: <20150106232103.GF31508@dastard> (raw)
In-Reply-To: <1420561721-9150-17-git-send-email-hch@lst.de>

On Tue, Jan 06, 2015 at 05:28:39PM +0100, Christoph Hellwig wrote:
> Current xfs_bmapi_write always allocates blocks when it encounters a
> hole.  But for unwritten extent conversions we do not have the proper
> transaction reservations to do that, and should error out instead.
> 
> Currently this doesn't matter too much because the writeback path
> ensures that all blocks are properly allocated, but the pNFS block
> server code will accept unwritten extent conversions from clients,
> and in case of recovery from a crashed server we might get conversion
> requests for blocks whose allocation transaction hasn't made it to
> disk before the crash.  Also in general it is a good idea to be
> defensive here, especially for client initiated requests.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/libxfs/xfs_bmap.c | 15 +++++++++++++++
>  fs/xfs/xfs_iomap.c       | 17 ++++++-----------
>  2 files changed, 21 insertions(+), 11 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index b5eb474..be08671 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -4580,6 +4580,20 @@ xfs_bmapi_write(
>  		 * that we found, if any.
>  		 */
>  		if (inhole || wasdelay) {
> +			if ((flags & (XFS_BMAPI_CONVERT|XFS_BMAPI_PREALLOC)) ==
> +					XFS_BMAPI_CONVERT) {
> +				xfs_filblks_t count;
> +
> +				if (eof)
> +					bma.got.br_startoff = end;
> +
> +				count = XFS_FILBLKS_MIN(len,
> +						bma.got.br_startoff - bno);
> +				bno += count;
> +				len -= count;
> +				goto next;
> +			}

Please add a comment to the code explaining why this check is needed.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

  reply	other threads:[~2015-01-06 23:21 UTC|newest]

Thread overview: 153+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-06 16:28 a simple and scalable pNFS block layout server Christoph Hellwig
2015-01-06 16:28 ` Christoph Hellwig
2015-01-06 16:28 ` Christoph Hellwig
2015-01-06 16:28 ` [PATCH 01/18] nfs: add LAYOUT_TYPE_MAX enum value Christoph Hellwig
2015-01-06 16:28   ` Christoph Hellwig
2015-01-06 16:28 ` [PATCH 02/18] fs: add FL_LAYOUT lease type Christoph Hellwig
2015-01-06 16:28   ` Christoph Hellwig
2015-01-06 18:46   ` Jeff Layton
2015-01-06 18:46     ` Jeff Layton
2015-01-07 10:30     ` Christoph Hellwig
2015-01-07 10:30       ` Christoph Hellwig
2015-01-06 16:28 ` [PATCH 03/18] nfsd: factor out a helper to decode nfstime4 values Christoph Hellwig
2015-01-06 16:28   ` Christoph Hellwig
     [not found]   ` <1420561721-9150-4-git-send-email-hch-jcswGhMUV9g@public.gmane.org>
2015-01-09 23:02     ` Tom Haynes
2015-01-09 23:02       ` Tom Haynes
2015-01-09 23:02       ` Tom Haynes
2015-01-11 11:42       ` Christoph Hellwig
2015-01-11 11:42         ` Christoph Hellwig
     [not found]         ` <20150111114242.GA11939-jcswGhMUV9g@public.gmane.org>
2015-01-11 23:53           ` Tom Haynes
2015-01-11 23:53             ` Tom Haynes
2015-01-11 23:53             ` Tom Haynes
2015-01-06 16:28 ` [PATCH 04/18] nfsd: move nfsd_fh_match to nfsfh.h Christoph Hellwig
2015-01-06 16:28   ` Christoph Hellwig
2015-01-06 16:28 ` [PATCH 05/18] nfsd: add fh_fsid_match helper Christoph Hellwig
2015-01-06 16:28   ` Christoph Hellwig
2015-01-06 16:28 ` [PATCH 06/18] nfsd: make lookup/alloc/unhash_stid available outside nfs4state.c Christoph Hellwig
2015-01-06 16:28   ` Christoph Hellwig
2015-01-06 16:28 ` [PATCH 07/18] nfsd: make find/get/put file " Christoph Hellwig
2015-01-06 16:28   ` Christoph Hellwig
     [not found] ` <1420561721-9150-1-git-send-email-hch-jcswGhMUV9g@public.gmane.org>
2015-01-06 16:28   ` [PATCH 08/18] nfsd: make find_any_file " Christoph Hellwig
2015-01-06 16:28     ` Christoph Hellwig
2015-01-06 16:28     ` Christoph Hellwig
2015-01-06 16:28   ` [PATCH 10/18] nfsd: implement pNFS layout recalls Christoph Hellwig
2015-01-06 16:28     ` Christoph Hellwig
2015-01-06 16:28     ` Christoph Hellwig
     [not found]     ` <1420561721-9150-11-git-send-email-hch-jcswGhMUV9g@public.gmane.org>
2015-01-06 17:25       ` J. Bruce Fields
2015-01-06 17:25         ` J. Bruce Fields
2015-01-06 17:25         ` J. Bruce Fields
2015-01-06 17:42         ` Christoph Hellwig
2015-01-06 17:42           ` Christoph Hellwig
     [not found]           ` <20150106174214.GB16200-jcswGhMUV9g@public.gmane.org>
2015-01-06 17:59             ` Tom Haynes
2015-01-06 17:59               ` Tom Haynes
2015-01-06 17:59               ` Tom Haynes
2015-01-06 16:28   ` [PATCH 11/18] nfsd: update documentation for pNFS support Christoph Hellwig
2015-01-06 16:28     ` Christoph Hellwig
2015-01-06 16:28     ` Christoph Hellwig
2015-01-06 16:28   ` [PATCH 12/18] nfsd: add trace events Christoph Hellwig
2015-01-06 16:28     ` Christoph Hellwig
2015-01-06 16:28     ` Christoph Hellwig
2015-01-06 16:28   ` [PATCH 14/18] nfsd: pNFS block layout driver Christoph Hellwig
2015-01-06 16:28     ` Christoph Hellwig
2015-01-06 16:28     ` Christoph Hellwig
2015-01-06 17:16     ` J. Bruce Fields
2015-01-06 17:16       ` J. Bruce Fields
2015-01-06 17:39       ` Christoph Hellwig
2015-01-06 17:39         ` Christoph Hellwig
2015-01-06 19:39         ` J. Bruce Fields
2015-01-06 19:39           ` J. Bruce Fields
     [not found]           ` <20150106193949.GD28003-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>
2015-01-06 19:42             ` Jeff Layton
2015-01-06 19:42               ` Jeff Layton
2015-01-06 19:42               ` Jeff Layton
2015-01-07 10:28               ` Christoph Hellwig
2015-01-07 10:28                 ` Christoph Hellwig
2015-01-08 20:41                 ` Jeff Layton
2015-01-08 20:41                   ` Jeff Layton
2015-01-08 20:54                   ` J. Bruce Fields
2015-01-08 20:54                     ` J. Bruce Fields
     [not found]     ` <1420561721-9150-15-git-send-email-hch-jcswGhMUV9g@public.gmane.org>
2015-01-12  4:56       ` Tom Haynes
2015-01-12  4:56         ` Tom Haynes
2015-01-12  4:56         ` Tom Haynes
2015-01-12 12:43         ` Christoph Hellwig
2015-01-12 12:43           ` Christoph Hellwig
2015-01-12  6:14     ` Tom Haynes
2015-01-12  6:14       ` Tom Haynes
2015-01-12 12:46       ` Christoph Hellwig
2015-01-12 12:46         ` Christoph Hellwig
2015-01-06 16:28   ` [PATCH 15/18] xfs: pass a 64-bit count argument to xfs_iomap_write_unwritten Christoph Hellwig
2015-01-06 16:28     ` Christoph Hellwig
2015-01-06 16:28     ` Christoph Hellwig
2015-01-06 16:28   ` [PATCH 16/18] xfs: do not allocate blocks when converting unwritten extents Christoph Hellwig
2015-01-06 16:28     ` Christoph Hellwig
2015-01-06 16:28     ` Christoph Hellwig
2015-01-06 23:21     ` Dave Chinner [this message]
2015-01-06 23:21       ` Dave Chinner
2015-01-06 16:28   ` [PATCH 17/18] xfs: implement pnfs export operations Christoph Hellwig
2015-01-06 16:28     ` Christoph Hellwig
2015-01-06 16:28     ` Christoph Hellwig
2015-01-07  0:24     ` Dave Chinner
2015-01-07  0:24       ` Dave Chinner
2015-01-07 10:40       ` Christoph Hellwig
2015-01-07 10:40         ` Christoph Hellwig
2015-01-07 10:40         ` Christoph Hellwig
2015-01-07 21:11         ` Dave Chinner
2015-01-07 21:11           ` Dave Chinner
2015-01-07 21:11           ` Dave Chinner
2015-01-08 12:43           ` Christoph Hellwig
2015-01-08 12:43             ` Christoph Hellwig
2015-01-08 21:04             ` Dave Chinner
2015-01-08 21:04               ` Dave Chinner
2015-01-09 11:41               ` Christoph Hellwig
2015-01-09 11:41                 ` Christoph Hellwig
     [not found]                 ` <20150109114159.GA25728-jcswGhMUV9g@public.gmane.org>
2015-01-12  3:04                   ` Dave Chinner
2015-01-12  3:04                     ` Dave Chinner
2015-01-12  3:04                     ` Dave Chinner
2015-01-14 10:08                     ` Christoph Hellwig
2015-01-14 10:08                       ` Christoph Hellwig
2015-01-14 10:08                       ` Christoph Hellwig
2015-01-06 16:28 ` [PATCH 09/18] nfsd: implement pNFS operations Christoph Hellwig
2015-01-06 16:28   ` Christoph Hellwig
     [not found]   ` <1420561721-9150-10-git-send-email-hch-jcswGhMUV9g@public.gmane.org>
2015-01-09  0:48     ` Jeff Layton
2015-01-09  0:48       ` Jeff Layton
2015-01-09  0:48       ` Jeff Layton
2015-01-09 10:05       ` Christoph Hellwig
2015-01-09 10:05         ` Christoph Hellwig
2015-01-09 16:51         ` Jeff Layton
2015-01-09 16:51           ` Jeff Layton
2015-01-09 17:16           ` Christoph Hellwig
2015-01-09 17:16             ` Christoph Hellwig
2015-01-09 17:28             ` Jeff Layton
2015-01-09 17:28               ` Jeff Layton
2015-01-09 17:33               ` Jeff Layton
2015-01-09 17:33                 ` Jeff Layton
2015-01-09 17:43                 ` Trond Myklebust
2015-01-09 17:43                   ` Trond Myklebust
2015-01-09 18:49                   ` rfc5661 errata 3901, was: " Christoph Hellwig
2015-01-14 19:16                     ` Tom Haynes
2015-01-15 16:26                       ` Christoph Hellwig
2015-01-15 16:30                         ` [nfsv4] " Trond Myklebust
2015-01-12 17:54     ` Tom Haynes
2015-01-12 17:54       ` Tom Haynes
2015-01-12 17:54       ` Tom Haynes
2015-01-06 16:28 ` [PATCH 13/18] exportfs: add methods for block layout exports Christoph Hellwig
2015-01-06 16:28   ` Christoph Hellwig
2015-01-06 16:28 ` [PATCH 18/18] xfs: recall pNFS layouts on conflicting access Christoph Hellwig
2015-01-06 16:28   ` Christoph Hellwig
     [not found]   ` <1420561721-9150-19-git-send-email-hch-jcswGhMUV9g@public.gmane.org>
2015-01-06 23:18     ` Dave Chinner
2015-01-06 23:18       ` Dave Chinner
2015-01-06 23:18       ` Dave Chinner
2015-01-07 10:31       ` Christoph Hellwig
2015-01-07 10:31         ` Christoph Hellwig
2015-01-06 17:32 ` a simple and scalable pNFS block layout server J. Bruce Fields
2015-01-06 17:32   ` J. Bruce Fields
2015-01-06 17:56   ` Christoph Hellwig
2015-01-06 17:56     ` Christoph Hellwig
     [not found]     ` <20150106175611.GA16413-jcswGhMUV9g@public.gmane.org>
2015-01-06 18:37       ` Jeff Layton
2015-01-06 18:37         ` Jeff Layton
2015-01-06 18:37         ` Jeff Layton
2015-01-06 18:39         ` Weston Andros Adamson
2015-01-06 18:39           ` Weston Andros Adamson
2015-01-06 18:39           ` Weston Andros Adamson
2015-01-06 19:17     ` J. Bruce Fields
2015-01-06 19:17       ` J. Bruce Fields
2015-01-06 19:17       ` J. Bruce Fields

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=20150106232103.GF31508@dastard \
    --to=david@fromorbit.com \
    --cc=bfields@fieldses.org \
    --cc=hch@lst.de \
    --cc=jlayton@primarydata.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=xfs@oss.sgi.com \
    /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.