Re: tgt infrastructure removal
I got an older Ack from Tomo for the removal, and some sort of acks for the ibmvscsi target removal from Brian, Paolo and Nathan. Can I get some formal reviews for the code removal patches (1-4) so I can queue this up for 3.17? I'll have to redo patch 5 for various changes and will resend it in time (hopefully after scsi-mq is in). -- 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 5/5] scsi: remove various exports that were only used by scsi_tgt
Il 15/04/2014 12:26, Christoph Hellwig ha scritto: Signed-off-by: Christoph Hellwig --- drivers/scsi/scsi.c | 10 +++--- drivers/scsi/scsi_lib.c | 10 -- include/scsi/scsi_cmnd.h |5 - include/scsi/scsi_host.h |2 -- 4 files changed, 7 insertions(+), 20 deletions(-) diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c index 88d46fe..c499172 100644 --- a/drivers/scsi/scsi.c +++ b/drivers/scsi/scsi.c @@ -235,7 +235,8 @@ fail: * Description: allocate a struct scsi_cmd from host's slab, recycling from the * host's free_list if necessary. */ -struct scsi_cmnd *__scsi_get_command(struct Scsi_Host *shost, gfp_t gfp_mask) +static struct scsi_cmnd * +__scsi_get_command(struct Scsi_Host *shost, gfp_t gfp_mask) { struct scsi_cmnd *cmd = scsi_host_alloc_command(shost, gfp_mask); @@ -265,7 +266,6 @@ struct scsi_cmnd *__scsi_get_command(struct Scsi_Host *shost, gfp_t gfp_mask) return cmd; } -EXPORT_SYMBOL_GPL(__scsi_get_command); /** * scsi_get_command - Allocate and setup a scsi command block @@ -291,14 +291,13 @@ struct scsi_cmnd *scsi_get_command(struct scsi_device *dev, gfp_t gfp_mask) cmd->jiffies_at_alloc = jiffies; return cmd; } -EXPORT_SYMBOL(scsi_get_command); /** * __scsi_put_command - Free a struct scsi_cmnd * @shost: dev->host * @cmd: Command to free */ -void __scsi_put_command(struct Scsi_Host *shost, struct scsi_cmnd *cmd) +static void __scsi_put_command(struct Scsi_Host *shost, struct scsi_cmnd *cmd) { unsigned long flags; @@ -314,7 +313,6 @@ void __scsi_put_command(struct Scsi_Host *shost, struct scsi_cmnd *cmd) if (likely(cmd != NULL)) scsi_host_free_command(shost, cmd); } -EXPORT_SYMBOL(__scsi_put_command); /** * scsi_put_command - Free a scsi command block @@ -338,7 +336,6 @@ void scsi_put_command(struct scsi_cmnd *cmd) __scsi_put_command(cmd->device->host, cmd); } -EXPORT_SYMBOL(scsi_put_command); static struct scsi_host_cmd_pool * scsi_find_host_cmd_pool(struct Scsi_Host *shost) @@ -801,7 +798,6 @@ void scsi_finish_command(struct scsi_cmnd *cmd) } scsi_io_completion(cmd, good_bytes); } -EXPORT_SYMBOL(scsi_finish_command); /** * scsi_adjust_queue_depth - Let low level drivers change a device's queue depth diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c index 7fa54fe..609cd61 100644 --- a/drivers/scsi/scsi_lib.c +++ b/drivers/scsi/scsi_lib.c @@ -511,6 +511,7 @@ void scsi_run_host_queues(struct Scsi_Host *shost) scsi_run_queue(sdev->request_queue); } +static void scsi_release_buffers(struct scsi_cmnd *cmd); static void __scsi_release_buffers(struct scsi_cmnd *, int); /* @@ -661,11 +662,10 @@ static void __scsi_release_buffers(struct scsi_cmnd *cmd, int do_bidi_check) * the scatter-gather table, and potentially any bounce * buffers. */ -void scsi_release_buffers(struct scsi_cmnd *cmd) +static void scsi_release_buffers(struct scsi_cmnd *cmd) { __scsi_release_buffers(cmd, 1); } -EXPORT_SYMBOL(scsi_release_buffers); /** * __scsi_error_from_host_byte - translate SCSI error code into errno @@ -1042,7 +1042,7 @@ static int scsi_init_sgtable(struct request *req, struct scsi_data_buffer *sdb, * BLKPREP_DEFER if the failure is retryable * BLKPREP_KILL if the failure is fatal */ -int scsi_init_io(struct scsi_cmnd *cmd, gfp_t gfp_mask) +static int scsi_init_io(struct scsi_cmnd *cmd, gfp_t gfp_mask) { struct scsi_device *sdev = cmd->device; struct request *rq = cmd->request; @@ -1095,7 +1095,6 @@ err_exit: put_device(&sdev->sdev_gendev); return error; } -EXPORT_SYMBOL(scsi_init_io); static struct scsi_cmnd *scsi_get_cmd_from_req(struct scsi_device *sdev, struct request *req) @@ -1649,7 +1648,7 @@ out_delay: blk_delay_queue(q, SCSI_QUEUE_DELAY); } -u64 scsi_calculate_bounce_limit(struct Scsi_Host *shost) +static u64 scsi_calculate_bounce_limit(struct Scsi_Host *shost) { struct device *host_dev; u64 bounce_limit = 0x; @@ -1669,7 +1668,6 @@ u64 scsi_calculate_bounce_limit(struct Scsi_Host *shost) return bounce_limit; } -EXPORT_SYMBOL(scsi_calculate_bounce_limit); struct request_queue *__scsi_alloc_queue(struct Scsi_Host *shost, request_fn_proc *request_fn) diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h index dd7c998..a8f3faf 100644 --- a/include/scsi/scsi_cmnd.h +++ b/include/scsi/scsi_cmnd.h @@ -140,18 +140,13 @@ static inline struct scsi_driver *scsi_cmd_to_driver(struct scsi_cmnd *cmd) } extern struct scsi_cmnd *scsi_get_command(struct scsi_device *, gfp_t); -extern struct scsi_cmnd *__scsi_get_command(struct Scsi_Host *, gfp_t); extern void scsi_put_command(struct scsi_cmnd *); -extern void __scsi_put_command(struct Scsi_Host *, struct scsi_cmnd *); e
Re: tgt infrastructure removal
Il 04/07/2014 11:36, Christoph Hellwig ha scritto: I got an older Ack from Tomo for the removal, and some sort of acks for the ibmvscsi target removal from Brian, Paolo and Nathan. Can I get some formal reviews for the code removal patches (1-4) so I can queue this up for 3.17? Not that there's much to review there. :) Reviewed-by: Paolo Bonzini 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: break from queue depth adjusting loops when device found
On 07/03/2014 07:11 PM, Christoph Hellwig wrote: On Thu, Jul 03, 2014 at 10:05:57AM -0500, Stephen M. Cameron wrote: From: Stephen M. Cameron Don't loop through all the devices even after finding the one we're looking for The comments in the code seem to indicate that we want to modify the queue depth for all LUNs on a given target. Ccing Mike and Vasu as they wrote this code. Indeed, that was the idea. This piece of code tries to keep track of the remote port queue depth, which isn't represented at all. Thing is, each remote target has a target queue depth which can hold only so many outstanding SCSI requests. If that is full it'll return BUSY for _all_ LUNs served from that port. And the very _next_ command after the one which filled the target queue will get the BUSY status. So we need to decrease the queue depth on _all_ LUNs here to avoid starvation of individual devices. Hence I guess this is not the correct fix. Cheers, Hannes -- Dr. Hannes Reinecke zSeries & Storage h...@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg) -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: tgt infrastructure removal
On 07/04/2014 11:52 AM, Paolo Bonzini wrote: Il 04/07/2014 11:36, Christoph Hellwig ha scritto: I got an older Ack from Tomo for the removal, and some sort of acks for the ibmvscsi target removal from Brian, Paolo and Nathan. Can I get some formal reviews for the code removal patches (1-4) so I can queue this up for 3.17? Not that there's much to review there. :) Reviewed-by: Paolo Bonzini And I checked with our partners which might have some interest in keeping it. Turns out they haven't. So: Reviewed-by: Hannes Reinecke Cheers, Hannes -- Dr. Hannes Reinecke zSeries & Storage h...@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg) -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: how to firing device event when SD card inserting usb card reader
hi Alan: 2014-06-26 23:57 GMT+08:00 Alan Stern : > On Thu, 26 Jun 2014, loody wrote: > >> hi all: >> I try below flow: >> 1. plug in usb card reader >> 2. wait 2 seconds >> 3. plug in SD card >> >> on ubuntu PC system, the udev can get SD plug in event >> but on my embedded system, there is no udev plug event when SD plug in. >> >> Is this issue related to media change command? >> if so, should I enable kernel config or do anything in user mode program? >> >> thanks for your help in advance, > > What does usbmon show? after we enable CONFIG_SCSI_MULTI_LUN, we can udevent when SD plug in card reader. appreciate your kind help, -- 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/2] Fixed for 64bit LUNs
Hi Christoph, here are some more fixes for 64bit LUNs, which should fix the latest built issues from Fengguang. Most notably I've introduced a new accessor 'sdev_scsi2lun' for those devices which are not capable of handling LUNs higher than 255. And I've removed the custom LUN handling code from ib_srp. Hannes Reinecke (2): Use sdev_scsi2lun for SCSI parallel drivers ib_srp: 64bit LUN fixes arch/ia64/hp/sim/simscsi.c| 2 +- drivers/infiniband/ulp/srp/ib_srp.c | 9 ++-- drivers/infiniband/ulp/srpt/ib_srpt.c | 81 + drivers/scsi/53c700.c | 12 ++--- drivers/scsi/BusLogic.c | 2 +- drivers/scsi/NCR5380.c| 32 ++--- drivers/scsi/NCR53c406a.c | 2 +- drivers/scsi/a100u2w.c| 4 +- drivers/scsi/advansys.c | 16 +++ drivers/scsi/aha152x.c| 13 +++--- drivers/scsi/aha1542.c| 4 +- drivers/scsi/aha1740.c| 2 +- drivers/scsi/aic7xxx/aic79xx_osm.c| 19 drivers/scsi/aic7xxx/aic79xx_proc.c | 2 +- drivers/scsi/aic7xxx/aic7xxx_osm.c| 23 +- drivers/scsi/aic7xxx/aic7xxx_proc.c | 2 +- drivers/scsi/arm/acornscsi.c | 20 drivers/scsi/arm/fas216.c | 15 +++--- drivers/scsi/arm/queue.c | 8 ++-- drivers/scsi/atari_NCR5380.c | 81 + drivers/scsi/atp870u.c| 2 +- drivers/scsi/bnx2fc/bnx2fc_io.c | 4 +- drivers/scsi/ch.c | 16 +++ drivers/scsi/dc395x.c | 73 ++--- drivers/scsi/eata.c | 4 +- drivers/scsi/eata_pio.c | 2 +- drivers/scsi/esp_scsi.c | 6 +-- drivers/scsi/g_NCR5380.c | 2 +- drivers/scsi/gdth.c | 4 +- drivers/scsi/hptiop.c | 6 +-- drivers/scsi/in2000.c | 36 --- drivers/scsi/initio.c | 4 +- drivers/scsi/megaraid.c | 10 ++-- drivers/scsi/mesh.c | 6 +-- drivers/scsi/mvumi.c | 2 +- drivers/scsi/ncr53c8xx.c | 33 +++--- drivers/scsi/nsp32.c | 14 +++--- drivers/scsi/ps3rom.c | 2 +- drivers/scsi/qlogicpti.c | 6 +-- drivers/scsi/scsi.c | 2 +- drivers/scsi/stex.c | 2 +- drivers/scsi/sun3_NCR5380.c | 86 +++ drivers/scsi/sym53c8xx_2/sym_glue.c | 19 drivers/scsi/sym53c8xx_2/sym_hipd.c | 10 ++-- drivers/scsi/tmscsim.c| 12 +++-- drivers/scsi/u14-34f.c| 6 +-- drivers/scsi/ufs/ufshcd.c | 4 +- drivers/scsi/ultrastor.c | 2 +- drivers/scsi/wd33c93.c| 35 -- drivers/scsi/wd7000.c | 3 +- drivers/staging/rts5208/rtsx.c| 4 +- include/scsi/scsi_device.h| 5 ++ 52 files changed, 373 insertions(+), 398 deletions(-) -- 1.7.12.4 -- 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/2] ib_srp: 64bit LUN fixes
SRP is capable of handling 64bit LUNs, so as we now have proper support for it we can modify the driver to use the standard functions. Cc: Bart van Assche Signed-off-by: Hannes Reinecke --- drivers/infiniband/ulp/srp/ib_srp.c | 9 ++-- drivers/infiniband/ulp/srpt/ib_srpt.c | 81 +-- 2 files changed, 7 insertions(+), 83 deletions(-) diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c index e3c2c5b..eb55f25 100644 --- a/drivers/infiniband/ulp/srp/ib_srp.c +++ b/drivers/infiniband/ulp/srp/ib_srp.c @@ -1713,7 +1713,8 @@ static void srp_process_aer_req(struct srp_target_port *target, s32 delta = be32_to_cpu(req->req_lim_delta); shost_printk(KERN_ERR, target->scsi_host, PFX -"ignoring AER for LUN %llu\n", be64_to_cpu(req->lun)); +"ignoring AER for LUN %llu\n", +scsilun_to_int((struct scsi_lun *)&req->lun)); if (srp_response_common(target, delta, &rsp, sizeof rsp)) shost_printk(KERN_ERR, target->scsi_host, PFX @@ -1887,7 +1888,7 @@ static int srp_queuecommand(struct Scsi_Host *shost, struct scsi_cmnd *scmnd) memset(cmd, 0, sizeof *cmd); cmd->opcode = SRP_CMD; - cmd->lun= cpu_to_be64((u64) scmnd->device->lun << 48); + int_to_scsilun(scmnd->device->lun, (struct scsi_lun *)&cmd->lun); cmd->tag= req->index; memcpy(cmd->cdb, scmnd->cmnd, scmnd->cmd_len); @@ -2305,7 +2306,7 @@ srp_change_queue_depth(struct scsi_device *sdev, int qdepth, int reason) } static int srp_send_tsk_mgmt(struct srp_target_port *target, -u64 req_tag, unsigned int lun, u8 func) +u64 req_tag, u64 lun, u8 func) { struct srp_rport *rport = target->rport; struct ib_device *dev = target->srp_host->srp_dev->dev; @@ -2338,7 +2339,7 @@ static int srp_send_tsk_mgmt(struct srp_target_port *target, memset(tsk_mgmt, 0, sizeof *tsk_mgmt); tsk_mgmt->opcode= SRP_TSK_MGMT; - tsk_mgmt->lun = cpu_to_be64((u64) lun << 48); + int_to_scsilun(lun, (struct scsi_lun *)&tsk_mgmt->lun); tsk_mgmt->tag = req_tag | SRP_TAG_TSK_MGMT; tsk_mgmt->tsk_mgmt_func = func; tsk_mgmt->task_tag = req_tag; diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c index fe09f27..232c29a 100644 --- a/drivers/infiniband/ulp/srpt/ib_srpt.c +++ b/drivers/infiniband/ulp/srpt/ib_srpt.c @@ -1601,81 +1601,6 @@ static int srpt_build_tskmgmt_rsp(struct srpt_rdma_ch *ch, return resp_len; } -#define NO_SUCH_LUN ((uint64_t)-1LL) - -/* - * SCSI LUN addressing method. See also SAM-2 and the section about - * eight byte LUNs. - */ -enum scsi_lun_addr_method { - SCSI_LUN_ADDR_METHOD_PERIPHERAL = 0, - SCSI_LUN_ADDR_METHOD_FLAT = 1, - SCSI_LUN_ADDR_METHOD_LUN = 2, - SCSI_LUN_ADDR_METHOD_EXTENDED_LUN = 3, -}; - -/* - * srpt_unpack_lun() - Convert from network LUN to linear LUN. - * - * Convert an 2-byte, 4-byte, 6-byte or 8-byte LUN structure in network byte - * order (big endian) to a linear LUN. Supports three LUN addressing methods: - * peripheral, flat and logical unit. See also SAM-2, section 4.9.4 (page 40). - */ -static uint64_t srpt_unpack_lun(const uint8_t *lun, int len) -{ - uint64_t res = NO_SUCH_LUN; - int addressing_method; - - if (unlikely(len < 2)) { - printk(KERN_ERR "Illegal LUN length %d, expected 2 bytes or " - "more", len); - goto out; - } - - switch (len) { - case 8: - if ((*((__be64 *)lun) & -__constant_cpu_to_be64(0xLL)) != 0) - goto out_err; - break; - case 4: - if (*((__be16 *)&lun[2]) != 0) - goto out_err; - break; - case 6: - if (*((__be32 *)&lun[2]) != 0) - goto out_err; - break; - case 2: - break; - default: - goto out_err; - } - - addressing_method = (*lun) >> 6; /* highest two bits of byte 0 */ - switch (addressing_method) { - case SCSI_LUN_ADDR_METHOD_PERIPHERAL: - case SCSI_LUN_ADDR_METHOD_FLAT: - case SCSI_LUN_ADDR_METHOD_LUN: - res = *(lun + 1) | (((*lun) & 0x3f) << 8); - break; - - case SCSI_LUN_ADDR_METHOD_EXTENDED_LUN: - default: - printk(KERN_ERR "Unimplemented LUN addressing method %u", - addressing_method); - break; - } - -out: - return res; - -out_err: - printk(KERN_ERR "Support for multi-level LUNs has not yet been" - " implemented"); - goto out; -} - static int srpt_check_stop_free(struct se_cmd *cmd) {
Re: [PATCH 2/2] ib_srp: 64bit LUN fixes
On 07/04/14 13:54, Hannes Reinecke wrote: > SRP is capable of handling 64bit LUNs, so as we now have proper > support for it we can modify the driver to use the standard functions. > > Cc: Bart van Assche > Signed-off-by: Hannes Reinecke > --- > drivers/infiniband/ulp/srp/ib_srp.c | 9 ++-- > drivers/infiniband/ulp/srpt/ib_srpt.c | 81 > +-- > 2 files changed, 7 insertions(+), 83 deletions(-) The SRP initiator and target drivers are independent drivers so this should be two patches instead of one. Furthermore, I do not agree with introducing a call to int_to_scsilun() in the hot path of the initiator driver since that causes a small but unnecessary additional overhead. Please keep in mind that there is no need to repeat the int_to_scsilun() conversion for every I/O request. Hence my earlier proposal to cache the int_to_scsilun() result. Bart. -- 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 2/2] ib_srp: 64bit LUN fixes
On 07/04/2014 02:31 PM, Bart Van Assche wrote: On 07/04/14 13:54, Hannes Reinecke wrote: SRP is capable of handling 64bit LUNs, so as we now have proper support for it we can modify the driver to use the standard functions. Cc: Bart van Assche Signed-off-by: Hannes Reinecke --- drivers/infiniband/ulp/srp/ib_srp.c | 9 ++-- drivers/infiniband/ulp/srpt/ib_srpt.c | 81 +-- 2 files changed, 7 insertions(+), 83 deletions(-) The SRP initiator and target drivers are independent drivers so this should be two patches instead of one. Furthermore, I do not agree with introducing a call to int_to_scsilun() in the hot path of the initiator driver since that causes a small but unnecessary additional overhead. Please keep in mind that there is no need to repeat the int_to_scsilun() conversion for every I/O request. Hence my earlier proposal to cache the int_to_scsilun() result. What I would like to do is to provide accessor functions for the scsi lun; I've started this already for the older SCSI parallel drivers (see my earlier patch). Once everything is moved over to accessors it should be trivial to move from the current scsilun_to_int representation in struct scsi_device to the 'real' LUN. Adding another field would be bit of an overkill IMO, seeing that we can achieve the very same thing with a bit of code reshuffling. James, Christoph, what's your opinion on using accessors? Would solve this issue nicely, and we can also use proper functions for HBAs only capable of handling 32bit LUNs. Drawback is that we would need accessors also for this capable of handling full 64bit LUNs, thus potentially incur some overhead there. Cheers, Hannes -- Dr. Hannes Reinecke zSeries & Storage h...@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg) -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] Use sdev_scsi2lun for SCSI parallel drivers
On Fri, Jul 04, 2014 at 01:54:34PM +0200, Hannes Reinecke wrote: > SCSI-2 defines only up to 256 LUNs with a flat namespace. > This patch introduces an accessor 'sdev_scsi2lun' which should be > used for these drivers to avoid problems when using 64-bit LUNs > internally. So what would be set in the upper 48 bits for those drivers? We receive the LUN over the wire to start with. -- 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 2/2] ib_srp: 64bit LUN fixes
On Fri, Jul 04, 2014 at 03:01:40PM +0200, Hannes Reinecke wrote: > What I would like to do is to provide accessor functions for the scsi lun; > I've started this already for the older SCSI parallel drivers (see my > earlier patch). > Once everything is moved over to accessors it should be trivial to move from > the current scsilun_to_int representation in struct scsi_device to the > 'real' LUN. > > Adding another field would be bit of an overkill IMO, seeing that we can > achieve the very same thing with a bit of code reshuffling. > > James, Christoph, what's your opinion on using accessors? > Would solve this issue nicely, and we can also use proper functions for HBAs > only capable of handling 32bit LUNs. > Drawback is that we would need accessors also for this capable of handling > full 64bit LUNs, thus potentially incur some overhead there. I think storing the struct scsi_lun in the scsi_device is the right way to go ahead. Any "accessors" for 8 or 32-bit LUNs should be simply enough by just ignoring bits in the array, so there's very little performance overhead. If we can get rid of the old scalar LUN field that would be great, and given that we know the printk format fallout already it doesn't look like too much work. Do you want to look into this? -- 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/2] Use sdev_scsi2lun for SCSI parallel drivers
On 07/04/2014 03:44 PM, Christoph Hellwig wrote: On Fri, Jul 04, 2014 at 01:54:34PM +0200, Hannes Reinecke wrote: SCSI-2 defines only up to 256 LUNs with a flat namespace. This patch introduces an accessor 'sdev_scsi2lun' which should be used for these drivers to avoid problems when using 64-bit LUNs internally. So what would be set in the upper 48 bits for those drivers? We receive the LUN over the wire to start with. None. All these devices are physically incapable of receiving or sending LUNs more that a byte wide. Cheers, Hannes -- Dr. Hannes Reinecke zSeries & Storage h...@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg) -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] ib_srp: 64bit LUN fixes
On 07/04/2014 03:48 PM, Christoph Hellwig wrote: On Fri, Jul 04, 2014 at 03:01:40PM +0200, Hannes Reinecke wrote: What I would like to do is to provide accessor functions for the scsi lun; I've started this already for the older SCSI parallel drivers (see my earlier patch). Once everything is moved over to accessors it should be trivial to move from the current scsilun_to_int representation in struct scsi_device to the 'real' LUN. Adding another field would be bit of an overkill IMO, seeing that we can achieve the very same thing with a bit of code reshuffling. James, Christoph, what's your opinion on using accessors? Would solve this issue nicely, and we can also use proper functions for HBAs only capable of handling 32bit LUNs. Drawback is that we would need accessors also for this capable of handling full 64bit LUNs, thus potentially incur some overhead there. I think storing the struct scsi_lun in the scsi_device is the right way to go ahead. Any "accessors" for 8 or 32-bit LUNs should be simply enough by just ignoring bits in the array, so there's very little performance overhead. If we can get rid of the old scalar LUN field that would be great, and given that we know the printk format fallout already it doesn't look like too much work. Do you want to look into this? Already working on it. Cheers, Hannes -- Dr. Hannes Reinecke zSeries & Storage h...@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg) -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
virtio_scsi LUN usage
Hi Paolo, virtio_scsi has this: static void virtio_scsi_init_hdr(struct virtio_scsi_cmd_req *cmd, struct scsi_cmnd *sc) { cmd->lun[0] = 1; cmd->lun[1] = sc->device->id; cmd->lun[2] = (sc->device->lun >> 8) | 0x40; cmd->lun[3] = sc->device->lun & 0xff; cmd->tag = (unsigned long)sc; cmd->task_attr = VIRTIO_SCSI_S_SIMPLE; cmd->prio = 0; cmd->crn = 0; } As you might've seen I'm currently working on updating the SCSI stack to use 64-bit LUNs. So while virtio scsi is in principle capable of using 64-bit LUNs, this peculiar usage makes it impossible to actually _use_ 64-bit LUNs. Can't we get rid of this encoding and let the backend provide is with correct numbers? It should work if we were just to implement a REPORT_LUN intercept, which sets the correct top two bytes in the command ... with that we should be getting only the LUNs for a particular target, and could then just use the provided LUN numbers as-is. Long term I would prefer to have another 'target' field (preferably also 64bit wide), which could be used to specify the target directly. But that would require a spec change and feature flags yadda yadda... Cheers, Hannes -- Dr. Hannes Reinecke zSeries & Storage h...@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg) -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] ib_srp: 64bit LUN fixes
On 07/04/14 16:12, Hannes Reinecke wrote: > On 07/04/2014 03:48 PM, Christoph Hellwig wrote: >> I think storing the struct scsi_lun in the scsi_device is the right way >> to go ahead. Any "accessors" for 8 or 32-bit LUNs should be simply >> enough by just ignoring bits in the array, so there's very little >> performance overhead. >> >> If we can get rid of the old scalar LUN field that would be great, >> and given that we know the printk format fallout already it doesn't look >> like too much work. Do you want to look into this? > > Already working on it. That sounds great. Regarding the SRP initiator driver: if the "__be64 lun" declarations in are changed into "struct scsi_lun lun" then that allows to encode / decode LUNs inside the SRP initiator without having to cast the address of these "lun" members into struct scsi_lun *. In case you would prefer me to have a look into this, please let me know. Bart. -- 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 2/2] ib_srp: 64bit LUN fixes
On 07/04/2014 04:38 PM, Bart Van Assche wrote: On 07/04/14 16:12, Hannes Reinecke wrote: On 07/04/2014 03:48 PM, Christoph Hellwig wrote: I think storing the struct scsi_lun in the scsi_device is the right way to go ahead. Any "accessors" for 8 or 32-bit LUNs should be simply enough by just ignoring bits in the array, so there's very little performance overhead. If we can get rid of the old scalar LUN field that would be great, and given that we know the printk format fallout already it doesn't look like too much work. Do you want to look into this? Already working on it. That sounds great. Regarding the SRP initiator driver: if the "__be64 lun" declarations in are changed into "struct scsi_lun lun" then that allows to encode / decode LUNs inside the SRP initiator without having to cast the address of these "lun" members into struct scsi_lun *. In case you would prefer me to have a look into this, please let me know. That would be perfect. Most of the HBAs supporting 64bit LUNs internally are already doing this. Cheers, Hannes -- Dr. Hannes Reinecke zSeries & Storage h...@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg) -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Some question about usb scsi storage driver
hi all: when I trace kernel driver source about usb scsi storage driver, I have below 2 questions: 1. in sd.c -> static int sd_probe(struct device *dev) we use below macro to get scsi_device. struct scsi_device *sdp = to_scsi_device(dev); take usb for example, is usb storage driver allocate memory for scsi_device then calling sd_probe? I am looking for usb_stor_probe1 and usb_stor_probe2 but not find where it is. 2. at the end of sd.c -> sd_probe, why we call async_schedule_domain like below async_schedule_domain(sd_probe_async, sdkp, &scsi_sd_probe_domain); to finish scsi device initialization? Couldn't we put what sd_probe_async directly in sd_probe? is there any benefit to do so? -- Regards, -- 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
uas - kernel panic on drive connection
Beginning with kernel 3.15.1, I am getting hard lockups every time I connect a drive to my USB 3 HDD dock with ASMedia ASM1051E UASP compliant chipset. The only way I am able work around this is to set the quirk to ignore uas for the device. Here are the kernel messages during connection followed by a backtrace: [ 229.882190] usb 2-1: new SuperSpeed USB device number 3 using xhci_hcd [ 229.908001] usb-storage 2-1:1.0: USB Mass Storage device detected [ 229.908218] usb-storage 2-1:1.0: Quirks match for vid 174c pid 55aa: 40 [ 229.908349] scsi7 : usb-storage 2-1:1.0 [ 230.912633] scsi 7:0:0:0: Direct-Access ASMT 2105 0PQ: 0 ANSI: 6 [ 230.914818] sd 7:0:0:0: [sdc] Attached SCSI removable disk [ 289.680267] usb 2-1: USB disconnect, device number 3 [ 319.599862] usb 2-1: new SuperSpeed USB device number 4 using xhci_hcd [ 319.626881] scsi8 : uas [ 319.628221] xhci_hcd :02:00.0: ERROR Transfer event TRB DMA ptr not part of current TD [ 320.029323] BUG: unable to handle kernel paging request at 0103 [ 320.029393] IP: [] kmem_cache_alloc+0x6b/0x170 [ 320.029445] PGD 40821a067 PUD 401bbf067 PMD 40411c067 PTE 0 [ 320.029505] Oops: [#1] PREEMPT SMP [ 320.029552] Modules linked in: uas usb_storage netconsole des_generic ecb md4 nls_utf8 cifs dns_resolver sch_fq_codel fuse nf_conntrack_ipv4 nf_defrag_ipv4 xt_tcpudp xt_conntrack nf_conntrack iptable_filter ip_tables x_tables vmnet(O) vsock vmci(O) vmmon(O) w83627ehf hwmon_vid adm1021 xfs libcrc32c nls_iso8859_1 nls_cp437 vfat fat coretemp hwmon iTCO_wdt intel_rapl iTCO_vendor_support x86_pkg_temp_thermal intel_powerclamp crct10dif_pclmul crc32_pclmul ppdev crc32c_intel snd_hda_codec_hdmi ghash_clmulni_intel aesni_intel snd_hda_codec_realtek snd_hda_codec_generic aes_x86_64 lrw gf128mul glue_helper ablk_helper cryptd e1000e hdpvr microcode evdev snd_hda_intel i915 joydev v4l2_dv_timings ptp mousedev psmouse r8168(O) mac_hid snd_hda_controller videodev serio_raw pcspkr media snd_hda_codec i2c_i801 lpc_ich pps_core snd_hwdep drm_kms_helper snd_pcm snd_timer drm snd battery tpm_tis parport_pc tpm intel_gtt parport i2c_algo_bit mei_me soundcore mei nuvoton_cir video shpchp button processor nfsd auth_rpcgss oid_registry nfs_acl nfs lockd fscache sunrpc lirc_zilog(C) i2c_core lirc_dev rc_core ext4 crc16 mbcache jbd2 hid_generic usbhid hid sd_mod crc_t10dif crct10dif_common atkbd libps2 ehci_pci xhci_hcd ehci_hcd firewire_ohci ahci libahci libata firewire_core crc_itu_t scsi_mod usbcore usb_common i8042 serio [ 320.030938] CPU: 0 PID: 782 Comm: mysqld Tainted: G C O 3.15.3-ARCH #1 [ 320.030988] Hardware name: To Be Filled By O.E.M. To Be Filled By O.E.M./Z68 Pro3-M, BIOS P2.30 06/29/2012 [ 320.031052] task: 8800c9488a30 ti: 8800cebd8000 task.ti: 8800cebd8000 [ 320.031103] RIP: 0010:[] [] kmem_cache_alloc+0x6b/0x170 [ 320.031165] RSP: 0018:8800cebdba78 EFLAGS: 00010082 [ 320.031203] RAX: RBX: a0118140 RCX: 00833300 [ 320.031251] RDX: 00833280 RSI: 0020 RDI: a00fd04b [ 320.031298] RBP: 8800cebdbaa8 R08: 00017560 R09: 8803fad57380 [ 320.031346] R10: 002f R11: 8803fad5002f R12: 0103 [ 320.031394] R13: 0020 R14: 880407af1000 R15: 88040ec01900 [ 320.031442] FS: 7f28c2bff700() GS:88041f20() knlGS: [ 320.031496] CS: 0010 DS: ES: CR0: 80050033 [ 320.031536] CR2: 0103 CR3: 000407474000 CR4: 000407f0 [ 320.031584] Stack: [ 320.031602] a00fd030 a0118140 8803fad57380 0020 [ 320.031669] 880407af1000 0020 8800cebdbae0 a00fd04b [ 320.031738] 8803c8e93780 880407af1000 880407430968 0084f7e0 [ 320.031806] Call Trace: [ 320.031839] [] ? scsi_host_alloc_command+0x30/0xd0 [scsi_mod] [ 320.031899] [] scsi_host_alloc_command+0x4b/0xd0 [scsi_mod] [ 320.031956] [] __scsi_get_command+0x18/0xe0 [scsi_mod] [ 320.032009] [] scsi_get_command+0x1b/0xd0 [scsi_mod] [ 320.032064] [] scsi_get_cmd_from_req+0x74/0xa0 [scsi_mod] [ 320.032121] [] scsi_setup_fs_cmnd+0x3d/0xb0 [scsi_mod] [ 320.032173] [] sd_prep_fn+0x3b8/0xd30 [sd_mod] [ 320.032219] [] blk_peek_request+0x131/0x280 [ 320.032268] [] scsi_request_fn+0x3e/0x4b0 [scsi_mod] [ 320.032315] [] __blk_run_queue+0x33/0x40 [ 320.032356] [] queue_unplugged+0x2e/0xd0 [ 320.032398] [] blk_flush_plug_list+0x1ed/0x270 [ 320.032442] [] blk_finish_plug+0x14/0x50 [ 320.032491] [] ext4_writepages+0x449/0xd20 [ext4] [ 320.032539] [] do_writepages+0x1e/0x30 [ 320.032579] [] __filemap_fdatawrite_range+0x5d/0x80 [ 320.032625] [] filemap_write_and_wait_range+0x2a/0x70 [ 320.032680] [] ext4_sync_file+0x110/0x370 [ext4] [ 320.032726] [] do_fsync+0x4e/0x80 [ 320.032764] [] SyS_fsync+0x10/0x20 [ 320.032804] [] system_call_fastpath+0x16/0x1b [ 320.032846] Code: 0f 84 c
uas - kernel panic on drive connection
Beginning with kernel 3.15.1, I am getting hard lockups every time I connect a drive to my USB 3 HDD dock with ASMedia ASM1051E UASP compliant chipset. The only way I am able work around this is to set the quirk to ignore uas for the device. Here are the kernel messages during connection followed by a backtrace: [ 229.882190] usb 2-1: new SuperSpeed USB device number 3 using xhci_hcd [ 229.908001] usb-storage 2-1:1.0: USB Mass Storage device detected [ 229.908218] usb-storage 2-1:1.0: Quirks match for vid 174c pid 55aa: 40 [ 229.908349] scsi7 : usb-storage 2-1:1.0 [ 230.912633] scsi 7:0:0:0: Direct-Access ASMT 2105 0 PQ: 0 ANSI: 6 [ 230.914818] sd 7:0:0:0: [sdc] Attached SCSI removable disk [ 289.680267] usb 2-1: USB disconnect, device number 3 [ 319.599862] usb 2-1: new SuperSpeed USB device number 4 using xhci_hcd [ 319.626881] scsi8 : uas [ 319.628221] xhci_hcd :02:00.0: ERROR Transfer event TRB DMA ptr not part of current TD [ 320.029323] BUG: unable to handle kernel paging request at 0103 [ 320.029393] IP: [] kmem_cache_alloc+0x6b/0x170 [ 320.029445] PGD 40821a067 PUD 401bbf067 PMD 40411c067 PTE 0 [ 320.029505] Oops: [#1] PREEMPT SMP [ 320.029552] Modules linked in: uas usb_storage netconsole des_generic ecb md4 nls_utf8 cifs dns_resolver sch_fq_codel fuse nf_conntrack_ipv4 nf_defrag_ipv4 xt_tcpudp xt_conntrack nf_conntrack iptable_filter ip_tables x_tables vmnet(O) vsock vmci(O) vmmon(O) w83627ehf hwmon_vid adm1021 xfs libcrc32c nls_iso8859_1 nls_cp437 vfat fat coretemp hwmon iTCO_wdt intel_rapl iTCO_vendor_support x86_pkg_temp_thermal intel_powerclamp crct10dif_pclmul crc32_pclmul ppdev crc32c_intel snd_hda_codec_hdmi ghash_clmulni_intel aesni_intel snd_hda_codec_realtek snd_hda_codec_generic aes_x86_64 lrw gf128mul glue_helper ablk_helper cryptd e1000e hdpvr microcode evdev snd_hda_intel i915 joydev v4l2_dv_timings ptp mousedev psmouse r8168(O) mac_hid snd_hda_controller videodev serio_raw pcspkr media snd_hda_codec i2c_i801 lpc_ich pps_core snd_hwdep drm_kms_helper snd_pcm snd_timer drm snd battery tpm_tis parport_pc tpm intel_gtt parport i2c_algo_bit mei_me soundcore mei nuvoton_cir video shpchp button proc essor nfsd auth_rpcgss oid_registry nfs_acl nfs lockd fscache sunrpc lirc_zilog(C) i2c_core lirc_dev rc_core ext4 crc16 mbcache jbd2 hid_generic usbhid hid sd_mod crc_t10dif crct10dif_common atkbd libps2 ehci_pci xhci_hcd ehci_hcd firewire_ohci ahci libahci libata firewire_core crc_itu_t scsi_mod usbcore usb_common i8042 serio [ 320.030938] CPU: 0 PID: 782 Comm: mysqld Tainted: G C O 3.15.3-ARCH #1 [ 320.030988] Hardware name: To Be Filled By O.E.M. To Be Filled By O.E.M./Z68 Pro3-M, BIOS P2.30 06/29/2012 [ 320.031052] task: 8800c9488a30 ti: 8800cebd8000 task.ti: 8800cebd8000 [ 320.031103] RIP: 0010:[] [] kmem_cache_alloc+0x6b/0x170 [ 320.031165] RSP: 0018:8800cebdba78 EFLAGS: 00010082 [ 320.031203] RAX: RBX: a0118140 RCX: 00833300 [ 320.031251] RDX: 00833280 RSI: 0020 RDI: a00fd04b [ 320.031298] RBP: 8800cebdbaa8 R08: 00017560 R09: 8803fad57380 [ 320.031346] R10: 002f R11: 8803fad5002f R12: 0103 [ 320.031394] R13: 0020 R14: 880407af1000 R15: 88040ec01900 [ 320.031442] FS: 7f28c2bff700() GS:88041f20() knlGS: [ 320.031496] CS: 0010 DS: ES: CR0: 80050033 [ 320.031536] CR2: 0103 CR3: 000407474000 CR4: 000407f0 [ 320.031584] Stack: [ 320.031602] a00fd030 a0118140 8803fad57380 0020 [ 320.031669] 880407af1000 0020 8800cebdbae0 a00fd04b [ 320.031738] 8803c8e93780 880407af1000 880407430968 0084f7e0 [ 320.031806] Call Trace: [ 320.031839] [] ? scsi_host_alloc_command+0x30/0xd0 [scsi_mod] [ 320.031899] [] scsi_host_alloc_command+0x4b/0xd0 [scsi_mod] [ 320.031956] [] __scsi_get_command+0x18/0xe0 [scsi_mod] [ 320.032009] [] scsi_get_command+0x1b/0xd0 [scsi_mod] [ 320.032064] [] scsi_get_cmd_from_req+0x74/0xa0 [scsi_mod] [ 320.032121] [] scsi_setup_fs_cmnd+0x3d/0xb0 [scsi_mod] [ 320.032173] [] sd_prep_fn+0x3b8/0xd30 [sd_mod] [ 320.032219] [] blk_peek_request+0x131/0x280 [ 320.032268] [] scsi_request_fn+0x3e/0x4b0 [scsi_mod] [ 320.032315] [] __blk_run_queue+0x33/0x40 [ 320.032356] [] queue_unplugged+0x2e/0xd0 [ 320.032398] [] blk_flush_plug_list+0x1ed/0x270 [ 320.032442] [] blk_finish_plug+0x14/0x50 [ 320.032491] [] ext4_writepages+0x449/0xd20 [ext4] [ 320.032539] [] do_writepages+0x1e/0x30 [ 320.032579] [] __filemap_fdatawrite_range+0x5d/0x80 [ 320.032625] [] filemap_write_and_wait_range+0x2a/0x70 [ 320.032680] [] ext4_sync_file+0x110/0x370 [ext4] [ 320.032726] [] do_fsync+0x4e/0x80 [ 320.032764] [] SyS_fsync+0x10/0x20 [ 320.032804] [] system
Re: Some question about usb scsi storage driver
On 07/04/2014 04:44 PM, loody wrote: > > 2. at the end of sd.c -> sd_probe, why we call async_schedule_domain like > below > async_schedule_domain(sd_probe_async, sdkp, &scsi_sd_probe_domain); > to finish scsi device initialization? > Couldn't we put what sd_probe_async directly in sd_probe? > is there any benefit to do so? A possible explanation may be that sd_probe_async() calls sd_spinup_disk(), this function spins up the drive and may block for some seconds, so it is better to do that asynchronously. Regards, Maurizio Lombardi -- 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: Some question about usb scsi storage driver
On Fri, 4 Jul 2014, loody wrote: > hi all: > when I trace kernel driver source about usb scsi storage driver, I > have below 2 questions: > 1. in sd.c -> static int sd_probe(struct device *dev) > we use below macro to get scsi_device. >struct scsi_device *sdp = to_scsi_device(dev); > > take usb for example, is usb storage driver allocate memory for > scsi_device then calling sd_probe? No. > I am looking for usb_stor_probe1 and usb_stor_probe2 but not find > where it is. Because it's not there. The memory is allocated by drivers/scsi/scsi_scan.c:scsi_alloc_sdev(). > 2. at the end of sd.c -> sd_probe, why we call async_schedule_domain like > below > async_schedule_domain(sd_probe_async, sdkp, &scsi_sd_probe_domain); > to finish scsi device initialization? > Couldn't we put what sd_probe_async directly in sd_probe? > is there any benefit to do so? Like Maurizio said, the benefit is that the probe routine will return quickly so that other devices can be probed while the drive spins up. 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 2/2] ib_srp: 64bit LUN fixes
On 07/04/14 16:41, Hannes Reinecke wrote: > On 07/04/2014 04:38 PM, Bart Van Assche wrote: >> On 07/04/14 16:12, Hannes Reinecke wrote: >>> On 07/04/2014 03:48 PM, Christoph Hellwig wrote: I think storing the struct scsi_lun in the scsi_device is the right way to go ahead. Any "accessors" for 8 or 32-bit LUNs should be simply enough by just ignoring bits in the array, so there's very little performance overhead. If we can get rid of the old scalar LUN field that would be great, and given that we know the printk format fallout already it doesn't look like too much work. Do you want to look into this? >>> >>> Already working on it. >> >> That sounds great. Regarding the SRP initiator driver: if the "__be64 >> lun" declarations in are changed into "struct scsi_lun lun" >> then that allows to encode / decode LUNs inside the SRP initiator >> without having to cast the address of these "lun" members into struct >> scsi_lun *. In case you would prefer me to have a look into this, please >> let me know. >> > That would be perfect. > Most of the HBAs supporting 64bit LUNs internally are already doing this. Here it is (compile tested only on x86 and ppc - will do more testing once the entire series is available). Please CC all maintainers and relevant mailing lists for the next version of the 64-bit LUN patch series. Several IB/SRP users are reading the linux-rdma mailing list but not the linux-scsi mailing list. From: Bart Van Assche Date: Fri, 4 Jul 2014 17:11:31 +0200 Subject: [PATCH] SRP: Convert lun into struct scsi_lun Checked the following structure sizes with gdb: * sizeof(struct srp_cmd) = 48 * sizeof(struct srp_tsk_mgmt) = 48 * sizeof(struct srp_aer_req) = 36 The ibmvscsi changes have been compile tested only (on a PPC system). Signed-off-by: Bart Van Assche --- drivers/infiniband/ulp/srp/ib_srp.c | 6 ++-- drivers/infiniband/ulp/srpt/ib_srpt.c | 68 ++- drivers/scsi/ibmvscsi/ibmvscsi.c | 6 ++-- drivers/scsi/libsrp.c | 7 ++-- include/scsi/srp.h| 6 ++-- 5 files changed, 14 insertions(+), 79 deletions(-) diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c index b017a3a..a04f245 100644 --- a/drivers/infiniband/ulp/srp/ib_srp.c +++ b/drivers/infiniband/ulp/srp/ib_srp.c @@ -1719,7 +1719,7 @@ static void srp_process_aer_req(struct srp_target_port *target, s32 delta = be32_to_cpu(req->req_lim_delta); shost_printk(KERN_ERR, target->scsi_host, PFX -"ignoring AER for LUN %llu\n", be64_to_cpu(req->lun)); +"ignoring AER for LUN %u\n", scsilun_to_int(&req->lun)); if (srp_response_common(target, delta, &rsp, sizeof rsp)) shost_printk(KERN_ERR, target->scsi_host, PFX @@ -1914,7 +1914,7 @@ static int srp_queuecommand(struct Scsi_Host *shost, struct scsi_cmnd *scmnd) memset(cmd, 0, sizeof *cmd); cmd->opcode = SRP_CMD; - cmd->lun= cpu_to_be64((u64) scmnd->device->lun << 48); + int_to_scsilun(scmnd->device->lun, &cmd->lun); cmd->tag= req->index; memcpy(cmd->cdb, scmnd->cmnd, scmnd->cmd_len); @@ -2365,7 +2365,7 @@ static int srp_send_tsk_mgmt(struct srp_target_port *target, memset(tsk_mgmt, 0, sizeof *tsk_mgmt); tsk_mgmt->opcode= SRP_TSK_MGMT; - tsk_mgmt->lun = cpu_to_be64((u64) lun << 48); + int_to_scsilun(lun, &tsk_mgmt->lun); tsk_mgmt->tag = req_tag | SRP_TAG_TSK_MGMT; tsk_mgmt->tsk_mgmt_func = func; tsk_mgmt->task_tag = req_tag; diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c index fe09f27..8a52cec 100644 --- a/drivers/infiniband/ulp/srpt/ib_srpt.c +++ b/drivers/infiniband/ulp/srpt/ib_srpt.c @@ -1614,68 +1614,6 @@ enum scsi_lun_addr_method { SCSI_LUN_ADDR_METHOD_EXTENDED_LUN = 3, }; -/* - * srpt_unpack_lun() - Convert from network LUN to linear LUN. - * - * Convert an 2-byte, 4-byte, 6-byte or 8-byte LUN structure in network byte - * order (big endian) to a linear LUN. Supports three LUN addressing methods: - * peripheral, flat and logical unit. See also SAM-2, section 4.9.4 (page 40). - */ -static uint64_t srpt_unpack_lun(const uint8_t *lun, int len) -{ - uint64_t res = NO_SUCH_LUN; - int addressing_method; - - if (unlikely(len < 2)) { - printk(KERN_ERR "Illegal LUN length %d, expected 2 bytes or " - "more", len); - goto out; - } - - switch (len) { - case 8: - if ((*((__be64 *)lun) & -__constant_cpu_to_be64(0xLL)) != 0) - goto out_err; - break; - case 4: - if (*((__be16 *)&lun[2]) != 0) - goto out_err; - break; - case 6: - if
[PATCH] qla4xxx: Return -ENOMEM on memory allocation failure
In this code, 0 is returned on memory allocation failure, even though other failures return -ENOMEM or other similar values. A simplified version of the Coccinelle semantic match that finds this problem is as follows: // @@ expression ret; expression x,e1,e2,e3; identifier alloc; @@ ret = 0 ... when != ret = e1 *x = alloc(...) ... when != ret = e2 if (x == NULL) { ... when != ret = e3 return ret; } // Signed-off-by: Himangi Saraogi Acked-by: Julia Lawall --- drivers/scsi/qla4xxx/ql4_os.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c index c5d9564..72ba671 100644 --- a/drivers/scsi/qla4xxx/ql4_os.c +++ b/drivers/scsi/qla4xxx/ql4_os.c @@ -1050,6 +1050,7 @@ static int qla4xxx_get_host_stats(struct Scsi_Host *shost, char *buf, int len) if (!ql_iscsi_stats) { ql4_printk(KERN_ERR, ha, "Unable to allocate memory for iscsi stats\n"); + ret = -ENOMEM; goto exit_host_stats; } -- 1.9.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: break from queue depth adjusting loops when device found
On 14-07-03 01:21 PM, Mike Christie wrote: On 07/03/2014 12:11 PM, Christoph Hellwig wrote: On Thu, Jul 03, 2014 at 10:05:57AM -0500, Stephen M. Cameron wrote: From: Stephen M. Cameron Don't loop through all the devices even after finding the one we're looking for The comments in the code seem to indicate that we want to modify the queue depth for all LUNs on a given target. Ccing Mike and Vasu as they wrote this code. Ccing James Smart for more details. We copied this behavior from lpfc's SAM_STAT_TASK_SET_FULL handling (look at around the 2.6.32 kernel for reference). Perhaps there should be a knob to control this, maybe within the scope of a host. I have one RAID controller that has a JBOD mode in which real disks are presented as LUs in the same target. Obviously the TSF (queue_type and other) capabilities of those real disks might differ. Doug Gilbert -- 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] aic79xx: replace kmalloc/memset 0 by kzalloc
Cc: Hannes Reinecke Cc: "James E.J. Bottomley" Cc: linux-scsi@vger.kernel.org Signed-off-by: Fabian Frederick --- drivers/scsi/aic7xxx/aic79xx_core.c | 3 +-- drivers/scsi/aic7xxx/aic79xx_osm.c | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/scsi/aic7xxx/aic79xx_core.c b/drivers/scsi/aic7xxx/aic79xx_core.c index 0bcacf7..9ce383c 100644 --- a/drivers/scsi/aic7xxx/aic79xx_core.c +++ b/drivers/scsi/aic7xxx/aic79xx_core.c @@ -10437,14 +10437,13 @@ ahd_handle_en_lun(struct ahd_softc *ahd, struct cam_sim *sim, union ccb *ccb) return; } } - lstate = kmalloc(sizeof(*lstate), GFP_ATOMIC); + lstate = kzalloc(sizeof(*lstate), GFP_ATOMIC); if (lstate == NULL) { xpt_print_path(ccb->ccb_h.path); printk("Couldn't allocate lstate\n"); ccb->ccb_h.status = CAM_RESRC_UNAVAIL; return; } - memset(lstate, 0, sizeof(*lstate)); status = xpt_create_path(&lstate->path, /*periph*/NULL, xpt_path_path_id(ccb->ccb_h.path), xpt_path_target_id(ccb->ccb_h.path), diff --git a/drivers/scsi/aic7xxx/aic79xx_osm.c b/drivers/scsi/aic7xxx/aic79xx_osm.c index 69d5c43..76b5fec 100644 --- a/drivers/scsi/aic7xxx/aic79xx_osm.c +++ b/drivers/scsi/aic7xxx/aic79xx_osm.c @@ -1325,10 +1325,9 @@ int ahd_platform_alloc(struct ahd_softc *ahd, void *platform_arg) { ahd->platform_data = - kmalloc(sizeof(struct ahd_platform_data), GFP_ATOMIC); + kzalloc(sizeof(struct ahd_platform_data), GFP_ATOMIC); if (ahd->platform_data == NULL) return (ENOMEM); - memset(ahd->platform_data, 0, sizeof(struct ahd_platform_data)); ahd->platform_data->irq = AHD_LINUX_NOIRQ; ahd_lockinit(ahd); ahd->seltime = (aic79xx_seltime & 0x3) << 4; -- 1.9.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: break from queue depth adjusting loops when device found
> -Original Message- > From: Hannes Reinecke [mailto:h...@suse.de] > Sent: Friday, 04 July, 2014 5:53 AM > To: Christoph Hellwig; Stephen M. Cameron > Cc: james.bottom...@parallels.com; Elliott, Robert (Server Storage); > stephenmcame...@gmail.com; linux-scsi@vger.kernel.org; Vasu Dev; Mike > Christie > Subject: Re: [PATCH] scsi: break from queue depth adjusting loops when device > found > > On 07/03/2014 07:11 PM, Christoph Hellwig wrote: > > On Thu, Jul 03, 2014 at 10:05:57AM -0500, Stephen M. Cameron wrote: > >> From: Stephen M. Cameron > >> > >> Don't loop through all the devices even after > >> finding the one we're looking for > > > > The comments in the code seem to indicate that we want to modify > > the queue depth for all LUNs on a given target. > > > > Ccing Mike and Vasu as they wrote this code. > > > Indeed, that was the idea. > This piece of code tries to keep track of the remote port queue > depth, which isn't represented at all. > Thing is, each remote target has a target queue depth which can hold > only so many outstanding SCSI requests. If that is full it'll return > BUSY for _all_ LUNs served from that port. > And the very _next_ command after the one which filled the target > queue will get the BUSY status. > So we need to decrease the queue depth on _all_ LUNs here to avoid > starvation of individual devices. > > Hence I guess this is not the correct fix. Some SCSI target device designs work that way, but others don't. In the original SCSI Architecture Model (SAM-1; 1995): * BUSY meant "the logical unit is busy" * TASK SET FULL meant "the logical unit does not have enough resources" It didn't say "SCSI target port" or "SCSI target device", so they were not intended to be affected by other logical units. That was based on the idealized notion that each I_T_L nexus was independent, also assumed by the hierarchical LUN addressing scheme and the SCSI Controller Commands (SCC) standard. With that interpretation, the command identifier/tag (the Q in I_T_L_Q nexus) is independent for each I_T_L nexus - two logical units behind the same SCSI target port could use the same tag value at the same time for different commands. However, every SCSI transport protocol after parallel SCSI has chosen to share command tags across all logical units; passing the (usually) 8-byte LUN field in every IU/frame is not practical. So, that idealized I_T_L nexus concept has broken down. Implementations really have a mix of SCSI target device, SCSI target port, and logical unit resources. When we added the optional status qualifier to SAM-4 (2006), we added a SCOPE field indicating if the scope of the BUSY or TASK SET FULL is for: * just the logical unit; * all logical units in the SCSI target device accessible through the SCSI target port; or * all logical units in the SCSI target device. There is no VPD page field defined to report which scope(s) are implemented or describing the resource limits. Some designs are simple, others are complicated. If TASK SET FULL means a SCSI target port limit has been reached, then decrementing the limit on all the logical units will over-correct. If the limit is 256 and there are 16 LUNs, nominally 16 per LUN, and you send a 257th command, then the total will drop to 16x15=240, not 256. mpt3sas assigns separate target numbers for target ports it discovers, so the SCSI midlayer queue depth logic is more correct if those SCSI target devices are implementing a SCSI target port scope for BUSY and TASK SET FULL, but incorrect if they are implementing a logical unit scope. [2.711047] scsi 0:0:0:0: Direct-Access HP EO0400JDVFB HPD1 PQ: 0 ANSI: 6 [2.711746] scsi 0:0:1:0: Direct-Access HP EO0400JDVFB HPD1 PQ: 0 ANSI: 6 [2.712423] scsi 0:0:2:0: Direct-Access HP EO0400JDVFB HPD1 PQ: 0 ANSI: 6 hpsa presents multiple LUNs in one target, and a TASK SET FULL on one logical unit doesn't mean that you'll get it on the others, so the SCSI midlayer queue depth logic is incorrect: [ 10.790885] scsi 2:0:0:0: Direct-Access HP LOGICAL VOLUME 10.0 PQ: 0 ANSI: 5 [ 10.791095] scsi 2:0:0:1: Direct-Access HP LOGICAL VOLUME 10.0 PQ: 0 ANSI: 5 [ 10.791300] scsi 2:0:0:2: Direct-Access HP LOGICAL VOLUME 10.0 PQ: 0 ANSI: 5 The SCSI host template .cmd_per_lun value, which is used to set the default initial queue depth for each device (i.e., each LUN), implies the logical unit scope. Perhaps the SCSI midlayer should keep track of both SCSI target port and logical unit queue depths, parse the status qualifier if present, and let the host template advise on the policy to assume if the status qualifier is not present. Short of that, I think it's best to assume logical unit scope, which always the presumption in SCSI, for BUSY and TASK SET FULL. There is some code for a scsi_target structure that I don't understand and have just been ignoring: target_bus
Re: [PATCH 1/2] Use sdev_scsi2lun for SCSI parallel drivers
On 14-07-04 07:54 AM, Hannes Reinecke wrote: SCSI-2 defines only up to 256 LUNs with a flat namespace. My SCSI-2 draft (revision 10b from August 1989) only has 3 bit LUNs placed in byte 1 of the command block, in the top 3 bits. There are also things called LUNTARs and LUNTRNs at the message level. 8 bit LUNs do ring a distant bell, perhaps after SCSI-2? Doug Gilbert -- 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] qla4xxx: Return -ENOMEM on memory allocation failure
> -Original Message- > From: linux-scsi-ow...@vger.kernel.org [mailto:linux-scsi- > ow...@vger.kernel.org] On Behalf Of Himangi Saraogi > Sent: Friday, 04 July, 2014 1:28 PM > To: Vikas Chaudhary; iscsi-dri...@qlogic.com; James E.J. Bottomley; linux- > s...@vger.kernel.org; linux-ker...@vger.kernel.org > Cc: julia.law...@lip6.fr > Subject: [PATCH] qla4xxx: Return -ENOMEM on memory allocation failure > > In this code, 0 is returned on memory allocation failure, even though > other failures return -ENOMEM or other similar values. > > A simplified version of the Coccinelle semantic match that finds this > problem is as follows: > > // > @@ > expression ret; > expression x,e1,e2,e3; > identifier alloc; > @@ > > ret = 0 > ... when != ret = e1 > *x = alloc(...) > ... when != ret = e2 > if (x == NULL) { ... when != ret = e3 > return ret; > } > // > > Signed-off-by: Himangi Saraogi > Acked-by: Julia Lawall > --- > drivers/scsi/qla4xxx/ql4_os.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c > index c5d9564..72ba671 100644 > --- a/drivers/scsi/qla4xxx/ql4_os.c > +++ b/drivers/scsi/qla4xxx/ql4_os.c > @@ -1050,6 +1050,7 @@ static int qla4xxx_get_host_stats(struct Scsi_Host > *shost, char *buf, int len) > if (!ql_iscsi_stats) { > ql4_printk(KERN_ERR, ha, > "Unable to allocate memory for iscsi stats\n"); > + ret = -ENOMEM; > goto exit_host_stats; > } > Also, the only caller of this function doesn't use the return value - it's overwritten by another function call: drivers/scsi/scsi_transport_iscsi.c: err = transport->get_host_stats(shost, buf, host_stats_size); actual_size = nlmsg_total_size(sizeof(*ev) + host_stats_size); skb_trim(skbhost_stats, NLMSG_ALIGN(actual_size)); nlhhost_stats->nlmsg_len = actual_size; err = iscsi_multicast_skb(skbhost_stats, ISCSI_NL_GRP_ISCSID, GFP_KERNEL); --- Rob ElliottHP Server Storage -- 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/2] Use sdev_scsi2lun for SCSI parallel drivers
> -Original Message- > From: linux-scsi-ow...@vger.kernel.org [mailto:linux-scsi- > ow...@vger.kernel.org] On Behalf Of Douglas Gilbert > Sent: Friday, 04 July, 2014 2:44 PM > To: Hannes Reinecke; James Bottomley > Cc: Christoph Hellwig; linux-scsi@vger.kernel.org > Subject: Re: [PATCH 1/2] Use sdev_scsi2lun for SCSI parallel drivers > > On 14-07-04 07:54 AM, Hannes Reinecke wrote: > > SCSI-2 defines only up to 256 LUNs with a flat namespace. > > My SCSI-2 draft (revision 10b from August 1989) only has > 3 bit LUNs placed in byte 1 of the command block, in the top > 3 bits. There are also things called LUNTARs and LUNTRNs > at the message level. > > 8 bit LUNs do ring a distant bell, perhaps after SCSI-2? > > Doug Gilbert The hierarchical LUN structures have some limited size LUN subfields: * the peripheral device addressing method includes an 8-bit TARGET OR LUN subfield, which specifies an 8-bit LUN at the current level (if the BUS IDENTIFIER field is 0) * the logical unit addressing method has a 5-bit LUN subfield --- Rob ElliottHP Server Storage -- 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] qla4xxx: Return -ENOMEM on memory allocation failure
On Fri, 4 Jul 2014, Elliott, Robert (Server Storage) wrote: > > > > -Original Message- > > From: linux-scsi-ow...@vger.kernel.org [mailto:linux-scsi- > > ow...@vger.kernel.org] On Behalf Of Himangi Saraogi > > Sent: Friday, 04 July, 2014 1:28 PM > > To: Vikas Chaudhary; iscsi-dri...@qlogic.com; James E.J. Bottomley; linux- > > s...@vger.kernel.org; linux-ker...@vger.kernel.org > > Cc: julia.law...@lip6.fr > > Subject: [PATCH] qla4xxx: Return -ENOMEM on memory allocation failure > > > > In this code, 0 is returned on memory allocation failure, even though > > other failures return -ENOMEM or other similar values. > > > > A simplified version of the Coccinelle semantic match that finds this > > problem is as follows: > > > > // > > @@ > > expression ret; > > expression x,e1,e2,e3; > > identifier alloc; > > @@ > > > > ret = 0 > > ... when != ret = e1 > > *x = alloc(...) > > ... when != ret = e2 > > if (x == NULL) { ... when != ret = e3 > > return ret; > > } > > // > > > > Signed-off-by: Himangi Saraogi > > Acked-by: Julia Lawall > > --- > > drivers/scsi/qla4xxx/ql4_os.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c > > index c5d9564..72ba671 100644 > > --- a/drivers/scsi/qla4xxx/ql4_os.c > > +++ b/drivers/scsi/qla4xxx/ql4_os.c > > @@ -1050,6 +1050,7 @@ static int qla4xxx_get_host_stats(struct Scsi_Host > > *shost, char *buf, int len) > > if (!ql_iscsi_stats) { > > ql4_printk(KERN_ERR, ha, > >"Unable to allocate memory for iscsi stats\n"); > > + ret = -ENOMEM; > > goto exit_host_stats; > > } > > > > Also, the only caller of this function doesn't use the return > value - it's overwritten by another function call: > > drivers/scsi/scsi_transport_iscsi.c: > err = transport->get_host_stats(shost, buf, host_stats_size); > > actual_size = nlmsg_total_size(sizeof(*ev) + host_stats_size); > skb_trim(skbhost_stats, NLMSG_ALIGN(actual_size)); > nlhhost_stats->nlmsg_len = actual_size; > > err = iscsi_multicast_skb(skbhost_stats, ISCSI_NL_GRP_ISCSID, > GFP_KERNEL); Maybe that is intentional? If get_host_stats fails, eg because of a lack of memory, the worst that will happen is that a region of the buffer will be 0. If the loop is done again, there seems to be a risk of an infinite loop. julia -- 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] qla4xxx: Return -ENOMEM on memory allocation failure
On Fri, 4 Jul 2014, Julia Lawall wrote: > > > On Fri, 4 Jul 2014, Elliott, Robert (Server Storage) wrote: > > > > > > > > -Original Message- > > > From: linux-scsi-ow...@vger.kernel.org [mailto:linux-scsi- > > > ow...@vger.kernel.org] On Behalf Of Himangi Saraogi > > > Sent: Friday, 04 July, 2014 1:28 PM > > > To: Vikas Chaudhary; iscsi-dri...@qlogic.com; James E.J. Bottomley; linux- > > > s...@vger.kernel.org; linux-ker...@vger.kernel.org > > > Cc: julia.law...@lip6.fr > > > Subject: [PATCH] qla4xxx: Return -ENOMEM on memory allocation failure > > > > > > In this code, 0 is returned on memory allocation failure, even though > > > other failures return -ENOMEM or other similar values. > > > > > > A simplified version of the Coccinelle semantic match that finds this > > > problem is as follows: > > > > > > // > > > @@ > > > expression ret; > > > expression x,e1,e2,e3; > > > identifier alloc; > > > @@ > > > > > > ret = 0 > > > ... when != ret = e1 > > > *x = alloc(...) > > > ... when != ret = e2 > > > if (x == NULL) { ... when != ret = e3 > > > return ret; > > > } > > > // > > > > > > Signed-off-by: Himangi Saraogi > > > Acked-by: Julia Lawall > > > --- > > > drivers/scsi/qla4xxx/ql4_os.c | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c > > > index c5d9564..72ba671 100644 > > > --- a/drivers/scsi/qla4xxx/ql4_os.c > > > +++ b/drivers/scsi/qla4xxx/ql4_os.c > > > @@ -1050,6 +1050,7 @@ static int qla4xxx_get_host_stats(struct Scsi_Host > > > *shost, char *buf, int len) > > > if (!ql_iscsi_stats) { > > > ql4_printk(KERN_ERR, ha, > > > "Unable to allocate memory for iscsi stats\n"); > > > + ret = -ENOMEM; > > > goto exit_host_stats; > > > } > > > > > > > Also, the only caller of this function doesn't use the return > > value - it's overwritten by another function call: > > > > drivers/scsi/scsi_transport_iscsi.c: > > err = transport->get_host_stats(shost, buf, > > host_stats_size); > > > > actual_size = nlmsg_total_size(sizeof(*ev) + > > host_stats_size); > > skb_trim(skbhost_stats, NLMSG_ALIGN(actual_size)); > > nlhhost_stats->nlmsg_len = actual_size; > > > > err = iscsi_multicast_skb(skbhost_stats, > > ISCSI_NL_GRP_ISCSID, > > GFP_KERNEL); > > Maybe that is intentional? If get_host_stats fails, eg because of a lack > of memory, the worst that will happen is that a region of the buffer will > be 0. If the loop is done again, there seems to be a risk of an infinite > loop. Or one could jump out of the function completely, as on failure of alloc_skb. julia -- 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