From: Hannes Reinecke <hare@suse.com>
To: Martin Wilck <mwilck@suse.com>,
"bmarzins@redhat.com" <bmarzins@redhat.com>,
Etienne Aujames <eaujames@ddn.com>
Cc: Christophe Varoqui <christophe.varoqui@opensvc.com>,
nilesh.javali@marvell.com, DM-DEVEL ML <dm-devel@lists.linux.dev>
Subject: Re: Problem with [PATCH] libmultipath: fix max_sectors_kb on adding path
Date: Fri, 12 Apr 2024 10:25:52 +0200 [thread overview]
Message-ID: <30522782-cafa-4901-9635-bb14bd4d810f@suse.com> (raw)
In-Reply-To: <20c44a0ebb3aef1c6924aa7ab6006785330bb744.camel@suse.com>
On 4/12/24 09:21, Martin Wilck wrote:
> On Fri, 2024-04-12 at 08:06 +0200, Hannes Reinecke wrote:
>>>
>> We have gone into great pains in the kernel to ensure the queue
>> limits
>> are sane, and updated correctly. Even for stacking devices.
>
> This is true, but only for the creation of stacked devices (table
> activation, as far as device mapper is concerned). Admins are free to
> change max_sectors_kb any time; there's no propagation of changed
> settings along the device stack, and no sanity checking in the kernel
> prevents them from setting values that will cause I/O errors.
>
>> I sinserely doubt we need this patch from multipath anymore.
>> Having to adjust max_sectors_kb really should be reserved for
>> corner-cases where the user has a dodgy hardware which doesn't
>> report correct limits.
>
> Right. We've seen a couple of cases where decreasing max_sectors_kb
> from the default value was the only remedy for weird I/O failures. This
> happened with remote storage reporting wrong limits, misbehaving
> elements in the fabric, and even with virtualized IO stacks.
>
>> But even that should rather be handled by blacklisting.
>> Can't we just set max_sectors_kb to readonly in the kernel and
>> be done with it?
>
> Personally, I think this goes a bit too far. I believe the kernel
> should disallow changing (more specifically, decreasing) the
> max_sectors_kb sysfs attribute for block devices that are either in use
> (bd_openers > 0) or held by other block devices (bd_holder != NULL).
> That would eliminate a large portion of bad cases, AFAICS. Admins could
> still increase max_sectors_kb at the top of the device stack, but that
> would arguably count as shooting oneself into the foot.
>
> Errors in valid configurations are possible, even without changing
> max_sectors_kb in sysfs. Consider a multipath map consisting of devices
> with different max_sectors (for example mixed iSCSI/tcp and
> iSCSI/bnx2i). If only the paths with large max_sectors are initially
> detected, and others are added later, the map's max_sectors will be
> decreased while in use, and the change will not be propagated to
> stacked block layers above multipath: bummer. The only way to avoid
> this in general is implementing limit propagation. I assume that the
> implementation of block limit propagation in the kernel would be a
> major effort with lots of possible race conditions. It's far easier to
> have admins simply impose max_sectors_kb on multipath maps in corner
> case scenarios like this.
>
Indeed; changing the max_sectors_kb while the queue is live should
be preceeded with a quiesce, to ensure that we're not changing request
limits in flight.
And disabling setting of max_sectors_kb for stacked devices is a good
idea, too.
Cheers,
Hannes
next prev parent reply other threads:[~2024-04-12 8:25 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-23 16:24 [dm-devel] [PATCH] libmultipath: fix max_sectors_kb on adding path Etienne Aujames
2023-08-23 19:51 ` Martin Wilck
2024-04-11 17:41 ` Problem with " Martin Wilck
2024-04-12 6:06 ` Hannes Reinecke
2024-04-12 7:21 ` Martin Wilck
2024-04-12 8:25 ` Hannes Reinecke [this message]
2023-08-29 19:33 ` [dm-devel] " Martin Wilck
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=30522782-cafa-4901-9635-bb14bd4d810f@suse.com \
--to=hare@suse.com \
--cc=bmarzins@redhat.com \
--cc=christophe.varoqui@opensvc.com \
--cc=dm-devel@lists.linux.dev \
--cc=eaujames@ddn.com \
--cc=mwilck@suse.com \
--cc=nilesh.javali@marvell.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 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).