Send an uevent to user space to indicate that a media change event has occurred.
Signed-off-by: Kristen Carlson Accardi <[email protected]>
Index: 2.6-git/block/genhd.c
===================================================================
--- 2.6-git.orig/block/genhd.c
+++ 2.6-git/block/genhd.c
@@ -643,6 +643,27 @@ struct seq_operations diskstats_op = {
.show = diskstats_show
};
+static void media_change_notify_thread(struct work_struct *work)
+{
+ struct gendisk *gd = container_of(work, struct gendisk, async_notify);
+ char event[] = "MEDIA_CHANGE=1";
+ char *envp[] = { event, NULL };
+
+ /*
+ * set enviroment vars to indicate which event this is for
+ * so that user space will know to go check the media status.
+ */
+ kobject_uevent_env(&gd->kobj, KOBJ_CHANGE, envp);
+ put_device(gd->driverfs_dev);
+}
+
+void genhd_media_change_notify(struct gendisk *disk)
+{
+ get_device(disk->driverfs_dev);
+ schedule_work(&disk->async_notify);
+}
+EXPORT_SYMBOL_GPL(genhd_media_change_notify);
+
struct gendisk *alloc_disk(int minors)
{
return alloc_disk_node(minors, -1);
@@ -672,6 +693,8 @@ struct gendisk *alloc_disk_node(int mino
kobj_set_kset_s(disk,block_subsys);
kobject_init(&disk->kobj);
rand_initialize_disk(disk);
+ INIT_WORK(&disk->async_notify,
+ media_change_notify_thread);
}
return disk;
}
Index: 2.6-git/include/linux/genhd.h
===================================================================
--- 2.6-git.orig/include/linux/genhd.h
+++ 2.6-git/include/linux/genhd.h
@@ -66,6 +66,7 @@ struct partition {
#include <linux/smp.h>
#include <linux/string.h>
#include <linux/fs.h>
+#include <linux/workqueue.h>
struct partition {
unsigned char boot_ind; /* 0x80 - active */
@@ -139,6 +140,7 @@ struct gendisk {
#else
struct disk_stats dkstats;
#endif
+ struct work_struct async_notify;
};
/* Structure for sysfs attributes on block devices */
@@ -419,7 +421,7 @@ extern struct gendisk *alloc_disk_node(i
extern struct gendisk *alloc_disk(int minors);
extern struct kobject *get_disk(struct gendisk *disk);
extern void put_disk(struct gendisk *disk);
-
+extern void genhd_media_change_notify(struct gendisk *disk);
extern void blk_register_region(dev_t dev, unsigned long range,
struct module *module,
struct kobject *(*probe)(dev_t, int *, void *),
--
On Wed, 9 May 2007 16:38:35 -0700
Kristen Carlson Accardi <[email protected]> wrote:
> Send an uevent to user space to indicate that a media change event has occurred.
>
> Signed-off-by: Kristen Carlson Accardi <[email protected]>
>
> Index: 2.6-git/block/genhd.c
> ===================================================================
> --- 2.6-git.orig/block/genhd.c
> +++ 2.6-git/block/genhd.c
> @@ -643,6 +643,27 @@ struct seq_operations diskstats_op = {
> .show = diskstats_show
> };
>
> +static void media_change_notify_thread(struct work_struct *work)
> +{
> + struct gendisk *gd = container_of(work, struct gendisk, async_notify);
> + char event[] = "MEDIA_CHANGE=1";
> + char *envp[] = { event, NULL };
> +
> + /*
> + * set enviroment vars to indicate which event this is for
> + * so that user space will know to go check the media status.
> + */
> + kobject_uevent_env(&gd->kobj, KOBJ_CHANGE, envp);
> + put_device(gd->driverfs_dev);
> +}
> +
> +void genhd_media_change_notify(struct gendisk *disk)
> +{
> + get_device(disk->driverfs_dev);
> + schedule_work(&disk->async_notify);
> +}
> +EXPORT_SYMBOL_GPL(genhd_media_change_notify);
> +
> struct gendisk *alloc_disk(int minors)
> {
> return alloc_disk_node(minors, -1);
> @@ -672,6 +693,8 @@ struct gendisk *alloc_disk_node(int mino
> kobj_set_kset_s(disk,block_subsys);
> kobject_init(&disk->kobj);
> rand_initialize_disk(disk);
> + INIT_WORK(&disk->async_notify,
> + media_change_notify_thread);
> }
> return disk;
Why does this do a schedule_work() rather than calling kobject_uevent_env()
directly?
On Tue, 22 May 2007 14:12:11 -0700
Andrew Morton <[email protected]> wrote:
> On Wed, 9 May 2007 16:38:35 -0700
> Kristen Carlson Accardi <[email protected]> wrote:
>
> > Send an uevent to user space to indicate that a media change event has occurred.
> >
> > Signed-off-by: Kristen Carlson Accardi <[email protected]>
> >
> > Index: 2.6-git/block/genhd.c
> > ===================================================================
> > --- 2.6-git.orig/block/genhd.c
> > +++ 2.6-git/block/genhd.c
> > @@ -643,6 +643,27 @@ struct seq_operations diskstats_op = {
> > .show = diskstats_show
> > };
> >
> > +static void media_change_notify_thread(struct work_struct *work)
> > +{
> > + struct gendisk *gd = container_of(work, struct gendisk, async_notify);
> > + char event[] = "MEDIA_CHANGE=1";
> > + char *envp[] = { event, NULL };
> > +
> > + /*
> > + * set enviroment vars to indicate which event this is for
> > + * so that user space will know to go check the media status.
> > + */
> > + kobject_uevent_env(&gd->kobj, KOBJ_CHANGE, envp);
> > + put_device(gd->driverfs_dev);
> > +}
> > +
> > +void genhd_media_change_notify(struct gendisk *disk)
> > +{
> > + get_device(disk->driverfs_dev);
> > + schedule_work(&disk->async_notify);
> > +}
> > +EXPORT_SYMBOL_GPL(genhd_media_change_notify);
> > +
> > struct gendisk *alloc_disk(int minors)
> > {
> > return alloc_disk_node(minors, -1);
> > @@ -672,6 +693,8 @@ struct gendisk *alloc_disk_node(int mino
> > kobj_set_kset_s(disk,block_subsys);
> > kobject_init(&disk->kobj);
> > rand_initialize_disk(disk);
> > + INIT_WORK(&disk->async_notify,
> > + media_change_notify_thread);
> > }
> > return disk;
>
> Why does this do a schedule_work() rather than calling kobject_uevent_env()
> directly?
>
Because it is called at Interrupt time.
Kristen
On Wed, 2007-05-23 at 09:31 -0700, Kristen Carlson Accardi wrote:
> On Tue, 22 May 2007 14:12:11 -0700
> Andrew Morton <[email protected]> wrote:
>
> > On Wed, 9 May 2007 16:38:35 -0700
> > Kristen Carlson Accardi <[email protected]> wrote:
> >
> > > Send an uevent to user space to indicate that a media change event has occurred.
> > >
> > > Signed-off-by: Kristen Carlson Accardi <[email protected]>
> > >
> > > Index: 2.6-git/block/genhd.c
> > > ===================================================================
> > > --- 2.6-git.orig/block/genhd.c
> > > +++ 2.6-git/block/genhd.c
> > > @@ -643,6 +643,27 @@ struct seq_operations diskstats_op = {
> > > .show = diskstats_show
> > > };
> > >
> > > +static void media_change_notify_thread(struct work_struct *work)
> > > +{
> > > + struct gendisk *gd = container_of(work, struct gendisk, async_notify);
> > > + char event[] = "MEDIA_CHANGE=1";
> > > + char *envp[] = { event, NULL };
> > > +
> > > + /*
> > > + * set enviroment vars to indicate which event this is for
> > > + * so that user space will know to go check the media status.
> > > + */
> > > + kobject_uevent_env(&gd->kobj, KOBJ_CHANGE, envp);
> > > + put_device(gd->driverfs_dev);
> > > +}
> > > +
> > > +void genhd_media_change_notify(struct gendisk *disk)
> > > +{
> > > + get_device(disk->driverfs_dev);
> > > + schedule_work(&disk->async_notify);
> > > +}
> > > +EXPORT_SYMBOL_GPL(genhd_media_change_notify);
> > > +
> > > struct gendisk *alloc_disk(int minors)
> > > {
> > > return alloc_disk_node(minors, -1);
> > > @@ -672,6 +693,8 @@ struct gendisk *alloc_disk_node(int mino
> > > kobj_set_kset_s(disk,block_subsys);
> > > kobject_init(&disk->kobj);
> > > rand_initialize_disk(disk);
> > > + INIT_WORK(&disk->async_notify,
> > > + media_change_notify_thread);
> > > }
> > > return disk;
> >
> > Why does this do a schedule_work() rather than calling kobject_uevent_env()
> > directly?
> >
>
> Because it is called at Interrupt time.
It better not be ... there's two GFP_KERNEL allocations just above this
line in the file.
James
On Wed, 23 May 2007 12:03:55 -0500
James Bottomley <[email protected]> wrote:
> On Wed, 2007-05-23 at 09:31 -0700, Kristen Carlson Accardi wrote:
> > On Tue, 22 May 2007 14:12:11 -0700
> > Andrew Morton <[email protected]> wrote:
> >
> > > On Wed, 9 May 2007 16:38:35 -0700
> > > Kristen Carlson Accardi <[email protected]> wrote:
> > >
> > > > Send an uevent to user space to indicate that a media change event has occurred.
> > > >
> > > > Signed-off-by: Kristen Carlson Accardi <[email protected]>
> > > >
> > > > Index: 2.6-git/block/genhd.c
> > > > ===================================================================
> > > > --- 2.6-git.orig/block/genhd.c
> > > > +++ 2.6-git/block/genhd.c
> > > > @@ -643,6 +643,27 @@ struct seq_operations diskstats_op = {
> > > > .show = diskstats_show
> > > > };
> > > >
> > > > +static void media_change_notify_thread(struct work_struct *work)
> > > > +{
> > > > + struct gendisk *gd = container_of(work, struct gendisk, async_notify);
> > > > + char event[] = "MEDIA_CHANGE=1";
> > > > + char *envp[] = { event, NULL };
> > > > +
> > > > + /*
> > > > + * set enviroment vars to indicate which event this is for
> > > > + * so that user space will know to go check the media status.
> > > > + */
> > > > + kobject_uevent_env(&gd->kobj, KOBJ_CHANGE, envp);
> > > > + put_device(gd->driverfs_dev);
> > > > +}
> > > > +
> > > > +void genhd_media_change_notify(struct gendisk *disk)
> > > > +{
> > > > + get_device(disk->driverfs_dev);
> > > > + schedule_work(&disk->async_notify);
> > > > +}
> > > > +EXPORT_SYMBOL_GPL(genhd_media_change_notify);
> > > > +
> > > > struct gendisk *alloc_disk(int minors)
> > > > {
> > > > return alloc_disk_node(minors, -1);
> > > > @@ -672,6 +693,8 @@ struct gendisk *alloc_disk_node(int mino
> > > > kobj_set_kset_s(disk,block_subsys);
> > > > kobject_init(&disk->kobj);
> > > > rand_initialize_disk(disk);
> > > > + INIT_WORK(&disk->async_notify,
> > > > + media_change_notify_thread);
> > > > }
> > > > return disk;
> > >
> > > Why does this do a schedule_work() rather than calling kobject_uevent_env()
> > > directly?
> > >
> >
> > Because it is called at Interrupt time.
>
> It better not be ... there's two GFP_KERNEL allocations just above this
> line in the file.
>
> James
>
Where? We are talking about the call to genhd_media_change_notify().
the calling path is this:
ahci_host_intr()->ata_scsi_media_change_notify()->genhd_media_change_notify().
I don't see the allocations - are they hidden somewhere?
thanks,
Kristen
On Wed, 2007-05-23 at 11:26 -0700, Kristen Carlson Accardi wrote:
> On Wed, 23 May 2007 12:03:55 -0500
> James Bottomley <[email protected]> wrote:
>
> > On Wed, 2007-05-23 at 09:31 -0700, Kristen Carlson Accardi wrote:
> > > On Tue, 22 May 2007 14:12:11 -0700
> > > Andrew Morton <[email protected]> wrote:
> > >
> > > > On Wed, 9 May 2007 16:38:35 -0700
> > > > Kristen Carlson Accardi <[email protected]> wrote:
> > > >
> > > > > Send an uevent to user space to indicate that a media change event has occurred.
> > > > >
> > > > > Signed-off-by: Kristen Carlson Accardi <[email protected]>
> > > > >
> > > > > Index: 2.6-git/block/genhd.c
> > > > > ===================================================================
> > > > > --- 2.6-git.orig/block/genhd.c
> > > > > +++ 2.6-git/block/genhd.c
> > > > > @@ -643,6 +643,27 @@ struct seq_operations diskstats_op = {
> > > > > .show = diskstats_show
> > > > > };
> > > > >
> > > > > +static void media_change_notify_thread(struct work_struct *work)
> > > > > +{
> > > > > + struct gendisk *gd = container_of(work, struct gendisk, async_notify);
> > > > > + char event[] = "MEDIA_CHANGE=1";
> > > > > + char *envp[] = { event, NULL };
> > > > > +
> > > > > + /*
> > > > > + * set enviroment vars to indicate which event this is for
> > > > > + * so that user space will know to go check the media status.
> > > > > + */
> > > > > + kobject_uevent_env(&gd->kobj, KOBJ_CHANGE, envp);
> > > > > + put_device(gd->driverfs_dev);
> > > > > +}
> > > > > +
> > > > > +void genhd_media_change_notify(struct gendisk *disk)
> > > > > +{
> > > > > + get_device(disk->driverfs_dev);
> > > > > + schedule_work(&disk->async_notify);
> > > > > +}
> > > > > +EXPORT_SYMBOL_GPL(genhd_media_change_notify);
> > > > > +
> > > > > struct gendisk *alloc_disk(int minors)
> > > > > {
> > > > > return alloc_disk_node(minors, -1);
> > > > > @@ -672,6 +693,8 @@ struct gendisk *alloc_disk_node(int mino
> > > > > kobj_set_kset_s(disk,block_subsys);
> > > > > kobject_init(&disk->kobj);
> > > > > rand_initialize_disk(disk);
> > > > > + INIT_WORK(&disk->async_notify,
> > > > > + media_change_notify_thread);
> > > > > }
> > > > > return disk;
> > > >
> > > > Why does this do a schedule_work() rather than calling kobject_uevent_env()
> > > > directly?
> > > >
> > >
> > > Because it is called at Interrupt time.
> >
> > It better not be ... there's two GFP_KERNEL allocations just above this
> > line in the file.
> >
> > James
> >
>
> Where? We are talking about the call to genhd_media_change_notify().
> the calling path is this:
> ahci_host_intr()->ata_scsi_media_change_notify()->genhd_media_change_notify().
>
> I don't see the allocations - are they hidden somewhere?
Sorry, I thought you were saying alloc_disk_node() could be called from
interrupt context.
<gets back on ball>
If you just want to invoke guaranteed user context from a place in the
code which *may* be called from interrupt, then
execute_in_process_context() might be a better way of doing it ... at
least it avoids setting up a workqueue where none is needed.
James
On Wed, 23 May 2007 13:51:51 -0500
James Bottomley <[email protected]> wrote:
> On Wed, 2007-05-23 at 11:26 -0700, Kristen Carlson Accardi wrote:
> > On Wed, 23 May 2007 12:03:55 -0500
> > James Bottomley <[email protected]> wrote:
> >
> > > On Wed, 2007-05-23 at 09:31 -0700, Kristen Carlson Accardi wrote:
> > > > On Tue, 22 May 2007 14:12:11 -0700
> > > > Andrew Morton <[email protected]> wrote:
> > > >
> > > > > On Wed, 9 May 2007 16:38:35 -0700
> > > > > Kristen Carlson Accardi <[email protected]> wrote:
> > > > >
> > > > > > Send an uevent to user space to indicate that a media change event has occurred.
> > > > > >
> > > > > > Signed-off-by: Kristen Carlson Accardi <[email protected]>
> > > > > >
> > > > > > Index: 2.6-git/block/genhd.c
> > > > > > ===================================================================
> > > > > > --- 2.6-git.orig/block/genhd.c
> > > > > > +++ 2.6-git/block/genhd.c
> > > > > > @@ -643,6 +643,27 @@ struct seq_operations diskstats_op = {
> > > > > > .show = diskstats_show
> > > > > > };
> > > > > >
> > > > > > +static void media_change_notify_thread(struct work_struct *work)
> > > > > > +{
> > > > > > + struct gendisk *gd = container_of(work, struct gendisk, async_notify);
> > > > > > + char event[] = "MEDIA_CHANGE=1";
> > > > > > + char *envp[] = { event, NULL };
> > > > > > +
> > > > > > + /*
> > > > > > + * set enviroment vars to indicate which event this is for
> > > > > > + * so that user space will know to go check the media status.
> > > > > > + */
> > > > > > + kobject_uevent_env(&gd->kobj, KOBJ_CHANGE, envp);
> > > > > > + put_device(gd->driverfs_dev);
> > > > > > +}
> > > > > > +
> > > > > > +void genhd_media_change_notify(struct gendisk *disk)
> > > > > > +{
> > > > > > + get_device(disk->driverfs_dev);
> > > > > > + schedule_work(&disk->async_notify);
> > > > > > +}
> > > > > > +EXPORT_SYMBOL_GPL(genhd_media_change_notify);
> > > > > > +
> > > > > > struct gendisk *alloc_disk(int minors)
> > > > > > {
> > > > > > return alloc_disk_node(minors, -1);
> > > > > > @@ -672,6 +693,8 @@ struct gendisk *alloc_disk_node(int mino
> > > > > > kobj_set_kset_s(disk,block_subsys);
> > > > > > kobject_init(&disk->kobj);
> > > > > > rand_initialize_disk(disk);
> > > > > > + INIT_WORK(&disk->async_notify,
> > > > > > + media_change_notify_thread);
> > > > > > }
> > > > > > return disk;
> > > > >
> > > > > Why does this do a schedule_work() rather than calling kobject_uevent_env()
> > > > > directly?
> > > > >
> > > >
> > > > Because it is called at Interrupt time.
> > >
> > > It better not be ... there's two GFP_KERNEL allocations just above this
> > > line in the file.
> > >
> > > James
> > >
> >
> > Where? We are talking about the call to genhd_media_change_notify().
> > the calling path is this:
> > ahci_host_intr()->ata_scsi_media_change_notify()->genhd_media_change_notify().
> >
> > I don't see the allocations - are they hidden somewhere?
>
> Sorry, I thought you were saying alloc_disk_node() could be called from
> interrupt context.
>
> <gets back on ball>
>
> If you just want to invoke guaranteed user context from a place in the
> code which *may* be called from interrupt, then
> execute_in_process_context() might be a better way of doing it ... at
> least it avoids setting up a workqueue where none is needed.
>
> James
>
That is good to know - although in this particular case we are guaranteed
to always be called from interrupt context since our uevent needs to be
sent in response to an Interrupt received from the disk, so it wouldn't
buy us anything.
Kristen