Re: [PATCH 1/6] blk-sysfs: Add 'chunk_sectors' to sysfs attributes

[Christoph Hellwig]

On Tue, Jul 19, 2016 at 03:20:34PM +0200, Hannes Reinecke wrote:
> The queue limits already have a 'chunk_sectors' setting, so
> we should be presenting it via sysfs.
> 
> Signed-off-by: Hannes Reinecke 

Looks fine for 4.8 independent of any ZBC implications:

Reviewed-by: Christoph Hellwig 
--
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/6] block: Introduce BLKPREP_DONE

[Christoph Hellwig]

> +++ b/block/blk-core.c
> @@ -2462,9 +2462,13 @@ struct request *blk_peek_request(struct request_queue 
> *q)
>  
>   rq = NULL;
>   break;
> - } else if (ret == BLKPREP_KILL || ret == BLKPREP_INVALID) {
> + } else if (ret == BLKPREP_KILL || ret == BLKPREP_INVALID ||
> +ret == BLKPREP_DONE) {
>   int err = (ret == BLKPREP_INVALID) ? -EREMOTEIO : -EIO;
>  
> + if (ret == BLKPREP_DONE)
> + err = 0;
> +

Please just use a proper switch statement for the return values.

static void blk_prep_end_request(struct request *req, int error)
{
rq->cmd_flags |= REQ_QUIET;
blk_start_request(rq);
__blk_end_request_all(rq, error);
}

struct request *blk_peek_request(struct request_queue *q)
{

...


switch (ret) {
...
case BLKPREP_KILL:
blk_prep_end_request(rq, -EIO);
break;
case BLKPREP_INVALID:
blk_prep_end_request(rq, -EREMOTEIO);
break;
case BLKPREP_DONE:
blk_prep_end_request(rq, 0);
break;
...
}
--
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/3] block: add flag for single-threaded submission

[Christoph Hellwig]

On Tue, Jul 19, 2016 at 04:02:56PM +0200, Hannes Reinecke wrote:
> Some devices (most notably SMR drives) support only
> one I/O stream eg for ensuring ordered I/O submission.
> This patch adds a new block queue flag
> 'BLK_QUEUE_SINGLE' to support these devices.

We'll need a blk-mq implementation of this flag as well before it can
be merged.
--
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 6/6] block: Add 'BLK_MQ_RQ_QUEUE_DONE' return value

[Christoph Hellwig]

On Tue, Jul 19, 2016 at 03:20:39PM +0200, Hannes Reinecke wrote:
> Add a return value BLK_MQ_RQ_QUEUE_DONE to terminate a request
> without error.

NAK.  You can just do a blk_mq_end_request on the request and return
0 from ->queue_rq.
--
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

Delivery notification..(View the attachment for confirmation of your delivery address)

[FedEx Courier Service]

FedEx-Delivery Post.docx
Description: MS-Word 2007 document

Re: [PATCH v2] scsi:libsas: fix oops caused by assigning a freed task to ->lldd_task

[Martin K. Petersen]

> "Wei" == Wei Fang  writes:

Wei> A freed task has been assigned to ->lldd_task when
Wei> lldd_execute_task() failed in sas_ata_qc_issue(), and access of
Wei> ->lldd_task will cause an oops:

Applied to 4.8/scsi-queue.

-- 
Martin K. Petersen  Oracle Linux Engineering
--
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] fnic: pci_dma_mapping_error() doesn't return an error code

[Martin K. Petersen]

> "Dan" == Dan Carpenter  writes:

Dan> pci_dma_mapping_error() returns true on error and false on success.
Dan> Fixes: fd6ddfa4c1dd ('fnic: check pci_map_single() return value')

Applied to 4.8/scsi-queue.

-- 
Martin K. Petersen  Oracle Linux Engineering
--
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] scsi: lpfc: avoid harmless comparison warning

[Martin K. Petersen]

> "Arnd" == Arnd Bergmann  writes:

Arnd> When building with -Wextra, we get a lot of warnings for the lpfc
Arnd> driver concerning expressions that are always true, starting with:

Applied to 4.8/scsi-queue.

-- 
Martin K. Petersen  Oracle Linux Engineering
--
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] fcoe: implement FIP VLAN responder

[Martin K. Petersen]

> "Hannes" == Hannes Reinecke  writes:

Hannes> When running in VN2VN mode there is no central instance which
Hannes> would send out any FIP VLAN discovery notifications. So this
Hannes> patch adds a new sysfs attribute 'fip_vlan_responder' which will
Hannes> activate a FIP VLAN discovery responder.

Applied to 4.8/scsi-queue.

-- 
Martin K. Petersen  Oracle Linux Engineering
--
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] lpfc: call lpfc_sli_validate_fcp_iocb() with the hbalock held

[Martin K. Petersen]

> "Johannes" == Johannes Thumshirn  writes:

Johannes> Call lpfc_sli_validate_fcp_iocb() with the hbalock held, as
Johannes> the pointer to iocbq is not guaranteed to still be valid after
Johannes> looking it up.

Applied to 4.8/scsi-queue.

-- 
Martin K. Petersen  Oracle Linux Engineering
--
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] fcoe: Rename 'fip_frame' to 'fip_vn2vn_notify_frame'

[Martin K. Petersen]

> "Hannes" == Hannes Reinecke  writes:

Hannes> Do not use a generic name to avoid confusions with other usages.

Applied to 4.8/scsi-queue.

-- 
Martin K. Petersen  Oracle Linux Engineering
--
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 v9] ibmvscsis: Initial commit of IBM VSCSI Tgt Driver

[Nicholas A. Bellinger]

On Wed, 2016-07-20 at 15:26 -0500, Bryant G. Ly wrote:
> Hi Nic, 
> 
> < SNIP >
> 
> > Note the deletion of drivers/scsi/libsrp.c + include/scsi/libsrp.h has
> > been dropped from the above commit, as it looks like they where changes
> > specific to your local tree.
> 
> > Please have a quick look, and let me know if anything doesn't look
> > right.
> 
> So the deletion is from James suggesting that I revert the deletion of libsrp 
> to make
> It clear that the code used to exist in the kernel and that im bringing it 
> back for this driver.
> Thus yeah you are right to not include it since those files don’t exist 
> currently.
> 

Thanks for the confirmation.  It should get picked up for today's
linux-next build by SFR & Co, so let's see if any build or merge issues
come up.

--
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: [net-next v2 0/6] common library for Chelsio drivers

[David Miller]

From: "Nicholas A. Bellinger" 
Date: Wed, 20 Jul 2016 14:13:55 -0700

> DaveM, if you'd prefer to pick it up, I'm happy to drop it.

There were review comments and changes requested.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 5/7] iscsi-target: replace iscsi_initiatorname_tolower() with strtolower()

[Markus Mayer]

On 20 July 2016 at 14:17, Nicholas A. Bellinger  wrote:
> On Wed, 2016-07-20 at 14:16 -0700, Nicholas A. Bellinger wrote:
>> On Fri, 2016-07-08 at 15:43 -0700, Markus Mayer wrote:
>> > After introducing generic strtolower(), iscsi_initiatorname_tolower() is
>> > no longer needed.
>> >
>> > Signed-off-by: Markus Mayer 
>> > ---
>> >  drivers/target/iscsi/iscsi_target_nego.c | 17 +
>> >  1 file changed, 1 insertion(+), 16 deletions(-)
>> >
>> > diff --git a/drivers/target/iscsi/iscsi_target_nego.c 
>> > b/drivers/target/iscsi/iscsi_target_nego.c
>> > index 89d34bd..fa20638 100644
>> > --- a/drivers/target/iscsi/iscsi_target_nego.c
>> > +++ b/drivers/target/iscsi/iscsi_target_nego.c
>> > @@ -987,21 +987,6 @@ static int iscsi_target_do_login(struct iscsi_conn 
>> > *conn, struct iscsi_login *lo
>> > return 0;
>> >  }
>> >
>> > -static void iscsi_initiatorname_tolower(
>> > -   char *param_buf)
>> > -{
>> > -   char *c;
>> > -   u32 iqn_size = strlen(param_buf), i;
>> > -
>> > -   for (i = 0; i < iqn_size; i++) {
>> > -   c = _buf[i];
>> > -   if (!isupper(*c))
>> > -   continue;
>> > -
>> > -   *c = tolower(*c);
>> > -   }
>> > -}
>> > -
>> >  /*
>> >   * Processes the first Login Request..
>> >   */
>> > @@ -1075,7 +1060,7 @@ int iscsi_target_locate_portal(
>> >  * RFC-3720 3.2.6.1. section c) that says that iSCSI IQNs
>> >  * are NOT case sensitive.
>> >  */
>> > -   iscsi_initiatorname_tolower(i_buf);
>> > +   strtolower(i_buf);
>> >
>> > if (!s_buf) {
>> > if (!login->leading_connection)
>>
>> Applied to target-pending/for-next.
>>
>> Thanks Markus.
>>
>
> Er, my bad.  It's not a stand-alone patch..

No, it's not. I was just about to point that out. Patch 1 is taking a
bit of time to get straightened out, so it hasn't been applied yet.

> In that case for the iscsi-target part:
>
> Acked-by: Nicholas Bellinger 

Thanks. I'll add the ACK to my tree.

-Markus
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 5/7] iscsi-target: replace iscsi_initiatorname_tolower() with strtolower()

[Nicholas A. Bellinger]

On Wed, 2016-07-20 at 14:16 -0700, Nicholas A. Bellinger wrote:
> On Fri, 2016-07-08 at 15:43 -0700, Markus Mayer wrote:
> > After introducing generic strtolower(), iscsi_initiatorname_tolower() is
> > no longer needed.
> > 
> > Signed-off-by: Markus Mayer 
> > ---
> >  drivers/target/iscsi/iscsi_target_nego.c | 17 +
> >  1 file changed, 1 insertion(+), 16 deletions(-)
> > 
> > diff --git a/drivers/target/iscsi/iscsi_target_nego.c 
> > b/drivers/target/iscsi/iscsi_target_nego.c
> > index 89d34bd..fa20638 100644
> > --- a/drivers/target/iscsi/iscsi_target_nego.c
> > +++ b/drivers/target/iscsi/iscsi_target_nego.c
> > @@ -987,21 +987,6 @@ static int iscsi_target_do_login(struct iscsi_conn 
> > *conn, struct iscsi_login *lo
> > return 0;
> >  }
> >  
> > -static void iscsi_initiatorname_tolower(
> > -   char *param_buf)
> > -{
> > -   char *c;
> > -   u32 iqn_size = strlen(param_buf), i;
> > -
> > -   for (i = 0; i < iqn_size; i++) {
> > -   c = _buf[i];
> > -   if (!isupper(*c))
> > -   continue;
> > -
> > -   *c = tolower(*c);
> > -   }
> > -}
> > -
> >  /*
> >   * Processes the first Login Request..
> >   */
> > @@ -1075,7 +1060,7 @@ int iscsi_target_locate_portal(
> >  * RFC-3720 3.2.6.1. section c) that says that iSCSI IQNs
> >  * are NOT case sensitive.
> >  */
> > -   iscsi_initiatorname_tolower(i_buf);
> > +   strtolower(i_buf);
> >  
> > if (!s_buf) {
> > if (!login->leading_connection)
> 
> Applied to target-pending/for-next.
> 
> Thanks Markus.
> 

Er, my bad.  It's not a stand-alone patch..

In that case for the iscsi-target part:

Acked-by: Nicholas Bellinger 


--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 5/7] iscsi-target: replace iscsi_initiatorname_tolower() with strtolower()

[Nicholas A. Bellinger]

On Fri, 2016-07-08 at 15:43 -0700, Markus Mayer wrote:
> After introducing generic strtolower(), iscsi_initiatorname_tolower() is
> no longer needed.
> 
> Signed-off-by: Markus Mayer 
> ---
>  drivers/target/iscsi/iscsi_target_nego.c | 17 +
>  1 file changed, 1 insertion(+), 16 deletions(-)
> 
> diff --git a/drivers/target/iscsi/iscsi_target_nego.c 
> b/drivers/target/iscsi/iscsi_target_nego.c
> index 89d34bd..fa20638 100644
> --- a/drivers/target/iscsi/iscsi_target_nego.c
> +++ b/drivers/target/iscsi/iscsi_target_nego.c
> @@ -987,21 +987,6 @@ static int iscsi_target_do_login(struct iscsi_conn 
> *conn, struct iscsi_login *lo
>   return 0;
>  }
>  
> -static void iscsi_initiatorname_tolower(
> - char *param_buf)
> -{
> - char *c;
> - u32 iqn_size = strlen(param_buf), i;
> -
> - for (i = 0; i < iqn_size; i++) {
> - c = _buf[i];
> - if (!isupper(*c))
> - continue;
> -
> - *c = tolower(*c);
> - }
> -}
> -
>  /*
>   * Processes the first Login Request..
>   */
> @@ -1075,7 +1060,7 @@ int iscsi_target_locate_portal(
>* RFC-3720 3.2.6.1. section c) that says that iSCSI IQNs
>* are NOT case sensitive.
>*/
> - iscsi_initiatorname_tolower(i_buf);
> + strtolower(i_buf);
>  
>   if (!s_buf) {
>   if (!login->leading_connection)

Applied to target-pending/for-next.

Thanks Markus.

--
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: [net-next v2 0/6] common library for Chelsio drivers

[Nicholas A. Bellinger]

Hi Varun & Co,

On Sat, 2016-07-16 at 22:49 +0530, Varun Prakash wrote:
> Hi,
> 
>  This patch series adds common library module(libcxgb.ko)
>  for Chelsio drivers to remove duplicate code.
> 
>  This series moves common iSCSI DDP Page Pod manager
>  code from cxgb4.ko to libcxgb.ko, earlier this code
>  was used by only cxgbit.ko now it is used by
>  three Chelsio iSCSI drivers cxgb3i, cxgb4i, cxgbit.
> 
>  In future this module will have common connection
>  management and hardware specific code that can
>  be shared by multiple Chelsio drivers(cxgb4,
>  csiostor, iw_cxgb4, cxgb4i, cxgbit).
> 
>  Please review.
> 
>  Thanks
> 
> -v2
> - updated CONFIG_CHELSIO_LIB to an invisible option
> - changed libcxgb.ko module license from GPL to Dual BSD/GPL
> 
> Varun Prakash (6):
>   libcxgb: add library module for Chelsio drivers
>   cxgb3i,cxgb4i,libcxgbi: remove iSCSI DDP support
>   cxgb4i,libcxgbi: add iSCSI DDP support
>   cxgb3i: add iSCSI DDP support
>   libcxgb: export ppm release and tagmask set api
>   cxgb3i,cxgb4i: fix symbol not declared sparse warning
> 
>  drivers/net/ethernet/chelsio/Kconfig   |  16 +-
>  drivers/net/ethernet/chelsio/Makefile  |   1 +
>  drivers/net/ethernet/chelsio/cxgb4/Makefile|   1 -
>  drivers/net/ethernet/chelsio/libcxgb/Makefile  |   3 +
>  .../{cxgb4/cxgb4_ppm.c => libcxgb/libcxgb_ppm.c}   |  57 +-
>  .../{cxgb4/cxgb4_ppm.h => libcxgb/libcxgb_ppm.h}   |  38 +-
>  drivers/scsi/cxgbi/Makefile|   2 +
>  drivers/scsi/cxgbi/cxgb3i/Kbuild   |   1 +
>  drivers/scsi/cxgbi/cxgb3i/Kconfig  |   1 +
>  drivers/scsi/cxgbi/cxgb3i/cxgb3i.c | 164 +++--
>  drivers/scsi/cxgbi/cxgb4i/Kbuild   |   1 +
>  drivers/scsi/cxgbi/cxgb4i/Kconfig  |   1 +
>  drivers/scsi/cxgbi/cxgb4i/cxgb4i.c | 203 +++---
>  drivers/scsi/cxgbi/libcxgbi.c  | 734 
> +++--
>  drivers/scsi/cxgbi/libcxgbi.h  | 188 +-
>  drivers/target/iscsi/cxgbit/Kconfig|   2 +-
>  drivers/target/iscsi/cxgbit/Makefile   |   1 +
>  drivers/target/iscsi/cxgbit/cxgbit.h   |   2 +-
>  drivers/target/iscsi/cxgbit/cxgbit_main.c  |   2 +
>  19 files changed, 552 insertions(+), 866 deletions(-)
>  create mode 100644 drivers/net/ethernet/chelsio/libcxgb/Makefile
>  rename drivers/net/ethernet/chelsio/{cxgb4/cxgb4_ppm.c => 
> libcxgb/libcxgb_ppm.c} (85%)
>  rename drivers/net/ethernet/chelsio/{cxgb4/cxgb4_ppm.h => 
> libcxgb/libcxgb_ppm.h} (84%)
> 

AFAICT this has not been picked up by DaveM for net-next code, so I've
included it in target-pending/for-next.

DaveM, if you'd prefer to pick it up, I'm happy to drop it.

--
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: Re [PATCH RFC]: LIO seems to use SCSI Allocation Length instead of SPDTL for calculating ResidualCount

[Nicholas A. Bellinger]

Hi Sumit,

Apologies for the delayed response.  Comments below..

On Sun, 2016-07-03 at 19:24 +0530, Sumit Rai wrote:
> > From: Sumit Rai 
> > Subject: LIO seems to use SCSI Allocation Length instead of SPDTL for 
> > calculating ResidualCount
> > Date: June 22, 2016 at 7:43:50 PM GMT+5:30
> > To: target-de...@vger.kernel.org
> > 
> > We are trying to test if LIO target sets ResidualCount correctly in case of 
> > ResidualOverflow for Data-In by following the below Steps:
> > 1. Send SCSI Inquiry command to iSCSI target, if there are no exceptions in 
> > the response (and no residual overflow) we are able to determine how many 
> > bytes the target SCSI layer on target side attempted to transfer to target 
> > iSCSI layer. On the initiator side when response is received: in the 
> > absence of any exceptions or residual overflow we assume all the data was 
> > successfully transferred and is equal to iSCSI DataSegmentLength. We will 
> > use this value as SPDTL for SCSI Inquiry command.
> > 
> > Note: SPDTL is detailed in RFC 7143 11.4.5.2 (and you may also refer to 
> > discussion on T10 mailing list: 
> > http://www.t10.org/pipermail/t10/2014-June/017367.html).
> > 
> > 2. Send out the same SCSI inquiry command as in Step 1. but set iSCSI EDTL 
> > to a value lower than SPDTL.
> > 
> > Assumption: Since we are not making any changes to iSCSI target LU 
> > configuration in our setup b/w Step 1. and 2, we assume amount of Inquiry 
> > Data target generates will be same of 1. and 2. since inquiry command is 
> > the same. During our testing we find this assumption to be true.
> > 
> > Expected Result:
> > Since in Step 2, SPDTL > EDTL, iSCSI target is expected to set 
> > ResidualOverflow flag and ResidualCount to SPDTL - EDTL.
> > In the attached PCAP text file, frame no. 11 (SCSI Inquiry Req.) and 12 
> > (Inquiry Response) are for Step 1. As explained, SPDTL = 0x24 or 36D.
> > Frame no. 14 is for (SCSI Inq. Req.) Step 2. EDTL = 0x08.
> > Hence, expected ResidualCount = 0x1C (28D) i.e. 0x24 (36D) - 0x08 (8D)
> > 
> > Observed Result:
> > iSCSI target sets the Residual Overflow flag correctly but value of 
> > ResidualCount doesn’t match expected value.
> > Target is setting, ResidualCount to 0x3f8 (1016D) as seen in frame no 16. 
> > In the frame no. 14, in the SCSI Inquiry CDB allocation length (SPC 5r01 
> > Section 4.2.5.6) was set to 0x0400 (1024D) and target seems to be using 
> > this value to calculate ResidualCount i.e. 0x0400 (1024D) - 0x08.
> > To further verify this we  changed the allocation length value in SCSI 
> > Inquiry CDB to 0x0200 (512D) bytes instead, we get ResidualCount of 0x1f8 
> > (504D).
> > 
> > Allocation Length is the maximum value of the SPDTL and not substitute for 
> > it, hence it shouldn’t be used to calculate ResidualCount except for cases 
> > where SPDTL > Allocation Length and Data is truncated (in that case both 
> > Alloc Len and SPDTL are same). (SPC 5r01 Section 4.2.5.6).
> > 
> > LIO Version: Datera Inc. iSCSI Target v4.1.0
> > Linux Kernel: 4.4.0-22-generic
> > 
> > RFC 7143:
> > 
> > 11.4.5.2.  Residuals Concepts Overview
> > 
> > 
> >   "SCSI-Presented Data Transfer Length (SPDTL)" is the term this
> >   document uses (see 
> > Section 2.2
> > for definition) to represent the
> >   aggregate data length that the target SCSI layer attempts to transfer
> >   using the local iSCSI layer for a task.  "Expected Data Transfer
> > 
> >   Length (EDTL)" is the iSCSI term that represents the length of data
> >   that the iSCSI layer expects to transfer for a task.  EDTL is
> >   specified in the SCSI Command PDU.
> > 
> >   When SPDTL = EDTL for a task, the target iSCSI layer completes the
> >   task with no residuals.  Whenever SPDTL differs from EDTL for a task,
> >   that task is said to have a residual.
> > 
> >   If SPDTL > EDTL for a task, iSCSI Overflow MUST be signaled in the
> >   SCSI Response PDU as specified in 
> > Section 11.4.5.1
> > .  The Residual
> >   Count MUST be set to the numerical value of (SPDTL - EDTL).
> > 
> >   If SPDTL < EDTL for a task, iSCSI Underflow MUST be signaled in the
> >   SCSI Response PDU as specified in 
> > Section 11.4.5.1
> > .  The Residual
> >   Count MUST be set to the numerical value of (EDTL - SPDTL).
> > 
> >   Note that the Overflow and Underflow scenarios are independent of
> >   Data-In and Data-Out.  Either scenario is logically possible in
> > 
> > 
> > SPC 5r01
> > 4.2.5.6 Allocation length
> > 
> > The ALLOCATION LENGTH field specifies the maximum number of bytes or blocks 
> > that an application client has allocated in the Data-In Buffer. The 
> > ALLOCATION LENGTH field specifies bytes unless a different requirement is 
> > stated in the command definition.
> > 
> > An allocation length of zero specifies that no data shall be transferred. 
> > This condition shall not be considered an error.
> > 
> > The device server shall terminate transfers to the Data-In Buffer when the 
> > 

Re: [PATCH v3 0/7] lib: string: add functions to case-convert strings

[Markus Mayer]

On Wed, Jul 13, 2016 at 03:52:38PM -0700, Markus Mayer wrote:
> On Sat, Jul 09, 2016 at 09:11:05PM -0700, Markus Mayer wrote:
>> On 9 July 2016 at 20:13, Chris Metcalf  wrote:
>>> On 7/8/2016 6:43 PM, Markus Mayer wrote:

 This series introduces a family of generic string case conversion
 functions. This kind of functionality is needed in several places in
 the kernel. Right now, everybody seems to be implementing their own
 copy of this functionality.

 Based on the discussion of the previous version of this series[1] and
 the use cases found in the kernel, it does look like having several
 flavours of case conversion functions is beneficial. The use cases fall
 into three categories:
  - copying a string and converting the case while specifying a
maximum length to mimic strlcpy()
  - copying a string and converting the case without specifying a
length to mimic strcpy()
  - converting the case of a string in-place (i.e. modifying the
string that was passed in)

 Consequently, I am proposing these new functions:
  void strlcpytoupper(char *dst, const char *src, size_t len);
  void strlcpytolower(char *dst, const char *src, size_t len);
  void strcpytoupper(char *dst, const char *src);
  void strcpytolower(char *dst, const char *src);
  void strtoupper(char *s);
  void strtolower(char *s);
>>>
>>>
>>> You may want to read the article here:
>>>
>>> https://lwn.net/Articles/659214/
>> 
>> I'll read that. Thanks.
> 
> It doesn't look like there is going to be the danger of "mass changes". 
> So far, I have two ACKs (one where the semantics doesn't change,
> because it's using strtolower()) and the other in a driver in staging.
> 
> But I understand the concern and will keep an eye out if there are
> other ACKs.
>  
>>> and follow up some of the discussion threads on LKML about the best
>>> semantics to advertise for the strlcpy/strscpy variants.  It might
>>> be helpful to return some kind of overflow/truncation error from
>>> your copy functions so people can error-check the result.
>> 
>> I am inclined to agree. However, everybody has been telling me that
>> these functions should be void. Originally they weren't.
> 
> What about something like this?  It might also work to keep the four
> static inline functions as "void" (since they won't ever return E2BIG
> anyway) and just have strlcpyto* return an integer (since that's where
> the buffer could be too small).
> 
> Rasmus, what's your take? 

Ping. Any thoughts on this proposal? Does it make sense for me to sent out a
new revision of the patch set incorporating these changes -- at least for
the strlcpyto* functions?

Thanks,
-Markus
 
> diff --git a/include/linux/string.h b/include/linux/string.h
> index ae82d13..6cc85dc 100644
> --- a/include/linux/string.h
> +++ b/include/linux/string.h
> @@ -116,8 +116,8 @@ extern void * memchr(const void *,int,__kernel_size_t);
>  #endif
>  void *memchr_inv(const void *s, int c, size_t n);
>  char *strreplace(char *s, char old, char new);
> -extern void strlcpytoupper(char *dst, const char *src, size_t len);
> -extern void strlcpytolower(char *dst, const char *src, size_t len);
> +extern int strlcpytoupper(char *dst, const char *src, size_t len);
> +extern int strlcpytolower(char *dst, const char *src, size_t len);
>  
>  extern void kfree_const(const void *x);
>  
> @@ -175,38 +175,46 @@ static inline const char *kbasename(const char *path)
>   * strcpytoupper - Copy string and convert to uppercase.
>   * @dst: The buffer to store the result.
>   * @src: The string to convert to uppercase.
> + *
> + * Returns the number of characters copied.
>   */
> -static inline void strcpytoupper(char *dst, const char *src)
> +static inline int strcpytoupper(char *dst, const char *src)
>  {
> - strlcpytoupper(dst, src, ~(size_t)0);
> + return strlcpytoupper(dst, src, ~(size_t)0);
>  }
>  
>  /**
>   * strcpytolower - Copy string and convert to lowercase.
>   * @dst: The buffer to store the result.
>   * @src: The string to convert to lowercase.
> + *
> + * Returns the number of characters copied.
>   */
> -static inline void strcpytolower(char *dst, const char *src)
> +static inline int strcpytolower(char *dst, const char *src)
>  {
> - strlcpytolower(dst, src, ~(size_t)0);
> + return strlcpytolower(dst, src, ~(size_t)0);
>  }
>  
>  /**
>   * strtoupper - Convert string to uppercase.
>   * @s: The string to operate on.
> + *
> + * Returns the number of characters copied.
>   */
> -static inline void strtoupper(char *s)
> +static inline int strtoupper(char *s)
>  {
> - strlcpytoupper(s, s, ~(size_t)0);
> + return strlcpytoupper(s, s, ~(size_t)0);
>  }
>  
>  /**
>   * strtolower - Convert string to lowercase.
>   * @s: The string to operate on.
> + *
> + * Returns the number of characters copied.
>   */
> -static inline void 

Re: [PATCH] tcm_fc: set and unset FCP_SPPF_TARG_FCN

[Nicholas A. Bellinger]

On Tue, 2016-07-19 at 15:01 +0200, Hannes Reinecke wrote:
> When registering and unregistering as an target port we should
> be setting the FC-4 service params correctly.
> 
> Signed-off-by: Hannes Reinecke 
> ---
>  drivers/target/tcm_fc/tfc_sess.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/target/tcm_fc/tfc_sess.c 
> b/drivers/target/tcm_fc/tfc_sess.c
> index f5186a7..6ffbb60 100644
> --- a/drivers/target/tcm_fc/tfc_sess.c
> +++ b/drivers/target/tcm_fc/tfc_sess.c
> @@ -91,6 +91,7 @@ static void ft_tport_delete(struct ft_tport *tport)
>  
>   ft_sess_delete_all(tport);
>   lport = tport->lport;
> + lport->service_params &= ~FCP_SPPF_TARG_FCN;
>   BUG_ON(tport != lport->prov[FC_TYPE_FCP]);
>   RCU_INIT_POINTER(lport->prov[FC_TYPE_FCP], NULL);
>  
> @@ -110,6 +111,7 @@ void ft_lport_add(struct fc_lport *lport, void *arg)
>  {
>   mutex_lock(_lport_lock);
>   ft_tport_get(lport);
> + lport->service_params |= FCP_SPPF_TARG_FCN;
>   mutex_unlock(_lport_lock);
>  }
>  

Applied to target-pending/for-next.

Thanks Hannes.


--
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] iscsi-target: fix panic when add the second TCP connection to iSCSI session

[Nicholas A. Bellinger]

Hi Feng & Co,

Apologies for the delayed follow-up.

On Tue, 2016-07-19 at 10:10 +0800, Feng Li wrote:
> Okay, thanks.
> 
> Anyone could help review this patch and merge into upstream?
> 

Applied to target-pending/for-next, with a CC' to 3.14+ stable.

Thanks Feng + Sumit for tracking this down.



--
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 v9] ibmvscsis: Initial commit of IBM VSCSI Tgt Driver

[Bryant G. Ly]

Hi Nic, 

< SNIP >

> Note the deletion of drivers/scsi/libsrp.c + include/scsi/libsrp.h has
> been dropped from the above commit, as it looks like they where changes
> specific to your local tree.

> Please have a quick look, and let me know if anything doesn't look
> right.

So the deletion is from James suggesting that I revert the deletion of libsrp 
to make
It clear that the code used to exist in the kernel and that im bringing it back 
for this driver.
Thus yeah you are right to not include it since those files don’t exist 
currently.

-Bryant

--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html




--
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 v9] ibmvscsis: Initial commit of IBM VSCSI Tgt Driver

