Re: [PATCH] scsi: Fix a harmless double shift bug

2018-12-07 Thread Jens Axboe
On 12/7/18 8:20 PM, Martin K. Petersen wrote: > > Jens, > > This went in through your tree. Can you please pick this fix up? Yep, applied, thanks Dan. -- Jens Axboe

Re: [PATCH 00/15] lpfc updates for 12.0.0.9

2018-12-07 Thread Martin K. Petersen
James, > Update lpfc to revision 12.0.0.9 > > This patch contains lpfc bug fixes Applied to 4.21/scsi-queue, thanks! -- Martin K. Petersen Oracle Linux Engineering

Re: [PATCH] scsi: bnx2fc: Fix NULL dereference in error handling

2018-12-07 Thread Martin K. Petersen
Dan, > If "interface" is NULL then we can't release it and trying to will > only lead to an Oops. Applied to 4.20/scsi-fixes, thanks! -- Martin K. Petersen Oracle Linux Engineering

Re: [PATCH] Revert "scsi: qla2xxx: Fix NVMe Target discovery"

2018-12-07 Thread Martin K. Petersen
Himanshu, > This reverts commit db186382af21e926e90df19499475f2552192b77. > > This commit introduced regression with FCP discovery so revert it back > to fix discovery for FCP luns Applied to 4.20/scsi-fixes. -- Martin K. Petersen Oracle Linux Engineering

Re: [PATCH] scsi: Fix a harmless double shift bug

2018-12-07 Thread Martin K. Petersen
Jens, This went in through your tree. Can you please pick this fix up? > On Thu, Nov 29, 2018 at 01:37:10PM +0300, Dan Carpenter wrote: >> Smatch generates a warning: >> >> drivers/scsi/scsi_lib.c:1656 scsi_mq_done() warn: test_bit() takes a bit >> number >> >> The problem is that

Re: [PATCH 00/20] smartpqi updates

2018-12-07 Thread Feng Li
r-PQI-Config-Table-handshake > . add support for get/set controller features. > - smartpqi-add-retries-for-device-resets > . re-attempt device reset. > - smartpqi-add-no_write_same-for-logical-volumes > . turn off WRITE SAME for logical volumes. > - smartpqi-correct-host-serial-

Re: [PATCH 0/2] zfcp: small bugfix on top of previous v4.21 patches

2018-12-07 Thread Martin K. Petersen
Steffen, > One new recovery fix, which is not urgent, for an old bug. It's > sufficient to apply it on top of the previously sent 23 zfcp updates > for the v4.21 merge window Applied to 4.21/scsi-queue, thanks! -- Martin K. Petersen Oracle Linux Engineering

Re: [PATCH v7 0/5] target: user configurable T10 Vendor ID

2018-12-07 Thread Martin K. Petersen
David, > This patch-set allows for the modification of the T10 Vendor > Identification string returned in the SCSI INQUIRY response, via the > target/core/$backstore/$name/wwn/vendor_id ConfigFS path. Applied to 4.21/scsi-queue, thank you! -- Martin K. Petersen Oracle Linux Engineering

Re: [PATCH v2] qla2xxx: Split the __qla2x00_abort_all_cmds() function

2018-12-07 Thread Martin K. Petersen
Bart, > Nesting in __qla2x00_abort_all_cmds() is way too deep. Reduce the > nesting level by introducing a helper function. This patch does not > change any functionality. Applied to 4.21/scsi-queue. Thank you. -- Martin K. Petersen Oracle Linux Engineering

Re: [PATCH] scsi: csiostor: remove flush_scheduled_work()

2018-12-07 Thread Martin K. Petersen
Varun, > flush_scheduled_work() is not required as csio_hw_exit_workers() calls > cancel_work_sync() for hw->evtq_work. Applied to 4.21/scsi-queue, thanks! -- Martin K. Petersen Oracle Linux Engineering

Re: [PATCH v2 01/23] zfcp: make DIX experimental, disabled, and independent of DIF

2018-12-07 Thread Martin K. Petersen
Steffen, > Introduce separate zfcp module parameters to individually select > support for: DIF which should work (zfcp.dif, which used to be > DIF+DIX, disabled) or DIX+DIF which can cause trouble (zfcp.dix, new, > disabled). Applied to 4.21/scsi-queue. -- Martin K. Petersen Oracle

Re: [PATCH] scsi: ufs: Remove redundant sense size definition

2018-12-07 Thread Martin K. Petersen
Avri, > By spec, the ufs sense data is 18 bytes long. Applied to 4.21/scsi-queue, thanks! -- Martin K. Petersen Oracle Linux Engineering

Re: BUG in copy_page_to_iter() when iscsi sets ENABLE_CLUSTERING

2018-12-07 Thread Christoph Hellwig
Note that independent of what we do in the Linux iSCSI initiator this is a network DOS, so we'll have to fix it. On Wed, Dec 05, 2018 at 12:09:40PM -0800, Lee Duncan wrote: > I recently found what I believe is a bug, and I'd appreciate feedback > on if that is correct, and if so how to proceed. >

Re: [PATCH 01/10] gdth: refactor ioc_general

2018-12-06 Thread Finn Thain
On Thu, 6 Dec 2018, Christoph Hellwig wrote: > This function is a huge mess with duplicated error handling. Split out > a few useful helpers and use goto labels to untangle the error handling > and no-data ioctl handling. > > Signed-off-by: Christoph Hellwig > --- > drivers/scsi/gdth.c |

Re: 9305-16i fault 0x5853 (regression from 4.17.2 to 4.19.2) and EEH fence errors

