All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: Sarthak Kukreti <sarthakkukreti@chromium.org>
Cc: Jens Axboe <axboe@kernel.dk>,
	Christoph Hellwig <hch@infradead.org>,
	Theodore Ts'o <tytso@mit.edu>,
	"Darrick J. Wong" <djwong@kernel.org>,
	Brian Foster <bfoster@redhat.com>,
	Bart Van Assche <bvanassche@google.com>,
	Mike Snitzer <snitzer@kernel.org>,
	linux-kernel@vger.kernel.org, linux-block@vger.kernel.org,
	dm-devel@redhat.com, Andreas Dilger <adilger.kernel@dilger.ca>,
	linux-fsdevel@vger.kernel.org, linux-ext4@vger.kernel.org,
	Alasdair Kergon <agk@redhat.com>
Subject: Re: [dm-devel] [PATCH v8 3/5] loop: Add support for provision requests
Date: Mon, 9 Oct 2023 10:37:52 +1100	[thread overview]
Message-ID: <ZSM9UDMFNs0050pr@dread.disaster.area> (raw)
In-Reply-To: <20231007012817.3052558-4-sarthakkukreti@chromium.org>

On Fri, Oct 06, 2023 at 06:28:15PM -0700, Sarthak Kukreti wrote:
> Add support for provision requests to loopback devices.
> Loop devices will configure provision support based on
> whether the underlying block device/file can support
> the provision request and upon receiving a provision bio,
> will map it to the backing device/storage. For loop devices
> over files, a REQ_OP_PROVISION request will translate to
> an fallocate mode 0 call on the backing file.
> 
> Signed-off-by: Sarthak Kukreti <sarthakkukreti@chromium.org>
> Signed-off-by: Mike Snitzer <snitzer@kernel.org>


Hmmmm.

This doesn't actually implement the required semantics of
REQ_PROVISION. Yes, it passes the command to the filesystem
fallocate() implementation, but fallocate() at the filesystem level
does not have the same semantics as REQ_PROVISION.

i.e. at the filesystem level, fallocate() only guarantees the next
write to the provisioned range will succeed without ENOSPC, it does
not guarantee *every* write to the range will succeed without
ENOSPC. If someone clones the loop file while it is in use (i.e.
snapshots it via cp --reflink) then all guarantees that the next
write to a provisioned LBA range will succeed without ENOSPC are
voided.

So while this will work for basic testing that the filesystem is
issuing REQ_PROVISION based IO correctly, it can't actually be used
for hosting production filesystems that need full REQ_PROVISION
guarantees when the loop device backing file is independently
shapshotted via FICLONE....

At minimuim, this set of implementation constraints needs tobe
documented somewhere...

-Dave.
-- 
Dave Chinner
david@fromorbit.com

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


WARNING: multiple messages have this Message-ID (diff)
From: Dave Chinner <david@fromorbit.com>
To: Sarthak Kukreti <sarthakkukreti@chromium.org>
Cc: dm-devel@redhat.com, linux-block@vger.kernel.org,
	linux-ext4@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-fsdevel@vger.kernel.org, Jens Axboe <axboe@kernel.dk>,
	Alasdair Kergon <agk@redhat.com>,
	Mike Snitzer <snitzer@kernel.org>,
	Christoph Hellwig <hch@infradead.org>,
	Brian Foster <bfoster@redhat.com>, Theodore Ts'o <tytso@mit.edu>,
	Andreas Dilger <adilger.kernel@dilger.ca>,
	Bart Van Assche <bvanassche@google.com>,
	"Darrick J. Wong" <djwong@kernel.org>
Subject: Re: [PATCH v8 3/5] loop: Add support for provision requests
Date: Mon, 9 Oct 2023 10:37:52 +1100	[thread overview]
Message-ID: <ZSM9UDMFNs0050pr@dread.disaster.area> (raw)
In-Reply-To: <20231007012817.3052558-4-sarthakkukreti@chromium.org>

On Fri, Oct 06, 2023 at 06:28:15PM -0700, Sarthak Kukreti wrote:
> Add support for provision requests to loopback devices.
> Loop devices will configure provision support based on
> whether the underlying block device/file can support
> the provision request and upon receiving a provision bio,
> will map it to the backing device/storage. For loop devices
> over files, a REQ_OP_PROVISION request will translate to
> an fallocate mode 0 call on the backing file.
> 
> Signed-off-by: Sarthak Kukreti <sarthakkukreti@chromium.org>
> Signed-off-by: Mike Snitzer <snitzer@kernel.org>


Hmmmm.