[Nicholas A. Bellinger]

Hi Michael, Bryant & Co,

Apologies for the extended follow-up over the summer holiday.

Comments below.

On Tue, 2016-06-28 at 17:05 -0500, Michael Cyr wrote:
> From: "Bryant G. Ly" 
> 
> This driver is a pick up of the old IBM VIO scsi Target Driver
> that was started by Nick and Fujita 2-4 years ago.
> http://comments.gmane.org/gmane.linux.scsi/90119
> 
> The driver provides a virtual SCSI device on IBM Power Servers.
> 
> This patch contains the fifth version for an initial merge of the
> tcm ibmvscsis driver. More information on this driver and config
> can be found:
> 
> https://github.com/powervm/ibmvscsis/wiki/Configuration
> http://www.linux-iscsi.org/wiki/IBM_vSCSI
> 
> Signed-off-by: Steven Royer 
> Signed-off-by: Tyrel Datwyler 
> Signed-off-by: Michael Cyr 
> Signed-off-by: Bryant G. Ly 
> ---
> Version 9:
> - Fixed issues raised by Christoph Hellwig.
> 
> Version 8:
> - Badly formed patch, ignore.
> 
> Version 7:
> - Removed old module from drivers/scsi/ibmvscsi/Makefile
> - Fixed styling from Joe Perches comments.
> 
> Version 6:
> - Removed modification of report luns
> - fixed Maintainers file
> - removed #include 
> 
> Version 5:
> - changed to use scsilun_to_int
> - removed inquiry modification
> - changed to use target_alloc_session
> - removed shutdown_session, etc
> 
> Version 4:
> - Changed some print statements to dev_err.
> -Also changed to use target_alloc_session instead of manually coding it.
> - Removed scsi_cmnd and scsi_host bits removed from libsrp to completely
> - Stripped out un-needed includes.
> - Added pre-allocation of commands before IO starts.
> - Added Support for Transport event, fast-fail support, MESSAGE_IN_CRQ format
> - Changed the way queues are handled for better performance, and state mgmt.
> 
> Version 3:
> - Revert old libsrp and make it clear resurrection old vscsi target driver
> - Made libsrp a linked file to the ibmvscsis module
> 
> Version 2:
> -Fixed comments from Bart/Joe in regards to styling and code structure
> 
> 
>  MAINTAINERS  |   10 +-
>  drivers/scsi/Kconfig |   27 +-
>  drivers/scsi/Makefile|2 +-
>  drivers/scsi/ibmvscsi/ibmvfc.h   |2 +-
>  drivers/scsi/ibmvscsi/ibmvscsi.h |2 +-
>  drivers/scsi/ibmvscsi_tgt/Makefile   |4 +
>  drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 4087 
> ++
>  drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.h |  346 ++
>  drivers/scsi/ibmvscsi_tgt/libsrp.c   |  427 +++
>  drivers/scsi/ibmvscsi_tgt/libsrp.h   |  123 +
>  drivers/scsi/libsrp.c|  447 ---
>  include/scsi/libsrp.h|   78 -
>  {drivers/scsi/ibmvscsi => include/scsi}/viosrp.h |   13 +-
>  13 files changed, 5020 insertions(+), 548 deletions(-)
>  create mode 100644 drivers/scsi/ibmvscsi_tgt/Makefile
>  create mode 100644 drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
>  create mode 100644 drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.h
>  create mode 100644 drivers/scsi/ibmvscsi_tgt/libsrp.c
>  create mode 100644 drivers/scsi/ibmvscsi_tgt/libsrp.h
>  delete mode 100644 drivers/scsi/libsrp.c
>  delete mode 100644 include/scsi/libsrp.h
>  rename {drivers/scsi/ibmvscsi => include/scsi}/viosrp.h (92%)

This -v9 patch has been applied to target-pending/for-next here:

https://git.kernel.org/cgit/linux/kernel/git/nab/target-pending.git/commit/?h=for-next=88a678bbc34cecce36bf2c7a8af4cba38f9f77ce

Note the deletion of drivers/scsi/libsrp.c + include/scsi/libsrp.h has
been dropped from the above commit, as it looks like they where changes
specific to your local tree.

Please have a quick look, and let me know if anything doesn't look
right.

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] target: Unsupported SCSI Opcode Fix

[Nicholas A. Bellinger]

On Thu, 2016-06-30 at 13:50 -0500, Bryant G. Ly wrote:
> If a Simple command is sent with a failure,
> target_setup_cmd_from_cdb returns with TCM_UNSUPPORTED_SCSI_OPCODE
> or TCM_INVALID_CDB_FIELD. So in the cases where target_setup_cmd_from_cdb
> returns an error, we never get far enough to call target_execute_cmd to
> increment simple_cmds. Since simple_cmds isn't incremented, the
> result of the failure from target_setup_cmd_from_cdb causes
> transport_generic_request_failure to decrement
> simple_cmds, due to call to transport_complete_task_attr.
> With this dev->simple_cmds or dev->dev_ordered_sync is now -1, not 0.
> So when a subsequent command with an Ordered Task is sent, it causes
> a hang, since dev->simple_cmds is at -1.
> 
> Signed-off-by: Michael Cyr 
> Signed-off-by: Bryant G. Ly 
> ---
>  drivers/target/target_core_transport.c | 6 ++
>  include/target/target_core_base.h  | 3 ++-
>  2 files changed, 8 insertions(+), 1 deletion(-)
> 

Applied to target-pending/for-next, preceeding the ibmvscsis commit.

Thanks again Bryant + Michael for reviewing + testing this patch.

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [dm-devel] dm-mq and end_clone_request()

[Bart Van Assche]

On 07/20/2016 07:27 AM, Mike Snitzer wrote:

On Wed, Jul 20 2016 at 10:08am -0400,
Mike Snitzer  wrote:
[ ... ]
diff --git a/drivers/md/dm-rq.c b/drivers/md/dm-rq.c
index 7a96618..347ff25 100644
--- a/drivers/md/dm-rq.c
+++ b/drivers/md/dm-rq.c
@@ -414,7 +414,7 @@ static void dm_complete_request(struct request *rq, int 
error)
if (!rq->q->mq_ops)
blk_complete_request(rq);
else
-   blk_mq_complete_request(rq, error);
+   blk_mq_complete_request(rq, 0);
 }

 /*


Hello Mike,

Thanks for the patch. But unfortunately even with this patch applied I 
see fio reporting I/O errors when run on top of dm-mq after a 
(simulated) cable pull. I think these errors occur because 
dm_softirq_done() fails requests if clone == NULL and tio->error != 
NULL. Adding WARN_ON_ONCE(error && !tio->clone) in dm_complete_request() 
produced the following call stack:


Workqueue: kblockd blk_mq_run_work_fn
Call Trace:
 [] dump_stack+0x68/0xa1
 [] __warn+0xc6/0xe0
 [] warn_slowpath_null+0x18/0x20
 [] dm_complete_request+0x8a/0xb0 [dm_mod]
 [] map_request+0x70/0x2e0 [dm_mod]
 [] dm_mq_queue_rq+0x7d/0x120 [dm_mod]
 [] __blk_mq_run_hw_queue+0x1da/0x350
 [] blk_mq_run_work_fn+0x10/0x20
 [] process_one_work+0x1f9/0x6a0
 [] worker_thread+0x49/0x490
 [] kthread+0xea/0x100
 [] ret_from_fork+0x1f/0x40

Please let me know if you need more information.

Thanks,

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: dm-mq and end_clone_request()

[Mike Snitzer]

On Wed, Jul 20 2016 at  1:37pm -0400,
Bart Van Assche  wrote:

> On 07/20/2016 07:27 AM, Mike Snitzer wrote:
> >On Wed, Jul 20 2016 at 10:08am -0400,
> >Mike Snitzer  wrote:
> >[ ... ]
> >diff --git a/drivers/md/dm-rq.c b/drivers/md/dm-rq.c
> >index 7a96618..347ff25 100644
> >--- a/drivers/md/dm-rq.c
> >+++ b/drivers/md/dm-rq.c
> >@@ -414,7 +414,7 @@ static void dm_complete_request(struct request *rq, int 
> >error)
> > if (!rq->q->mq_ops)
> > blk_complete_request(rq);
> > else
> >-blk_mq_complete_request(rq, error);
> >+blk_mq_complete_request(rq, 0);
> > }
> >
> > /*
> 
> Hello Mike,
> 
> Thanks for the patch. But unfortunately even with this patch applied
> I see fio reporting I/O errors when run on top of dm-mq after a
> (simulated) cable pull. I think these errors occur because
> dm_softirq_done() fails requests if clone == NULL and tio->error !=
> NULL. Adding WARN_ON_ONCE(error && !tio->clone) in
> dm_complete_request() produced the following call stack:
> 
> Workqueue: kblockd blk_mq_run_work_fn
> Call Trace:
>  [] dump_stack+0x68/0xa1
>  [] __warn+0xc6/0xe0
>  [] warn_slowpath_null+0x18/0x20
>  [] dm_complete_request+0x8a/0xb0 [dm_mod]
>  [] map_request+0x70/0x2e0 [dm_mod]
>  [] dm_mq_queue_rq+0x7d/0x120 [dm_mod]
>  [] __blk_mq_run_hw_queue+0x1da/0x350
>  [] blk_mq_run_work_fn+0x10/0x20
>  [] process_one_work+0x1f9/0x6a0
>  [] worker_thread+0x49/0x490
>  [] kthread+0xea/0x100
>  [] ret_from_fork+0x1f/0x40
> 
> Please let me know if you need more information.

Cable pull testing isn't new.  I'd have thought it covered (albeit
simulated) via mptest.  Does mptest pass for you?
https://github.com/snitm/mptest

Last I knew your tests were all passing with dm-mq.  Would love to
understand how/when they have regressed.

If clone is NULL then the request wasn't actually issued to (or even
allocated from the tag space of) the underlying blk-mq request_queue
(via clone_and_map_rq's call to __multipath_map).

The original request could've been previously cloned, failed due to
cable pull, it began retry via requeue, but on retry
blk_mq_alloc_request() could've failed in __multipath_map() -- (maybe
now the request_queue was "dying"?).  Or __multipath_map() failed for
some other reason.

Would be interesting to know the error returned from map_request()'s
ti->type->clone_and_map_rq().  Really should just be DM_MAPIO_REQUEUE.
But the stack you've provided shows map_request calling
dm_complete_request(), which implies dm_kill_unmapped_request() is being
called due to ti->type->clone_and_map_rq() returning < 0.

Could be that __multipath_map() is returning an error even before it
would've otherwise called blk_mq_alloc_request().

You said you are using queue_if_no_path?  Would be interesting to see
where __multipth_map is returning with an error.

Could be that __multipath_map() should respond differently if
blk_mq_alloc_request() fails and the queue is dying -- at a minimum the
path should be failed.  But before we explore that we need to understand
why the clone_and_map_rq() is returning < 0.
--
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 v2] libata-scsi: fix read-only bits checking in ata_mselect_*()

[Tejun Heo]

So, just reverted this patch.

On Wed, Jul 20, 2016 at 06:59:23AM +0800, tom.t...@gmail.com wrote:
> From: Tom Yan 
> 
> Commit 7780081c1f04 ("libata-scsi: Set information sense field for
> invalid parameter") changed how ata_mselect_*() make sure read-only
> bits are not modified. The new implementation introduced a bug that
> the read-only bits in the byte that has a changeable bit will not
> be checked.
> 
> Added the necessary check, with comments explaining the heuristic.
> 
> Signed-off-by: Tom Yan 
> 
> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index 06afe63..b47c3ce 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
> @@ -3617,8 +3617,18 @@ static int ata_mselect_caching(struct ata_queued_cmd 
> *qc,
>*/
>   ata_msense_caching(dev->id, mpage, false);
>   for (i = 0; i < CACHE_MPAGE_LEN - 2; i++) {
> - if (i == 0)
> - continue;
> + /* Check the first byte */
> + if (i == 0) {
> + /* except the WCE bit */
> + if (mpage[i + 2] & 0xfb != buf[i] & 0xfb) {

This not only triggered compiler warning but is actually wrong.  The
above is

mpage[i + 1] & (0xfb != buf[i]) & 0xfb

which is non-sensical.  So, this patch wasn't tested at all.  Not even
compile test.  Please don't do this.

Thanks.

-- 
tejun
--
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 v2] libata-scsi: fix read-only bits checking in ata_mselect_*()

[Tejun Heo]

On Wed, Jul 20, 2016 at 06:59:23AM +0800, tom.t...@gmail.com wrote:
> From: Tom Yan 
> 
> Commit 7780081c1f04 ("libata-scsi: Set information sense field for
> invalid parameter") changed how ata_mselect_*() make sure read-only
> bits are not modified. The new implementation introduced a bug that
> the read-only bits in the byte that has a changeable bit will not
> be checked.
> 
> Added the necessary check, with comments explaining the heuristic.
> 
> Signed-off-by: Tom Yan 

Applied to libata/for-4.8.

This thread was confusing to look at.  Please use the "n/N" sequence
tagging for the patches (the -n option of git-format-patch).

Thanks.

-- 
tejun
--
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 v4] libata-scsi: better style in ata_msense_*()

[Tejun Heo]

On Wed, Jul 20, 2016 at 04:39:28AM +0800, tom.t...@gmail.com wrote:
> From: Tom Yan 
> 
> `changeable` is the "version" of mode page requested by the user.
> It will be less confusing/misleading if we do not check it
> "together" with the setting bits of the drive.
> 
> Not to mention that we currently have ata_mselect_*() implemented
> in a way that each of them will serve exclusively a particular bit
> on each page. The old style will hence make the condition look even
> more unnecessarily arcane if the ata_msense_*() is reflecting more
> than one bit.
> 
> Signed-off-by: Tom Yan 

Applied to libata/for-4.8.

Thanks.

-- 
tejun
--
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 v4] libata-scsi: minor cleanup in ata_mselect_*()

[Tejun Heo]

On Wed, Jul 20, 2016 at 05:11:50AM +0800, tom.t...@gmail.com wrote:
> From: Tom Yan 
> 
> 1. Removed a repeated bit masking in ata_mselect_control()
> 2. Moved `wce`/`d_sense` assignment below the page validity checks
> 3. Added/Removed empty lines where appropriate
> 
> Signed-off-by: Tom Yan 

Applied to libata/for-4.8.

Thanks.

-- 
tejun
--
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

GOOD DAY!!!

[a . victima . lara]

I am Mr.Saeed Bin Salem Executive Director and Chief Financial Officer of the 
National Commercial Bank Libya.I have a secured business suggestion for you 
reply me on my email: saeedbi...@qq.com
--
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/5] sd: Implement new RESET_WP provisioning mode

[Hannes Reinecke]

On 07/20/2016 02:49 AM, Damien Le Moal wrote:
> Hi Hannes,
> 
> On 7/19/16 22:25, Hannes Reinecke wrote:
>> We can map the RESET WRITE POINTER command onto a 'discard'
>> request.
>>
>> Signed-off-by: Hannes Reinecke 
>> ---
>>  drivers/scsi/sd.c | 65
>> ---
>>  drivers/scsi/sd.h |  1 +
>>  2 files changed, 53 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
>> index 249ea81..52dda83 100644
>> --- a/drivers/scsi/sd.c
>> +++ b/drivers/scsi/sd.c
>> @@ -369,6 +369,7 @@ static const char *lbp_mode[] = {
>>  [SD_LBP_WS16]= "writesame_16",
>>  [SD_LBP_WS10]= "writesame_10",
>>  [SD_LBP_ZERO]= "writesame_zero",
>> +[SD_ZBC_RESET_WP]= "reset_wp",
>>  [SD_LBP_DISABLE]= "disabled",
>>  };
>>
>> @@ -391,6 +392,13 @@ provisioning_mode_store(struct device *dev,
>> struct device_attribute *attr,
>>  if (!capable(CAP_SYS_ADMIN))
>>  return -EACCES;
>>
>> +if (sdkp->zoned == 1) {
>> +if (!strncmp(buf, lbp_mode[SD_ZBC_RESET_WP], 20)) {
>> +sd_config_discard(sdkp, SD_ZBC_RESET_WP);
>> +return count;
>> +}
>> +return -EINVAL;
>> +}
>>  if (sdp->type != TYPE_DISK)
>>  return -EINVAL;
>>
>> @@ -683,6 +691,11 @@ static void sd_config_discard(struct scsi_disk
>> *sdkp, unsigned int mode)
>>  q->limits.discard_zeroes_data = sdkp->lbprz;
>>  break;
>>
>> +case SD_ZBC_RESET_WP:
>> +max_blocks = sdkp->unmap_granularity;
>> +q->limits.discard_zeroes_data = 1;
>> +break;
>> +
>>  case SD_LBP_ZERO:
>>  max_blocks = min_not_zero(sdkp->max_ws_blocks,
>>(u32)SD_MAX_WS10_BLOCKS);
> 
> I am still wondering if setting discard_zeroes_data to 1 is the right
> choice here since nothing will happen for conventional zones (no
> zeroing, no reset, nothing). discard_zeroes_data=0 may be a safer
> choice, even though I have not hit any issue with it set to 1.
> 
This setting needs to be reviewed once hchs zero-out patches are in.
Thing is, we need to properly differentiate between 'discard' and
'zero-out'. Unfortunately ATM the libata stack only implements a
translation for 'write_same', which is then mapped onto DSM TRIM.
So the 'write_discard_zeroes' is even incorrect for current libata usage.

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)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: dm-mq and end_clone_request()

