Add a 'wait-scan' command to /proc/scsi/scsi.

PATCH In review Bill Nottingham 4 participants 10 messages Mar 25, 2009 – Mar 26, 2009

Subsystems (2)

VFS SCSI

lore

Share & tools

lore

Bill Nottingham

author

2009-03-25 19:00 UTC #0 lore raw

PATCH 1/1 Add a 'wait-scan' command to /proc/scsi/scsi.

scsi_wait_scan.ko is a bad interface for a variety of reasons:
- once you load it, it stays in memory doing nothing
- if you need to call it again, you first need to check for the module and unload it first
- waiting for scsi scans shouldn't require module loading privleges in any case

This creates a simpler interface that doesn't require as much scaffolding in userspace.

Signed-off-by Bill Nottingham <[email protected]>

---
 drivers/scsi/scsi_proc.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

drivers/scsi/scsi_proc.c +2 -0

index 82f7b2d..c9f9e0c 100644
@@ -370,6 +370,8 @@ static ssize_t proc_scsi_write(struct file *file, const char __user *buf,
 		lun = simple_strtoul(p + 1, &p, 0);
 
 		err = scsi_remove_single_device(host, channel, id, lun);
+	} else if (!strncmp("scsi wait-scan", buffer, 14)) {
+	        err = scsi_complete_async_scans();
 	}
 
 	/*

Signature · 1 line

1.6.2

Bill Nottingham

author

2009-03-25 19:00 UTC #1 lore raw

PATCH 1/1 Declare PIO_CMAP/GIO_CMAP as compatbile ioctls.

Otherwise, these don't work when called from 32-bit userspace on 64-bit kernels.

---
 fs/compat_ioctl.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

fs/compat_ioctl.c +2 -0

index 5235c67..9ee71a0 100644
@@ -1937,6 +1937,8 @@ ULONG_IOCTL(SET_BITMAP_FILE)
 /* Big K */
 COMPATIBLE_IOCTL(PIO_FONT)
 COMPATIBLE_IOCTL(GIO_FONT)
+COMPATIBLE_IOCTL(PIO_CMAP)
+COMPATIBLE_IOCTL(GIO_CMAP)
 ULONG_IOCTL(KDSIGACCEPT)
 COMPATIBLE_IOCTL(KDGETKEYCODE)
 COMPATIBLE_IOCTL(KDSETKEYCODE)

Signature · 1 line

1.6.1.2

Bill Nottingham

author

2009-03-25 19:02 UTC #2 lore raw

Re: [PATCH] Declare PIO_CMAP/GIO_CMAP as compatbile ioctls.

Quoted 1 line · Bill Nottingham

Otherwise, these don't work when called from 32-bit userspace on 64-bit kernels.

Sorry about this last bit - git hiccup.

Bill

Christoph Hellwig

2009-03-25 19:01 UTC #3 lore raw

In reply to Bill Nottingham

Quoted 9 lines · Bill Nottingham

scsi_wait_scan.ko is a bad interface for a variety of reasons:
- once you load it, it stays in memory doing nothing
- if you need to call it again, you first need to check for the module and unload it first
- waiting for scsi scans shouldn't require module loading privleges in any case
This creates a simpler interface that doesn't require as much scaffolding in userspace.

/proc/scsi/scsi is deprecated, plese don't add more features to it.

Bill Nottingham

author

2009-03-25 19:04 UTC #4 lore raw

Quoted 8 lines · Christoph Hellwig

Quoted 7 lines · Bill Nottingham
scsi_wait_scan.ko is a bad interface for a variety of reasons:
- once you load it, it stays in memory doing nothing
- if you need to call it again, you first need to check for the module and unload it first
- waiting for scsi scans shouldn't require module loading privleges in any case

Quoted 4 lines

Quoted 2 lines
This creates a simpler interface that doesn't require as much scaffolding in userspace.
/proc/scsi/scsi is deprecated, plese don't add more features to it.

Then where is a better place to put this, as scsi_wait_scan.ko is a ridiculous interface for userspace?

Bill

Matthew Wilcox

2009-03-25 19:27 UTC #5 lore raw

Quoted 2 lines · Bill Nottingham

Then where is a better place to put this, as scsi_wait_scan.ko is a ridiculous interface for userspace?

It would be nice if people would comment on "ridiculous interface"s when they're asked for feedback, instead of waiting more than two years.

I think you're misunderstanding how to use scsi_wait_scan. The idea was that the bit of userspace that probes all the device drivers would do:

modprobe fusion.ko
modprobe aic79xx.ko
modprobe sym53c8xx.ko
modprobe scsi_wait_scan
rmmod scsi_wait_scan

et voila, you're done. It seems like you want random other bits of userspace to wait for scsi scanning to be done, and that wasn't the original intent.

I suppose we could add Yet Another Damn Sysfs File, say /sys/bus/scsi/wait_scan

Signature · 4 lines

Matthew Wilcox				Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

Bill Nottingham

author

2009-03-25 19:48 UTC #6 lore raw

Quoted 6 lines · Matthew Wilcox

Quoted 2 lines · Bill Nottingham
Then where is a better place to put this, as scsi_wait_scan.ko is a ridiculous interface for userspace?
It would be nice if people would comment on "ridiculous interface"s when they're asked for feedback, instead of waiting more than two years.

Sure, but asking all people who might eventually have to use it to always watch any possible interface addition isn't practical.

I would have hoped that the fact that the interface required loading a module and immediately removing it by hand is suboptimal enough that it wouldn't have gotten in in the first place.

Quoted 9 lines

I think you're misunderstanding how to use scsi_wait_scan. The idea was that the bit of userspace that probes all the device drivers would do:
modprobe fusion.ko
modprobe aic79xx.ko
modprobe sym53c8xx.ko
modprobe scsi_wait_scan
rmmod scsi_wait_scan