2018-12-06 Thread Matt Corallo
Would be nice to be pointed to the correct place to report major regressions, if not here. Note that the same error occurs: * on 4.18.19 * on 4.19.8-rc1 * with the latest firmware (16.00.01) * on a number of other peoples' powerpc64/power9 hardware. Note that both 4.18.X and 4.19.Y will,

Re: [PATCH 03/10] gdth: remove gdth_{alloc,free}_ioctl

2018-12-06 Thread Johannes Thumshirn
On 06/12/2018 17:50, Johannes Thumshirn wrote: > Why not calling dma_alloc_coherent() directly instead of using the > pci_alloc_consistent() wrapper? Ah should've read the whole series -- Johannes ThumshirnSUSE Labs Filesystems jthumsh...@suse.de

Re: [PATCH 03/10] gdth: remove gdth_{alloc,free}_ioctl

2018-12-06 Thread Johannes Thumshirn
On 06/12/2018 16:57, Christoph Hellwig wrote: > Out of the three callers once insists on the scratch buffer, and the > others are fine with a new allocation. Switch those two to juse use > pci_alloc_consistent directly, and open code the scratch buffer > allocation in the remaining one. This

Re: [PATCH v10 4/6] scsi: ufs: Add core reset support

2018-12-06 Thread Marc Gonzalez
On 23/10/2018 06:35, Can Guo wrote: > From: Dov Levenglick > > Enables core reset support. Add full initialization of the PHY and the > controller before initializing UFS PHY and during link recovery. > > Signed-off-by: Dov Levenglick > Signed-off-by: Amit Nischal > Signed-off-by: Subhash

Re: [PATCH v7 3/5] target: add device vendor_id configfs attribute

2018-12-06 Thread Martin K. Petersen
David, > Indeed, the comment should refer to page 0x83. > @Martin: all patches in this series have now been reviewed+acked. Can > you fix the above comment (s/0x80/0x83) if/when you merge, or >should I resend the series with this fixed? I'll fix it up. -- Martin K. Petersen

Re: [PATCH v7 3/5] target: add device vendor_id configfs attribute

