All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mike Snitzer <snitzer@redhat.com>
To: Keith Busch <kbusch@meta.com>
Cc: axboe@kernel.dk, ebiggers@kernel.org,
	linux-block@vger.kernel.org, dm-devel@redhat.com,
	mpatocka@redhat.com, stefanha@redhat.com,
	Keith Busch <kbusch@kernel.org>,
	me@demsh.org
Subject: Re: [dm-devel] [PATCHv2 0/5] fix direct io device mapper errors
Date: Fri, 11 Nov 2022 13:07:05 -0500	[thread overview]
Message-ID: <Y26PSYu2nY/AE5Xh@redhat.com> (raw)
In-Reply-To: <20221110184501.2451620-1-kbusch@meta.com>

On Thu, Nov 10 2022 at  1:44P -0500,
Keith Busch <kbusch@meta.com> wrote:

> From: Keith Busch <kbusch@kernel.org>
> 
> The 6.0 kernel made some changes to the direct io interface to allow
> offsets in user addresses. This based on the hardware's capabilities
> reported in the request_queue's dma_alignment attribute.
> 
> dm-crypt, -log-writes and -integrity require direct io be aligned to the
> block size. Since it was only ever using the default 511 dma mask, this
> requirement may fail if formatted to something larger, like 4k, which
> will result in unexpected behavior with direct-io.
> 
> Changes since v1: Added the same fix for -integrity and -log-writes
> 
> The first three were reported successfully tested by Dmitrii Tcvetkov,
> but I don't have an official Tested-by: tag.
> 
>   https://lore.kernel.org/linux-block/20221103194140.06ce3d36@xps.demsh.org/T/#mba1d0b13374541cdad3b669ec4257a11301d1860
> 
> Keitio errors on Busch (5):
>   block: make dma_alignment a stacking queue_limit
>   dm-crypt: provide dma_alignment limit in io_hints
>   block: make blk_set_default_limits() private
>   dm-integrity: set dma_alignment limit in io_hints
>   dm-log-writes: set dma_alignment limit in io_hints
> 
>  block/blk-core.c           |  1 -
>  block/blk-settings.c       |  9 +++++----
>  block/blk.h                |  1 +
>  drivers/md/dm-crypt.c      |  1 +
>  drivers/md/dm-integrity.c  |  1 +
>  drivers/md/dm-log-writes.c |  1 +
>  include/linux/blkdev.h     | 16 ++++++++--------
>  7 files changed, 17 insertions(+), 13 deletions(-)
> 
> -- 
> 2.30.2
> 

There are other DM targets that override logical_block_size in their
.io_hints hook (writecache, ebs, zoned). Have you reasoned through why
those do _not_ need updating too?

