Re: [dm-devel] [PATCH 0/3] fix direct io errors on dm-crypt

2022-11-10 Thread Keith Busch
On Thu, Nov 10, 2022 at 06:24:03PM +, Eric Biggers wrote:
> On Thu, Nov 03, 2022 at 08:25:56AM -0700, Keith Busch wrote:
> > From: Keith Busch 
> > 
> > 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 requires 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.
> > 
> > There are two parts to fixing this:
> > 
> >   First, the attribute needs to be moved to the queue_limit so that it
> >   can properly stack with device mappers.
> > 
> >   Second, dm-crypt provides its minimum required limit to match the
> >   logical block size.
> > 
> > Keith Busch (3):
> >   block: make dma_alignment a stacking queue_limit
> >   dm-crypt: provide dma_alignment limit in io_hints
> >   block: make blk_set_default_limits() private
> 
> Hi Keith, can you send out an updated version of this patch series that
> addresses the feedback?
> 
> I'd really like for this bug to be fixed before 6.1 is released, so that there
> isn't a known bug in STATX_DIOALIGN already upon release.

Sorry for the delay, v2 sent.

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



Re: [dm-devel] [PATCH 0/3] fix direct io errors on dm-crypt

2022-11-10 Thread Eric Biggers
On Thu, Nov 03, 2022 at 08:25:56AM -0700, Keith Busch wrote:
> From: Keith Busch 
> 
> 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 requires 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.
> 
> There are two parts to fixing this:
> 
>   First, the attribute needs to be moved to the queue_limit so that it
>   can properly stack with device mappers.
> 
>   Second, dm-crypt provides its minimum required limit to match the
>   logical block size.
> 
> Keith Busch (3):
>   block: make dma_alignment a stacking queue_limit
>   dm-crypt: provide dma_alignment limit in io_hints
>   block: make blk_set_default_limits() private

Hi Keith, can you send out an updated version of this patch series that
addresses the feedback?

I'd really like for this bug to be fixed before 6.1 is released, so that there
isn't a known bug in STATX_DIOALIGN already upon release.

Thanks!

- Eric

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



Re: [dm-devel] [PATCH 0/3] fix direct io errors on dm-crypt

2022-11-04 Thread Mikulas Patocka



On Thu, 3 Nov 2022, Keith Busch wrote:

> On Thu, Nov 03, 2022 at 12:33:19PM -0400, Mikulas Patocka wrote:
> > Hi
> > 
> > The patchset seems OK - but dm-integrity also has a limitation that the 
> > bio vectors must be aligned on logical block size.
> > 
> > dm-writecache and dm-verity seem to handle unaligned bioset, but you 
> > should check them anyway.
> > 
> > I'm not sure about dm-log-writes.
> 
> Yeah, dm-integrity definitly needs it too. This is easy enough to use
> the same io_hint that I added for dm-crypt.
> 
> dm-log-writes is doing some weird things with writes that I don't think
> would work with some low level drivers without the same alignment
> constraint.
> 
> The other two appear to handle this fine, but I'll run everything
> through some focused test cases to be sure.
> 
> In the meantime, do you want to see the remaining mappers fixed in a v2,
> or are you okay if they follow after this series?

I don't care if you make a separate patch or fold it into an existing 
patch.

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



[dm-devel] [PATCH 0/3] fix direct io errors on dm-crypt

2022-11-04 Thread Keith Busch
From: Keith Busch 

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 requires 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.

There are two parts to fixing this:

  First, the attribute needs to be moved to the queue_limit so that it
  can properly stack with device mappers.

  Second, dm-crypt provides its minimum required limit to match the
  logical block size.

Keith Busch (3):
  block: make dma_alignment a stacking queue_limit
  dm-crypt: provide dma_alignment limit in io_hints
  block: make blk_set_default_limits() private

 block/blk-core.c   |  1 -
 block/blk-settings.c   |  9 +
 block/blk.h|  1 +
 drivers/md/dm-crypt.c  |  1 +
 include/linux/blkdev.h | 16 
 5 files changed, 15 insertions(+), 13 deletions(-)

-- 
2.30.2

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



Re: [dm-devel] [PATCH 0/3] fix direct io errors on dm-crypt

2022-11-04 Thread Dmitrii Tcvetkov
On Thu, 3 Nov 2022 08:25:56 -0700
Keith Busch  wrote:

> From: Keith Busch 
> 
> 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 requires 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.
> 
> There are two parts to fixing this:
> 
>   First, the attribute needs to be moved to the queue_limit so that it
>   can properly stack with device mappers.
> 
>   Second, dm-crypt provides its minimum required limit to match the
>   logical block size.
> 
> Keith Busch (3):
>   block: make dma_alignment a stacking queue_limit
>   dm-crypt: provide dma_alignment limit in io_hints
>   block: make blk_set_default_limits() private
> 
>  block/blk-core.c   |  1 -
>  block/blk-settings.c   |  9 +
>  block/blk.h|  1 +
>  drivers/md/dm-crypt.c  |  1 +
>  include/linux/blkdev.h | 16 
>  5 files changed, 15 insertions(+), 13 deletions(-)
> 

Applied on top 6.1-rc3, after that issue[1] doesn't reproduce.

[1] https://lore.kernel.org/linux-block/20221101001558.648ee...@xps.demsh.org/

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



Re: [dm-devel] [PATCH 0/3] fix direct io errors on dm-crypt

2022-11-04 Thread Keith Busch
On Thu, Nov 03, 2022 at 12:33:19PM -0400, Mikulas Patocka wrote:
> Hi
> 
> The patchset seems OK - but dm-integrity also has a limitation that the 
> bio vectors must be aligned on logical block size.
> 
> dm-writecache and dm-verity seem to handle unaligned bioset, but you 
> should check them anyway.
> 
> I'm not sure about dm-log-writes.

Yeah, dm-integrity definitly needs it too. This is easy enough to use
the same io_hint that I added for dm-crypt.

dm-log-writes is doing some weird things with writes that I don't think
would work with some low level drivers without the same alignment
constraint.

The other two appear to handle this fine, but I'll run everything
through some focused test cases to be sure.

In the meantime, do you want to see the remaining mappers fixed in a v2,
or are you okay if they follow after this series?

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



Re: [dm-devel] [PATCH 0/3] fix direct io errors on dm-crypt

2022-11-03 Thread Mikulas Patocka
Hi

The patchset seems OK - but dm-integrity also has a limitation that the 
bio vectors must be aligned on logical block size.

dm-writecache and dm-verity seem to handle unaligned bioset, but you 
should check them anyway.

I'm not sure about dm-log-writes.

Mikulas


On Thu, 3 Nov 2022, Keith Busch wrote:

> From: Keith Busch 
> 
> 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 requires 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.
> 
> There are two parts to fixing this:
> 
>   First, the attribute needs to be moved to the queue_limit so that it
>   can properly stack with device mappers.
> 
>   Second, dm-crypt provides its minimum required limit to match the
>   logical block size.
> 
> Keith Busch (3):
>   block: make dma_alignment a stacking queue_limit
>   dm-crypt: provide dma_alignment limit in io_hints
>   block: make blk_set_default_limits() private
> 
>  block/blk-core.c   |  1 -
>  block/blk-settings.c   |  9 +
>  block/blk.h|  1 +
>  drivers/md/dm-crypt.c  |  1 +
>  include/linux/blkdev.h | 16 
>  5 files changed, 15 insertions(+), 13 deletions(-)
> 
> -- 
> 2.30.2
> 
--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel