From: Mike Snitzer <snitzer@kernel.org>
To: Theodore Ts'o <tytso@mit.edu>
Cc: dm-devel@lists.linux.dev, fstests@vger.kernel.org,
linux-ext4@vger.kernel.org, regressions@lists.linux.dev,
Christoph Hellwig <hch@lst.de>,
linux-block@vger.kernel.org
Subject: Re: dm: use queue_limits_set
Date: Sun, 19 May 2024 01:05:40 -0400 [thread overview]
Message-ID: <ZkmIpCRaZE0237OH@kernel.org> (raw)
In-Reply-To: <20240518022646.GA450709@mit.edu>
Hi Ted,
On Fri, May 17, 2024 at 10:26:46PM -0400, Theodore Ts'o wrote:
> #regzbot introduced: 1c0e720228ad
>
> While doing final regression testing before sending a pull request for
> the ext4 tree, I found a regression which was triggered by generic/347
> and generic/405 on on multiple fstests configurations, including
> both ext4/4k and xfs/4k.
>
> It bisects cleanly to commit 1c0e720228ad ("dm: use
> queue_limits_set"), and the resulting WARNING is attached below. This
> stack trace can be seen for both generic/347 and generic/405. And if
> I revert this commit on top of linux-next, the failure goes away, so
> it pretty clearly root causes to 1c0e720228ad.
>
> For now, I'll add generic/347 and generic/405 to my global exclude
> file, but maybe we should consider reverting the commit if it can't be
> fixed quickly?
Commit 1c0e720228ad is a red herring, it switches DM over to using
queue_limits_set() which I now see is clearly disregarding DM's desire
to disable discards (in blk_validate_limits).
It looks like the combo of commit d690cb8ae14bd ("block: add an API to
atomically update queue limits") and 4f563a64732da ("block: add a
max_user_discard_sectors queue limit") needs fixing.
This being one potential fix from code inspection I've done to this
point, please see if it resolves your fstests failures (but I haven't
actually looked at those fstests yet _and_ I still need to review
commits d690cb8ae14bd and 4f563a64732da further -- will do on Monday,
sorry for the trouble):
diff --git a/block/blk-settings.c b/block/blk-settings.c
index cdbaef159c4b..c442f7ec3a6b 100644
--- a/block/blk-settings.c
+++ b/block/blk-settings.c
@@ -165,11 +165,13 @@ static int blk_validate_limits(struct queue_limits *lim)
lim->max_discard_sectors =
min(lim->max_hw_discard_sectors, lim->max_user_discard_sectors);
- if (!lim->max_discard_segments)
- lim->max_discard_segments = 1;
+ if (lim->max_discard_sectors) {
+ if (!lim->max_discard_segments)
+ lim->max_discard_segments = 1;
- if (lim->discard_granularity < lim->physical_block_size)
- lim->discard_granularity = lim->physical_block_size;
+ if (lim->discard_granularity < lim->physical_block_size)
+ lim->discard_granularity = lim->physical_block_size;
+ }
/*
* By default there is no limit on the segment boundary alignment,
diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
index 88114719fe18..e647e1bcd50c 100644
--- a/drivers/md/dm-table.c
+++ b/drivers/md/dm-table.c
@@ -1969,6 +1969,7 @@ int dm_table_set_restrictions(struct dm_table *t, struct request_queue *q,
blk_queue_flag_clear(QUEUE_FLAG_NOWAIT, q);
if (!dm_table_supports_discards(t)) {
+ limits->max_user_discard_sectors = 0;
limits->max_hw_discard_sectors = 0;
limits->discard_granularity = 0;
limits->discard_alignment = 0;
next prev parent reply other threads:[~2024-05-19 5:05 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-18 2:26 [REGRESSION] dm: use queue_limits_set Theodore Ts'o
2024-05-19 5:05 ` Mike Snitzer [this message]
2024-05-19 5:42 ` Mike Snitzer
2024-05-20 15:06 ` Christoph Hellwig
2024-05-20 15:39 ` Mike Snitzer
2024-05-20 15:44 ` Christoph Hellwig
2024-05-20 15:54 ` Mike Snitzer
2024-05-20 17:17 ` Mike Snitzer
2024-05-20 20:12 ` Christoph Hellwig
2024-05-20 22:03 ` Mike Snitzer
2024-05-21 0:45 ` Mike Snitzer
2024-05-21 15:29 ` Mike Snitzer
2024-05-20 15:47 ` Mike Snitzer
2024-05-20 15:50 ` Christoph Hellwig
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=ZkmIpCRaZE0237OH@kernel.org \
--to=snitzer@kernel.org \
--cc=dm-devel@lists.linux.dev \
--cc=fstests@vger.kernel.org \
--cc=hch@lst.de \
--cc=linux-block@vger.kernel.org \
--cc=linux-ext4@vger.kernel.org \
--cc=regressions@lists.linux.dev \
--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 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).