[patch 5/7] genhd: send async notification on media change

[patch 5/7] genhd: send async notification on media change

[Kristen Carlson Accardi]

Send an uevent to user space to indicate that a media change event has occurred.

Signed-off-by: Kristen Carlson Accardi <kristen.c.accardi@intel.com>

Index: 2.6-git/block/genhd.c
===================================================================
--- 2.6-git.orig/block/genhd.c
+++ 2.6-git/block/genhd.c
@@ -643,6 +643,25 @@ 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);
+}
+
+void genhd_media_change_notify(struct gendisk *disk)
+{
+	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 +691,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 *),

-- 

Re: [patch 5/7] genhd: send async notification on media change

[Tejun Heo]

Kristen Carlson Accardi wrote:
> Send an uevent to user space to indicate that a media change event has occurred.
> 
> Signed-off-by: Kristen Carlson Accardi <kristen.c.accardi@intel.com>
> 
> Index: 2.6-git/block/genhd.c
> ===================================================================
> --- 2.6-git.orig/block/genhd.c
> +++ 2.6-git/block/genhd.c
> @@ -643,6 +643,25 @@ 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);
> +}
> +
> +void genhd_media_change_notify(struct gendisk *disk)
> +{
> +	schedule_work(&disk->async_notify);
> +}
> +EXPORT_SYMBOL_GPL(genhd_media_change_notify);

genhd might go away while async_notify work is in-flight.  You'll need
to either grab a reference or wait for the work to finish in release
routine.

-- 
tejun

Re: [patch 5/7] genhd: send async notification on media change

[Kristen Carlson Accardi]

Send an uevent to user space to indicate that a media change event has occurred.

Changes from last version:
* use get/put_device to increment reference count on the device struct

Signed-off-by: Kristen Carlson Accardi <kristen.c.accardi@intel.com>

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 *),

Re: [patch 5/7] genhd: send async notification on media change

[Kristen Carlson Accardi]

Send an uevent to user space to indicate that a media change event has occurred.

Changes from last version:
* use get/put_device to increment reference count on the device struct

Signed-off-by: Kristen Carlson Accardi <kristen.c.accardi@intel.com>

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 *),

[patch 1/7] libata: check for AN support

[Kristen Carlson Accardi]

Check to see if an ATAPI device supports Asynchronous Notification.
If so, enable it.

Signed-off-by: Kristen Carlson Accardi <kristen.c.accardi@intel.com>

Index: 2.6-git/drivers/ata/libata-core.c
===================================================================
--- 2.6-git.orig/drivers/ata/libata-core.c
+++ 2.6-git/drivers/ata/libata-core.c
@@ -70,6 +70,7 @@ const unsigned long sata_deb_timing_long
 static unsigned int ata_dev_init_params(struct ata_device *dev,
 					u16 heads, u16 sectors);
 static unsigned int ata_dev_set_xfermode(struct ata_device *dev);
+static unsigned int ata_dev_set_AN(struct ata_device *dev);
 static void ata_dev_xfermask(struct ata_device *dev);
 
 unsigned int ata_print_id = 1;
@@ -1981,6 +1982,22 @@ int ata_dev_configure(struct ata_device 
 		}
 		dev->cdb_len = (unsigned int) rc;
 
+		/*
+		 * check to see if this ATAPI device supports
+		 * Asynchronous Notification
+		 */
+		if ((ap->flags & ATA_FLAG_AN) && ata_id_has_AN(id)) {
+			int err;
+			/* issue SET feature command to turn this on */
+			err = ata_dev_set_AN(dev);
+			if (err)
+				ata_dev_printk(dev, KERN_ERR,
+						"unable to set AN, err %x\n",
+						err);
+			else
+				dev->flags |= ATA_DFLAG_AN;
+		}
+
 		if (ata_id_cdb_intr(dev->id)) {
 			dev->flags |= ATA_DFLAG_CDB_INTR;
 			cdb_intr_string = ", CDB intr";
@@ -3908,6 +3925,42 @@ static unsigned int ata_dev_set_xfermode
 }
 
 /**
+ *	ata_dev_set_AN - Issue SET FEATURES - SATA FEATURES
+ *                       with sector count set to indicate
+ *                       Asynchronous Notification feature
+ *	@dev: Device to which command will be sent
+ *
+ *	Issue SET FEATURES - SATA FEATURES command to device @dev
+ *	on port @ap.
+ *
+ *	LOCKING:
+ *	PCI/etc. bus probe sem.
+ *
+ *	RETURNS:
+ *	0 on success, AC_ERR_* mask otherwise.
+ */
+static unsigned int ata_dev_set_AN(struct ata_device *dev)
+{
+	struct ata_taskfile tf;
+	unsigned int err_mask;
+
+	/* set up set-features taskfile */
+	DPRINTK("set features - SATA features\n");
+
+	ata_tf_init(dev, &tf);
+	tf.command = ATA_CMD_SET_FEATURES;
+	tf.feature = SETFEATURES_SATA_ENABLE;
+	tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE;
+	tf.protocol = ATA_PROT_NODATA;
+	tf.nsect = SATA_AN;
+
+	err_mask = ata_exec_internal(dev, &tf, NULL, DMA_NONE, NULL, 0);
+
+	DPRINTK("EXIT, err_mask=%x\n", err_mask);
+	return err_mask;
+}
+
+/**
  *	ata_dev_init_params - Issue INIT DEV PARAMS command
  *	@dev: Device to which command will be sent
  *	@heads: Number of heads (taskfile parameter)
Index: 2.6-git/include/linux/ata.h
===================================================================
--- 2.6-git.orig/include/linux/ata.h
+++ 2.6-git/include/linux/ata.h
@@ -204,6 +204,12 @@ enum {
 
 	SETFEATURES_SPINUP	= 0x07, /* Spin-up drive */
 
+	SETFEATURES_SATA_ENABLE = 0x10, /* Enable use of SATA feature */
+	SETFEATURES_SATA_DISABLE = 0x90, /* Disable use of SATA feature */
+
+	/* SETFEATURE Sector counts for SATA features */
+	SATA_AN			= 0x05,  /* Asynchronous Notification */
+
 	/* ATAPI stuff */
 	ATAPI_PKT_DMA		= (1 << 0),
 	ATAPI_DMADIR		= (1 << 2),	/* ATAPI data dir:
@@ -309,6 +315,9 @@ struct ata_taskfile {
 #define ata_id_queue_depth(id)	(((id)[75] & 0x1f) + 1)
 #define ata_id_removeable(id)	((id)[0] & (1 << 7))
 #define ata_id_has_dword_io(id)	((id)[50] & (1 << 0))
+#define ata_id_has_AN(id)	\
+	( (((id)[76] != 0x0000) && ((id)[76] != 0xffff)) && \
+	  ((id)[78] & (1 << 5)) )
 #define ata_id_iordy_disable(id) ((id)[49] & (1 << 10))
 #define ata_id_has_iordy(id) ((id)[49] & (1 << 9))
 #define ata_id_u32(id,n)	\
Index: 2.6-git/include/linux/libata.h
===================================================================
--- 2.6-git.orig/include/linux/libata.h
+++ 2.6-git/include/linux/libata.h
@@ -136,6 +136,7 @@ enum {
 	ATA_DFLAG_CDB_INTR	= (1 << 2), /* device asserts INTRQ when ready for CDB */
 	ATA_DFLAG_NCQ		= (1 << 3), /* device supports NCQ */
 	ATA_DFLAG_FLUSH_EXT	= (1 << 4), /* do FLUSH_EXT instead of FLUSH */
+	ATA_DFLAG_AN		= (1 << 5), /* device supports Async notification */
 	ATA_DFLAG_CFG_MASK	= (1 << 8) - 1,
 
 	ATA_DFLAG_PIO		= (1 << 8), /* device limited to PIO mode */