This doesn't actually implement the required semantics of
REQ_PROVISION. Yes, it passes the command to the filesystem
fallocate() implementation, but fallocate() at the filesystem level
does not have the same semantics as REQ_PROVISION.

i.e. at the filesystem level, fallocate() only guarantees the next
write to the provisioned range will succeed without ENOSPC, it does
not guarantee *every* write to the range will succeed without
ENOSPC. If someone clones the loop file while it is in use (i.e.
snapshots it via cp --reflink) then all guarantees that the next
write to a provisioned LBA range will succeed without ENOSPC are
voided.

So while this will work for basic testing that the filesystem is
issuing REQ_PROVISION based IO correctly, it can't actually be used
for hosting production filesystems that need full REQ_PROVISION
guarantees when the loop device backing file is independently
shapshotted via FICLONE....

At minimuim, this set of implementation constraints needs tobe
documented somewhere...

-Dave.
-- 
Dave Chinner
david@fromorbit.com

  reply	other threads:[~2023-10-08 23:38 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-07  1:28 [dm-devel] [PATCH v8 0/5] Introduce provisioning primitives Sarthak Kukreti
2023-10-07  1:28 ` Sarthak Kukreti
2023-10-07  1:28 ` [dm-devel] [PATCH v8 1/5] block: Don't invalidate pagecache for invalid falloc modes Sarthak Kukreti
2023-10-07  1:28   ` Sarthak Kukreti
2023-10-11  5:59   ` [dm-devel] " Christoph Hellwig
2023-10-11  5:59     ` Christoph Hellwig
2023-10-07  1:28 ` [dm-devel] [PATCH v8 2/5] block: Introduce provisioning primitives Sarthak Kukreti
2023-10-07  1:28   ` Sarthak Kukreti
2023-10-07  1:28 ` [dm-devel] [PATCH v8 3/5] loop: Add support for provision requests Sarthak Kukreti
2023-10-07  1:28   ` Sarthak Kukreti
2023-10-08 23:37   ` Dave Chinner [this message]
2023-10-08 23:37     ` Dave Chinner
2023-10-10 22:43     ` [dm-devel] " Sarthak Kukreti
2023-10-10 22:43       ` Sarthak Kukreti
2023-10-10 23:59       ` [dm-devel] " Dave Chinner
2023-10-10 23:59         ` Dave Chinner
2023-10-07  1:28 ` [dm-devel] [PATCH v8 4/5] dm: Add block provisioning support Sarthak Kukreti
2023-10-07  1:28   ` Sarthak Kukreti
2023-10-07  1:28 ` [dm-devel] [PATCH v8 5/5] block: Pass unshare intent via REQ_OP_PROVISION Sarthak Kukreti
2023-10-07  1:28   ` Sarthak Kukreti
2023-10-08 23:27   ` [dm-devel] " Dave Chinner
2023-10-08 23:27     ` Dave Chinner
2023-10-10 22:42     ` [dm-devel] " Sarthak Kukreti
2023-10-10 22:42       ` Sarthak Kukreti
2023-10-11  0:06       ` [dm-devel] " Dave Chinner
2023-10-11  0:06         ` Dave Chinner
2023-10-08 23:41 ` [dm-devel] [RFC PATCH 6/5] xfs: detect block devices requiring provisioning Dave Chinner
2023-10-08 23:41   ` Dave Chinner
2023-10-08 23:45 ` [dm-devel] [RFC PATCH 7/5] xfs: add block device provisioning for fallocate Dave Chinner
2023-10-08 23:45   ` Dave Chinner
2023-10-08 23:50 ` [dm-devel] [PATCH v8 0/5] Introduce provisioning primitives Dave Chinner
2023-10-08 23:50   ` Dave Chinner
2023-10-10 22:42   ` [dm-devel] " Sarthak Kukreti
2023-10-10 22:42     ` Sarthak Kukreti
2023-10-11  0:13     ` [dm-devel] " Dave Chinner
2023-10-11  0:13       ` Dave Chinner

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=ZSM9UDMFNs0050pr@dread.disaster.area \
    --to=david@fromorbit.com \
    --cc=adilger.kernel@dilger.ca \
    --cc=agk@redhat.com \
    --cc=axboe@kernel.dk \
    --cc=bfoster@redhat.com \
    --cc=bvanassche@google.com \
    --cc=djwong@kernel.org \
    --cc=dm-devel@redhat.com \
    --cc=hch@infradead.org \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sarthakkukreti@chromium.org \
    --cc=snitzer@kernel.org \
    --cc=tytso@mit.edu \
    /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.