From: Ming Lei <ming.lei@redhat.com>
To: Christoph Hellwig <hch@infradead.org>
Cc: Mike Snitzer <snitzer@kernel.org>,
Linus Torvalds <torvalds@linux-foundation.org>,
Jens Axboe <axboe@kernel.dk>,
Johannes Weiner <hannes@cmpxchg.org>,
"linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
dm-devel@lists.linux.dev, ming.lei@redhat.com
Subject: Re: [GIT PULL] Block updates for 6.9-rc1
Date: Wed, 13 Mar 2024 21:11:12 +0800 [thread overview]
Message-ID: <ZfGl8HzUpiOxCLm3@fedora> (raw)
In-Reply-To: <ZfDEtchBNeFLG-GV@infradead.org>
On Tue, Mar 12, 2024 at 02:10:13PM -0700, Christoph Hellwig wrote:
> On Tue, Mar 12, 2024 at 11:22:53AM -0400, Mike Snitzer wrote:
> > blk_validate_limits() is currently very pedantic. I discussed with Jens
> > briefly and we're thinking it might make sense for blk_validate_limits()
> > to be more forgiving by _not_ imposing hard -EINVAL failure. That in
> > the interim, during this transition to more curated and atomic limits, a
> > WARN_ON_ONCE() splat should serve as enough notice to developers (be it
> > lower level nvme or higher-level virtual devices like DM).
>
> I guess. And it more closely matches the status quo. That being said
> I want to move to hard rejection eventually to catch all the issues.
>
> > BUT for this specific max_segment_size case, the constraints of dm-crypt
> > are actually more conservative due to crypto requirements.
>
> Honestly, to me the dm-crypt requirement actually doesn't make much
> sense: max_segment_size is for hardware drivers that have requirements
> for SGLs or equivalent hardware interfaces. If dm-crypt never wants to
> see more than a single page per bio_vec it should just always iterate
> them using bio_for_each_segment.
>
> > Yet nvme's
> > more general "don't care, but will care if non-nvme driver does" for
> > this particular max_segment_size limit is being imposed when validating
> > the combined limits that dm-crypt will impose at the top-level.
>
> The real problem is that we combine the limits while we shouldn't.
> Every since we've supported immutable biovecs and do the splitting
> down in blk-mq there is no point to even inherit such limits in the
> upper drivers.
In theory, it is yes, DM even doesn't use the combined segment size
& virt boundary, but MD uses that(maybe unnecessarily), however
the two are often stacked.
There may be corner cases, and removing the two limits combination can
be one big change for DM/MD since this way has been used long time.
The warning & failure in blk_validate_limits() can fail any MD/DM
which is over scsi & nvme, so I'd suggest to remove the 'warning &
-EINVAL' first, otherwise more complaints may follow.
Thanks,
Ming
prev parent reply other threads:[~2024-03-13 13:11 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <eaeec3b6-75c2-4b65-8c50-2d37450ccdd9@kernel.dk>
[not found] ` <20240311235023.GA1205@cmpxchg.org>
[not found] ` <CAHk-=wgOfw8NBQ2Qyh8QUjhp5z4v--PuciLE7drn5LJkTtgPVw@mail.gmail.com>
[not found] ` <e3fea6c3-7704-46cd-b078-0c6e8d966474@kernel.dk>
[not found] ` <CAHk-=wgXZ6dE1yHK_jQmnSjbEbndyzZHC5dJNsmQYTD2K-m61w@mail.gmail.com>
[not found] ` <Ze-hwnd3ocfJc9xU@redhat.com>
[not found] ` <Ze-rRvKpux44ueao@infradead.org>
2024-03-12 15:22 ` [GIT PULL] Block updates for 6.9-rc1 Mike Snitzer
2024-03-12 16:28 ` Keith Busch
2024-03-12 21:10 ` Christoph Hellwig
2024-03-12 22:22 ` Mike Snitzer
2024-03-12 22:30 ` Christoph Hellwig
2024-03-12 22:50 ` Mike Snitzer
2024-03-12 22:58 ` Christoph Hellwig
2024-04-11 20:15 ` [PATCH for-6.10 0/2] dm: use late bio-splitting and queue_limits_set Mike Snitzer
2024-04-11 20:15 ` [PATCH for-6.10 1/2] dm-crypt: stop constraining max_segment_size to PAGE_SIZE Mike Snitzer
2024-04-12 6:11 ` Christoph Hellwig
2024-04-15 14:08 ` Mikulas Patocka
2024-04-23 7:32 ` Ming Lei
2024-04-11 20:15 ` [PATCH for-6.10 2/2] dm: use queue_limits_set Mike Snitzer
2024-04-23 7:33 ` Ming Lei
2024-03-13 13:11 ` Ming Lei [this message]
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=ZfGl8HzUpiOxCLm3@fedora \
--to=ming.lei@redhat.com \
--cc=axboe@kernel.dk \
--cc=dm-devel@lists.linux.dev \
--cc=hannes@cmpxchg.org \
--cc=hch@infradead.org \
--cc=linux-block@vger.kernel.org \
--cc=snitzer@kernel.org \
--cc=torvalds@linux-foundation.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).