Is there any risk of just introducing a finalization method in block
core (that DM's .io_hints would call) that would ensure limits that
are a funtion of another are always in sync?  Would avoid whack-a-mole
issues in the future.

Thanks,
Mike

--
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: Mike Snitzer <snitzer@redhat.com>
To: Keith Busch <kbusch@meta.com>
Cc: linux-block@vger.kernel.org, dm-devel@redhat.com,
	axboe@kernel.dk, stefanha@redhat.com, ebiggers@kernel.org,
	me@demsh.org, mpatocka@redhat.com,
	Keith Busch <kbusch@kernel.org>
Subject: Re: [PATCHv2 0/5] fix direct io device mapper errors
Date: Fri, 11 Nov 2022 13:07:05 -0500	[thread overview]
Message-ID: <Y26PSYu2nY/AE5Xh@redhat.com> (raw)
In-Reply-To: <20221110184501.2451620-1-kbusch@meta.com>

On Thu, Nov 10 2022 at  1:44P -0500,
Keith Busch <kbusch@meta.com> wrote:

> From: Keith Busch <kbusch@kernel.org>
> 
> The 6.0 kernel made some changes to the direct io interface to allow
> offsets in user addresses. This based on the hardware's capabilities
> reported in the request_queue's dma_alignment attribute.
> 
> dm-crypt, -log-writes and -integrity require direct io be aligned to the
> block size. Since it was only ever using the default 511 dma mask, this
> requirement may fail if formatted to something larger, like 4k, which
> will result in unexpected behavior with direct-io.
> 
> Changes since v1: Added the same fix for -integrity and -log-writes
> 
> The first three were reported successfully tested by Dmitrii Tcvetkov,
> but I don't have an official Tested-by: tag.
> 
>   https://lore.kernel.org/linux-block/20221103194140.06ce3d36@xps.demsh.org/T/#mba1d0b13374541cdad3b669ec4257a11301d1860
> 
> Keitio errors on Busch (5):
>   block: make dma_alignment a stacking queue_limit
>   dm-crypt: provide dma_alignment limit in io_hints
>   block: make blk_set_default_limits() private
>   dm-integrity: set dma_alignment limit in io_hints
>   dm-log-writes: set dma_alignment limit in io_hints
> 
>  block/blk-core.c           |  1 -
>  block/blk-settings.c       |  9 +++++----
>  block/blk.h                |  1 +
>  drivers/md/dm-crypt.c      |  1 +
>  drivers/md/dm-integrity.c  |  1 +
>  drivers/md/dm-log-writes.c |  1 +
>  include/linux/blkdev.h     | 16 ++++++++--------
>  7 files changed, 17 insertions(+), 13 deletions(-)
> 
> -- 
> 2.30.2
> 

There are other DM targets that override logical_block_size in their
.io_hints hook (writecache, ebs, zoned). Have you reasoned through why
those do _not_ need updating too?

Is there any risk of just introducing a finalization method in block
core (that DM's .io_hints would call) that would ensure limits that
are a funtion of another are always in sync?  Would avoid whack-a-mole
issues in the future.

Thanks,
Mike


  parent reply	other threads:[~2022-11-11 18:07 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-10 18:44 [dm-devel] [PATCHv2 0/5] fix direct io device mapper errors Keith Busch
2022-11-10 18:44 ` Keith Busch
2022-11-10 18:44 ` [dm-devel] [PATCHv2 1/5] block: make dma_alignment a stacking queue_limit Keith Busch
2022-11-10 18:44   ` Keith Busch
2022-11-10 18:44 ` [dm-devel] [PATCHv2 2/5] dm-crypt: provide dma_alignment limit in io_hints Keith Busch
2022-11-10 18:44   ` Keith Busch
2022-11-16 20:05   ` [dm-devel] " Mike Snitzer
2022-11-16 20:05     ` Mike Snitzer
2022-11-10 18:44 ` [dm-devel] [PATCHv2 3/5] block: make blk_set_default_limits() private Keith Busch
2022-11-10 18:44   ` Keith Busch
2022-11-10 18:45 ` [dm-devel] [PATCHv2 4/5] dm-integrity: set dma_alignment limit in io_hints Keith Busch
2022-11-10 18:45   ` Keith Busch
2022-11-16 20:06   ` [dm-devel] " Mike Snitzer
2022-11-16 20:06     ` Mike Snitzer
2022-11-10 18:45 ` [dm-devel] [PATCHv2 5/5] dm-log-writes: " Keith Busch
2022-11-10 18:45   ` Keith Busch
2022-11-16 20:07   ` [dm-devel] " Mike Snitzer
2022-11-16 20:07     ` Mike Snitzer
2022-11-11 18:07 ` Mike Snitzer [this message]
2022-11-11 18:07   ` [PATCHv2 0/5] fix direct io device mapper errors Mike Snitzer
2022-11-11 18:31   ` [dm-devel] " Keith Busch
2022-11-11 18:31     ` Keith Busch
2022-11-14 11:31     ` [dm-devel] " Mikulas Patocka
2022-11-14 11:31       ` Mikulas Patocka
2022-11-14 18:12       ` [dm-devel] " Keith Busch
2022-11-14 18:12         ` Keith Busch
2022-11-16 22:58 ` [dm-devel] " Jens Axboe
2022-11-16 22:58   ` Jens Axboe

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=Y26PSYu2nY/AE5Xh@redhat.com \
    --to=snitzer@redhat.com \
    --cc=axboe@kernel.dk \
    --cc=dm-devel@redhat.com \
    --cc=ebiggers@kernel.org \
    --cc=kbusch@kernel.org \
    --cc=kbusch@meta.com \
    --cc=linux-block@vger.kernel.org \
    --cc=me@demsh.org \
    --cc=mpatocka@redhat.com \
    --cc=stefanha@redhat.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.