Re: virtio(-scsi) vs. chained sg_lists (was Re: [PATCH] scsi: virtio-scsi: Fix address translation failure of HighMem pages used by sg list)
On Thu, 26 Jul 2012 15:05:39 +0200, Paolo Bonzini wrote: > Il 26/07/2012 09:58, Paolo Bonzini ha scritto: > > > >> > Please CC me on the "convert to sg copy-less" patches, It looks > >> > interesting > > Sure. > > Well, here is the gist of it (note it won't apply on any public tree, > hence no SoB yet). It should be split in multiple changesets and you > can make more simplifications on top of it, because > virtio_scsi_target_state is not anymore variable-sized, but that's > secondary. ISTR starting on such a patch years ago, but the primitives to manipulate a chained sg_list were nonexistent, so I dropped it, waiting for it to be fully-baked or replaced. That hasn't happened: > + /* Remove scatterlist terminator, we will tack more items soon. */ > + vblk->sg[num + out - 1].page_link &= ~0x2; I hate this interface: > +int virtqueue_add_buf_sg(struct virtqueue *_vq, > + struct scatterlist *sg_out, > + unsigned int out, > + struct scatterlist *sg_in, > + unsigned int in, > + void *data, > + gfp_t gfp) The point of chained scatterlists is they're self-terminated, so the in & out counts should be calculated. Counting them is not *that* bad, since we're about to read them all anyway. (Yes, the chained scatterlist stuff is complete crack, but I lost that debate years ago.) Here's my variant. Networking, console and block seem OK, at least (ie. it booted!). From: Rusty Russell Subject: virtio: use chained scatterlists. Rather than handing a scatterlist[] and out and in numbers to virtqueue_add_buf(), hand two separate ones which can be chained. I shall refrain from ranting about what a disgusting hack chained scatterlists are. I'll just note that this doesn't make things simpler (see diff). The scatterlists we use can be too large for the stack, so we put them in our device struct and reuse them. But in many cases we don't want to pay the cost of sg_init_table() as we don't know how many elements we'll have and we'd have to initialize the entire table. This means we have two choices: carefully reset the end markers after we call virtqueue_add_buf(), which we do in virtio_net for the xmit path where it's easy and we want to be optimal. Elsewhere we implement a helper to unset the end markers after we've filled the array. Signed-off-by: Rusty Russell --- drivers/block/virtio_blk.c | 37 +- drivers/char/hw_random/virtio-rng.c |2 - drivers/char/virtio_console.c |6 +-- drivers/net/virtio_net.c| 67 ++--- drivers/virtio/virtio_balloon.c |6 +-- drivers/virtio/virtio_ring.c| 71 ++-- include/linux/virtio.h |5 +- net/9p/trans_virtio.c | 46 +-- 8 files changed, 159 insertions(+), 81 deletions(-) diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c --- a/drivers/block/virtio_blk.c +++ b/drivers/block/virtio_blk.c @@ -100,6 +100,14 @@ static void blk_done(struct virtqueue *v spin_unlock_irqrestore(vblk->disk->queue->queue_lock, flags); } +static void sg_unset_end_markers(struct scatterlist *sg, unsigned int num) +{ + unsigned int i; + + for (i = 0; i < num; i++) + sg[i].page_link &= ~0x02; +} + static bool do_req(struct request_queue *q, struct virtio_blk *vblk, struct request *req) { @@ -140,6 +148,7 @@ static bool do_req(struct request_queue } } + /* We layout out scatterlist in a single array, out then in. */ sg_set_buf(&vblk->sg[out++], &vbr->out_hdr, sizeof(vbr->out_hdr)); /* @@ -151,17 +160,8 @@ static bool do_req(struct request_queue if (vbr->req->cmd_type == REQ_TYPE_BLOCK_PC) sg_set_buf(&vblk->sg[out++], vbr->req->cmd, vbr->req->cmd_len); + /* This marks the end of the sg list at vblk->sg[out]. */ num = blk_rq_map_sg(q, vbr->req, vblk->sg + out); - - if (vbr->req->cmd_type == REQ_TYPE_BLOCK_PC) { - sg_set_buf(&vblk->sg[num + out + in++], vbr->req->sense, SCSI_SENSE_BUFFERSIZE); - sg_set_buf(&vblk->sg[num + out + in++], &vbr->in_hdr, - sizeof(vbr->in_hdr)); - } - - sg_set_buf(&vblk->sg[num + out + in++], &vbr->status, - sizeof(vbr->status)); - if (num) { if (rq_data_dir(vbr->req) == WRITE) { vbr->out_hdr.type |= VIRTIO_BLK_T_OUT; @@ -172,7 +172,22 @@ static bool do_req(struct request_queue } } - if (virtqueue_add_buf(vblk->vq, vblk->sg, out, in, vbr, GFP_ATOMIC)<0) { + if (vbr->req->cmd_type == REQ_TYPE_BLOCK_PC) { + sg_set_buf(&vblk->sg[out + in++], vbr->req->sense, + SCSI_SENSE_BUFFERSIZE); +
Re: virtio(-scsi) vs. chained sg_lists (was Re: [PATCH] scsi: virtio-scsi: Fix address translation failure of HighMem pages used by sg list)
Il 27/07/2012 08:27, Rusty Russell ha scritto: >> > +int virtqueue_add_buf_sg(struct virtqueue *_vq, >> > + struct scatterlist *sg_out, >> > + unsigned int out, >> > + struct scatterlist *sg_in, >> > + unsigned int in, >> > + void *data, >> > + gfp_t gfp) > The point of chained scatterlists is they're self-terminated, so the > in & out counts should be calculated. > > Counting them is not *that* bad, since we're about to read them all > anyway. > > (Yes, the chained scatterlist stuff is complete crack, but I lost that > debate years ago.) > > Here's my variant. Networking, console and block seem OK, at least > (ie. it booted!). I hate the for loops, even though we're about indeed to read all the scatterlists anyway... all they do is lengthen critical sections. Also, being the first user of chained scatterlist doesn't exactly give me warm fuzzies. I think it's simpler if we provide an API to add individual buffers to the virtqueue, so that you can do multiple virtqueue_add_buf_more (whatever) before kicking the virtqueue. The idea is that I can still use indirect buffers for the scatterlists that come from the block layer or from an skb, but I will use direct buffers for the request/response descriptors. The direct buffers are always a small number (usually 2), so you can balance the effect by making the virtqueue bigger. And for small reads and writes, you save a kmalloc on a very hot path. (BTW, scatterlists will have separate entries for each page; we do not need this in virtio buffers. Collapsing physically-adjacent entries will speed up QEMU and will also help avoiding indirect buffers). Paolo -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 4/7] scsi: sr: block events when runtime suspended
When the ODD is runtime suspended, there is no need to poll it for events, so block events poll for it and unblock when resumed. Signed-off-by: Aaron Lu --- block/genhd.c | 2 ++ drivers/scsi/sr.c | 7 --- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/block/genhd.c b/block/genhd.c index 9cf5583..bdb3682 100644 --- a/block/genhd.c +++ b/block/genhd.c @@ -1458,6 +1458,7 @@ void disk_block_events(struct gendisk *disk) mutex_unlock(&ev->block_mutex); } +EXPORT_SYMBOL(disk_block_events); static void __disk_unblock_events(struct gendisk *disk, bool check_now) { @@ -1502,6 +1503,7 @@ void disk_unblock_events(struct gendisk *disk) if (disk->ev) __disk_unblock_events(disk, false); } +EXPORT_SYMBOL(disk_unblock_events); /** * disk_flush_events - schedule immediate event checking and flushing diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c index acfd10a..cbc14ea 100644 --- a/drivers/scsi/sr.c +++ b/drivers/scsi/sr.c @@ -205,6 +205,8 @@ static int sr_suspend(struct device *dev, pm_message_t msg) return -EBUSY; } + disk_block_events(cd->disk); + return 0; } @@ -226,6 +228,8 @@ static int sr_resume(struct device *dev) atomic_set(&cd->suspend_count, 1); } + disk_unblock_events(cd->disk); + return 0; } @@ -314,9 +318,6 @@ static unsigned int sr_check_events(struct cdrom_device_info *cdi, if (CDSL_CURRENT != slot) return 0; - if (pm_runtime_suspended(&cd->device->sdev_gendev)) - return 0; - /* if the logical unit just finished loading/unloading, do a TUR */ if (cd->device->can_power_off && cd->dbml && sr_unit_load_done(cd)) { events = 0; -- 1.7.11.3 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 6/7] scsi: sr: balance sr disk events block depth
When the ODD is resumed, disk_unblock_events should be called when: 1 The ODD is runtime resumed; 2 System is resuming from S3 and the ODD is runtime suspended before S3; But not when the system is resuming from S3 and the ODD is runtime active before S3. So seperate the resume calls, one for system resume and one for runtime resume to do different things accordingly. Signed-off-by: Aaron Lu --- drivers/scsi/sr.c | 23 +++ 1 file changed, 23 insertions(+) diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c index cbc14ea..f0c4aa2 100644 --- a/drivers/scsi/sr.c +++ b/drivers/scsi/sr.c @@ -82,6 +82,11 @@ static int sr_remove(struct device *); static int sr_done(struct scsi_cmnd *); static int sr_suspend(struct device *, pm_message_t msg); static int sr_resume(struct device *); +static int sr_runtime_resume(struct device *); + +static struct dev_pm_ops sr_pm_ops = { + .runtime_resume = sr_runtime_resume, +}; static struct scsi_driver sr_template = { .owner = THIS_MODULE, @@ -91,6 +96,7 @@ static struct scsi_driver sr_template = { .remove = sr_remove, .suspend= sr_suspend, .resume = sr_resume, + .pm = &sr_pm_ops, }, .done = sr_done, }; @@ -213,6 +219,23 @@ static int sr_suspend(struct device *dev, pm_message_t msg) static int sr_resume(struct device *dev) { struct scsi_cd *cd; + + /* +* If ODD is runtime suspended before system pm, unblock disk +* events now since on system resume we will fully resume it +* and set its runtime status to active. +*/ + if (pm_runtime_suspended(dev)) { + cd = dev_get_drvdata(dev); + disk_unblock_events(cd->disk); + } + + return 0; +} + +static int sr_runtime_resume(struct device *dev) +{ + struct scsi_cd *cd; struct scsi_sense_hdr sshdr; cd = dev_get_drvdata(dev); -- 1.7.11.3 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 5/7] scsi: pm: use runtime resume callback if available
When runtime resume a scsi device, if the device's driver has implemented runtime resume callback, use that. sr driver needs this to do different things for system resume and runtime resume. Signed-off-by: Aaron Lu --- drivers/scsi/scsi_pm.c | 15 ++- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/drivers/scsi/scsi_pm.c b/drivers/scsi/scsi_pm.c index 83edb93..690136c 100644 --- a/drivers/scsi/scsi_pm.c +++ b/drivers/scsi/scsi_pm.c @@ -34,14 +34,19 @@ static int scsi_dev_type_suspend(struct device *dev, pm_message_t msg) return err; } -static int scsi_dev_type_resume(struct device *dev) +static int scsi_dev_type_resume(struct device *dev, bool runtime) { struct device_driver *drv; int err = 0; + int (*resume)(struct device *); drv = dev->driver; - if (drv && drv->resume) - err = drv->resume(dev); + if (runtime && drv && drv->pm && drv->pm->runtime_resume) + resume = drv->pm->runtime_resume; + else + resume = drv ? drv->resume : NULL; + if (resume) + err = resume(dev); scsi_device_resume(to_scsi_device(dev)); dev_dbg(dev, "scsi resume: %d\n", err); return err; @@ -85,7 +90,7 @@ static int scsi_bus_resume_common(struct device *dev) pm_runtime_get_sync(dev->parent); if (scsi_is_sdev_device(dev)) - err = scsi_dev_type_resume(dev); + err = scsi_dev_type_resume(dev, false); if (err == 0) { pm_runtime_disable(dev); pm_runtime_set_active(dev); @@ -160,7 +165,7 @@ static int scsi_runtime_resume(struct device *dev) dev_dbg(dev, "scsi_runtime_resume\n"); if (scsi_is_sdev_device(dev)) - err = scsi_dev_type_resume(dev); + err = scsi_dev_type_resume(dev, true); /* Insert hooks here for targets, hosts, and transport classes */ -- 1.7.11.3 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 1/7] scsi: sr: check support for device busy class events
Signed-off-by: Aaron Lu --- drivers/scsi/sr.c | 23 +++ drivers/scsi/sr.h | 1 + include/linux/cdrom.h | 43 +++ 3 files changed, 67 insertions(+) diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c index 5fc97d2..abfefab 100644 --- a/drivers/scsi/sr.c +++ b/drivers/scsi/sr.c @@ -101,6 +101,7 @@ static DEFINE_MUTEX(sr_ref_mutex); static int sr_open(struct cdrom_device_info *, int); static void sr_release(struct cdrom_device_info *); +static void check_dbml(struct scsi_cd *); static void get_sectorsize(struct scsi_cd *); static void get_capabilities(struct scsi_cd *); @@ -728,6 +729,28 @@ fail: return error; } +static void check_dbml(struct scsi_cd *cd) +{ + struct packet_command cgc; + unsigned char buffer[16]; + struct rm_feature_desc *rfd; + + init_cdrom_command(&cgc, buffer, sizeof(buffer), CGC_DATA_READ); + cgc.cmd[0] = GPCMD_GET_CONFIGURATION; + cgc.cmd[3] = CDF_RM; + cgc.cmd[8] = sizeof(buffer); + cgc.quiet = 1; + + if (cd->cdi.ops->generic_packet(&cd->cdi, &cgc)) + return; + + rfd = (struct rm_feature_desc *)&buffer[sizeof(struct feature_header)]; + if (be16_to_cpu(rfd->feature_code) != CDF_RM) + return; + + if (rfd->dbml) + cd->dbml = 1; +} static void get_sectorsize(struct scsi_cd *cd) { diff --git a/drivers/scsi/sr.h b/drivers/scsi/sr.h index 37c8f6b..7cc40ad 100644 --- a/drivers/scsi/sr.h +++ b/drivers/scsi/sr.h @@ -41,6 +41,7 @@ typedef struct scsi_cd { unsigned readcd_known:1;/* drive supports READ_CD (0xbe) */ unsigned readcd_cdda:1; /* reading audio data using READ_CD */ unsigned media_present:1; /* media is present */ + unsigned dbml:1;/* generates device busy class events */ /* GET_EVENT spurious event handling, blk layer guarantees exclusion */ int tur_mismatch; /* nr of get_event TUR mismatches */ diff --git a/include/linux/cdrom.h b/include/linux/cdrom.h index dfd7f18..962be39 100644 --- a/include/linux/cdrom.h +++ b/include/linux/cdrom.h @@ -727,6 +727,7 @@ struct request_sense { /* * feature profile */ +#define CDF_RM 0x0003 /* "Removable Medium" */ #define CDF_RWRT 0x0020 /* "Random Writable" */ #define CDF_HWDM 0x0024 /* "Hardware Defect Management" */ #define CDF_MRW0x0028 @@ -739,6 +740,48 @@ struct request_sense { #define CDM_MRW_BGFORMAT_ACTIVE2 #define CDM_MRW_BGFORMAT_COMPLETE 3 +/* Removable medium feature descriptor */ +struct rm_feature_desc { + __be16 feature_code; +#if defined(__BIG_ENDIAN_BITFIELD) + __u8 reserved1 : 2; + __u8 feature_version: 4; + __u8 persistent : 1; + __u8 curr : 1; +#elif defined(__LITTLE_ENDIAN_BITFIELD) + __u8 curr : 1; + __u8 persistent : 1; + __u8 feature_version: 4; + __u8 reserved1 : 2; +#endif + __u8 add_len; +#if defined(__BIG_ENDIAN_BITFIELD) + __u8 mech_type : 3; + __u8 load : 1; + __u8 eject : 1; + __u8 pvnt_jmpr : 1; + __u8 dbml : 1; + __u8 lock : 1; +#elif defined(__LITTLE_ENDIAN_BITFIELD) + __u8 lock : 1; + __u8 dbml : 1; + __u8 pvnt_jmpr : 1; + __u8 eject : 1; + __u8 load : 1; + __u8 mech_type : 3; +#endif + __u8 reserved2; + __u8 reserved3; + __u8 reserved4; +}; + +struct device_busy_event_desc { + __u8 device_busy_event : 4; + __u8 reserved1 : 4; + __u8 device_busy_status; + __u8 time; +}; + /* * mrw address spaces */ -- 1.7.11.3 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 7/7] block: genhd: add an interface to set disk's poll interval
Set the ODD's in kernel poll interval to 2s for the user in case the user is using an old distro on which udev will not set the system wide block parameter events_dfl_poll_msecs. Signed-off-by: Aaron Lu --- block/genhd.c | 23 +-- drivers/scsi/sr.c | 1 + include/linux/genhd.h | 1 + 3 files changed, 19 insertions(+), 6 deletions(-) diff --git a/block/genhd.c b/block/genhd.c index bdb3682..de9b9d9 100644 --- a/block/genhd.c +++ b/block/genhd.c @@ -1619,6 +1619,19 @@ static void disk_events_workfn(struct work_struct *work) kobject_uevent_env(&disk_to_dev(disk)->kobj, KOBJ_CHANGE, envp); } +int disk_events_set_poll_msecs(struct gendisk *disk, long intv) +{ + if (intv < 0 && intv != -1) + return -EINVAL; + + disk_block_events(disk); + disk->ev->poll_msecs = intv; + __disk_unblock_events(disk, true); + + return 0; +} +EXPORT_SYMBOL(disk_events_set_poll_msecs); + /* * A disk events enabled device has the following sysfs nodes under * its /sys/block/X/ directory. @@ -1675,16 +1688,14 @@ static ssize_t disk_events_poll_msecs_store(struct device *dev, { struct gendisk *disk = dev_to_disk(dev); long intv; + int ret; if (!count || !sscanf(buf, "%ld", &intv)) return -EINVAL; - if (intv < 0 && intv != -1) - return -EINVAL; - - disk_block_events(disk); - disk->ev->poll_msecs = intv; - __disk_unblock_events(disk, true); + ret = disk_events_set_poll_msecs(disk, intv); + if (ret) + return ret; return count; } diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c index f0c4aa2..e6e5549 100644 --- a/drivers/scsi/sr.c +++ b/drivers/scsi/sr.c @@ -864,6 +864,7 @@ static int sr_probe(struct device *dev) dev_set_drvdata(dev, cd); disk->flags |= GENHD_FL_REMOVABLE; add_disk(disk); + disk_events_set_poll_msecs(disk, 2000); sdev_printk(KERN_DEBUG, sdev, "Attached scsi CD-ROM %s\n", cd->cdi.name); diff --git a/include/linux/genhd.h b/include/linux/genhd.h index ae0aaa9..308d47e 100644 --- a/include/linux/genhd.h +++ b/include/linux/genhd.h @@ -417,6 +417,7 @@ extern void disk_block_events(struct gendisk *disk); extern void disk_unblock_events(struct gendisk *disk); extern void disk_flush_events(struct gendisk *disk, unsigned int mask); extern unsigned int disk_clear_events(struct gendisk *disk, unsigned int mask); +extern int disk_events_set_poll_msecs(struct gendisk *disk, long intv); /* drivers/char/random.c */ extern void add_disk_randomness(struct gendisk *disk); -- 1.7.11.3 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 0/7] ZPODD patches
v4: Rebase on top of Linus' tree, due to this, the problem of a missing flag in v3 is gone; Add a new function scsi_autopm_put_device_autosuspend to first mark last busy for the device and then put autosuspend it as suggested by Oliver Neukum. Typo fix as pointed by Sergei Shtylyov. Check can_power_off flag before any runtime pm operations in sr. v3: Rebase on top of scsi-misc tree; Add the sr related patches previously in Jeff's libata tree; Re-organize the sr patches. A problem for now: for patch scsi: sr: support zero power ODD(ZPODD) I can't set a flag in libata-acpi.c since a related function is missing in scsi-misc tree. Will fix this when 3.6-rc1 released. v2: Bug fix for v1; Use scsi_autopm_* in sr driver instead of pm_runtime_*; v1: Here are some patches to make ZPODD easier to use for end users and a fix for using ZPODD with system suspend. Aaron Lu (7): scsi: sr: check support for device busy class events scsi: pm: add interface to autosuspend scsi device scsi: sr: support zero power ODD(ZPODD) scsi: sr: block events when runtime suspended scsi: pm: use runtime resume callback if available scsi: sr: balance sr disk events block depth block: genhd: add an interface to set disk's poll interval block/genhd.c | 25 +-- drivers/ata/libata-acpi.c | 4 +- drivers/scsi/scsi_pm.c | 22 -- drivers/scsi/sr.c | 179 - drivers/scsi/sr.h | 3 + include/linux/cdrom.h | 43 +++ include/linux/genhd.h | 1 + include/scsi/scsi_device.h | 3 + 8 files changed, 267 insertions(+), 13 deletions(-) -- 1.7.11.3 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 2/7] scsi: pm: add interface to autosuspend scsi device
Add a new interface scsi_autopm_put_device_autosuspend to mark last busy for the device and then put autosuspend the device. Signed-off-by: Aaron Lu --- drivers/scsi/scsi_pm.c | 7 +++ include/scsi/scsi_device.h | 2 ++ 2 files changed, 9 insertions(+) diff --git a/drivers/scsi/scsi_pm.c b/drivers/scsi/scsi_pm.c index dc0ad85..83edb93 100644 --- a/drivers/scsi/scsi_pm.c +++ b/drivers/scsi/scsi_pm.c @@ -201,6 +201,13 @@ void scsi_autopm_put_device(struct scsi_device *sdev) } EXPORT_SYMBOL_GPL(scsi_autopm_put_device); +void scsi_autopm_put_device_autosuspend(struct scsi_device *sdev) +{ + pm_runtime_mark_last_busy(&sdev->sdev_gendev); + pm_runtime_put_autosuspend(&sdev->sdev_gendev); +} +EXPORT_SYMBOL_GPL(scsi_autopm_put_device_autosuspend); + void scsi_autopm_get_target(struct scsi_target *starget) { pm_runtime_get_sync(&starget->dev); diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h index 9895f69..3636146 100644 --- a/include/scsi/scsi_device.h +++ b/include/scsi/scsi_device.h @@ -395,9 +395,11 @@ extern int scsi_execute_req(struct scsi_device *sdev, const unsigned char *cmd, #ifdef CONFIG_PM_RUNTIME extern int scsi_autopm_get_device(struct scsi_device *); extern void scsi_autopm_put_device(struct scsi_device *); +extern void scsi_autopm_put_device_autosuspend(struct scsi_device *); #else static inline int scsi_autopm_get_device(struct scsi_device *d) { return 0; } static inline void scsi_autopm_put_device(struct scsi_device *d) {} +static inline void scsi_autopm_put_device_autosuspend(struct scsi_device *d) {} #endif /* CONFIG_PM_RUNTIME */ static inline int __must_check scsi_device_reprobe(struct scsi_device *sdev) -- 1.7.11.3 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 3/7] scsi: sr: support zero power ODD(ZPODD)
The ODD will be placed into suspend state when: 1 For tray type ODD, no media inside and door closed; 2 For slot type ODD, no media inside; And together with ACPI, when we suspend the ODD's parent(the port it attached to), we will omit the power altogether to reduce power consumption(done in libata-acpi.c). The ODD can be resumed either by user or by software. For user to resume the suspended ODD: 1 For tray type ODD, press the eject button; 2 For slot type ODD, insert a disc; Once such events happened, an ACPI notification will be sent and in our handler, we will power up the ODD and set its status back to active(again in libata-acpi.c). For software to resume the suspended ODD, we did this in ODD's open/release function: we scsi_autopm_get/put_device in scsi_cd_get/put. On old distros, the udisk daemon will poll the ODD and thus ODD will be open/closed every 2 seconds. To make use of ZPODD, udisks' poll has to be inhibited: $ udisks --inhibit-polling /dev/sr0 All of the above depends on if the device can be powered off runtime, which is reflected by the can_power_off flag. Signed-off-by: Aaron Lu --- drivers/ata/libata-acpi.c | 4 +- drivers/scsi/sr.c | 131 - drivers/scsi/sr.h | 2 + include/scsi/scsi_device.h | 1 + 4 files changed, 136 insertions(+), 2 deletions(-) diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c index 902b5a4..a2b16c9 100644 --- a/drivers/ata/libata-acpi.c +++ b/drivers/ata/libata-acpi.c @@ -985,8 +985,10 @@ static void ata_acpi_wake_dev(acpi_handle handle, u32 event, void *context) struct ata_device *ata_dev = context; if (event == ACPI_NOTIFY_DEVICE_WAKE && ata_dev && - pm_runtime_suspended(&ata_dev->sdev->sdev_gendev)) + pm_runtime_suspended(&ata_dev->sdev->sdev_gendev)) { + ata_dev->sdev->wakeup_by_user = 1; scsi_autopm_get_device(ata_dev->sdev); + } } static void ata_acpi_add_pm_notifier(struct ata_device *dev) diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c index abfefab..acfd10a 100644 --- a/drivers/scsi/sr.c +++ b/drivers/scsi/sr.c @@ -45,6 +45,7 @@ #include #include #include +#include #include #include @@ -79,6 +80,8 @@ static DEFINE_MUTEX(sr_mutex); static int sr_probe(struct device *); static int sr_remove(struct device *); static int sr_done(struct scsi_cmnd *); +static int sr_suspend(struct device *, pm_message_t msg); +static int sr_resume(struct device *); static struct scsi_driver sr_template = { .owner = THIS_MODULE, @@ -86,6 +89,8 @@ static struct scsi_driver sr_template = { .name = "sr", .probe = sr_probe, .remove = sr_remove, + .suspend= sr_suspend, + .resume = sr_resume, }, .done = sr_done, }; @@ -147,8 +152,12 @@ static inline struct scsi_cd *scsi_cd_get(struct gendisk *disk) kref_get(&cd->kref); if (scsi_device_get(cd->device)) goto out_put; + if (cd->device->can_power_off && scsi_autopm_get_device(cd->device)) + goto out_pm; goto out; + out_pm: + scsi_device_put(cd->device); out_put: kref_put(&cd->kref, sr_kref_release); cd = NULL; @@ -164,9 +173,93 @@ static void scsi_cd_put(struct scsi_cd *cd) mutex_lock(&sr_ref_mutex); kref_put(&cd->kref, sr_kref_release); scsi_device_put(sdev); + if (sdev->can_power_off) + scsi_autopm_put_device_autosuspend(sdev); mutex_unlock(&sr_ref_mutex); } +static int sr_suspend(struct device *dev, pm_message_t msg) +{ + int poweroff; + struct scsi_sense_hdr sshdr; + struct scsi_cd *cd = dev_get_drvdata(dev); + + /* no action for system pm */ + if (!PMSG_IS_AUTO(msg)) + return 0; + + /* do another TUR to see if the ODD is still ready to be powered off */ + scsi_test_unit_ready(cd->device, SR_TIMEOUT, MAX_RETRIES, &sshdr); + + if (cd->cdi.mask & CDC_CLOSE_TRAY) + /* no media for caddy/slot type ODD */ + poweroff = scsi_sense_valid(&sshdr) && sshdr.asc == 0x3a; + else + /* no media and door closed for tray type ODD */ + poweroff = scsi_sense_valid(&sshdr) && sshdr.asc == 0x3a && + sshdr.ascq == 0x01; + + if (!poweroff) { + pm_runtime_get_noresume(dev); + atomic_set(&cd->suspend_count, 1); + return -EBUSY; + } + + return 0; +} + +static int sr_resume(struct device *dev) +{ + struct scsi_cd *cd; + struct scsi_sense_hdr sshdr; + + cd = dev_get_drvdata(dev); + + /* get the disk ready */ + scsi_test_unit_ready(cd->device, SR_TIMEOUT, MAX_RETRIES, &sshdr); + + /* if user wa
Re: [PATCH] sd: do not set changed flag on all unit attention conditions
On 07/17/2012 11:59 PM, James Bottomley wrote: > On Tue, 2012-07-17 at 12:36 -0400, Christoph Hellwig wrote: >> On Tue, Jul 17, 2012 at 10:11:57AM +0100, James Bottomley wrote: >>> There's no such thing in the market today as a removable disk that's >>> resizeable. Removable disks are for things like backup cartridges and >>> ageing jazz drives. Worse: most removeable devices today are USB card >>> readers whose standards compliance varies from iffy to non existent. >>> Resizeable disks are currently the province of storage arrays. >> >> The virtual disks exported by aacraid are both marked removable and >> can be resized. > > So what are properties of these things? ... or is this just an instance > of a RAID manufacturer hacking around a problem by adding a removable > flag? > Presumably. The general intention is to automatically catch any disk resizing. As the SCSI stack (used to) ignore these things that was their way of working around it. Curiously, though; the aacraid driver is the only one doing this, plus the process is quite involved (using a proprietary application for doing so etc). None of the FC driver do this, despite the fact that resizing a disk is even easier here. I even tried to remove that line once, but then got told off by then Adaptec that I would break their apps. Since then there's a patch in the SLES kernel for adding a module option switching off this behaviour. We should ask Adaptec/PMC-Sierra here. Cheers, Hannes -- Dr. Hannes Reinecke zSeries & Storage h...@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg) -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [RFC] [SCSI] mpt fusion: add support for 0x1000/0x0055
On Sat, 21 Jul 2012, Jiri Kosina wrote: > The device identifies itself as > > 0d:05.0 SCSI storage controller: LSI Logic / Symbios Logic SAS1068 PCI-X > Fusion-MPT SAS (rev 01) Subsystem: NEC Corporation SAS1068 > > and seems to be functionally compatible with 0x0054 PID. > > The request for support of this device has been raised on mailinglists several > times in the past (see [1] [2] and more), but aparently the PCI ID never made > it > to mptsas_pci_table[]. > > [1] http://comments.gmane.org/gmane.linux.scsi/63836 > [2] http://lkml.indiana.edu/hypermail/linux/kernel/0701.2/1715.html > > Signed-off-by: Jiri Kosina > --- > > > I guess the "Subsystem: NEC Corporation" is telling us some rebranding > story, including the PID change ... ? Hi guys, any feedback on this please? Thanks. > > > > drivers/message/fusion/lsi/mpi_cnfg.h |1 + > drivers/message/fusion/mptsas.c |2 ++ > 2 files changed, 3 insertions(+), 0 deletions(-) > > diff --git a/drivers/message/fusion/lsi/mpi_cnfg.h > b/drivers/message/fusion/lsi/mpi_cnfg.h > index d9bcfba..29d54c9 100644 > --- a/drivers/message/fusion/lsi/mpi_cnfg.h > +++ b/drivers/message/fusion/lsi/mpi_cnfg.h > @@ -582,6 +582,7 @@ typedef struct _MSG_CONFIG_REPLY > #define MPI_MANUFACTPAGE_DEVID_SAS1066 (0x005E) > #define MPI_MANUFACTPAGE_DEVID_SAS1066E (0x005A) > #define MPI_MANUFACTPAGE_DEVID_SAS1068 (0x0054) > +#define MPI_MANUFACTPAGE_DEVID_SAS1068_2(0x0055) > #define MPI_MANUFACTPAGE_DEVID_SAS1068E (0x0058) > #define MPI_MANUFACTPAGE_DEVID_SAS1068_820XELP (0x0059) > #define MPI_MANUFACTPAGE_DEVID_SAS1078 (0x0062) > diff --git a/drivers/message/fusion/mptsas.c b/drivers/message/fusion/mptsas.c > index 551262e..2475f8c 100644 > --- a/drivers/message/fusion/mptsas.c > +++ b/drivers/message/fusion/mptsas.c > @@ -5370,6 +5370,8 @@ static struct pci_device_id mptsas_pci_table[] = { > PCI_ANY_ID, PCI_ANY_ID }, > { PCI_VENDOR_ID_LSI_LOGIC, MPI_MANUFACTPAGE_DEVID_SAS1068, > PCI_ANY_ID, PCI_ANY_ID }, > + { PCI_VENDOR_ID_LSI_LOGIC, MPI_MANUFACTPAGE_DEVID_SAS1068_2, > + PCI_ANY_ID, PCI_ANY_ID }, > { PCI_VENDOR_ID_LSI_LOGIC, MPI_MANUFACTPAGE_DEVID_SAS1064E, > PCI_ANY_ID, PCI_ANY_ID }, > { PCI_VENDOR_ID_LSI_LOGIC, MPI_MANUFACTPAGE_DEVID_SAS1068E, > > -- > Jiri Kosina > SUSE Labs > -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND PATCH] SCSI: scsi_lib: fix scsi_io_completion's SG_IO error propagation
On 07/25/2012 07:46 PM, Mike Snitzer wrote: > The following v3.4-rc1 commit unmasked an existing bug in > scsi_io_completion's SG_IO error handling: > 47ac56d [SCSI] scsi_error: classify some ILLEGAL_REQUEST sense as a permanent > TARGET_ERROR > > Given that certain ILLEGAL_REQUEST are now properly categorized as > TARGET_ERROR the host_byte is being set (before host_byte wasn't ever > set for these ILLEGAL_REQUEST). > > In scsi_io_completion, initialize req->errors with cmd->result _after_ > the SG_IO block that calls __scsi_error_from_host_byte (which may > modify the host_byte). > > Before this fix: > > cdb to send: 12 01 01 00 00 00 > ioctl(3, SG_IO, {'S', SG_DXFER_NONE, cmd[6]=[12, 01, 01, 00, 00, 00], > mx_sb_len=32, iovec_count=0, dxfer_len=0, timeout=2, flags=0, > status=02, masked_status=01, sb[19]=[70, 00, 05, 00, 00, 00, 00, 0b, > 00, 00, 00, 00, 24, 00, 00, 00, 00, 00, 00], host_status=0x10, > driver_status=0x8, resid=0, duration=0, info=0x1}) = 0 > SCSI Status: Check Condition > > Sense Information: > sense buffer empty > > After: > > cdb to send: 12 01 01 00 00 00 > ioctl(3, SG_IO, {'S', SG_DXFER_NONE, cmd[6]=[12, 01, 01, 00, 00, 00], > mx_sb_len=32, iovec_count=0, dxfer_len=0, timeout=2, flags=0, > status=02, masked_status=01, sb[19]=[70, 00, 05, 00, 00, 00, 00, 0b, > 00, 00, 00, 00, 24, 00, 00, 00, 00, 00, 00], host_status=0, > driver_status=0x8, resid=0, duration=0, info=0x1}) = 0 > SCSI Status: Check Condition > > Sense Information: > Fixed format, current; Sense key: Illegal Request > Additional sense: Invalid field in cdb > Raw sense data (in hex): > 70 00 05 00 00 00 00 0b 00 00 00 00 24 00 00 00 > 00 00 00 > > Signed-off-by: Mike Snitzer > Reported-by: Paolo Bonzini > Tested-by: Paolo Bonzini > Reviewed-by: Babu Moger > Cc: sta...@vger.kernel.org [v3.4+] Acked-by: Hannes Reinecke Cheers, Hannes-- Dr. Hannes Reinecke zSeries & Storage h...@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg) -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [dm-devel] [RESEND PATCH 3/3] dm mpath: add ability to disable partition creation
On 07/26/2012 07:03 PM, Benjamin Marzinski wrote: > On Wed, Jul 25, 2012 at 02:02:15PM +0200, Hannes Reinecke wrote: [ .. ] >> But that would mean one would have to maintain _two_ configuration >> files, one for multipath and one for kpartx. >> >> Also kpartx initially doesn't have any clue about device names, SCSI >> WWIDs etc. It just takes a device-mapper device and acts upon it. >> The only information it's got is the device-mapper name (which could >> be anything) and the device-mapper UUID if present. >> >> Same goes for udev rules; one would have to traverse the device >> stack backwards to retrieve any information about the hardware. >> >> So when moving the configuration into kpartx we would lose quite >> some flexibility. And reliability, methinks. >> >> Flexilibity is a valid goal, but not when it can achieved only by >> adding complexity to other pieces. > > Kpartx does already grab the dm device name, uuid and target type > (although it doesn't currently use the target type for anything). So, > aside from adding code to read a config file, we don't really have to > add any complexity to be able to have kpartx configure a device based on > dm name (which admittedly probably isn't that useful), dm UUID, or > target type. We could also have it configure based on the blkid > results, which should be present when it's called by udev. When > called manually, kpartx could use libblkid. > Hmpf. Feels like a waste; multipath already has this information ... Anyway. > We do lose the ability to have kpartx easily configure a device based on > hardware type. However, that's not really essential. Multipath needs > it because it can't even finish setting up a device until it knows the > hardware type. For kpartx, it could make configuration simpler in cases > where all of the LUNs from a particular hardware type needed to be > configured the same way. This may be handy, but its not necessary, and > you could achieve the same results using UUIDs. What we gain is the > ability to set this property for non-multipath devices, which currently > isn't possible. Also, blkid based (fs type, fs uuid, and fs label) > configuration seems to me like it would be the most useful. > Oh. Right. Yes, indeed, other target types beside from multipath will benefit from this approach, too. > Now I'm not arguing that adding configuration file handling isn't adding > complexity, and adding libblkid calls would add more. But since > multipath always sets the UUID, I don't see this losing any reliability, > and I does gain the fexibility to work with any dm target type. I > don't feel strongly about this one way or the other, but do see the > benefits of having kpartx own this configuration setting. > The main problem I have with the kpartx approach is that none of the information in the device-mapper device is well-defined. The name certainly isn't, and even the UUID is a case of "let's hope no-one else is daft enough using this". Did we ever get around formalizing how the UUID should look like? If so, and if we could enforce this policy, then we can be sure the UUID is actually meaningful. But if we can't then kpartx would have to crawl back to the parent device to grab some information from there. blkid notwithstanding. And that's just horrible. Cheers, Hannes -- Dr. Hannes Reinecke zSeries & Storage h...@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg) -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] scsi : Adding all the definitions of host bytes in hostbyte_table
Submitted this patch earlier. But it still did not make it to kernel tree. Resubmitting again(recreated on top of 3.5 kernel). Adding all the definitions of host bytes in hostbyte_table. Without this patch, scsi_show_result prints hostbyte as invalid for statuses that are not defined in hostbyte_table(when scsi logging is enabled). Look at scsi_print_result function for better understanding. Signed-off-by: Babu Moger --- --- linux-3.5-rc7/drivers/scsi/constants.c.orig 2012-07-27 13:02:28.0 -0500 +++ linux-3.5-rc7/drivers/scsi/constants.c 2012-07-27 13:03:26.0 -0500 @@ -1422,7 +1422,8 @@ static const char * const hostbyte_table "DID_OK", "DID_NO_CONNECT", "DID_BUS_BUSY", "DID_TIME_OUT", "DID_BAD_TARGET", "DID_ABORT", "DID_PARITY", "DID_ERROR", "DID_RESET", "DID_BAD_INTR", "DID_PASSTHROUGH", "DID_SOFT_ERROR", "DID_IMM_RETRY", "DID_REQUEUE", -"DID_TRANSPORT_DISRUPTED", "DID_TRANSPORT_FAILFAST" }; +"DID_TRANSPORT_DISRUPTED", "DID_TRANSPORT_FAILFAST", "DID_TARGET_FAILURE", +"DID_NEXUS_FAILURE" }; #define NUM_HOSTBYTE_STRS ARRAY_SIZE(hostbyte_table) static const char * const driverbyte_table[]={ -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [RFC] [SCSI] mpt fusion: add support for 0x1000/0x0055
On 7/27/12, Jiri Kosina wrote: > On Sat, 21 Jul 2012, Jiri Kosina wrote: > >> The device identifies itself as >> >> 0d:05.0 SCSI storage controller: LSI Logic / Symbios Logic SAS1068 PCI-X >> Fusion-MPT SAS (rev 01) Subsystem: NEC Corporation SAS1068 >> >> and seems to be functionally compatible with 0x0054 PID. >> >> The request for support of this device has been raised on mailinglists >> several >> times in the past (see [1] [2] and more), but aparently the PCI ID never >> made it >> to mptsas_pci_table[]. >> >> [1] http://comments.gmane.org/gmane.linux.scsi/63836 >> [2] http://lkml.indiana.edu/hypermail/linux/kernel/0701.2/1715.html >> >> Signed-off-by: Jiri Kosina >> --- >> >> >> I guess the "Subsystem: NEC Corporation" is telling us some rebranding >> story, including the PID change ... ? > > Hi guys, > > any feedback on this please? > > Thanks. NACK. Vendor 0x1000, Device id 0x0055 is actually an old LSI MegaRAID 1068 based software raid board. This device was never qualified nor intended to be used with the mpt fusion driver. -Adam -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [RFC] [SCSI] mpt fusion: add support for 0x1000/0x0055
On Fri, 27 Jul 2012, adam radford wrote: > >> The device identifies itself as > >> > >> 0d:05.0 SCSI storage controller: LSI Logic / Symbios Logic SAS1068 PCI-X > >> Fusion-MPT SAS (rev 01) Subsystem: NEC Corporation SAS1068 > >> > >> and seems to be functionally compatible with 0x0054 PID. > >> > >> The request for support of this device has been raised on mailinglists > >> several > >> times in the past (see [1] [2] and more), but aparently the PCI ID never > >> made it > >> to mptsas_pci_table[]. > >> > >> [1] http://comments.gmane.org/gmane.linux.scsi/63836 > >> [2] http://lkml.indiana.edu/hypermail/linux/kernel/0701.2/1715.html > >> > >> Signed-off-by: Jiri Kosina > >> --- > >> > >> > >> I guess the "Subsystem: NEC Corporation" is telling us some rebranding > >> story, including the PID change ... ? > > > > Hi guys, > > > > any feedback on this please? > > > > Thanks. > > NACK. > > Vendor 0x1000, Device id 0x0055 is actually an old LSI MegaRAID 1068 > based software raid board. This device was never qualified nor > intended to be used with the mpt fusion driver. So, what is the alternative? The only thing I know is that it works at least in basic mode (haven't tested performance at all). Thanks, -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Status update for tcm_vhost driver merge
Hello folks, The RFC-v5 patch for tcm_vhost kernel code was sent out for review a bit less than 24 hours ago, and thus far there has not been any additional comments.. Thanks to everyone who has been participating in the various threads over the past week and giving their feedback! Also, just a heads up that I've added a target-pending/for-linus branch containing the same RFC-v5 code cut against recent commit bdc0077af574 @ -rc0 code has been added to include the net-next + scsi-misc bits now merged into mainline. This afternoon running tcm_vhost on -rc0 code the virtio-scsi client (running 3.5-rc2 + scsi-misc bugfix now in mainline) is able to connect + perform target vhost LUN SCAN as expected. The same heavy small block mixed random I/O workloads are running with IBLOCK via raw flash backends, and I'll be running more tests on -rc0 code over the next days. So this merge commit will likely be updated once more as MST returns from a week of full of restful thoughts, but at this point his last minor off-list comments have been addressed in RFC-v5 code. Also, I'm not sure for an initial merge if upstream would prefer to just pull with the necessary ACK from MST via for-next-merge here (and sort out the vhost duplicates himself), or take a for-linus branch that's cut on top of -rc0 with the necessary mainline deps..? Linus (CC'ed), what would you prefer in this type of case..? Thank you, --nab -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [RFC] [SCSI] mpt fusion: add support for 0x1000/0x0055
On 7/27/12, Jiri Kosina wrote: ... > > So, what is the alternative? > > The only thing I know is that it works at least in basic mode (haven't > tested performance at all). The driver for your card is a closed source driver called 'megasr'. Here is a link to the LSI download page for this card/driver: http://www.lsi.com/support/Pages/Download-Results.aspx?productcode=P00041&assettype=0&component=Storage%20Component&productfamily=Legacy%20RAID%20Controllers&productname=MegaRAID%20SAS%208208XLP -Adam -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html