| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <460B0539.9040103@pobox.com>
Date: Wed, 28 Mar 2007 20:15:53 -0400
From: Jeff Garzik <jgarzik@...ox.com>
To: Kristen Carlson Accardi <kristen.c.accardi@...el.com>
CC: linux-kernel@...r.kernel.org, akpm@...ux-foundation.org,
linux-ide@...r.kernel.org, linux-scsi <linux-scsi@...r.kernel.org>
Subject: Re: [patch 2/3] libata: expose AN support to user space via sysfs
Kristen Carlson Accardi wrote:
> Allow user space to determine if an ATAPI device supports
> async notification (AN) of media changes. This is done by
> adding a new sysfs file "async_notification" to genhd.
> If the file reads 1, then the device supports async
> notification. If the file reads 0, it does not.
>
> A flag is set in the generic disk to indicate whether
> or not AN is supported. This flag is set by the SCSI
> subsystem when it registers with add_disk. The SCSI
> system gets information from libata on whether the
> device supports AN during dev_configure.
>
> Signed-off-by: Kristen Carlson Accardi <kristen.c.accardi@...el.com>
>
> Index: 2.6-mm/block/genhd.c
> ===================================================================
> --- 2.6-mm.orig/block/genhd.c
> +++ 2.6-mm/block/genhd.c
> @@ -372,6 +372,11 @@ static ssize_t disk_size_read(struct gen
> {
> return sprintf(page, "%llu\n", (unsigned long long)get_capacity(disk));
> }
> +static ssize_t disk_AN_read(struct gendisk * disk, char *page)
> +{
> + return sprintf(page, "%d\n",
> + (disk->flags & GENHD_FL_ASYNC_NOTIFICATION ? 1 : 0));
> +}
>
> static ssize_t disk_stats_read(struct gendisk * disk, char *page)
> {
> @@ -419,6 +424,10 @@ static struct disk_attribute disk_attr_s
> .attr = {.name = "stat", .mode = S_IRUGO },
> .show = disk_stats_read
> };
> +static struct disk_attribute disk_attr_AN = {
> + .attr = {.name = "media_change_events", .mode = S_IRUGO },
> + .show = disk_AN_read
> +};
>
> #ifdef CONFIG_FAIL_MAKE_REQUEST
>
> @@ -455,6 +464,7 @@ static struct attribute * default_attrs[
> &disk_attr_removable.attr,
> &disk_attr_size.attr,
> &disk_attr_stat.attr,
> + &disk_attr_AN.attr,
> #ifdef CONFIG_FAIL_MAKE_REQUEST
> &disk_attr_fail.attr,
> #endif
> Index: 2.6-mm/include/linux/genhd.h
> ===================================================================
> --- 2.6-mm.orig/include/linux/genhd.h
> +++ 2.6-mm/include/linux/genhd.h
> @@ -94,6 +94,7 @@ struct hd_struct {
>
> #define GENHD_FL_REMOVABLE 1
> #define GENHD_FL_DRIVERFS 2
> +#define GENHD_FL_ASYNC_NOTIFICATION 4
> #define GENHD_FL_CD 8
> #define GENHD_FL_UP 16
> #define GENHD_FL_SUPPRESS_PARTITION_INFO 32
> Index: 2.6-mm/include/scsi/scsi_device.h
> ===================================================================
> --- 2.6-mm.orig/include/scsi/scsi_device.h
> +++ 2.6-mm/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 async_notification:1; /* device supports async notification */
> unsigned int device_blocked; /* Device returned QUEUE_FULL. */
>
> unsigned int max_device_blocked; /* what device_blocked counts down from */
> Index: 2.6-mm/drivers/ata/libata-scsi.c
> ===================================================================
> --- 2.6-mm.orig/drivers/ata/libata-scsi.c
> +++ 2.6-mm/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->async_notification = 1;
> +
> if (dev->flags & ATA_DFLAG_NCQ) {
> int depth;
>
> Index: 2.6-mm/drivers/scsi/sr.c
> ===================================================================
> --- 2.6-mm.orig/drivers/scsi/sr.c
> +++ 2.6-mm/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->async_notification)
> + disk->flags |= GENHD_FL_ASYNC_NOTIFICATION;
> add_disk(disk);
(added linux-scsi to CC)
Comments:
1) From a procedural standpoint, you'll want to separate this patch into
three patches: generic block layer stuff, SCSI stuff, and libata stuff.
2) I don't claim to be a sysfs expert, but this seems like a reasonable
approach for reporting async-notification capabilities
3) I would make the contents of 'media_change_events' be a list of
flags, rather than a boolean. Thus, when AN is present,
media_change_events would return "AN\n". It would return "\n" (no
flags) when AN is absent. This permits future expansion of this
capabilities reporting variable.
4) Figure out some place to document 'media_change_events', in
Documentation/*
5) I think the method of delivery probably needs discussing, and some
work. Presumably the normal hotplug paths should be traversed for this
sort of thing.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists