[PATCH v4 0/2] [SCSI] Asynchronous event notification infrastructure

[PATCH v4 0/2] [SCSI] Asynchronous event notification infrastructure

[Jeff Garzik]

This is the next revision of the SCSI event notification infrastructure
patchset, enabling SATA Asynchronous Notification ("AN") for CD/DVD
devices that support it.

For devices that support SATA AN (only very recent ones do), this means
that HAL and other userspace utilities no longer need to repeatedly poll
the CD/DVD device to determine if the user has changed the media.

This revision takes into account James' comments from earlier today,
modulo the following notes:

* I think the various event attributes should always be present,
  for all devices at all times.  If various events are not supported,
  the attribute will of course return zero (false, not supported).

* I do not think this work should be blocked behind a revamp
  of the attribute group interface.

* I was slack and did not bother to implement the 'set' operation
  for the attributes.  This can easily be done at a later time in a
  separate patch.  It is not a merge stopper to have the driver
  exclusively control the event mask, rather than driver+sysfs.

[PATCH v4 1/2] SCSI: Asynchronous event notification infrastructure

[Jeff Garzik]

Originally based on a patch by Kristen Carlson Accardi @ Intel.
Copious input from James Bottomley.

Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
---
 drivers/scsi/scsi_lib.c    |   66 ++++++++++++++++++++++++++++++++++++++++++++
 drivers/scsi/scsi_scan.c   |    2 +
 drivers/scsi/scsi_sysfs.c  |   20 +++++++++++++
 include/scsi/scsi_device.h |   12 ++++++++
 4 files changed, 100 insertions(+), 0 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 61fdaf0..f55ec80 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -18,6 +18,7 @@
 #include <linux/delay.h>
 #include <linux/hardirq.h>
 #include <linux/scatterlist.h>
+#include <linux/bitmap.h>
 
 #include <scsi/scsi.h>
 #include <scsi/scsi_cmnd.h>
@@ -2115,6 +2116,71 @@ scsi_device_set_state(struct scsi_device *sdev, enum scsi_device_state state)
 EXPORT_SYMBOL(scsi_device_set_state);
 
 /**
+ * 	sdev_evt_thread - send a uevent for each scsi event
+ *	@work: work struct for scsi_device
+ *
+ *	Emit all queued media events as environment variables
+ *	sent during a uevent.
+ */
+void scsi_evt_thread(struct work_struct *work)
+{
+	struct scsi_device *sdev;
+	char *envp[SDEV_EVT_LAST + 2];
+	DECLARE_BITMAP(mask, SDEV_EVT_MAXBITS);
+	unsigned long flags;
+	int evt, idx;
+
+	sdev = container_of(work, struct scsi_device, event_work);
+
+	spin_lock_irqsave(&sdev->list_lock, flags);
+	bitmap_copy(mask, sdev->event_mask, SDEV_EVT_MAXBITS);
+	bitmap_zero(sdev->event_mask, SDEV_EVT_MAXBITS);
+	spin_unlock_irqrestore(&sdev->list_lock, flags);
+
+	idx = 0;
+	for (evt = 0; evt < SDEV_EVT_LAST; evt++) {
+		if (!test_bit(evt, mask))
+			continue;
+
+		switch (evt) {
+		case SDEV_EVT_MEDIA_CHANGE:
+			envp[idx++] = "SDEV_MEDIA_CHANGE=1";
+			break;
+		}
+	}
+	envp[idx++] = NULL;
+
+	kobject_uevent_env(&sdev->sdev_gendev.kobj, KOBJ_CHANGE, envp);
+}
+
+/**
+ * 	sdev_evt_notify - send asserted events to uevent thread
+ *	@sdev: scsi_device event occurred on
+ *	@map: the bitmapped list of asserted events (SDEV_EVT_xxx)
+ *
+ *	
+ */
+void sdev_evt_notify(struct scsi_device *sdev, const unsigned long *map)
+{
+	DECLARE_BITMAP(tmp_map, SDEV_EVT_MAXBITS);
+	unsigned long flags;
+
+	spin_lock_irqsave(&sdev->list_lock, flags);
+
+	bitmap_and(tmp_map, sdev->supported_events, map, SDEV_EVT_MAXBITS);
+
+	if (!bitmap_empty(tmp_map, SDEV_EVT_MAXBITS)) {
+		bitmap_or(sdev->event_mask, sdev->event_mask, tmp_map,
+			  SDEV_EVT_MAXBITS);
+
+		schedule_work(&sdev->event_work);
+	}
+
+	spin_unlock_irqrestore(&sdev->list_lock, flags);
+}
+EXPORT_SYMBOL_GPL(sdev_evt_notify);
+
+/**
  *	scsi_device_quiesce - Block user issued commands.
  *	@sdev:	scsi device to quiesce.
  *
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index b53c5f6..3632b62 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -236,6 +236,7 @@ static struct scsi_device *scsi_alloc_sdev(struct scsi_target *starget,
 	struct scsi_device *sdev;
 	int display_failure_msg = 1, ret;
 	struct Scsi_Host *shost = dev_to_shost(starget->dev.parent);
+	extern void scsi_evt_thread(struct work_struct *work);
 
 	sdev = kzalloc(sizeof(*sdev) + shost->transportt->device_size,
 		       GFP_ATOMIC);
@@ -255,6 +256,7 @@ static struct scsi_device *scsi_alloc_sdev(struct scsi_target *starget,
 	INIT_LIST_HEAD(&sdev->cmd_list);
 	INIT_LIST_HEAD(&sdev->starved_entry);
 	spin_lock_init(&sdev->list_lock);
+	INIT_WORK(&sdev->event_work, scsi_evt_thread);
 
 	sdev->sdev_gendev.parent = get_device(&starget->dev);
 	sdev->sdev_target = starget;
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index d531cee..9a1b0c5 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -282,6 +282,8 @@ static void scsi_device_dev_release_usercontext(struct work_struct *work)
 	list_del(&sdev->starved_entry);
 	spin_unlock_irqrestore(sdev->host->host_lock, flags);
 
+	cancel_work_sync(&sdev->event_work);
+
 	if (sdev->request_queue) {
 		sdev->request_queue->queuedata = NULL;
 		/* user context needed to free queue */
@@ -614,6 +616,23 @@ sdev_show_modalias(struct device *dev, struct device_attribute *attr, char *buf)
 }
 static DEVICE_ATTR(modalias, S_IRUGO, sdev_show_modalias, NULL);
 
+#define DECLARE_EVT_SHOW(name, Cap_name)				\
+static ssize_t								\
+sdev_show_##name(struct device *dev, struct device_attribute *attr,	\
+				char *buf)				\
+{									\
+	struct scsi_device *sdev = to_scsi_device(dev);			\
+	int val = test_bit(SDEV_EVT_##Cap_name, sdev->supported_events);\
+	return snprintf(buf, 20, "%d\n", val);				\
+}
+
+#define DECLARE_EVT(name, Cap_name) \
+	DECLARE_EVT_SHOW(name, Cap_name) \
+	static DEVICE_ATTR(evt_##name, S_IRUGO, sdev_show_##name, NULL);
+#define REF_EVT(name) &dev_attr_evt_##name.attr
+
+DECLARE_EVT(media_change, MEDIA_CHANGE)
+
 /* Default template for device attributes.  May NOT be modified */
 static struct attribute *scsi_sdev_attrs[] = {
 	&dev_attr_device_blocked.attr,
@@ -631,6 +650,7 @@ static struct attribute *scsi_sdev_attrs[] = {
 	&dev_attr_iodone_cnt.attr,
 	&dev_attr_ioerr_cnt.attr,
 	&dev_attr_modalias.attr,
+	REF_EVT(media_change),
 	NULL
 };
 
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index d5057bc..063827a 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -46,6 +46,13 @@ enum scsi_device_state {
 				 * to the scsi lld. */
 };
 
+enum scsi_device_event {
+	SDEV_EVT_MEDIA_CHANGE	= 1,	/* media has changed */
+
+	SDEV_EVT_LAST		= SDEV_EVT_MEDIA_CHANGE,
+	SDEV_EVT_MAXBITS	= SDEV_EVT_LAST + 1
+};
+
 struct scsi_device {
 	struct Scsi_Host *host;
 	struct request_queue *request_queue;
@@ -127,6 +134,10 @@ struct scsi_device {
 	unsigned guess_capacity:1;	/* READ_CAPACITY might be too high by 1 */
 	unsigned retry_hwerror:1;	/* Retry HARDWARE_ERROR */
 
+	DECLARE_BITMAP(supported_events, SDEV_EVT_MAXBITS); /* supported events */
+	DECLARE_BITMAP(event_mask, SDEV_EVT_MAXBITS); /* asserted events */
+	struct work_struct event_work;
+
 	unsigned int device_blocked;	/* Device returned QUEUE_FULL. */
 
 	unsigned int max_device_blocked; /* what device_blocked counts down from  */
@@ -275,6 +286,7 @@ extern int scsi_test_unit_ready(struct scsi_device *sdev, int timeout,
 				int retries);
 extern int scsi_device_set_state(struct scsi_device *sdev,
 				 enum scsi_device_state state);
+extern void sdev_evt_notify(struct scsi_device *sdev, const unsigned long *map);
 extern int scsi_device_quiesce(struct scsi_device *sdev);
 extern void scsi_device_resume(struct scsi_device *sdev);
 extern void scsi_target_quiesce(struct scsi_target *);
-- 
1.5.2.4

Re: [PATCH v4 1/2] SCSI: Asynchronous event notification infrastructure

[James Bottomley]

On Mon, 2007-10-29 at 10:42 -0400, Jeff Garzik wrote:
> Originally based on a patch by Kristen Carlson Accardi @ Intel.
> Copious input from James Bottomley.
> 
> Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
> ---
>  drivers/scsi/scsi_lib.c    |   66 ++++++++++++++++++++++++++++++++++++++++++++
>  drivers/scsi/scsi_scan.c   |    2 +
>  drivers/scsi/scsi_sysfs.c  |   20 +++++++++++++
>  include/scsi/scsi_device.h |   12 ++++++++
>  4 files changed, 100 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index 61fdaf0..f55ec80 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -18,6 +18,7 @@
>  #include <linux/delay.h>
>  #include <linux/hardirq.h>
>  #include <linux/scatterlist.h>
> +#include <linux/bitmap.h>
>  
>  #include <scsi/scsi.h>
>  #include <scsi/scsi_cmnd.h>
> @@ -2115,6 +2116,71 @@ scsi_device_set_state(struct scsi_device *sdev, enum scsi_device_state state)
>  EXPORT_SYMBOL(scsi_device_set_state);
>  
>  /**
> + * 	sdev_evt_thread - send a uevent for each scsi event
> + *	@work: work struct for scsi_device
> + *
> + *	Emit all queued media events as environment variables
> + *	sent during a uevent.
> + */
> +void scsi_evt_thread(struct work_struct *work)
> +{
> +	struct scsi_device *sdev;
> +	char *envp[SDEV_EVT_LAST + 2];
> +	DECLARE_BITMAP(mask, SDEV_EVT_MAXBITS);
> +	unsigned long flags;
> +	int evt, idx;
> +
> +	sdev = container_of(work, struct scsi_device, event_work);
> +
> +	spin_lock_irqsave(&sdev->list_lock, flags);
> +	bitmap_copy(mask, sdev->event_mask, SDEV_EVT_MAXBITS);
> +	bitmap_zero(sdev->event_mask, SDEV_EVT_MAXBITS);
> +	spin_unlock_irqrestore(&sdev->list_lock, flags);
> +
> +	idx = 0;
> +	for (evt = 0; evt < SDEV_EVT_LAST; evt++) {
> +		if (!test_bit(evt, mask))
> +			continue;
> +
> +		switch (evt) {
> +		case SDEV_EVT_MEDIA_CHANGE:
> +			envp[idx++] = "SDEV_MEDIA_CHANGE=1";
> +			break;
> +		}
> +	}
> +	envp[idx++] = NULL;
> +
> +	kobject_uevent_env(&sdev->sdev_gendev.kobj, KOBJ_CHANGE, envp);
> +}
> +
> +/**
> + * 	sdev_evt_notify - send asserted events to uevent thread
> + *	@sdev: scsi_device event occurred on
> + *	@map: the bitmapped list of asserted events (SDEV_EVT_xxx)
> + *
> + *	
> + */
> +void sdev_evt_notify(struct scsi_device *sdev, const unsigned long *map)
> +{
> +	DECLARE_BITMAP(tmp_map, SDEV_EVT_MAXBITS);
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&sdev->list_lock, flags);
> +
> +	bitmap_and(tmp_map, sdev->supported_events, map, SDEV_EVT_MAXBITS);
> +
> +	if (!bitmap_empty(tmp_map, SDEV_EVT_MAXBITS)) {
> +		bitmap_or(sdev->event_mask, sdev->event_mask, tmp_map,
> +			  SDEV_EVT_MAXBITS);
> +
> +		schedule_work(&sdev->event_work);
> +	}
> +
> +	spin_unlock_irqrestore(&sdev->list_lock, flags);

This still doesn't solve the fundamental corruption problem:
sdev->event_work has to contain the work entry until the workqueue has
finished executing it (which is some unspecified time in the future).
As soon as you drop the sdev->list_lock, the system thinks
sdev->event_work is available for reuse.  If we fire another event
before the work queue finished processing the prior event, the queue
will be corrupted.

Although I hate GFP_ATOMIC allocations, I think that's the only viable
way to get out of this corruption problem (using a mechanism similar to
what I proposed yesterday).

Also, I think Kristin's initial use of execute_in_user_context() was a
good call .. if we already have a user context, there's no need to
bother the workqueue ... some of these events will likely trigger from
thread backed kernel daemons.

James

Re: [PATCH v4 1/2] SCSI: Asynchronous event notification infrastructure

[Jeff Garzik]

James Bottomley wrote:
> This still doesn't solve the fundamental corruption problem:
> sdev->event_work has to contain the work entry until the workqueue has
> finished executing it (which is some unspecified time in the future).
> As soon as you drop the sdev->list_lock, the system thinks
> sdev->event_work is available for reuse.  If we fire another event
> before the work queue finished processing the prior event, the queue
> will be corrupted.

I think you're misunderstanding the workqueue code?  You can call 
schedule_work(&sdev->event_work) from anywhere, any time you like, as 
many times as you like.


> Also, I think Kristin's initial use of execute_in_user_context() was a
> good call .. if we already have a user context, there's no need to
> bother the workqueue ... some of these events will likely trigger from
> thread backed kernel daemons.

Quite agreed that sdev_evt_notify() might be called from kernel daemons, 
but in general this is a fire-and-forget API that is -likely- to be 
called from interrupt or completion context of many drivers, just like 
scsi_done or other completion APIs.  It is a fundamentally parallel 
interface.

If thread-backed kernel daemons want to use this, it is trivial for them 
to schedule work, then sync.

	Jeff

Re: [PATCH v4 1/2] SCSI: Asynchronous event notification infrastructure

[James Bottomley]

On Mon, 2007-10-29 at 12:07 -0400, Jeff Garzik wrote:
> James Bottomley wrote:
> > This still doesn't solve the fundamental corruption problem:
> > sdev->event_work has to contain the work entry until the workqueue has
> > finished executing it (which is some unspecified time in the future).
> > As soon as you drop the sdev->list_lock, the system thinks
> > sdev->event_work is available for reuse.  If we fire another event
> > before the work queue finished processing the prior event, the queue
> > will be corrupted.
> 
> I think you're misunderstanding the workqueue code?  You can call 
> schedule_work(&sdev->event_work) from anywhere, any time you like, as 
> many times as you like.

OK, take me through it slowly then ... I think schedule_work(work)
inserts work->entry onto the workqueue list (in
workqueue.c:insert_work()).  If the event hasn't fired, it will already
be on the list, so adding the same entry to a list twice causes a list
corruption problem.

Plus, unfortunately, the CC/UA events are going to have to carry extra
sense data; they're not simply going to be triggers saying something
happened.

James

Re: [PATCH v4 1/2] SCSI: Asynchronous event notification infrastructure

[Jeff Garzik]

James Bottomley wrote:
> On Mon, 2007-10-29 at 12:07 -0400, Jeff Garzik wrote:
>> James Bottomley wrote:
>>> This still doesn't solve the fundamental corruption problem:
>>> sdev->event_work has to contain the work entry until the workqueue has
>>> finished executing it (which is some unspecified time in the future).
>>> As soon as you drop the sdev->list_lock, the system thinks
>>> sdev->event_work is available for reuse.  If we fire another event
>>> before the work queue finished processing the prior event, the queue
>>> will be corrupted.
>> I think you're misunderstanding the workqueue code?  You can call 
>> schedule_work(&sdev->event_work) from anywhere, any time you like, as 
>> many times as you like.
> 
> OK, take me through it slowly then ... I think schedule_work(work)
> inserts work->entry onto the workqueue list (in
> workqueue.c:insert_work()).  If the event hasn't fired, it will already
> be on the list, so adding the same entry to a list twice causes a list
> corruption problem.

It does a test_and_set_bit() first thing in queue_work().  Similar 
exclusivity logic is found in net device land.  Ah, the fun of locking 
without locks that benh grumbles about :)


> Plus, unfortunately, the CC/UA events are going to have to carry extra
> sense data; they're not simply going to be triggers saying something
> happened.

OK this is a fair criticism.

If additional data must be carried, then I must ditch the beloved bitmap 
implementation and go back to a list (with associated GFP_ATOMIC alloc).

I will fix this, unless I receive email to the contrary...

	Jeff

Re: [PATCH v4 1/2] SCSI: Asynchronous event notification infrastructure

[James Bottomley]

On Mon, 2007-10-29 at 12:29 -0400, Jeff Garzik wrote:
> James Bottomley wrote:
> > On Mon, 2007-10-29 at 12:07 -0400, Jeff Garzik wrote:
> >> James Bottomley wrote:
> >>> This still doesn't solve the fundamental corruption problem:
> >>> sdev->event_work has to contain the work entry until the workqueue has
> >>> finished executing it (which is some unspecified time in the future).
> >>> As soon as you drop the sdev->list_lock, the system thinks
> >>> sdev->event_work is available for reuse.  If we fire another event
> >>> before the work queue finished processing the prior event, the queue
> >>> will be corrupted.
> >> I think you're misunderstanding the workqueue code?  You can call 
> >> schedule_work(&sdev->event_work) from anywhere, any time you like, as 
> >> many times as you like.
> > 
> > OK, take me through it slowly then ... I think schedule_work(work)
> > inserts work->entry onto the workqueue list (in
> > workqueue.c:insert_work()).  If the event hasn't fired, it will already
> > be on the list, so adding the same entry to a list twice causes a list
> > corruption problem.
> 
> It does a test_and_set_bit() first thing in queue_work().  Similar 
> exclusivity logic is found in net device land.  Ah, the fun of locking 
> without locks that benh grumbles about :)

Ah, OK, sorry ... I was actually looking at __queue_work().

> > Plus, unfortunately, the CC/UA events are going to have to carry extra
> > sense data; they're not simply going to be triggers saying something
> > happened.
> 
> OK this is a fair criticism.
> 
> If additional data must be carried, then I must ditch the beloved bitmap 
> implementation and go back to a list (with associated GFP_ATOMIC alloc).
> 
> I will fix this, unless I receive email to the contrary...

Yes, unfortunately, thanks.  If all events were a simple number, it's
easy, but the CC/UA events carry data as well.

James

[PATCH v5 0/2] SCSI asynchronous event notification API

[Jeff Garzik]

This is the next revision of the SCSI event notification infrastructure
patchset, enabling SATA Asynchronous Notification ("AN") for CD/DVD
devices that support it.

For devices that support SATA AN (only very recent ones do), this means
that HAL and other userspace utilities no longer need to repeatedly poll
the CD/DVD device to determine if the user has changed the media.

This revision takes into account James' comments from earlier today,
up to patchset revision #4 (this one is revision 5).

Changes since last revision:

1) sysfs 'store' operation is implemented, to eliminate any confusion
   about how it should work.

2) if we are going to emit events more complex than a single "I'm
   asserted" bit, then we cannot use the nifty bitmap.h stuff.

a) struct scsi_event is created.