[Mike Snitzer]

On Wed, Jul 20 2016 at 10:08am -0400,
Mike Snitzer  wrote:

> On Tue, Jul 19 2016 at  6:57pm -0400,
> Bart Van Assche  wrote:
> 
> > Hello Mike,
> > 
> > If I run a fio data integrity test against kernel v4.7-rc7 then I
> > see often that fio reports I/O errors if a path is removed despite
> > queue_if_no_path having been set in /etc/multipath.conf. Further
> > analysis showed that this happens because during SCSI device removal
> > a SCSI device enters state SDEV_CANCEL before the block layer queue
> > is marked as "dying". In that state I/O requests submitted to that
> > SCSI device are failed with -EIO. The behavior for
> > end_clone_request() in drivers/md/dm.c for such requests is as
...
> > - With multiqueue support enabled, pass the "error" argument to
> >   dm_complete_request().
> 
> The error arg is passed to dm_complete_request() regardless of queue
> type but it is only immediately used by the blk-mq API (via
> blk_mq_complete_request).
> 
> > Shouldn't end_clone_request() requeue failed requests in both cases
> > instead of passing the I/O error to the submitter only if multiqueue
> > is enabled?
> 
> Pretty sure you'll find it is _not_ blk-mq that is passing the error
> up.  (But if I'm proven wrong that will be welcomed news).
>
> The error passed to dm_complete_request() is always used to set
> tio->error which is later used by dm_done().  DM core handles errors
> later via softirq in dm_done() -- where the error is passed into the
> target_type's rq_end_io hook.
> 
> So in DM multipath you'll see do_end_io() we do finally act on the error
> we got from the lower layer.  And if the error is -EIO, noretry_error()
> will return true and -EIO will be returned up the IO stack.

For some reason I thought -EIO was considered not retryable.  That's
obviously wrong (e.g. noretry_error() doesn't seize on -EIO).

> In the end we're relying on SCSI to properly categorize the underlying
> faults as retryable vs not -- via SCSI's differentiated IO errors.
> 
> Unfortunately I'm not seeing anything that DM multipath can do
> differently here.  -EIO is _always_ propagated up.
> 
> It is strange that all the dm-mq testing that has been done didn't ever
> catch this.  The mptest testsuite is a baseline for validating DM
> multipath (and request-based DM core) changes.  But I've also had Red
> Hat's QE hammer dm-mq with heavy IO (in terms of the "dt" utility) on a
> larger NetApp testbed in the face of regular controller faults.
> 
> Must be this scenario of SDEV_CANCEL is a race that is relatively
> unique/rare to your testbed?
> 
> This raises the question: should SCSI be returning something other than
> -EIO for this case?  E.g. an error that is retryable?

So it must be that blk-mq is somehow returning -EIO earlier based on
rq->errors that is established during blk_mq_complete_request().

Please try this patch (not happy with it since it assumes all
request-based DM targets will handle IO errors -- which is fine for now
since DM multipath is the only one).  Could be you've already tried
this?  Does it fix your problem?

diff --git a/drivers/md/dm-rq.c b/drivers/md/dm-rq.c
index 7a96618..347ff25 100644
--- a/drivers/md/dm-rq.c
+++ b/drivers/md/dm-rq.c
@@ -414,7 +414,7 @@ static void dm_complete_request(struct request *rq, int 
error)
if (!rq->q->mq_ops)
blk_complete_request(rq);
else
-   blk_mq_complete_request(rq, error);
+   blk_mq_complete_request(rq, 0);
 }
 
 /*
--
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 v5 4/8] char: rpmb: provide a user space interface

[Paul Gortmaker]

[RE: [PATCH v5 4/8] char: rpmb: provide a user space interface] On 20/07/2016 
(Wed 09:02) Winkler, Tomas wrote:

> > 
> > On Mon, Jul 18, 2016 at 4:27 PM, Tomas Winkler 
> > wrote:
> > > The user space API is achieved via two synchronous IOCTL.
> > > Simplified one, RPMB_IOC_REQ_CMD, were read result cycles is
> > performed
> > > by the framework on behalf the user and second, RPMB_IOC_SEQ_CMD
> > where
> > > the whole RPMB sequence including RESULT_READ is supplied by the caller.
> > > The latter is intended for  easier adjusting  of the  applications
> > > that use MMC_IOC_MULTI_CMD ioctl.
> > >
> > > Signed-off-by: Tomas Winkler 
> > > ---
> > 
> > [...]
> > 
> > > diff --git a/drivers/char/rpmb/Kconfig b/drivers/char/rpmb/Kconfig
> > > index c5e6e909efce..6794be9fcc5e 100644
> > > --- a/drivers/char/rpmb/Kconfig
> > > +++ b/drivers/char/rpmb/Kconfig
> > > @@ -6,3 +6,10 @@ config RPMB
> > >   access RPMB partition.
> > >
> > >   If unsure, select N.
> > > +
> > > +config RPMB_INTF_DEV
> > > +   bool "RPMB character device interface /dev/rpmbN"
> > 
> > A bool Kconfig should ideally
> > 
> > > +   depends on RPMB
> > > +   help
> > > + Say yes here if you want to access RPMB from user space
> > > + via character device interface /dev/rpmb%d
> > > diff --git a/drivers/char/rpmb/Makefile b/drivers/char/rpmb/Makefile
> > > index 812b3ed264c0..b5dc087b1299 100644
> > > --- a/drivers/char/rpmb/Makefile
> > > +++ b/drivers/char/rpmb/Makefile
> > > @@ -1,4 +1,5 @@
> > >  obj-$(CONFIG_RPMB) += rpmb.o
> > >  rpmb-objs += core.o
> > > +rpmb-$(CONFIG_RPMB_INTF_DEV) += cdev.o
> 
> This is not a builtin, this is an optional part of the module 

Object files that are not stand-alone, but linked into a larger object
to create a module generally still don't need module.h if they
themselves specifically aren't registering the module or similar.

The exception is a module that doesn't actively do anything but export
symbols (i.e. a library).  Then somewhere in it there needs to be a
MODULE_LICENSE so that the kernel can audit GPL and taint etc.

> 
> > > +#include 
> > 
> > not use module.h or any MODULE_ macros from within it.
> 
> Can be dropped in this case as no macros are used, 
> but the pattern Kconfig bool -> no include module.h  you are following has 
> false positive cases.

Yes, of course there are other things, like the exception table
searches, and symbol_get / symbol_put, macros defining string
lengths, etc... and I try to watch for those via inspection and
build testing. 

However the majority bool->module.h are there for two reasons:

1) in core code we had to use module.h in the past when export.h
   did not exist.

2) in driver code that largely capitalizes on copying from other
   drivers without explicitly considering modularity.

...and it is worth our while to clean both up, I think.

If you can think of specific false positives that I might not be
aware of, please don't hesitate to call them out.

Thanks,
Paul.
--

> 
> Thanks
> Tomas
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: dm-mq and end_clone_request()

[Mike Snitzer]

On Tue, Jul 19 2016 at  6:57pm -0400,
Bart Van Assche  wrote:

> Hello Mike,
> 
> If I run a fio data integrity test against kernel v4.7-rc7 then I
> see often that fio reports I/O errors if a path is removed despite
> queue_if_no_path having been set in /etc/multipath.conf. Further
> analysis showed that this happens because during SCSI device removal
> a SCSI device enters state SDEV_CANCEL before the block layer queue
> is marked as "dying". In that state I/O requests submitted to that
> SCSI device are failed with -EIO. The behavior for
> end_clone_request() in drivers/md/dm.c for such requests is as
> follows:
> - With multiqueue support disabled, call __blk_put_request() and ignore
>   the "error" argument passed to end_clone_request().

The __blk_put_request() isn't contributing to this blk-mq problem.  The
need for it is unique to the request_fn case.

> - With multiqueue support enabled, pass the "error" argument to
>   dm_complete_request().

The error arg is passed to dm_complete_request() regardless of queue
type but it is only immediately used by the blk-mq API (via
blk_mq_complete_request).

> Shouldn't end_clone_request() requeue failed requests in both cases
> instead of passing the I/O error to the submitter only if multiqueue
> is enabled?

Pretty sure you'll find it is _not_ blk-mq that is passing the error
up.  (But if I'm proven wrong that will be welcomed news).

The error passed to dm_complete_request() is always used to set
tio->error which is later used by dm_done().  DM core handles errors
later via softirq in dm_done() -- where the error is passed into the
target_type's rq_end_io hook.

So in DM multipath you'll see do_end_io() we do finally act on the error
we got from the lower layer.  And if the error is -EIO, noretry_error()
will return true and -EIO will be returned up the IO stack.

In the end we're relying on SCSI to properly categorize the underlying
faults as retryable vs not -- via SCSI's differentiated IO errors.

Unfortunately I'm not seeing anything that DM multipath can do
differently here.  -EIO is _always_ propagated up.

It is strange that all the dm-mq testing that has been done didn't ever
catch this.  The mptest testsuite is a baseline for validating DM
multipath (and request-based DM core) changes.  But I've also had Red
Hat's QE hammer dm-mq with heavy IO (in terms of the "dt" utility) on a
larger NetApp testbed in the face of regular controller faults.

Must be this scenario of SDEV_CANCEL is a race that is relatively
unique/rare to your testbed?

This raises the question: should SCSI be returning something other than
-EIO for this case?  E.g. an error that is retryable?

Mike
--
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/4] scsi: bnx2fc: convert per-CPU thread to kworker

[Johannes Thumshirn]

On Wed, Jul 20, 2016 at 02:07:19PM +0200, Sebastian Andrzej Siewior wrote:
> On 07/20/2016 01:04 PM, Johannes Thumshirn wrote:
> > Hi Sebastian,
> 
> Hi Johannes,
> 
> > I've tried to test your series and unfortunately I encountered the following
> > oops.
> thanks for testing.
> …
> > 
> > I'll try to narrow it down to the specific patch, but currently I'm a bit
> > busy, sorry.
> 
> I looked over the patch and noticed nothing. It was possible to run the
> worker but somehow the spinlock exploded.

Strange...

I'll re-apply the patches again, maybe something went wrong there.

Johannes

-- 
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
--
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/4] scsi: bnx2fc: convert per-CPU thread to kworker

[Sebastian Andrzej Siewior]

On 07/20/2016 01:04 PM, Johannes Thumshirn wrote:
> Hi Sebastian,

Hi Johannes,

> I've tried to test your series and unfortunately I encountered the following
> oops.
thanks for testing.
…
> 
> I'll try to narrow it down to the specific patch, but currently I'm a bit
> busy, sorry.

I looked over the patch and noticed nothing. It was possible to run the
worker but somehow the spinlock exploded.

> 
> Johannes
> 
Sebastian
--
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 v2 2/3] xen-scsiback: Rename jump labels in scsiback_device_action()

[Juergen Gross]

On 20/07/16 13:34, SF Markus Elfring wrote:
> From: Markus Elfring 
> Date: Wed, 20 Jul 2016 13:03:16 +0200
> 
> * Adjust jump targets according to the Linux coding style convention.
> 
> * A bit of refactoring for the control flow
> 
> Suggested-by: Jürgen Groß 
> Signed-off-by: Markus Elfring 

Reviewed-by: Juergen Gross 


Juergen
--
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 v2 1/3] xen-scsiback: Delete an unnecessary check before the function call "kfree"

[Juergen Gross]

On 20/07/16 13:30, SF Markus Elfring wrote:
> From: Markus Elfring 
> Date: Tue, 19 Jul 2016 15:42:19 +0200
> 
> The kfree() function tests whether its argument is NULL and then
> returns immediately. Thus the test around the call is not needed.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring 

Even if already given, here it is again:

Reviewed-by: Juergen Gross 


Juergen
--
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 v2 3/3] xen-scsiback: Pass a failure indication as a constant

[Juergen Gross]

On 20/07/16 13:36, SF Markus Elfring wrote:
> From: Markus Elfring 
> Date: Wed, 20 Jul 2016 13:12:33 +0200
> 
> Pass the constant "FAILED" in a function call directly instead of
> using an intialisation for a local variable.
> 
> Signed-off-by: Markus Elfring 

Even if already given, here it is again:

Reviewed-by: Juergen Gross 


Juergen
--
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 v2 3/3] xen-scsiback: Pass a failure indication as a constant

[SF Markus Elfring]

From: Markus Elfring 
Date: Wed, 20 Jul 2016 13:12:33 +0200

Pass the constant "FAILED" in a function call directly instead of
using an intialisation for a local variable.

Signed-off-by: Markus Elfring 
---
v2: Rebased on source files from "Linux next-20160719"

 drivers/xen/xen-scsiback.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/xen/xen-scsiback.c b/drivers/xen/xen-scsiback.c
index eb274df..fa08ec6 100644
--- a/drivers/xen/xen-scsiback.c
+++ b/drivers/xen/xen-scsiback.c
@@ -601,7 +601,7 @@ static void scsiback_device_action(struct vscsibk_pend 
*pending_req,
struct se_cmd *se_cmd = _req->se_cmd;
struct scsiback_tmr *tmr;
u64 unpacked_lun = pending_req->v2p->lun;
-   int rc, err = FAILED;
+   int rc, err;
 
tmr = kzalloc(sizeof(struct scsiback_tmr), GFP_KERNEL);
if (!tmr)
@@ -628,7 +628,7 @@ put_cmd:
target_put_sess_cmd(se_cmd);
 free_tmr:
kfree(tmr);
-   scsiback_do_resp_with_sense(NULL, err, 0, pending_req);
+   scsiback_do_resp_with_sense(NULL, FAILED, 0, pending_req);
 }
 
 /*
-- 
2.9.2

--
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 v2 2/3] xen-scsiback: Rename jump labels in scsiback_device_action()

[SF Markus Elfring]

From: Markus Elfring 
Date: Wed, 20 Jul 2016 13:03:16 +0200

* Adjust jump targets according to the Linux coding style convention.

* A bit of refactoring for the control flow

Suggested-by: Jürgen Groß 
Signed-off-by: Markus Elfring 
---
v2: Rebased on source files from "Linux next-20160719"
Changes from a bit of code review

 drivers/xen/xen-scsiback.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/xen/xen-scsiback.c b/drivers/xen/xen-scsiback.c
index 4a48c06..eb274df 100644
--- a/drivers/xen/xen-scsiback.c
+++ b/drivers/xen/xen-scsiback.c
@@ -604,10 +604,8 @@ static void scsiback_device_action(struct vscsibk_pend 
*pending_req,
int rc, err = FAILED;
 
tmr = kzalloc(sizeof(struct scsiback_tmr), GFP_KERNEL);
-   if (!tmr) {
-   target_put_sess_cmd(se_cmd);
-   goto err;
-   }
+   if (!tmr)
+   goto put_cmd;
 
init_waitqueue_head(>tmr_wait);
 
@@ -616,7 +614,7 @@ static void scsiback_device_action(struct vscsibk_pend 
*pending_req,
   unpacked_lun, tmr, act, GFP_KERNEL,
   tag, TARGET_SCF_ACK_KREF);
if (rc)
-   goto err;
+   goto free_tmr;
 
wait_event(tmr->tmr_wait, atomic_read(>tmr_complete));
 
@@ -626,7 +624,9 @@ static void scsiback_device_action(struct vscsibk_pend 
*pending_req,
scsiback_do_resp_with_sense(NULL, err, 0, pending_req);
transport_generic_free_cmd(_req->se_cmd, 1);
return;
-err:
+put_cmd:
+   target_put_sess_cmd(se_cmd);
+free_tmr:
kfree(tmr);
scsiback_do_resp_with_sense(NULL, err, 0, pending_req);
 }
-- 
2.9.2

--
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 v2 1/3] xen-scsiback: Delete an unnecessary check before the function call "kfree"

[SF Markus Elfring]

From: Markus Elfring 
Date: Tue, 19 Jul 2016 15:42:19 +0200

The kfree() function tests whether its argument is NULL and then
returns immediately. Thus the test around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
v2: Rebased on source files from "Linux next-20160719"

 drivers/xen/xen-scsiback.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/xen/xen-scsiback.c b/drivers/xen/xen-scsiback.c
index d6950e0..4a48c06 100644
--- a/drivers/xen/xen-scsiback.c
+++ b/drivers/xen/xen-scsiback.c
@@ -627,8 +627,7 @@ static void scsiback_device_action(struct vscsibk_pend 
*pending_req,
transport_generic_free_cmd(_req->se_cmd, 1);
return;
 err:
-   if (tmr)
-   kfree(tmr);
+   kfree(tmr);
scsiback_do_resp_with_sense(NULL, err, 0, pending_req);
 }
 
-- 
2.9.2

--
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 v2 0/3] xen-scsiback: Fine-tuning for scsiback_device_action()

[SF Markus Elfring]

From: Markus Elfring 
Date: Wed, 20 Jul 2016 13:20:04 +0200

Further update suggestions were taken into account
after a patch was applied from static source code analysis.

Markus Elfring (3):
  Delete an unnecessary check before the function call "kfree"
  Rename jump labels in scsiback_device_action()
  Pass a failure indication as a constant

 drivers/xen/xen-scsiback.c | 19 +--
 1 file changed, 9 insertions(+), 10 deletions(-)

-- 
2.9.2

--
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/4] scsi: bnx2fc: convert per-CPU thread to kworker

[Johannes Thumshirn]

On Tue, Jul 05, 2016 at 06:08:46PM +0200, Sebastian Andrzej Siewior wrote:
> The driver creates its own per-CPU threads which are updated based on CPU
> hotplug events. It is also possible to use kworkers and remove some of the
> infrastructure get the same job done while saving a few lines of code.
> 
> bnx2fc_percpu_io_thread() becomes bnx2fc_percpu_io_work() which is
> mostly the same code. The outer loop (kthread_should_stop()) gets
> removed and the remaining code is shifted to the left.
> In bnx2fc_process_new_cqes() the code checked for ->iothread to
> decide if there is an active per-CPU thread. With the kworkers this
> is no longer possible nor required. The allocation of a new work item
> (via bnx2fc_alloc_work()) does no longer happen with the ->fp_work_lock
> lock held. It performs only a memory allocation + initialization which
> does not require any kind of serialization. The lock is held while
> adding the new member to fps->work_list list.
> 
> The remaining part is the removal CPU hotplug notifier since it is taken
> care by the kworker code.
> 
> This patch was only compile-tested due to -ENODEV.
> 
> Cc: qlogic-storage-upstr...@qlogic.com
> Cc: Christoph Hellwig 
> Signed-off-by: Sebastian Andrzej Siewior 

Hi Sebastian,

I've tried to test your series and unfortunately I encountered the following
oops.

[  113.969654] Modules linked in: af_packet(E) 8021q(E) garp(E) stp(E) llc(E) 
mrp(E) mgag200(E) i2c_algo_bit(E) bnx2x(E) drm_kms_helper(E) syscopyarea(E) 
mdio(E) sysfillrect(E) libcrc32c(E) sysimgblt(E) dm_service_time(E) bnx2fc(E) 
fb_sys_fops(E) geneve(E) xhci_pci(E) uhci_hcd(E) ttm(E) sd_mod(E) ehci_hcd(E) 
xhci_hcd(E) cnic(E) vxlan(E) uio(E) ip6_udp_tunnel(E) udp_tunnel(E) 
crc32c_intel(E) hpsa(E) ptp(E) usbcore(E) drm(E) scsi_transport_sas(E) 
usb_common(E) pps_core(E) fjes(E) scsi_dh_rdac(E) scsi_dh_emc(E) 
scsi_dh_alua(E) dm_mirror(E) dm_region_hash(E) dm_log(E) fcoe(E) libfcoe(E) 
libfc(E) scsi_transport_fc(E) sg(E) dm_multipath(E) dm_mod(E) scsi_mod(E) 
efivarfs(E) autofs4(E)
[  113.969669] Supported: Yes
[  113.969671] CPU: 11 PID: 398 Comm: kworker/11:1 Tainted: GEL 
4.4.15-0.g6f92308-default #1
[  113.969672] Hardware name: HP ProLiant BL460c Gen9, BIOS I36 12/24/2014
[  113.969674] Workqueue: events bnx2fc_percpu_io_work [bnx2fc]
[  113.969675] task: 880667a38600 ti: 880667a3c000 task.ti: 
880667a3c000
[  113.969676] RIP: 0010:[]  [] 
native_queued_spin_lock_slowpath+0x16f/0x190
[  113.969677] RSP: 0018:880667a3fde8  EFLAGS: 0202
[  113.969678] RAX: 0101 RBX: 88065553cd20 RCX: 0001
[  113.969678] RDX: 0101 RSI: 0001 RDI: 8806676d80d0
[  113.969679] RBP: 880667a3fdf8 R08: 0101 R09: 0001
[  113.969679] R10: 81d46fc0 R11:  R12: 880667a3fdf8
[  113.969680] R13: 8806676d80a0 R14: 8806676d80c0 R15: 8806676d80d0
[  113.969681] FS:  () GS:8806676c() 
knlGS:
[  113.969681] CS:  0010 DS:  ES:  CR0: 80050033
[  113.969682] CR2: 7fee621724a0 CR3: 01c0a000 CR4: 001406e0
[  113.969682] Stack:
[  113.969683]  81181a17 a026a1dc 880667a3fdf8 
880667a3fdf8
[  113.969684]  8806676d80a0 88017c303b80 8806676d5400 
e8fa068c1200
[  113.969685]   02c0 8109394e 
67a38600
[  113.969685] Call Trace:
[  113.969688]  [] queued_spin_lock_slowpath+0x7/0xa
[  113.969690]  [] bnx2fc_percpu_io_work+0xac/0xd0 [bnx2fc]
[  113.969693]  [] process_one_work+0x14e/0x410
[  113.969695]  [] worker_thread+0x116/0x490
[  113.969697]  [] kthread+0xbd/0xe0
[  113.969699]  [] ret_from_fork+0x3f/0x70
[  113.970696] DWARF2 unwinder stuck at ret_from_fork+0x3f/0x70
[  113.970696] 
[  113.970696] Leftover inexact backtrace:
[  113.970696] 
[  113.970698]  [] ? kthread_park+0x50/0x50
[  113.970705] Code: 89 d0 66 31 c0 39 c1 74 e7 4d 85 c9 c6 07 01 74 2d 41 c7 
41 08 01 00 00 00 e9 52 ff ff ff 83 fa 01 74 17 8b 07 84 c0 74 08 f3 90 <8b> 07 
84 c0 75 f8 b8 01 00 00 00 66 89 07 c3 f3 c3 f3 90 4d 8b 

I'll try to narrow it down to the specific patch, but currently I'm a bit
busy, sorry.

Johannes

--
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: [v2] cxl: remove dead Kconfig options

[Michael Ellerman]

On Mon, 2016-18-07 at 04:52:57 UTC, Andrew Donnellan wrote:
> Remove the CXL_KERNEL_API and CXL_EEH Kconfig options, as they were only
> needed to coordinate the merging of the cxlflash driver. Also remove the
> stub implementation of cxl_perst_reloads_same_image() in cxlflash which is
> only used if CXL_EEH isn't defined (i.e. never).
> 
> Suggested-by: Ian Munsie 
> Signed-off-by: Andrew Donnellan 
> Acked-by: Ian Munsie 
> Acked-by: Matthew R. Ochs 

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/1e44727a0b220f6ead12fefcff

cheers
--
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 v5 4/8] char: rpmb: provide a user space interface

[Winkler, Tomas]

> 
> On Mon, Jul 18, 2016 at 4:27 PM, Tomas Winkler 
> wrote:
> > The user space API is achieved via two synchronous IOCTL.
> > Simplified one, RPMB_IOC_REQ_CMD, were read result cycles is
> performed
> > by the framework on behalf the user and second, RPMB_IOC_SEQ_CMD
> where
> > the whole RPMB sequence including RESULT_READ is supplied by the caller.
> > The latter is intended for  easier adjusting  of the  applications
> > that use MMC_IOC_MULTI_CMD ioctl.
> >
> > Signed-off-by: Tomas Winkler 
> > ---
> 
> [...]
> 
> > diff --git a/drivers/char/rpmb/Kconfig b/drivers/char/rpmb/Kconfig
> > index c5e6e909efce..6794be9fcc5e 100644
> > --- a/drivers/char/rpmb/Kconfig
> > +++ b/drivers/char/rpmb/Kconfig
> > @@ -6,3 +6,10 @@ config RPMB
> >   access RPMB partition.
> >
> >   If unsure, select N.
> > +
> > +config RPMB_INTF_DEV
> > +   bool "RPMB character device interface /dev/rpmbN"
> 
> A bool Kconfig should ideally
> 
> > +   depends on RPMB
> > +   help
> > + Say yes here if you want to access RPMB from user space
> > + via character device interface /dev/rpmb%d
> > diff --git a/drivers/char/rpmb/Makefile b/drivers/char/rpmb/Makefile
> > index 812b3ed264c0..b5dc087b1299 100644
> > --- a/drivers/char/rpmb/Makefile
> > +++ b/drivers/char/rpmb/Makefile
> > @@ -1,4 +1,5 @@
> >  obj-$(CONFIG_RPMB) += rpmb.o
> >  rpmb-objs += core.o
> > +rpmb-$(CONFIG_RPMB_INTF_DEV) += cdev.o

This is not a builtin, this is an optional part of the module 

> > +#include 
> 
> not use module.h or any MODULE_ macros from within it.

Can be dropped in this case as no macros are used, 
but the pattern Kconfig bool -> no include module.h  you are following has 
false positive cases.

Thanks
Tomas

HDD Unrecovered readerror issue

[Dmitry Monakhov]

Drive:WDC WD1003FZEX-00MK2A0
I have got this in logs:

ata1.00: failed command: READ FPDMA QUEUED
ata1.00: cmd 60/a0:a0:f0:c0:c5/00:00:04:00:00/40 tag 20 ncq 81920 in res 
41/40:00:88:c1:c5/00:00:04:00:00/00 Emask 0x409 (media error)
ata1.00: status: { DRDY ERR }
ata1.00: error: { UNC }
ata1.00: configured for UDMA/133
sd 0:0:0:0: [sda] FAILED Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE
sd 0:0:0:0: [sda] Sense Key : Medium Error[current] [descriptor]
sd 0:0:0:0: [sda] Add. Sense: Unrecovered readerror - auto reallocate failed
sd 0:0:0:0: [sda] CDB: Read(10) 28 00 04 c5 c0 f0 00 00 a0 00
blk_update_request: I/O error, dev sda, sector 80069000
ata1: EH complete

I can reproduce this easily
#xfs_io -c "pread $((80069000/2))k 4k" -d  /dev/sda
pread64: Input/output error
##Got EIO
##Smartctl also detect this
#smartctl -t short /dev/sda
#smartctl -l selftest /dev/sda

Short offline   Completed: read failure   90%  4682 80069000

But once I rewrite this block, problem goes away.
#xfs_io -c "pwrite -S 0x0 $((80069000/2))k 4k" -d  /dev/sda

Now I can read it w/o any errors and smartctl is happy
#smartctl -t short /dev/sda
#smartctl -l selftest /dev/sda
Num  Test_DescriptionStatus  Remaining
LifeTime(hours)  LBA_of_first_error
# 1  Short offline   Completed without error   00%  4683 -

So my disk is not dead right? Why the hell HDD fail read from very beginning
Is this because HDD firmware detect internal crcXX sum corruption?
How this can happen? Is this because of power failure?
AFAIK standard guarantees that sector will be updated atomically.
But it happens! Please guide me how to fix such problems in general.


signature.asc
Description: PGP signature