fix media change events for polled devices

Hi James,
we like to see this in 2.6.25, as "change" events for polled devices are broken at the moment.

Thanks,
Kay

From: Kay Sievers <[email protected]>
Subject: [SCSI] fix media change events for polled devices

Commit:
a341cd0f (SCSI: add asynchronous event notification API)
breaks:
285e9670 (sr,sd: send media state change modification events)
by introducing an event filter, which is removed here, to make events, we are depending on, happen again.

---

index ba21d97..ad1dc03 100644
@@ -2162,11 +2162,6 @@ void sdev_evt_send(struct scsi_device *sdev, struct scsi_event *evt)
 {
 	unsigned long flags;
 
-	if (!test_bit(evt->evt_type, sdev->supported_events)) {
-		kfree(evt);
-		return;
-	}
-
 	spin_lock_irqsave(&sdev->list_lock, flags);
 	list_add_tail(&evt->node, &sdev->event_list);
 	schedule_work(&sdev->event_work);

I'm not happy just killing the code, since when the AN infrastructure gets fixed, it will work. How about this instead as a fix for 2.6.25?

Also, acks from Jeff and Kristin would be nice.

James

---

From: Kay Sievers <[email protected]>
Date: Wed, 19 Mar 2008 13:04:47 +0100
Subject: [SCSI] fix media change events for polled devices

Commit:
a341cd0f (SCSI: add asynchronous event notification API)
breaks:
285e9670 (sr,sd: send media state change modification events)
by introducing an event filter, which is removed here, to make events, we are depending on, happen again.

Fix this by removing the event filter. It's pretty much broken at the moment, since a user can't set it (the attribute being read only). A proper fix will be to make the event discriminator distinguish between AN and Polled media change events.

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

index ba21d97..f40898d 100644
@@ -2162,10 +2162,15 @@ void sdev_evt_send(struct scsi_device *sdev, struct scsi_event *evt)
 {
 	unsigned long flags;
 
+#if 0
+	/* FIXME: currently this check eliminates all media change events
+	 * for polled devices.  Need to update to discriminate between AN
+	 * and polled events */
 	if (!test_bit(evt->evt_type, sdev->supported_events)) {
 		kfree(evt);
 		return;
 	}
+#endif
 
 	spin_lock_irqsave(&sdev->list_lock, flags);
 	list_add_tail(&evt->node, &sdev->event_list);

1.5.4.1

Who is going to fix what and when? Jeff did not even reply the last two weeks to the simple question why he added this filter, and why events need to dropped if something wants to send them. :)

These events are for non-AN devices, which we still poll for media changes. Userspace wants the same kind of events for both types of devices.

We are currently working on a the next generation of the storage part of the hal daemon, which needs these events. So it would be nice, if we can make them working now, like they worked with Kristens original patches.

Thanks,
Kay

Oh ... who else ... I'll fix it. It looks pretty simple: we need two separate events for the filter: polled and AN media change notifies. We should be able to turn either of them off, so two separate flags. They need to be unified into the single media change event at the top of the stack before we call udev.

This will allow us to turn off the AN notifies if we get a problem, which was the original idea.

If we emit the same event at the top of the stack, what more needs be done? I take it all you want is an indication that we will be using AN instead of polling, which you will be able to get from the sysfs file.

James

Ah, great! :)

Yes, they should be both simple "change" uevents.

Sure, that sounds useful.

We just want "change" uevents when the media state for the LUN changes, regardless if it is an AN event, or an event caused by revalidating the device after open(). Userspace will look at the device with every change event. If possible, we could carry a variable in the event environment to distinguish both event sources.

The sysfs flag will be read and if it indicates a non-AN device, we will not start polling the device periodically, if we don't need to.

That way the polling process will be completely dumb and optional, and the media handling side will be the same code for all types of devices.

Thanks,
Kay