@@ -174,6 +175,7 @@ enum {
 	ATA_FLAG_SETXFER_POLLING= (1 << 14), /* use polling for SETXFER */
 	ATA_FLAG_IGN_SIMPLEX	= (1 << 15), /* ignore SIMPLEX */
 	ATA_FLAG_NO_IORDY	= (1 << 16), /* controller lacks iordy */
+	ATA_FLAG_AN		= (1 << 17), /* controller supports AN */
 
 	/* The following flag belongs to ap->pflags but is kept in
 	 * ap->flags because it's referenced in many LLDs and will be
Index: 2.6-git/drivers/ata/ahci.c
===================================================================
--- 2.6-git.orig/drivers/ata/ahci.c
+++ 2.6-git/drivers/ata/ahci.c
@@ -327,14 +327,15 @@ static const struct ata_port_operations 
 static const struct ata_port_info ahci_port_info[] = {
 	/* board_ahci */
 	{
-		.flags		= AHCI_FLAG_COMMON,
+		.flags		= AHCI_FLAG_COMMON | ATA_FLAG_AN,
 		.pio_mask	= 0x1f, /* pio0-4 */
 		.udma_mask	= 0x7f, /* udma0-6 ; FIXME */
 		.port_ops	= &ahci_ops,
 	},
 	/* board_ahci_pi */
 	{
-		.flags		= AHCI_FLAG_COMMON | AHCI_FLAG_HONOR_PI,
+		.flags		= AHCI_FLAG_COMMON | AHCI_FLAG_HONOR_PI |
+				  ATA_FLAG_AN,
 		.pio_mask	= 0x1f, /* pio0-4 */
 		.udma_mask	= 0x7f, /* udma0-6 ; FIXME */
 		.port_ops	= &ahci_ops,

-- 

[patch 2/7] genhd: expose AN to user space

[Kristen Carlson Accardi]

Allow user space to determine if a disk supports Asynchronous Notification
of media changes.  This is done by adding a new sysfs file "capability_flags",
which is documented in (insert file name).  This sysfs file will export all
disk capabilities flags to user space.  We also define a new flag to define
the media change notification capability.

Signed-off-by: Kristen Carlson Accardi <kristen.c.accardi@intel.com>

Index: 2.6-git/block/genhd.c
===================================================================
--- 2.6-git.orig/block/genhd.c
+++ 2.6-git/block/genhd.c
@@ -370,7 +370,10 @@ static ssize_t disk_size_read(struct gen
 {
 	return sprintf(page, "%llu\n", (unsigned long long)get_capacity(disk));
 }
-
+static ssize_t disk_capability_read(struct gendisk *disk, char *page)
+{
+	return sprintf(page, "%x\n", disk->flags);
+}
 static ssize_t disk_stats_read(struct gendisk * disk, char *page)
 {
 	preempt_disable();
@@ -413,6 +416,10 @@ static struct disk_attribute disk_attr_s
 	.attr = {.name = "size", .mode = S_IRUGO },
 	.show	= disk_size_read
 };
+static struct disk_attribute disk_attr_capability = {
+	.attr = {.name = "capability", .mode = S_IRUGO },
+	.show	= disk_capability_read
+};
 static struct disk_attribute disk_attr_stat = {
 	.attr = {.name = "stat", .mode = S_IRUGO },
 	.show	= disk_stats_read
@@ -453,6 +460,7 @@ static struct attribute * default_attrs[
 	&disk_attr_removable.attr,
 	&disk_attr_size.attr,
 	&disk_attr_stat.attr,
+	&disk_attr_capability.attr,
 #ifdef CONFIG_FAIL_MAKE_REQUEST
 	&disk_attr_fail.attr,
 #endif
Index: 2.6-git/include/linux/genhd.h
===================================================================
--- 2.6-git.orig/include/linux/genhd.h
+++ 2.6-git/include/linux/genhd.h
@@ -94,6 +94,7 @@ struct hd_struct {
 
 #define GENHD_FL_REMOVABLE			1
 #define GENHD_FL_DRIVERFS			2
+#define GENHD_FL_MEDIA_CHANGE_NOTIFY		4
 #define GENHD_FL_CD				8
 #define GENHD_FL_UP				16
 #define GENHD_FL_SUPPRESS_PARTITION_INFO	32
Index: 2.6-git/Documentation/block/capability.txt
===================================================================
--- /dev/null
+++ 2.6-git/Documentation/block/capability.txt
@@ -0,0 +1,15 @@
+Generic Block Device Capability
+===============================================================================
+This file documents the sysfs file block/<disk>/capability
+
+capability is a hex word indicating which capabilities a specific disk
+supports.  For more information on bits not listed here, see
+include/linux/genhd.h
+
+Capability				Value
+-------------------------------------------------------------------------------
+GENHD_FL_MEDIA_CHANGE_NOTIFY		4
+	When this bit is set, the disk supports Asynchronous Notification
+	of media change events.  These events will be broadcast to user
+	space via kernel uevent.
+

-- 

[patch 3/7] scsi: expose AN to user space

[Kristen Carlson Accardi]

Get media change notification capability from disk and pass this information
to genhd by setting appropriate flag.

Signed-off-by: Kristen Carlson Accardi <kristen.c.accardi@intel.com>

Index: 2.6-git/drivers/scsi/sr.c
===================================================================
--- 2.6-git.orig/drivers/scsi/sr.c
+++ 2.6-git/drivers/scsi/sr.c
@@ -603,6 +603,8 @@ static int sr_probe(struct device *dev)
 
 	dev_set_drvdata(dev, cd);
 	disk->flags |= GENHD_FL_REMOVABLE;
+	if (sdev->media_change_notify)
+		disk->flags |= GENHD_FL_MEDIA_CHANGE_NOTIFY;
 	add_disk(disk);
 
 	sdev_printk(KERN_DEBUG, sdev,
Index: 2.6-git/include/scsi/scsi_device.h
===================================================================
--- 2.6-git.orig/include/scsi/scsi_device.h
+++ 2.6-git/include/scsi/scsi_device.h
@@ -126,7 +126,7 @@ struct scsi_device {
 	unsigned fix_capacity:1;	/* READ_CAPACITY is too high by 1 */
 	unsigned guess_capacity:1;	/* READ_CAPACITY might be too high by 1 */
 	unsigned retry_hwerror:1;	/* Retry HARDWARE_ERROR */
-
+	unsigned media_change_notify:1; /* dev supports async media notify */
 	unsigned int device_blocked;	/* Device returned QUEUE_FULL. */
 
 	unsigned int max_device_blocked; /* what device_blocked counts down from  */
Index: 2.6-git/drivers/scsi/sd.c
===================================================================
--- 2.6-git.orig/drivers/scsi/sd.c
+++ 2.6-git/drivers/scsi/sd.c
@@ -1668,6 +1668,9 @@ static int sd_probe(struct device *dev)
 	if (sdp->removable)
 		gd->flags |= GENHD_FL_REMOVABLE;
 
+	if (sdp->media_change_notify)
+		gd->flags |= GENHD_FL_MEDIA_CHANGE_NOTIFY;
+
 	dev_set_drvdata(dev, sdkp);
 	add_disk(gd);
 

-- 

[patch 4/7] libata: expose AN to user space

[Kristen Carlson Accardi]

If Asynchronous Notification of media change events is supported,
pass that information up to the SCSI layer.

Signed-off-by: Kristen Carlson Accardi <kristen.c.accardi@intel.com>

Index: 2.6-git/drivers/ata/libata-scsi.c
===================================================================
--- 2.6-git.orig/drivers/ata/libata-scsi.c
+++ 2.6-git/drivers/ata/libata-scsi.c
@@ -899,6 +899,9 @@ static void ata_scsi_dev_config(struct s
 		blk_queue_max_hw_segments(q, q->max_hw_segments - 1);
 	}
 
+	if (dev->flags & ATA_DFLAG_AN)
+		sdev->media_change_notify = 1;
+
 	if (dev->flags & ATA_DFLAG_NCQ) {
 		int depth;
 

-- 

[patch 5/7] genhd: send async notification on media change

[Kristen Carlson Accardi]

Send an uevent to user space to indicate that a media change event has occurred.

Signed-off-by: Kristen Carlson Accardi <kristen.c.accardi@intel.com>

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 *),

-- 

[patch 6/7] SCSI: save disk in scsi_device

[Kristen Carlson Accardi]

Give anyone who has access to scsi_device access to the genhd struct as well.

Signed-off-by: Kristen Carlson Accardi <kristen.c.accardi@intel.com>
Index: 2.6-git/drivers/scsi/sd.c
===================================================================
--- 2.6-git.orig/drivers/scsi/sd.c
+++ 2.6-git/drivers/scsi/sd.c
@@ -1673,6 +1673,7 @@ static int sd_probe(struct device *dev)
 
 	dev_set_drvdata(dev, sdkp);
 	add_disk(gd);
+	sdp->disk = gd;
 
 	sd_printk(KERN_NOTICE, sdkp, "Attached SCSI %sdisk\n",
 		  sdp->removable ? "removable " : "");
Index: 2.6-git/drivers/scsi/sr.c
===================================================================
--- 2.6-git.orig/drivers/scsi/sr.c
+++ 2.6-git/drivers/scsi/sr.c
@@ -606,6 +606,7 @@ static int sr_probe(struct device *dev)
 	if (sdev->media_change_notify)
 		disk->flags |= GENHD_FL_MEDIA_CHANGE_NOTIFY;
 	add_disk(disk);
+	sdev->disk = disk;
 
 	sdev_printk(KERN_DEBUG, sdev,
 		    "Attached scsi CD-ROM %s\n", cd->cdi.name);
Index: 2.6-git/include/scsi/scsi_device.h
===================================================================
--- 2.6-git.orig/include/scsi/scsi_device.h
+++ 2.6-git/include/scsi/scsi_device.h
@@ -140,7 +140,7 @@ struct scsi_device {
 
 	struct device		sdev_gendev;
 	struct class_device	sdev_classdev;
-
+	struct gendisk 		*disk;
 	struct execute_work	ew; /* used to get process context on put */
 
 	enum scsi_device_state sdev_state;

-- 

[patch 7/7] libata: send event when AN received

[Kristen Carlson Accardi]

When we get an SDB FIS with the 'N' bit set, we should send
an event to user space to indicate that there has been a
media change.  This will be done via the block device. 

Signed-off-by: Kristen Carlson Accardi <kristen.c.accardi@intel.com>
Index: 2.6-git/drivers/ata/ahci.c
===================================================================
--- 2.6-git.orig/drivers/ata/ahci.c
+++ 2.6-git/drivers/ata/ahci.c
@@ -1218,6 +1218,28 @@ static void ahci_host_intr(struct ata_po
 		return;
 	}
 
+	if (status & PORT_IRQ_SDB_FIS) {
+		/*
+		 * if this is an ATAPI device with AN turned on,
+		 * then we should interrogate the device to
+		 * determine the cause of the interrupt
+		 *
+		 * for AN - this we should check the SDB FIS
+		 * and find the I and N bits set
+		 */
+		const u32 *f = pp->rx_fis + RX_FIS_SDB;
+
+		/* check the 'N' bit in word 0 of the FIS */
+		if (f[0] & (1 << 15)) {
+			int port_addr =  ((f[0] & 0x00000f00) >> 8);
+			struct ata_device *adev;
+			if (port_addr < ATA_MAX_DEVICES) {
+				adev = &ap->device[port_addr];
+				if (adev->flags & ATA_DFLAG_AN)
+					ata_scsi_media_change_notify(adev);
+			}
+		}
+	}
 	if (ap->sactive)
 		qc_active = readl(port_mmio + PORT_SCR_ACT);
 	else
Index: 2.6-git/include/linux/libata.h
===================================================================
--- 2.6-git.orig/include/linux/libata.h
+++ 2.6-git/include/linux/libata.h
@@ -721,6 +721,7 @@ extern void ata_host_init(struct ata_hos
 extern int ata_scsi_detect(struct scsi_host_template *sht);
 extern int ata_scsi_ioctl(struct scsi_device *dev, int cmd, void __user *arg);
 extern int ata_scsi_queuecmd(struct scsi_cmnd *cmd, void (*done)(struct scsi_cmnd *));
+extern void ata_scsi_media_change_notify(struct ata_device *atadev);
 extern void ata_sas_port_destroy(struct ata_port *);
 extern struct ata_port *ata_sas_port_alloc(struct ata_host *,
 					   struct ata_port_info *, struct Scsi_Host *);
Index: 2.6-git/drivers/ata/libata-scsi.c
===================================================================
--- 2.6-git.orig/drivers/ata/libata-scsi.c
+++ 2.6-git/drivers/ata/libata-scsi.c
@@ -3112,6 +3112,22 @@ static void ata_scsi_remove_dev(struct a
 }
 
 /**
+ *	ata_scsi_media_change_notify - send media change event
+ *	@atadev: Pointer to the disk device with media change event
+ *
+ *	Tell the block layer to send a media change notification
+ *	event.
+ *
+ * 	LOCKING:
+ * 	interrupt context, may not sleep.
+ */
+void ata_scsi_media_change_notify(struct ata_device *atadev)
+{
+	genhd_media_change_notify(atadev->sdev->disk);
+}
+EXPORT_SYMBOL_GPL(ata_scsi_media_change_notify);
+
+/**
  *	ata_scsi_hotplug - SCSI part of hotplug
  *	@work: Pointer to ATA port to perform SCSI hotplug on
  *

-- 

Re: [patch 1/7] libata: check for AN support

[Andrew Morton]

On Wed, 9 May 2007 16:38:09 -0700 Kristen Carlson Accardi <kristen.c.accardi@intel.com> wrote:

>  /**
> + *	ata_dev_set_AN - Issue SET FEATURES - SATA FEATURES
> + *                       with sector count set to indicate
> + *                       Asynchronous Notification feature

I think kenreldoc requires that all this be on a single line?

> + *	@dev: Device to which command will be sent
> + *
> + *	Issue SET FEATURES - SATA FEATURES command to device @dev
> + *	on port @ap.
> + *
> + *	LOCKING:
> + *	PCI/etc. bus probe sem.
> + *
> + *	RETURNS:
> + *	0 on success, AC_ERR_* mask otherwise.
> + */

ooh, locking and return value documentation.  Often missed, and nice.

Re: [patch 1/7] libata: check for AN support

[Jeff Garzik]

Andrew Morton wrote:
>> + *	@dev: Device to which command will be sent
>> + *
>> + *	Issue SET FEATURES - SATA FEATURES command to device @dev
>> + *	on port @ap.
>> + *
>> + *	LOCKING:
>> + *	PCI/etc. bus probe sem.
>> + *
>> + *	RETURNS:
>> + *	0 on success, AC_ERR_* mask otherwise.
>> + */
> 
> ooh, locking and return value documentation.  Often missed, and nice.


We set high standards in the libata world ;-)

	Jeff

Re: [patch 1/7] libata: check for AN support

[Andrew Morton]

On Thu, 10 May 2007 01:14:38 -0400 Jeff Garzik <jeff@garzik.org> wrote:

> Andrew Morton wrote:
> >> + *	@dev: Device to which command will be sent
> >> + *
> >> + *	Issue SET FEATURES - SATA FEATURES command to device @dev
> >> + *	on port @ap.
> >> + *
> >> + *	LOCKING:
> >> + *	PCI/etc. bus probe sem.
> >> + *
> >> + *	RETURNS:
> >> + *	0 on success, AC_ERR_* mask otherwise.
> >> + */
> > 
> > ooh, locking and return value documentation.  Often missed, and nice.
> 
> 
> We set high standards in the libata world ;-)
> 

It seems to be working.  This series is perhaps the most idiomatic and
generally kernelly-looking code I've seen in ages.

Who cares if it works? ;)

Re: [patch 1/7] libata: check for AN support

[Randy Dunlap]

On Wed, 9 May 2007 22:09:52 -0700 Andrew Morton wrote:

> On Wed, 9 May 2007 16:38:09 -0700 Kristen Carlson Accardi <kristen.c.accardi@intel.com> wrote:
> 
> >  /**
> > + *	ata_dev_set_AN - Issue SET FEATURES - SATA FEATURES
> > + *                       with sector count set to indicate
> > + *                       Asynchronous Notification feature
> 
> I think kenreldoc requires that all this be on a single line?

Correct.

> > + *	@dev: Device to which command will be sent
> > + *
> > + *	Issue SET FEATURES - SATA FEATURES command to device @dev
> > + *	on port @ap.
> > + *
> > + *	LOCKING:
> > + *	PCI/etc. bus probe sem.
> > + *
> > + *	RETURNS:
> > + *	0 on success, AC_ERR_* mask otherwise.
> > + */
> 
> ooh, locking and return value documentation.  Often missed, and nice.

---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

Re: [patch 1/7] libata: check for AN support

[Kristen Carlson Accardi]

Check to see if an ATAPI device supports Asynchronous Notification.
If so, enable it.

Signed-off-by: Kristen Carlson Accardi <kristen.c.accardi@intel.com>
---
Andrew, I cleaned up the function header to properly comply with kernel
doc requirements.  Other than that, this patch is the same.  

Index: 2.6-mm/drivers/ata/libata-core.c
===================================================================
--- 2.6-mm.orig/drivers/ata/libata-core.c
+++ 2.6-mm/drivers/ata/libata-core.c
@@ -70,6 +70,7 @@ const unsigned long sata_deb_timing_long
 static unsigned int ata_dev_init_params(struct ata_device *dev,
 					u16 heads, u16 sectors);
 static unsigned int ata_dev_set_xfermode(struct ata_device *dev);
+static unsigned int ata_dev_set_AN(struct ata_device *dev);
 static void ata_dev_xfermask(struct ata_device *dev);
 
 unsigned int ata_print_id = 1;
@@ -1983,6 +1984,22 @@ int ata_dev_configure(struct ata_device 
 		}
 		dev->cdb_len = (unsigned int) rc;
 
+		/*
+		 * check to see if this ATAPI device supports
+		 * Asynchronous Notification
+		 */
+		if ((ap->flags & ATA_FLAG_AN) && ata_id_has_AN(id)) {
+			int err;
+			/* issue SET feature command to turn this on */
+			err = ata_dev_set_AN(dev);
+			if (err)
+				ata_dev_printk(dev, KERN_ERR,
+						"unable to set AN, err %x\n",
+						err);
+			else
+				dev->flags |= ATA_DFLAG_AN;
+		}
+
 		if (ata_id_cdb_intr(dev->id)) {
 			dev->flags |= ATA_DFLAG_CDB_INTR;
 			cdb_intr_string = ", CDB intr";
@@ -3910,6 +3927,41 @@ static unsigned int ata_dev_set_xfermode
 }
 
 /**
+ *	ata_dev_set_AN - Issue SET FEATURES - SATA FEATURES
+ *	@dev: Device to which command will be sent
+ *
+ *	Issue SET FEATURES - SATA FEATURES command to device @dev
+ *	on port @ap with sector count set to indicate Asynchronous
+ *	Notification feature
+ *
+ *	LOCKING:
+ *	PCI/etc. bus probe sem.
+ *
+ *	RETURNS:
+ *	0 on success, AC_ERR_* mask otherwise.
+ */
+static unsigned int ata_dev_set_AN(struct ata_device *dev)
+{
+	struct ata_taskfile tf;
+	unsigned int err_mask;
+
+	/* set up set-features taskfile */
+	DPRINTK("set features - SATA features\n");
+
+	ata_tf_init(dev, &tf);
+	tf.command = ATA_CMD_SET_FEATURES;
+	tf.feature = SETFEATURES_SATA_ENABLE;
+	tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE;
+	tf.protocol = ATA_PROT_NODATA;
+	tf.nsect = SATA_AN;
+
+	err_mask = ata_exec_internal(dev, &tf, NULL, DMA_NONE, NULL, 0);
+
+	DPRINTK("EXIT, err_mask=%x\n", err_mask);
+	return err_mask;
+}
+
+/**
  *	ata_dev_init_params - Issue INIT DEV PARAMS command
  *	@dev: Device to which command will be sent
  *	@heads: Number of heads (taskfile parameter)
Index: 2.6-mm/include/linux/ata.h
===================================================================
--- 2.6-mm.orig/include/linux/ata.h
+++ 2.6-mm/include/linux/ata.h
@@ -204,6 +204,12 @@ enum {
 
 	SETFEATURES_SPINUP	= 0x07, /* Spin-up drive */
 
+	SETFEATURES_SATA_ENABLE = 0x10, /* Enable use of SATA feature */
+	SETFEATURES_SATA_DISABLE = 0x90, /* Disable use of SATA feature */
+
+	/* SETFEATURE Sector counts for SATA features */
+	SATA_AN			= 0x05,  /* Asynchronous Notification */
+
 	/* ATAPI stuff */
 	ATAPI_PKT_DMA		= (1 << 0),
 	ATAPI_DMADIR		= (1 << 2),	/* ATAPI data dir:
@@ -309,6 +315,9 @@ struct ata_taskfile {
 #define ata_id_queue_depth(id)	(((id)[75] & 0x1f) + 1)
 #define ata_id_removeable(id)	((id)[0] & (1 << 7))
 #define ata_id_has_dword_io(id)	((id)[50] & (1 << 0))
+#define ata_id_has_AN(id)	\
+	( (((id)[76] != 0x0000) && ((id)[76] != 0xffff)) && \
+	  ((id)[78] & (1 << 5)) )
 #define ata_id_iordy_disable(id) ((id)[49] & (1 << 10))
 #define ata_id_has_iordy(id) ((id)[49] & (1 << 9))
 #define ata_id_u32(id,n)	\
Index: 2.6-mm/include/linux/libata.h
===================================================================
--- 2.6-mm.orig/include/linux/libata.h
+++ 2.6-mm/include/linux/libata.h
@@ -136,6 +136,7 @@ enum {
 	ATA_DFLAG_CDB_INTR	= (1 << 2), /* device asserts INTRQ when ready for CDB */
 	ATA_DFLAG_NCQ		= (1 << 3), /* device supports NCQ */
 	ATA_DFLAG_FLUSH_EXT	= (1 << 4), /* do FLUSH_EXT instead of FLUSH */
+	ATA_DFLAG_AN		= (1 << 5), /* device supports Async notification */
 	ATA_DFLAG_CFG_MASK	= (1 << 8) - 1,
 
 	ATA_DFLAG_PIO		= (1 << 8), /* device limited to PIO mode */
@@ -174,6 +175,7 @@ enum {
 	ATA_FLAG_SETXFER_POLLING= (1 << 14), /* use polling for SETXFER */
 	ATA_FLAG_IGN_SIMPLEX	= (1 << 15), /* ignore SIMPLEX */
 	ATA_FLAG_NO_IORDY	= (1 << 16), /* controller lacks iordy */
+	ATA_FLAG_AN		= (1 << 17), /* controller supports AN */
 
 	/* The following flag belongs to ap->pflags but is kept in
 	 * ap->flags because it's referenced in many LLDs and will be
Index: 2.6-mm/drivers/ata/ahci.c
===================================================================
--- 2.6-mm.orig/drivers/ata/ahci.c
+++ 2.6-mm/drivers/ata/ahci.c
@@ -327,14 +327,15 @@ static const struct ata_port_operations 
 static const struct ata_port_info ahci_port_info[] = {
 	/* board_ahci */
 	{
-		.flags		= AHCI_FLAG_COMMON,
+		.flags		= AHCI_FLAG_COMMON | ATA_FLAG_AN,
 		.pio_mask	= 0x1f, /* pio0-4 */
 		.udma_mask	= 0x7f, /* udma0-6 ; FIXME */
 		.port_ops	= &ahci_ops,
 	},
 	/* board_ahci_pi */
 	{
-		.flags		= AHCI_FLAG_COMMON | AHCI_FLAG_HONOR_PI,
+		.flags		= AHCI_FLAG_COMMON | AHCI_FLAG_HONOR_PI |
+				  ATA_FLAG_AN,
 		.pio_mask	= 0x1f, /* pio0-4 */
 		.udma_mask	= 0x7f, /* udma0-6 ; FIXME */
 		.port_ops	= &ahci_ops,

Re: [patch 5/7] genhd: send async notification on media change

[Andrew Morton]

On Wed, 9 May 2007 16:38:35 -0700
Kristen Carlson Accardi <kristen.c.accardi@intel.com> wrote:

> Send an uevent to user space to indicate that a media change event has occurred.
> 
> Signed-off-by: Kristen Carlson Accardi <kristen.c.accardi@intel.com>
> 
> 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?

Re: [patch 5/7] genhd: send async notification on media change

[Kristen Carlson Accardi]

On Tue, 22 May 2007 14:12:11 -0700
Andrew Morton <akpm@linux-foundation.org> wrote:

> On Wed, 9 May 2007 16:38:35 -0700
> Kristen Carlson Accardi <kristen.c.accardi@intel.com> wrote:
> 
> > Send an uevent to user space to indicate that a media change event has occurred.
> > 
> > Signed-off-by: Kristen Carlson Accardi <kristen.c.accardi@intel.com>
> > 
> > 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

Re: [patch 5/7] genhd: send async notification on media change

[James Bottomley]

On Wed, 2007-05-23 at 09:31 -0700, Kristen Carlson Accardi wrote:
> On Tue, 22 May 2007 14:12:11 -0700
> Andrew Morton <akpm@linux-foundation.org> wrote:
> 
> > On Wed, 9 May 2007 16:38:35 -0700
> > Kristen Carlson Accardi <kristen.c.accardi@intel.com> wrote:
> > 
> > > Send an uevent to user space to indicate that a media change event has occurred.
> > > 
> > > Signed-off-by: Kristen Carlson Accardi <kristen.c.accardi@intel.com>
> > > 
> > > 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

Re: [patch 5/7] genhd: send async notification on media change

[Kristen Carlson Accardi]

On Wed, 23 May 2007 12:03:55 -0500
James Bottomley <James.Bottomley@SteelEye.com> 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 <akpm@linux-foundation.org> wrote:
> > 
> > > On Wed, 9 May 2007 16:38:35 -0700
> > > Kristen Carlson Accardi <kristen.c.accardi@intel.com> wrote:
> > > 
> > > > Send an uevent to user space to indicate that a media change event has occurred.
> > > > 
> > > > Signed-off-by: Kristen Carlson Accardi <kristen.c.accardi@intel.com>
> > > > 
> > > > 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
 

Re: [patch 5/7] genhd: send async notification on media change

[James Bottomley]

On Wed, 2007-05-23 at 11:26 -0700, Kristen Carlson Accardi wrote:
> On Wed, 23 May 2007 12:03:55 -0500
> James Bottomley <James.Bottomley@SteelEye.com> 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 <akpm@linux-foundation.org> wrote:
> > > 
> > > > On Wed, 9 May 2007 16:38:35 -0700
> > > > Kristen Carlson Accardi <kristen.c.accardi@intel.com> wrote:
> > > > 
> > > > > Send an uevent to user space to indicate that a media change event has occurred.
> > > > > 
> > > > > Signed-off-by: Kristen Carlson Accardi <kristen.c.accardi@intel.com>
> > > > > 
> > > > > 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

Re: [patch 5/7] genhd: send async notification on media change

[Kristen Carlson Accardi]

On Wed, 23 May 2007 13:51:51 -0500
James Bottomley <James.Bottomley@SteelEye.com> 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 <James.Bottomley@SteelEye.com> 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 <akpm@linux-foundation.org> wrote:
> > > > 
> > > > > On Wed, 9 May 2007 16:38:35 -0700
> > > > > Kristen Carlson Accardi <kristen.c.accardi@intel.com> wrote:
> > > > > 
> > > > > > Send an uevent to user space to indicate that a media change event has occurred.
> > > > > > 
> > > > > > Signed-off-by: Kristen Carlson Accardi <kristen.c.accardi@intel.com>
> > > > > > 
> > > > > > 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

Re: [patch 1/7] libata: check for AN support

[Jeff Garzik]

Kristen Carlson Accardi wrote:
> Check to see if an ATAPI device supports Asynchronous Notification.
> If so, enable it.
> 
> Signed-off-by: Kristen Carlson Accardi <kristen.c.accardi@intel.com>
> ---
> Andrew, I cleaned up the function header to properly comply with kernel
> doc requirements.  Other than that, this patch is the same.  

I would ask for a simple revision:  update ata_dev_set_AN() such that it 
takes a second argument 'enable'.  This boolean indicates to the 
function whether SETFEATURES_SATA_ENABLE or SETFEATURES_SATA_DISABLE 
should be passed to the device.

Otherwise than that, it's ready to merge I would say.

Re: [patch 1/7] libata: check for AN support

[Kristen Carlson Accardi]

On Thu, 24 May 2007 23:15:56 -0400
Jeff Garzik <jeff@garzik.org> wrote:

> Kristen Carlson Accardi wrote:
> > Check to see if an ATAPI device supports Asynchronous Notification.
> > If so, enable it.
> > 
> > Signed-off-by: Kristen Carlson Accardi <kristen.c.accardi@intel.com>
> > ---
> > Andrew, I cleaned up the function header to properly comply with kernel
> > doc requirements.  Other than that, this patch is the same.  
> 
> I would ask for a simple revision:  update ata_dev_set_AN() such that it 
> takes a second argument 'enable'.  This boolean indicates to the 
> function whether SETFEATURES_SATA_ENABLE or SETFEATURES_SATA_DISABLE 
> should be passed to the device.
> 
> Otherwise than that, it's ready to merge I would say.
> 

Jeff - can you fold this into the original patch, or would you like me
to resubmit the whole thing?

Kristen

Modify ata_dev_set_AN to take a second argument 'enable'.  This
boolean indicates to the function whether SETFEATURES_SATA_ENABLE
or SETFEATURES_SATA_DISABLE should be passed to the device.

Signed-off-by:  Kristen Carlson Accardi <kristen.c.accardi@intel.com>

Index: 2.6-git/drivers/ata/libata-core.c
===================================================================
--- 2.6-git.orig/drivers/ata/libata-core.c
+++ 2.6-git/drivers/ata/libata-core.c
@@ -70,7 +70,7 @@ const unsigned long sata_deb_timing_long
 static unsigned int ata_dev_init_params(struct ata_device *dev,
 					u16 heads, u16 sectors);
 static unsigned int ata_dev_set_xfermode(struct ata_device *dev);
-static unsigned int ata_dev_set_AN(struct ata_device *dev);
+static unsigned int ata_dev_set_AN(struct ata_device *dev, u8 enable);
 static void ata_dev_xfermask(struct ata_device *dev);
 
 unsigned int ata_print_id = 1;
@@ -2010,7 +2010,7 @@ int ata_dev_configure(struct ata_device 
 		if ((ap->flags & ATA_FLAG_AN) && ata_id_has_AN(id)) {
 			int err;
 			/* issue SET feature command to turn this on */
-			err = ata_dev_set_AN(dev);
+			err = ata_dev_set_AN(dev, SETFEATURES_SATA_ENABLE);
 			if (err)
 				ata_dev_printk(dev, KERN_ERR,
 						"unable to set AN, err %x\n",
@@ -3966,6 +3966,7 @@ static unsigned int ata_dev_set_xfermode
 /**
  *	ata_dev_set_AN - Issue SET FEATURES - SATA FEATURES
  *	@dev: Device to which command will be sent
+ *	@enable: Whether to enable or disable the feature
  *
  *	Issue SET FEATURES - SATA FEATURES command to device @dev
  *	on port @ap with sector count set to indicate Asynchronous
@@ -3977,7 +3978,7 @@ static unsigned int ata_dev_set_xfermode
  *	RETURNS:
  *	0 on success, AC_ERR_* mask otherwise.
  */
-static unsigned int ata_dev_set_AN(struct ata_device *dev)
+static unsigned int ata_dev_set_AN(struct ata_device *dev, u8 enable)
 {
 	struct ata_taskfile tf;
 	unsigned int err_mask;
@@ -3987,7 +3988,7 @@ static unsigned int ata_dev_set_AN(struc
 
 	ata_tf_init(dev, &tf);
 	tf.command = ATA_CMD_SET_FEATURES;
-	tf.feature = SETFEATURES_SATA_ENABLE;
+	tf.feature = enable;
 	tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE;
 	tf.protocol = ATA_PROT_NODATA;
 	tf.nsect = SATA_AN;