[PATCH 0/1] Fix oops while resetting an ipr adapter
When hotplug removing an adapter, we've seen the issues with scsi_unblock_requests running after the adapter's memory has been freed. This patch fixes the oops while resetting an ipr adapter. Thanks! Wendy -- -- 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 1/1] ipr: Fix oops while resetting an ipr adapter
From: Brian King When resetting an ipr adapter, we use scsi_block_requests to block any new commands from scsi core, and then unblock after the reset. When hotplug removing an adapter, we shut it down and go through this same code, but we've seen issues with scsi_unblock_requests running after the adapter's memory has been freed. There is really no need to block/unblock when the adapter is being removed, so this patch skips the block/unblock and will immediately fail any commands that happen to make it to queuecommand while the adapter is being shutdown. Signed-off-by: Brian King Signed-off-by: Wen Xiong --- drivers/scsi/ipr.c | 33 +++-- drivers/scsi/ipr.h |1 + 2 files changed, 24 insertions(+), 10 deletions(-) Index: b/drivers/scsi/ipr.c === --- a/drivers/scsi/ipr.c2013-01-21 13:45:10.0 -0600 +++ b/drivers/scsi/ipr.c2013-01-22 14:21:00.394286989 -0600 @@ -6112,7 +6112,7 @@ static int ipr_queuecommand(struct Scsi_ * We have told the host to stop giving us new requests, but * ERP ops don't count. FIXME */ - if (unlikely(!hrrq->allow_cmds && !hrrq->ioa_is_dead)) { + if (unlikely(!hrrq->allow_cmds && !hrrq->ioa_is_dead && !hrrq->removing_ioa)) { spin_unlock_irqrestore(hrrq->lock, hrrq_flags); return SCSI_MLQUEUE_HOST_BUSY; } @@ -6121,7 +6121,7 @@ static int ipr_queuecommand(struct Scsi_ * FIXME - Create scsi_set_host_offline interface * and the ioa_is_dead check can be removed */ - if (unlikely(hrrq->ioa_is_dead || !res)) { + if (unlikely(hrrq->ioa_is_dead || hrrq->removing_ioa || !res)) { spin_unlock_irqrestore(hrrq->lock, hrrq_flags); goto err_nodev; } @@ -6741,14 +6741,17 @@ static int ipr_ioa_bringdown_done(struct struct ipr_ioa_cfg *ioa_cfg = ipr_cmd->ioa_cfg; ENTER; + if (!ioa_cfg->hrrq[IPR_INIT_HRRQ].removing_ioa) { + ipr_trace; + spin_unlock_irq(ioa_cfg->host->host_lock); + scsi_unblock_requests(ioa_cfg->host); + spin_lock_irq(ioa_cfg->host->host_lock); + } + ioa_cfg->in_reset_reload = 0; ioa_cfg->reset_retries = 0; list_add_tail(&ipr_cmd->queue, &ipr_cmd->hrrq->hrrq_free_q); wake_up_all(&ioa_cfg->reset_wait_q); - - spin_unlock_irq(ioa_cfg->host->host_lock); - scsi_unblock_requests(ioa_cfg->host); - spin_lock_irq(ioa_cfg->host->host_lock); LEAVE; return IPR_RC_JOB_RETURN; @@ -8494,7 +8497,8 @@ static void _ipr_initiate_ioa_reset(stru spin_unlock(&ioa_cfg->hrrq[i]._lock); } wmb(); - scsi_block_requests(ioa_cfg->host); + if (!ioa_cfg->hrrq[IPR_INIT_HRRQ].removing_ioa) + scsi_block_requests(ioa_cfg->host); ipr_cmd = ipr_get_free_ipr_cmnd(ioa_cfg); ioa_cfg->reset_cmd = ipr_cmd; @@ -8549,9 +8553,11 @@ static void ipr_initiate_ioa_reset(struc ipr_fail_all_ops(ioa_cfg); wake_up_all(&ioa_cfg->reset_wait_q); - spin_unlock_irq(ioa_cfg->host->host_lock); - scsi_unblock_requests(ioa_cfg->host); - spin_lock_irq(ioa_cfg->host->host_lock); + if (!ioa_cfg->hrrq[IPR_INIT_HRRQ].removing_ioa) { + spin_unlock_irq(ioa_cfg->host->host_lock); + scsi_unblock_requests(ioa_cfg->host); + spin_lock_irq(ioa_cfg->host->host_lock); + } return; } else { ioa_cfg->in_ioa_bringdown = 1; @@ -9695,6 +9701,7 @@ static void __ipr_remove(struct pci_dev { unsigned long host_lock_flags = 0; struct ipr_ioa_cfg *ioa_cfg = pci_get_drvdata(pdev); + int i; ENTER; spin_lock_irqsave(ioa_cfg->host->host_lock, host_lock_flags); @@ -9704,6 +9711,12 @@ static void __ipr_remove(struct pci_dev spin_lock_irqsave(ioa_cfg->host->host_lock, host_lock_flags); } + for (i = 0; i < ioa_cfg->hrrq_num; i++) { + spin_lock(&ioa_cfg->hrrq[i]._lock); + ioa_cfg->hrrq[i].removing_ioa = 1; + spin_unlock(&ioa_cfg->hrrq[i]._lock); + } + wmb(); ipr_initiate_ioa_bringdown(ioa_cfg, IPR_SHUTDOWN_NORMAL); spin_unlock_irqrestore(ioa_cfg->host->host_lock, host_lock_flags); Index: b/drivers/scsi/ipr.h === --- a/drivers/scsi/ipr.h2013-01-21 13:45:10.0 -0600 +++ b/drivers/scsi/ipr.h2013-01-22 11:10:23.414280773 -0600 @@ -493,6 +493,7 @@ struct ipr_hrr_queue { u8 allow_interrupts:1; u8 ioa_is_dead:1; u8 allow_c
Re: [PATCH v8 4/4] sd: change to auto suspend mode
On Wed, Jan 30, 2013 at 10:38:26AM -0500, Alan Stern wrote: > On Wed, 30 Jan 2013, Aaron Lu wrote: > > > From: Lin Ming > > > > Uses block layer runtime pm helper functions in > > scsi_runtime_suspend/resume for devices that take advantage of it. > > > > Remove scsi_autopm_* from sd open/release path and check_events path. > > > > Signed-off-by: Lin Ming > > Signed-off-by: Aaron Lu > > A couple of very minor suggestions... > > > --- a/drivers/scsi/scsi_pm.c > > +++ b/drivers/scsi/scsi_pm.c > > > @@ -144,18 +144,44 @@ static int scsi_bus_restore(struct device *dev) > > > > #ifdef CONFIG_PM_RUNTIME > > > > +static int scsi_blk_runtime_suspend(struct device *dev) > > +{ > > + struct scsi_device *sdev = to_scsi_device(dev); > > For this routine and the other new ones, it may be slightly more > efficient to pass both dev and sdev as arguments (this depends on how > smart the compiler's optimizer is). The caller already knows both of > them, after all. What about passing only scsi_device? When device is needed, I can use &sdev->sdev_gendev. Is this equally efficient? > > static int scsi_runtime_suspend(struct device *dev) > > { > > int err = 0; > > - const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; > > > > dev_dbg(dev, "scsi_runtime_suspend\n"); > > if (scsi_is_sdev_device(dev)) { > > - err = scsi_dev_type_suspend(dev, > > - pm ? pm->runtime_suspend : NULL); > > - if (err == -EAGAIN) > > - pm_schedule_suspend(dev, jiffies_to_msecs( > > - round_jiffies_up_relative(HZ/10))); > > + struct scsi_device *sdev = to_scsi_device(dev); > > There should be a blank line between the declaration and the > executable code. OK, will change this. > > @@ -185,10 +233,17 @@ static int scsi_runtime_idle(struct device *dev) > > > > /* Insert hooks here for targets, hosts, and transport classes */ > > > > - if (scsi_is_sdev_device(dev)) > > - err = pm_schedule_suspend(dev, 100); > > - else > > + if (scsi_is_sdev_device(dev)) { > > + struct scsi_device *sdev = to_scsi_device(dev); > > Blank line. > > > + if (sdev->request_queue->dev) { > > + pm_runtime_mark_last_busy(dev); > > + err = pm_runtime_autosuspend(dev); > > + } else { > > + err = pm_schedule_suspend(dev, 100); > > + } > > + } else { > > err = pm_runtime_suspend(dev); > > + } > > return err; Shall we ignore the return value for these pm_xxx_suspend functions? I mean we do not need to record the return value for them and return it, since pm core doesn't care the return value of idle callback. Thanks, Aaron -- 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 v8 2/4] block: add runtime pm helpers
On Wed, Jan 30, 2013 at 10:54:53AM -0500, Alan Stern wrote: > On Wed, 30 Jan 2013, Aaron Lu wrote: > > > From: Lin Ming > > > > Add runtime pm helper functions: > > > > void blk_pm_runtime_init(struct request_queue *q, struct device *dev) > > - Initialization function for drivers to call. > > > > int blk_pre_runtime_suspend(struct request_queue *q) > > - If any requests are in the queue, mark last busy and return -EBUSY. > > Otherwise set q->rpm_status to RPM_SUSPENDING and return 0. > > > > void blk_post_runtime_suspend(struct request_queue *q, int err) > > - If the suspend succeeded then set q->rpm_status to RPM_SUSPENDED. > > Otherwise set it to RPM_ACTIVE. > > > > void blk_pre_runtime_resume(struct request_queue *q) > > - Set q->rpm_status to RPM_RESUMING. > > > > void blk_post_runtime_resume(struct request_queue *q, int err) > > - If the resume succeeded then set q->rpm_status to RPM_ACTIVE > > and call __blk_run_queue, then mark last busy and autosuspend. > > Otherwise set q->rpm_status to RPM_SUSPENDED. > > > > Signed-off-by: Lin Ming > > Signed-off-by: Aaron Lu > > > +void blk_pm_runtime_init(struct request_queue *q, struct device *dev) > > +{ > > + q->dev = dev; > > + q->rpm_status = RPM_ACTIVE; > > + pm_runtime_set_autosuspend_delay(q->dev, -1); > > + pm_runtime_use_autosuspend(q->dev); > > + pm_runtime_mark_last_busy(q->dev); > > + pm_runtime_autosuspend(q->dev); > > This last line is no longer needed. It can't do anything useful, since > autosuspends are disabled (the delay is -1). Right, thanks. And the mark_last_busy can probably be removed too, it didn't make much sense here and we can add "driver should call pm_runtime_mark_last_busy and pm_runtime_autosuspend in its runtime idle callback" to the kernel doc. What do you think? Thanks, Aaron -- 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 1/6] qla2xxx: Determine the number of outstanding commands based on available resources.
FYI, the fix the smatch warning: + drivers/scsi/qla2xxx/qla_os.c:364 qla2x00_free_req_que() info: + redundant null check on req->outstanding_cmds calling kfree() has been merged with this patch. On Wed, 30 Jan 2013, Saurav Kashyap wrote: From: Chad Dupuis Base the number of outstanding requests the driver will keep track of on the available resources instead of being hard-coded. Signed-off-by: Chad Dupuis Signed-off-by: Saurav Kashyap --- drivers/scsi/qla2xxx/qla_bsg.c|2 +- drivers/scsi/qla2xxx/qla_dbg.c|2 +- drivers/scsi/qla2xxx/qla_def.h| 12 drivers/scsi/qla2xxx/qla_gbl.h|3 ++ drivers/scsi/qla2xxx/qla_init.c | 54 +++- drivers/scsi/qla2xxx/qla_iocb.c | 37 - drivers/scsi/qla2xxx/qla_isr.c| 10 +++--- drivers/scsi/qla2xxx/qla_mbx.c|8 +++--- drivers/scsi/qla2xxx/qla_mid.c|7 - drivers/scsi/qla2xxx/qla_nx.c |2 +- drivers/scsi/qla2xxx/qla_os.c | 11 +-- drivers/scsi/qla2xxx/qla_target.c |4 +- drivers/scsi/qla2xxx/qla_target.h |5 ++- 13 files changed, 110 insertions(+), 47 deletions(-) diff --git a/drivers/scsi/qla2xxx/qla_bsg.c b/drivers/scsi/qla2xxx/qla_bsg.c index 9f34ded..f7cb6a3 100644 --- a/drivers/scsi/qla2xxx/qla_bsg.c +++ b/drivers/scsi/qla2xxx/qla_bsg.c @@ -1950,7 +1950,7 @@ qla24xx_bsg_timeout(struct fc_bsg_job *bsg_job) if (!req) continue; - for (cnt = 1; cnt < MAX_OUTSTANDING_COMMANDS; cnt++) { + for (cnt = 1; cnt < req->num_outstanding_cmds; cnt++) { sp = req->outstanding_cmds[cnt]; if (sp) { if (((sp->type == SRB_CT_CMD) || diff --git a/drivers/scsi/qla2xxx/qla_dbg.c b/drivers/scsi/qla2xxx/qla_dbg.c index 53f9e49..2f3e765 100644 --- a/drivers/scsi/qla2xxx/qla_dbg.c +++ b/drivers/scsi/qla2xxx/qla_dbg.c @@ -11,7 +11,7 @@ * -- * | Level| Last Value Used | Holes| * -- - * | Module Init and Probe| 0x0125 | 0x4b,0xba,0xfa | + * | Module Init and Probe| 0x0126 | 0x4b,0xba,0xfa | * | Mailbox commands | 0x114f | 0x111a-0x111b | * | || 0x112c-0x112e | * | || 0x113a | diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h index 6e7727f..a84bb8d 100644 --- a/drivers/scsi/qla2xxx/qla_def.h +++ b/drivers/scsi/qla2xxx/qla_def.h @@ -253,8 +253,8 @@ #define LOOP_DOWN_TIME255 /* 240 */ #define LOOP_DOWN_RESET (LOOP_DOWN_TIME - 30) -/* Maximum outstanding commands in ISP queues (1-65535) */ -#define MAX_OUTSTANDING_COMMANDS 1024 +#define DEFAULT_OUTSTANDING_COMMANDS 1024 +#define MIN_OUTSTANDING_COMMANDS 128 /* ISP request and response entry counts (37-65535) */ #define REQUEST_ENTRY_CNT_2100128 /* Number of request entries. */ @@ -2533,8 +2533,9 @@ struct req_que { uint16_t qos; uint16_t vp_idx; struct rsp_que *rsp; - srb_t *outstanding_cmds[MAX_OUTSTANDING_COMMANDS]; + srb_t **outstanding_cmds; uint32_t current_outstanding_cmd; + uint16_t num_outstanding_cmds; int max_q_depth; }; @@ -2561,7 +2562,7 @@ struct qlt_hw_data { void *target_lport_ptr; struct qla_tgt_func_tmpl *tgt_ops; struct qla_tgt *qla_tgt; - struct qla_tgt_cmd *cmds[MAX_OUTSTANDING_COMMANDS]; + struct qla_tgt_cmd *cmds[DEFAULT_OUTSTANDING_COMMANDS]; uint16_t current_handle; struct qla_tgt_vp_map *tgt_vp_map; @@ -2887,6 +2888,7 @@ struct qla_hw_data { #define RISC_START_ADDRESS_2300 0x800 #define RISC_START_ADDRESS_2400 0x10 uint16_tfw_xcb_count; + uint16_tfw_iocb_count; uint16_tfw_options[16]; /* slots: 1,2,3,10,11 */ uint8_t fw_seriallink_options[4]; @@ -3248,8 +3250,6 @@ struct qla_tgt_vp_map { #define NVRAM_DELAY() udelay(10) -#define INVALID_HANDLE (MAX_OUTSTANDING_COMMANDS+1) - /* * Flash support definitions */ diff --git a/drivers/scsi/qla2xxx/qla_gbl.h b/drivers/scsi/qla2xxx/qla_gbl.h index 2411d1a..fba0651 100644 --- a/drivers/scsi/qla2xxx/qla_gbl.h +++ b/drivers/scsi/qla2xxx/qla_gbl.h @@ -84,6 +84,9 @@ extern int qla83xx_nic_core_reset(scsi_qla_host_t *); extern void qla83xx_reset_ownership(scsi_qla_host_t *); extern int qla2xxx_mctp_dump(scsi_qla_host_t *); +extern int +qla2x00_alloc_outstanding_cmds(struct qla_hw_data *, struct req_que *); + /* * Global Data in qla_os.c source file. */ diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c index 563eee3..81e8cca 100644 --- a/drivers/scsi/qla2xxx/qla_init.c ++
RE: storvsc loops with No Sense messages
> -Original Message- > From: Olaf Hering [mailto:o...@aepfle.de] > Sent: Wednesday, January 30, 2013 8:35 AM > To: KY Srinivasan > Cc: linux-scsi@vger.kernel.org > Subject: Re: storvsc loops with No Sense messages > > On Wed, Jan 30, Olaf Hering wrote: > > > Is there a way to not use WRITE_SAME at all? While browsing the code its > > not clear if there is a conditional for this command. > > It seems scsi_device->no_write_same may avoid this command, I will > test this patch: > > # Subject: [PATCH] scsi: storvsc: avoid usage of WRITE_SAME > > Set scsi_device->no_write_same because the host does not support it. > Also blacklist WRITE_SAME to avoid (and log) accident usage. > Olaf, Thanks for looking into this. Acked-by: K. Y. Srinivasan > Signed-off-by: Olaf Hering > --- > drivers/scsi/storvsc_drv.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c > index 0144078..21f788a 100644 > --- a/drivers/scsi/storvsc_drv.c > +++ b/drivers/scsi/storvsc_drv.c > @@ -1155,6 +1155,8 @@ static int storvsc_device_configure(struct scsi_device > *sdevice) > > blk_queue_bounce_limit(sdevice->request_queue, BLK_BOUNCE_ANY); > > + sdevice->no_write_same = 1; > + > return 0; > } > > @@ -1241,6 +1243,7 @@ static bool storvsc_scsi_cmd_ok(struct scsi_cmnd > *scmnd) >* smartd sends this command and the host does not handle >* this. So, don't send it. >*/ > + case WRITE_SAME: > case SET_WINDOW: > scmnd->result = ILLEGAL_REQUEST << 16; > allowed = false; > -- > 1.8.1.1 > >
RE: storvsc loops with No Sense messages
> -Original Message- > From: Olaf Hering [mailto:o...@aepfle.de] > Sent: Wednesday, January 30, 2013 5:00 AM > To: KY Srinivasan > Cc: linux-scsi@vger.kernel.org > Subject: Re: storvsc loops with No Sense messages > > On Tue, Jan 29, Olaf Hering wrote: > > > On Tue, Jan 29, KY Srinivasan wrote: > > > > > Was the installation done on a vhd or VHDX. I checked with Hyper-V > > > guys and they say they never supported WRITE_SAME. I am wondering how > > > it worked on ws2008. > > > > Its a vhdx image, both fixed and dynamically size have the same issue. > > I will see what happens with ws2008, at least the install proceeds > > without issues. > > I see that WRITE_SAME is also used when running on ws2008, but > appearently it does not cause any issues. > This triggers only when the default ext4 is used, not when I force ext3 > for the root filesystem. Talking to the MSFT storage guys, WRITE_SAME has never been supported - not on ws2008 or on ws2012. So I am not sure why or how this works on ws2008. K. Y > So to me it looks like ws2012 has issues with WRITE_SAME handling. > > > Is there a way to not use WRITE_SAME at all? While browsing the code its > not clear if there is a conditional for this command. > > > Olaf >
Re: [patch] [SCSI] libosd: check for kzalloc() failure
On 01/30/2013 09:06 AM, Dan Carpenter wrote: > There wasn't any error handling for this kzalloc(). > ACK-by: Boaz Harrosh James please queue for inclusion > Signed-off-by: Dan Carpenter Thanks Dan > > diff --git a/drivers/scsi/osd/osd_initiator.c > b/drivers/scsi/osd/osd_initiator.c > index c06b8e5..d8293f2 100644 > --- a/drivers/scsi/osd/osd_initiator.c > +++ b/drivers/scsi/osd/osd_initiator.c > @@ -144,6 +144,10 @@ static int _osd_get_print_system_info(struct osd_dev *od, > odi->osdname_len = get_attrs[a].len; > /* Avoid NULL for memcmp optimization 0-length is good enough */ > odi->osdname = kzalloc(odi->osdname_len + 1, GFP_KERNEL); > + if (!odi->osdname) { > + ret = -ENOMEM; > + goto out; > + } > if (odi->osdname_len) > memcpy(odi->osdname, get_attrs[a].val_ptr, odi->osdname_len); > OSD_INFO("OSD_NAME [%s]\n", odi->osdname); > -- 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 v8 2/4] block: add runtime pm helpers
On Wed, 30 Jan 2013, Aaron Lu wrote: > From: Lin Ming > > Add runtime pm helper functions: > > void blk_pm_runtime_init(struct request_queue *q, struct device *dev) > - Initialization function for drivers to call. > > int blk_pre_runtime_suspend(struct request_queue *q) > - If any requests are in the queue, mark last busy and return -EBUSY. > Otherwise set q->rpm_status to RPM_SUSPENDING and return 0. > > void blk_post_runtime_suspend(struct request_queue *q, int err) > - If the suspend succeeded then set q->rpm_status to RPM_SUSPENDED. > Otherwise set it to RPM_ACTIVE. > > void blk_pre_runtime_resume(struct request_queue *q) > - Set q->rpm_status to RPM_RESUMING. > > void blk_post_runtime_resume(struct request_queue *q, int err) > - If the resume succeeded then set q->rpm_status to RPM_ACTIVE > and call __blk_run_queue, then mark last busy and autosuspend. > Otherwise set q->rpm_status to RPM_SUSPENDED. > > Signed-off-by: Lin Ming > Signed-off-by: Aaron Lu > +void blk_pm_runtime_init(struct request_queue *q, struct device *dev) > +{ > + q->dev = dev; > + q->rpm_status = RPM_ACTIVE; > + pm_runtime_set_autosuspend_delay(q->dev, -1); > + pm_runtime_use_autosuspend(q->dev); > + pm_runtime_mark_last_busy(q->dev); > + pm_runtime_autosuspend(q->dev); This last line is no longer needed. It can't do anything useful, since autosuspends are disabled (the delay is -1). Alan Stern -- 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] [SCSI] libosd: check for kzalloc() failure
On 01/30/2013 04:34 PM, walter harms wrote: > <> > I start to see the complexity of the situation. Would you mind to add > the comment "it can be anything. UTF-8 is more likely but not guaranteed > either" ? > For now using a pascal-string seems the best solution but it should be warned > that get_attrs[a].val_ptr is NOT a c-string and therefore can not be used with > the printf-family (i guess the situation will become more clear in future) > OK, So... The long story The STD says that osdname can be *anything* binary or otherwise, and of *any* length. And is used to uniquely identify the OSD-device/partition in a cluster. We decided that if so we would stick an 40 char UUID in it, generated by a uuidgen and all the external utilities and surrounding code forces it, and treat it as a c-string. But this code here in the core cannot make that assumption and still support the STD. On the other hand we did want the osdname to be printable in traces and messages because it is such an important identifier. So I have decided to sacrifice an extra char in-memory to carry a \NUL and safely stick it into printk(s). Those (none existent) OSD devices that will put unprintable characters in here will still function fine, but will look real scary in printk(s). Please note that the one that sets policy is the osd-target vendor. (And they all currently use my code base) So save the kzalloc check this code (tested) is safe and will show strings when present, but will gracefully show ugly things but still work when not. > I have no clue why you need that, using c-strings makes life more easy in the > last minute a sprintf(buf,"%u") will save the day ;) > Actually it is very funny, because just recently (last week) I have discovered something that eliminates all those funny business. printf("%*s", odi->osdname, odi->osdname_len); The "*" will instruct c to expect an extra variable following %s which is the max_length of %s. This is exactly for pascal strings and the such like above. So I added a TODO to clean that a bit by always printing with "%*s" >> > It look clever to add the NULL test here. > Noone reviewing the code will understand that. > (Rule of least surprise) > Thanks for looking, I agree it needs a fat comment, I'll do that when I'll convert to above system. Thanks for looking, That code is really old and never had any extra eyes on it. > just my 2 cents, > re, > wh > Cheers Boaz -- 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 v8 4/4] sd: change to auto suspend mode
On Wed, 30 Jan 2013, Aaron Lu wrote: > From: Lin Ming > > Uses block layer runtime pm helper functions in > scsi_runtime_suspend/resume for devices that take advantage of it. > > Remove scsi_autopm_* from sd open/release path and check_events path. > > Signed-off-by: Lin Ming > Signed-off-by: Aaron Lu A couple of very minor suggestions... > --- a/drivers/scsi/scsi_pm.c > +++ b/drivers/scsi/scsi_pm.c > @@ -144,18 +144,44 @@ static int scsi_bus_restore(struct device *dev) > > #ifdef CONFIG_PM_RUNTIME > > +static int scsi_blk_runtime_suspend(struct device *dev) > +{ > + struct scsi_device *sdev = to_scsi_device(dev); For this routine and the other new ones, it may be slightly more efficient to pass both dev and sdev as arguments (this depends on how smart the compiler's optimizer is). The caller already knows both of them, after all. > + int err; > + > + err = blk_pre_runtime_suspend(sdev->request_queue); > + if (err) > + return err; > + err = pm_generic_runtime_suspend(dev); > + blk_post_runtime_suspend(sdev->request_queue, err); > + > + return err; > +} > + > +static int scsi_dev_runtime_suspend(struct device *dev) > +{ > + const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; > + int err; > + > + err = scsi_dev_type_suspend(dev, pm ? pm->runtime_suspend : NULL); > + if (err == -EAGAIN) > + pm_schedule_suspend(dev, jiffies_to_msecs( > + round_jiffies_up_relative(HZ/10))); > + > + return err; > +} > + > static int scsi_runtime_suspend(struct device *dev) > { > int err = 0; > - const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; > > dev_dbg(dev, "scsi_runtime_suspend\n"); > if (scsi_is_sdev_device(dev)) { > - err = scsi_dev_type_suspend(dev, > - pm ? pm->runtime_suspend : NULL); > - if (err == -EAGAIN) > - pm_schedule_suspend(dev, jiffies_to_msecs( > - round_jiffies_up_relative(HZ/10))); > + struct scsi_device *sdev = to_scsi_device(dev); There should be a blank line between the declaration and the executable code. > + if (sdev->request_queue->dev) > + err = scsi_blk_runtime_suspend(dev); > + else > + err = scsi_dev_runtime_suspend(dev); > } > > /* Insert hooks here for targets, hosts, and transport classes */ > @@ -163,14 +189,36 @@ static int scsi_runtime_suspend(struct device *dev) > return err; > } > > +static int scsi_blk_runtime_resume(struct device *dev) > +{ > + struct scsi_device *sdev = to_scsi_device(dev); > + int err; > + > + blk_pre_runtime_resume(sdev->request_queue); > + err = pm_generic_runtime_resume(dev); > + blk_post_runtime_resume(sdev->request_queue, err); > + > + return err; > +} > + > +static int scsi_dev_runtime_resume(struct device *dev) > +{ > + const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; Blank line. > + return scsi_dev_type_resume(dev, pm ? pm->runtime_resume : NULL); > +} > + > static int scsi_runtime_resume(struct device *dev) > { > int err = 0; > - const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; > > dev_dbg(dev, "scsi_runtime_resume\n"); > - if (scsi_is_sdev_device(dev)) > - err = scsi_dev_type_resume(dev, pm ? pm->runtime_resume : NULL); > + if (scsi_is_sdev_device(dev)) { > + struct scsi_device *sdev = to_scsi_device(dev); Blank line. > + if (sdev->request_queue->dev) > + err = scsi_blk_runtime_resume(dev); > + else > + err = scsi_dev_runtime_resume(dev); > + } > > /* Insert hooks here for targets, hosts, and transport classes */ > > @@ -185,10 +233,17 @@ static int scsi_runtime_idle(struct device *dev) > > /* Insert hooks here for targets, hosts, and transport classes */ > > - if (scsi_is_sdev_device(dev)) > - err = pm_schedule_suspend(dev, 100); > - else > + if (scsi_is_sdev_device(dev)) { > + struct scsi_device *sdev = to_scsi_device(dev); Blank line. > + if (sdev->request_queue->dev) { > + pm_runtime_mark_last_busy(dev); > + err = pm_runtime_autosuspend(dev); > + } else { > + err = pm_schedule_suspend(dev, 100); > + } > + } else { > err = pm_runtime_suspend(dev); > + } > return err; > } Alan Stern -- 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: storvsc: avoid usage of WRITE_SAME
Set scsi_device->no_write_same because the host does not support it. Also blacklist WRITE_SAME to avoid (and log) accident usage. If the guest uses the ext4 filesystem, storvsc hangs while it prints these messages in an endless loop: ... [ 161.459523] hv_storvsc vmbus_0_1: cmd 0x41 scsi status 0x2 srb status 0x6 [ 161.462157] sd 2:0:0:0: [sda] [ 161.463135] Sense Key : No Sense [current] [ 161.464983] sd 2:0:0:0: [sda] [ 161.465899] Add. Sense: No additional sense information [ 161.468211] hv_storvsc vmbus_0_1: cmd 0x41 scsi status 0x2 srb status 0x6 [ 161.475766] sd 2:0:0:0: [sda] [ 161.476728] Sense Key : No Sense [current] [ 161.478284] sd 2:0:0:0: [sda] [ 161.479441] Add. Sense: No additional sense information ... This happens with a guest running on Windows Server 2012, but happens to work while running on Windows Server 2008. WRITE_SAME isnt really supported by both versions, so disable the command usage globally. Signed-off-by: Olaf Hering Cc: KY Srinivasan --- drivers/scsi/storvsc_drv.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c index 0144078..314586c 100644 --- a/drivers/scsi/storvsc_drv.c +++ b/drivers/scsi/storvsc_drv.c @@ -1155,6 +1155,8 @@ static int storvsc_device_configure(struct scsi_device *sdevice) blk_queue_bounce_limit(sdevice->request_queue, BLK_BOUNCE_ANY); + sdevice->no_write_same = 1; + return 0; } @@ -1237,6 +1239,8 @@ static bool storvsc_scsi_cmd_ok(struct scsi_cmnd *scmnd) u8 scsi_op = scmnd->cmnd[0]; switch (scsi_op) { + /* the host does not handle WRITE_SAME, log accident usage */ + case WRITE_SAME: /* * smartd sends this command and the host does not handle * this. So, don't send it. -- 1.8.1.1 -- 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] [SCSI] libosd: check for kzalloc() failure
Am 30.01.2013 14:40, schrieb Benny Halevy: > On Wed, Jan 30, 2013 at 3:00 PM, walter harms wrote: >> >> >> Am 30.01.2013 10:51, schrieb Benny Halevy: >>> On Wed, Jan 30, 2013 at 10:57 AM, walter harms wrote: Am 30.01.2013 09:27, schrieb Dan Carpenter: > On Wed, Jan 30, 2013 at 09:15:43AM +0100, walter harms wrote: >> >> >> Am 30.01.2013 08:06, schrieb Dan Carpenter: >>> There wasn't any error handling for this kzalloc(). >>> >>> Signed-off-by: Dan Carpenter >>> >>> diff --git a/drivers/scsi/osd/osd_initiator.c >>> b/drivers/scsi/osd/osd_initiator.c >>> index c06b8e5..d8293f2 100644 >>> --- a/drivers/scsi/osd/osd_initiator.c >>> +++ b/drivers/scsi/osd/osd_initiator.c >>> @@ -144,6 +144,10 @@ static int _osd_get_print_system_info(struct >>> osd_dev *od, >>> odi->osdname_len = get_attrs[a].len; >>> /* Avoid NULL for memcmp optimization 0-length is good enough */ >>> odi->osdname = kzalloc(odi->osdname_len + 1, GFP_KERNEL); >>> + if (!odi->osdname) { >>> + ret = -ENOMEM; >>> + goto out; >>> + } >>> if (odi->osdname_len) >>> memcpy(odi->osdname, get_attrs[a].val_ptr, >>> odi->osdname_len); >>> OSD_INFO("OSD_NAME [%s]\n", odi->osdname); >>> -- >> >> this looks like strdup() ? >> > > Maybe? It's a funny thing going on with the NUL terminator and I > don't understand what the comment is about. > > It appears that normally "get_attrs[a].val_ptr" is a NUL terminated > string but "get_attrs[a].len" does not count the terminator. > > Odd. > i have no clue what the programmer was thinking. if i read this correct osdname is u8 *osdname; so a simple strdup() or strndup() would be ok the comment seems to indicate that get_attrs[a].val_ptr could be NULL but where is the check ? Perhaps they are not using ascii here ? then a memdup(get_attrs[a].len) would be better. >>> >>> There are subtle differences between kstrdup or kmemdup and this >>> implementation. >>> >>> kmemdup is problematic as get_attrs[a].val_ptr does not contain the >>> NUL terminator >> >> ok, i understand - but can we assume that we are talking ascii here ? >> > > No, it can be anything. UTF-8 is more likely but not guaranteed either. > I start to see the complexity of the situation. Would you mind to add the comment "it can be anything. UTF-8 is more likely but not guaranteed either" ? For now using a pascal-string seems the best solution but it should be warned that get_attrs[a].val_ptr is NOT a c-string and therefore can not be used with the printf-family (i guess the situation will become more clear in future) I have no clue why you need that, using c-strings makes life more easy in the last minute a sprintf(buf,"%u") will save the day ;) >>> In the following case: >>> if get_attrs[a].len == 0 >>> then get_attrs[a].val_ptr == NULL >>> >>> The end result should be >>> odi->osdname_len = 0 >>> odi->osdname = kzalloc(1, GFP_KERNEL); >>> >>> while with kstrdup, odi->osdname will end up being NULL >>> as it's input arg "s" is NULL. >>> >> >> and you want the argument to be "" ? >> May i ask why ? kfree() can handle NULL and kprintf() (-family) also. > > It was Boaz' decision at the time to simplify internal tests > like _the_same_or_null in drivers/scsi/osd/osd_uld.c > that doesn't check for NULL > It look clever to add the NULL test here. Noone reviewing the code will understand that. (Rule of least surprise) just my 2 cents, re, wh > Nothing spectacular :) > > Benny > >> >> re, >> wh >> >> >>> Benny >>> re, wh >>> >>> > > -- 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] [SCSI] libosd: check for kzalloc() failure
On Wed, Jan 30, 2013 at 3:00 PM, walter harms wrote: > > > Am 30.01.2013 10:51, schrieb Benny Halevy: >> On Wed, Jan 30, 2013 at 10:57 AM, walter harms wrote: >>> >>> >>> Am 30.01.2013 09:27, schrieb Dan Carpenter: On Wed, Jan 30, 2013 at 09:15:43AM +0100, walter harms wrote: > > > Am 30.01.2013 08:06, schrieb Dan Carpenter: >> There wasn't any error handling for this kzalloc(). >> >> Signed-off-by: Dan Carpenter >> >> diff --git a/drivers/scsi/osd/osd_initiator.c >> b/drivers/scsi/osd/osd_initiator.c >> index c06b8e5..d8293f2 100644 >> --- a/drivers/scsi/osd/osd_initiator.c >> +++ b/drivers/scsi/osd/osd_initiator.c >> @@ -144,6 +144,10 @@ static int _osd_get_print_system_info(struct >> osd_dev *od, >> odi->osdname_len = get_attrs[a].len; >> /* Avoid NULL for memcmp optimization 0-length is good enough */ >> odi->osdname = kzalloc(odi->osdname_len + 1, GFP_KERNEL); >> + if (!odi->osdname) { >> + ret = -ENOMEM; >> + goto out; >> + } >> if (odi->osdname_len) >> memcpy(odi->osdname, get_attrs[a].val_ptr, odi->osdname_len); >> OSD_INFO("OSD_NAME [%s]\n", odi->osdname); >> -- > > this looks like strdup() ? > Maybe? It's a funny thing going on with the NUL terminator and I don't understand what the comment is about. It appears that normally "get_attrs[a].val_ptr" is a NUL terminated string but "get_attrs[a].len" does not count the terminator. Odd. >>> i have no clue what the programmer was thinking. if i read this correct >>> osdname is u8 *osdname; so a simple strdup() or strndup() would be ok >>> the comment seems to indicate that get_attrs[a].val_ptr could be NULL >>> but where is the check ? >>> Perhaps they are not using ascii here ? then a memdup(get_attrs[a].len) >>> would be better. >> >> There are subtle differences between kstrdup or kmemdup and this >> implementation. >> >> kmemdup is problematic as get_attrs[a].val_ptr does not contain the >> NUL terminator > > ok, i understand - but can we assume that we are talking ascii here ? > No, it can be anything. UTF-8 is more likely but not guaranteed either. >> In the following case: >> if get_attrs[a].len == 0 >> then get_attrs[a].val_ptr == NULL >> >> The end result should be >> odi->osdname_len = 0 >> odi->osdname = kzalloc(1, GFP_KERNEL); >> >> while with kstrdup, odi->osdname will end up being NULL >> as it's input arg "s" is NULL. >> > > and you want the argument to be "" ? > May i ask why ? kfree() can handle NULL and kprintf() (-family) also. It was Boaz' decision at the time to simplify internal tests like _the_same_or_null in drivers/scsi/osd/osd_uld.c that doesn't check for NULL Nothing spectacular :) Benny > > re, > wh > > >> Benny >> >>> >>> re, >>> wh >>> >>> >> >> -- 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: storvsc loops with No Sense messages
On Wed, Jan 30, Olaf Hering wrote: > Is there a way to not use WRITE_SAME at all? While browsing the code its > not clear if there is a conditional for this command. It seems scsi_device->no_write_same may avoid this command, I will test this patch: # Subject: [PATCH] scsi: storvsc: avoid usage of WRITE_SAME Set scsi_device->no_write_same because the host does not support it. Also blacklist WRITE_SAME to avoid (and log) accident usage. Signed-off-by: Olaf Hering --- drivers/scsi/storvsc_drv.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c index 0144078..21f788a 100644 --- a/drivers/scsi/storvsc_drv.c +++ b/drivers/scsi/storvsc_drv.c @@ -1155,6 +1155,8 @@ static int storvsc_device_configure(struct scsi_device *sdevice) blk_queue_bounce_limit(sdevice->request_queue, BLK_BOUNCE_ANY); + sdevice->no_write_same = 1; + return 0; } @@ -1241,6 +1243,7 @@ static bool storvsc_scsi_cmd_ok(struct scsi_cmnd *scmnd) * smartd sends this command and the host does not handle * this. So, don't send it. */ + case WRITE_SAME: case SET_WINDOW: scmnd->result = ILLEGAL_REQUEST << 16; allowed = false; -- 1.8.1.1 -- 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] [SCSI] libosd: check for kzalloc() failure
Am 30.01.2013 10:51, schrieb Benny Halevy: > On Wed, Jan 30, 2013 at 10:57 AM, walter harms wrote: >> >> >> Am 30.01.2013 09:27, schrieb Dan Carpenter: >>> On Wed, Jan 30, 2013 at 09:15:43AM +0100, walter harms wrote: Am 30.01.2013 08:06, schrieb Dan Carpenter: > There wasn't any error handling for this kzalloc(). > > Signed-off-by: Dan Carpenter > > diff --git a/drivers/scsi/osd/osd_initiator.c > b/drivers/scsi/osd/osd_initiator.c > index c06b8e5..d8293f2 100644 > --- a/drivers/scsi/osd/osd_initiator.c > +++ b/drivers/scsi/osd/osd_initiator.c > @@ -144,6 +144,10 @@ static int _osd_get_print_system_info(struct osd_dev > *od, > odi->osdname_len = get_attrs[a].len; > /* Avoid NULL for memcmp optimization 0-length is good enough */ > odi->osdname = kzalloc(odi->osdname_len + 1, GFP_KERNEL); > + if (!odi->osdname) { > + ret = -ENOMEM; > + goto out; > + } > if (odi->osdname_len) > memcpy(odi->osdname, get_attrs[a].val_ptr, odi->osdname_len); > OSD_INFO("OSD_NAME [%s]\n", odi->osdname); > -- this looks like strdup() ? >>> >>> Maybe? It's a funny thing going on with the NUL terminator and I >>> don't understand what the comment is about. >>> >>> It appears that normally "get_attrs[a].val_ptr" is a NUL terminated >>> string but "get_attrs[a].len" does not count the terminator. >>> >>> Odd. >>> >> i have no clue what the programmer was thinking. if i read this correct >> osdname is u8 *osdname; so a simple strdup() or strndup() would be ok >> the comment seems to indicate that get_attrs[a].val_ptr could be NULL >> but where is the check ? >> Perhaps they are not using ascii here ? then a memdup(get_attrs[a].len) >> would be better. > > There are subtle differences between kstrdup or kmemdup and this > implementation. > > kmemdup is problematic as get_attrs[a].val_ptr does not contain the > NUL terminator ok, i understand - but can we assume that we are talking ascii here ? > In the following case: > if get_attrs[a].len == 0 > then get_attrs[a].val_ptr == NULL > > The end result should be > odi->osdname_len = 0 > odi->osdname = kzalloc(1, GFP_KERNEL); > > while with kstrdup, odi->osdname will end up being NULL > as it's input arg "s" is NULL. > and you want the argument to be "" ? May i ask why ? kfree() can handle NULL and kprintf() (-family) also. re, wh > Benny > >> >> re, >> wh >> >> > > -- 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] [SCSI] qla4xxx: don't free NULL dma pool
-Original Message- From: Dan Carpenter Date: Wednesday 30 January 2013 12:37 PM To: Ravi Anand Cc: Vikas , Dept-Eng iSCSI Driver , "James E.J. Bottomley" , scsi , "kernel-janit...@vger.kernel.org" Subject: [patch] [SCSI] qla4xxx: don't free NULL dma pool >The error path calls dma_pool_free() on this path but "chap_table" is >NULL and "chap_dma" is uninitialized. It's cleaner to just return >directly. > >Signed-off-by: Dan Carpenter > >diff --git a/drivers/scsi/qla4xxx/ql4_mbx.c >b/drivers/scsi/qla4xxx/ql4_mbx.c >index 1c57c22..887e409 100644 >--- a/drivers/scsi/qla4xxx/ql4_mbx.c >+++ b/drivers/scsi/qla4xxx/ql4_mbx.c >@@ -1404,10 +1404,8 @@ int qla4xxx_get_chap(struct scsi_qla_host *ha, >char *username, char *password, > dma_addr_t chap_dma; > > chap_table = dma_pool_alloc(ha->chap_dma_pool, GFP_KERNEL, &chap_dma); >- if (chap_table == NULL) { >- ret = -ENOMEM; >- goto exit_get_chap; >- } >+ if (chap_table == NULL) >+ return -ENOMEM; > > chap_size = sizeof(struct ql4_chap_table); > memset(chap_table, 0, chap_size); Thanks for a fix. Acked-by: Vikas Chaudhary This message and any attached documents contain information from QLogic Corporation or its wholly-owned subsidiaries that may be confidential. If you are not the intended recipient, you may not read, copy, distribute, or use this information. If you have received this transmission in error, please notify the sender immediately by reply e-mail and then delete this message. -- 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] [SCSI] libosd: check for kzalloc() failure
On Wed, Jan 30, 2013 at 9:06 AM, Dan Carpenter wrote: > There wasn't any error handling for this kzalloc(). > > Signed-off-by: Dan Carpenter > > diff --git a/drivers/scsi/osd/osd_initiator.c > b/drivers/scsi/osd/osd_initiator.c > index c06b8e5..d8293f2 100644 > --- a/drivers/scsi/osd/osd_initiator.c > +++ b/drivers/scsi/osd/osd_initiator.c > @@ -144,6 +144,10 @@ static int _osd_get_print_system_info(struct osd_dev *od, > odi->osdname_len = get_attrs[a].len; > /* Avoid NULL for memcmp optimization 0-length is good enough */ > odi->osdname = kzalloc(odi->osdname_len + 1, GFP_KERNEL); > + if (!odi->osdname) { > + ret = -ENOMEM; > + goto out; > + } good catch! Benny > if (odi->osdname_len) > memcpy(odi->osdname, get_attrs[a].val_ptr, odi->osdname_len); > OSD_INFO("OSD_NAME [%s]\n", odi->osdname); -- 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: storvsc loops with No Sense messages
On Tue, Jan 29, Olaf Hering wrote: > On Tue, Jan 29, KY Srinivasan wrote: > > > Was the installation done on a vhd or VHDX. I checked with Hyper-V > > guys and they say they never supported WRITE_SAME. I am wondering how > > it worked on ws2008. > > Its a vhdx image, both fixed and dynamically size have the same issue. > I will see what happens with ws2008, at least the install proceeds > without issues. I see that WRITE_SAME is also used when running on ws2008, but appearently it does not cause any issues. This triggers only when the default ext4 is used, not when I force ext3 for the root filesystem. So to me it looks like ws2012 has issues with WRITE_SAME handling. Is there a way to not use WRITE_SAME at all? While browsing the code its not clear if there is a conditional for this command. Olaf -- 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] [SCSI] libosd: check for kzalloc() failure
On Wed, Jan 30, 2013 at 10:57 AM, walter harms wrote: > > > Am 30.01.2013 09:27, schrieb Dan Carpenter: >> On Wed, Jan 30, 2013 at 09:15:43AM +0100, walter harms wrote: >>> >>> >>> Am 30.01.2013 08:06, schrieb Dan Carpenter: There wasn't any error handling for this kzalloc(). Signed-off-by: Dan Carpenter diff --git a/drivers/scsi/osd/osd_initiator.c b/drivers/scsi/osd/osd_initiator.c index c06b8e5..d8293f2 100644 --- a/drivers/scsi/osd/osd_initiator.c +++ b/drivers/scsi/osd/osd_initiator.c @@ -144,6 +144,10 @@ static int _osd_get_print_system_info(struct osd_dev *od, odi->osdname_len = get_attrs[a].len; /* Avoid NULL for memcmp optimization 0-length is good enough */ odi->osdname = kzalloc(odi->osdname_len + 1, GFP_KERNEL); + if (!odi->osdname) { + ret = -ENOMEM; + goto out; + } if (odi->osdname_len) memcpy(odi->osdname, get_attrs[a].val_ptr, odi->osdname_len); OSD_INFO("OSD_NAME [%s]\n", odi->osdname); -- >>> >>> this looks like strdup() ? >>> >> >> Maybe? It's a funny thing going on with the NUL terminator and I >> don't understand what the comment is about. >> >> It appears that normally "get_attrs[a].val_ptr" is a NUL terminated >> string but "get_attrs[a].len" does not count the terminator. >> >> Odd. >> > i have no clue what the programmer was thinking. if i read this correct > osdname is u8 *osdname; so a simple strdup() or strndup() would be ok > the comment seems to indicate that get_attrs[a].val_ptr could be NULL > but where is the check ? > Perhaps they are not using ascii here ? then a memdup(get_attrs[a].len) > would be better. There are subtle differences between kstrdup or kmemdup and this implementation. kmemdup is problematic as get_attrs[a].val_ptr does not contain the NUL terminator In the following case: if get_attrs[a].len == 0 then get_attrs[a].val_ptr == NULL The end result should be odi->osdname_len = 0 odi->osdname = kzalloc(1, GFP_KERNEL); while with kstrdup, odi->osdname will end up being NULL as it's input arg "s" is NULL. Benny > > re, > wh > > -- 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 v8 2/4] block: add runtime pm helpers
From: Lin Ming Add runtime pm helper functions: void blk_pm_runtime_init(struct request_queue *q, struct device *dev) - Initialization function for drivers to call. int blk_pre_runtime_suspend(struct request_queue *q) - If any requests are in the queue, mark last busy and return -EBUSY. Otherwise set q->rpm_status to RPM_SUSPENDING and return 0. void blk_post_runtime_suspend(struct request_queue *q, int err) - If the suspend succeeded then set q->rpm_status to RPM_SUSPENDED. Otherwise set it to RPM_ACTIVE. void blk_pre_runtime_resume(struct request_queue *q) - Set q->rpm_status to RPM_RESUMING. void blk_post_runtime_resume(struct request_queue *q, int err) - If the resume succeeded then set q->rpm_status to RPM_ACTIVE and call __blk_run_queue, then mark last busy and autosuspend. Otherwise set q->rpm_status to RPM_SUSPENDED. Signed-off-by: Lin Ming Signed-off-by: Aaron Lu --- block/blk-core.c | 143 + include/linux/blkdev.h | 27 ++ 2 files changed, 170 insertions(+) diff --git a/block/blk-core.c b/block/blk-core.c index 66d3168..1ec09f6 100644 --- a/block/blk-core.c +++ b/block/blk-core.c @@ -30,6 +30,7 @@ #include #include #include +#include #define CREATE_TRACE_POINTS #include @@ -3043,6 +3044,148 @@ void blk_finish_plug(struct blk_plug *plug) } EXPORT_SYMBOL(blk_finish_plug); +#ifdef CONFIG_PM_RUNTIME +/** + * blk_pm_runtime_init - Block layer runtime PM initialization routine + * @q: the queue of the device + * @dev: the device the queue belongs to + * + * Description: + *Initialize runtime-PM-related fields for @q and start auto suspend for + *@dev. Drivers that want to take advantage of request-based runtime PM + *should call this function after @dev has been initialized, and its + *request queue @q has been allocated, and runtime PM for it can not happen + *yet(either due to disabled/forbidden or its usage_count > 0). In most + *cases, driver should call this function before any I/O has taken place. + * + *This function takes care of setting up using auto suspend for the device, + *the autosuspend delay is set to -1 to make runtime suspend impossible + *until an updated value is either set by user or by driver. Drivers do + *not need to touch other autosuspend settings. + * + *The block layer runtime PM is request based, so only works for drivers + *that use request as their IO unit instead of those directly use bio's. + */ +void blk_pm_runtime_init(struct request_queue *q, struct device *dev) +{ + q->dev = dev; + q->rpm_status = RPM_ACTIVE; + pm_runtime_set_autosuspend_delay(q->dev, -1); + pm_runtime_use_autosuspend(q->dev); + pm_runtime_mark_last_busy(q->dev); + pm_runtime_autosuspend(q->dev); +} +EXPORT_SYMBOL(blk_pm_runtime_init); + +/** + * blk_pre_runtime_suspend - Pre runtime suspend check + * @q: the queue of the device + * + * Description: + *This function will check if runtime suspend is allowed for the device + *by examining if there are any requests pending in the queue. If there + *are requests pending, the device can not be runtime suspended; otherwise, + *the queue's status will be updated to SUSPENDING and the driver can + *proceed to suspend the device. + * + *For the not allowed case, we mark last busy for the device so that + *runtime PM core will try to autosuspend it some time later. + * + *This function should be called near the start of the device's + *runtime_suspend callback. + * + * Return: + *0- OK to runtime suspend the device + *-EBUSY - Device should not be runtime suspended + */ +int blk_pre_runtime_suspend(struct request_queue *q) +{ + int ret = 0; + + spin_lock_irq(q->queue_lock); + if (q->nr_pending) { + ret = -EBUSY; + pm_runtime_mark_last_busy(q->dev); + } else { + q->rpm_status = RPM_SUSPENDING; + } + spin_unlock_irq(q->queue_lock); + return ret; +} +EXPORT_SYMBOL(blk_pre_runtime_suspend); + +/** + * blk_post_runtime_suspend - Post runtime suspend processing + * @q: the queue of the device + * @err: return value of the device's runtime_suspend function + * + * Description: + *Update the queue's runtime status according to the return value of the + *device's runtime suspend function. + * + *This function should be called near the end of the device's + *runtime_suspend callback. + */ +void blk_post_runtime_suspend(struct request_queue *q, int err) +{ + spin_lock_irq(q->queue_lock); + if (!err) + q->rpm_status = RPM_SUSPENDED; + else + q->rpm_status = RPM_ACTIVE; + spin_unlock_irq(q->queue_lock); +} +EXPORT_SYMBOL(blk_post_runtime_suspend); + +/** + * blk_pre_runtime_resume - Pre runtime resume processing + * @q: the queue of the device + * + * Description:
[PATCH v8 3/4] block: implement runtime pm strategy
From: Lin Ming When a request is added: If device is suspended or is suspending and the request is not a PM request, resume the device. When the last request finishes: Call pm_runtime_mark_last_busy(). When pick a request: If device is resuming/suspending, then only PM request is allowed to go. Signed-off-by: Lin Ming Signed-off-by: Aaron Lu --- block/blk-core.c | 39 +++ block/elevator.c | 26 ++ 2 files changed, 65 insertions(+) diff --git a/block/blk-core.c b/block/blk-core.c index 1ec09f6..82cf678 100644 --- a/block/blk-core.c +++ b/block/blk-core.c @@ -1264,6 +1264,16 @@ void part_round_stats(int cpu, struct hd_struct *part) } EXPORT_SYMBOL_GPL(part_round_stats); +#ifdef CONFIG_PM_RUNTIME +static void blk_pm_put_request(struct request *rq) +{ + if (rq->q->dev && !(rq->cmd_flags & REQ_PM) && !--rq->q->nr_pending) + pm_runtime_mark_last_busy(rq->q->dev); +} +#else +static inline void blk_pm_put_request(struct request *rq) {} +#endif + /* * queue lock must be held */ @@ -1274,6 +1284,8 @@ void __blk_put_request(struct request_queue *q, struct request *req) if (unlikely(--req->ref_count)) return; + blk_pm_put_request(req); + elv_completed_request(q, req); /* this is a bio leak */ @@ -2051,6 +2063,28 @@ static void blk_account_io_done(struct request *req) } } +#ifdef CONFIG_PM_RUNTIME +/* + * Don't process normal requests when queue is suspended + * or in the process of suspending/resuming + */ +static struct request *blk_pm_peek_request(struct request_queue *q, + struct request *rq) +{ + if (q->dev && (q->rpm_status == RPM_SUSPENDED || + (q->rpm_status != RPM_ACTIVE && !(rq->cmd_flags & REQ_PM + return NULL; + else + return rq; +} +#else +static inline struct request *blk_pm_peek_request(struct request_queue *q, + struct request *rq) +{ + return rq; +} +#endif + /** * blk_peek_request - peek at the top of a request queue * @q: request queue to peek at @@ -2073,6 +2107,11 @@ struct request *blk_peek_request(struct request_queue *q) int ret; while ((rq = __elv_next_request(q)) != NULL) { + + rq = blk_pm_peek_request(q, rq); + if (!rq) + break; + if (!(rq->cmd_flags & REQ_STARTED)) { /* * This is the first time the device driver diff --git a/block/elevator.c b/block/elevator.c index 11683bb..29c5c7e 100644 --- a/block/elevator.c +++ b/block/elevator.c @@ -34,6 +34,7 @@ #include #include #include +#include #include @@ -515,6 +516,27 @@ void elv_bio_merged(struct request_queue *q, struct request *rq, e->type->ops.elevator_bio_merged_fn(q, rq, bio); } +#ifdef CONFIG_PM_RUNTIME +static void blk_pm_requeue_request(struct request *rq) +{ + if (rq->q->dev && !(rq->cmd_flags & REQ_PM)) + rq->q->nr_pending--; +} + +static void blk_pm_add_request(struct request_queue *q, struct request *rq) +{ + if (q->dev && !(rq->cmd_flags & REQ_PM) && q->nr_pending++ == 0 && + (q->rpm_status == RPM_SUSPENDED || q->rpm_status == RPM_SUSPENDING)) + pm_request_resume(q->dev); +} +#else +static inline void blk_pm_requeue_request(struct request *rq) {} +static inline void blk_pm_add_request(struct request_queue *q, + struct request *rq) +{ +} +#endif + void elv_requeue_request(struct request_queue *q, struct request *rq) { /* @@ -529,6 +551,8 @@ void elv_requeue_request(struct request_queue *q, struct request *rq) rq->cmd_flags &= ~REQ_STARTED; + blk_pm_requeue_request(rq); + __elv_add_request(q, rq, ELEVATOR_INSERT_REQUEUE); } @@ -551,6 +575,8 @@ void __elv_add_request(struct request_queue *q, struct request *rq, int where) { trace_block_rq_insert(q, rq); + blk_pm_add_request(q, rq); + rq->q = q; if (rq->cmd_flags & REQ_SOFTBARRIER) { -- 1.8.1 -- 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 v8 4/4] sd: change to auto suspend mode
From: Lin Ming Uses block layer runtime pm helper functions in scsi_runtime_suspend/resume for devices that take advantage of it. Remove scsi_autopm_* from sd open/release path and check_events path. Signed-off-by: Lin Ming Signed-off-by: Aaron Lu --- drivers/scsi/scsi_pm.c | 79 ++ drivers/scsi/sd.c | 13 + 2 files changed, 68 insertions(+), 24 deletions(-) diff --git a/drivers/scsi/scsi_pm.c b/drivers/scsi/scsi_pm.c index 8f6b12c..6ce00c5 100644 --- a/drivers/scsi/scsi_pm.c +++ b/drivers/scsi/scsi_pm.c @@ -144,18 +144,44 @@ static int scsi_bus_restore(struct device *dev) #ifdef CONFIG_PM_RUNTIME +static int scsi_blk_runtime_suspend(struct device *dev) +{ + struct scsi_device *sdev = to_scsi_device(dev); + int err; + + err = blk_pre_runtime_suspend(sdev->request_queue); + if (err) + return err; + err = pm_generic_runtime_suspend(dev); + blk_post_runtime_suspend(sdev->request_queue, err); + + return err; +} + +static int scsi_dev_runtime_suspend(struct device *dev) +{ + const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; + int err; + + err = scsi_dev_type_suspend(dev, pm ? pm->runtime_suspend : NULL); + if (err == -EAGAIN) + pm_schedule_suspend(dev, jiffies_to_msecs( + round_jiffies_up_relative(HZ/10))); + + return err; +} + static int scsi_runtime_suspend(struct device *dev) { int err = 0; - const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; dev_dbg(dev, "scsi_runtime_suspend\n"); if (scsi_is_sdev_device(dev)) { - err = scsi_dev_type_suspend(dev, - pm ? pm->runtime_suspend : NULL); - if (err == -EAGAIN) - pm_schedule_suspend(dev, jiffies_to_msecs( - round_jiffies_up_relative(HZ/10))); + struct scsi_device *sdev = to_scsi_device(dev); + if (sdev->request_queue->dev) + err = scsi_blk_runtime_suspend(dev); + else + err = scsi_dev_runtime_suspend(dev); } /* Insert hooks here for targets, hosts, and transport classes */ @@ -163,14 +189,36 @@ static int scsi_runtime_suspend(struct device *dev) return err; } +static int scsi_blk_runtime_resume(struct device *dev) +{ + struct scsi_device *sdev = to_scsi_device(dev); + int err; + + blk_pre_runtime_resume(sdev->request_queue); + err = pm_generic_runtime_resume(dev); + blk_post_runtime_resume(sdev->request_queue, err); + + return err; +} + +static int scsi_dev_runtime_resume(struct device *dev) +{ + const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; + return scsi_dev_type_resume(dev, pm ? pm->runtime_resume : NULL); +} + static int scsi_runtime_resume(struct device *dev) { int err = 0; - const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; dev_dbg(dev, "scsi_runtime_resume\n"); - if (scsi_is_sdev_device(dev)) - err = scsi_dev_type_resume(dev, pm ? pm->runtime_resume : NULL); + if (scsi_is_sdev_device(dev)) { + struct scsi_device *sdev = to_scsi_device(dev); + if (sdev->request_queue->dev) + err = scsi_blk_runtime_resume(dev); + else + err = scsi_dev_runtime_resume(dev); + } /* Insert hooks here for targets, hosts, and transport classes */ @@ -185,10 +233,17 @@ static int scsi_runtime_idle(struct device *dev) /* Insert hooks here for targets, hosts, and transport classes */ - if (scsi_is_sdev_device(dev)) - err = pm_schedule_suspend(dev, 100); - else + if (scsi_is_sdev_device(dev)) { + struct scsi_device *sdev = to_scsi_device(dev); + if (sdev->request_queue->dev) { + pm_runtime_mark_last_busy(dev); + err = pm_runtime_autosuspend(dev); + } else { + err = pm_schedule_suspend(dev, 100); + } + } else { err = pm_runtime_suspend(dev); + } return err; } diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index 698923f..bf04dbf 100644 --- a/drivers/scsi/sd.c +++ b/drivers/scsi/sd.c @@ -1121,10 +1121,6 @@ static int sd_open(struct block_device *bdev, fmode_t mode) sdev = sdkp->device; - retval = scsi_autopm_get_device(sdev); - if (retval) - goto error_autopm; - /* * If the device is in error recovery, wait until it is done. * If the device is offline, then disallow any access to it. @@ -1169,8 +1165,6 @@ static int sd_open(struct block_device *bdev, fmode_t mode) return 0;
[PATCH v8 0/4] block layer runtime pm
In August 2010, Jens and Alan discussed about "Runtime PM and the block layer". http://marc.info/?t=12825910841&r=1&w=2 And then Alan has given a detailed implementation guide: http://marc.info/?l=linux-scsi&m=133727953625963&w=2 To test: # ls -l /sys/block/sda /sys/devices/pci:00/:00:1f.2/ata1/host0/target0:0:0/0:0:0:0/block/sda # echo 1 > /sys/devices/pci:00/:00:1f.2/ata1/host0/target0:0:0/0:0:0:0/power/autosuspend_delay_ms # echo auto > /sys/devices/pci:00/:00:1f.2/ata1/host0/target0:0:0/0:0:0:0/power/control Then you'll see sda is suspended after 10secs idle. [ 1767.680192] sd 2:0:0:0: [sda] Synchronizing SCSI cache [ 1767.680317] sd 2:0:0:0: [sda] Stopping disk And if you do some IO, it will resume immediately. [ 1791.052438] sd 2:0:0:0: [sda] Starting disk For test, I often set the autosuspend time to 1 second. If you are using a GUI, the 10 seconds delay may be too long that the disk can not enter runtime suspended state. Note that sd's runtime suspend callback will dump some kernel messages and the syslog daemon will write kernel message to /var/log/messages, making the disk instantly resume after suspended. So for test, the syslog daemon should better be temporarily stopped. A git repo for it, on top of libata/upstream, scsi/for-next and block/for-next: https://github.com/aaronlu/linux.git blockpm v8: - Set default autosuspend delay to -1 to avoid suspend till an updated value is set as suggested by Alan Stern; - Always check the dev field of the queue structure, as it is incorrect and meaningless to do any operation on devices that do not use block layer runtime PM as reminded by Alan Stern; - Update scsi bus level runtime PM callback to take care of scsi devices that use block layer runtime PM and that don't. v7: - Add kernel doc for block layer runtime PM API as suggested by Alan Stern; - Add back check for q->dev, as that serves as a flag if driver is using block layer runtime PM; - Do not auto suspend when last request is finished, as that's a hot path and auto suspend is not a trivial function. Instead, mark last busy in pre_suspend so that runtim PM core will retry suspend some time later to solve the 1st problem demostrated in v6, suggested by Alan Stern. - Move block layer runtime PM strtegy functions to where they are needed instead of in include/linux/blkdev.h as suggested by Alan Stern since clients of block layer do not need to know those functions. v6: Take over from Lin Ming. - Instead of put the device into autosuspend state in blk_post_runtime_suspend, do it when the last request is finished. This can also solve the problem illustrated below: thread Athread B |suspend timer expired | | ... ... |a new request comes in, | ... ... |blk_pm_add_request | ... ... |skip request_resume due to | ... ... |q->status is still RPM_ACTIVE | rpm_suspend | ... ... |scsi_runtime_suspend | ... ... | blk_pre_runtime_suspend | ... ... | return -EBUSY due to nr_pending | ... ... | rpm_suspend done | ... ... | |blk_pm_put_request, mark last busy But no more trigger point, and the device will stay at RPM_ACTIVE state. Run pm_runtime_autosuspend after the last request is finished solved this problem. - Requests which have the REQ_PM flag should not involve nr_pending counting, or we may lose the condition to resume the device: Suppose queue is active and nr_pending is 0. Then a REQ_PM request comes and nr_pending will be increased to 1, but since the request has REQ_PM flag, it will not cause resume. Before it is finished, a normal request comes in, and since nr_pending is 1 now, it will not trigger the resume of the device either. Bug. - Do not quiesce the device in scsi bus level runtime suspend callback. Since the only reason the device is to be runtime suspended is due to no more requests pending for it, quiesce it is pointless. - Remove scsi_autopm_* from sd_check_events as we are request driven. - Call blk_pm_runtime_init in scsi_sysfs_initialize_dev, so that we do not need to check queue's device in blk_pm_add/put_request. - Do not mark last busy and initiate an autosuspend for the device in blk_pm_runtime_init function. - Do not mark last busy and initiate an autosuspend for the device in block_post_runtime_resume, as when the request that triggered the resume finished, the blk_pm_put_request will mark last busy and initiate an autosuspend. v5: - rename scsi_execute_req to scsi_execute_req_flags and wrap scsi_execute_req around it. - add detail function descriptions in patch 2 log - define static helper functions to do runtime pm work on block layer and put the definitions inside
[PATCH v8 1/4] block: add a flag to identify PM request
From: Lin Ming Add a flag REQ_PM to identify the request is PM related, such requests will not change the device request queue's runtime status. It is intended to be used in driver's runtime PM callback, so that driver can perform some IO to the device there with the queue's runtime status unaffected. e.g. in SCSI disk's runtime suspend callback, the disk will be put into stopped power state, and this require sending a command to the device. Such command processing should not change the disk's runtime status. As an example, modify scsi code to use this flag. Signed-off-by: Lin Ming Signed-off-by: Aaron Lu --- drivers/scsi/scsi_lib.c| 9 - drivers/scsi/sd.c | 8 include/linux/blk_types.h | 2 ++ include/scsi/scsi_device.h | 16 4 files changed, 22 insertions(+), 13 deletions(-) diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c index 765398c..23f795f 100644 --- a/drivers/scsi/scsi_lib.c +++ b/drivers/scsi/scsi_lib.c @@ -271,11 +271,10 @@ int scsi_execute(struct scsi_device *sdev, const unsigned char *cmd, } EXPORT_SYMBOL(scsi_execute); - -int scsi_execute_req(struct scsi_device *sdev, const unsigned char *cmd, +int scsi_execute_req_flags(struct scsi_device *sdev, const unsigned char *cmd, int data_direction, void *buffer, unsigned bufflen, struct scsi_sense_hdr *sshdr, int timeout, int retries, -int *resid) +int *resid, int flags) { char *sense = NULL; int result; @@ -286,14 +285,14 @@ int scsi_execute_req(struct scsi_device *sdev, const unsigned char *cmd, return DRIVER_ERROR << 24; } result = scsi_execute(sdev, cmd, data_direction, buffer, bufflen, - sense, timeout, retries, 0, resid); + sense, timeout, retries, flags, resid); if (sshdr) scsi_normalize_sense(sense, SCSI_SENSE_BUFFERSIZE, sshdr); kfree(sense); return result; } -EXPORT_SYMBOL(scsi_execute_req); +EXPORT_SYMBOL(scsi_execute_req_flags); /* * Function:scsi_init_cmd_errh() diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index 7992635..698923f 100644 --- a/drivers/scsi/sd.c +++ b/drivers/scsi/sd.c @@ -1424,8 +1424,8 @@ static int sd_sync_cache(struct scsi_disk *sdkp) * Leave the rest of the command zero to indicate * flush everything. */ - res = scsi_execute_req(sdp, cmd, DMA_NONE, NULL, 0, &sshdr, - SD_FLUSH_TIMEOUT, SD_MAX_RETRIES, NULL); + res = scsi_execute_req_flags(sdp, cmd, DMA_NONE, NULL, 0, &sshdr, + SD_FLUSH_TIMEOUT, SD_MAX_RETRIES, NULL, REQ_PM); if (res == 0) break; } @@ -3021,8 +3021,8 @@ static int sd_start_stop_device(struct scsi_disk *sdkp, int start) if (!scsi_device_online(sdp)) return -ENODEV; - res = scsi_execute_req(sdp, cmd, DMA_NONE, NULL, 0, &sshdr, - SD_TIMEOUT, SD_MAX_RETRIES, NULL); + res = scsi_execute_req_flags(sdp, cmd, DMA_NONE, NULL, 0, &sshdr, + SD_TIMEOUT, SD_MAX_RETRIES, NULL, REQ_PM); if (res) { sd_printk(KERN_WARNING, sdkp, "START_STOP FAILED\n"); sd_print_result(sdkp, res); diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h index cdf1119..fcc1ce2 100644 --- a/include/linux/blk_types.h +++ b/include/linux/blk_types.h @@ -175,6 +175,7 @@ enum rq_flag_bits { __REQ_IO_STAT, /* account I/O stat */ __REQ_MIXED_MERGE, /* merge of different types, fail separately */ __REQ_KERNEL, /* direct IO to kernel pages */ + __REQ_PM, /* runtime pm request */ __REQ_NR_BITS, /* stops here */ }; @@ -223,5 +224,6 @@ enum rq_flag_bits { #define REQ_MIXED_MERGE(1 << __REQ_MIXED_MERGE) #define REQ_SECURE (1 << __REQ_SECURE) #define REQ_KERNEL (1 << __REQ_KERNEL) +#define REQ_PM (1 << __REQ_PM) #endif /* __LINUX_BLK_TYPES_H */ diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h index a7f9cba..cc64587 100644 --- a/include/scsi/scsi_device.h +++ b/include/scsi/scsi_device.h @@ -394,10 +394,18 @@ extern int scsi_execute(struct scsi_device *sdev, const unsigned char *cmd, int data_direction, void *buffer, unsigned bufflen, unsigned char *sense, int timeout, int retries, int flag, int *resid); -extern int scsi_execute_req(struct scsi_device *sdev, const unsigned char *cmd, - int data_direction, void *buffer, unsigned bufflen, - struct scsi_sense_hdr *, int timeout, int retries, -
Re: [PATCH 0/3] target: Fix zero-length regressions in v3.8-rc1 code
Il 29/01/2013 23:26, Nicholas A. Bellinger ha scritto: > From: Nicholas Bellinger > > Hi folks, > > The following are a handful of zero-length CDB regression bugfixes to address > breakage introduced by the recent sense_reason_t conversion in v3.8-rc1 code, > which incorrectly assumed CHECK_CONDITION status (in all CDB emulation cases) > when NULL was returned by transport_kmap_data_sg(). > > Please review, as I'd like to get these into v3.8-rc6. > > Thank you, > > --nab > > Nicholas Bellinger (3): > target: Fix zero-length INQUIRY additional sense code regression > target: Fix zero-length MODE_SENSE regression > target: Fix zero-length READ_CAPACITY_16 regression > > drivers/target/target_core_sbc.c | 18 +++ > drivers/target/target_core_spc.c | 44 + > 2 files changed, 19 insertions(+), 43 deletions(-) > Looks good, but given the mess I made in my own zero-length patches, don't really count it as a Reviewed-by. 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
Re: [patch] [SCSI] libosd: check for kzalloc() failure
Am 30.01.2013 09:27, schrieb Dan Carpenter: > On Wed, Jan 30, 2013 at 09:15:43AM +0100, walter harms wrote: >> >> >> Am 30.01.2013 08:06, schrieb Dan Carpenter: >>> There wasn't any error handling for this kzalloc(). >>> >>> Signed-off-by: Dan Carpenter >>> >>> diff --git a/drivers/scsi/osd/osd_initiator.c >>> b/drivers/scsi/osd/osd_initiator.c >>> index c06b8e5..d8293f2 100644 >>> --- a/drivers/scsi/osd/osd_initiator.c >>> +++ b/drivers/scsi/osd/osd_initiator.c >>> @@ -144,6 +144,10 @@ static int _osd_get_print_system_info(struct osd_dev >>> *od, >>> odi->osdname_len = get_attrs[a].len; >>> /* Avoid NULL for memcmp optimization 0-length is good enough */ >>> odi->osdname = kzalloc(odi->osdname_len + 1, GFP_KERNEL); >>> + if (!odi->osdname) { >>> + ret = -ENOMEM; >>> + goto out; >>> + } >>> if (odi->osdname_len) >>> memcpy(odi->osdname, get_attrs[a].val_ptr, odi->osdname_len); >>> OSD_INFO("OSD_NAME [%s]\n", odi->osdname); >>> -- >> >> this looks like strdup() ? >> > > Maybe? It's a funny thing going on with the NUL terminator and I > don't understand what the comment is about. > > It appears that normally "get_attrs[a].val_ptr" is a NUL terminated > string but "get_attrs[a].len" does not count the terminator. > > Odd. > i have no clue what the programmer was thinking. if i read this correct osdname is u8 *osdname; so a simple strdup() or strndup() would be ok the comment seems to indicate that get_attrs[a].val_ptr could be NULL but where is the check ? Perhaps they are not using ascii here ? then a memdup(get_attrs[a].len) would be better. re, wh -- 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 5/6] qla2xxx: Prevent enabling target mode for unsupported HBAs.
From: Arun Easi Signed-off-by: Arun Easi Signed-off-by: Saurav Kashyap --- drivers/scsi/qla2xxx/qla_dbg.c|2 +- drivers/scsi/qla2xxx/qla_def.h|1 + drivers/scsi/qla2xxx/qla_target.c |6 ++ 3 files changed, 8 insertions(+), 1 deletions(-) diff --git a/drivers/scsi/qla2xxx/qla_dbg.c b/drivers/scsi/qla2xxx/qla_dbg.c index e690d05..f81e938 100644 --- a/drivers/scsi/qla2xxx/qla_dbg.c +++ b/drivers/scsi/qla2xxx/qla_dbg.c @@ -38,7 +38,7 @@ * | ISP82XX Specific | 0xb084 | 0xb002,0xb024 | * | MultiQ | 0xc00c | | * | Misc | 0xd010 | | - * | Target Mode | 0xe06f || + * | Target Mode | 0xe070 || * | Target Mode Management | 0xf072 || * | Target Mode Task Management | 0x1000b || * -- diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h index b5fc478..5c42c91 100644 --- a/drivers/scsi/qla2xxx/qla_def.h +++ b/drivers/scsi/qla2xxx/qla_def.h @@ -2798,6 +2798,7 @@ struct qla_hw_data { #define IS_PI_SPLIT_DET_CAPABLE(ha)(IS_PI_SPLIT_DET_CAPABLE_HBA(ha) && \ (((ha)->fw_attributes_h << 16 | (ha)->fw_attributes) & BIT_22)) #define IS_ATIO_MSIX_CAPABLE(ha) (IS_QLA83XX(ha)) +#define IS_TGT_MODE_CAPABLE(ha)(ha->tgt.atio_q_length) /* HBA serial number */ uint8_t serial0; diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c index cb8ea44..61b5d8c 100644 --- a/drivers/scsi/qla2xxx/qla_target.c +++ b/drivers/scsi/qla2xxx/qla_target.c @@ -4306,6 +4306,12 @@ int qlt_add_target(struct qla_hw_data *ha, struct scsi_qla_host *base_vha) if (!QLA_TGT_MODE_ENABLED()) return 0; + if (!IS_TGT_MODE_CAPABLE(ha)) { + ql_log(ql_log_warn, base_vha, 0xe070, + "This adapter does not support target mode.\n"); + return 0; + } + ql_dbg(ql_dbg_tgt, base_vha, 0xe03b, "Registering target for host %ld(%p)", base_vha->host_no, ha); -- 1.7.7 -- 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 6/6] qla2xxx: Correction to the message ids.
Signed-off-by: Giridhar Malavali Signed-off-by: Saurav Kashyap --- drivers/scsi/qla2xxx/qla_dbg.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/drivers/scsi/qla2xxx/qla_dbg.c b/drivers/scsi/qla2xxx/qla_dbg.c index f81e938..ba2d7a8 100644 --- a/drivers/scsi/qla2xxx/qla_dbg.c +++ b/drivers/scsi/qla2xxx/qla_dbg.c @@ -25,6 +25,7 @@ * | || 0x5047,0x5052 | * | Timer Routines | 0x6011 || * | User Space Interactions | 0x70c3 | 0x7018,0x702e, | + * | || 0x7020,0x7024, | * | || 0x7039,0x7045, | * | || 0x7073-0x7075, | * | || 0x708c,| -- 1.7.7 -- 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 2/6] qla2xxx: Ramp down queue depth for attached SCSI devices when driver resources are low.
From: Chad Dupuis Signed-off-by: Chad Dupuis Signed-off-by: Saurav Kashyap --- drivers/scsi/qla2xxx/qla_dbg.c|2 +- drivers/scsi/qla2xxx/qla_def.h|9 +++ drivers/scsi/qla2xxx/qla_gbl.h|1 + drivers/scsi/qla2xxx/qla_inline.h | 19 +++ drivers/scsi/qla2xxx/qla_isr.c|4 ++ drivers/scsi/qla2xxx/qla_os.c | 107 ++-- 6 files changed, 135 insertions(+), 7 deletions(-) diff --git a/drivers/scsi/qla2xxx/qla_dbg.c b/drivers/scsi/qla2xxx/qla_dbg.c index 2f3e765..f7cdd25 100644 --- a/drivers/scsi/qla2xxx/qla_dbg.c +++ b/drivers/scsi/qla2xxx/qla_dbg.c @@ -17,7 +17,7 @@ * | || 0x113a | * | Device Discovery | 0x2087 | 0x2020-0x2022, | * | || 0x2016 | - * | Queue Command and IO tracing | 0x3030 | 0x3006-0x300b | + * | Queue Command and IO tracing | 0x3031 | 0x3006-0x300b | * | || 0x3027-0x3028 | * | || 0x302d-0x302e | * | DPC Thread | 0x401d | 0x4002,0x4013 | diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h index a84bb8d..06c6271 100644 --- a/drivers/scsi/qla2xxx/qla_def.h +++ b/drivers/scsi/qla2xxx/qla_def.h @@ -2536,6 +2536,7 @@ struct req_que { srb_t **outstanding_cmds; uint32_t current_outstanding_cmd; uint16_t num_outstanding_cmds; +#defineMAX_Q_DEPTH 32 int max_q_depth; }; @@ -3058,6 +3059,12 @@ struct qla_hw_data { struct work_struct idc_state_handler; struct work_struct nic_core_unrecoverable; +#define HOST_QUEUE_RAMPDOWN_INTERVAL (60 * HZ) +#define HOST_QUEUE_RAMPUP_INTERVAL (30 * HZ) + unsigned long host_last_rampdown_time; + unsigned long host_last_rampup_time; + int cfg_lun_q_depth; + struct qlt_hw_data tgt; }; @@ -3117,6 +3124,8 @@ typedef struct scsi_qla_host { #define MPI_RESET_NEEDED 19 /* Initiate MPI FW reset */ #define ISP_QUIESCE_NEEDED 20 /* Driver need some quiescence */ #define SCR_PENDING21 /* SCR in target mode */ +#define HOST_RAMP_DOWN_QUEUE_DEPTH 22 +#define HOST_RAMP_UP_QUEUE_DEPTH 23 uint32_tdevice_flags; #define SWITCH_FOUND BIT_0 diff --git a/drivers/scsi/qla2xxx/qla_gbl.h b/drivers/scsi/qla2xxx/qla_gbl.h index fba0651..1732713 100644 --- a/drivers/scsi/qla2xxx/qla_gbl.h +++ b/drivers/scsi/qla2xxx/qla_gbl.h @@ -97,6 +97,7 @@ extern int qlport_down_retry; extern int ql2xplogiabsentdevice; extern int ql2xloginretrycount; extern int ql2xfdmienable; +extern int ql2xmaxqdepth; extern int ql2xallocfwdump; extern int ql2xextended_error_logging; extern int ql2xiidmaenable; diff --git a/drivers/scsi/qla2xxx/qla_inline.h b/drivers/scsi/qla2xxx/qla_inline.h index c0462c0..deb8618 100644 --- a/drivers/scsi/qla2xxx/qla_inline.h +++ b/drivers/scsi/qla2xxx/qla_inline.h @@ -213,3 +213,22 @@ qla2x00_gid_list_size(struct qla_hw_data *ha) { return sizeof(struct gid_list_info) * ha->max_fibre_devices; } + +static inline void +qla2x00_do_host_ramp_up(scsi_qla_host_t *vha) +{ + if (vha->hw->cfg_lun_q_depth >= ql2xmaxqdepth) + return; + + /* Wait at least HOST_QUEUE_RAMPDOWN_INTERVAL before ramping up */ + if (time_before(jiffies, (vha->hw->host_last_rampdown_time + + HOST_QUEUE_RAMPDOWN_INTERVAL))) + return; + + /* Wait at least HOST_QUEUE_RAMPUP_INTERVAL between each ramp up */ + if (time_before(jiffies, (vha->hw->host_last_rampup_time + + HOST_QUEUE_RAMPUP_INTERVAL))) + return; + + set_bit(HOST_RAMP_UP_QUEUE_DEPTH, &vha->dpc_flags); +} diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c index 4513073..1b192c8 100644 --- a/drivers/scsi/qla2xxx/qla_isr.c +++ b/drivers/scsi/qla2xxx/qla_isr.c @@ -1934,6 +1934,7 @@ qla2x00_status_entry(scsi_qla_host_t *vha, struct rsp_que *rsp, void *pkt) /* Fast path completion. */ if (comp_status == CS_COMPLETE && scsi_status == 0) { + qla2x00_do_host_ramp_up(vha); qla2x00_process_completed_request(vha, req, handle); return; @@ -2193,6 +2194,9 @@ out: cp->cmnd[8], cp->cmnd[9], scsi_bufflen(cp), rsp_info_len, resid_len, fw_resid_len); + if (!res) + qla2x00_do_host_ramp_up(vha); + if (rsp->status_srb == NULL) sp->done(ha, sp, res); } diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c index 53efffc..da86d38 100644 --- a/drivers/scsi/qla2xxx/qla_os.c +++ b/drivers/scsi/qla2xxx/qla_os.c @@ -111,8 +111,7 @@ MODULE_PARM_DESC(ql2xfdmienable,
[PATCH 1/6] qla2xxx: Determine the number of outstanding commands based on available resources.
From: Chad Dupuis Base the number of outstanding requests the driver will keep track of on the available resources instead of being hard-coded. Signed-off-by: Chad Dupuis Signed-off-by: Saurav Kashyap --- drivers/scsi/qla2xxx/qla_bsg.c|2 +- drivers/scsi/qla2xxx/qla_dbg.c|2 +- drivers/scsi/qla2xxx/qla_def.h| 12 drivers/scsi/qla2xxx/qla_gbl.h|3 ++ drivers/scsi/qla2xxx/qla_init.c | 54 +++- drivers/scsi/qla2xxx/qla_iocb.c | 37 - drivers/scsi/qla2xxx/qla_isr.c| 10 +++--- drivers/scsi/qla2xxx/qla_mbx.c|8 +++--- drivers/scsi/qla2xxx/qla_mid.c|7 - drivers/scsi/qla2xxx/qla_nx.c |2 +- drivers/scsi/qla2xxx/qla_os.c | 11 +-- drivers/scsi/qla2xxx/qla_target.c |4 +- drivers/scsi/qla2xxx/qla_target.h |5 ++- 13 files changed, 110 insertions(+), 47 deletions(-) diff --git a/drivers/scsi/qla2xxx/qla_bsg.c b/drivers/scsi/qla2xxx/qla_bsg.c index 9f34ded..f7cb6a3 100644 --- a/drivers/scsi/qla2xxx/qla_bsg.c +++ b/drivers/scsi/qla2xxx/qla_bsg.c @@ -1950,7 +1950,7 @@ qla24xx_bsg_timeout(struct fc_bsg_job *bsg_job) if (!req) continue; - for (cnt = 1; cnt < MAX_OUTSTANDING_COMMANDS; cnt++) { + for (cnt = 1; cnt < req->num_outstanding_cmds; cnt++) { sp = req->outstanding_cmds[cnt]; if (sp) { if (((sp->type == SRB_CT_CMD) || diff --git a/drivers/scsi/qla2xxx/qla_dbg.c b/drivers/scsi/qla2xxx/qla_dbg.c index 53f9e49..2f3e765 100644 --- a/drivers/scsi/qla2xxx/qla_dbg.c +++ b/drivers/scsi/qla2xxx/qla_dbg.c @@ -11,7 +11,7 @@ * -- * | Level| Last Value Used | Holes | * -- - * | Module Init and Probe| 0x0125 | 0x4b,0xba,0xfa | + * | Module Init and Probe| 0x0126 | 0x4b,0xba,0xfa | * | Mailbox commands | 0x114f | 0x111a-0x111b | * | || 0x112c-0x112e | * | || 0x113a | diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h index 6e7727f..a84bb8d 100644 --- a/drivers/scsi/qla2xxx/qla_def.h +++ b/drivers/scsi/qla2xxx/qla_def.h @@ -253,8 +253,8 @@ #define LOOP_DOWN_TIME 255 /* 240 */ #defineLOOP_DOWN_RESET (LOOP_DOWN_TIME - 30) -/* Maximum outstanding commands in ISP queues (1-65535) */ -#define MAX_OUTSTANDING_COMMANDS 1024 +#define DEFAULT_OUTSTANDING_COMMANDS 1024 +#define MIN_OUTSTANDING_COMMANDS 128 /* ISP request and response entry counts (37-65535) */ #define REQUEST_ENTRY_CNT_2100 128 /* Number of request entries. */ @@ -2533,8 +2533,9 @@ struct req_que { uint16_t qos; uint16_t vp_idx; struct rsp_que *rsp; - srb_t *outstanding_cmds[MAX_OUTSTANDING_COMMANDS]; + srb_t **outstanding_cmds; uint32_t current_outstanding_cmd; + uint16_t num_outstanding_cmds; int max_q_depth; }; @@ -2561,7 +2562,7 @@ struct qlt_hw_data { void *target_lport_ptr; struct qla_tgt_func_tmpl *tgt_ops; struct qla_tgt *qla_tgt; - struct qla_tgt_cmd *cmds[MAX_OUTSTANDING_COMMANDS]; + struct qla_tgt_cmd *cmds[DEFAULT_OUTSTANDING_COMMANDS]; uint16_t current_handle; struct qla_tgt_vp_map *tgt_vp_map; @@ -2887,6 +2888,7 @@ struct qla_hw_data { #define RISC_START_ADDRESS_2300 0x800 #define RISC_START_ADDRESS_2400 0x10 uint16_tfw_xcb_count; + uint16_tfw_iocb_count; uint16_tfw_options[16]; /* slots: 1,2,3,10,11 */ uint8_t fw_seriallink_options[4]; @@ -3248,8 +3250,6 @@ struct qla_tgt_vp_map { #define NVRAM_DELAY() udelay(10) -#define INVALID_HANDLE (MAX_OUTSTANDING_COMMANDS+1) - /* * Flash support definitions */ diff --git a/drivers/scsi/qla2xxx/qla_gbl.h b/drivers/scsi/qla2xxx/qla_gbl.h index 2411d1a..fba0651 100644 --- a/drivers/scsi/qla2xxx/qla_gbl.h +++ b/drivers/scsi/qla2xxx/qla_gbl.h @@ -84,6 +84,9 @@ extern int qla83xx_nic_core_reset(scsi_qla_host_t *); extern void qla83xx_reset_ownership(scsi_qla_host_t *); extern int qla2xxx_mctp_dump(scsi_qla_host_t *); +extern int +qla2x00_alloc_outstanding_cmds(struct qla_hw_data *, struct req_que *); + /* * Global Data in qla_os.c source file. */ diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c index 563eee3..81e8cca 100644 --- a/drivers/scsi/qla2xxx/qla_init.c +++ b/drivers/scsi/qla2xxx/qla_init.c @@ -1559,6 +1559,47 @@ done: return rval; } +int +qla2x00_alloc_outstanding_cmds(struct qla_hw_data *ha, struct
[PATCH 3/6] qla2xxx: Enable target mode support for ISP83xx.
From: Arun Easi Signed-off-by: Arun Easi Signed-off-by: Saurav Kashyap --- drivers/scsi/qla2xxx/qla_dbg.c|2 +- drivers/scsi/qla2xxx/qla_def.h|8 ++ drivers/scsi/qla2xxx/qla_fw.h |3 +- drivers/scsi/qla2xxx/qla_init.c |6 +- drivers/scsi/qla2xxx/qla_isr.c| 16 +++- drivers/scsi/qla2xxx/qla_os.c |4 + drivers/scsi/qla2xxx/qla_target.c | 159 + drivers/scsi/qla2xxx/qla_target.h | 14 +++- 8 files changed, 187 insertions(+), 25 deletions(-) diff --git a/drivers/scsi/qla2xxx/qla_dbg.c b/drivers/scsi/qla2xxx/qla_dbg.c index f7cdd25..e690d05 100644 --- a/drivers/scsi/qla2xxx/qla_dbg.c +++ b/drivers/scsi/qla2xxx/qla_dbg.c @@ -39,7 +39,7 @@ * | MultiQ | 0xc00c | | * | Misc | 0xd010 | | * | Target Mode | 0xe06f || - * | Target Mode Management | 0xf071 || + * | Target Mode Management | 0xf072 || * | Target Mode Task Management | 0x1000b || * -- */ diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h index 06c6271..b5fc478 100644 --- a/drivers/scsi/qla2xxx/qla_def.h +++ b/drivers/scsi/qla2xxx/qla_def.h @@ -537,6 +537,8 @@ struct device_reg_25xxmq { uint32_t req_q_out; uint32_t rsp_q_in; uint32_t rsp_q_out; + uint32_t atio_q_in; + uint32_t atio_q_out; }; typedef union { @@ -563,6 +565,9 @@ typedef union { &(reg)->u.isp2100.mailbox5 : \ &(reg)->u.isp2300.rsp_q_out) +#define ISP_ATIO_Q_IN(vha) (vha->hw->tgt.atio_q_in) +#define ISP_ATIO_Q_OUT(vha) (vha->hw->tgt.atio_q_out) + #define MAILBOX_REG(ha, reg, num) \ (IS_QLA2100(ha) || IS_QLA2200(ha) ? \ (num < 8 ? \ @@ -2559,6 +2564,8 @@ struct qlt_hw_data { struct atio *atio_ring_ptr; /* Current address. */ uint16_t atio_ring_index; /* Current index. */ uint16_t atio_q_length; + uint32_t __iomem *atio_q_in; + uint32_t __iomem *atio_q_out; void *target_lport_ptr; struct qla_tgt_func_tmpl *tgt_ops; @@ -2790,6 +2797,7 @@ struct qla_hw_data { #define IS_PI_SPLIT_DET_CAPABLE_HBA(ha)(IS_QLA83XX(ha)) #define IS_PI_SPLIT_DET_CAPABLE(ha)(IS_PI_SPLIT_DET_CAPABLE_HBA(ha) && \ (((ha)->fw_attributes_h << 16 | (ha)->fw_attributes) & BIT_22)) +#define IS_ATIO_MSIX_CAPABLE(ha) (IS_QLA83XX(ha)) /* HBA serial number */ uint8_t serial0; diff --git a/drivers/scsi/qla2xxx/qla_fw.h b/drivers/scsi/qla2xxx/qla_fw.h index be6d61a..7105d5e 100644 --- a/drivers/scsi/qla2xxx/qla_fw.h +++ b/drivers/scsi/qla2xxx/qla_fw.h @@ -300,7 +300,8 @@ struct init_cb_24xx { uint32_t prio_request_q_address[2]; uint16_t msix; - uint8_t reserved_2[6]; + uint16_t msix_atio; + uint8_t reserved_2[4]; uint16_t atio_q_inpointer; uint16_t atio_q_length; diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c index 81e8cca..a581c85 100644 --- a/drivers/scsi/qla2xxx/qla_init.c +++ b/drivers/scsi/qla2xxx/qla_init.c @@ -1964,7 +1964,7 @@ qla24xx_config_rings(struct scsi_qla_host *vha) WRT_REG_DWORD(®->isp24.rsp_q_in, 0); WRT_REG_DWORD(®->isp24.rsp_q_out, 0); } - qlt_24xx_config_rings(vha, reg); + qlt_24xx_config_rings(vha); /* PCI posting */ RD_REG_DWORD(&ioreg->hccr); @@ -5579,6 +5579,8 @@ qla81xx_nvram_config(scsi_qla_host_t *vha) if (IS_T10_PI_CAPABLE(ha)) nv->frame_payload_size &= ~7; + qlt_81xx_config_nvram_stage1(vha, nv); + /* Reset Initialization control block */ memset(icb, 0, ha->init_cb_size); @@ -5619,6 +5621,8 @@ qla81xx_nvram_config(scsi_qla_host_t *vha) qla2x00_set_model_info(vha, nv->model_name, sizeof(nv->model_name), "QLE8XXX"); + qlt_81xx_config_nvram_stage2(vha, icb); + /* Use alternate WWN? */ if (nv->host_p & __constant_cpu_to_le32(BIT_15)) { memcpy(icb->node_name, nv->alternate_node_name, WWN_SIZE); diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c index 1b192c8..26a3086 100644 --- a/drivers/scsi/qla2xxx/qla_isr.c +++ b/drivers/scsi/qla2xxx/qla_isr.c @@ -13,6 +13,8 @@ #include #include +#include "qla_target.h" + static void qla2x00_mbx_completion(scsi_qla_host_t *, uint16_t); static void qla2x00_process_completed_request(struct scsi_qla_host *, struct req_que *, uint32_t); @@ -2751,6 +2753,12 @@ static struct qla_init_msix_entry qla82xx_msix_entries[2] = { { "qla2xxx (rsp_q)", qla82xx_msix_rsp_q }, }; +static struct qla_init_msix_entry qla83xx_msix_entries[3] = { + { "qla2xxx (default)", qla24xx_ms
[PATCH 4/6] qla2xxx: Allow ISP81xx to create ATIO queues.
From: Arun Easi Signed-off-by: Arun Easi Signed-off-by: Saurav Kashyap --- drivers/scsi/qla2xxx/qla_os.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c index e93408f..2d2afdb 100644 --- a/drivers/scsi/qla2xxx/qla_os.c +++ b/drivers/scsi/qla2xxx/qla_os.c @@ -2425,6 +2425,7 @@ qla2x00_probe_one(struct pci_dev *pdev, const struct pci_device_id *id) ha->mbx_count = MAILBOX_REGISTER_COUNT; req_length = REQUEST_ENTRY_CNT_24XX; rsp_length = RESPONSE_ENTRY_CNT_2300; + ha->tgt.atio_q_length = ATIO_ENTRY_CNT_24XX; ha->max_loop_id = SNS_LAST_LOOP_ID_2300; ha->init_cb_size = sizeof(struct mid_init_cb_81xx); ha->gid_list_info_size = 8; -- 1.7.7 -- 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 0/6] qla2xxx: Patches for scsi "misc" branch.
Hi James, Please apply the following patches to the scsi tree at your earliest convenience for inclusion in the next mainline merge window. Thanks, ~Saurav Arun Easi (3): qla2xxx: Enable target mode support for ISP83xx. qla2xxx: Allow ISP81xx to create ATIO queues. qla2xxx: Prevent enabling target mode for unsupported HBAs. Chad Dupuis (2): qla2xxx: Determine the number of outstanding commands based on available resources. qla2xxx: Ramp down queue depth for attached SCSI devices when driver resources are low. Saurav Kashyap (1): qla2xxx: Correction to the message ids. drivers/scsi/qla2xxx/qla_bsg.c|2 +- drivers/scsi/qla2xxx/qla_dbg.c|9 +- drivers/scsi/qla2xxx/qla_def.h| 30 +-- drivers/scsi/qla2xxx/qla_fw.h |3 +- drivers/scsi/qla2xxx/qla_gbl.h|4 + drivers/scsi/qla2xxx/qla_init.c | 60 +- drivers/scsi/qla2xxx/qla_inline.h | 19 drivers/scsi/qla2xxx/qla_iocb.c | 37 drivers/scsi/qla2xxx/qla_isr.c| 30 +-- drivers/scsi/qla2xxx/qla_mbx.c|8 +- drivers/scsi/qla2xxx/qla_mid.c|7 ++- drivers/scsi/qla2xxx/qla_nx.c |2 +- drivers/scsi/qla2xxx/qla_os.c | 123 +-- drivers/scsi/qla2xxx/qla_target.c | 169 + drivers/scsi/qla2xxx/qla_target.h | 19 +++-- 15 files changed, 442 insertions(+), 80 deletions(-) -- 1.7.7 -- 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] target/iblock: Use backend REQ_FLUSH hint for WriteCacheEnabled status
On Wed, 2013-01-30 at 15:39 +0800, Asias He wrote: > On 01/30/2013 02:57 PM, Nicholas A. Bellinger wrote: > > From: Nicholas Bellinger > > > > This patch allows IBLOCK to check block hints in request_queue->flush_flags > > when reporting current backend device WriteCacheEnabled status to a remote > > SCSI initiator port. > > > > This is done via a se_subsystem_api->get_write_cache() call instead of a > > backend se_device creation time flag, as we expect REQ_FLUSH bits to > > possibly > > change from an underlying blk_queue_flush() by the SCSI disk driver, or > > internal raw struct block_device driver usage. > > > > Also go ahead and update iblock_execute_rw() bio I/O path code to use > > REQ_FLUSH + REQ_FUA hints when determining WRITE_FUA usage, and make SPC > > emulation code use a spc_check_dev_wce() helper to handle both types of > > cases for virtual backend subsystem drivers. > > > > Reported-by: majianpeng > > Cc: majianpeng > > Cc: Christoph Hellwig > > Cc: Jens Axboe > > Cc: James Bottomley > > Signed-off-by: Nicholas Bellinger > > --- > > drivers/target/target_core_device.c |6 ++ > > drivers/target/target_core_iblock.c | 31 +++ > > drivers/target/target_core_spc.c | 21 ++--- > > include/target/target_core_backend.h |1 + > > 4 files changed, 48 insertions(+), 11 deletions(-) > > > > diff --git a/drivers/target/target_core_device.c > > b/drivers/target/target_core_device.c > > index e269510..1d71930 100644 > > --- a/drivers/target/target_core_device.c > > +++ b/drivers/target/target_core_device.c > > @@ -772,6 +772,12 @@ int se_dev_set_emulate_write_cache(struct se_device > > *dev, int flag) > > pr_err("emulate_write_cache not supported for pSCSI\n"); > > return -EINVAL; > > } > > + if (dev->transport->get_write_cache != NULL) { > > one nit: > > if (dev->transport->get_write_cache) { > > ? Dropping extra NULL comparison here. > > > + pr_warn("emulate_write_cache cannot be changed when underlying" > > + " HW reports WriteCacheEnabled, ignoring request\n"); > > + return 0; > > + } > > + > > dev->dev_attrib.emulate_write_cache = flag; > > pr_debug("dev[%p]: SE Device WRITE_CACHE_EMULATION flag: %d\n", > > dev, dev->dev_attrib.emulate_write_cache); > > diff --git a/drivers/target/target_core_iblock.c > > b/drivers/target/target_core_iblock.c > > index b526d23..fc45683 100644 > > --- a/drivers/target/target_core_iblock.c > > +++ b/drivers/target/target_core_iblock.c > > @@ -154,6 +154,7 @@ static int iblock_configure_device(struct se_device > > *dev) > > > > if (blk_queue_nonrot(q)) > > dev->dev_attrib.is_nonrot = 1; > > + > > return 0; > > > > out_free_bioset: > > @@ -654,20 +655,24 @@ iblock_execute_rw(struct se_cmd *cmd) > > u32 sg_num = sgl_nents; > > sector_t block_lba; > > unsigned bio_cnt; > > - int rw; > > + int rw = 0; > > int i; > > > > if (data_direction == DMA_TO_DEVICE) { > > + struct iblock_dev *ib_dev = IBLOCK_DEV(dev); > > + struct request_queue *q = bdev_get_queue(ib_dev->ibd_bd); > > /* > > -* Force data to disk if we pretend to not have a volatile > > -* write cache, or the initiator set the Force Unit Access bit. > > +* Force writethrough using WRITE_FUA if a volatile write cache > > +* is not enabled, or if initiator set the Force Unit Access > > bit. > > */ > > - if (dev->dev_attrib.emulate_write_cache == 0 || > > - (dev->dev_attrib.emulate_fua_write > 0 && > > -(cmd->se_cmd_flags & SCF_FUA))) > > - rw = WRITE_FUA; > > - else > > + if (q->flush_flags & REQ_FUA) { > > + if (cmd->se_cmd_flags & SCF_FUA) > > + rw = WRITE_FUA; > > + else if (!(q->flush_flags & REQ_FLUSH)) > > + rw = WRITE_FUA; > > + } else { > > rw = WRITE; > > + } > > } else { > > rw = READ; > > } > > @@ -774,6 +779,15 @@ iblock_parse_cdb(struct se_cmd *cmd) > > return sbc_parse_cdb(cmd, &iblock_sbc_ops); > > } > > > > +bool iblock_get_write_cache(struct se_device *dev) > > +{ > > + struct iblock_dev *ib_dev = IBLOCK_DEV(dev); > > + struct block_device *bd = ib_dev->ibd_bd; > > + struct request_queue *q = bdev_get_queue(bd); > > + > > + return (q->flush_flags & REQ_FLUSH); > > no need of the (). > Dropping unnecessary () for bit-wise comparison. Thanks Asias! --nab > > +} > > + > > static struct se_subsystem_api iblock_template = { > > .name = "iblock", > > .inquiry_prod = "IBLOCK", > > @@ -790,6 +804,7 @@ static struct se_subsystem_api iblock_template = { > > .show_configfs_dev_params = iblock_sh
Re: [patch] [SCSI] libosd: check for kzalloc() failure
On Wed, Jan 30, 2013 at 09:15:43AM +0100, walter harms wrote: > > > Am 30.01.2013 08:06, schrieb Dan Carpenter: > > There wasn't any error handling for this kzalloc(). > > > > Signed-off-by: Dan Carpenter > > > > diff --git a/drivers/scsi/osd/osd_initiator.c > > b/drivers/scsi/osd/osd_initiator.c > > index c06b8e5..d8293f2 100644 > > --- a/drivers/scsi/osd/osd_initiator.c > > +++ b/drivers/scsi/osd/osd_initiator.c > > @@ -144,6 +144,10 @@ static int _osd_get_print_system_info(struct osd_dev > > *od, > > odi->osdname_len = get_attrs[a].len; > > /* Avoid NULL for memcmp optimization 0-length is good enough */ > > odi->osdname = kzalloc(odi->osdname_len + 1, GFP_KERNEL); > > + if (!odi->osdname) { > > + ret = -ENOMEM; > > + goto out; > > + } > > if (odi->osdname_len) > > memcpy(odi->osdname, get_attrs[a].val_ptr, odi->osdname_len); > > OSD_INFO("OSD_NAME [%s]\n", odi->osdname); > > -- > > this looks like strdup() ? > Maybe? It's a funny thing going on with the NUL terminator and I don't understand what the comment is about. It appears that normally "get_attrs[a].val_ptr" is a NUL terminated string but "get_attrs[a].len" does not count the terminator. Odd. regards, dan carpenter -- 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] [SCSI] libosd: check for kzalloc() failure
Am 30.01.2013 08:06, schrieb Dan Carpenter: > There wasn't any error handling for this kzalloc(). > > Signed-off-by: Dan Carpenter > > diff --git a/drivers/scsi/osd/osd_initiator.c > b/drivers/scsi/osd/osd_initiator.c > index c06b8e5..d8293f2 100644 > --- a/drivers/scsi/osd/osd_initiator.c > +++ b/drivers/scsi/osd/osd_initiator.c > @@ -144,6 +144,10 @@ static int _osd_get_print_system_info(struct osd_dev *od, > odi->osdname_len = get_attrs[a].len; > /* Avoid NULL for memcmp optimization 0-length is good enough */ > odi->osdname = kzalloc(odi->osdname_len + 1, GFP_KERNEL); > + if (!odi->osdname) { > + ret = -ENOMEM; > + goto out; > + } > if (odi->osdname_len) > memcpy(odi->osdname, get_attrs[a].val_ptr, odi->osdname_len); > OSD_INFO("OSD_NAME [%s]\n", odi->osdname); > -- this looks like strdup() ? re, wh -- 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 v3] qla2xxx: silence two GCC warnings
On Wed, 2013-01-30 at 08:07 +, Saurav Kashyap wrote: > I am submitting some correction patches today and this patch will be part > of the scsi-misc submission after that set. Thanks. Paul Bolle -- 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 v3] qla2xxx: silence two GCC warnings
>On Mon, 2012-10-08 at 11:15 -0500, Saurav Kashyap wrote: >> Acked-by: Saurav Kashyap >> >> Thanks, >> ~Saurav >> >> >Compiling qla_gs.o (part of the qla2xxx module) triggers two GCC >> >warnings: >> >drivers/scsi/qla2xxx/qla_gs.c: In function Œqla2x00_fdmi_rhba¹: >> >drivers/scsi/qla2xxx/qla_gs.c:1339:7: warning: array subscript is >> >above array bounds [-Warray-bounds] >> >drivers/scsi/qla2xxx/qla_gs.c: In function Œqla2x00_fdmi_register¹: >> >drivers/scsi/qla2xxx/qla_gs.c:1663:15: warning: array subscript is >> >above array bounds [-Warray-bounds] > >This patch was originally posted to silence two GCC warnings while >building v3.6-rc7. Basically identical warnings can still be seen while >building v3.8-rc5. What's the status of this patch? Hi Paul, I am submitting some correction patches today and this patch will be part of the scsi-misc submission after that set. Thanks, ~Saurav > > >Paul Bolle > >-- >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 > This message and any attached documents contain information from QLogic Corporation or its wholly-owned subsidiaries that may be confidential. If you are not the intended recipient, you may not read, copy, distribute, or use this information. If you have received this transmission in error, please notify the sender immediately by reply e-mail and then delete this message.