2018-12-06 Thread David Disseldorp
On Thu, 6 Dec 2018 15:25:42 +0300, Roman Bolshakov wrote: > > /* > > + * STANDARD and VPD page 0x80 T10 Vendor Identification > > Perhaps you meant 0x83 (Device Identification VPD page, T10 vendor ID > based designator). INQUIRY page 0x80 is Unit Serial Number. Indeed, the comment should

Re: [PATCH v7 2/5] target: consistently null-terminate t10_wwn strings

2018-12-06 Thread Roman Bolshakov
On Wed, Dec 05, 2018 at 01:18:35PM +0100, David Disseldorp wrote: > In preparation for supporting user provided vendor strings, add an extra > byte to the vendor, model and revision arrays in struct t10_wwn. This > ensures that the full INQUIRY data can be carried in the arrays along > with a

Re: [PATCH v7 3/5] target: add device vendor_id configfs attribute

2018-12-06 Thread Roman Bolshakov
On Wed, Dec 05, 2018 at 01:18:36PM +0100, David Disseldorp wrote: > The vendor_id attribute will allow for the modification of the T10 > Vendor Identification string returned in inquiry responses. Its value > can be viewed and modified via the ConfigFS path at: >

Re: [PATCH] scsi: t10-pi: Return correct ref tag when queue has no integrity profile

2018-12-06 Thread chenxiang (M)
Hi, 在 2018/12/6 20:04, John Garry 写道: On 06/12/2018 04:17, Martin K. Petersen wrote: + Bart, Had you considered to use lower_32_bits() instead of "0x"? That would to avoid that reviewers have to count the 'f'-s to verify correctness of t10_pi_ref_tag(). I hadn't. I guess I tend

Re: [PATCH v10 0/6] Support for Qualcomm UFS QMP PHY on SDM845

2018-12-06 Thread Marc Gonzalez
4/6, I have already reviewed the entire series, > and it looks good. > If adding new bindings for sdm845 needs a further review, can you separate out > just the phy patches from this series (patch 1, 2, 3 & 6), and re-send them. > We can ask Kishon if he can pull them in for

Re: [PATCH] scsi: t10-pi: Return correct ref tag when queue has no integrity profile

2018-12-06 Thread John Garry
On 06/12/2018 04:17, Martin K. Petersen wrote: + Bart, Had you considered to use lower_32_bits() instead of "0x"? That would to avoid that reviewers have to count the 'f'-s to verify correctness of t10_pi_ref_tag(). I hadn't. I guess I tend to think of lower_32_bits() as something

Re: [PATCH RFC] sr: mark the device as changed when burning a CD

2018-12-06 Thread Maurizio Lombardi
Dne 6.12.2018 v 11:34 Maurizio Lombardi napsal(a): > This is what I see when a cd burn operation completes: > This is the complete blktrace log: 11,034 0.81759 11653 D W 63488 (2a 00 00 03 3c 29 00 00 1f 00 ..) [wodim] 11,034 0.81759 11653 D

Re: [PATCH RFC] sr: mark the device as changed when burning a CD

2018-12-06 Thread Maurizio Lombardi
Hi Jens, Dne 20.6.2018 v 16:09 Jens Axboe napsal(a): > On 6/20/18 5:52 AM, Maurizio Lombardi wrote: >> Hi Jens, >> >> Dne 23.5.2018 v 16:42 Jens Axboe napsal(a): >>> On 5/23/18 3:19 AM, Maurizio Lombardi wrote: Dne 22.5.2018 v 16:47 Jens Axboe napsal(a): > It's been many years,

Re: [PATCH] scsi: t10-pi: Return correct ref tag when queue has no integrity profile

2018-12-05 Thread Martin K. Petersen
Bart, > Had you considered to use lower_32_bits() instead of "0x"? > That would to avoid that reviewers have to count the 'f'-s to verify > correctness of t10_pi_ref_tag(). I hadn't. I guess I tend to think of lower_32_bits() as something you do to pointers, not to block numbers. --

Re: DISABLE_CLUSTERING in scsi drivers

2018-12-05 Thread Lee Duncan
On 11/21/18 1:41 AM, Christoph Hellwig wrote: > Hi all, > > you in the To list maintain or wrote SCSI drivers that set the > DISABLE_CLUSTERING flag, which basically disable merges of any > bio segments. We already have the actual max_segment size limit > to say which length a segment should

Re: [PATCH] scsi: t10-pi: Return correct ref tag when queue has no integrity profile

2018-12-05 Thread Bart Van Assche
On 12/5/18 6:04 AM, Martin K. Petersen wrote: Since the return value of this function is 'u32', can the ' & 0x' be left out? Absolutely, and I almost zapped it. However, I decided to leave it to emphasize the point that the reference tag is truncated to a 32-bit value. To me, this is

Re: [PATCH] scsi: t10-pi: Return correct ref tag when queue has no integrity profile

2018-12-05 Thread Martin K. Petersen
Hi Bart, > Since the return value of this function is 'u32', can the ' & > 0x' be left out? Absolutely, and I almost zapped it. However, I decided to leave it to emphasize the point that the reference tag is truncated to a 32-bit value. To me, this is more obvious than having to

Re: [PATCH] scsi: t10-pi: Return correct ref tag when queue has no integrity profile

2018-12-04 Thread Bart Van Assche
On 12/4/18 6:31 PM, Martin K. Petersen wrote: Commit ddd0bc756983 ("block: move ref_tag calculation func to the block layer") moved ref tag calculation from SCSI to a library function. However, this change broke returning the correct ref tag for devices operating in DIF mode since these do not

Re: [PATCH v6 3/5] target: add device vendor_id configfs attribute

2018-12-04 Thread Bart Van Assche
On Tue, 2018-12-04 at 16:26 +0300, Roman Bolshakov wrote: > wrt PATCH 5 in the series. Should we allow to set vendor_id for for > pscsi? I think we should allow that. Bart.

Re: [PATCH v6 3/5] target: add device vendor_id configfs attribute

2018-12-04 Thread David Disseldorp
On Tue, 4 Dec 2018 15:13:51 +0300, Roman Bolshakov wrote: > > + /* Assume ASCII encoding. Strip any newline added from userspace. */ > > + BUILD_BUG_ON(sizeof(dev->t10_wwn.vendor) != INQUIRY_VENDOR_LEN + 1); > > + strlcpy(dev->t10_wwn.vendor, strstrip(buf), > > +

Re: [PATCH v6 3/5] target: add device vendor_id configfs attribute

2018-12-04 Thread David Disseldorp
On Tue, 4 Dec 2018 16:26:59 +0300, Roman Bolshakov wrote: > wrt PATCH 5 in the series. Should we allow to set vendor_id for for > pscsi? target_transport_configure sets t10_wwn fields for pscsi, but but > an attempt to set vendor_id will overwrite the value recieved from > scsi_device. I

Re: [PATCH v6 3/5] target: add device vendor_id configfs attribute

2018-12-04 Thread Roman Bolshakov
On Tue, Dec 04, 2018 at 03:13:51PM +0300, Roman Bolshakov wrote: > On Tue, Dec 04, 2018 at 11:12:36AM +0100, David Disseldorp wrote: > > The vendor_id attribute will allow for the modification of the T10 > > Vendor Identification string returned in inquiry responses. Its value > > can be viewed

Re: [PATCH v6 5/5] target: perform t10_wwn ID initialisation in target_alloc_device()

2018-12-04 Thread Roman Bolshakov
On Tue, Dec 04, 2018 at 11:12:38AM +0100, David Disseldorp wrote: > Initialise the t10_wwn vendor, model and revision defaults when a > device is allocated instead of when it's enabled. This ensures that > custom vendor or model strings set prior to enablement are not later > overwritten with

Re: [PATCH v6 4/5] target: remove hardcoded T10 Vendor ID in INQUIRY response

2018-12-04 Thread Roman Bolshakov
On Tue, Dec 04, 2018 at 11:12:37AM +0100, David Disseldorp wrote: > Use the value stored in t10_wwn.vendor, which defaults to "LIO-ORG", but > can be reconfigured via the vendor_id ConfigFS attribute. > > Signed-off-by: David Disseldorp > Reviewed-by: Bryant G. Ly > Reviewed-by: Lee Duncan >

Re: [PATCH v6 3/5] target: add device vendor_id configfs attribute

2018-12-04 Thread Roman Bolshakov
On Tue, Dec 04, 2018 at 11:12:36AM +0100, David Disseldorp wrote: > The vendor_id attribute will allow for the modification of the T10 > Vendor Identification string returned in inquiry responses. Its value > can be viewed and modified via the ConfigFS path at: >

Re: [PATCH v5 2/5] target: consistently null-terminate t10_wwn strings

2018-12-04 Thread Roman Bolshakov
On Sat, Dec 01, 2018 at 12:34:20AM +0100, David Disseldorp wrote: > In preparation for supporting user provided vendor strings, add an extra > byte to the vendor, model and revision arrays in struct t10_wwn. This > ensures that the full INQUIRY data can be carried in the arrays along > with a

Re: [PATCH v5 1/5] target: use consistent left-aligned ASCII INQUIRY data

2018-12-04 Thread Roman Bolshakov
On Sat, Dec 01, 2018 at 12:34:19AM +0100, David Disseldorp wrote: > spc5r17.pdf specifies: > 4.3.1 ASCII data field requirements > ASCII data fields shall contain only ASCII printable characters (i.e., > code values 20h to 7Eh) and may be terminated with one or more ASCII > null (00h)

RE: [PATCH 0/3] scsi: ufs-bsg: Add read descriptor

2018-12-03 Thread Avri Altman
Shchislowski > ; Alex Lemberg ; > Bart Van Assche ; Evan Green > ; Doug Anderson ; > Tomas Winkler ; adrian.hun...@intel.com; > Sayali Lokhande > Subject: RE: [PATCH 0/3] scsi: ufs-bsg: Add read descriptor > > A gentle ping. > > Cheers, > Avri > > > -Origi

Re: [PATCH v5 5/5] target: perform t10_wwn ID initialisation in target_alloc_device()

2018-12-03 Thread Bart Van Assche
On Sat, 2018-12-01 at 15:59 +0100, Hannes Reinecke wrote: > On 12/1/18 12:34 AM, David Disseldorp wrote: > > Initialise the t10_wwn vendor, model and revision defaults when a > > device is allocated instead of when it's enabled. This ensures that > > custom vendor or model strings set prior to

Re: DISABLE_CLUSTERING in scsi drivers

2018-12-03 Thread Finn Thain
On Mon, 3 Dec 2018, Hannes Reinecke wrote: > As I said: I need to do PIO for the last two bytes of the data buffer. > For everything else DMA works nicely, it's just the last two bytes which > might be left over in the FIFO buffer under certain circumstances. I read the driver a few times

Re: DISABLE_CLUSTERING in scsi drivers

2018-12-03 Thread Hannes Reinecke
On 12/2/18 11:13 PM, Finn Thain wrote: On Sun, 2 Dec 2018, Hannes Reinecke wrote: On 12/2/18 10:21 PM, Finn Thain wrote: On Sun, 2 Dec 2018, Hannes Reinecke wrote: Well, that lone 'kmap' is due to a quirk/errata in the datasheet; essentially we have to PIO a lone byte out of the FIFO to

Re: [PATCH v5 5/5] target: perform t10_wwn ID initialisation in target_alloc_device()

2018-12-03 Thread David Disseldorp
On Sun, 2 Dec 2018 23:22:23 +0100, David Disseldorp wrote: > > > + if (!(dev->transport->transport_flags & TRANSPORT_FLAG_PASSTHROUGH)) { > > > + strlcpy(dev->t10_wwn.vendor, "LIO-ORG", > > > + sizeof(dev->t10_wwn.vendor)); > > > + strlcpy(dev->t10_wwn.model,

Re: [PATCH v5 5/5] target: perform t10_wwn ID initialisation in target_alloc_device()

2018-12-02 Thread David Disseldorp
Hi Hannes, Thanks for the feedback... On Sat, 1 Dec 2018 15:59:25 +0100, Hannes Reinecke wrote: > On 12/1/18 12:34 AM, David Disseldorp wrote: ... > > @@ -810,6 +810,23 @@ struct se_device *target_alloc_device(struct se_hba > > *hba, const char *name) > >

Re: DISABLE_CLUSTERING in scsi drivers

2018-12-02 Thread Finn Thain
On Sun, 2 Dec 2018, Hannes Reinecke wrote: > On 12/2/18 10:21 PM, Finn Thain wrote: > > On Sun, 2 Dec 2018, Hannes Reinecke wrote: > > > > > Well, that lone 'kmap' is due to a quirk/errata in the datasheet; > > > essentially > > > we have to PIO a lone byte out of the FIFO to clear it up. > > >

Re: DISABLE_CLUSTERING in scsi drivers

2018-12-02 Thread Hannes Reinecke
On 12/2/18 10:21 PM, Finn Thain wrote: On Sun, 2 Dec 2018, Hannes Reinecke wrote: Well, that lone 'kmap' is due to a quirk/errata in the datasheet; essentially we have to PIO a lone byte out of the FIFO to clear it up. And this byte is technically still part of the SCSI data, so we need to

Re: DISABLE_CLUSTERING in scsi drivers

2018-12-02 Thread Finn Thain
On Sun, 2 Dec 2018, Hannes Reinecke wrote: > Well, that lone 'kmap' is due to a quirk/errata in the datasheet; essentially > we have to PIO a lone byte out of the FIFO to clear it up. > And this byte is technically still part of the SCSI data, so we need to > stuff it onto the end of the actual

Re: DISABLE_CLUSTERING in scsi drivers

2018-12-02 Thread Hannes Reinecke
On 11/26/18 10:46 AM, Finn Thain wrote: On Mon, 26 Nov 2018, Christoph Hellwig wrote: On Thu, Nov 22, 2018 at 09:02:13AM +1100, Finn Thain wrote: you in the To list maintain or wrote SCSI drivers that set the DISABLE_CLUSTERING flag, which basically disable merges of any bio segments. We

Re: [PATCH v5 5/5] target: perform t10_wwn ID initialisation in target_alloc_device()

2018-12-01 Thread Hannes Reinecke
On 12/1/18 12:34 AM, David Disseldorp wrote: Initialise the t10_wwn vendor, model and revision defaults when a device is allocated instead of when it's enabled. This ensures that custom vendor or model strings set prior to enablement are not later overwritten with default values. Signed-off-by:

Re: [PATCH v5 4/5] target: remove hardcoded T10 Vendor ID in INQUIRY response

2018-12-01 Thread Hannes Reinecke
On 12/1/18 12:34 AM, David Disseldorp wrote: Use the value stored in t10_wwn.vendor, which defaults to "LIO-ORG", but can be reconfigured via the vendor_id ConfigFS attribute. Signed-off-by: David Disseldorp Reviewed-by: Bryant G. Ly Reviewed-by: Lee Duncan ---

Re: [PATCH v5 3/5] target: add device vendor_id configfs attribute

2018-12-01 Thread Hannes Reinecke
On 12/1/18 12:34 AM, David Disseldorp wrote: The vendor_id attribute will allow for the modification of the T10 Vendor Identification string returned in inquiry responses. Its value can be viewed and modified via the ConfigFS path at: target/core/$backstore/$name/wwn/vendor_id "LIO-ORG" remains

Re: [PATCH v5 2/5] target: consistently null-terminate t10_wwn strings

2018-12-01 Thread Hannes Reinecke
On 12/1/18 12:34 AM, David Disseldorp wrote: In preparation for supporting user provided vendor strings, add an extra byte to the vendor, model and revision arrays in struct t10_wwn. This ensures that the full INQUIRY data can be carried in the arrays along with a null-terminator. Change a

Re: [PATCH v5 1/5] target: use consistent left-aligned ASCII INQUIRY data

2018-12-01 Thread Hannes Reinecke
On 12/1/18 12:34 AM, David Disseldorp wrote: spc5r17.pdf specifies: 4.3.1 ASCII data field requirements ASCII data fields shall contain only ASCII printable characters (i.e., code values 20h to 7Eh) and may be terminated with one or more ASCII null (00h) characters. ASCII data

Re: [PATCH v4 1/7] target: use consistent left-aligned ASCII INQUIRY data

2018-11-30 Thread Lee Duncan
On 11/30/18 4:44 AM, David Disseldorp wrote: > On Wed, 28 Nov 2018 17:23:07 -0800, Lee Duncan wrote: > >>> +* unused bytes at the end of the field (i.e., highest offset) and the >>> +* unused bytes shall be filled with ASCII space characters (20h). >>> +*/ >>> + memset([8], 0x20, 8

Re: [PATCH v4 6/7] target: remove hardcoded T10 Vendor ID in INQUIRY response

2018-11-30 Thread Bart Van Assche
On Fri, 2018-11-30 at 15:41 +0100, David Disseldorp wrote: > On Fri, 30 Nov 2018 14:17:49 +0100, David Disseldorp wrote: > > > > Where is the code that initializes dev->t10_wwn.vendor to "LIO-ORG"? Did I > > > perhaps overlook something? > > > > This is done in target_configure_device() . > >

Re: [PATCH v4 6/7] target: remove hardcoded T10 Vendor ID in INQUIRY response

2018-11-30 Thread David Disseldorp
On Fri, 30 Nov 2018 14:17:49 +0100, David Disseldorp wrote: > > Where is the code that initializes dev->t10_wwn.vendor to "LIO-ORG"? Did I > > perhaps overlook something? > > This is done in target_configure_device() . Hmm, continuing to do it there may cause a bit of confusion if vendor_id

Re: [PATCH v4 6/7] target: remove hardcoded T10 Vendor ID in INQUIRY response

2018-11-30 Thread David Disseldorp
On Thu, 29 Nov 2018 08:35:26 -0800, Bart Van Assche wrote: > Where is the code that initializes dev->t10_wwn.vendor to "LIO-ORG"? Did I > perhaps overlook something? This is done in target_configure_device() . > Additionally, why are you using strnlen() for > a string of which it is guaranteed

RE: [PATCH 0/3] scsi: ufs-bsg: Add read descriptor

2018-11-29 Thread Avri Altman
A gentle ping. Cheers, Avri > -Original Message- > From: linux-scsi-ow...@vger.kernel.org > On Behalf Of Avri Altman > Sent: Monday, November 26, 2018 11:03 AM > To: James E.J. Bottomley ; Martin K. Petersen > ; linux-scsi@vger.kernel.org > Cc: Christoph Hellwig ; Bart Van Assche > ;

Re: [PATCH v4 5/7] target: add device vendor_id configfs attribute

2018-11-29 Thread David Disseldorp
On Thu, 29 Nov 2018 08:32:47 -0800, Bart Van Assche wrote: > > + unsigned char buf[INQUIRY_VENDOR_LEN + 1]; > > + > > + if (strlen(page) > INQUIRY_VENDOR_LEN) { > > + pr_err("Emulated T10 Vendor Identification exceeds" > > + " INQUIRY_VENDOR_LEN: "

Re: [PATCH v4 3/7] target: consistently null-terminate t10_wwn.model

2018-11-29 Thread Bart Van Assche
On Thu, 2018-11-29 at 21:31 +0100, David Disseldorp wrote: > On Thu, 29 Nov 2018 08:24:38 -0800, Bart Van Assche wrote: > > On Thu, 2018-11-29 at 02:01 +0100, David Disseldorp wrote: > > > [ ... ] > > Additionally, have you considered to use strlcpy() > > instead of snprintf()? > > Happy to

Re: [PATCH v4 3/7] target: consistently null-terminate t10_wwn.model

2018-11-29 Thread David Disseldorp
On Thu, 29 Nov 2018 08:24:38 -0800, Bart Van Assche wrote: > On Thu, 2018-11-29 at 02:01 +0100, David Disseldorp wrote: > > diff --git a/drivers/target/target_core_configfs.c > > b/drivers/target/target_core_configfs.c > > index f6b1549f4142..9f49b1afd685 100644 > > ---

Re: [PATCH v4 7/7] target: use printf width specifier for t10_wwn field dumps

2018-11-29 Thread Bart Van Assche
On Thu, 2018-11-29 at 02:01 +0100, David Disseldorp wrote: > The existing for loops step over null-terminators for right-padding. > Padding can be achieved in a much simpler way using printf width > specifiers. How about squashing patches 2, 3, 4 and 7 into a single patch? I think that would make

Re: [PATCH v4 6/7] target: remove hardcoded T10 Vendor ID in INQUIRY response

2018-11-29 Thread Bart Van Assche
On Thu, 2018-11-29 at 02:01 +0100, David Disseldorp wrote: > Use the value stored in t10_wwn.vendor, which defaults to "LIO-ORG", but > can be reconfigured via the vendor_id ConfigFS attribute. > > Signed-off-by: David Disseldorp > --- > drivers/target/target_core_spc.c | 8 +--- > 1 file

Re: [PATCH v4 5/7] target: add device vendor_id configfs attribute

2018-11-29 Thread Bart Van Assche
On Thu, 2018-11-29 at 02:01 +0100, David Disseldorp wrote: > +static ssize_t target_wwn_vendor_id_show(struct config_item *item, > + char *page) > +{ > + return sprintf(page, "%s\n", _t10_wwn(item)->vendor[0]); > +} The "&" and "[0]" are superfluous in the above sprintf()

Re: [PATCH v4 4/7] target: consistently null-terminate t10_wwn.revision

2018-11-29 Thread Bart Van Assche
On Thu, 2018-11-29 at 02:01 +0100, David Disseldorp wrote: > strncpy(>t10_wwn.revision[0], > - dev->transport->inquiry_rev, 4); > + dev->transport->inquiry_rev, INQUIRY_REVISION_LEN); > + dev->t10_wwn.revision[INQUIRY_REVISION_LEN]

Re: [PATCH v4 3/7] target: consistently null-terminate t10_wwn.model

2018-11-29 Thread Bart Van Assche
On Thu, 2018-11-29 at 02:01 +0100, David Disseldorp wrote: > diff --git a/drivers/target/target_core_configfs.c > b/drivers/target/target_core_configfs.c > index f6b1549f4142..9f49b1afd685 100644 > --- a/drivers/target/target_core_configfs.c > +++ b/drivers/target/target_core_configfs.c > @@

Re: [PATCH v4 2/7] target: consistently null-terminate t10_wwn.vendor

2018-11-29 Thread Bart Van Assche
On Thu, 2018-11-29 at 02:01 +0100, David Disseldorp wrote: > - strncpy(>t10_wwn.vendor[0], "LIO-ORG", 8); > + strncpy(>t10_wwn.vendor[0], "LIO-ORG", INQUIRY_VENDOR_LEN); > + dev->t10_wwn.vendor[INQUIRY_VENDOR_LEN] = '\0'; This looks weird to me. Have you

Re: [PATCH] scsi: Fix a harmless double shift bug

2018-11-29 Thread Keith Busch
On Thu, Nov 29, 2018 at 01:37:10PM +0300, Dan Carpenter wrote: > Smatch generates a warning: > > drivers/scsi/scsi_lib.c:1656 scsi_mq_done() warn: test_bit() takes a bit > number > > The problem is that SCMD_STATE_COMPLETE is supposed to be bit number 0 > and not a mask like "(1 << 0)". It

Re: [PATCH 0/2] Two refactoring patches for the qla2xxx driver

2018-11-28 Thread Martin K. Petersen
Bart, > The two patches in this series make the qla2xxx driver source code > easier to read without changing the driver functionality. Please > consider these patches for kernel v4.21. I applied patch #1. #2 had conflicts, please rebase. Thanks! -- Martin K. Petersen Oracle Linux

Re: [PATCH v2 0/9] qedi bug fixes

2018-11-28 Thread Martin K. Petersen
Nilesh, > Please consider below patch set for next 'scsi-fixes' submission. Some of these smelled more like features than bug fixes. So I applied the series to 4.21/scsi-queue. -- Martin K. Petersen Oracle Linux Engineering

Re: [PATCH 01/23] zfcp: make DIX experimental, disabled, and independent of DIF

2018-11-28 Thread Martin K. Petersen
Steffen, As I said, I don't have a problem with having module parameters. > There's one more important thing that has performance impact: We need to > pack payload and protection data into the same queue of limited > length. So for the worst case with DIX, we have to use half the size for >

Re: [PATCH v4 7/7] target: use printf width specifier for t10_wwn field dumps

2018-11-28 Thread Lee Duncan
On 11/28/18 5:01 PM, David Disseldorp wrote: > From: Bart Van Assche > > The existing for loops step over null-terminators for right-padding. > Padding can be achieved in a much simpler way using printf width > specifiers. > > Signed-off-by: David Disseldorp > --- >

Re: [PATCH v4 6/7] target: remove hardcoded T10 Vendor ID in INQUIRY response

2018-11-28 Thread Lee Duncan
On 11/28/18 5:01 PM, David Disseldorp wrote: > Use the value stored in t10_wwn.vendor, which defaults to "LIO-ORG", but > can be reconfigured via the vendor_id ConfigFS attribute. > > Signed-off-by: David Disseldorp > --- > drivers/target/target_core_spc.c | 8 +--- > 1 file changed, 5

Re: [PATCH v4 5/7] target: add device vendor_id configfs attribute

2018-11-28 Thread Lee Duncan
On 11/28/18 5:01 PM, David Disseldorp wrote: > The vendor_id attribute will allow for the modification of the T10 > Vendor Identification string returned in inquiry responses. Its value > can be viewed and modified via the ConfigFS path at: > target/core/$backstore/$name/wwn/vendor_id > >

Re: [PATCH v4 4/7] target: consistently null-terminate t10_wwn.revision

2018-11-28 Thread Lee Duncan
On 11/28/18 5:01 PM, David Disseldorp wrote: > The pscsi_set_inquiry_info() codepath doesn't currently explicitly > null-terminate t10_wwn.revision. > Add an extra byte to the t10_wwn.model buffer and perform null string > termination in all cases. > > Signed-off-by: David Disseldorp > --- >

Re: [PATCH v4 3/7] target: consistently null-terminate t10_wwn.model

2018-11-28 Thread Lee Duncan
On 11/28/18 5:01 PM, David Disseldorp wrote: > The pscsi_set_inquiry_info() and emulate_model_alias_store() codepaths > don't currently explicitly null-terminate t10_wwn.model. > Add an extra byte to the t10_wwn.model buffer and perform null string > termination in all cases. > >

Re: [PATCH v4 2/7] target: consistently null-terminate t10_wwn.vendor

2018-11-28 Thread Lee Duncan
On 11/28/18 5:01 PM, David Disseldorp wrote: > In preparation for supporting user provided vendor strings, add an extra > byte to t10_wwn.vendor which will ensure null string termination when an > eight byte vendor string is provided by the user. > > Signed-off-by: David Disseldorp > --- >

Re: [PATCH v4 2/7] target: consistently null-terminate t10_wwn.vendor

2018-11-28 Thread Ly, Bryant
> On Nov 28, 2018, at 7:01 PM, David Disseldorp wrote: > > In preparation for supporting user provided vendor strings, add an extra > byte to t10_wwn.vendor which will ensure null string termination when an > eight byte vendor string is provided by the user. > > Signed-off-by: David

Re: [PATCH v4 1/7] target: use consistent left-aligned ASCII INQUIRY data

2018-11-28 Thread Ly, Bryant
> On Nov 28, 2018, at 7:01 PM, David Disseldorp wrote: > > spc5r17.pdf specifies: > 4.3.1 ASCII data field requirements > ASCII data fields shall contain only ASCII printable characters (i.e., > code values 20h to 7Eh) and may be terminated with one or more ASCII > null (00h) characters.

Re: [PATCH v4 1/7] target: use consistent left-aligned ASCII INQUIRY data

2018-11-28 Thread Lee Duncan
On 11/28/18 5:01 PM, David Disseldorp wrote: > spc5r17.pdf specifies: > 4.3.1 ASCII data field requirements > ASCII data fields shall contain only ASCII printable characters (i.e., > code values 20h to 7Eh) and may be terminated with one or more ASCII > null (00h) characters. > ASCII

Re: [PATCH v4 6/7] target: remove hardcoded T10 Vendor ID in INQUIRY response

2018-11-28 Thread Ly, Bryant
> On Nov 28, 2018, at 7:01 PM, David Disseldorp wrote: > > Use the value stored in t10_wwn.vendor, which defaults to "LIO-ORG", but > can be reconfigured via the vendor_id ConfigFS attribute. > > Signed-off-by: David Disseldorp > --- > drivers/target/target_core_spc.c | 8 +--- > 1 file

Re: [PATCH v4 5/7] target: add device vendor_id configfs attribute

2018-11-28 Thread Ly, Bryant
> On Nov 28, 2018, at 7:01 PM, David Disseldorp wrote: > > The vendor_id attribute will allow for the modification of the T10 > Vendor Identification string returned in inquiry responses. Its value > can be viewed and modified via the ConfigFS path at: >

Re: [PATCH v4 3/7] target: consistently null-terminate t10_wwn.model

2018-11-28 Thread Ly, Bryant
> On Nov 28, 2018, at 7:01 PM, David Disseldorp wrote: > > The pscsi_set_inquiry_info() and emulate_model_alias_store() codepaths > don't currently explicitly null-terminate t10_wwn.model. > Add an extra byte to the t10_wwn.model buffer and perform null string > termination in all cases. > >

Re: [PATCH v4 4/7] target: consistently null-terminate t10_wwn.revision

2018-11-28 Thread Ly, Bryant
> On Nov 28, 2018, at 7:01 PM, David Disseldorp wrote: > > The pscsi_set_inquiry_info() codepath doesn't currently explicitly > null-terminate t10_wwn.revision. > Add an extra byte to the t10_wwn.model buffer and perform null string > termination in all cases. > > Signed-off-by: David

Re: [PATCH v2 8/9] qedi: Move LL2 producer index processing in BH.

2018-11-28 Thread Lee Duncan
On 11/21/18 1:25 AM, Nilesh Javali wrote: > From: Manish Rangankar > > 1. Removed logic to update HW producer index in interrupt context. > 2. Update HW producer index after UIO ring and buffer gets initialized. > > Signed-off-by: Manish Rangankar > --- > drivers/scsi/qedi/qedi_main.c | 31

Re: [PATCH v2 7/9] qedi: add module param to set ping packet size

2018-11-28 Thread Lee Duncan
On 11/21/18 1:25 AM, Nilesh Javali wrote: > Default packet size is 0x400. > For jumbo packets set to 0x2400. > > Signed-off-by: Nilesh Javali > --- > drivers/scsi/qedi/qedi.h | 1 - > drivers/scsi/qedi/qedi_main.c | 13 + > 2 files changed, 9 insertions(+), 5 deletions(-) > >

Re: [PATCH v2 6/9] qedi: Add packet filter in light L2 Rx path.

2018-11-28 Thread Lee Duncan
On 11/21/18 1:25 AM, Nilesh Javali wrote: > From: Manish Rangankar > > Add packet filter to avoid unnecessary packet processing in iscsiuio. > > Signed-off-by: Manish Rangankar > --- > drivers/scsi/qedi/qedi_main.c | 24 > 1 file changed, 24 insertions(+) > > diff

Re: [PATCH v2 5/9] qedi: Check for session online before getting iSCSI TLV data.

2018-11-28 Thread Lee Duncan
On 11/21/18 1:25 AM, Nilesh Javali wrote: > From: Manish Rangankar > > The kernel panic was observed after switch side perturbation, > > BUG: unable to handle kernel NULL pointer dereference at (null) > IP: [] strcmp+0x20/0x40 > PGD 0 Oops: [#1] SMP > CPU: 8 PID: 647 Comm:

Re: [PATCH v2 3/9] qedi: Replace PAGE_SIZE with QEDI_PAGE_SIZE

2018-11-28 Thread Lee Duncan
On 11/21/18 1:25 AM, Nilesh Javali wrote: > Use QEDI_PAGE_SIZE for enablement of module on systems with 64K page size. > > Signed-off-by: Nilesh Javali > --- > drivers/scsi/qedi/qedi_main.c | 16 +--- > 1 file changed, 9 insertions(+), 7 deletions(-) > > diff --git

Re: [PATCH v2 2/9] qedi: Fix spelling mistake "OUSTANDING" -> "OUTSTANDING"

2018-11-28 Thread Lee Duncan
On 11/21/18 1:25 AM, Nilesh Javali wrote: > Fix trivial spelling mistake within macro definition. > > Signed-off-by: Nilesh Javali > --- > drivers/scsi/qedi/qedi.h | 4 ++-- > drivers/scsi/qedi/qedi_main.c | 2 +- > 2 files changed, 3 insertions(+), 3 deletions(-) > > diff --git

Re: [PATCH v2 1/9] qedi: Cleanup redundant QEDI_PAGE_SIZE macro definition

2018-11-28 Thread Lee Duncan
On 11/21/18 1:25 AM, Nilesh Javali wrote: > Remove redundant macro definition. > > Signed-off-by: Nilesh Javali > --- > drivers/scsi/qedi/qedi.h | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/scsi/qedi/qedi.h b/drivers/scsi/qedi/qedi.h > index a6f96b3..e966855 100644 > ---

Re: [PATCH v2 4/9] qedi: Allocate IRQs based on msix_cnt

2018-11-28 Thread Lee Duncan
On 11/21/18 1:25 AM, Nilesh Javali wrote: > The driver load on some systems failed with error, > [0004:01:00.5]:[qedi_request_msix_irq:2524]:8: request_irq failed. > > Allocate the IRQs based on MSIX count obtained from qed module > instead of number of queues. > > Signed-off-by: Nilesh Javali

Re: [PATCH 0/3] target: drop unneeded pi_prot_format and get_fabric_name()

2018-11-28 Thread Martin K. Petersen
David, > This patchset removes unneeded se_dev_attrib.pi_prot_format and > fabric_ops.get_fabric_name() struct members. Removal of the latter > allowed for further cleanup due to the fact that all fabric modules > except iscsi_target_mod provide matching strings for fabric_ops.name > and

Re: [PATCH 2/2] qla2xxx: Split the __qla2x00_abort_all_cmds() function

2018-11-28 Thread Madhani, Himanshu
> On Nov 27, 2018, at 3:04 PM, Bart Van Assche wrote: > > External Email > > Nesting in __qla2x00_abort_all_cmds() is way too deep. Reduce the nesting > level by introducing a helper function. This patch does not change any > functionality. > > Cc: Himanshu Madhani > Signed-off-by: Bart Van

Re: [PATCH 1/2] qla2xxx: Introduce a switch/case statement in qlt_xmit_tm_rsp()

2018-11-28 Thread Madhani, Himanshu
> On Nov 27, 2018, at 3:04 PM, Bart Van Assche wrote: > > External Email > > This patch improves code readability but does not change any > functionality. > > Cc: Himanshu Madhani > Signed-off-by: Bart Van Assche > --- > drivers/scsi/qla2xxx/qla_target.c | 14 +++--- > 1 file

Re: [PATCH] scsi: lpfc: fix block guard enablement on SLI3 adapters

2018-11-28 Thread Martin K. Petersen
Martin, > Since f44ac12f1dcc, BG enablement is tracked with the > LPFC_SLI3_BG_ENABLED bit, which is set in lpfc_get_cfgparam before > lpfc_sli_config_sli_port() is called. The bit shouldn't be cleared > before checking the feature. Based on problem analysis by David Bond. Applied to

Re: [PATCH v3 3/4] target: add device vendor_id configfs attribute

2018-11-28 Thread Bart Van Assche
On Wed, 2018-11-28 at 17:44 +0100, David Disseldorp wrote: > Hi Bart, > > On Wed, 28 Nov 2018 08:36:19 -0800, Bart Van Assche wrote: > > > Maybe I'm missing something, but why is zeroing of unused bytes in these > > functions > > necessary? Would the following be correct if all strings in

  1   2   3   4   5   6   7   8   9   10   >