Re: [PATCH v2 00/15] qla2xxx: Add Target Multiqueue support
Hey guys, Apologies for missing this earlier. Comments below. On Tue, 2017-06-13 at 20:47 -0700, Himanshu Madhani wrote: > Hi Martin, Nic, > > This patch series adds support for multiqueue for qla2xxx target mode driver. > > This series depends on the seris applied to Martin's scsi/for-next branch > (https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git/log/?h=for-next) > > I've also added followig patch ("qla2xxx: Include Exchange offload/Extended > Login into FW dump") which was dropped from earlier series for rework. > > I am planning to send series for FC-NVME support as well which depends on this > series, so I would like this series to be applied to scsi tree instead of > target-pending for only this submission, as both series touches common code > in qla2xxx driver. > > Please apply this series to for-next for inclusion in 4.13 merge window. > > Note: Comment from Bart for patch#1 will be addresed as new patch after our > FC-NVME series is posted. > > Changes from v1 --> v2 > > o Fixed 0-day kernel compile failure > > Thanks, > Himanshu > > Himanshu Madhani (1): > qla2xxx: Update driver version to 9.01.00.00-k > > Quinn Tran (13): > qla2xxx: Combine Active command arrays. > qla2xxx: Preparation for Target MQ. > qla2xxx: Enable Target Multi Queue > qla2xxx: Add debug knob for user control workload > qla2xxx: Add fw_started flags to qpair > qla2xxx: move fields from qla_hw_data to qla_qpair > qla2xxx: use shadow register for ISP27XX > qla2xxx: Add function call to qpair for door bell > qla2xxx: Add debug logging routine for qpair > qla2xxx: Remove unused tgt_enable_64bit_addr flag > qla2xxx: Remove datasegs_per_cmd and datasegs_per_cont field > qla2xxx: Move target stat counters from vha to qpair. > qla2xxx: Include Exchange offload/Extended Login into FW dump > > Sawan Chandak (1): > qla2xxx: Fix mailbox failure while deleting Queue pairs > > drivers/scsi/qla2xxx/qla_attr.c|4 +- > drivers/scsi/qla2xxx/qla_dbg.c | 150 ++ > drivers/scsi/qla2xxx/qla_dbg.h | 17 + > drivers/scsi/qla2xxx/qla_def.h | 119 - > drivers/scsi/qla2xxx/qla_dfs.c | 137 - > drivers/scsi/qla2xxx/qla_gbl.h | 19 +- > drivers/scsi/qla2xxx/qla_init.c| 53 +- > drivers/scsi/qla2xxx/qla_inline.h | 44 ++ > drivers/scsi/qla2xxx/qla_iocb.c| 32 +- > drivers/scsi/qla2xxx/qla_isr.c | 93 +++- > drivers/scsi/qla2xxx/qla_mid.c | 44 +- > drivers/scsi/qla2xxx/qla_os.c | 156 -- > drivers/scsi/qla2xxx/qla_target.c | 1028 > +--- > drivers/scsi/qla2xxx/qla_target.h | 50 +- > drivers/scsi/qla2xxx/qla_version.h |4 +- > drivers/scsi/qla2xxx/tcm_qla2xxx.c |8 +- > 16 files changed, 1348 insertions(+), 610 deletions(-) > If it hasn't already been merged to MKP's tree, then: Acked-by: Nicholas Bellinger Btw, I don't think my missing ACK should hold up qla2xxx patches from being merged in the future, especially ones that are exposing new qla2xxx specific functionality. The only ones that would really need my ACK are ones specific to interaction with target-core.
[PATCH] iscsi-target: Add login_keys_workaround attribute for non RFC initiators
From: Nicholas Bellinger This patch re-introduces part of a long standing login workaround that was recently dropped by: commit 1c99de981f30b3e7868b8d20ce5479fa1c0fea46 Author: Nicholas Bellinger Date: Sun Apr 2 13:36:44 2017 -0700 iscsi-target: Drop work-around for legacy GlobalSAN initiator Namely, the workaround for FirstBurstLength ended up being required by Mellanox Flexboot PXE boot ROMs as reported by Robert. So this patch re-adds the work-around for FirstBurstLength within iscsi_check_proposer_for_optional_reply(), and makes the key optional to respond when the initiator does not propose, nor respond to it. Also as requested by Arun, this patch introduces a new TPG attribute named 'login_keys_workaround' that controls the use of both the FirstBurstLength workaround, as well as the two other existing workarounds for gPXE iSCSI boot client. By default, the workaround is enabled with login_keys_workaround=1, since Mellanox FlexBoot requires it, and Arun has verified the Qlogic MSFT initiator already proposes FirstBurstLength, so it's uneffected by this re-adding this part of the original work-around. Reported-by: Robert LeBlanc Cc: Robert LeBlanc Cc: Arun Easi Signed-off-by: Nicholas Bellinger --- drivers/target/iscsi/iscsi_target_configfs.c | 2 ++ drivers/target/iscsi/iscsi_target_nego.c | 6 ++-- drivers/target/iscsi/iscsi_target_parameters.c | 41 ++ drivers/target/iscsi/iscsi_target_parameters.h | 2 +- drivers/target/iscsi/iscsi_target_tpg.c| 19 drivers/target/iscsi/iscsi_target_tpg.h| 1 + include/target/iscsi/iscsi_target_core.h | 9 ++ 7 files changed, 64 insertions(+), 16 deletions(-) diff --git a/drivers/target/iscsi/iscsi_target_configfs.c b/drivers/target/iscsi/iscsi_target_configfs.c index 535a8e0..0dd4c45 100644 --- a/drivers/target/iscsi/iscsi_target_configfs.c +++ b/drivers/target/iscsi/iscsi_target_configfs.c @@ -781,6 +781,7 @@ static int lio_target_init_nodeacl(struct se_node_acl *se_nacl, DEF_TPG_ATTRIB(t10_pi); DEF_TPG_ATTRIB(fabric_prot_type); DEF_TPG_ATTRIB(tpg_enabled_sendtargets); +DEF_TPG_ATTRIB(login_keys_workaround); static struct configfs_attribute *lio_target_tpg_attrib_attrs[] = { &iscsi_tpg_attrib_attr_authentication, @@ -796,6 +797,7 @@ static int lio_target_init_nodeacl(struct se_node_acl *se_nacl, &iscsi_tpg_attrib_attr_t10_pi, &iscsi_tpg_attrib_attr_fabric_prot_type, &iscsi_tpg_attrib_attr_tpg_enabled_sendtargets, + &iscsi_tpg_attrib_attr_login_keys_workaround, NULL, }; diff --git a/drivers/target/iscsi/iscsi_target_nego.c b/drivers/target/iscsi/iscsi_target_nego.c index 96df63f..7a6751f 100644 --- a/drivers/target/iscsi/iscsi_target_nego.c +++ b/drivers/target/iscsi/iscsi_target_nego.c @@ -864,7 +864,8 @@ static int iscsi_target_handle_csg_zero( SENDER_TARGET, login->rsp_buf, &login->rsp_length, - conn->param_list); + conn->param_list, + conn->tpg->tpg_attrib.login_keys_workaround); if (ret < 0) return -1; @@ -934,7 +935,8 @@ static int iscsi_target_handle_csg_one(struct iscsi_conn *conn, struct iscsi_log SENDER_TARGET, login->rsp_buf, &login->rsp_length, - conn->param_list); + conn->param_list, + conn->tpg->tpg_attrib.login_keys_workaround); if (ret < 0) { iscsit_tx_login_rsp(conn, ISCSI_STATUS_CLS_INITIATOR_ERR, ISCSI_LOGIN_STATUS_INIT_ERR); diff --git a/drivers/target/iscsi/iscsi_target_parameters.c b/drivers/target/iscsi/iscsi_target_parameters.c index fce6276..caab104 100644 --- a/drivers/target/iscsi/iscsi_target_parameters.c +++ b/drivers/target/iscsi/iscsi_target_parameters.c @@ -765,7 +765,8 @@ static int iscsi_check_for_auth_key(char *key) return 0; } -static void iscsi_check_proposer_for_optional_reply(struct iscsi_param *param) +static void iscsi_check_proposer_for_optional_reply(struct iscsi_param *param, + bool keys_workaround) { if (IS_TYPE_BOOL_AND(param)) { if (!strcmp(param->value, NO)) @@ -773,19 +774,31 @@ static void iscsi_check_proposer_for_optional_reply(struct iscsi_param *param) } else if (IS_TYPE_BOOL_OR(param)) { if (!strcmp(param->value, YES)) SET_PSTATE_REPLY_OPTIONAL(param); -/* - * Required for gPXE iSCSI boot client - */ - if (!strcmp(param->name, IMMEDIATEDATA)) - SET_PSTATE_REPLY_OPTIONAL(param); + + if (keys_workaround) { + /* +* Required f
RE: device support in hpasa, was: Re: OOPS from cciss_ioctl in 4.12+git
> -Original Message- > From: Hannes Reinecke [mailto:h...@suse.de] > Sent: Friday, July 07, 2017 11:05 AM > To: Jens Axboe ; Christoph Hellwig > ; Meelis Roos > Cc: Linux Kernel list ; linux- > bl...@vger.kernel.org; Don Brace ; Scott > Benesh ; Scott Teel > ; Kevin Barnett > ; linux-scsi@vger.kernel.org > Subject: Re: device support in hpasa, was: Re: OOPS from cciss_ioctl in > 4.12+git > > EXTERNAL EMAIL > > > On 07/07/2017 05:03 PM, Jens Axboe wrote: > > On 07/07/2017 09:00 AM, Christoph Hellwig wrote: > >> On Thu, Jul 06, 2017 at 12:55:04PM +0300, Meelis Roos wrote: > Also we're trying to move people away from the cciss driver, can you > check if the hpsa SCSI driver works for you as well? > >>> > >>> I have older adapter: > >>> > >>> Compaq Computer Corporation Smart Array 64xx [0e11:0046] (rev 01) > >>> > >>> That does not seem to be supported by hpsa AFAICS. > >> > >> Looks like. Although hpsa has support for various SA5 controllers > >> it seems like it decided to skip all Compaq branded controllers. > >> > >> As far as I can tell we could simply add support for those to > >> hpsa. Ccing hpsa folks to figure out if that's the case. > > > > Pretty sure Hannes had a patch he tested for that, he talked about > > that back at LSFMM earlier this year. Hannes? > > > Oh, I do. > hpsa is working happily on SLES for _all_ SmartArray controllers. > You need to enable 'hpsa_allow_any=1', though. > But I'm perfectly happy with making that the default and drop cciss > completely. > > Cheers, > > Hannes > -- > Dr. Hannes ReineckeTeamlead Storage & Networking > h...@suse.de +49 911 74053 688 > SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg > GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton > HRB 21284 (AG Nürnberg) The 6400 controllers are quite old and are no longer tested. So, it would be nice to deprecate the cciss driver, but we would not support those formerly cciss devices with hpsa. I do not recall seeing a cciss_ioctl issue, what was the issue? Thanks, Don Brace ESC - Smart Storage Microsemi Corporation
Re: device support in hpsa, was: Re: OOPS from cciss_ioctl in 4.12+git
On 07/07/2017 02:08 PM, Christoph Hellwig wrote: On Fri, Jul 07, 2017 at 11:42:38AM -0400, Laurence Oberman wrote: What happens when hpsa_allow_any=1 with the Smart Array 64xx It should probe. But only if it has a HP vendor ID as far as I can tell. We'd still need to add the compaq ids so that these controllers get probed. But maybe it's time to add them and flip the hpsa_allow_any default (maybe conditionally on a config option?) and mark cciss deprecated. Agreed, I vote yes.
Re: device support in hpsa, was: Re: OOPS from cciss_ioctl in 4.12+git
On Fri, Jul 07, 2017 at 11:42:38AM -0400, Laurence Oberman wrote: > What happens when hpsa_allow_any=1 with the Smart Array 64xx > It should probe. But only if it has a HP vendor ID as far as I can tell. We'd still need to add the compaq ids so that these controllers get probed. But maybe it's time to add them and flip the hpsa_allow_any default (maybe conditionally on a config option?) and mark cciss deprecated.
Re: [PATCH v2] scsi: sg: fix SG_DXFER_FROM_DEV transfers
On 07/07/17 09:56, Johannes Thumshirn wrote: > SG_DXFER_FROM_DEV transfers do not necessarily have a dxferp as we set > it to NULL for the old sg_io read/write interface, but must have a length > bigger than 0. This fixes a regression introduced by commit 28676d869bbb > ("scsi: sg: check for valid direction before starting the request") > I've tested this new patch and the Nero applications can still find the optical drives on my laptop. Tested-by: Chris Clayton > Signed-off-by: Johannes Thumshirn > Fixes: 28676d869bbb ("scsi: sg: check for valid direction before starting the > request") > Reported-by: Chris Clayton > Tested-by: Chris Clayton > Cc: Douglas Gilbert > Reviewed-by: Hannes Reinecke > --- > Changes to v1: > * Fix breakage of the sg_io v3 interface, verified using sg_inq > > drivers/scsi/sg.c | 5 - > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c > index 21225d62b0c1..1e82d4128a84 100644 > --- a/drivers/scsi/sg.c > +++ b/drivers/scsi/sg.c > @@ -758,8 +758,11 @@ static bool sg_is_valid_dxfer(sg_io_hdr_t *hp) > if (hp->dxferp || hp->dxfer_len > 0) > return false; > return true; > - case SG_DXFER_TO_DEV: > case SG_DXFER_FROM_DEV: > + if (hp->dxfer_len < 0) > + return false; > + return true; > + case SG_DXFER_TO_DEV: > case SG_DXFER_TO_FROM_DEV: > if (!hp->dxferp || hp->dxfer_len == 0) > return false; >
Re: [PATCH 4/5] target: user: Fix sense data handling
On 07/06/2017 11:50 PM, Nicholas A. Bellinger wrote: > Hey MNC & Co, > > On Wed, 2017-06-28 at 12:44 -0500, Mike Christie wrote: >> On 06/28/2017 12:58 AM, Damien Le Moal wrote: >>> If the user request handler completed the request with a CHECK CONDITION >>> status, tcmu_handle_completion() copies the command entry sense data >>> into the session request structure sense data. However, the sense data >>> length indicated by the field scsi_sense_length is not set and equal to >>> 0, resulting in the copy being a no-op and failure to propagate the >>> sense data back to the initiator. To fix this, properly set the session >>> command sense data length and also set the session command >>> SCF_TRANSPORT_TASK_SENSE flag to indicate that the command has valid >>> sense data. >>> >>> Signed-off-by: Damien Le Moal >>> --- >>> drivers/target/target_core_user.c | 4 +++- >>> 1 file changed, 3 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/target/target_core_user.c >>> b/drivers/target/target_core_user.c >>> index beb5f09..7426b4c 100644 >>> --- a/drivers/target/target_core_user.c >>> +++ b/drivers/target/target_core_user.c >>> @@ -831,7 +831,9 @@ static void tcmu_handle_completion(struct tcmu_cmd >>> *cmd, struct tcmu_cmd_entry * >>> entry->rsp.scsi_status = SAM_STAT_CHECK_CONDITION; >>> } else if (entry->rsp.scsi_status == SAM_STAT_CHECK_CONDITION) { >>> memcpy(se_cmd->sense_buffer, entry->rsp.sense_buffer, >>> - se_cmd->scsi_sense_length); >>> + TRANSPORT_SENSE_BUFFER); >>> + se_cmd->scsi_sense_length = TRANSPORT_SENSE_BUFFER; >>> + se_cmd->se_cmd_flags |= SCF_TRANSPORT_TASK_SENSE; >>> } else if (se_cmd->se_cmd_flags & SCF_BIDI) { >>> /* Get Data-In buffer before clean up */ >>> gather_data_area(udev, cmd, true); >>> >> >> I have a patch similar to this and 5/5 in my set: >> >> https://www.spinics.net/lists/target-devel/msg15430.html >> >> If yours gets merged first then I will build my set over them, so patch >> looks ok to me. >> >> Reviewed-by: Mike Christie > > The RFC patches above from May 31st weren't merged because I thought you > where going to send out a second series.. I am/was. Sorry for the confusion. Above, I meant if Daemien's patches gets merged before I can repost. I have been having troubles testing the usb patch and was not sure how long it would take me. I should have just broken out 1 - 6 like you just did for me below. > > https://www.spinics.net/lists/target-devel/msg15505.html > > Since that hasn't been the case, I'll go ahead and merge the bugfixes in > patches #1-6 for v4.13 now. :) Thanks. > > Please resend patches #7-13 as post v4.13 items at your earliest > convenience. > > Apologies for the confusion. > It was my fault :)
Re: device support in hpasa, was: Re: OOPS from cciss_ioctl in 4.12+git
On 07/07/2017 05:03 PM, Jens Axboe wrote: > On 07/07/2017 09:00 AM, Christoph Hellwig wrote: >> On Thu, Jul 06, 2017 at 12:55:04PM +0300, Meelis Roos wrote: Also we're trying to move people away from the cciss driver, can you check if the hpsa SCSI driver works for you as well? >>> >>> I have older adapter: >>> >>> Compaq Computer Corporation Smart Array 64xx [0e11:0046] (rev 01) >>> >>> That does not seem to be supported by hpsa AFAICS. >> >> Looks like. Although hpsa has support for various SA5 controllers >> it seems like it decided to skip all Compaq branded controllers. >> >> As far as I can tell we could simply add support for those to >> hpsa. Ccing hpsa folks to figure out if that's the case. > > Pretty sure Hannes had a patch he tested for that, he talked about > that back at LSFMM earlier this year. Hannes? > Oh, I do. hpsa is working happily on SLES for _all_ SmartArray controllers. You need to enable 'hpsa_allow_any=1', though. But I'm perfectly happy with making that the default and drop cciss completely. Cheers, Hannes -- Dr. Hannes ReineckeTeamlead Storage & Networking h...@suse.de +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG Nürnberg)
Re: device support in hpasa, was: Re: OOPS from cciss_ioctl in 4.12+git
On 07/07/2017 11:03 AM, Jens Axboe wrote: On 07/07/2017 09:00 AM, Christoph Hellwig wrote: On Thu, Jul 06, 2017 at 12:55:04PM +0300, Meelis Roos wrote: Also we're trying to move people away from the cciss driver, can you check if the hpsa SCSI driver works for you as well? I have older adapter: Compaq Computer Corporation Smart Array 64xx [0e11:0046] (rev 01) That does not seem to be supported by hpsa AFAICS. Looks like. Although hpsa has support for various SA5 controllers it seems like it decided to skip all Compaq branded controllers. As far as I can tell we could simply add support for those to hpsa. Ccing hpsa folks to figure out if that's the case. Pretty sure Hannes had a patch he tested for that, he talked about that back at LSFMM earlier this year. Hannes? What happens when hpsa_allow_any=1 with the Smart Array 64xx It should probe.
Re: device support in hpasa, was: Re: OOPS from cciss_ioctl in 4.12+git
On 07/07/2017 09:00 AM, Christoph Hellwig wrote: > On Thu, Jul 06, 2017 at 12:55:04PM +0300, Meelis Roos wrote: >>> Also we're trying to move people away from the cciss driver, can you >>> check if the hpsa SCSI driver works for you as well? >> >> I have older adapter: >> >> Compaq Computer Corporation Smart Array 64xx [0e11:0046] (rev 01) >> >> That does not seem to be supported by hpsa AFAICS. > > Looks like. Although hpsa has support for various SA5 controllers > it seems like it decided to skip all Compaq branded controllers. > > As far as I can tell we could simply add support for those to > hpsa. Ccing hpsa folks to figure out if that's the case. Pretty sure Hannes had a patch he tested for that, he talked about that back at LSFMM earlier this year. Hannes? -- Jens Axboe
device support in hpasa, was: Re: OOPS from cciss_ioctl in 4.12+git
On Thu, Jul 06, 2017 at 12:55:04PM +0300, Meelis Roos wrote: > > Also we're trying to move people away from the cciss driver, can you > > check if the hpsa SCSI driver works for you as well? > > I have older adapter: > > Compaq Computer Corporation Smart Array 64xx [0e11:0046] (rev 01) > > That does not seem to be supported by hpsa AFAICS. Looks like. Although hpsa has support for various SA5 controllers it seems like it decided to skip all Compaq branded controllers. As far as I can tell we could simply add support for those to hpsa. Ccing hpsa folks to figure out if that's the case.
Re: [PATCH 1/5] scsi: bnx2i: convert to workqueue
On 2017-07-07 09:20:02 [-0400], Chad Dupuis wrote: > What was the question? My observation is that the patch I proposed fixed > the issue we saw on testing the patch set. With that small change > (essentially modulo by the number of active CPUs vs. the total number) > your patch set worked ok. That mail at the bottom of this mail where I said why I think your patch is a nop in this context. Sebastian On 2017-05-17 17:07:34 [+0200], To Chad Dupuis wrote: > > > Sebastian, can you add this change to your patch set? > > > > Are sure that you can reliably reproduce the issue and fix it with the > > patch above? Because this patch: > > oh. Okay. Now it clicked. It can fix the issue but it is still possible, > that CPU0 goes down between your check for it and schedule_work_on() > returning. Let my think of something… Oh wait. I already thought about this: it may take bnx2fc_percpu from CPU7 and run the worker on CPU3. The job isn't lost, because the worker does: | static void bnx2fc_percpu_io_work(struct work_struct *work_s) | { | struct bnx2fc_percpu_s *p; … | p = container_of(work_s, struct bnx2fc_percpu_s, work); | | spin_lock_bh(&p->fp_work_lock); and so will access bnx2fc_percpu of CPU7 running on CPU3. So I *think* that your patch should make no difference and there should be no leak if schedule_work_on() is invoked on an offline CPU.
Re: [PATCH 1/5] scsi: bnx2i: convert to workqueue
On Fri, 7 Jul 2017, 9:14am, Sebastian Andrzej Siewior wrote: > On 2017-06-29 15:57:56 [+0200], Johannes Thumshirn wrote: > > So here we are again, > > Tested-by: Johannes Thumshirn > > > > FCoE will follow as soon as my setup can speak FCoE again. > > So it all looks good, doesn't it? Chad never responded to my question > on his patch. I still doubt that it fixes the problem he observed. > > Sebastian > What was the question? My observation is that the patch I proposed fixed the issue we saw on testing the patch set. With that small change (essentially modulo by the number of active CPUs vs. the total number) your patch set worked ok.
Re: [PATCH 1/5] scsi: bnx2i: convert to workqueue
On 2017-06-29 15:57:56 [+0200], Johannes Thumshirn wrote: > So here we are again, > Tested-by: Johannes Thumshirn > > FCoE will follow as soon as my setup can speak FCoE again. So it all looks good, doesn't it? Chad never responded to my question on his patch. I still doubt that it fixes the problem he observed. Sebastian
Re: [patch] scsi: qedi: silence sprintf() overflow warning
On Tue, Feb 07, 2017 at 02:27:09PM +0100, walter harms wrote: > > > Am 07.02.2017 14:01, schrieb Dan Carpenter: > > The problem here is this: > > > > sprintf(host_buf, "qedi_ofld%d", qedi->shost->host_no); > > > > host_buf is 16 character so we only have 6 characters left for > > ->host_no. But ->host_no is set in scsi_host_alloc(): > > > > index = ida_simple_get(&host_index_ida, 0, 0, GFP_KERNEL); > > > > It could theoretically go up to 0x800 so we need space for 10 > > digits. > > > > Signed-off-by: Dan Carpenter > > > > diff --git a/drivers/scsi/qedi/qedi_main.c b/drivers/scsi/qedi/qedi_main.c > > index 5eda21d903e9..0dcf3b08230c 100644 > > --- a/drivers/scsi/qedi/qedi_main.c > > +++ b/drivers/scsi/qedi/qedi_main.c > > @@ -1735,7 +1735,7 @@ static int __qedi_probe(struct pci_dev *pdev, int > > mode) > > u32 dp_module = 0; > > u8 dp_level = 0; > > bool is_vf = false; > > - char host_buf[16]; > > + char host_buf[20]; > > struct qed_link_params link_params; > > struct qed_slowpath_params sp_params; > > struct qed_probe_params qed_params; > > -- > > To unsubscribe from this list: send the line "unsubscribe kernel-janitors" > > in > > the body of a message to majord...@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > > > any chance to use snprintf here ? > sprintf(host_buf, "qedi_ofld%d", qedi->shost->host_no); > > or something like asprint() :) > > if ever anyone change the type to very_long_type in the future it would > simply break > but not hurt. No, I don't think that's required. There are infinite possible futures and the future you're describing is not likely. We'd just end up making the code more complicated for no reason. regards, dan carpenter
[PATCH v2] scsi: sg: fix SG_DXFER_FROM_DEV transfers
SG_DXFER_FROM_DEV transfers do not necessarily have a dxferp as we set it to NULL for the old sg_io read/write interface, but must have a length bigger than 0. This fixes a regression introduced by commit 28676d869bbb ("scsi: sg: check for valid direction before starting the request") Signed-off-by: Johannes Thumshirn Fixes: 28676d869bbb ("scsi: sg: check for valid direction before starting the request") Reported-by: Chris Clayton Tested-by: Chris Clayton Cc: Douglas Gilbert Reviewed-by: Hannes Reinecke --- Changes to v1: * Fix breakage of the sg_io v3 interface, verified using sg_inq drivers/scsi/sg.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c index 21225d62b0c1..1e82d4128a84 100644 --- a/drivers/scsi/sg.c +++ b/drivers/scsi/sg.c @@ -758,8 +758,11 @@ static bool sg_is_valid_dxfer(sg_io_hdr_t *hp) if (hp->dxferp || hp->dxfer_len > 0) return false; return true; - case SG_DXFER_TO_DEV: case SG_DXFER_FROM_DEV: + if (hp->dxfer_len < 0) + return false; + return true; + case SG_DXFER_TO_DEV: case SG_DXFER_TO_FROM_DEV: if (!hp->dxferp || hp->dxfer_len == 0) return false; -- 2.12.3
Re: [PATCH] scsi: sg: fix SG_DXFER_FROM_DEV transfers
On Thu, Jul 06, 2017 at 02:47:22PM -0400, Douglas Gilbert wrote: > Can you check your patch with one of the utilities from sg3_utils > such as sg_inq which will use SG_DXFER_FROM_DEV with the newer > interface? Correct, this patch broke sg_inq. I'll send a corrected v2. > BTW I'm not sure why dxferp is set to NULL for SG_DXFER_FROM_DEV > transfers; perhaps some magic done by the block layer. Maybe a > comment in the code (e.g. on line 654) would help. This is due to: commit fad7f01e61bf737fe8a3740d803f000db57ecac6 Author: FUJITA Tomonori Date: Tue Sep 2 16:20:20 2008 +0900 sg: set dxferp to NULL for READ with the older SG interface With the older SG interface, we don't know a user-space address to trasfer data when executing a SCSI command. So we can't pass a user-space address to blk_rq_map_user. This patch fixes sg to pass a NULL user-space address to blk_rq_map_user so that it just sets up a request and bios with page frames propely without data transfer. Signed-off-by: FUJITA Tomonori Signed-off-by: Jens Axboe > > Also sg_is_valid_dxfer() is only called once and is more complex > than it looks; so perhaps it could be inlined back in > sg_common_write(). The compiler will inline it anyways (at least the one I checked with) and inlining it into sg_common_write() won't make the code more readable IMHO. But ultimately it's your driver so if you insist I'll do. -- Johannes Thumshirn Storage jthumsh...@suse.de+49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg) Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850