b) sdev_evt_send() -- send an event.  event intentionally and
   silently dropped, if not in supported_events mask.

c) sdev_evt_alloc() -- allocate an event.  structured such that
   future event types can be added easily.

d) sdev_evt_send_simple() -- for simple events where we only need
   to know the event type, we can use this simple interface (libata uses
   this).

e) Events are sent one-at-a-time.  Unlike the bitmap, complex events
   should not be merged together.

[PATCH v5 1/2] SCSI: add asynchronous event notification API

[Jeff Garzik]

Originally based on a patch by Kristen Carlson Accardi @ Intel.
Copious input from James Bottomley.

Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
---
 drivers/scsi/scsi_lib.c    |  136 ++++++++++++++++++++++++++++++++++++++++++++
 drivers/scsi/scsi_scan.c   |    3 +
 drivers/scsi/scsi_sysfs.c  |   47 +++++++++++++++
 include/scsi/scsi_device.h |   25 ++++++++
 4 files changed, 211 insertions(+), 0 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 61fdaf0..304213e 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -2115,6 +2115,142 @@ scsi_device_set_state(struct scsi_device *sdev, enum scsi_device_state state)
 EXPORT_SYMBOL(scsi_device_set_state);
 
 /**
+ * 	sdev_evt_emit - emit a single SCSI device uevent
+ *	@sdev: associated SCSI device
+ *	@evt: event to emit
+ *
+ *	Send a single uevent (scsi_event) to the associated scsi_device.
+ */
+static void scsi_evt_emit(struct scsi_device *sdev, struct scsi_event *evt)
+{
+	int idx = 0;
+	char *envp[3];
+
+	switch (evt->evt_type) {
+	case SDEV_EVT_MEDIA_CHANGE:
+		envp[idx++] = "SDEV_MEDIA_CHANGE=1";
+		break;
+
+	default:
+		/* do nothing */
+		break;
+	}
+
+	envp[idx++] = NULL;
+
+	kobject_uevent_env(&sdev->sdev_gendev.kobj, KOBJ_CHANGE, envp);
+}
+
+/**
+ * 	sdev_evt_thread - send a uevent for each scsi event
+ *	@work: work struct for scsi_device
+ *
+ *	Dispatch queued events to their associated scsi_device kobjects
+ *	as uevents.
+ */
+void scsi_evt_thread(struct work_struct *work)
+{
+	struct scsi_device *sdev;
+	LIST_HEAD(event_list);
+
+	sdev = container_of(work, struct scsi_device, event_work);
+
+	while (1) {
+		struct scsi_event *evt;
+		struct list_head *this, *tmp;
+		unsigned long flags;
+
+		spin_lock_irqsave(&sdev->list_lock, flags);
+		list_splice_init(&sdev->event_list, &event_list);
+		spin_unlock_irqrestore(&sdev->list_lock, flags);
+
+		if (list_empty(&event_list))
+			break;
+
+		list_for_each_safe(this, tmp, &event_list) {
+			evt = list_entry(this, struct scsi_event, node);
+			list_del(&evt->node);
+			scsi_evt_emit(sdev, evt);
+			kfree(evt);
+		}
+	}
+}
+
+/**
+ * 	sdev_evt_send - send asserted event to uevent thread
+ *	@sdev: scsi_device event occurred on
+ *	@evt: event to send
+ *
+ *	Assert scsi device event asynchronously.
+ */
+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);
+	spin_unlock_irqrestore(&sdev->list_lock, flags);
+}
+EXPORT_SYMBOL_GPL(sdev_evt_send);
+
+/**
+ * 	sdev_evt_alloc - allocate a new scsi event
+ *	@evt_type: type of event to allocate
+ *	@gfpflags: GFP flags for allocation
+ *
+ *	Allocates and returns a new scsi_event.
+ */
+struct scsi_event *sdev_evt_alloc(enum scsi_device_event evt_type,
+				  gfp_t gfpflags)
+{
+	struct scsi_event *evt = kzalloc(sizeof(struct scsi_event), gfpflags);
+	if (!evt)
+		return NULL;
+
+	evt->evt_type = evt_type;
+	INIT_LIST_HEAD(&evt->node);
+
+	/* evt_type-specific initialization, if any */
+	switch (evt_type) {
+	case SDEV_EVT_MEDIA_CHANGE:
+	default:
+		/* do nothing */
+		break;
+	}
+
+	return evt;
+}
+EXPORT_SYMBOL_GPL(sdev_evt_alloc);
+
+/**
+ * 	sdev_evt_send_simple - send asserted event to uevent thread
+ *	@sdev: scsi_device event occurred on
+ *	@evt_type: type of event to send
+ *	@gfpflags: GFP flags for allocation
+ *
+ *	Assert scsi device event asynchronously, given an event type.
+ */
+void sdev_evt_send_simple(struct scsi_device *sdev,
+			  enum scsi_device_event evt_type, gfp_t gfpflags)
+{
+	struct scsi_event *evt = sdev_evt_alloc(evt_type, gfpflags);
+	if (!evt) {
+		sdev_printk(KERN_ERR, sdev, "event %d eaten due to OOM\n",
+			    evt_type);
+		return;
+	}
+
+	sdev_evt_send(sdev, evt);
+}
+EXPORT_SYMBOL_GPL(sdev_evt_send_simple);
+
+/**
  *	scsi_device_quiesce - Block user issued commands.
  *	@sdev:	scsi device to quiesce.
  *
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index b53c5f6..40ea71c 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -236,6 +236,7 @@ static struct scsi_device *scsi_alloc_sdev(struct scsi_target *starget,
 	struct scsi_device *sdev;
 	int display_failure_msg = 1, ret;
 	struct Scsi_Host *shost = dev_to_shost(starget->dev.parent);
+	extern void scsi_evt_thread(struct work_struct *work);
 
 	sdev = kzalloc(sizeof(*sdev) + shost->transportt->device_size,
 		       GFP_ATOMIC);
@@ -254,7 +255,9 @@ static struct scsi_device *scsi_alloc_sdev(struct scsi_target *starget,
 	INIT_LIST_HEAD(&sdev->same_target_siblings);
 	INIT_LIST_HEAD(&sdev->cmd_list);
 	INIT_LIST_HEAD(&sdev->starved_entry);
+	INIT_LIST_HEAD(&sdev->event_list);
 	spin_lock_init(&sdev->list_lock);
+	INIT_WORK(&sdev->event_work, scsi_evt_thread);
 
 	sdev->sdev_gendev.parent = get_device(&starget->dev);
 	sdev->sdev_target = starget;
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index d531cee..f374fdc 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -268,6 +268,7 @@ static void scsi_device_dev_release_usercontext(struct work_struct *work)
 	struct scsi_device *sdev;
 	struct device *parent;
 	struct scsi_target *starget;
+	struct list_head *this, *tmp;
 	unsigned long flags;
 
 	sdev = container_of(work, struct scsi_device, ew.work);
@@ -282,6 +283,16 @@ static void scsi_device_dev_release_usercontext(struct work_struct *work)
 	list_del(&sdev->starved_entry);
 	spin_unlock_irqrestore(sdev->host->host_lock, flags);
 
+	cancel_work_sync(&sdev->event_work);
+
+	list_for_each_safe(this, tmp, &sdev->event_list) {
+		struct scsi_event *evt;
+
+		evt = list_entry(this, struct scsi_event, node);
+		list_del(&evt->node);
+		kfree(evt);
+	}
+
 	if (sdev->request_queue) {
 		sdev->request_queue->queuedata = NULL;
 		/* user context needed to free queue */