Quoted 3 lines

et voila, you're done. It seems like you want random other bits of userspace to wait for scsi scanning to be done, and that wasn't the original intent.

Well, in the case I'm looking at, udev is what's loading the host controllers, and there needs to be some sort of synchronization point between that and LVM invocations, fsck, mount, etc. Since scans aren't sent over as events for udev to catch, 'udevadm settle' isn't enough. Removing, loading, and removing scsi_wait_scan works here, but it just seems like a kludge.

I can trigger a load of scsi_wait_scan when hosts are registered in udev, but that's still ugly, and sort of overkill.

Bill

Matthew Wilcox

2009-03-25 20:36 UTC #7 lore raw

Quoted 10 lines · Bill Nottingham

Quoted 6 lines · Matthew Wilcox
Quoted 2 lines · Bill Nottingham
Then where is a better place to put this, as scsi_wait_scan.ko is a ridiculous interface for userspace?
It would be nice if people would comment on "ridiculous interface"s when they're asked for feedback, instead of waiting more than two years.
Sure, but asking all people who might eventually have to use it to always watch any possible interface addition isn't practical.

Right. I asked several people at Red Hat about the interface and I got a "yeah, OK, whatever" kind of response. Clearly you need to educate your colleagues to pass these kinds of interface questions along to you.

Quoted 3 lines

I would have hoped that the fact that the interface required loading a module and immediately removing it by hand is suboptimal enough that it wouldn't have gotten in in the first place.

It seems pretty elegant to me, actually. There's no overhead after you're done (unlike having a sysfs file, or even including a new ability in a procfs file).

Quoted 13 lines

Quoted 9 lines
I think you're misunderstanding how to use scsi_wait_scan. The idea was that the bit of userspace that probes all the device drivers would do:
modprobe fusion.ko
modprobe aic79xx.ko
modprobe sym53c8xx.ko
modprobe scsi_wait_scan
rmmod scsi_wait_scan
Quoted 3 lines
et voila, you're done. It seems like you want random other bits of userspace to wait for scsi scanning to be done, and that wasn't the original intent.

Quoted 5 lines

Well, in the case I'm looking at, udev is what's loading the host controllers, and there needs to be some sort of synchronization point between that and LVM invocations, fsck, mount, etc. Since scans aren't sent over as events for udev to catch, 'udevadm settle' isn't enough.

So ... if we sent a udev event when the scan list was empty, you'd be OK?

Quoted 2 lines

Removing, loading, and removing scsi_wait_scan works here, but it just seems like a kludge.

I don't quite understand why it was loaded, and not unloaded immediately.

Quoted 2 lines

I can trigger a load of scsi_wait_scan when hosts are registered in udev, but that's still ugly, and sort of overkill.

That would rather miss the point, yes.

Signature · 4 lines

Matthew Wilcox				Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

Bill Nottingham

author

2009-03-26 14:47 UTC #8 lore raw

Quoted 6 lines · Matthew Wilcox

Quoted 2 lines
Sure, but asking all people who might eventually have to use it to always watch any possible interface addition isn't practical.
Right. I asked several people at Red Hat about the interface and I got a "yeah, OK, whatever" kind of response. Clearly you need to educate your colleagues to pass these kinds of interface questions along to you.

Heh. Of course, I suspect many of them and I wouldn't see eye-to-eye in any case.

Quoted 7 lines

Quoted 5 lines
Well, in the case I'm looking at, udev is what's loading the host controllers, and there needs to be some sort of synchronization point between that and LVM invocations, fsck, mount, etc. Since scans aren't sent over as events for udev to catch, 'udevadm settle' isn't enough.
So ... if we sent a udev event when the scan list was empty, you'd be OK?

I'm CC'ing Peter, who has some more ideas - it would definitely be a good start, but we'd probably at least need to know when the scan list started being filled as well.

This also doesn't help with USB, where the scan isn't scheduled until some indeterminate time after the host is registered, but given that USB is always hotpluggable, you can never fully be sure there.

Ideally, once we get to the point where:

- MD
- DM/LVM/etc
- fsck
- mount

can all be event-driven on startup, we won't need this. But we're not there yet.

Quoted 4 lines

Quoted 2 lines
Removing, loading, and removing scsi_wait_scan works here, but it just seems like a kludge.
I don't quite understand why it was loaded, and not unloaded immediately.

In the initramfs (as we use it) it is, but any time later in userspace it seems prudent to make sure it's removed first. I toyed with the idea of having module_init() return an error so that the module unregisters itself after doing the wait-for-scans, but the error of course propagates back to userspace, giving lots of ugly messages in the default config. (You could do a modprobe rule that automatically removes it after insertion.)

Bill

Kay Sievers

2009-03-26 17:25 UTC #9 lore raw

Quoted 8 lines · Bill Nottingham

Quoted 6 lines · Matthew Wilcox
Quoted 5 lines
Well, in the case I'm looking at, udev is what's loading the host controllers, and there needs to be some sort of synchronization point between that and LVM invocations, fsck, mount, etc. Since scans aren't sent over as events for udev to catch, 'udevadm settle' isn't enough.

Quoted 5 lines

Quoted 1 line
So ... if we sent a udev event when the scan list was empty, you'd be OK?
I'm CC'ing Peter, who has some more ideas - it would definitely be a good start, but we'd probably at least need to know when the scan list started being filled as well.

I don't like to see any uevent send for issues like this. This would just be a hack around badly designed system services, which should be fixed themselves and not worked around in the kernel. This hack will run useless events for system who don't need them.

Something like a poll()'able sysfs file, which other subsytems already use, and which can be used to wake up a process that can blocks, might work fine here.

Thanks,
Kay