From: Ewan D. Milne emilne@redhat.com
scsi: sd: Add "probe_type" module parameter to allow synchronous probing
Setting probe_type="sync" will force synchronous probing in the driver core, which forces the ordering of minor device number assignments when SCSI disk devices are probed, for compatibility with prior versions of RHEL.
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2140017 Upstream Status: RHEL-only
Signed-off-by: Ewan D. Milne emilne@redhat.com
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index blahblah..blahblah 100644 --- a/drivers/scsi/sd.c +++ b/drivers/scsi/sd.c @@ -129,6 +129,14 @@ static const char *sd_cache_types[] = { "write back, no read (daft)" };
+static const char *sd_probe_types[] = { "async", "sync" }; + +static char sd_probe_type[6] = "async"; +module_param_string(probe, sd_probe_type, sizeof(sd_probe_type), + S_IRUGO|S_IWUSR); +MODULE_PARM_DESC(probe, "async or sync. Setting to 'sync' disables asynchronous " + "device number assignments (sda, sdb, ...)."); + static void sd_set_flush_flag(struct scsi_disk *sdkp) { bool wc = false, fua = false; @@ -3858,6 +3866,8 @@ static int __init init_sd(void) goto err_out_class; }
+ if (!strcmp(sd_probe_type, "sync")) + sd_template.gendrv.probe_type = PROBE_FORCE_SYNCHRONOUS; err = scsi_register_driver(&sd_template.gendrv); if (err) goto err_out_driver;
-- https://gitlab.com/cki-project/kernel-ark/-/merge_requests/2504
From: Justin M. Forbes on gitlab.com https://gitlab.com/cki-project/kernel-ark/-/merge_requests/2504#note_1434692...
Has this been submitted upstream at all? What was the discussion on it?
From: Ewan D. Milne on gitlab.com https://gitlab.com/cki-project/kernel-ark/-/merge_requests/2504#note_1438567...
I presented about this at LSF/MM in May.
The problem basically is that Bart Van Assche's change to use the driver core for asynchronous SCSI disk ("sd") probing had a side effect that resulted in the previously (mostly) consistent ordering of minor device number assignments to become unpredictable.
commit f049cf1a7b6737c75884247c3f6383ef104d255a Author: Bart Van Assche bvanassche@acm.org Date: Tue Apr 30 14:39:18 2019 -0700
scsi: sd: Rely on the driver core for asynchronous probing
drivers/scsi/sd.c used to assign minor numbers in the synchronous probing path, and then use its own asynchronous ("..part2") mechanism to issue the commands to obtain the device attributes. Now that everything is asynchronous with the above change, the minor # assignment is too.
This is causing major problems for our customers.
We had a discussion at LSF/MM, and, as I expected, they rejected doing anything about this. Their response was to "use udev". The problem is that the behavior change is a perceived longstanding behavior regression in RHEL 9. And there are cases (e.g. w/virtualization) where there were implicit expectations about how device numbering would be done. Also, subsequent *synchronous* calls to scsi_add_device() e.g. from the mpt3sas driver, do not result in ordered minor number assignments, which breaks LSI's functionality.
We do document in the RHEL documentation that customers are supposed to use persistent device names (e.g. /dev/by-id/xxx) rather that minor numbers, especially for SAN- attached devices, but the unpredictability of minor # (/dev/sda, /dev/sdb) now happens even for *locally attached disks*.
Hence, the module option to force synchronous probing for people who really need it.
I do welcome any discussion on the desirability of a RHEL-only change, though.
From: Don Zickus on gitlab.com https://gitlab.com/cki-project/kernel-ark/-/merge_requests/2504#note_1438601...
hi @e-milne, for starters, i would recommend wrapping this with "#ifdef CONFIG_RHEL_DIFFERENCES", so @jmflinuxtx can easily remove it for fedora systems.
is there a path forward on resolving this long term? rhel-9, I understand supporting this because we didn't announce any changes. rhel-10, we have a chance to deprecate legacy technology or strongly encourage certain application/driver behaviours to change. Can we do that here? Or tweak things such that we could deprecate 'synchronous' probing? i am trying to avoid carrying this patch for years.
From: Justin M. Forbes on gitlab.com https://gitlab.com/cki-project/kernel-ark/-/merge_requests/2504#note_1438705...
I actually don't think it needs the CONFIG_RHEL_DIFFERENCES ifdef, as the code path requires a module parameter to invoke, and I will just drop the patch all together for Fedora stable branches. While it might be nice if it had RHEL in the the actual 1 line for easier identification, that does not seem worth additional effort at this point.
From: Ewan D. Milne on gitlab.com https://gitlab.com/cki-project/kernel-ark/-/merge_requests/2504#note_1440687...
I'm happy to change it to however we would like the process to work, if we want.
At LSF/MM Martin Petersen said that Oracle had some device labeling changes w/udev so that customer did not have to use hard-to-remember /dev/by-id/xxx names (some of only differ by a few characters in the middle of a string like "naa.600601603390400037c9ddb0cdcde811") but could instead define their own names. He was going to see if that could be released as open source, so for RHEL 10 we could have a better solution. What I would like to avoid is RH coming up with its own scheme which was not what other vendors adopt.
I could add a warning message that this option will be deprecated in RHEL 9 and removed in RHEL 10 once we have a plan for an alternate solution.
From: Justin M. Forbes on gitlab.com https://gitlab.com/cki-project/kernel-ark/-/merge_requests/2504#note_1440757...
What I would like to avoid is RH coming up with its own scheme which was not
what other vendors adopt.
This is my main concern. I don't want to carry a patch forever. As for the contents of this patch, I don't have a problem with it. It is a matter of what is upstream's solution to this problem. If they have a solution which does not flow well with what RHEL 9 users are expecting, a temporary fix for RHEL 9 is acceptable. But it seems long term we need a better solution that works with upstream.
From: Tomas Henzl on gitlab.com https://gitlab.com/cki-project/kernel-ark/-/merge_requests/2504#note_1443883...
This patch makes the ordering less random but doesn't guarantee ordering stable between reboots not even for local disks in all cases. The user should use persistent names. I fear that this patch may make the impression of getting persistent and easy disk names so we should add a warning saying or pointing to our documentation that while it improves the stability significantly it cannot guarantee it.
From: Ewan D. Milne on gitlab.com https://gitlab.com/cki-project/kernel-ark/-/merge_requests/2504#note_1444144...
We have never *guaranteed* consistent names across reboots, It just happens that this almost always works unless the device fails to respond, which we can't fix anyway. However, testing by users has found that Bart's change to use the driver code made the boot ordering *very* randomized. I agree that we should not document this as guaranteeing anything, just as making the probing synchronous, like it used to be. Really, that's all people are asking for, to get the behavior consistent with earlier RHEL.
It does not help e.g. for fabric attached SAN devices because the connection to the storage may not be up at boot time, users have dealt with that for decades. It helps with local disks.
From: Ewan D. Milne on gitlab.com https://gitlab.com/cki-project/kernel-ark/-/merge_requests/2504#note_1450075...
Uh...
[emilne@emilne kernel-ark.forked]$ git rebase kernel-ark/os-build Current branch bz2140017 is up to date. [emilne@emilne kernel-ark.forked]$ git push -u -f origin bz2140017 Branch 'bz2140017' set up to track remote branch 'bz2140017' from 'origin'. Everything up-to-date
kernel@lists.fedoraproject.org