| Message ID | 20170504005013.2865393-2-songliubraving@fb.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
> On May 3, 2017, at 5:50 PM, Song Liu <songliubraving@fb.com> wrote: > > This patch adds capability for SCSI layer to generate uevent for SCSI > sense code. The feature is gated by CONFIG_SCSI_SENSE_UEVENT. > > We can configure which sense keys generate uevent for each device > through sysfs entry sense_event_filter. For example, the following > enables uevent for MEDIUM_ERROR (0x03) and HARDWARE_ERROR (0x04) > on scsi drive sdc: > > echo 0x000c > /sys/block/sdc/device/sense_event_filter > > Here is an example output captured by udevadm: > > KERNEL[1214.945358] change /devices/pci0000:00/XXXXXXX > ACTION=change > DEVPATH=/devices/pci0000:00/0000:00:01.0/0000:01:00.0/host6/XXXXXXX > DEVTYPE=scsi_device > DRIVER=sd > LBA=0 > MODALIAS=scsi:t-0x00 > SDEV_UA=SCSI_SENSE > SENSE_CODE=3/11/14 > SEQNUM=4536 > SIZE=4096 > SUBSYSTEM=scsi > > Signed-off-by: Song Liu <songliubraving@fb.com> > --- > drivers/scsi/Kconfig | 14 +++++++++++ > drivers/scsi/scsi_error.c | 26 ++++++++++++++++++++ > drivers/scsi/scsi_lib.c | 27 +++++++++++++++++++-- > drivers/scsi/scsi_sysfs.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++ > include/scsi/scsi_device.h | 26 +++++++++++++++++++- > 5 files changed, 150 insertions(+), 3 deletions(-) > > diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig > index 3c52867..4f7f211 100644 > --- a/drivers/scsi/Kconfig > +++ b/drivers/scsi/Kconfig > @@ -237,6 +237,20 @@ config SCSI_LOGGING > there should be no noticeable performance impact as long as you have > logging turned off. > > +config SCSI_SENSE_UEVENT > + bool "SCSI sense code logging" > + depends on SCSI > + default n > + ---help--- > + This turns on uevent for SCSI sense code. > + > + You can configure which sense keys generate uevent for each device > + through sysfs entry sense_event_filter. For example, the following > + enables uevent for MEDIUM_ERROR (0x03) and HARDWARE_ERROR (0x04) > + on scsi drive sdc: > + > + echo 0x000c > /sys/block/sdc/device/sense_event_filter > + > config SCSI_SCAN_ASYNC > bool "Asynchronous SCSI scanning" > depends on SCSI > diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c > index d70c67c..eda150e 100644 > --- a/drivers/scsi/scsi_error.c > +++ b/drivers/scsi/scsi_error.c > @@ -426,6 +426,31 @@ static void scsi_report_sense(struct scsi_device *sdev, > } > } > > +/* > + * generate uevent when receiving sense code from device > + */ > +static void scsi_send_sense_uevent(struct scsi_device *sdev, > + struct scsi_cmnd *scmd, > + struct scsi_sense_hdr *sshdr) > +{ > +#ifdef CONFIG_SCSI_SENSE_UEVENT > + struct scsi_event *evt; > + > + if (!test_bit(sshdr->sense_key & 0xf, > + &sdev->sense_event_filter)) > + return; > + evt = sdev_evt_alloc(SDEV_EVT_SCSI_SENSE, GFP_ATOMIC); > + if (!evt) > + return; > + > + evt->sense_evt_data.lba = scsi_get_lba(scmd); > + evt->sense_evt_data.size = blk_rq_bytes(scmd->request); > + memcpy(&evt->sense_evt_data.sshdr, sshdr, > + sizeof(struct scsi_sense_hdr)); > + sdev_evt_send(sdev, evt); > +#endif > +} > + > /** > * scsi_check_sense - Examine scsi cmd sense > * @scmd: Cmd to have sense checked. > @@ -446,6 +471,7 @@ int scsi_check_sense(struct scsi_cmnd *scmd) > return FAILED; /* no valid sense data */ > > scsi_report_sense(sdev, &sshdr); > + scsi_send_sense_uevent(sdev, scmd, &sshdr); > > if (scsi_sense_is_deferred(&sshdr)) > return NEEDS_RETRY; > diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c > index 95f963b..1095f27 100644 > --- a/drivers/scsi/scsi_lib.c > +++ b/drivers/scsi/scsi_lib.c > @@ -2656,8 +2656,9 @@ EXPORT_SYMBOL(scsi_device_set_state); > */ > static void scsi_evt_emit(struct scsi_device *sdev, struct scsi_event *evt) > { > - int idx = 0; > - char *envp[3]; > + int idx = 0, i; > + char *envp[5]; /* SDEV_EVT_SCSI_SENSE needs most entries (4) */ > + int free_envp = -1; > > switch (evt->evt_type) { > case SDEV_EVT_MEDIA_CHANGE: > @@ -2682,6 +2683,23 @@ static void scsi_evt_emit(struct scsi_device *sdev, struct scsi_event *evt) > case SDEV_EVT_ALUA_STATE_CHANGE_REPORTED: > envp[idx++] = "SDEV_UA=ASYMMETRIC_ACCESS_STATE_CHANGED"; > break; > +#ifdef CONFIG_SCSI_SENSE_UEVENT > + case SDEV_EVT_SCSI_SENSE: > + envp[idx++] = "SDEV_UA=SCSI_SENSE"; > + for (i = idx; i < idx + 3; ++i) { > + envp[i] = kzalloc(32, GFP_ATOMIC); > + if (!envp[i]) > + break; > + free_envp = i; > + } > + snprintf(envp[idx++], 32, "LBA=%lu", evt->sense_evt_data.lba); > + snprintf(envp[idx++], 32, "SIZE=%d", evt->sense_evt_data.size); > + snprintf(envp[idx++], 32, "SENSE_CODE=%1x/%02x/%02x", > + evt->sense_evt_data.sshdr.sense_key, > + evt->sense_evt_data.sshdr.asc, > + evt->sense_evt_data.sshdr.ascq); > + break; > +#endif > default: > /* do nothing */ > break; > @@ -2690,6 +2708,10 @@ static void scsi_evt_emit(struct scsi_device *sdev, struct scsi_event *evt) > envp[idx++] = NULL; > > kobject_uevent_env(&sdev->sdev_gendev.kobj, KOBJ_CHANGE, envp); > + > + /* no need to free envp[0], so start with i = 1 */ > + for (i = 1 ; i < free_envp; ++i) > + kfree(envp[i]); > } > > /** > @@ -2786,6 +2808,7 @@ struct scsi_event *sdev_evt_alloc(enum scsi_device_event evt_type, > case SDEV_EVT_MODE_PARAMETER_CHANGE_REPORTED: > case SDEV_EVT_LUN_CHANGE_REPORTED: > case SDEV_EVT_ALUA_STATE_CHANGE_REPORTED: > + case SDEV_EVT_SCSI_SENSE: > default: > /* do nothing */ > break; > diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c > index 82dfe07..cfc7380 100644 > --- a/drivers/scsi/scsi_sysfs.c > +++ b/drivers/scsi/scsi_sysfs.c > @@ -1072,6 +1072,63 @@ static DEVICE_ATTR(queue_ramp_up_period, S_IRUGO | S_IWUSR, > sdev_show_queue_ramp_up_period, > sdev_store_queue_ramp_up_period); > > +#ifdef CONFIG_SCSI_SENSE_UEVENT > + > +/* > + * SCSI sense key could be 0x00 - 0x08, 0x0a, 0x0b, 0x0d, 0x0e, so the > + * mask is 0x6dff. > + */ > +#define SCSI_SENSE_EVENT_FILTER_MASK 0x6dff > + > +static ssize_t > +sdev_show_sense_event_filter(struct device *dev, > + struct device_attribute *attr, > + char *buf) > +{ > + struct scsi_device *sdev; > + > + sdev = to_scsi_device(dev); > + return snprintf(buf, 20, "0x%04lx\n", > + (sdev->sense_event_filter & > + SCSI_SENSE_EVENT_FILTER_MASK)); > +} > + > +static ssize_t > +sdev_store_sense_event_filter(struct device *dev, > + struct device_attribute *attr, > + const char *buf, size_t count) > +{ > + struct scsi_device *sdev = to_scsi_device(dev); > + unsigned long filter; > + int i; > + > + if (buf[0] == '0' && buf[1] == 'x') { > + if (kstrtoul(buf + 2, 16, &filter)) > + return -EINVAL; > + } else > + if (kstrtoul(buf, 10, &filter)) > + return -EINVAL; > + > + /* > + * Accurate mask for all sense keys is 0x6dff. However, we allow > + * user to enable event for all sense keys by echoing 0xffff > + */ > + if ((filter & 0xffff) != filter) > + return -EINVAL; > + > + for (i = 0; i < 15; i++) > + if (filter & SCSI_SENSE_EVENT_FILTER_MASK & (1 << i)) > + set_bit(i, &sdev->sense_event_filter); > + else > + clear_bit(i, &sdev->sense_event_filter); > + return count; > +} > + > +static DEVICE_ATTR(sense_event_filter, 0644, > + sdev_show_sense_event_filter, > + sdev_store_sense_event_filter); > +#endif > + > static umode_t scsi_sdev_attr_is_visible(struct kobject *kobj, > struct attribute *attr, int i) > { > @@ -1142,6 +1199,9 @@ static struct attribute *scsi_sdev_attrs[] = { > &dev_attr_preferred_path.attr, > #endif > &dev_attr_queue_ramp_up_period.attr, > +#ifdef CONFIG_SCSI_SENSE_UEVENT > + &dev_attr_sense_event_filter.attr, > +#endif > REF_EVT(media_change), > REF_EVT(inquiry_change_reported), > REF_EVT(capacity_change_reported), > diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h > index 05641ae..d160bd7 100644 > --- a/include/scsi/scsi_device.h > +++ b/include/scsi/scsi_device.h > @@ -64,13 +64,23 @@ enum scsi_device_event { > SDEV_EVT_MODE_PARAMETER_CHANGE_REPORTED, /* 2A 01 UA reported */ > SDEV_EVT_LUN_CHANGE_REPORTED, /* 3F 0E UA reported */ > SDEV_EVT_ALUA_STATE_CHANGE_REPORTED, /* 2A 06 UA reported */ > + SDEV_EVT_SCSI_SENSE, > > SDEV_EVT_FIRST = SDEV_EVT_MEDIA_CHANGE, > - SDEV_EVT_LAST = SDEV_EVT_ALUA_STATE_CHANGE_REPORTED, > + SDEV_EVT_LAST = SDEV_EVT_SCSI_SENSE, > > SDEV_EVT_MAXBITS = SDEV_EVT_LAST + 1 > }; > > +#ifdef CONFIG_SCSI_SENSE_UEVENT > +/* data for for SDEV_EVT_SCSI_SENSE */ > +struct scsi_sense_uevent_data { > + sector_t lba; /* LBA from the scsi command */ > + int size; /* size of the request */ > + struct scsi_sense_hdr sshdr; > +}; > +#endif > + > struct scsi_event { > enum scsi_device_event evt_type; > struct list_head node; > @@ -78,6 +88,11 @@ struct scsi_event { > /* put union of data structures, for non-simple event types, > * here > */ > + union { > +#ifdef CONFIG_SCSI_SENSE_UEVENT > + struct scsi_sense_uevent_data sense_evt_data; > +#endif > + }; > }; > > struct scsi_device { > @@ -197,6 +212,15 @@ struct scsi_device { > atomic_t iodone_cnt; > atomic_t ioerr_cnt; > > +#ifdef CONFIG_SCSI_SENSE_UEVENT > + /* > + * filter of sense code uevent > + * setting bit X (0x00 - 0x0e) of sense_event_filter enables > + * uevent for sense key X > + */ > + unsigned long sense_event_filter; > +#endif > + > struct device sdev_gendev, > sdev_dev; > > -- > 2.9.3 Dear Hannes and James, Could you please share your feedback on this change? Thanks, Song
On 05/12/2017 09:02 PM, Song Liu wrote: > >> On May 3, 2017, at 5:50 PM, Song Liu <songliubraving@fb.com> wrote: >> >> This patch adds capability for SCSI layer to generate uevent for SCSI >> sense code. The feature is gated by CONFIG_SCSI_SENSE_UEVENT. >> >> We can configure which sense keys generate uevent for each device >> through sysfs entry sense_event_filter. For example, the following >> enables uevent for MEDIUM_ERROR (0x03) and HARDWARE_ERROR (0x04) >> on scsi drive sdc: >> >> echo 0x000c > /sys/block/sdc/device/sense_event_filter >> >> Here is an example output captured by udevadm: >> >> KERNEL[1214.945358] change /devices/pci0000:00/XXXXXXX >> ACTION=change >> DEVPATH=/devices/pci0000:00/0000:00:01.0/0000:01:00.0/host6/XXXXXXX >> DEVTYPE=scsi_device >> DRIVER=sd >> LBA=0 >> MODALIAS=scsi:t-0x00 >> SDEV_UA=SCSI_SENSE >> SENSE_CODE=3/11/14 >> SEQNUM=4536 >> SIZE=4096 >> SUBSYSTEM=scsi >> >> Signed-off-by: Song Liu <songliubraving@fb.com> >> --- >> drivers/scsi/Kconfig | 14 +++++++++++ >> drivers/scsi/scsi_error.c | 26 ++++++++++++++++++++ >> drivers/scsi/scsi_lib.c | 27 +++++++++++++++++++-- >> drivers/scsi/scsi_sysfs.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++ >> include/scsi/scsi_device.h | 26 +++++++++++++++++++- >> 5 files changed, 150 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig >> index 3c52867..4f7f211 100644 >> --- a/drivers/scsi/Kconfig >> +++ b/drivers/scsi/Kconfig >> @@ -237,6 +237,20 @@ config SCSI_LOGGING >> there should be no noticeable performance impact as long as you have >> logging turned off. >> >> +config SCSI_SENSE_UEVENT >> + bool "SCSI sense code logging" >> + depends on SCSI >> + default n >> + ---help--- >> + This turns on uevent for SCSI sense code. >> + >> + You can configure which sense keys generate uevent for each device >> + through sysfs entry sense_event_filter. For example, the following >> + enables uevent for MEDIUM_ERROR (0x03) and HARDWARE_ERROR (0x04) >> + on scsi drive sdc: >> + >> + echo 0x000c > /sys/block/sdc/device/sense_event_filter >> + >> config SCSI_SCAN_ASYNC >> bool "Asynchronous SCSI scanning" >> depends on SCSI >> diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c >> index d70c67c..eda150e 100644 >> --- a/drivers/scsi/scsi_error.c >> +++ b/drivers/scsi/scsi_error.c >> @@ -426,6 +426,31 @@ static void scsi_report_sense(struct scsi_device *sdev, >> } >> } >> >> +/* >> + * generate uevent when receiving sense code from device >> + */ >> +static void scsi_send_sense_uevent(struct scsi_device *sdev, >> + struct scsi_cmnd *scmd, >> + struct scsi_sense_hdr *sshdr) >> +{ >> +#ifdef CONFIG_SCSI_SENSE_UEVENT >> + struct scsi_event *evt; >> + >> + if (!test_bit(sshdr->sense_key & 0xf, >> + &sdev->sense_event_filter)) >> + return; >> + evt = sdev_evt_alloc(SDEV_EVT_SCSI_SENSE, GFP_ATOMIC); >> + if (!evt) >> + return; >> + >> + evt->sense_evt_data.lba = scsi_get_lba(scmd); >> + evt->sense_evt_data.size = blk_rq_bytes(scmd->request); >> + memcpy(&evt->sense_evt_data.sshdr, sshdr, >> + sizeof(struct scsi_sense_hdr)); >> + sdev_evt_send(sdev, evt); >> +#endif >> +} >> + >> /** >> * scsi_check_sense - Examine scsi cmd sense >> * @scmd: Cmd to have sense checked. >> @@ -446,6 +471,7 @@ int scsi_check_sense(struct scsi_cmnd *scmd) >> return FAILED; /* no valid sense data */ >> >> scsi_report_sense(sdev, &sshdr); >> + scsi_send_sense_uevent(sdev, scmd, &sshdr); >> >> if (scsi_sense_is_deferred(&sshdr)) >> return NEEDS_RETRY; >> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c >> index 95f963b..1095f27 100644 >> --- a/drivers/scsi/scsi_lib.c >> +++ b/drivers/scsi/scsi_lib.c >> @@ -2656,8 +2656,9 @@ EXPORT_SYMBOL(scsi_device_set_state); >> */ >> static void scsi_evt_emit(struct scsi_device *sdev, struct scsi_event *evt) >> { >> - int idx = 0; >> - char *envp[3]; >> + int idx = 0, i; >> + char *envp[5]; /* SDEV_EVT_SCSI_SENSE needs most entries (4) */ >> + int free_envp = -1; >> >> switch (evt->evt_type) { >> case SDEV_EVT_MEDIA_CHANGE: >> @@ -2682,6 +2683,23 @@ static void scsi_evt_emit(struct scsi_device *sdev, struct scsi_event *evt) >> case SDEV_EVT_ALUA_STATE_CHANGE_REPORTED: >> envp[idx++] = "SDEV_UA=ASYMMETRIC_ACCESS_STATE_CHANGED"; >> break; >> +#ifdef CONFIG_SCSI_SENSE_UEVENT >> + case SDEV_EVT_SCSI_SENSE: >> + envp[idx++] = "SDEV_UA=SCSI_SENSE"; >> + for (i = idx; i < idx + 3; ++i) { >> + envp[i] = kzalloc(32, GFP_ATOMIC); >> + if (!envp[i]) >> + break; >> + free_envp = i; >> + } >> + snprintf(envp[idx++], 32, "LBA=%lu", evt->sense_evt_data.lba); >> + snprintf(envp[idx++], 32, "SIZE=%d", evt->sense_evt_data.size); >> + snprintf(envp[idx++], 32, "SENSE_CODE=%1x/%02x/%02x", >> + evt->sense_evt_data.sshdr.sense_key, >> + evt->sense_evt_data.sshdr.asc, >> + evt->sense_evt_data.sshdr.ascq); >> + break; >> +#endif >> default: >> /* do nothing */ >> break; >> @@ -2690,6 +2708,10 @@ static void scsi_evt_emit(struct scsi_device *sdev, struct scsi_event *evt) >> envp[idx++] = NULL; >> >> kobject_uevent_env(&sdev->sdev_gendev.kobj, KOBJ_CHANGE, envp); >> + >> + /* no need to free envp[0], so start with i = 1 */ >> + for (i = 1 ; i < free_envp; ++i) >> + kfree(envp[i]); >> } >> This can't be right; you're just allocating envp starting from 'idx', but free up all of them. >> /** >> @@ -2786,6 +2808,7 @@ struct scsi_event *sdev_evt_alloc(enum scsi_device_event evt_type, >> case SDEV_EVT_MODE_PARAMETER_CHANGE_REPORTED: >> case SDEV_EVT_LUN_CHANGE_REPORTED: >> case SDEV_EVT_ALUA_STATE_CHANGE_REPORTED: >> + case SDEV_EVT_SCSI_SENSE: >> default: >> /* do nothing */ >> break; >> diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c >> index 82dfe07..cfc7380 100644 >> --- a/drivers/scsi/scsi_sysfs.c >> +++ b/drivers/scsi/scsi_sysfs.c >> @@ -1072,6 +1072,63 @@ static DEVICE_ATTR(queue_ramp_up_period, S_IRUGO | S_IWUSR, >> sdev_show_queue_ramp_up_period, >> sdev_store_queue_ramp_up_period); >> >> +#ifdef CONFIG_SCSI_SENSE_UEVENT >> + >> +/* >> + * SCSI sense key could be 0x00 - 0x08, 0x0a, 0x0b, 0x0d, 0x0e, so the >> + * mask is 0x6dff. >> + */ >> +#define SCSI_SENSE_EVENT_FILTER_MASK 0x6dff >> + >> +static ssize_t >> +sdev_show_sense_event_filter(struct device *dev, >> + struct device_attribute *attr, >> + char *buf) >> +{ >> + struct scsi_device *sdev; >> + >> + sdev = to_scsi_device(dev); >> + return snprintf(buf, 20, "0x%04lx\n", >> + (sdev->sense_event_filter & >> + SCSI_SENSE_EVENT_FILTER_MASK)); >> +} >> + >> +static ssize_t >> +sdev_store_sense_event_filter(struct device *dev, >> + struct device_attribute *attr, >> + const char *buf, size_t count) >> +{ >> + struct scsi_device *sdev = to_scsi_device(dev); >> + unsigned long filter; >> + int i; >> + >> + if (buf[0] == '0' && buf[1] == 'x') { >> + if (kstrtoul(buf + 2, 16, &filter)) >> + return -EINVAL; >> + } else >> + if (kstrtoul(buf, 10, &filter)) >> + return -EINVAL; >> + >> + /* >> + * Accurate mask for all sense keys is 0x6dff. However, we allow >> + * user to enable event for all sense keys by echoing 0xffff >> + */ >> + if ((filter & 0xffff) != filter) >> + return -EINVAL; >> + >> + for (i = 0; i < 15; i++) >> + if (filter & SCSI_SENSE_EVENT_FILTER_MASK & (1 << i)) >> + set_bit(i, &sdev->sense_event_filter); >> + else >> + clear_bit(i, &sdev->sense_event_filter); >> + return count; >> +} >> + >> +static DEVICE_ATTR(sense_event_filter, 0644, >> + sdev_show_sense_event_filter, >> + sdev_store_sense_event_filter); >> +#endif >> + >> static umode_t scsi_sdev_attr_is_visible(struct kobject *kobj, >> struct attribute *attr, int i) >> { >> @@ -1142,6 +1199,9 @@ static struct attribute *scsi_sdev_attrs[] = { >> &dev_attr_preferred_path.attr, >> #endif >> &dev_attr_queue_ramp_up_period.attr, >> +#ifdef CONFIG_SCSI_SENSE_UEVENT >> + &dev_attr_sense_event_filter.attr, >> +#endif >> REF_EVT(media_change), >> REF_EVT(inquiry_change_reported), >> REF_EVT(capacity_change_reported), >> diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h >> index 05641ae..d160bd7 100644 >> --- a/include/scsi/scsi_device.h >> +++ b/include/scsi/scsi_device.h >> @@ -64,13 +64,23 @@ enum scsi_device_event { >> SDEV_EVT_MODE_PARAMETER_CHANGE_REPORTED, /* 2A 01 UA reported */ >> SDEV_EVT_LUN_CHANGE_REPORTED, /* 3F 0E UA reported */ >> SDEV_EVT_ALUA_STATE_CHANGE_REPORTED, /* 2A 06 UA reported */ >> + SDEV_EVT_SCSI_SENSE, >> >> SDEV_EVT_FIRST = SDEV_EVT_MEDIA_CHANGE, >> - SDEV_EVT_LAST = SDEV_EVT_ALUA_STATE_CHANGE_REPORTED, >> + SDEV_EVT_LAST = SDEV_EVT_SCSI_SENSE, >> >> SDEV_EVT_MAXBITS = SDEV_EVT_LAST + 1 >> }; >> >> +#ifdef CONFIG_SCSI_SENSE_UEVENT >> +/* data for for SDEV_EVT_SCSI_SENSE */ >> +struct scsi_sense_uevent_data { >> + sector_t lba; /* LBA from the scsi command */ >> + int size; /* size of the request */ >> + struct scsi_sense_hdr sshdr; >> +}; >> +#endif >> + >> struct scsi_event { >> enum scsi_device_event evt_type; >> struct list_head node; >> @@ -78,6 +88,11 @@ struct scsi_event { >> /* put union of data structures, for non-simple event types, >> * here >> */ >> + union { >> +#ifdef CONFIG_SCSI_SENSE_UEVENT >> + struct scsi_sense_uevent_data sense_evt_data; >> +#endif >> + }; >> }; >> >> struct scsi_device { >> @@ -197,6 +212,15 @@ struct scsi_device { >> atomic_t iodone_cnt; >> atomic_t ioerr_cnt; >> >> +#ifdef CONFIG_SCSI_SENSE_UEVENT >> + /* >> + * filter of sense code uevent >> + * setting bit X (0x00 - 0x0e) of sense_event_filter enables >> + * uevent for sense key X >> + */ >> + unsigned long sense_event_filter; >> +#endif >> + >> struct device sdev_gendev, >> sdev_dev; >> >> -- >> 2.9.3 > In general I like the idea; however, the 'filter' thingie is somewhat odd. I could somewhat buy into the idea of filtering for sense keys, but then I would have expected to use the 'sense_event_filter' as a bitmap of allowed sense keys. With your current design you can only filter for one specific sense key, _and_ you leave the remaining bits of the sense_event_filter variable unused. So either turn it into a bitmap and allow for several sense keys to be filtered, or treat it as mask for the _entire_ sense, but then you'd need for ASC and ASCQ, too. Cheers, Hannes
On Mon, 2017-05-15 at 08:04 +0200, Hannes Reinecke wrote: > In general I like the idea; however, the 'filter' thingie is somewhat > odd. I could somewhat buy into the idea of filtering for sense keys, but > then I would have expected to use the 'sense_event_filter' as a bitmap > of allowed sense keys. > With your current design you can only filter for one specific sense key, > _and_ you leave the remaining bits of the sense_event_filter variable > unused. > So either turn it into a bitmap and allow for several sense keys to be > filtered, or treat it as mask for the _entire_ sense, but then you'd > need for ASC and ASCQ, too. > > Cheers, > > Hannes I think what would help here is understanding what you are trying to accomplish with this change. Are you trying to allow generation of a uevent on a particular sense KCQ combination you know in advance, or a set of them, or do you want to be able to normally generate uevents for every sense keys/codes (i.e. all the bitmap bits set) and then perhaps narrow down to a specific one when you are looking for a problem? When I added the sense code uevent generation for Unit Attentions, one big concern I had was how many events per second would be generated. The userspace event handling is very slow, and can only handle a few hundred events per second before it starts backing up, and then you can lose events, which you don't want. Generating a uevent on MEDIUM ERROR is somewhat worrying, when a drive goes bad you could easily get these on every I/O. Your actual uevent emit: +#ifdef CONFIG_SCSI_SENSE_UEVENT + case SDEV_EVT_SCSI_SENSE: + envp[idx++] = "SDEV_UA=SCSI_SENSE"; + for (i = idx; i < idx + 3; ++i) { + envp[i] = kzalloc(32, GFP_ATOMIC); + if (!envp[i]) + break; + free_envp = i; + } + snprintf(envp[idx++], 32, "LBA=%lu", evt->sense_evt_data.lba); + snprintf(envp[idx++], 32, "SIZE=%d", evt->sense_evt_data.size); + snprintf(envp[idx++], 32, "SENSE_CODE=%1x/%02x/%02x", + evt->sense_evt_data.sshdr.sense_key, + evt->sense_evt_data.sshdr.asc, + evt->sense_evt_data.sshdr.ascq); + break; +#endif Is a little strange. The "SDEV_UA" property implies that it was a UNIT ATTENTION, but you are filtering on other sense types, so you would probably want a different property. Many SCSI commands do not have a LOGICAL BLOCK ADDRESS field, or a TRANSFER LENGTH field, so you might be reporting garbage if they don't. And, since this is a SCSI command, you probably want to report the length from the command, not the count in bytes from the request structure. So I think a customizable sense reporting feature could be very useful, but it needs a little more development. -Ewan
> On May 14, 2017, at 11:04 PM, Hannes Reinecke <hare@suse.de> wrote: > > On 05/12/2017 09:02 PM, Song Liu wrote: >> >>> On May 3, 2017, at 5:50 PM, Song Liu <songliubraving@fb.com> wrote: >>> >>> This patch adds capability for SCSI layer to generate uevent for SCSI >>> sense code. The feature is gated by CONFIG_SCSI_SENSE_UEVENT. >>> >>> We can configure which sense keys generate uevent for each device >>> through sysfs entry sense_event_filter. For example, the following >>> enables uevent for MEDIUM_ERROR (0x03) and HARDWARE_ERROR (0x04) >>> on scsi drive sdc: >>> >>> echo 0x000c > /sys/block/sdc/device/sense_event_filter >>> >>> Here is an example output captured by udevadm: >>> >>> KERNEL[1214.945358] change /devices/pci0000:00/XXXXXXX >>> ACTION=change >>> DEVPATH=/devices/pci0000:00/0000:00:01.0/0000:01:00.0/host6/XXXXXXX >>> DEVTYPE=scsi_device >>> DRIVER=sd >>> LBA=0 >>> MODALIAS=scsi:t-0x00 >>> SDEV_UA=SCSI_SENSE >>> SENSE_CODE=3/11/14 >>> SEQNUM=4536 >>> SIZE=4096 >>> SUBSYSTEM=scsi >>> >>> Signed-off-by: Song Liu <songliubraving@fb.com> >>> --- >>> drivers/scsi/Kconfig | 14 +++++++++++ >>> drivers/scsi/scsi_error.c | 26 ++++++++++++++++++++ >>> drivers/scsi/scsi_lib.c | 27 +++++++++++++++++++-- >>> drivers/scsi/scsi_sysfs.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++ >>> include/scsi/scsi_device.h | 26 +++++++++++++++++++- >>> 5 files changed, 150 insertions(+), 3 deletions(-) >>> >>> diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig >>> index 3c52867..4f7f211 100644 >>> --- a/drivers/scsi/Kconfig >>> +++ b/drivers/scsi/Kconfig >>> @@ -237,6 +237,20 @@ config SCSI_LOGGING >>> there should be no noticeable performance impact as long as you have >>> logging turned off. >>> >>> +config SCSI_SENSE_UEVENT >>> + bool "SCSI sense code logging" >>> + depends on SCSI >>> + default n >>> + ---help--- >>> + This turns on uevent for SCSI sense code. >>> + >>> + You can configure which sense keys generate uevent for each device >>> + through sysfs entry sense_event_filter. For example, the following >>> + enables uevent for MEDIUM_ERROR (0x03) and HARDWARE_ERROR (0x04) >>> + on scsi drive sdc: >>> + >>> + echo 0x000c > /sys/block/sdc/device/sense_event_filter >>> + >>> config SCSI_SCAN_ASYNC >>> bool "Asynchronous SCSI scanning" >>> depends on SCSI >>> diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c >>> index d70c67c..eda150e 100644 >>> --- a/drivers/scsi/scsi_error.c >>> +++ b/drivers/scsi/scsi_error.c >>> @@ -426,6 +426,31 @@ static void scsi_report_sense(struct scsi_device *sdev, >>> } >>> } >>> >>> +/* >>> + * generate uevent when receiving sense code from device >>> + */ >>> +static void scsi_send_sense_uevent(struct scsi_device *sdev, >>> + struct scsi_cmnd *scmd, >>> + struct scsi_sense_hdr *sshdr) >>> +{ >>> +#ifdef CONFIG_SCSI_SENSE_UEVENT >>> + struct scsi_event *evt; >>> + >>> + if (!test_bit(sshdr->sense_key & 0xf, >>> + &sdev->sense_event_filter)) >>> + return; >>> + evt = sdev_evt_alloc(SDEV_EVT_SCSI_SENSE, GFP_ATOMIC); >>> + if (!evt) >>> + return; >>> + >>> + evt->sense_evt_data.lba = scsi_get_lba(scmd); >>> + evt->sense_evt_data.size = blk_rq_bytes(scmd->request); >>> + memcpy(&evt->sense_evt_data.sshdr, sshdr, >>> + sizeof(struct scsi_sense_hdr)); >>> + sdev_evt_send(sdev, evt); >>> +#endif >>> +} >>> + >>> /** >>> * scsi_check_sense - Examine scsi cmd sense >>> * @scmd: Cmd to have sense checked. >>> @@ -446,6 +471,7 @@ int scsi_check_sense(struct scsi_cmnd *scmd) >>> return FAILED; /* no valid sense data */ >>> >>> scsi_report_sense(sdev, &sshdr); >>> + scsi_send_sense_uevent(sdev, scmd, &sshdr); >>> >>> if (scsi_sense_is_deferred(&sshdr)) >>> return NEEDS_RETRY; >>> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c >>> index 95f963b..1095f27 100644 >>> --- a/drivers/scsi/scsi_lib.c >>> +++ b/drivers/scsi/scsi_lib.c >>> @@ -2656,8 +2656,9 @@ EXPORT_SYMBOL(scsi_device_set_state); >>> */ >>> static void scsi_evt_emit(struct scsi_device *sdev, struct scsi_event *evt) >>> { >>> - int idx = 0; >>> - char *envp[3]; >>> + int idx = 0, i; >>> + char *envp[5]; /* SDEV_EVT_SCSI_SENSE needs most entries (4) */ >>> + int free_envp = -1; >>> >>> switch (evt->evt_type) { >>> case SDEV_EVT_MEDIA_CHANGE: >>> @@ -2682,6 +2683,23 @@ static void scsi_evt_emit(struct scsi_device *sdev, struct scsi_event *evt) >>> case SDEV_EVT_ALUA_STATE_CHANGE_REPORTED: >>> envp[idx++] = "SDEV_UA=ASYMMETRIC_ACCESS_STATE_CHANGED"; >>> break; >>> +#ifdef CONFIG_SCSI_SENSE_UEVENT >>> + case SDEV_EVT_SCSI_SENSE: >>> + envp[idx++] = "SDEV_UA=SCSI_SENSE"; >>> + for (i = idx; i < idx + 3; ++i) { >>> + envp[i] = kzalloc(32, GFP_ATOMIC); >>> + if (!envp[i]) >>> + break; >>> + free_envp = i; >>> + } >>> + snprintf(envp[idx++], 32, "LBA=%lu", evt->sense_evt_data.lba); >>> + snprintf(envp[idx++], 32, "SIZE=%d", evt->sense_evt_data.size); >>> + snprintf(envp[idx++], 32, "SENSE_CODE=%1x/%02x/%02x", >>> + evt->sense_evt_data.sshdr.sense_key, >>> + evt->sense_evt_data.sshdr.asc, >>> + evt->sense_evt_data.sshdr.ascq); >>> + break; >>> +#endif >>> default: >>> /* do nothing */ >>> break; >>> @@ -2690,6 +2708,10 @@ static void scsi_evt_emit(struct scsi_device *sdev, struct scsi_event *evt) >>> envp[idx++] = NULL; >>> >>> kobject_uevent_env(&sdev->sdev_gendev.kobj, KOBJ_CHANGE, envp); >>> + >>> + /* no need to free envp[0], so start with i = 1 */ >>> + for (i = 1 ; i < free_envp; ++i) >>> + kfree(envp[i]); >>> } >>> > This can't be right; you're just allocating envp starting from 'idx', > but free up all of them. Yeah, I did some math manually, which is not good. I will fix it with one more variables (free_start_envp). > >>> /** >>> @@ -2786,6 +2808,7 @@ struct scsi_event *sdev_evt_alloc(enum scsi_device_event evt_type, >>> case SDEV_EVT_MODE_PARAMETER_CHANGE_REPORTED: >>> case SDEV_EVT_LUN_CHANGE_REPORTED: >>> case SDEV_EVT_ALUA_STATE_CHANGE_REPORTED: >>> + case SDEV_EVT_SCSI_SENSE: >>> default: >>> /* do nothing */ >>> break; >>> diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c >>> index 82dfe07..cfc7380 100644 >>> --- a/drivers/scsi/scsi_sysfs.c >>> +++ b/drivers/scsi/scsi_sysfs.c >>> @@ -1072,6 +1072,63 @@ static DEVICE_ATTR(queue_ramp_up_period, S_IRUGO | S_IWUSR, >>> sdev_show_queue_ramp_up_period, >>> sdev_store_queue_ramp_up_period); >>> >>> +#ifdef CONFIG_SCSI_SENSE_UEVENT >>> + >>> +/* >>> + * SCSI sense key could be 0x00 - 0x08, 0x0a, 0x0b, 0x0d, 0x0e, so the >>> + * mask is 0x6dff. >>> + */ >>> +#define SCSI_SENSE_EVENT_FILTER_MASK 0x6dff >>> + >>> +static ssize_t >>> +sdev_show_sense_event_filter(struct device *dev, >>> + struct device_attribute *attr, >>> + char *buf) >>> +{ >>> + struct scsi_device *sdev; >>> + >>> + sdev = to_scsi_device(dev); >>> + return snprintf(buf, 20, "0x%04lx\n", >>> + (sdev->sense_event_filter & >>> + SCSI_SENSE_EVENT_FILTER_MASK)); >>> +} >>> + >>> +static ssize_t >>> +sdev_store_sense_event_filter(struct device *dev, >>> + struct device_attribute *attr, >>> + const char *buf, size_t count) >>> +{ >>> + struct scsi_device *sdev = to_scsi_device(dev); >>> + unsigned long filter; >>> + int i; >>> + >>> + if (buf[0] == '0' && buf[1] == 'x') { >>> + if (kstrtoul(buf + 2, 16, &filter)) >>> + return -EINVAL; >>> + } else >>> + if (kstrtoul(buf, 10, &filter)) >>> + return -EINVAL; >>> + >>> + /* >>> + * Accurate mask for all sense keys is 0x6dff. However, we allow >>> + * user to enable event for all sense keys by echoing 0xffff >>> + */ >>> + if ((filter & 0xffff) != filter) >>> + return -EINVAL; >>> + >>> + for (i = 0; i < 15; i++) >>> + if (filter & SCSI_SENSE_EVENT_FILTER_MASK & (1 << i)) >>> + set_bit(i, &sdev->sense_event_filter); >>> + else >>> + clear_bit(i, &sdev->sense_event_filter); >>> + return count; >>> +} >>> + >>> +static DEVICE_ATTR(sense_event_filter, 0644, >>> + sdev_show_sense_event_filter, >>> + sdev_store_sense_event_filter); >>> +#endif >>> + >>> static umode_t scsi_sdev_attr_is_visible(struct kobject *kobj, >>> struct attribute *attr, int i) >>> { >>> @@ -1142,6 +1199,9 @@ static struct attribute *scsi_sdev_attrs[] = { >>> &dev_attr_preferred_path.attr, >>> #endif >>> &dev_attr_queue_ramp_up_period.attr, >>> +#ifdef CONFIG_SCSI_SENSE_UEVENT >>> + &dev_attr_sense_event_filter.attr, >>> +#endif >>> REF_EVT(media_change), >>> REF_EVT(inquiry_change_reported), >>> REF_EVT(capacity_change_reported), >>> diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h >>> index 05641ae..d160bd7 100644 >>> --- a/include/scsi/scsi_device.h >>> +++ b/include/scsi/scsi_device.h >>> @@ -64,13 +64,23 @@ enum scsi_device_event { >>> SDEV_EVT_MODE_PARAMETER_CHANGE_REPORTED, /* 2A 01 UA reported */ >>> SDEV_EVT_LUN_CHANGE_REPORTED, /* 3F 0E UA reported */ >>> SDEV_EVT_ALUA_STATE_CHANGE_REPORTED, /* 2A 06 UA reported */ >>> + SDEV_EVT_SCSI_SENSE, >>> >>> SDEV_EVT_FIRST = SDEV_EVT_MEDIA_CHANGE, >>> - SDEV_EVT_LAST = SDEV_EVT_ALUA_STATE_CHANGE_REPORTED, >>> + SDEV_EVT_LAST = SDEV_EVT_SCSI_SENSE, >>> >>> SDEV_EVT_MAXBITS = SDEV_EVT_LAST + 1 >>> }; >>> >>> +#ifdef CONFIG_SCSI_SENSE_UEVENT >>> +/* data for for SDEV_EVT_SCSI_SENSE */ >>> +struct scsi_sense_uevent_data { >>> + sector_t lba; /* LBA from the scsi command */ >>> + int size; /* size of the request */ >>> + struct scsi_sense_hdr sshdr; >>> +}; >>> +#endif >>> + >>> struct scsi_event { >>> enum scsi_device_event evt_type; >>> struct list_head node; >>> @@ -78,6 +88,11 @@ struct scsi_event { >>> /* put union of data structures, for non-simple event types, >>> * here >>> */ >>> + union { >>> +#ifdef CONFIG_SCSI_SENSE_UEVENT >>> + struct scsi_sense_uevent_data sense_evt_data; >>> +#endif >>> + }; >>> }; >>> >>> struct scsi_device { >>> @@ -197,6 +212,15 @@ struct scsi_device { >>> atomic_t iodone_cnt; >>> atomic_t ioerr_cnt; >>> >>> +#ifdef CONFIG_SCSI_SENSE_UEVENT >>> + /* >>> + * filter of sense code uevent >>> + * setting bit X (0x00 - 0x0e) of sense_event_filter enables >>> + * uevent for sense key X >>> + */ >>> + unsigned long sense_event_filter; >>> +#endif >>> + >>> struct device sdev_gendev, >>> sdev_dev; >>> >>> -- >>> 2.9.3 >> > In general I like the idea; however, the 'filter' thingie is somewhat > odd. I could somewhat buy into the idea of filtering for sense keys, but > then I would have expected to use the 'sense_event_filter' as a bitmap > of allowed sense keys. > With your current design you can only filter for one specific sense key, > _and_ you leave the remaining bits of the sense_event_filter variable > unused. > So either turn it into a bitmap and allow for several sense keys to be > filtered, or treat it as mask for the _entire_ sense, but then you'd > need for ASC and ASCQ, too. Maybe I didn't explain/implement this cleanly, but my original design is to use the filter as a bitmap: >>> + if (!test_bit(sshdr->sense_key & 0xf, >>> + &sdev->sense_event_filter)) >>> + return; If the filter is set to 0x000f, the code will generate event for sense keys 0, 1, 2, and 3, but not other sense keys. Thanks, Song
> On May 15, 2017, at 5:31 AM, Ewan D. Milne <emilne@redhat.com> wrote: > > On Mon, 2017-05-15 at 08:04 +0200, Hannes Reinecke wrote: >> In general I like the idea; however, the 'filter' thingie is somewhat >> odd. I could somewhat buy into the idea of filtering for sense keys, but >> then I would have expected to use the 'sense_event_filter' as a bitmap >> of allowed sense keys. >> With your current design you can only filter for one specific sense key, >> _and_ you leave the remaining bits of the sense_event_filter variable >> unused. >> So either turn it into a bitmap and allow for several sense keys to be >> filtered, or treat it as mask for the _entire_ sense, but then you'd >> need for ASC and ASCQ, too. >> >> Cheers, >> >> Hannes > > I think what would help here is understanding what you are trying to > accomplish with this change. Are you trying to allow generation of > a uevent on a particular sense KCQ combination you know in advance, > or a set of them, or do you want to be able to normally generate > uevents for every sense keys/codes (i.e. all the bitmap bits set) > and then perhaps narrow down to a specific one when you are looking > for a problem? Thanks for these feedbacks! My goal here is to generate uevent for a set of sense keys. I would like to store these information for a fleet of many HDDs over a couple years. We are hoping to use these information to get insights in the quality of the HDDs. > When I added the sense code uevent generation for Unit Attentions, > one big concern I had was how many events per second would be generated. > The userspace event handling is very slow, and can only handle a few > hundred events per second before it starts backing up, and then you > can lose events, which you don't want. Thanks for this information. I am trying to limit the rate with the filter. But I am not sure whether this is sufficient. > Generating a uevent on MEDIUM ERROR is somewhat worrying, when a drive > goes bad you could easily get these on every I/O. MEDIUM ERROR is an important event that we would like to keep. Maybe we need better mechanism for rate limit, for example, no more than 10 MEDIUM ERROR per second per drive. > > Your actual uevent emit: > > +#ifdef CONFIG_SCSI_SENSE_UEVENT > + case SDEV_EVT_SCSI_SENSE: > + envp[idx++] = "SDEV_UA=SCSI_SENSE"; > + for (i = idx; i < idx + 3; ++i) { > + envp[i] = kzalloc(32, GFP_ATOMIC); > + if (!envp[i]) > + break; > + free_envp = i; > + } > + snprintf(envp[idx++], 32, "LBA=%lu", evt->sense_evt_data.lba); > + snprintf(envp[idx++], 32, "SIZE=%d", evt->sense_evt_data.size); > + snprintf(envp[idx++], 32, "SENSE_CODE=%1x/%02x/%02x", > + evt->sense_evt_data.sshdr.sense_key, > + evt->sense_evt_data.sshdr.asc, > + evt->sense_evt_data.sshdr.ascq); > + break; > +#endif > > Is a little strange. The "SDEV_UA" property implies that it was a > UNIT ATTENTION, but you are filtering on other sense types, so you > would probably want a different property. Many SCSI commands do not > have a LOGICAL BLOCK ADDRESS field, or a TRANSFER LENGTH field, so > you might be reporting garbage if they don't. And, since this is a > SCSI command, you probably want to report the length from the command, > not the count in bytes from the request structure. I didn't realize SDEV_UA means UNIT ATTENTION. Let me think about a different name. For the data being logged, I was thinking to log the whole SCSI command and full sense buffer. Let me try a few different things. Thanks again, Song
Hello, On Wed, May 03, 2017 at 05:50:13PM -0700, Song Liu wrote: > This patch adds capability for SCSI layer to generate uevent for SCSI > sense code. The feature is gated by CONFIG_SCSI_SENSE_UEVENT. > > We can configure which sense keys generate uevent for each device > through sysfs entry sense_event_filter. For example, the following > enables uevent for MEDIUM_ERROR (0x03) and HARDWARE_ERROR (0x04) > on scsi drive sdc: > > echo 0x000c > /sys/block/sdc/device/sense_event_filter > I know its an RFC, but the user-interface definitely needs better documentation. You also don't mention the 'wildcard' you document in the code below. Would there be any public tooling for this, when it gets in? There is already some events we can generate.. for example when the host-mapping changed, but I am not aware of any tooling that would ever make use of them (although, this probably would even be vert useful). And in the end that seems like dead-code for me. But maybe thats just me not knowing the tooling. > > Here is an example output captured by udevadm: > > KERNEL[1214.945358] change /devices/pci0000:00/XXXXXXX > ACTION=change > DEVPATH=/devices/pci0000:00/0000:00:01.0/0000:01:00.0/host6/XXXXXXX > DEVTYPE=scsi_device > DRIVER=sd > LBA=0 > MODALIAS=scsi:t-0x00 > SDEV_UA=SCSI_SENSE > SENSE_CODE=3/11/14 > SEQNUM=4536 > SIZE=4096 > SUBSYSTEM=scsi > > Signed-off-by: Song Liu <songliubraving@fb.com> > --- > drivers/scsi/Kconfig | 14 +++++++++++ > drivers/scsi/scsi_error.c | 26 ++++++++++++++++++++ > drivers/scsi/scsi_lib.c | 27 +++++++++++++++++++-- > drivers/scsi/scsi_sysfs.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++ > include/scsi/scsi_device.h | 26 +++++++++++++++++++- > 5 files changed, 150 insertions(+), 3 deletions(-) > > diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig > index 3c52867..4f7f211 100644 > --- a/drivers/scsi/Kconfig > +++ b/drivers/scsi/Kconfig > @@ -237,6 +237,20 @@ config SCSI_LOGGING > there should be no noticeable performance impact as long as you have > logging turned off. > > +config SCSI_SENSE_UEVENT > + bool "SCSI sense code logging" > + depends on SCSI > + default n > + ---help--- > + This turns on uevent for SCSI sense code. > + > + You can configure which sense keys generate uevent for each device > + through sysfs entry sense_event_filter. For example, the following > + enables uevent for MEDIUM_ERROR (0x03) and HARDWARE_ERROR (0x04) > + on scsi drive sdc: > + > + echo 0x000c > /sys/block/sdc/device/sense_event_filter > + > config SCSI_SCAN_ASYNC > bool "Asynchronous SCSI scanning" > depends on SCSI > diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c > index d70c67c..eda150e 100644 > --- a/drivers/scsi/scsi_error.c > +++ b/drivers/scsi/scsi_error.c > @@ -426,6 +426,31 @@ static void scsi_report_sense(struct scsi_device *sdev, > } > } > > +/* > + * generate uevent when receiving sense code from device > + */ > +static void scsi_send_sense_uevent(struct scsi_device *sdev, > + struct scsi_cmnd *scmd, > + struct scsi_sense_hdr *sshdr) > +{ > +#ifdef CONFIG_SCSI_SENSE_UEVENT > + struct scsi_event *evt; > + > + if (!test_bit(sshdr->sense_key & 0xf, > + &sdev->sense_event_filter)) > + return; > + evt = sdev_evt_alloc(SDEV_EVT_SCSI_SENSE, GFP_ATOMIC); > + if (!evt) > + return; > + > + evt->sense_evt_data.lba = scsi_get_lba(scmd); > + evt->sense_evt_data.size = blk_rq_bytes(scmd->request); > + memcpy(&evt->sense_evt_data.sshdr, sshdr, > + sizeof(struct scsi_sense_hdr)); > + sdev_evt_send(sdev, evt); > +#endif > +} > + So when I turn this CONFIG_ off, do I get all kinds of warning about unused variables? > /** > * scsi_check_sense - Examine scsi cmd sense > * @scmd: Cmd to have sense checked. > @@ -446,6 +471,7 @@ int scsi_check_sense(struct scsi_cmnd *scmd) > return FAILED; /* no valid sense data */ > > scsi_report_sense(sdev, &sshdr); > + scsi_send_sense_uevent(sdev, scmd, &sshdr); > > if (scsi_sense_is_deferred(&sshdr)) > return NEEDS_RETRY; > diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c > index 95f963b..1095f27 100644 > --- a/drivers/scsi/scsi_lib.c > +++ b/drivers/scsi/scsi_lib.c > @@ -2656,8 +2656,9 @@ EXPORT_SYMBOL(scsi_device_set_state); > */ > static void scsi_evt_emit(struct scsi_device *sdev, struct scsi_event *evt) > { > - int idx = 0; > - char *envp[3]; > + int idx = 0, i; > + char *envp[5]; /* SDEV_EVT_SCSI_SENSE needs most entries (4) */ > + int free_envp = -1; > > switch (evt->evt_type) { > case SDEV_EVT_MEDIA_CHANGE: > @@ -2682,6 +2683,23 @@ static void scsi_evt_emit(struct scsi_device *sdev, struct scsi_event *evt) > case SDEV_EVT_ALUA_STATE_CHANGE_REPORTED: > envp[idx++] = "SDEV_UA=ASYMMETRIC_ACCESS_STATE_CHANGED"; > break; > +#ifdef CONFIG_SCSI_SENSE_UEVENT > + case SDEV_EVT_SCSI_SENSE: > + envp[idx++] = "SDEV_UA=SCSI_SENSE"; > + for (i = idx; i < idx + 3; ++i) { > + envp[i] = kzalloc(32, GFP_ATOMIC); Why try so hard with GFP_ATOMIC? This is run in its own work-item and AFAIK there is no need to be so strict here. > + if (!envp[i]) > + break; The error-handling seems a bit optimistic. Especially when your write on the pointers afterwards. > + free_envp = i; > + } > + snprintf(envp[idx++], 32, "LBA=%lu", evt->sense_evt_data.lba); > + snprintf(envp[idx++], 32, "SIZE=%d", evt->sense_evt_data.size); > + snprintf(envp[idx++], 32, "SENSE_CODE=%1x/%02x/%02x", > + evt->sense_evt_data.sshdr.sense_key, > + evt->sense_evt_data.sshdr.asc, > + evt->sense_evt_data.sshdr.ascq); > + break; How do you get to this 3x'32'? Seems like quite some waste when we talk about frequent events. > +#endif > default: > /* do nothing */ > break; > @@ -2690,6 +2708,10 @@ static void scsi_evt_emit(struct scsi_device *sdev, struct scsi_event *evt) > envp[idx++] = NULL; > > kobject_uevent_env(&sdev->sdev_gendev.kobj, KOBJ_CHANGE, envp); > + > + /* no need to free envp[0], so start with i = 1 */ > + for (i = 1 ; i < free_envp; ++i) > + kfree(envp[i]); > } > > /** > @@ -2786,6 +2808,7 @@ struct scsi_event *sdev_evt_alloc(enum scsi_device_event evt_type, > case SDEV_EVT_MODE_PARAMETER_CHANGE_REPORTED: > case SDEV_EVT_LUN_CHANGE_REPORTED: > case SDEV_EVT_ALUA_STATE_CHANGE_REPORTED: > + case SDEV_EVT_SCSI_SENSE: > default: > /* do nothing */ > break; > diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c > index 82dfe07..cfc7380 100644 > --- a/drivers/scsi/scsi_sysfs.c > +++ b/drivers/scsi/scsi_sysfs.c > @@ -1072,6 +1072,63 @@ static DEVICE_ATTR(queue_ramp_up_period, S_IRUGO | S_IWUSR, > sdev_show_queue_ramp_up_period, > sdev_store_queue_ramp_up_period); > > +#ifdef CONFIG_SCSI_SENSE_UEVENT > + > +/* > + * SCSI sense key could be 0x00 - 0x08, 0x0a, 0x0b, 0x0d, 0x0e, so the > + * mask is 0x6dff. > + */ > +#define SCSI_SENSE_EVENT_FILTER_MASK 0x6dff Why omit 0x09? Its vendor specific, but that doesn't mean no-one ever will use it? Well, I don't know a real-life example, but anyway, doesn't hurt IMO. Also SPC-4 specifies 0x0F. > + > +static ssize_t > +sdev_show_sense_event_filter(struct device *dev, > + struct device_attribute *attr, > + char *buf) > +{ > + struct scsi_device *sdev; > + > + sdev = to_scsi_device(dev); > + return snprintf(buf, 20, "0x%04lx\n", > + (sdev->sense_event_filter & > + SCSI_SENSE_EVENT_FILTER_MASK)); > +} > + > +static ssize_t > +sdev_store_sense_event_filter(struct device *dev, > + struct device_attribute *attr, > + const char *buf, size_t count) > +{ > + struct scsi_device *sdev = to_scsi_device(dev); > + unsigned long filter; > + int i; > + > + if (buf[0] == '0' && buf[1] == 'x') { > + if (kstrtoul(buf + 2, 16, &filter)) > + return -EINVAL; > + } else > + if (kstrtoul(buf, 10, &filter)) > + return -EINVAL; Why the manual parsing? AFAIK kstrtoul can do that already, if you pass 0 as second argument. Beste Grüße / Best regards, - Benjamin Block > + > + /* > + * Accurate mask for all sense keys is 0x6dff. However, we allow > + * user to enable event for all sense keys by echoing 0xffff > + */ > + if ((filter & 0xffff) != filter) > + return -EINVAL; > + > + for (i = 0; i < 15; i++) > + if (filter & SCSI_SENSE_EVENT_FILTER_MASK & (1 << i)) > + set_bit(i, &sdev->sense_event_filter); > + else > + clear_bit(i, &sdev->sense_event_filter); > + return count; > +} > + > +static DEVICE_ATTR(sense_event_filter, 0644, > + sdev_show_sense_event_filter, > + sdev_store_sense_event_filter); > +#endif > + > static umode_t scsi_sdev_attr_is_visible(struct kobject *kobj, > struct attribute *attr, int i) > { > @@ -1142,6 +1199,9 @@ static struct attribute *scsi_sdev_attrs[] = { > &dev_attr_preferred_path.attr, > #endif > &dev_attr_queue_ramp_up_period.attr, > +#ifdef CONFIG_SCSI_SENSE_UEVENT > + &dev_attr_sense_event_filter.attr, > +#endif > REF_EVT(media_change), > REF_EVT(inquiry_change_reported), > REF_EVT(capacity_change_reported), > diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h > index 05641ae..d160bd7 100644 > --- a/include/scsi/scsi_device.h > +++ b/include/scsi/scsi_device.h > @@ -64,13 +64,23 @@ enum scsi_device_event { > SDEV_EVT_MODE_PARAMETER_CHANGE_REPORTED, /* 2A 01 UA reported */ > SDEV_EVT_LUN_CHANGE_REPORTED, /* 3F 0E UA reported */ > SDEV_EVT_ALUA_STATE_CHANGE_REPORTED, /* 2A 06 UA reported */ > + SDEV_EVT_SCSI_SENSE, > > SDEV_EVT_FIRST = SDEV_EVT_MEDIA_CHANGE, > - SDEV_EVT_LAST = SDEV_EVT_ALUA_STATE_CHANGE_REPORTED, > + SDEV_EVT_LAST = SDEV_EVT_SCSI_SENSE, > > SDEV_EVT_MAXBITS = SDEV_EVT_LAST + 1 > }; > > +#ifdef CONFIG_SCSI_SENSE_UEVENT > +/* data for for SDEV_EVT_SCSI_SENSE */ > +struct scsi_sense_uevent_data { > + sector_t lba; /* LBA from the scsi command */ > + int size; /* size of the request */ > + struct scsi_sense_hdr sshdr; > +}; > +#endif > + > struct scsi_event { > enum scsi_device_event evt_type; > struct list_head node; > @@ -78,6 +88,11 @@ struct scsi_event { > /* put union of data structures, for non-simple event types, > * here > */ > + union { > +#ifdef CONFIG_SCSI_SENSE_UEVENT > + struct scsi_sense_uevent_data sense_evt_data; > +#endif > + }; > }; > > struct scsi_device { > @@ -197,6 +212,15 @@ struct scsi_device { > atomic_t iodone_cnt; > atomic_t ioerr_cnt; > > +#ifdef CONFIG_SCSI_SENSE_UEVENT > + /* > + * filter of sense code uevent > + * setting bit X (0x00 - 0x0e) of sense_event_filter enables > + * uevent for sense key X > + */ > + unsigned long sense_event_filter; > +#endif > + > struct device sdev_gendev, > sdev_dev; > > -- > 2.9.3 > -- Linux on z Systems Development / IBM Systems & Technology Group IBM Deutschland Research & Development GmbH Vorsitz. AufsR.: Martina Koederitz / Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen / Registergericht: AmtsG Stuttgart, HRB 243294
> On May 16, 2017, at 10:00 AM, Benjamin Block <bblock@linux.vnet.ibm.com> wrote: > > Hello, > > On Wed, May 03, 2017 at 05:50:13PM -0700, Song Liu wrote: >> This patch adds capability for SCSI layer to generate uevent for SCSI >> sense code. The feature is gated by CONFIG_SCSI_SENSE_UEVENT. >> >> We can configure which sense keys generate uevent for each device >> through sysfs entry sense_event_filter. For example, the following >> enables uevent for MEDIUM_ERROR (0x03) and HARDWARE_ERROR (0x04) >> on scsi drive sdc: >> >> echo 0x000c > /sys/block/sdc/device/sense_event_filter >> > > I know its an RFC, but the user-interface definitely needs better > documentation. You also don't mention the 'wildcard' you document in the > code below. > > Would there be any public tooling for this, when it gets in? > > There is already some events we can generate.. for example when the > host-mapping changed, but I am not aware of any tooling that would ever > make use of them (although, this probably would even be vert useful). > And in the end that seems like dead-code for me. But maybe thats just me > not knowing the tooling. Hi Benjamin, Thanks for these feedbacks. I will incorporate them into next version. In term of tooling, we will open source useful tools we build to consume these events. >> >> Here is an example output captured by udevadm: >> >> KERNEL[1214.945358] change /devices/pci0000:00/XXXXXXX >> ACTION=change >> DEVPATH=/devices/pci0000:00/0000:00:01.0/0000:01:00.0/host6/XXXXXXX >> DEVTYPE=scsi_device >> DRIVER=sd >> LBA=0 >> MODALIAS=scsi:t-0x00 >> SDEV_UA=SCSI_SENSE >> SENSE_CODE=3/11/14 >> SEQNUM=4536 >> SIZE=4096 >> SUBSYSTEM=scsi >> >> Signed-off-by: Song Liu <songliubraving@fb.com> >> --- >> drivers/scsi/Kconfig | 14 +++++++++++ >> drivers/scsi/scsi_error.c | 26 ++++++++++++++++++++ >> drivers/scsi/scsi_lib.c | 27 +++++++++++++++++++-- >> drivers/scsi/scsi_sysfs.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++ >> include/scsi/scsi_device.h | 26 +++++++++++++++++++- >> 5 files changed, 150 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig >> index 3c52867..4f7f211 100644 >> --- a/drivers/scsi/Kconfig >> +++ b/drivers/scsi/Kconfig >> @@ -237,6 +237,20 @@ config SCSI_LOGGING >> there should be no noticeable performance impact as long as you have >> logging turned off. >> >> +config SCSI_SENSE_UEVENT >> + bool "SCSI sense code logging" >> + depends on SCSI >> + default n >> + ---help--- >> + This turns on uevent for SCSI sense code. >> + >> + You can configure which sense keys generate uevent for each device >> + through sysfs entry sense_event_filter. For example, the following >> + enables uevent for MEDIUM_ERROR (0x03) and HARDWARE_ERROR (0x04) >> + on scsi drive sdc: >> + >> + echo 0x000c > /sys/block/sdc/device/sense_event_filter >> + >> config SCSI_SCAN_ASYNC >> bool "Asynchronous SCSI scanning" >> depends on SCSI >> diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c >> index d70c67c..eda150e 100644 >> --- a/drivers/scsi/scsi_error.c >> +++ b/drivers/scsi/scsi_error.c >> @@ -426,6 +426,31 @@ static void scsi_report_sense(struct scsi_device *sdev, >> } >> } >> >> +/* >> + * generate uevent when receiving sense code from device >> + */ >> +static void scsi_send_sense_uevent(struct scsi_device *sdev, >> + struct scsi_cmnd *scmd, >> + struct scsi_sense_hdr *sshdr) >> +{ >> +#ifdef CONFIG_SCSI_SENSE_UEVENT >> + struct scsi_event *evt; >> + >> + if (!test_bit(sshdr->sense_key & 0xf, >> + &sdev->sense_event_filter)) >> + return; >> + evt = sdev_evt_alloc(SDEV_EVT_SCSI_SENSE, GFP_ATOMIC); >> + if (!evt) >> + return; >> + >> + evt->sense_evt_data.lba = scsi_get_lba(scmd); >> + evt->sense_evt_data.size = blk_rq_bytes(scmd->request); >> + memcpy(&evt->sense_evt_data.sshdr, sshdr, >> + sizeof(struct scsi_sense_hdr)); >> + sdev_evt_send(sdev, evt); >> +#endif >> +} >> + > > So when I turn this CONFIG_ off, do I get all kinds of warning about > unused variables? I don't get any warning at this time. I will keep testing w/ and w/o the config enabled. Thanks, Song
diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig index 3c52867..4f7f211 100644 --- a/drivers/scsi/Kconfig +++ b/drivers/scsi/Kconfig @@ -237,6 +237,20 @@ config SCSI_LOGGING there should be no noticeable performance impact as long as you have logging turned off. +config SCSI_SENSE_UEVENT + bool "SCSI sense code logging" + depends on SCSI + default n + ---help--- + This turns on uevent for SCSI sense code. + + You can configure which sense keys generate uevent for each device + through sysfs entry sense_event_filter. For example, the following + enables uevent for MEDIUM_ERROR (0x03) and HARDWARE_ERROR (0x04) + on scsi drive sdc: + + echo 0x000c > /sys/block/sdc/device/sense_event_filter + config SCSI_SCAN_ASYNC bool "Asynchronous SCSI scanning" depends on SCSI diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c index d70c67c..eda150e 100644 --- a/drivers/scsi/scsi_error.c +++ b/drivers/scsi/scsi_error.c @@ -426,6 +426,31 @@ static void scsi_report_sense(struct scsi_device *sdev, } } +/* + * generate uevent when receiving sense code from device + */ +static void scsi_send_sense_uevent(struct scsi_device *sdev, + struct scsi_cmnd *scmd, + struct scsi_sense_hdr *sshdr) +{ +#ifdef CONFIG_SCSI_SENSE_UEVENT + struct scsi_event *evt; + + if (!test_bit(sshdr->sense_key & 0xf, + &sdev->sense_event_filter)) + return; + evt = sdev_evt_alloc(SDEV_EVT_SCSI_SENSE, GFP_ATOMIC); + if (!evt) + return; + + evt->sense_evt_data.lba = scsi_get_lba(scmd); + evt->sense_evt_data.size = blk_rq_bytes(scmd->request); + memcpy(&evt->sense_evt_data.sshdr, sshdr, + sizeof(struct scsi_sense_hdr)); + sdev_evt_send(sdev, evt); +#endif +} + /** * scsi_check_sense - Examine scsi cmd sense * @scmd: Cmd to have sense checked. @@ -446,6 +471,7 @@ int scsi_check_sense(struct scsi_cmnd *scmd) return FAILED; /* no valid sense data */ scsi_report_sense(sdev, &sshdr); + scsi_send_sense_uevent(sdev, scmd, &sshdr); if (scsi_sense_is_deferred(&sshdr)) return NEEDS_RETRY; diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c index 95f963b..1095f27 100644 --- a/drivers/scsi/scsi_lib.c +++ b/drivers/scsi/scsi_lib.c @@ -2656,8 +2656,9 @@ EXPORT_SYMBOL(scsi_device_set_state); */ static void scsi_evt_emit(struct scsi_device *sdev, struct scsi_event *evt) { - int idx = 0; - char *envp[3]; + int idx = 0, i; + char *envp[5]; /* SDEV_EVT_SCSI_SENSE needs most entries (4) */ + int free_envp = -1; switch (evt->evt_type) { case SDEV_EVT_MEDIA_CHANGE: @@ -2682,6 +2683,23 @@ static void scsi_evt_emit(struct scsi_device *sdev, struct scsi_event *evt) case SDEV_EVT_ALUA_STATE_CHANGE_REPORTED: envp[idx++] = "SDEV_UA=ASYMMETRIC_ACCESS_STATE_CHANGED"; break; +#ifdef CONFIG_SCSI_SENSE_UEVENT + case SDEV_EVT_SCSI_SENSE: + envp[idx++] = "SDEV_UA=SCSI_SENSE"; + for (i = idx; i < idx + 3; ++i) { + envp[i] = kzalloc(32, GFP_ATOMIC); + if (!envp[i]) + break; + free_envp = i; + } + snprintf(envp[idx++], 32, "LBA=%lu", evt->sense_evt_data.lba); + snprintf(envp[idx++], 32, "SIZE=%d", evt->sense_evt_data.size); + snprintf(envp[idx++], 32, "SENSE_CODE=%1x/%02x/%02x", + evt->sense_evt_data.sshdr.sense_key, + evt->sense_evt_data.sshdr.asc, + evt->sense_evt_data.sshdr.ascq); + break; +#endif default: /* do nothing */ break; @@ -2690,6 +2708,10 @@ static void scsi_evt_emit(struct scsi_device *sdev, struct scsi_event *evt) envp[idx++] = NULL; kobject_uevent_env(&sdev->sdev_gendev.kobj, KOBJ_CHANGE, envp); + + /* no need to free envp[0], so start with i = 1 */ + for (i = 1 ; i < free_envp; ++i) + kfree(envp[i]); } /** @@ -2786,6 +2808,7 @@ struct scsi_event *sdev_evt_alloc(enum scsi_device_event evt_type, case SDEV_EVT_MODE_PARAMETER_CHANGE_REPORTED: case SDEV_EVT_LUN_CHANGE_REPORTED: case SDEV_EVT_ALUA_STATE_CHANGE_REPORTED: + case SDEV_EVT_SCSI_SENSE: default: /* do nothing */ break; diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c index 82dfe07..cfc7380 100644 --- a/drivers/scsi/scsi_sysfs.c +++ b/drivers/scsi/scsi_sysfs.c @@ -1072,6 +1072,63 @@ static DEVICE_ATTR(queue_ramp_up_period, S_IRUGO | S_IWUSR, sdev_show_queue_ramp_up_period, sdev_store_queue_ramp_up_period); +#ifdef CONFIG_SCSI_SENSE_UEVENT + +/* + * SCSI sense key could be 0x00 - 0x08, 0x0a, 0x0b, 0x0d, 0x0e, so the + * mask is 0x6dff. + */ +#define SCSI_SENSE_EVENT_FILTER_MASK 0x6dff + +static ssize_t +sdev_show_sense_event_filter(struct device *dev, + struct device_attribute *attr, + char *buf) +{ + struct scsi_device *sdev; + + sdev = to_scsi_device(dev); + return snprintf(buf, 20, "0x%04lx\n", + (sdev->sense_event_filter & + SCSI_SENSE_EVENT_FILTER_MASK)); +} + +static ssize_t +sdev_store_sense_event_filter(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t count) +{ + struct scsi_device *sdev = to_scsi_device(dev); + unsigned long filter; + int i; + + if (buf[0] == '0' && buf[1] == 'x') { + if (kstrtoul(buf + 2, 16, &filter)) + return -EINVAL; + } else + if (kstrtoul(buf, 10, &filter)) + return -EINVAL; + + /* + * Accurate mask for all sense keys is 0x6dff. However, we allow + * user to enable event for all sense keys by echoing 0xffff + */ + if ((filter & 0xffff) != filter) + return -EINVAL; + + for (i = 0; i < 15; i++) + if (filter & SCSI_SENSE_EVENT_FILTER_MASK & (1 << i)) + set_bit(i, &sdev->sense_event_filter); + else + clear_bit(i, &sdev->sense_event_filter); + return count; +} + +static DEVICE_ATTR(sense_event_filter, 0644, + sdev_show_sense_event_filter, + sdev_store_sense_event_filter); +#endif + static umode_t scsi_sdev_attr_is_visible(struct kobject *kobj, struct attribute *attr, int i) { @@ -1142,6 +1199,9 @@ static struct attribute *scsi_sdev_attrs[] = { &dev_attr_preferred_path.attr, #endif &dev_attr_queue_ramp_up_period.attr, +#ifdef CONFIG_SCSI_SENSE_UEVENT + &dev_attr_sense_event_filter.attr, +#endif REF_EVT(media_change), REF_EVT(inquiry_change_reported), REF_EVT(capacity_change_reported), diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h index 05641ae..d160bd7 100644 --- a/include/scsi/scsi_device.h +++ b/include/scsi/scsi_device.h @@ -64,13 +64,23 @@ enum scsi_device_event { SDEV_EVT_MODE_PARAMETER_CHANGE_REPORTED, /* 2A 01 UA reported */ SDEV_EVT_LUN_CHANGE_REPORTED, /* 3F 0E UA reported */ SDEV_EVT_ALUA_STATE_CHANGE_REPORTED, /* 2A 06 UA reported */ + SDEV_EVT_SCSI_SENSE, SDEV_EVT_FIRST = SDEV_EVT_MEDIA_CHANGE, - SDEV_EVT_LAST = SDEV_EVT_ALUA_STATE_CHANGE_REPORTED, + SDEV_EVT_LAST = SDEV_EVT_SCSI_SENSE, SDEV_EVT_MAXBITS = SDEV_EVT_LAST + 1 }; +#ifdef CONFIG_SCSI_SENSE_UEVENT +/* data for for SDEV_EVT_SCSI_SENSE */ +struct scsi_sense_uevent_data { + sector_t lba; /* LBA from the scsi command */ + int size; /* size of the request */ + struct scsi_sense_hdr sshdr; +}; +#endif + struct scsi_event { enum scsi_device_event evt_type; struct list_head node; @@ -78,6 +88,11 @@ struct scsi_event { /* put union of data structures, for non-simple event types, * here */ + union { +#ifdef CONFIG_SCSI_SENSE_UEVENT + struct scsi_sense_uevent_data sense_evt_data; +#endif + }; }; struct scsi_device { @@ -197,6 +212,15 @@ struct scsi_device { atomic_t iodone_cnt; atomic_t ioerr_cnt; +#ifdef CONFIG_SCSI_SENSE_UEVENT + /* + * filter of sense code uevent + * setting bit X (0x00 - 0x0e) of sense_event_filter enables + * uevent for sense key X + */ + unsigned long sense_event_filter; +#endif + struct device sdev_gendev, sdev_dev;
This patch adds capability for SCSI layer to generate uevent for SCSI sense code. The feature is gated by CONFIG_SCSI_SENSE_UEVENT. We can configure which sense keys generate uevent for each device through sysfs entry sense_event_filter. For example, the following enables uevent for MEDIUM_ERROR (0x03) and HARDWARE_ERROR (0x04) on scsi drive sdc: echo 0x000c > /sys/block/sdc/device/sense_event_filter Here is an example output captured by udevadm: KERNEL[1214.945358] change /devices/pci0000:00/XXXXXXX ACTION=change DEVPATH=/devices/pci0000:00/0000:00:01.0/0000:01:00.0/host6/XXXXXXX DEVTYPE=scsi_device DRIVER=sd LBA=0 MODALIAS=scsi:t-0x00 SDEV_UA=SCSI_SENSE SENSE_CODE=3/11/14 SEQNUM=4536 SIZE=4096 SUBSYSTEM=scsi Signed-off-by: Song Liu <songliubraving@fb.com> --- drivers/scsi/Kconfig | 14 +++++++++++ drivers/scsi/scsi_error.c | 26 ++++++++++++++++++++ drivers/scsi/scsi_lib.c | 27 +++++++++++++++++++-- drivers/scsi/scsi_sysfs.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++ include/scsi/scsi_device.h | 26 +++++++++++++++++++- 5 files changed, 150 insertions(+), 3 deletions(-)