@@ -614,6 +625,41 @@ sdev_show_modalias(struct device *dev, struct device_attribute *attr, char *buf)
 }
 static DEVICE_ATTR(modalias, S_IRUGO, sdev_show_modalias, NULL);
 
+#define DECLARE_EVT_SHOW(name, Cap_name)				\
+static ssize_t								\
+sdev_show_evt_##name(struct device *dev, struct device_attribute *attr,	\
+				char *buf)				\
+{									\
+	struct scsi_device *sdev = to_scsi_device(dev);			\
+	int val = test_bit(SDEV_EVT_##Cap_name, sdev->supported_events);\
+	return snprintf(buf, 20, "%d\n", val);				\
+}
+
+#define DECLARE_EVT_STORE(name, Cap_name)				\
+static ssize_t								\
+sdev_store_evt_##name(struct device *dev, struct device_attribute *attr, \
+		      const char *buf, size_t count)			\
+{									\
+	struct scsi_device *sdev = to_scsi_device(dev);			\
+	int val = simple_strtoul(buf, NULL, 0);				\
+	if (val == 0)							\
+		clear_bit(SDEV_EVT_##Cap_name, sdev->supported_events);	\
+	else if (val == 1)						\
+		set_bit(SDEV_EVT_##Cap_name, sdev->supported_events);	\
+	else								\
+		return -EINVAL;						\
+	return count;							\
+}
+
+#define DECLARE_EVT(name, Cap_name)					\
+	DECLARE_EVT_SHOW(name, Cap_name)				\
+	DECLARE_EVT_STORE(name, Cap_name)				\
+	static DEVICE_ATTR(evt_##name, S_IRUGO, sdev_show_evt_##name,	\
+			   sdev_store_evt_##name);
+#define REF_EVT(name) &dev_attr_evt_##name.attr
+
+DECLARE_EVT(media_change, MEDIA_CHANGE)
+
 /* Default template for device attributes.  May NOT be modified */
 static struct attribute *scsi_sdev_attrs[] = {
 	&dev_attr_device_blocked.attr,
@@ -631,6 +677,7 @@ static struct attribute *scsi_sdev_attrs[] = {
 	&dev_attr_iodone_cnt.attr,
 	&dev_attr_ioerr_cnt.attr,
 	&dev_attr_modalias.attr,
+	REF_EVT(media_change),
 	NULL
 };
 
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index d5057bc..66e9058 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -46,6 +46,22 @@ enum scsi_device_state {
 				 * to the scsi lld. */
 };
 
+enum scsi_device_event {
+	SDEV_EVT_MEDIA_CHANGE	= 1,	/* media has changed */
+
+	SDEV_EVT_LAST		= SDEV_EVT_MEDIA_CHANGE,
+	SDEV_EVT_MAXBITS	= SDEV_EVT_LAST + 1
+};
+
+struct scsi_event {
+	enum scsi_device_event	evt_type;
+	struct list_head	node;
+
+	/* put union of data structures, for non-simple event types,
+	 * here
+	 */
+};
+
 struct scsi_device {
 	struct Scsi_Host *host;
 	struct request_queue *request_queue;
@@ -127,6 +143,10 @@ struct scsi_device {
 	unsigned guess_capacity:1;	/* READ_CAPACITY might be too high by 1 */
 	unsigned retry_hwerror:1;	/* Retry HARDWARE_ERROR */
 
+	DECLARE_BITMAP(supported_events, SDEV_EVT_MAXBITS); /* supported events */
+	struct list_head event_list;	/* asserted events */
+	struct work_struct event_work;
+
 	unsigned int device_blocked;	/* Device returned QUEUE_FULL. */
 
 	unsigned int max_device_blocked; /* what device_blocked counts down from  */
@@ -275,6 +295,11 @@ extern int scsi_test_unit_ready(struct scsi_device *sdev, int timeout,
 				int retries);
 extern int scsi_device_set_state(struct scsi_device *sdev,
 				 enum scsi_device_state state);
+extern struct scsi_event *sdev_evt_alloc(enum scsi_device_event evt_type,
+					  gfp_t gfpflags);
+extern void sdev_evt_send(struct scsi_device *sdev, struct scsi_event *evt);
+extern void sdev_evt_send_simple(struct scsi_device *sdev,
+			  enum scsi_device_event evt_type, gfp_t gfpflags);
 extern int scsi_device_quiesce(struct scsi_device *sdev);
 extern void scsi_device_resume(struct scsi_device *sdev);
 extern void scsi_target_quiesce(struct scsi_target *);
-- 
1.5.2.4

[PATCH v5 2/2] libata: Utilize new SCSI event infrastructure

[Jeff Garzik]

An end to CD-ROM polling (if you have a device that supports AN)...
hooray!

Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
---
 drivers/ata/libata-scsi.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 93bd36c..3cbee22 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -841,6 +841,9 @@ static void ata_scsi_dev_config(struct scsi_device *sdev,
 		blk_queue_max_hw_segments(q, q->max_hw_segments - 1);
 	}
 
+	if (dev->flags & ATA_DFLAG_AN)
+		set_bit(SDEV_EVT_MEDIA_CHANGE, sdev->supported_events);
+
 	if (dev->flags & ATA_DFLAG_NCQ) {
 		int depth;
 
@@ -3293,10 +3296,9 @@ static void ata_scsi_handle_link_detach(struct ata_link *link)
  */
 void ata_scsi_media_change_notify(struct ata_device *dev)
 {
-#ifdef OTHER_AN_PATCHES_HAVE_BEEN_APPLIED
 	if (dev->sdev)
-		scsi_device_event_notify(dev->sdev, SDEV_MEDIA_CHANGE);
-#endif
+		sdev_evt_send_simple(dev->sdev, SDEV_EVT_MEDIA_CHANGE,
+				     GFP_ATOMIC);
 }
 
 /**
-- 
1.5.2.4

[PATCH v4 2/2] libata: Utilize new SCSI event infrastructure

[Jeff Garzik]

An end to CD-ROM polling (if you have a device that supports AN)...
hooray!

Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
---
 drivers/ata/libata-scsi.c |   12 ++++++++----
 1 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index f752edd..e6d5627 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -773,6 +773,9 @@ static void ata_scsi_dev_config(struct scsi_device *sdev,
 		blk_queue_max_hw_segments(q, q->max_hw_segments - 1);
 	}
 
+	if (dev->flags & ATA_DFLAG_AN)
+		set_bit(SDEV_EVT_MEDIA_CHANGE, sdev->supported_events);
+
 	if (dev->flags & ATA_DFLAG_NCQ) {
 		int depth;
 
@@ -3225,10 +3228,11 @@ static void ata_scsi_handle_link_detach(struct ata_link *link)
  */
 void ata_scsi_media_change_notify(struct ata_device *dev)
 {
-#ifdef OTHER_AN_PATCHES_HAVE_BEEN_APPLIED
-	if (dev->sdev)
-		scsi_device_event_notify(dev->sdev, SDEV_MEDIA_CHANGE);
-#endif
+	if (dev->sdev) {
+		DECLARE_BITMAP(map, SDEV_EVT_MAXBITS);
+		__set_bit(SDEV_EVT_MEDIA_CHANGE, map);
+		sdev_evt_notify(dev->sdev, map);
+	}
 }
 
 /**
-- 
1.5.2.4

Re: [PATCH v4 0/2] [SCSI] Asynchronous event notification infrastructure

[James Bottomley]

On Mon, 2007-10-29 at 10:42 -0400, Jeff Garzik wrote:
> This is the next revision of the SCSI event notification infrastructure
> patchset, enabling SATA Asynchronous Notification ("AN") for CD/DVD
> devices that support it.
> 
> For devices that support SATA AN (only very recent ones do), this means
> that HAL and other userspace utilities no longer need to repeatedly poll
> the CD/DVD device to determine if the user has changed the media.
> 
> This revision takes into account James' comments from earlier today,
> modulo the following notes:
> 
> * I think the various event attributes should always be present,
>   for all devices at all times.  If various events are not supported,
>   the attribute will of course return zero (false, not supported).

Actually, I don't think so.  We have precedent for this in the transport
classes: if a device doesn't support a feature, we don't export the flag
for that feature through sysfs.  This allows not only feature control,
but an immediate view of the device capabilities simply by viewing the
sysfs directory.

I think this functionality is very easy to layer in, so there's no
reason not to do it.

> * I do not think this work should be blocked behind a revamp
>   of the attribute group interface.

Assuming gregkh or kay ack, it won't be.

> * I was slack and did not bother to implement the 'set' operation
>   for the attributes.  This can easily be done at a later time in a
>   separate patch.  It is not a merge stopper to have the driver
>   exclusively control the event mask, rather than driver+sysfs.

James

Re: [PATCH v4 0/2] [SCSI] Asynchronous event notification infrastructure

[Jeff Garzik]

James Bottomley wrote:
> On Mon, 2007-10-29 at 10:42 -0400, Jeff Garzik wrote:
>> This is the next revision of the SCSI event notification infrastructure
>> patchset, enabling SATA Asynchronous Notification ("AN") for CD/DVD
>> devices that support it.
>>
>> For devices that support SATA AN (only very recent ones do), this means
>> that HAL and other userspace utilities no longer need to repeatedly poll
>> the CD/DVD device to determine if the user has changed the media.
>>
>> This revision takes into account James' comments from earlier today,
>> modulo the following notes:
>>
>> * I think the various event attributes should always be present,
>>   for all devices at all times.  If various events are not supported,
>>   the attribute will of course return zero (false, not supported).
> 
> Actually, I don't think so.  We have precedent for this in the transport
> classes: if a device doesn't support a feature, we don't export the flag
> for that feature through sysfs.  This allows not only feature control,
> but an immediate view of the device capabilities simply by viewing the
> sysfs directory.

Think about about the values being exported by these sysfs attributes: 
they indicate whether or not that feature is supported.

Thus, using the presence/absence of an attribute to communicate the same 
thing would be redundant.

This suggestion adds a whole lot of complexity -- mirroring every change 
to sdev->supported_events by dynamically adding or removing attributes.

The current nice, simple, elegant bitops-based interface is suddenly a 
lot more cumbersome if forced to deal with attribute creation and disposal.

Finally, this additional complexity of dynamic attribute management also 
eliminates some key information:  userland can test the existence of the 
attribute to determine if that support is present in the kernel.

	Jeff

Re: [PATCH v4 0/2] [SCSI] Asynchronous event notification infrastructure

[James Bottomley]

On Mon, 2007-10-29 at 11:58 -0400, Jeff Garzik wrote:
> James Bottomley wrote:
> > On Mon, 2007-10-29 at 10:42 -0400, Jeff Garzik wrote:
> >> This is the next revision of the SCSI event notification infrastructure
> >> patchset, enabling SATA Asynchronous Notification ("AN") for CD/DVD
> >> devices that support it.
> >>
> >> For devices that support SATA AN (only very recent ones do), this means
> >> that HAL and other userspace utilities no longer need to repeatedly poll
> >> the CD/DVD device to determine if the user has changed the media.
> >>
> >> This revision takes into account James' comments from earlier today,
> >> modulo the following notes:
> >>
> >> * I think the various event attributes should always be present,
> >>   for all devices at all times.  If various events are not supported,
> >>   the attribute will of course return zero (false, not supported).
> > 
> > Actually, I don't think so.  We have precedent for this in the transport
> > classes: if a device doesn't support a feature, we don't export the flag
> > for that feature through sysfs.  This allows not only feature control,
> > but an immediate view of the device capabilities simply by viewing the
> > sysfs directory.
> 
> Think about about the values being exported by these sysfs attributes: 
> they indicate whether or not that feature is supported.

Ah, OK; I haven't communicated what we need very clearly.  We need a way
to see if the event is supported by the device, as well as a way to turn
it off.  For some of the events (possibly not the SATA AN one, since I
know all SATA devices will be well behaved) there's going to be a need
to deal with berserk or broken devices that become trigger happy, so
turning off the event will be a useful (and possibly essential) way of
coping.

> Thus, using the presence/absence of an attribute to communicate the same 
> thing would be redundant.
> 
> This suggestion adds a whole lot of complexity -- mirroring every change 
> to sdev->supported_events by dynamically adding or removing attributes.
> 
> The current nice, simple, elegant bitops-based interface is suddenly a 
> lot more cumbersome if forced to deal with attribute creation and disposal.
> 
> Finally, this additional complexity of dynamic attribute management also 
> eliminates some key information:  userland can test the existence of the 
> attribute to determine if that support is present in the kernel.

James

Re: [PATCH v4 0/2] [SCSI] Asynchronous event notification infrastructure

[Jeff Garzik]

James Bottomley wrote:
> Ah, OK; I haven't communicated what we need very clearly.  We need a way
> to see if the event is supported by the device, as well as a way to turn
> it off.  For some of the events (possibly not the SATA AN one, since I
> know all SATA devices will be well behaved) there's going to be a need
> to deal with berserk or broken devices that become trigger happy, so
> turning off the event will be a useful (and possibly essential) way of
> coping.


That's possible with the presented interface[1]:

	# see if event is supported
	cat $path/evt_media_change

	# turn off event to deal with broken/beserk devices
	echo 0 > $path/evt_media_change

Some sillyhead can always do

	echo 1 > $path/evt_some_event_my_device_does_not_support

but that will be obviously be a no-op because their device simply will 
not send such events.

Granted ls(1) is no longer a method for viewing supported-at-boot-time 
list of events -- ls(1) in the presented interface lists what events the 
_kernel_ supports, and cat(1) is used to discover which events are 
actually enabled.

I think that is the only difference between our two positions:  [if I 
understand you correctly] you want ls(1) to be able to list the device's 
supported events.  However, I feel that is inconsistent:  for your 
proposal, userspace must perform two checks in order to determine a 
feature's availability: 1) does the file exist? 2) is the file context 
non-zero?

Regards,

	Jeff


[1] modulo my comment from the original email in this thread:
> * I was slack and did not bother to implement the 'set' operation
>   for the attributes.  This can easily be done at a later time in a
>   separate patch.

Re: [PATCH v4 0/2] [SCSI] Asynchronous event notification infrastructure

[James Bottomley]

On Mon, 2007-10-29 at 12:24 -0400, Jeff Garzik wrote:
> James Bottomley wrote:
> > Ah, OK; I haven't communicated what we need very clearly.  We need a way
> > to see if the event is supported by the device, as well as a way to turn
> > it off.  For some of the events (possibly not the SATA AN one, since I
> > know all SATA devices will be well behaved) there's going to be a need
> > to deal with berserk or broken devices that become trigger happy, so
> > turning off the event will be a useful (and possibly essential) way of
> > coping.
> 
> 
> That's possible with the presented interface[1]:
> 
> 	# see if event is supported
> 	cat $path/evt_media_change
> 
> 	# turn off event to deal with broken/beserk devices
> 	echo 0 > $path/evt_media_change
> 
> Some sillyhead can always do
> 
> 	echo 1 > $path/evt_some_event_my_device_does_not_support
> 
> but that will be obviously be a no-op because their device simply will 
> not send such events.
> 
> Granted ls(1) is no longer a method for viewing supported-at-boot-time 
> list of events -- ls(1) in the presented interface lists what events the 
> _kernel_ supports, and cat(1) is used to discover which events are 
> actually enabled.
> 
> I think that is the only difference between our two positions:  [if I 
> understand you correctly] you want ls(1) to be able to list the device's 
> supported events.  However, I feel that is inconsistent:  for your 
> proposal, userspace must perform two checks in order to determine a 
> feature's availability: 1) does the file exist? 2) is the file context 
> non-zero?

Yes, I agree ... however, open file is one op for the user -ENXIO means
device doesn't support the event; value indicates whether the event is
currently triggering.

I just would rather we use the file exists if device supports event,
because it's consistent with all the rest of our SCSI interfaces.

> Regards,
> 
> 	Jeff
> 
> 
> [1] modulo my comment from the original email in this thread:
> > * I was slack and did not bother to implement the 'set' operation
> >   for the attributes.  This can easily be done at a later time in a
> >   separate patch.

James

Re: [PATCH v4 0/2] [SCSI] Asynchronous event notification infrastructure

[Jeff Garzik]

James Bottomley wrote:
> On Mon, 2007-10-29 at 12:24 -0400, Jeff Garzik wrote:
>> James Bottomley wrote:
>>> Ah, OK; I haven't communicated what we need very clearly.  We need a way
>>> to see if the event is supported by the device, as well as a way to turn
>>> it off.  For some of the events (possibly not the SATA AN one, since I
>>> know all SATA devices will be well behaved) there's going to be a need
>>> to deal with berserk or broken devices that become trigger happy, so
>>> turning off the event will be a useful (and possibly essential) way of
>>> coping.
>>
>> That's possible with the presented interface[1]:
>>
>> 	# see if event is supported
>> 	cat $path/evt_media_change
>>
>> 	# turn off event to deal with broken/beserk devices
>> 	echo 0 > $path/evt_media_change
>>
>> Some sillyhead can always do
>>
>> 	echo 1 > $path/evt_some_event_my_device_does_not_support
>>
>> but that will be obviously be a no-op because their device simply will 
>> not send such events.
>>
>> Granted ls(1) is no longer a method for viewing supported-at-boot-time 
>> list of events -- ls(1) in the presented interface lists what events the 
>> _kernel_ supports, and cat(1) is used to discover which events are 
>> actually enabled.
>>
>> I think that is the only difference between our two positions:  [if I 
>> understand you correctly] you want ls(1) to be able to list the device's 
>> supported events.  However, I feel that is inconsistent:  for your 
>> proposal, userspace must perform two checks in order to determine a 
>> feature's availability: 1) does the file exist? 2) is the file context 
>> non-zero?
> 
> Yes, I agree ... however, open file is one op for the user -ENXIO means
> device doesn't support the event; value indicates whether the event is
> currently triggering.
> 
> I just would rather we use the file exists if device supports event,
> because it's consistent with all the rest of our SCSI interfaces.

Two problems with what you just described:

1) "value indicates current event state" is a new concept in this thread 
(maybe you were thinking this all along, but I didn't get that from your 
writing).

Watching the sysfs node for event activity is definitely outside the 
scope of this work, and IMO not very useful.  The time from when LLDD 
calls sdev_evt_notify() until uevent completion is very short, so the 
time window for actually receiving a useful value in your scenario is 
also short.

My patch presented the attributes purely as control nodes, only affected 
sdev->supported_events and nothing more.  You seem to be suggesting 
exporting the true-for-only-a-few-milliseconds activity state, rather 
then enable/disable state.


2) Event support itself is dynamic, which causes me to revisit the 
"complexity" argument.  In libata, for example, we only note that the 
media-change event is supported after some time passes -- not in the 
initial slave_config.  Or error handler may disable it at runtime 
because that event is problematic.

As such, that implies that the LLDD (with help from scsi_lib) is 
dynamically adding and removing these attributes at runtime -- a lot 
more complexity than is really needed AFAICS.

It is easy and straightforward for the driver to set a bit.

We cannot assume the state of event support bits are constant from 
modprobe/slave_config time.

	Jeff