RE: [PATCH v2] storvsc: Set up correct queue depth values for IDE devices

[Long Li]

> Subject: Re: [PATCH v2] storvsc: Set up correct queue depth values for IDE
> devices
> 
> 
> Long,
> 
> > Can you take a look at the following patch?
> 
> >> > + max_sub_channels =
> >> > +(num_cpus - 1) / storvsc_vcpus_per_sub_channel;
> 
> What happens if num_cpus = 1?

If num_cpus=1, we don't have any sub channels.

The host offers one sub channel for VM with 5 CPUs, after that it offers an 
additional sub channel every 4 CPUs.

The primary channel is always offered.

> 
> --
> Martin K. PetersenOracle Linux Engineering

Re: [PATCH v9] scsi: new zorro_esp.c for Amiga Zorro NCR53C9x boards

[Michael Schmitz]

Hi Martin,

thank you so much!

Ben - please drop the Amiga SCSI patch for Debian testing from kernel
version 4.18!

Cheers,

Michael


Am 19.04.2018 um 16:01 schrieb Martin K. Petersen:
> 
> Michael,
> 
>> Anything else needed for linux-scsi?
> 
> Applied to 4.18/scsi-queue. Thanks!
> 

Re: [PATCH] target: Change return type to vm_fault_t

[Martin K. Petersen]

Souptick,

> Use new return type vm_fault_t for fault handler in struct
> vm_operations_struct.

Applied to 4.18/scsi-queue. Thank you.

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCHv2] target: prefer dbroot of /etc/target over /var/target

[Martin K. Petersen]

Lee,

> The target database root directory, dbroot, has defaulted to
> /var/target for a while, but its main client, targetcli-fb, has been
> moving it to /etc/target for quite some time. With the plethora of
> target drivers now appearing, it has become more difficult to
> initialize this attribute before use by any child drivers.
>
> If the directory /etc/target exists, use that as the DB
> root. Otherwise, fall back to using /var/target.
>
> The ability to override this dbroot attribute still exists via sysfs.

Applied to 4.18/scsi-queue. Thanks!

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH] target: fix crash with iscsi target and dvd

[Martin K. Petersen]

Ming,

> When the current page can't be added to bio, one new bio should be
> created for adding this page again, instead of ignoring this page.
>
> This patch fixes kernel crash with iscsi target and dvd, as reported
> by Wakko.

I queued this up in 4.17/scsi-fixes.

-- 
Martin K. Petersen  Oracle Linux Engineering

[PATCH] scsi_debug: replace SDEG_RES_IMMED_MASK with cmnd flag

[Douglas Gilbert]

This patch uses a cleaner method to convey the presence of the IMMED
cdb bit back to the generic delay code in the scsi_debug driver. The
previous method used a temporary mask over the SCSI result value
(a 32 bit integer soon to be restructured) that was not visible
outside this driver. It has still caused some confusion so a more
conventional method is now used: adding an extra flag to the
scsi_cmnd "host_scribble" area.

Signed-off-by: Douglas Gilbert 
---

This patch is designed to simplify part of the patchset titled:
"Rework SCSI results handling" that touched the scsi_debug driver.
This patch is against Martin's 4.18/scsi-queue branch. It will
probably break soon as that branch doesn't yet contain my
"scsi_debug: IMMED related delay adjustments" patch which touches
the same area. The technique being used should be clear.


 drivers/scsi/scsi_debug.c | 30 ++
 1 file changed, 14 insertions(+), 16 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 9ef5e3b810f6..65809f3d0d33 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -234,7 +234,7 @@ static const char *sdebug_version_date = "20180128";
 #define F_INV_OP   0x200
 #define F_FAKE_RW  0x400
 #define F_M_ACCESS 0x800   /* media access */
-#define F_LONG_DELAY   0x1000
+#define F_LONG_DELAY   0x1000  /* IMMED bit on commands with this */
 
 #define FF_RESPOND (F_RL_WLUN_OK | F_SKIP_UA | F_DELAY_OVERR)
 #define FF_MEDIA_IO (F_M_ACCESS | F_FAKE_RW)
@@ -294,6 +294,7 @@ struct sdebug_queued_cmd {
unsigned int inj_dix:1;
unsigned int inj_short:1;
unsigned int inj_host_busy:1;
+   bool immed_shortens_delay;
 };
 
 struct sdebug_queue {
@@ -399,14 +400,6 @@ static const unsigned char opcode_ind_arr[256] = {
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
 };
 
-/*
- * The following "response" functions return the SCSI mid-level's 4 byte
- * tuple-in-an-int. To handle commands with an IMMED bit, for a faster
- * command completion, they can mask their return value with
- * SDEG_RES_IMMED_MASK .
- */
-#define SDEG_RES_IMMED_MASK 0x4000
-
 static int resp_inquiry(struct scsi_cmnd *, struct sdebug_dev_info *);
 static int resp_report_luns(struct scsi_cmnd *, struct sdebug_dev_info *);
 static int resp_requests(struct scsi_cmnd *, struct sdebug_dev_info *);
@@ -1607,6 +1600,8 @@ static int resp_start_stop(struct scsi_cmnd *scp,
 {
unsigned char *cmd = scp->cmnd;
int power_cond, stop;
+   struct sdebug_queued_cmd *sqcp =
+   (struct sdebug_queued_cmd *)scp->host_scribble;
 
power_cond = (cmd[4] & 0xf0) >> 4;
if (power_cond) {
@@ -1615,7 +1610,8 @@ static int resp_start_stop(struct scsi_cmnd *scp,
}
stop = !(cmd[4] & 1);
atomic_xchg(>stopped, stop);
-   return (cmd[1] & 0x1) ? SDEG_RES_IMMED_MASK : 0; /* check IMMED bit */
+   sqcp->immed_shortens_delay = !!(cmd[1] & 0x1);
+   return 0;
 }
 
 static sector_t get_sdebug_capacity(void)
@@ -3586,6 +3582,8 @@ static int resp_sync_cache(struct scsi_cmnd *scp,
u64 lba;
u32 num_blocks;
u8 *cmd = scp->cmnd;
+   struct sdebug_queued_cmd *sqcp =
+   (struct sdebug_queued_cmd *)scp->host_scribble;
 
if (cmd[0] == SYNCHRONIZE_CACHE) {  /* 10 byte cdb */
lba = get_unaligned_be32(cmd + 2);
@@ -3598,7 +3596,8 @@ static int resp_sync_cache(struct scsi_cmnd *scp,
mk_sense_buffer(scp, ILLEGAL_REQUEST, LBA_OUT_OF_RANGE, 0);
return check_condition_result;
}
-   return (cmd[1] & 0x2) ? SDEG_RES_IMMED_MASK : 0; /* check IMMED bit */
+   sqcp->immed_shortens_delay = !!(cmd[1] & 0x2);
+   return 0;
 }
 
 #define RL_BUCKET_ELEMS 8
@@ -4399,12 +4398,12 @@ static int schedule_resp(struct scsi_cmnd *cmnd, struct 
sdebug_dev_info *devip,
}
 
cmnd->result = pfp != NULL ? pfp(cmnd, devip) : 0;
-   if (cmnd->result & SDEG_RES_IMMED_MASK) {
+   if (sqcp->immed_shortens_delay) {
/*
-* This is the F_DELAY_OVERR case. No delay.
+* IMMED bit set overrides F_LONG_DELAY.
 */
-   cmnd->result &= ~SDEG_RES_IMMED_MASK;
-   delta_jiff = ndelay = 0;
+   delta_jiff = 0;
+   ndelay = 0;
}
if (cmnd->result == 0 && scsi_result != 0)
cmnd->result = scsi_result;
@@ -4456,7 +4455,6 @@ static int schedule_resp(struct scsi_cmnd *cmnd, struct 
sdebug_dev_info *devip,
 
 respond_in_thread: /* call back to mid-layer using invocation thread */
cmnd->result = pfp != NULL ? pfp(cmnd, devip) : 0;
-   cmnd->result &= ~SDEG_RES_IMMED_MASK;
if (cmnd->result == 0 && scsi_result != 0)
cmnd->result = scsi_result;
cmnd->scsi_done(cmnd);
-- 
2.14.1

Re: [PATCH 01/12] iscsi_tcp: don't set a bounce limit

[Martin K. Petersen]

Christoph,

> The default already is to never bounce, so the call is a no-op.

Applied to 4.18/scsi-queue. Thanks!

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH 02/12] storsvc: don't set a bounce limit

[Martin K. Petersen]

Christoph,

> The default already is to never bounce, so the call is a no-op.

Applied to 4.18/scsi-queue. Thanks!

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH] scsi: Change return type to vm_fault_t

[Martin K. Petersen]

Souptick,

> Use new return type vm_fault_t for fault handler in struct
> vm_operations_struct.

Applied to 4.18/scsi-queue. Thanks!

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH v2 00/14] mpt3sas: Enhancements and Defect fixes.

[Martin K. Petersen]

> Chaitra P B (14):
>   mpt3sas: Bug fix for big endian systems.
>   mpt3sas: Pre-allocate RDPQ Array at driver boot time.
>   mpt3sas: Lockless access for chain buffers.
>   mpt3sas: Optimize I/O memory consumption in driver.
>   mpt3sas: Enhanced handling of Sense Buffer.
>   mpt3sas: Added support for SAS Device Discovery Error Event.
>   mpt3sas: Increase event log buffer to support 24 port HBA's.
>   mpt3sas: Allow processing of events during driver unload.
>   mpt3sas: Cache enclosure pages during enclosure add.
>   mpt3sas: Report Firmware Package Version from HBA Driver.
>   mpt3sas: Update MPI Headers
>   mpt3sas: For NVME device, issue a protocol level reset instead of
> hot reset and use TM timeout value exposed in PCIe Device Page 2.
>   mpt3sas: fix possible memory leak.
>   mpt3sas: Update driver version "25.100.00.00"

Somebody please review.

Thanks!

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: scsi sd driver is not emitting a CHANGE uevent for online disk resizes

[Martin K. Petersen]

Sara,

> Summary: scsi sd driver is not emitting a CHANGE uevent for online
> disk resizes using VMware’s vsphere.

For the SCSI layer to emit an event on capacity change, the storage
device must generate a UNIT ATTENTION with ASC 0x2a and ASCQ 0x9. Very
few devices to that.

When you do a rescan in sysfs you essentially force SCSI and block to
manually go out and discover all the device's properties from scratch.

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH] scsi: mptfc: fix spelling mistake in macro names

[Martin K. Petersen]

Colin,

> Rename macros MPI_FCPORTPAGE0_SUPPORT_SPEED_UKNOWN and
> MPI_FCPORTPAGE0_CURRENT_SPEED_UKNOWN to add in missing N in UNKNOWN

Applied to 4.18/scsi-queue. Thanks.

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH 0/3] sd_zbc: Avoid that resetting a zone fails sporadically

[Martin K. Petersen]

Bart,

> This patch series, when combined with the block layer patch that
> prevents that the report and reset zone ioctls are executed while a
> queue is frozen, prevents that these ioctls fail sporadically with
> -EIO. Since this race is relatively easy to trigger with the SMR
> support for fio that I'm working on, please consider this patch series
> for kernel v4.17.

I queued the cleanup patches for 4.18 and the bug fix patch for
4.17. Thanks!

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH v9] scsi: new zorro_esp.c for Amiga Zorro NCR53C9x boards

[Martin K. Petersen]

Michael,

> Anything else needed for linux-scsi?

Applied to 4.18/scsi-queue. Thanks!

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH] target: fix crash with iscsi target and dvd

[Bart Van Assche]

On Wed, 2018-04-18 at 23:27 -0400, Martin K. Petersen wrote:
> Christoph,
> 
> > Btw, seems like someone needs to volunteer for putting together a pull
> > request with target fixes for Linus - I haven't heard from Nic for a
> > while, and we've got quite a few fixes out on the list.
> 
> Still happy to take things through SCSI if Nic doesn't materialize.

Hello Martin,

Can you have a look at the following patch series: "[PATCH 00/14] SCSI target
patches for kernel v4.17" 
(https://www.spinics.net/lists/target-devel/msg16523.html /
https://patchwork.kernel.org/project/target-devel/list/). Please let me know if 
you
want me to repost that patch series.

Thanks,

Bart.

Re: [PATCH v2] storvsc: Set up correct queue depth values for IDE devices

[Martin K. Petersen]

Long,

> Can you take a look at the following patch?

>> > + max_sub_channels =
>> > +  (num_cpus - 1) / storvsc_vcpus_per_sub_channel;

What happens if num_cpus = 1?

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH] target: fix crash with iscsi target and dvd

[Martin K. Petersen]

Christoph,

> Btw, seems like someone needs to volunteer for putting together a pull
> request with target fixes for Linus - I haven't heard from Nic for a
> while, and we've got quite a few fixes out on the list.

Still happy to take things through SCSI if Nic doesn't materialize.

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH 1/1] scsi: sd: improved drive sanitize error handling

[Martin K. Petersen]

Mahesh,

> During the drive sanitization, when the sd driver issues TEST UNIT
> READY (TUR), drive reports Sense Key: NOT_READY, ASC: 0x4 and ASCQ:
> 0x1b.

Applied to 4.17/scsi-fixes. Thank you!

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [VERY EARLY RFC 08/13] treewide: use set_host_byte

[Finn Thain]

On Wed, 18 Apr 2018, Johannes Thumshirn wrote:

> diff --git a/drivers/scsi/NCR5380.c b/drivers/scsi/NCR5380.c
> index d670cfe4d0e7..a0b79899bce3 100644
> --- a/drivers/scsi/NCR5380.c
> +++ b/drivers/scsi/NCR5380.c
> @@ -548,7 +548,8 @@ static int NCR5380_queue_command(struct Scsi_Host 
> *instance,
>   case WRITE_6:
>   case WRITE_10:
>   shost_printk(KERN_DEBUG, instance, "WRITE attempted with 
> NDEBUG_NO_WRITE set\n");
> - cmd->result = (DID_ERROR << 16);
> + cmd->result = 0;
> + set_host_byte(cmd, DID_ERROR);
>   cmd->scsi_done(cmd);
>   return 0;
>   }
> @@ -1118,7 +1119,8 @@ static struct scsi_cmnd *NCR5380_select(struct 
> Scsi_Host *instance,
>   NCR5380_write(SELECT_ENABLE_REG, hostdata->id_mask);
>   /* Can't touch cmd if it has been reclaimed by the scsi ML */
>   if (hostdata->selecting) {
> - cmd->result = DID_BAD_TARGET << 16;
> + cmd->result = 0;
> + set_host_byte(cmd, DID_BAD_TARGET);
>   complete_cmd(instance, cmd);
>   dsprintk(NDEBUG_SELECTION, instance, "target did not 
> respond within 250ms\n");
>   cmd = NULL;
> @@ -1168,7 +1170,8 @@ static struct scsi_cmnd *NCR5380_select(struct 
> Scsi_Host *instance,
>   NCR5380_transfer_pio(instance, , , );
>   if (len) {
>   NCR5380_write(INITIATOR_COMMAND_REG, ICR_BASE);
> - cmd->result = DID_ERROR << 16;
> + cmd->result = 0;
> + set_host_byte(cmd, DID_ERROR);
>   complete_cmd(instance, cmd);
>   dsprintk(NDEBUG_SELECTION, instance, "IDENTIFY message transfer 
> failed\n");
>   cmd = NULL;
> @@ -1708,7 +1711,8 @@ static void NCR5380_information_transfer(struct 
> Scsi_Host *instance)
>   shost_printk(KERN_DEBUG, instance, 
> "NDEBUG_NO_DATAOUT set, attempted DATAOUT aborted\n");
>   sink = 1;
>   do_abort(instance);
> - cmd->result = DID_ERROR << 16;
> + cmd->result = 0;
> + set_host_byte(cmd, DID_ERROR);
>   complete_cmd(instance, cmd);
>   hostdata->connected = NULL;
>   return;
> @@ -1757,7 +1761,8 @@ static void NCR5380_information_transfer(struct 
> Scsi_Host *instance)
>   cmd->device->borken = 1;
>   sink = 1;
>   do_abort(instance);
> - cmd->result = DID_ERROR << 16;
> + cmd->result = 0;
> + set_host_byte(cmd, DID_ERROR);
>   /* XXX - need to source or sink 
> data here, as appropriate */
>   }
>   } else {
> @@ -1951,7 +1956,8 @@ static void NCR5380_information_transfer(struct 
> Scsi_Host *instance)
>   NCR5380_transfer_pio(instance, , , 
> );
>   if (msgout == ABORT) {
>   hostdata->connected = NULL;
> - cmd->result = DID_ERROR << 16;
> + cmd->result = 0;
> + set_host_byte(cmd, DID_ERROR);
>   complete_cmd(instance, cmd);
>   maybe_release_dma_irq(instance);
>   NCR5380_write(SELECT_ENABLE_REG, 
> hostdata->id_mask);
> @@ -2228,7 +2234,8 @@ static int NCR5380_abort(struct scsi_cmnd *cmd)
>   if (list_del_cmd(>unissued, cmd)) {
>   dsprintk(NDEBUG_ABORT, instance,
>"abort: removed %p from issue queue\n", cmd);
> - cmd->result = DID_ABORT << 16;
> + cmd->result = 0;
> + set_host_byte(cmd, DID_ABORT);
>   cmd->scsi_done(cmd); /* No tag or busy flag to worry about */
>   goto out;
>   }
> @@ -2237,7 +2244,8 @@ static int NCR5380_abort(struct scsi_cmnd *cmd)
>   dsprintk(NDEBUG_ABORT, instance,
>"abort: cmd %p == selecting\n", cmd);
>   hostdata->selecting = NULL;
> - cmd->result = DID_ABORT << 16;
> + cmd->result = 0;
> + set_host_byte(cmd, DID_ABORT);
>   complete_cmd(instance, cmd);
>   goto out;
>   }
> @@ -2327,12 +2335,13 @@ static int NCR5380_host_reset(struct scsi_cmnd *cmd)
>*/
>  
>   if (list_del_cmd(>unissued, cmd)) {
> - cmd->result = DID_RESET << 

Re: [VERY EARLY RFC 01/13] scsi: use host_byte() accessor

[Finn Thain]

On Wed, 18 Apr 2018, Johannes Thumshirn wrote:

> --- a/drivers/scsi/dc395x.c
> +++ b/drivers/scsi/dc395x.c
> @@ -3473,7 +3473,7 @@ static void srb_done(struct AdapterCtlBlk *acb, struct 
> DeviceCtlBlk *dcb,
>  
>   /*if( srb->cmd->cmnd[0] == INQUIRY && */
>   /*  (host_byte(cmd->result) == DID_OK || status_byte(cmd->result) & 
> CHECK_CONDITION) ) */
> - if ((cmd->result == (DID_OK << 16)
> + if ((host_byte(cmd->result) == DID_OK
>|| status_byte(cmd->result) &
>CHECK_CONDITION)) {
>   if (!dcb->init_tcq_flag) {

That's not quite the same. The old test is effectively cmd->result == 0. 
Maybe this should be a separate patch?

-- 

RE: [PATCH v2] storvsc: Set up correct queue depth values for IDE devices

[Long Li]

Hi Martin

Can you take a look at the following patch?

Long

 
> > -Original Message-
> > From: linux-kernel-ow...@vger.kernel.org
> >  On Behalf Of Long Li
> > Sent: Thursday, March 22, 2018 2:47 PM
> > To: KY Srinivasan ; Haiyang Zhang
> > ; Stephen Hemminger
> ;
> > James E . J . Bottomley ; Martin K . Petersen
> > ; de...@linuxdriverproject.org; linux-
> > s...@vger.kernel.org; linux-ker...@vger.kernel.org
> > Cc: Long Li 
> > Subject: [PATCH v2] storvsc: Set up correct queue depth values for IDE
> > devices
> >
> > From: Long Li 
> >
> > Unlike SCSI and FC, we don't use multiple channels for IDE.
> > Also fix the calculation for sub-channels.
> >
> > Change log:
> > v2: Addressed comment on incorrect number of sub-channels.
> > (Michael Kelley )
> >
> > Signed-off-by: Long Li 
> 
> Reviewed-by: Michael Kelley 
> 
> > ---
> >  drivers/scsi/storvsc_drv.c | 7 +--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
> > index 8c51d628b52e..a2ec0bc9e9fa 100644
> > --- a/drivers/scsi/storvsc_drv.c
> > +++ b/drivers/scsi/storvsc_drv.c
> > @@ -1722,11 +1722,14 @@ static int storvsc_probe(struct hv_device
> *device,
> > max_targets = STORVSC_MAX_TARGETS;
> > max_channels = STORVSC_MAX_CHANNELS;
> > /*
> > -* On Windows8 and above, we support sub-channels for
> storage.
> > +* On Windows8 and above, we support sub-channels for
> storage
> > +* on SCSI and FC controllers.
> >  * The number of sub-channels offerred is based on the
> number of
> >  * VCPUs in the guest.
> >  */
> > -   max_sub_channels = (num_cpus /
> storvsc_vcpus_per_sub_channel);
> > +   if (!dev_is_ide)
> > +   max_sub_channels =
> > +   (num_cpus - 1) /
> storvsc_vcpus_per_sub_channel;
> > }
> >
> > scsi_driver.can_queue = (max_outstanding_req_per_channel *
> > --
> > 2.14.1

Re: [PATCH v3 1/6] ilog2: create truly constant version for sparse

[Linus Torvalds]

On Wed, Apr 18, 2018 at 1:12 AM, Martin Wilck  wrote:
>
> No, it doesn't (gcc 7.3.0). -> https://paste.opensuse.org/27471594
> It doesn't even warn on an expression like this:
>
>   #define SIZE (1<<10)
>   static int foo[ilog2(SIZE)];

Ok, I think this is the "random gcc versions act differently" issue.

Sometimes __builtin_constant_p() to a ternary operation acts as a
constant expression, and sometimes it doesn't.

Oh well.

> sparse 0.5.2 doesn't warn about that either.

Yeah, sparse doesn't get picky about "constant value" vs "constant
expression" normally. But some things do use the strict form, and
array index expressions is one of those cases.

> So maybe I was wrong, and this is actually a false positive in sparse.

No, it's correct, it's just that the semantics of exactly when
something is considered constant are a bit fluid.

> Do you want me to convert the patch to your approach anyway?

I suspect using the __is_constexpr() trick would make it rather more
technically correct, but would actually generate much worse code.

Because it would make us do that dynamic "__ilog2_uXX()" function call
even when you have a compile-time constant value, if it wasn't
actually a constant expression (ie a constant argument passed to an
inline function, for example).

So I guess your patch is fine, but I also wonder if we should just
default sparse to allow the good old non-strict "constant values are
ok" for indexes.

And turn on strict mode only with -pedantic or something.

 Linus

scsi sd driver is not emitting a CHANGE uevent for online disk resizes

[Sara]

Hello -

This is an issue I've encountered while trying to detect disk resizing 
with utilities listening to udev. Let me know if there's a better place 
for me to post this.


Summary: scsi sd driver is not emitting a CHANGE uevent for online disk 
resizes using VMware’s vsphere.


Configuration: I’m using Ubuntu 17.10 on VM based on a VMWare ESXi host, 
version 5.5.0. I identified the driver by checking dmesg.


Steps to reproduce:
Run udevadm monitor
Connect to the appropriate ESX host using vSphere
Add a new device by selecting ‘edit settings’ -> ‘add’ -> ‘hard disk’-> 
‘create a new virtual disk’ -> ‘finish’

Observe that the expected ADD uevents are tracked by udevadm
Increase the size of the device using vSphere
Observe that no CHANGE (or other) uevents appear in udevadm
Verify that the resize did occur, detected by triggering a rescan with 
sys/block//device/rescan and logged in dmesg: sdc: detected 
capacity change from 8589934592 to 10737418240


I also used the eBPF trace script to verify that no calls to 
kobject_uevent occur when I did the resize (though they did appear when 
the device was first added).


I’d be happy to look into making a patch myself, though I’m pretty new 
to drivers and I’d need some guidance. For example, from reading the 
source, I’m not yet sure if ADD uevents for resize are expected to be 
working or just haven't been implemented yet (i.e. is this a bug or a 
feature). I’m also unsure about which level of the driver is the 
appropriate place to emit these events.


Let me know if there’s any other information I can provide and thank you 
for your time!


Sara

Re: [PATCH] Waiting for scsi_host_template release

[Anatoliy Glagolev]

Sent out a new patch with "bsg referencing bus driver module" in subj.

On Wed, Apr 11, 2018 at 4:37 PM, Anatoliy Glagolev  wrote:
> On "what was the actual error": it is deref of an invalid address, not
> NULL. Attaching crash dump analysis for the reference.
>
> On module reference count: good point. I decided against it at first,
> but I can reconsider. "modprobe -r qla2xxx" will fail if there is an
> extra reference to the module, and the module_exit function will not
> even run, right? Waiting for references to go away would be more
> convenient for me. But I can see why the module reference count is a
> better approach in general. I can work around and retry "modprobe -r
> qla2xxx" multiple times in my scripts.
>
> I think that it is still a SCSI mid-layer job to do the references.
> There is no way qla2xxx can reference itself and then dereference at
> the right time.
>
> qla2xxx (or any other driver) provides a pointer to its module in
> scsi_host_template when it requests Scsi_Host creation. As far as I
> can see, no one ever takes a reference on that module. SCSI mid-layer
> just relies on the module to be around. Scsi_Host is a device itself;
> that is the device that is referenced on open/close from user mode,
> and not the bus driver that triggered the Scsi_Host creation.
>
> SCSI mid layer taking a reference on the template's module at
> Scsi_Host creation in scsi_host_alloc(..) and dropping it in
> scsi_host_dev_release (called when the last reference to Scsi_Host is
> gone) will not work. Assuming that the module_exit function does not
> run at an attempt to unload a referenced module, qla2xxx's Scsi_Host-s
> corresponding to the adapter's ports will stay forever.
>
> Let me think more about it; the idea is to intercept open/close at
> Scsi_Host and increment/decrement module reference at that time.
>
> Thanks a lot for the input!
>
>
> On Wed, Apr 11, 2018 at 1:12 PM, James Bottomley
>  wrote:
>> On Wed, 2018-04-11 at 12:22 -0700, Anatoliy Glagolev wrote:
>>> Hannes, James, thanks a lot for taking a look!
>>>
>>> On the problem the patch is solving: it is in the "Description" part
>>> of my initial e-mail. If you agree that a Scsi_Host may be around
>>> after a driver has unloaded, the problem applies to any driver
>>> creating a new Scsi_Host.
>>
>> No, I don't agree: as I said, the template is part of the module and
>> the module should be reference counted.  Any use after free of the
>> template means there's a refcounting bug somewhere.
>>
>>>  I fixed it in qla2xxx to illustrate the usage of the new function
>>> and scsi_host_template's flag; also, qla2xxx is where I actually
>>> observe crashes. Other drivers may do the same if they want to
>>> address the problem.
>>>
>>> Here are details on the qla2xxx crash repro, if that is what you were
>>> asking about. If I run "qaucli" utility that retrieves some info from
>>> the driver via SCSI mid-layer, and unload the driver in parallel, the
>>> kernel crashes with the following stack:
>>>
>>> [16834.636216,07] Call Trace:
>>>   ...
>>> scsi_proc_hostdir_rm
>>> [16834.641944,07]  []
>>> scsi_host_dev_release+0x3f/0x130
>>> [16834.647740,07]  [] device_release+0x32/0xa0
>>> [16834.653423,07]  [] kobject_cleanup+0x77/0x190
>>> [16834.659002,07]  [] kobject_put+0x25/0x50
>>> [16834.664430,07]  [] put_device+0x17/0x20
>>> [16834.669740,07]  []
>>> bsg_kref_release_function+0x24/0x30
>>> [16834.675007,07]  [] bsg_release+0x166/0x1d0
>>> [16834.680148,07]  [] __fput+0xcb/0x1d0
>>> [16834.685156,07]  [] fput+0xe/0x10
>>> [16834.690017,07]  [] task_work_run+0x86/0xb0
>>> [16834.694781,07]  []
>>> exit_to_usermode_loop+0x6b/0x9a
>>> [16834.699466,07]  []
>>> syscall_return_slowpath+0x55/0x60
>>> [16834.704110,07]  []
>>> int_ret_from_sys_call+0x25/0x9f
>>
>> This one's a bit baffling: open of the bsg device should have already
>> taken the module reference.  What was the actual error: NULL deref?
>>
>> The thing which is supposed to hold the module is the device open/close
>> which does scsi_device_put on sd_release ... unless this is some sort
>> of non-scsi device and qlogic forgot how to refcount?
>>
>>> On refcount for scsi_host_template: valid point, I did consider it.
>>> Existing drivers allocate scsi_host_template statically. We cannot
>>> change them all at once. So we have to allow 2 ways of allocating
>>> scsi_host_template: the dynamic one with refcounts and the static one
>>> for legacy driver support. That is kind of ugly, too. In addition,
>>> having a refcounted scsi_host_template after driver unload is
>>> confusing: the memory of scsi_host_template is OK, but any attempt to
>>> call a method from the template still causes a crash.
>>
>> No, the static template already is part of the module so it should be
>> refcounted as a module reference.
>>
>> James
>>

Re: [VERY EARLY RFC 00/13] Rework SCSI results handling

[Douglas Gilbert]

On 2018-04-18 11:01 AM, Johannes Thumshirn wrote:

Hey all,

here's a early preview of my SCSI results rework so we can eventually
discuss things next week at LSF/MM (it still has compiler errors on
aic7xxx and scsi_debug).

The motivation behing this is that some drivers have failed to set the
scsi_cmnd::result bytes correctly in the past and this is resulting in
hard to case down errors.

The open points:
1) 148 files changed, treewide. That's huge. Is it worth it?
2) remove the old status byte definitions
3) add a scsi_cmnd::result == 0 wrapper
3) convert aic7xx's CAM stuff so this series compiles cleanly
4) What the heck is the SDEG_RES_IMMED_MASK stuff in scsi_debug doing


It's a hack. The cdb tables have only one delay value per command and
for Start Stop Unit and Synchronize Cache those are relatively long
delays. However as you may be aware both those commands have an IMMED
bit which makes those commands return more or less immediately.

In the work flow of the scsi_debug driver the work done by a specific
command is done more or less immediately and any delay associated
with that command is done by generic code just prior to calling
scsi_cmnd::done(). So for those two commands I needed an additional
return value from the resp_start_stop() and resp_sync_cache() to
indicate that their normal delay should be short circuited.

And the signatures of those functions are function pointers in many
of the scsi_debug tables, so expanding the number of arguments for
these two cases would have been messy. So the (hacky) solution was to
overload the return int with an extra flag. And that return int is none
other than the infamous SCSI result.

With the benefit of hindsight a better solution is to add a new bool
to struct sdebug_dev_info:
bool immed_shortens_delay;

that is set by those two commands when their IMMED bit is set (in their
cdbs). Then the code in schedule_resp() can act on that bool (when set)
to short circuit the delay. That will be a worthwhile improvement so
other commands (if and when implemented) can react properly when their
IMMED bit is set (e.g. FORMAT UNIT, PRE-FETCH and SANITIZE).


Does that clear up this matter?

Doug Gilbert


5) change scsi_execute() so we get a newish 'struct scsi_results' instead of an 
int
6) {to,from}_scsi_result() are odd
7) find suitable commit messages

In case someone want's it in a more viewable form I've pushed the
series to my kernel.org git:
https://git.kernel.org/pub/scm/linux/kernel/git/jth/linux.git/log/?h=scsi-results

Johannes Thumshirn (13):
   scsi: use host_byte() accessor
   scsi: remove Scsi_Cmnd typedef
   scsi: add enum for host byte codes
   scsi: add enum for driver byte codes
   scsi: add enum for message byte codes
   scsi: introduce set_scsi_result
   scsi: use set_driver_byte
   treewide: use set_host_byte
   scsi: use set_msg_byte
   scsi: introduce set_status_byte and convert LLDDs to use it
   scsi: Change status bytes to use SAM-3 version
   reewide: introduce clear_scsi_result() and convert drivers
   scsi: introduce struct scsi_result

  arch/ia64/hp/sim/simscsi.c  |   6 +-
  block/bsg-lib.c |   8 +-
  block/bsg.c |   8 +-
  block/scsi_ioctl.c  |  12 +-
  drivers/ata/libata-scsi.c   |  54 +++---
  drivers/firewire/sbp2.c |   2 +-
  drivers/infiniband/ulp/srp/ib_srp.c |  23 ++-
  drivers/message/fusion/mptfc.c  |   8 +-
  drivers/message/fusion/mptsas.c |   3 +-
  drivers/message/fusion/mptscsih.c   | 122 -
  drivers/message/fusion/mptspi.c |   6 +-
  drivers/s390/scsi/zfcp_fc.h |   2 +-
  drivers/s390/scsi/zfcp_scsi.c   |   4 +-
  drivers/scsi/3w-9xxx.c  |  18 +-
  drivers/scsi/3w-sas.c   |  16 +-
  drivers/scsi/3w-.c  |  36 ++--
  drivers/scsi/53c700.c   |   3 +-
  drivers/scsi/BusLogic.c |  24 ++-
  drivers/scsi/NCR5380.c  |  39 +++--
  drivers/scsi/a100u2w.c  |   2 +-
  drivers/scsi/aacraid/aachba.c   | 221 +---
  drivers/scsi/aacraid/commsup.c  |   5 +-
  drivers/scsi/aacraid/linit.c|  12 +-
  drivers/scsi/advansys.c |  58 +++
  drivers/scsi/aha152x.c  |  76 
  drivers/scsi/aha1542.c  |   2 +-
  drivers/scsi/aha1740.c  |  11 +-
  drivers/scsi/aha1740.h  |   4 +-
  drivers/scsi/aic7xxx/aic79xx_osm.c  |   5 +-
  drivers/scsi/aic7xxx/aic79xx_osm.h  |   6 +-
  drivers/scsi/aic7xxx/aic7xxx_osm.c  |   5 +-
  drivers/scsi/aic7xxx/aic7xxx_osm.h  

[Bug 199435] HPSA + P420i resetting logical Direct-Access never complete

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=199435

--- Comment #4 from Don (don.br...@microsemi.com) ---
Your stack trace does not show and hpsa driver components, but I do see the
reset issued but not completing.

I'm hoping that the attached patch helps diagnose the issue a little better.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 199435] HPSA + P420i resetting logical Direct-Access never complete

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=199435

--- Comment #3 from Don (don.br...@microsemi.com) ---
Created attachment 275437
  --> https://bugzilla.kernel.org/attachment.cgi?id=275437=edit
Patch to use local work-queue insead of system work-queue

If the driver initiates a re-scan from a system work-queue, the kernel can
hang.

This patch has not been submitted to linux-scsi, I will be sending this patch
out soon.

-- 
You are receiving this mail because:
You are the assignee for the bug.

Re: MegaCli fails to communicate with Raid-Controller

[Martin K. Petersen]

Volker,

> after our latest kernel-update from 4.6. to 4.14.14 we are having
> trouble getting data out of our LSI-raid-controllers using the
> megacli-binary.
>
> For every execution of the megacli-binary a line shows up in the kern.log
>
> ###
> [547216.425556] megaraid_sas :03:00.0: Failed to alloc kernel SGL buffer 
> for IOCTL
> ###

Well, that explains why things aren't working. The kernel is unable to
allocate a DMA buffer for the ioctl.

There really hasn't been any changes to this code since 4.6. The only
thing that springs to mind is some mucking around with the DMA mask in a
previous megaraid update. But given how old your controller is, I'd
expect this mask to be 32 bits both before and after.

Kashyap? Sumit?

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: Multi-Actuator SAS HDD First Look

[Tim Walker]

On Wed, Apr 18, 2018 at 10:20 AM, Bart Van Assche
 wrote:
> On Wed, 2018-04-18 at 05:16 -0600, Tim Walker wrote:
>> It would be good if we could set up an informal meeting time and
>> location at LSFMM to discuss these dual actuator topics. so far you,
>> Doug, Hannes, and Christoph have expressed the most interest, plus
>> Damien. Can we set an hour aside one afternoon?
>
> Hello Tim,
>
> Had you noticed that the official LSF/MM agenda mentions the following:
> Tuesday, 11 - 11:30 AM: James Borden, Multi-actuator disk drives.
>
> Thanks,
>
> Bart.
>
>
>

No, I hadn't checked that, yet. James probably won't be there. I guess
that solves my problem. Thanks.

-- 
Tim Walker
Product Design Systems Engineering, Seagate Technology
(303) 775-3770

Re: Multi-Actuator SAS HDD First Look

[Bart Van Assche]

On Wed, 2018-04-18 at 05:16 -0600, Tim Walker wrote:
> It would be good if we could set up an informal meeting time and
> location at LSFMM to discuss these dual actuator topics. so far you,
> Doug, Hannes, and Christoph have expressed the most interest, plus
> Damien. Can we set an hour aside one afternoon?

Hello Tim,

Had you noticed that the official LSF/MM agenda mentions the following:
Tuesday, 11 - 11:30 AM: James Borden, Multi-actuator disk drives.

Thanks,

Bart.

Re: [VERY EARLY RFC 00/13] Rework SCSI results handling

[Bart Van Assche]

On Wed, 2018-04-18 at 17:01 +0200, Johannes Thumshirn wrote:
> here's a early preview of my SCSI results rework so we can eventually
> discuss things next week at LSF/MM (it still has compiler errors on
> aic7xxx and scsi_debug).
> 
> The motivation behing this is that some drivers have failed to set the
> scsi_cmnd::result bytes correctly in the past and this is resulting in
> hard to case down errors.
> 
> The open points:
> 1) 148 files changed, treewide. That's huge. Is it worth it?
> 2) remove the old status byte definitions
> 3) add a scsi_cmnd::result == 0 wrapper
> 3) convert aic7xx's CAM stuff so this series compiles cleanly
> 4) What the heck is the SDEG_RES_IMMED_MASK stuff in scsi_debug doing
> 5) change scsi_execute() so we get a newish 'struct scsi_results' instead of 
> an int
> 6) {to,from}_scsi_result() are odd
> 7) find suitable commit messages

Hello Johannes,

Thank you for having come up with this so quickly. Something I do not
like about this patch series is that several new very short helper functions
are introduced, e.g. set_scsi_result(), clear_scsi_result(), to_scsi_result()
and from_scsi_result(). If we would make scsi_result a union of a 32-bit
integer and a struct with the driver, host, msg and status bytes then we
would not need any of these new helper functions. Additionally, that approach
would allow us to eliminate the {set,get}_{driver,host,msg,status}_byte()
functions.

Thanks,

Bart.

[VERY EARLY RFC 10/13] scsi: introduce set_status_byte and convert LLDDs to use it

[Johannes Thumshirn]

@@
struct scsi_cmnd *c;
expression E1;
@@
(
-c->result |= E1;
+set_status_byte(c, E1);
|
-c->result |= (E1);
+set_status_byte(c, E1);
)


Signed-off-by: Johannes Thumshirn 
---
 drivers/ata/libata-scsi.c| 20 
 drivers/infiniband/ulp/srp/ib_srp.c  |  2 +-
 drivers/message/fusion/mptscsih.c|  4 ++--
 drivers/s390/scsi/zfcp_fc.h  |  2 +-
 drivers/s390/scsi/zfcp_scsi.c|  2 +-
 drivers/scsi/3w-9xxx.c   |  3 ++-
 drivers/scsi/3w-sas.c|  4 +++-
 drivers/scsi/aacraid/aachba.c|  6 ++---
 drivers/scsi/advansys.c  |  7 ++
 drivers/scsi/aic7xxx/aic79xx_osm.h   |  2 +-
 drivers/scsi/aic7xxx/aic7xxx_osm.h   |  2 +-
 drivers/scsi/atp870u.c   |  7 +++---
 drivers/scsi/dpt_i2o.c   |  2 +-
 drivers/scsi/hpsa.c  |  4 ++--
 drivers/scsi/hptiop.c|  2 +-
 drivers/scsi/ibmvscsi/ibmvscsi.c |  2 +-
 drivers/scsi/ipr.c   |  6 ++---
 drivers/scsi/libfc/fc_fcp.c  |  2 +-
 drivers/scsi/libsas/sas_ata.c|  5 ++--
 drivers/scsi/libsas/sas_expander.c   |  2 +-
 drivers/scsi/megaraid.c  | 13 +++
 drivers/scsi/megaraid/megaraid_mbox.c|  4 ++--
 drivers/scsi/mpt3sas/mpt3sas_scsih.c | 12 ++
 drivers/scsi/mvsas/mv_sas.c  |  2 +-
 drivers/scsi/mvumi.c |  2 +-
 drivers/scsi/pm8001/pm8001_sas.c |  5 ++--
 drivers/scsi/pmcraid.c   |  4 ++--
 drivers/scsi/qla1280.c   |  2 +-
 drivers/scsi/qla2xxx/qla_isr.c   |  6 ++---
 drivers/scsi/scsi_error.c| 40 
 drivers/scsi/smartpqi/smartpqi_init.c|  6 ++---
 drivers/scsi/storvsc_drv.c   |  2 +-
 drivers/scsi/virtio_scsi.c   |  2 +-
 drivers/staging/rts5208/rtsx_transport.c |  4 ++--
 drivers/target/loopback/tcm_loop.c   |  6 ++---
 drivers/usb/image/microtek.c |  5 ++--
 drivers/usb/storage/datafab.c|  4 ++--
 drivers/usb/storage/isd200.c | 22 +-
 drivers/usb/storage/jumpshot.c   |  4 ++--
 drivers/usb/storage/realtek_cr.c | 12 ++
 drivers/usb/storage/transport.c  | 17 +++---
 drivers/usb/storage/uas.c|  4 ++--
 drivers/usb/storage/usb.c|  2 +-
 include/scsi/scsi.h  |  6 -
 include/scsi/scsi_cmnd.h |  6 +
 include/scsi/scsi_proto.h| 25 ++--
 46 files changed, 163 insertions(+), 140 deletions(-)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index cedf4b70f06f..0d6547351d85 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -1550,7 +1550,7 @@ static unsigned int ata_scsi_start_stop_xlat(struct 
ata_queued_cmd *qc)
ata_scsi_set_invalid_field(qc->dev, scmd, fp, bp);
return 1;
  skip:
-   scmd->result = SAM_STAT_GOOD;
+   set_status_byte(scmd, SAM_STAT_GOOD);
return 1;
 }
 
@@ -1801,7 +1801,7 @@ static unsigned int ata_scsi_verify_xlat(struct 
ata_queued_cmd *qc)
return 1;
 
 nothing_to_do:
-   scmd->result = SAM_STAT_GOOD;
+   set_status_byte(scmd, SAM_STAT_GOOD);
return 1;
 }
 
@@ -1913,7 +1913,7 @@ static unsigned int ata_scsi_rw_xlat(struct 
ata_queued_cmd *qc)
return 1;
 
 nothing_to_do:
-   scmd->result = SAM_STAT_GOOD;
+   set_status_byte(scmd, SAM_STAT_GOOD);
return 1;
 }
 
@@ -1946,11 +1946,11 @@ static void ata_scsi_qc_complete(struct ata_queued_cmd 
*qc)
((cdb[2] & 0x20) || need_sense))
ata_gen_passthru_sense(qc);
else if (qc->flags & ATA_QCFLAG_SENSE_VALID)
-   cmd->result = SAM_STAT_CHECK_CONDITION;
+   set_status_byte(cmd, SAM_STAT_CHECK_CONDITION);
else if (need_sense)
ata_gen_ata_sense(qc);
else
-   cmd->result = SAM_STAT_GOOD;
+   set_status_byte(cmd, SAM_STAT_GOOD);
 
if (need_sense && !ap->ops->error_handler)
ata_dump_status(ap->print_id, >result_tf);
@@ -2131,7 +2131,7 @@ static void ata_scsi_rbuf_fill(struct ata_scsi_args *args,
ata_scsi_rbuf_put(cmd, rc == 0, );
 
if (rc == 0)
-   cmd->result = SAM_STAT_GOOD;
+   set_status_byte(cmd, SAM_STAT_GOOD);
 }
 
 /**
@@ -2910,14 +2910,14 @@ static void atapi_qc_complete(struct ata_queued_cmd *qc)
if (qc->cdb[0] == ALLOW_MEDIUM_REMOVAL && qc->dev->sdev)
qc->dev->sdev->locked = 0;
 
-   qc->scsicmd->result = SAM_STAT_CHECK_CONDITION;
+   set_status_byte(qc->scsicmd, SAM_STAT_CHECK_CONDITION);
ata_qc_done(qc);
return;
}
 
/* successful completion or old EH 

[VERY EARLY RFC 06/13] scsi: introduce set_scsi_result

[Johannes Thumshirn]

Introduce set_scsi_result() for setting the bytes of the scsi_cmnd's result
filed and flip over all users that set at least two bytes to using it.

The conversion has been done using the following coccinelle spatch:

@@
struct scsi_cmnd *c;
expression E1, E2, E3, E4;
@@

(
- c->result = E1 << 24 | E2 << 16 | E3 << 8 | E4;
+ set_scsi_result(c, E1, E2, E3, E4);
|
- c->result = E1 << 24 | E2 << 16 | E3 << 8;
+ set_scsi_result(c, E1, E2, E3, 0);
|
- c->result = E2 << 16 | E3 << 8 | E4;
+ set_scsi_result(c, 0, E2, E3, E4);
|
- c->result = E2 << 16 | E3 << 8;
+ set_scsi_result(c, 0, E2, E3, 0);
|
- c->result = (E2 << 16) | E4;
+ set_scsi_result(c, 0, E2, 0, E4);
|
- c->result = (E2 << 16) | (E4);
+ set_scsi_result(c, 0, E2, 0, E4);
)

@@
struct scsi_cmnd *c;
expression E1, E2;
identifier ScsiResult;
@@

-c->result = ScsiResult(E1, E2);
+set_scsi_result(c, 0, E1, 0, E2);

@@
identifier ScsiResult, host_code,scsi_code;
@@
(
-#define ScsiResult(host_code, scsi_code) (((host_code) << 16) | scsi_code)
|
-#define ScsiResult(host_code, scsi_code) (((host_code) << 16) + ((scsi_code) & 
0x7f))
)


Signed-off-by: Johannes Thumshirn 
---
 drivers/ata/libata-scsi.c   |   8 +-
 drivers/infiniband/ulp/srp/ib_srp.c |   7 +-
 drivers/message/fusion/mptscsih.c   |   6 +-
 drivers/scsi/3w-9xxx.c  |   3 +-
 drivers/scsi/3w-.c  |   9 +-
 drivers/scsi/NCR5380.c  |   6 +-
 drivers/scsi/a100u2w.c  |   2 +-
 drivers/scsi/aacraid/aachba.c   | 200 +++-
 drivers/scsi/aacraid/commsup.c  |   5 +-
 drivers/scsi/advansys.c |  26 ++--
 drivers/scsi/arcmsr/arcmsr_hba.c|   5 +-
 drivers/scsi/arm/acornscsi.c|   3 +-
 drivers/scsi/arm/fas216.c   |   4 +-
 drivers/scsi/be2iscsi/be_main.c |   2 +-
 drivers/scsi/bfa/bfad_im.c  |  16 +--
 drivers/scsi/bfa/bfad_im.h  |   1 -
 drivers/scsi/bnx2fc/bnx2fc_io.c |   6 +-
 drivers/scsi/csiostor/csio_scsi.c   |   4 +-
 drivers/scsi/cxlflash/main.c|   3 +-
 drivers/scsi/dc395x.c   |  23 ++--
 drivers/scsi/dpt_i2o.c  |   4 +-
 drivers/scsi/esas2r/esas2r_main.c   |   7 +-
 drivers/scsi/esp_scsi.c |   7 +-
 drivers/scsi/fnic/fnic_scsi.c   |  22 +--
 drivers/scsi/gdth.c |  17 ++-
 drivers/scsi/hptiop.c   |   2 +-
 drivers/scsi/imm.c  |   2 +-
 drivers/scsi/initio.c   |   2 +-
 drivers/scsi/ips.c  |   5 +-
 drivers/scsi/libfc/fc_fcp.c |   9 +-
 drivers/scsi/libiscsi.c |  18 ++-
 drivers/scsi/libsas/sas_scsi_host.c |   2 +-
 drivers/scsi/lpfc/lpfc_crtn.h   |   1 -
 drivers/scsi/lpfc/lpfc_scsi.c   |  51 +++
 drivers/scsi/megaraid.c |  10 +-
 drivers/scsi/megaraid/megaraid_mbox.c   |  23 ++--
 drivers/scsi/megaraid/megaraid_sas_base.c   |   7 +-
 drivers/scsi/megaraid/megaraid_sas_fusion.c |   4 +-
 drivers/scsi/mesh.c |   2 +-
 drivers/scsi/mpt3sas/mpt3sas_scsih.c|  14 +-
 drivers/scsi/mvumi.c|   6 +-
 drivers/scsi/ncr53c8xx.c|  24 ++--
 drivers/scsi/nsp32.c|   5 +-
 drivers/scsi/pcmcia/nsp_cs.c|  12 +-
 drivers/scsi/pcmcia/sym53c500_cs.c  |   4 +-
 drivers/scsi/ppa.c  |   3 +-
 drivers/scsi/ps3rom.c   |   3 +-
 drivers/scsi/qedf/qedf_io.c |  12 +-
 drivers/scsi/qla4xxx/ql4_isr.c  |   8 +-
 drivers/scsi/snic/snic_scsi.c   |   4 +-
 drivers/scsi/stex.c |  17 ++-
 drivers/scsi/sym53c8xx_2/sym_glue.c |   2 +-
 drivers/scsi/sym53c8xx_2/sym_glue.h |   2 +-
 drivers/scsi/vmw_pvscsi.c   |   2 +-
 drivers/scsi/wd33c93.c  |  17 ++-
 drivers/usb/storage/cypress_atacb.c |   5 +-
 include/scsi/scsi_cmnd.h|   8 ++
 57 files changed, 360 insertions(+), 322 deletions(-)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 89a9d4a2efc8..f34650ada9d7 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -357,7 +357,7 @@ void ata_scsi_set_sense(struct ata_device *dev, struct 
scsi_cmnd *cmd,
if (!cmd)
return;
 
-   cmd->result = (DRIVER_SENSE << 24) | SAM_STAT_CHECK_CONDITION;
+   set_scsi_result(cmd, DRIVER_SENSE, 0, 0, SAM_STAT_CHECK_CONDITION);
 
scsi_build_sense_buffer(d_sense, cmd->sense_buffer, sk, asc, ascq);
 }
@@ -873,7 +873,7 @@ static struct ata_queued_cmd *ata_scsi_qc_new(struct 
ata_device *dev,
qc->sg = 

[VERY EARLY RFC 13/13] scsi: introduce struct scsi_result

[Johannes Thumshirn]

Introduce struct scsi_result.

Signed-off-by: Johannes Thumshirn 
Suggested-by: Bart Van Assche 
---
 block/bsg-lib.c |  8 --
 block/bsg.c |  8 --
 block/scsi_ioctl.c  | 12 
 drivers/ata/libata-scsi.c   | 12 +---
 drivers/firewire/sbp2.c |  2 +-
 drivers/infiniband/ulp/srp/ib_srp.c | 11 
 drivers/message/fusion/mptfc.c  |  2 +-
 drivers/message/fusion/mptscsih.c   | 10 ---
 drivers/scsi/BusLogic.c |  4 +--
 drivers/scsi/NCR5380.c  |  2 +-
 drivers/scsi/aha152x.c  |  2 +-
 drivers/scsi/bfa/bfad_im.c  | 10 +++
 drivers/scsi/bnx2fc/bnx2fc_io.c |  2 +-
 drivers/scsi/ch.c   |  2 +-
 drivers/scsi/constants.c|  4 +--
 drivers/scsi/csiostor/csio_scsi.c   |  2 +-
 drivers/scsi/dc395x.c   | 11 
 drivers/scsi/device_handler/scsi_dh_alua.c  | 10 +--
 drivers/scsi/dpt_i2o.c  |  2 +-
 drivers/scsi/esp_scsi.c |  2 +-
 drivers/scsi/fnic/fnic_scsi.c   |  2 +-
 drivers/scsi/hpsa.c |  2 +-
 drivers/scsi/libfc/fc_fcp.c |  2 +-
 drivers/scsi/libiscsi.c |  6 ++--
 drivers/scsi/lpfc/lpfc_scsi.c   | 19 +++--
 drivers/scsi/megaraid/megaraid_sas_fusion.c |  6 ++--
 drivers/scsi/mpt3sas/mpt3sas_scsih.c|  6 ++--
 drivers/scsi/osst.c |  3 +-
 drivers/scsi/pmcraid.c  |  4 +--
 drivers/scsi/qedf/qedf_io.c |  8 +++---
 drivers/scsi/qedi/qedi_fw.c |  2 +-
 drivers/scsi/qla1280.c  |  4 +--
 drivers/scsi/qla2xxx/qla_isr.c  |  6 ++--
 drivers/scsi/qla2xxx/qla_mr.c   |  4 +--
 drivers/scsi/qla2xxx/qla_os.c   |  8 +++---
 drivers/scsi/qla4xxx/ql4_os.c   | 10 ---
 drivers/scsi/qlogicfas408.c |  2 +-
 drivers/scsi/scsi.c |  5 ++--
 drivers/scsi/scsi_debug.c   | 23 +--
 drivers/scsi/scsi_debugfs.c |  2 +-
 drivers/scsi/scsi_error.c   | 12 
 drivers/scsi/scsi_ioctl.c   |  2 +-
 drivers/scsi/scsi_lib.c | 21 --
 drivers/scsi/scsi_logging.c |  5 ++--
 drivers/scsi/scsi_scan.c|  2 +-
 drivers/scsi/scsi_transport_spi.c   |  2 +-
 drivers/scsi/sd.c   | 22 +++
 drivers/scsi/sd_zbc.c   |  4 +--
 drivers/scsi/sg.c   | 10 +++
 drivers/scsi/snic/snic_scsi.c   |  2 +-
 drivers/scsi/sr.c   |  4 +--
 drivers/scsi/sr_ioctl.c |  2 +-
 drivers/scsi/st.c   |  2 +-
 drivers/scsi/stex.c |  2 +-
 drivers/scsi/storvsc_drv.c  |  2 +-
 drivers/scsi/sym53c8xx_2/sym_glue.h |  1 -
 drivers/scsi/ufs/ufshcd.c   |  6 ++--
 drivers/scsi/vmw_pvscsi.c   |  3 +-
 drivers/scsi/xen-scsifront.c|  2 +-
 drivers/staging/unisys/visorhba/visorhba_main.c |  4 +--
 drivers/target/target_core_pscsi.c  | 12 +---
 drivers/usb/image/microtek.c| 11 +---
 drivers/usb/storage/cypress_atacb.c |  8 +++---
 drivers/usb/storage/transport.c |  3 +-
 drivers/usb/storage/uas.c   |  2 +-
 drivers/usb/storage/usb.c   |  4 +--
 drivers/xen/xen-scsiback.c  |  6 ++--
 include/scsi/scsi.h | 37 +++--
 include/scsi/scsi_cmnd.h| 18 +++-
 include/trace/events/scsi.h |  2 +-
 70 files changed, 262 insertions(+), 193 deletions(-)

diff --git a/block/bsg-lib.c b/block/bsg-lib.c
index fc2e5ff2c4b9..6231cf710b8d 100644
--- a/block/bsg-lib.c
+++ b/block/bsg-lib.c
@@ -57,14 +57,16 @@ static int bsg_transport_complete_rq(struct request *rq, 
struct sg_io_v4 *hdr)
 {
struct bsg_job *job = blk_mq_rq_to_pdu(rq);
int ret = 0;
+   struct scsi_result sres = { 0 };
 
/*
 * The assignments below don't make much sense, but are kept for
 * bug by bug backwards compatibility:
 */
-   hdr->device_status = job->result & 0xff;
-   hdr->transport_status = host_byte(job->result);
-   

[VERY EARLY RFC 05/13] scsi: add enum for message byte codes

[Johannes Thumshirn]

Add enum for message byte codes and adopt set_msg_byte()'s
signature.

Signed-off-by: Johannes Thumshirn 
Suggested-by: Bart Van Assche 
---
 include/scsi/scsi.h  | 79 +---
 include/scsi/scsi_cmnd.h |  3 +-
 2 files changed, 44 insertions(+), 38 deletions(-)

diff --git a/include/scsi/scsi.h b/include/scsi/scsi.h
index 2694faafaddf..f51320dfac9b 100644
--- a/include/scsi/scsi.h
+++ b/include/scsi/scsi.h
@@ -92,42 +92,43 @@ static inline int scsi_is_wlun(u64 lun)
 /*
  *  MESSAGE CODES
  */
-
-#define COMMAND_COMPLETE0x00
-#define EXTENDED_MESSAGE0x01
-#define EXTENDED_MODIFY_DATA_POINTER0x00
-#define EXTENDED_SDTR   0x01
-#define EXTENDED_EXTENDED_IDENTIFY  0x02/* SCSI-I only */
-#define EXTENDED_WDTR   0x03
-#define EXTENDED_PPR0x04
-#define EXTENDED_MODIFY_BIDI_DATA_PTR   0x05
-#define SAVE_POINTERS   0x02
-#define RESTORE_POINTERS0x03
-#define DISCONNECT  0x04
-#define INITIATOR_ERROR 0x05
-#define ABORT_TASK_SET  0x06
-#define MESSAGE_REJECT  0x07
-#define NOP 0x08
-#define MSG_PARITY_ERROR0x09
-#define LINKED_CMD_COMPLETE 0x0a
-#define LINKED_FLG_CMD_COMPLETE 0x0b
-#define TARGET_RESET0x0c
-#define ABORT_TASK  0x0d
-#define CLEAR_TASK_SET  0x0e
-#define INITIATE_RECOVERY   0x0f/* SCSI-II only */
-#define RELEASE_RECOVERY0x10/* SCSI-II only */
-#define CLEAR_ACA   0x16
-#define LOGICAL_UNIT_RESET  0x17
-#define SIMPLE_QUEUE_TAG0x20
-#define HEAD_OF_QUEUE_TAG   0x21
-#define ORDERED_QUEUE_TAG   0x22
-#define IGNORE_WIDE_RESIDUE 0x23
-#define ACA 0x24
-#define QAS_REQUEST 0x55
-
-/* Old SCSI2 names, don't use in new code */
-#define BUS_DEVICE_RESETTARGET_RESET
-#define ABORT   ABORT_TASK_SET
+enum scsi_msg_byte {
+   COMMAND_COMPLETE = 0x00,
+   EXTENDED_MESSAGE = 0x01,
+   EXTENDED_MODIFY_DATA_POINTER = 0x00,
+   EXTENDED_SDTR = 0x01,
+   EXTENDED_EXTENDED_IDENTIFY = 0x02,   /* SCSI-I only */
+   EXTENDED_WDTR = 0x03,
+   EXTENDED_PPR = 0x04,
+   EXTENDED_MODIFY_BIDI_DATA_PTR = 0x05,
+   SAVE_POINTERS = 0x02,
+   RESTORE_POINTERS = 0x03,
+   DISCONNECT = 0x04,
+   INITIATOR_ERROR = 0x05,
+   ABORT_TASK_SET = 0x06,
+   MESSAGE_REJECT = 0x07,
+   NOP = 0x08,
+   MSG_PARITY_ERROR = 0x09,
+   LINKED_CMD_COMPLETE = 0x0a,
+   LINKED_FLG_CMD_COMPLETE = 0x0b,
+   TARGET_RESET = 0x0c,
+   ABORT_TASK = 0x0d,
+   CLEAR_TASK_SET = 0x0e,
+   INITIATE_RECOVERY = 0x0f,/* SCSI-II only */
+   RELEASE_RECOVERY = 0x10, /* SCSI-II only */
+   CLEAR_ACA = 0x16,
+   LOGICAL_UNIT_RESET = 0x17,
+   SIMPLE_QUEUE_TAG = 0x20,
+   HEAD_OF_QUEUE_TAG = 0x21,
+   ORDERED_QUEUE_TAG = 0x22,
+   IGNORE_WIDE_RESIDUE = 0x23,
+   ACA = 0x24,
+   QAS_REQUEST = 0x55,
+
+   /* Old SCSI2 names, don't use in new code */
+   BUS_DEVICE_RESET = TARGET_RESET,
+   ABORT = ABORT_TASK_SET,
+};
 
 /*
  * Host byte codes
@@ -213,7 +214,11 @@ enum scsi_driver_byte {
  *  driver_byte = set by mid-level.
  */
 #define status_byte(result) (((result) >> 1) & 0x7f)
-#define msg_byte(result)(((result) >> 8) & 0xff)
+static inline enum scsi_msg_byte msg_byte(int result)
+{
+   return (result >> 8) & 0xff;
+}
+
 static inline enum scsi_host_byte host_byte(int result)
 {
return (result >> 16) & 0xff;
diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
index da8ea89ccc0a..45d20d4a8f72 100644
--- a/include/scsi/scsi_cmnd.h
+++ b/include/scsi/scsi_cmnd.h
@@ -337,7 +337,8 @@ static inline struct scsi_data_buffer *scsi_prot(struct 
scsi_cmnd *cmd)
 #define scsi_for_each_prot_sg(cmd, sg, nseg, __i)  \
for_each_sg(scsi_prot_sglist(cmd), sg, nseg, __i)
 
-static inline void set_msg_byte(struct scsi_cmnd *cmd, char status)
+static inline void set_msg_byte(struct scsi_cmnd *cmd,
+   enum scsi_msg_byte status)
 {
cmd->result = (cmd->result & 0x00ff) | (status << 8);
 }
-- 
2.16.3

[VERY EARLY RFC 11/13] scsi: Change status bytes to use SAM-3 version

[Johannes Thumshirn]

Signed-off-by: Johannes Thumshirn 
---
 drivers/ata/libata-scsi.c |  2 +-
 drivers/scsi/3w-9xxx.c|  2 +-
 drivers/scsi/3w-.c|  6 --
 drivers/scsi/arcmsr/arcmsr_hba.c  |  4 ++--
 drivers/scsi/dc395x.c |  4 ++--
 drivers/scsi/dpt_i2o.c|  3 ++-
 drivers/scsi/gdth.c   | 12 ++--
 drivers/scsi/megaraid.c   | 10 +-
 drivers/scsi/megaraid/megaraid_mbox.c | 13 +++--
 9 files changed, 30 insertions(+), 26 deletions(-)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 0d6547351d85..96cd5c838c89 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -873,7 +873,7 @@ static struct ata_queued_cmd *ata_scsi_qc_new(struct 
ata_device *dev,
qc->sg = scsi_sglist(cmd);
qc->n_elem = scsi_sg_count(cmd);
} else {
-   set_scsi_result(cmd, 0, DID_OK, 0, (QUEUE_FULL << 1));
+   set_scsi_result(cmd, 0, DID_OK, 0, SAM_STAT_TASK_SET_FULL);
cmd->scsi_done(cmd);
}
 
diff --git a/drivers/scsi/3w-9xxx.c b/drivers/scsi/3w-9xxx.c
index ccdf7b08d117..7da983720550 100644
--- a/drivers/scsi/3w-9xxx.c
+++ b/drivers/scsi/3w-9xxx.c
@@ -1336,7 +1336,7 @@ static irqreturn_t twa_interrupt(int irq, void 
*dev_instance)
if (error == 1) {
/* Ask for a host reset */
set_scsi_result(cmd, 0, DID_OK, 0,
-   (CHECK_CONDITION << 1));
+   
SAM_STAT_CHECK_CONDITION);
}
 
/* Report residual bytes for single sgl */
diff --git a/drivers/scsi/3w-.c b/drivers/scsi/3w-.c
index da3c0ff13dac..ea259daf3089 100644
--- a/drivers/scsi/3w-.c
+++ b/drivers/scsi/3w-.c
@@ -431,7 +431,7 @@ static int tw_decode_sense(TW_Device_Extension *tw_dev, int 
request_id, int fill
 
set_scsi_result(tw_dev->srb[request_id],
0, DID_OK, 0,
-   (CHECK_CONDITION << 1));
+   
SAM_STAT_CHECK_CONDITION);
return TW_ISR_DONT_RESULT; /* Special 
case for isr to not over-write result */
}
}
@@ -2163,7 +2163,9 @@ static irqreturn_t tw_interrupt(int irq, void 
*dev_instance)
/* If error, command failed */
if (error == 1) {
/* Ask for a host reset */
-   
set_scsi_result(tw_dev->srb[request_id], 0, DID_OK, 0, CHECK_CONDITION << 1);
+   set_scsi_result(tw_dev->srb[request_id],
+   0, DID_OK, 0,
+   
SAM_STAT_CHECK_CONDITION);
}
 
/* Now complete the io */
diff --git a/drivers/scsi/arcmsr/arcmsr_hba.c b/drivers/scsi/arcmsr/arcmsr_hba.c
index 426d14c0104e..bea71f085679 100644
--- a/drivers/scsi/arcmsr/arcmsr_hba.c
+++ b/drivers/scsi/arcmsr/arcmsr_hba.c
@@ -1202,7 +1202,7 @@ static void arcmsr_report_sense_info(struct 
CommandControlBlock *ccb)
 
struct scsi_cmnd *pcmd = ccb->pcmd;
struct SENSE_DATA *sensebuffer = (struct SENSE_DATA 
*)pcmd->sense_buffer;
-   set_scsi_result(pcmd, 0, DID_OK, 0, (CHECK_CONDITION << 1));
+   set_scsi_result(pcmd, 0, DID_OK, 0, SAM_STAT_CHECK_CONDITION);
if (sensebuffer) {
int sense_data_length =
sizeof(struct SENSE_DATA) < SCSI_SENSE_BUFFERSIZE
@@ -3030,7 +3030,7 @@ static int arcmsr_queue_command_lck(struct scsi_cmnd *cmd,
return SCSI_MLQUEUE_HOST_BUSY;
if (arcmsr_build_ccb( acb, ccb, cmd ) == FAILED) {
set_scsi_result(cmd, 0, DID_ERROR, 0,
-   (RESERVATION_CONFLICT << 1));
+   SAM_STAT_RESERVATION_CONFLICT);
cmd->scsi_done(cmd);
return 0;
}
diff --git a/drivers/scsi/dc395x.c b/drivers/scsi/dc395x.c
index eebae800131d..89995bb09a94 100644
--- a/drivers/scsi/dc395x.c
+++ b/drivers/scsi/dc395x.c
@@ -3329,7 +3329,7 @@ static void srb_done(struct AdapterCtlBlk *acb, struct 
DeviceCtlBlk *dcb,
 */
srb->flag &= ~AUTO_REQSENSE;
srb->adapter_status = 0;
-   srb->target_status = CHECK_CONDITION << 1;
+   srb->target_status = SAM_STAT_CHECK_CONDITION;
if (debug_enabled(DBG_1)) {
  

[VERY EARLY RFC 03/13] scsi: add enum for host byte codes

[Johannes Thumshirn]

Add enum for host byte codes and adopt set_host_byte()'s and host_byte()'s
signature.

Signed-off-by: Johannes Thumshirn 
Suggested-by: Bart Van Assche 
---
 drivers/scsi/dc395x.c |  3 +--
 drivers/scsi/megaraid.c   |  6 --
 drivers/scsi/scsi_error.c |  2 ++
 include/scsi/scsi.h   | 55 ++-
 include/scsi/scsi_cmnd.h  |  3 ++-
 5 files changed, 39 insertions(+), 30 deletions(-)

diff --git a/drivers/scsi/dc395x.c b/drivers/scsi/dc395x.c
index da28f08ae185..379a1bc37576 100644
--- a/drivers/scsi/dc395x.c
+++ b/drivers/scsi/dc395x.c
@@ -173,7 +173,6 @@
 #define SET_RES_TARGET(who,tgt) { who &= ~RES_TARGET; who |= (int)(tgt); }
 #define SET_RES_TARGET_LNX(who,tgt) { who &= ~RES_TARGET_LNX; who |= 
(int)(tgt) << 1; }
 #define SET_RES_MSG(who,msg) { who &= ~RES_ENDMSG; who |= (int)(msg) << 8; }
-#define SET_RES_DID(who,did) { who &= ~RES_DID; who |= (int)(did) << 16; }
 #define SET_RES_DRV(who,drv) { who &= ~RES_DRV; who |= (int)(drv) << 24; }
 
 #define TAG_NONE 255
@@ -3443,7 +3442,7 @@ static void srb_done(struct AdapterCtlBlk *acb, struct 
DeviceCtlBlk *dcb,
 
srb->adapter_status = 0;
srb->target_status = 0;
-   SET_RES_DID(cmd->result, DID_OK);
+   set_host_byte(cmd, DID_OK);
}
}
 
diff --git a/drivers/scsi/megaraid.c b/drivers/scsi/megaraid.c
index 4d769db4435a..284bc8c4c6d5 100644
--- a/drivers/scsi/megaraid.c
+++ b/drivers/scsi/megaraid.c
@@ -1473,7 +1473,8 @@ mega_cmd_done(adapter_t *adapter, u8 completed[], int 
nstatus, int status)
"aborted cmd [%x] complete\n",
scb->idx);
 
-   scb->cmd->result = (DID_ABORT << 16);
+   scb->cmd->result = 0;
+   set_host_byte(scb->cmd, DID_ABORT);
 
list_add_tail(SCSI_LIST(scb->cmd),
>completed_list);
@@ -1492,7 +1493,8 @@ mega_cmd_done(adapter_t *adapter, u8 completed[], int 
nstatus, int status)
"reset cmd [%x] complete\n",
scb->idx);
 
-   scb->cmd->result = (DID_RESET << 16);
+   scb->cmd->result = 0;
+   set_host_byte(scb->cmd, DID_RESET);
 
list_add_tail(SCSI_LIST(scb->cmd),
>completed_list);
diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index 946039117bf4..7e80f457367e 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -1699,6 +1699,8 @@ int scsi_noretry_cmd(struct scsi_cmnd *scmd)
/* fall through */
case DID_SOFT_ERROR:
return (scmd->request->cmd_flags & REQ_FAILFAST_DRIVER);
+   default:
+   break;
}
 
if (status_byte(scmd->result) != CHECK_CONDITION)
diff --git a/include/scsi/scsi.h b/include/scsi/scsi.h
index eb7853c1a23b..4dfc5e11a5b2 100644
--- a/include/scsi/scsi.h
+++ b/include/scsi/scsi.h
@@ -132,33 +132,35 @@ static inline int scsi_is_wlun(u64 lun)
 /*
  * Host byte codes
  */
-
-#define DID_OK  0x00   /* NO error*/
-#define DID_NO_CONNECT  0x01   /* Couldn't connect before timeout period  */
-#define DID_BUS_BUSY0x02   /* BUS stayed busy through time out period */
-#define DID_TIME_OUT0x03   /* TIMED OUT for other reason  */
-#define DID_BAD_TARGET  0x04   /* BAD target. */
-#define DID_ABORT   0x05   /* Told to abort for some other reason */
-#define DID_PARITY  0x06   /* Parity error*/
-#define DID_ERROR   0x07   /* Internal error  */
-#define DID_RESET   0x08   /* Reset by somebody.  */
-#define DID_BAD_INTR0x09   /* Got an interrupt we weren't expecting.  */
-#define DID_PASSTHROUGH 0x0a   /* Force command past mid-layer*/
-#define DID_SOFT_ERROR  0x0b   /* The low level driver just wish a retry  */
-#define DID_IMM_RETRY   0x0c   /* Retry without decrementing retry count  */
-#define DID_REQUEUE0x0d/* Requeue command (no immediate retry) also
+enum scsi_host_byte {
+   DID_OK, /* NO error*/
+   DID_NO_CONNECT, /* Couldn't connect before timeout period  */
+   DID_BUS_BUSY,   /* BUS stayed busy through time out period */
+   DID_TIME_OUT,   /* TIMED OUT for other reason  */
+   DID_BAD_TARGET, /* BAD target. */
+   DID_ABORT,  /* Told to abort for some other reason */
+   DID_PARITY,   

[VERY EARLY RFC 02/13] scsi: remove Scsi_Cmnd typedef

[Johannes Thumshirn]

This will make subsequent refactoring easier to handle.


@rem_Scsi_Cmnd@
@@
- typedef struct scsi_cmnd Scsi_Cmnd;

@fix_Scsi_Cmnd@
typedef Scsi_Cmnd;
@@
-Scsi_Cmnd
+struct scsi_cmnd

@fix_sizeof_Scsi_Cmnd@
@@
-sizeof(Scsi_Cmnd)
+sizeof(struct scsi_cmnd)



Note: this patch is nowhere checkpatch clean.

Signed-off-by: Johannes Thumshirn 
---
 drivers/scsi/3w-.c   |  2 +-
 drivers/scsi/advansys.c  |  2 +-
 drivers/scsi/aha152x.c   | 71 +---
 drivers/scsi/aha1740.c   |  9 ++---
 drivers/scsi/aha1740.h   |  4 +--
 drivers/scsi/gdth.c  | 67 +++--
 drivers/scsi/gdth.h  | 10 +++---
 drivers/scsi/gdth_proc.c |  2 +-
 drivers/scsi/ibmvscsi/ibmvfc.c   |  2 +-
 drivers/scsi/ibmvscsi/ibmvscsi.c |  2 +-
 drivers/scsi/libiscsi_tcp.c  |  2 +-
 drivers/scsi/megaraid.c  | 29 
 drivers/scsi/megaraid.h  | 14 
 drivers/scsi/nsp32_debug.c   |  2 +-
 drivers/scsi/scsi.h  |  3 --
 drivers/scsi/scsi_typedefs.h |  2 --
 16 files changed, 114 insertions(+), 109 deletions(-)
 delete mode 100644 drivers/scsi/scsi_typedefs.h

diff --git a/drivers/scsi/3w-.c b/drivers/scsi/3w-.c
index 33261b690774..2a9a1326023b 100644
--- a/drivers/scsi/3w-.c
+++ b/drivers/scsi/3w-.c
@@ -1922,7 +1922,7 @@ static int tw_scsi_queue_lck(struct scsi_cmnd *SCpnt, 
void (*done)(struct scsi_c
if (test_bit(TW_IN_RESET, _dev->flags))
return SCSI_MLQUEUE_HOST_BUSY;
 
-   /* Save done function into Scsi_Cmnd struct */
+   /* Save done function into struct scsi_cmnd struct */
SCpnt->scsi_done = done;
 
/* Queue the command and get a request id */
diff --git a/drivers/scsi/advansys.c b/drivers/scsi/advansys.c
index 24e57e770432..c9a52905070e 100644
--- a/drivers/scsi/advansys.c
+++ b/drivers/scsi/advansys.c
@@ -8466,7 +8466,7 @@ static int AdvExeScsiQueue(ADV_DVC_VAR *asc_dvc, 
adv_req_t *reqp)
 }
 
 /*
- * Execute a single 'Scsi_Cmnd'.
+ * Execute a single 'struct scsi_cmnd'.
  */
 static int asc_execute_scsi_cmnd(struct scsi_cmnd *scp)
 {
diff --git a/drivers/scsi/aha152x.c b/drivers/scsi/aha152x.c
index bc0058df31c6..4d7b0e0adbf7 100644
--- a/drivers/scsi/aha152x.c
+++ b/drivers/scsi/aha152x.c
@@ -422,16 +422,16 @@ enum aha152x_state {
  *
  */
 struct aha152x_hostdata {
-   Scsi_Cmnd *issue_SC;
+   struct scsi_cmnd *issue_SC;
/* pending commands to issue */
 
-   Scsi_Cmnd *current_SC;
+   struct scsi_cmnd *current_SC;
/* current command on the bus */
 
-   Scsi_Cmnd *disconnected_SC;
+   struct scsi_cmnd *disconnected_SC;
/* commands that disconnected */
 
-   Scsi_Cmnd *done_SC;
+   struct scsi_cmnd *done_SC;
/* command that was completed */
 
spinlock_t lock;
@@ -510,7 +510,7 @@ struct aha152x_hostdata {
  *
  */
 struct aha152x_scdata {
-   Scsi_Cmnd *next;/* next sc in queue */
+   struct scsi_cmnd *next; /* next sc in queue */
struct completion *done;/* semaphore to block on */
struct scsi_eh_save ses;
 };
@@ -633,7 +633,7 @@ static void aha152x_error(struct Scsi_Host *shpnt, char 
*msg);
 static void done(struct Scsi_Host *shpnt, int error);
 
 /* diagnostics */
-static void show_command(Scsi_Cmnd * ptr);
+static void show_command(struct scsi_cmnd * ptr);
 static void show_queues(struct Scsi_Host *shpnt);
 static void disp_enintr(struct Scsi_Host *shpnt);
 
@@ -642,9 +642,9 @@ static void disp_enintr(struct Scsi_Host *shpnt);
  *  queue services:
  *
  */
-static inline void append_SC(Scsi_Cmnd **SC, Scsi_Cmnd *new_SC)
+static inline void append_SC(struct scsi_cmnd **SC, struct scsi_cmnd *new_SC)
 {
-   Scsi_Cmnd *end;
+   struct scsi_cmnd *end;
 
SCNEXT(new_SC) = NULL;
if (!*SC)
@@ -656,9 +656,9 @@ static inline void append_SC(Scsi_Cmnd **SC, Scsi_Cmnd 
*new_SC)
}
 }
 
-static inline Scsi_Cmnd *remove_first_SC(Scsi_Cmnd ** SC)
+static inline struct scsi_cmnd *remove_first_SC(struct scsi_cmnd ** SC)
 {
-   Scsi_Cmnd *ptr;
+   struct scsi_cmnd *ptr;
 
ptr = *SC;
if (ptr) {
@@ -668,9 +668,10 @@ static inline Scsi_Cmnd *remove_first_SC(Scsi_Cmnd ** SC)
return ptr;
 }
 
-static inline Scsi_Cmnd *remove_lun_SC(Scsi_Cmnd ** SC, int target, int lun)
+static inline struct scsi_cmnd *remove_lun_SC(struct scsi_cmnd ** SC,
+ int target, int lun)
 {
-   Scsi_Cmnd *ptr, *prev;
+   struct scsi_cmnd *ptr, *prev;
 
for (ptr = *SC, prev = NULL;
 ptr && ((ptr->device->id != target) || (ptr->device->lun != lun));
@@ -689,9 +690,10 @@ static inline Scsi_Cmnd *remove_lun_SC(Scsi_Cmnd ** SC, 
int target, int lun)
return ptr;
 }
 
-static inline Scsi_Cmnd *remove_SC(Scsi_Cmnd **SC, Scsi_Cmnd *SCp)

[VERY EARLY RFC 07/13] scsi: use set_driver_byte

[Johannes Thumshirn]

@@
struct scsi_cmnd *c;
expression E1, E2;
@@
(
- c->result = E1 << 24 | E2;
+ c->result = 0;
+ set_driver_byte(c, E1);
+ c->result |= E2;
|
- c->result |= E1 << 24 | E2;
+ set_driver_byte(c, E1);
+ c->result |= E2;
|
- c->result = E1 << 24;
+ c->result = 0;
+ set_driver_byte(c, E1);
|
- c->result |= E1 << 24;
+ set_driver_byte(c, E1);
)


Signed-off-by: Johannes Thumshirn 
---
 drivers/ata/libata-scsi.c   | 2 +-
 drivers/scsi/advansys.c | 1 -
 drivers/scsi/aic7xxx/aic79xx_osm.c  | 2 +-
 drivers/scsi/aic7xxx/aic7xxx_osm.c  | 2 +-
 drivers/scsi/arcmsr/arcmsr_hba.c| 2 +-
 drivers/scsi/megaraid/megaraid_sas_base.c   | 2 +-
 drivers/scsi/megaraid/megaraid_sas_fusion.c | 2 +-
 drivers/scsi/mvumi.c| 5 +++--
 drivers/scsi/scsi.c | 2 +-
 drivers/scsi/scsi_error.c   | 2 +-
 drivers/scsi/vmw_pvscsi.c   | 4 ++--
 11 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index f34650ada9d7..f019de473a97 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -4475,7 +4475,7 @@ void ata_scsi_simulate(struct ata_device *dev, struct 
scsi_cmnd *cmd)
 
case REQUEST_SENSE:
ata_scsi_set_sense(dev, cmd, 0, 0, 0);
-   cmd->result = (DRIVER_SENSE << 24);
+   set_driver_byte(cmd, DRIVER_SENSE);
break;
 
/* if we reach this, then writeback caching is disabled,
diff --git a/drivers/scsi/advansys.c b/drivers/scsi/advansys.c
index 0f93fe6c58b0..0e3f464e011d 100644
--- a/drivers/scsi/advansys.c
+++ b/drivers/scsi/advansys.c
@@ -2093,7 +2093,6 @@ do { \
 #define STATUS_BYTE(byte)   (byte)
 #define MSG_BYTE(byte)  ((byte) << 8)
 #define HOST_BYTE(byte) ((byte) << 16)
-#define DRIVER_BYTE(byte)   ((byte) << 24)
 
 #define ASC_STATS(shost, counter) ASC_STATS_ADD(shost, counter, 1)
 #ifndef ADVANSYS_STATS
diff --git a/drivers/scsi/aic7xxx/aic79xx_osm.c 
b/drivers/scsi/aic7xxx/aic79xx_osm.c
index 2588b8f84ba0..af748242cdb3 100644
--- a/drivers/scsi/aic7xxx/aic79xx_osm.c
+++ b/drivers/scsi/aic7xxx/aic79xx_osm.c
@@ -1941,7 +1941,7 @@ ahd_linux_handle_scsi_status(struct ahd_softc *ahd,
memcpy(cmd->sense_buffer,
   ahd_get_sense_buf(ahd, scb)
   + sense_offset, sense_size);
-   cmd->result |= (DRIVER_SENSE << 24);
+   set_driver_byte(cmd, DRIVER_SENSE);
 
 #ifdef AHD_DEBUG
if (ahd_debug & AHD_SHOW_SENSE) {
diff --git a/drivers/scsi/aic7xxx/aic7xxx_osm.c 
b/drivers/scsi/aic7xxx/aic7xxx_osm.c
index c6be3aeb302b..ac204238b5c1 100644
--- a/drivers/scsi/aic7xxx/aic7xxx_osm.c
+++ b/drivers/scsi/aic7xxx/aic7xxx_osm.c
@@ -1856,7 +1856,7 @@ ahc_linux_handle_scsi_status(struct ahc_softc *ahc,
if (sense_size < SCSI_SENSE_BUFFERSIZE)
memset(>sense_buffer[sense_size], 0,
   SCSI_SENSE_BUFFERSIZE - sense_size);
-   cmd->result |= (DRIVER_SENSE << 24);
+   set_driver_byte(cmd, DRIVER_SENSE);
 #ifdef AHC_DEBUG
if (ahc_debug & AHC_SHOW_SENSE) {
int i;
diff --git a/drivers/scsi/arcmsr/arcmsr_hba.c b/drivers/scsi/arcmsr/arcmsr_hba.c
index a99d44d36e5f..9fd87860ef63 100644
--- a/drivers/scsi/arcmsr/arcmsr_hba.c
+++ b/drivers/scsi/arcmsr/arcmsr_hba.c
@@ -1211,7 +1211,7 @@ static void arcmsr_report_sense_info(struct 
CommandControlBlock *ccb)
memcpy(sensebuffer, ccb->arcmsr_cdb.SenseData, 
sense_data_length);
sensebuffer->ErrorCode = SCSI_SENSE_CURRENT_ERRORS;
sensebuffer->Valid = 1;
-   pcmd->result |= (DRIVER_SENSE << 24);
+   set_driver_byte(pcmd, DRIVER_SENSE);
}
 }
 
diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c 
b/drivers/scsi/megaraid/megaraid_sas_base.c
index 9c3dec24b6bf..6861e8d1aea5 100644
--- a/drivers/scsi/megaraid/megaraid_sas_base.c
+++ b/drivers/scsi/megaraid/megaraid_sas_base.c
@@ -3309,7 +3309,7 @@ megasas_complete_cmd(struct megasas_instance *instance, 
struct megasas_cmd *cmd,
memcpy(cmd->scmd->sense_buffer, cmd->sense,
   hdr->sense_len);
 
-   cmd->scmd->result |= DRIVER_SENSE << 24;
+   set_driver_byte(cmd->scmd, DRIVER_SENSE);
}
 
break;
diff --git a/drivers/scsi/megaraid/megaraid_sas_fusion.c 
b/drivers/scsi/megaraid/megaraid_sas_fusion.c
index 8c5757d5770a..5ffeb21872f3 100644
--- a/drivers/scsi/megaraid/megaraid_sas_fusion.c
+++ b/drivers/scsi/megaraid/megaraid_sas_fusion.c
@@ -1795,7 +1795,7 @@ map_cmd_status(struct fusion_context 

[VERY EARLY RFC 00/13] Rework SCSI results handling

[Johannes Thumshirn]

Hey all,

here's a early preview of my SCSI results rework so we can eventually
discuss things next week at LSF/MM (it still has compiler errors on
aic7xxx and scsi_debug).

The motivation behing this is that some drivers have failed to set the
scsi_cmnd::result bytes correctly in the past and this is resulting in
hard to case down errors.

The open points:
1) 148 files changed, treewide. That's huge. Is it worth it?
2) remove the old status byte definitions
3) add a scsi_cmnd::result == 0 wrapper
3) convert aic7xx's CAM stuff so this series compiles cleanly
4) What the heck is the SDEG_RES_IMMED_MASK stuff in scsi_debug doing
5) change scsi_execute() so we get a newish 'struct scsi_results' instead of an 
int
6) {to,from}_scsi_result() are odd
7) find suitable commit messages

In case someone want's it in a more viewable form I've pushed the
series to my kernel.org git:
https://git.kernel.org/pub/scm/linux/kernel/git/jth/linux.git/log/?h=scsi-results

Johannes Thumshirn (13):
  scsi: use host_byte() accessor
  scsi: remove Scsi_Cmnd typedef
  scsi: add enum for host byte codes
  scsi: add enum for driver byte codes
  scsi: add enum for message byte codes
  scsi: introduce set_scsi_result
  scsi: use set_driver_byte
  treewide: use set_host_byte
  scsi: use set_msg_byte
  scsi: introduce set_status_byte and convert LLDDs to use it
  scsi: Change status bytes to use SAM-3 version
  reewide: introduce clear_scsi_result() and convert drivers
  scsi: introduce struct scsi_result

 arch/ia64/hp/sim/simscsi.c  |   6 +-
 block/bsg-lib.c |   8 +-
 block/bsg.c |   8 +-
 block/scsi_ioctl.c  |  12 +-
 drivers/ata/libata-scsi.c   |  54 +++---
 drivers/firewire/sbp2.c |   2 +-
 drivers/infiniband/ulp/srp/ib_srp.c |  23 ++-
 drivers/message/fusion/mptfc.c  |   8 +-
 drivers/message/fusion/mptsas.c |   3 +-
 drivers/message/fusion/mptscsih.c   | 122 -
 drivers/message/fusion/mptspi.c |   6 +-
 drivers/s390/scsi/zfcp_fc.h |   2 +-
 drivers/s390/scsi/zfcp_scsi.c   |   4 +-
 drivers/scsi/3w-9xxx.c  |  18 +-
 drivers/scsi/3w-sas.c   |  16 +-
 drivers/scsi/3w-.c  |  36 ++--
 drivers/scsi/53c700.c   |   3 +-
 drivers/scsi/BusLogic.c |  24 ++-
 drivers/scsi/NCR5380.c  |  39 +++--
 drivers/scsi/a100u2w.c  |   2 +-
 drivers/scsi/aacraid/aachba.c   | 221 +---
 drivers/scsi/aacraid/commsup.c  |   5 +-
 drivers/scsi/aacraid/linit.c|  12 +-
 drivers/scsi/advansys.c |  58 +++
 drivers/scsi/aha152x.c  |  76 
 drivers/scsi/aha1542.c  |   2 +-
 drivers/scsi/aha1740.c  |  11 +-
 drivers/scsi/aha1740.h  |   4 +-
 drivers/scsi/aic7xxx/aic79xx_osm.c  |   5 +-
 drivers/scsi/aic7xxx/aic79xx_osm.h  |   6 +-
 drivers/scsi/aic7xxx/aic7xxx_osm.c  |   5 +-
 drivers/scsi/aic7xxx/aic7xxx_osm.h  |   6 +-
 drivers/scsi/arcmsr/arcmsr_hba.c|  56 +++---
 drivers/scsi/arm/acornscsi.c|  11 +-
 drivers/scsi/arm/fas216.c   |   6 +-
 drivers/scsi/atp870u.c  |  18 +-
 drivers/scsi/be2iscsi/be_main.c |   8 +-
 drivers/scsi/bfa/bfad_im.c  |  38 ++--
 drivers/scsi/bfa/bfad_im.h  |   1 -
 drivers/scsi/bnx2fc/bnx2fc_io.c |  17 +-
 drivers/scsi/ch.c   |   2 +-
 drivers/scsi/constants.c|   4 +-
 drivers/scsi/csiostor/csio_scsi.c   |  12 +-
 drivers/scsi/cxlflash/main.c|  42 +++--
 drivers/scsi/dc395x.c   |  57 +++---
 drivers/scsi/device_handler/scsi_dh_alua.c  |  10 +-
 drivers/scsi/dpt_i2o.c  |  45 +++--
 drivers/scsi/esas2r/esas2r_main.c   |  19 +-
 drivers/scsi/esp_scsi.c |  18 +-
 drivers/scsi/fnic/fnic_scsi.c   |  48 +++--
 drivers/scsi/gdth.c | 105 ++-
 drivers/scsi/gdth.h |  10 +-
 drivers/scsi/gdth_proc.c|   2 +-
 drivers/scsi/hpsa.c |  94 ++
 drivers/scsi/hptiop.c   |  27 +--
 drivers/scsi/ibmvscsi/ibmvfc.c  |  26 ++-
 drivers/scsi/ibmvscsi/ibmvscsi.c|  28 +--
 drivers/scsi/imm.c  |  12 +-
 drivers/scsi/initio.c   

[VERY EARLY RFC 09/13] scsi: use set_msg_byte

[Johannes Thumshirn]

@@
struct scsi_cmnd *c;
expression E1, E2;
@@
(
- c->result = E1 << 8 | E2;
+ c->result = 0;
+ set_msg_byte(c, E1);
+ c->result |= E2;
|
- c->result |= E1 << 8 | E2;
+ set_msg_byte(c, E1);
+ c->result |= E2;
|
- c->result = E1 << 8;
+ c->result = 0;
+ set_msg_byte(c, E1);
|
- c->result |= E1 << 8;
+ c->result = 0;
+ set_msg_byte(c, E1);
|
- c->result = (E1 << 8);
+ set_msg_byte(c, E1);
|
- c->result |= (E1 << 8);
+ set_msg_byte(c, E1);
)


Signed-off-by: Johannes Thumshirn 
---
 drivers/scsi/advansys.c | 1 -
 drivers/scsi/hpsa.c | 4 ++--
 drivers/scsi/mesh.c | 2 +-
 3 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/advansys.c b/drivers/scsi/advansys.c
index 846917538e2b..63c0d97d52a0 100644
--- a/drivers/scsi/advansys.c
+++ b/drivers/scsi/advansys.c
@@ -2091,7 +2091,6 @@ do { \
 
 /* struct scsi_cmnd function return codes */
 #define STATUS_BYTE(byte)   (byte)
-#define MSG_BYTE(byte)  ((byte) << 8)
 
 #define ASC_STATS(shost, counter) ASC_STATS_ADD(shost, counter, 1)
 #ifndef ADVANSYS_STATS
diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index c27d1ef7158b..0a4aa7ad4437 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -2365,7 +2365,7 @@ static int handle_ioaccel_mode2_error(struct ctlr_info *h,
case IOACCEL2_STATUS_SR_UNDERRUN:
cmd->result = 0;
set_host_byte(cmd, DID_OK); /* host byte */
-   cmd->result |= (COMMAND_COMPLETE << 8); /* msg byte */
+   set_msg_byte(cmd, COMMAND_COMPLETE);/* msg byte */
ioaccel2_resid = get_unaligned_le32(
>error_data.resid_cnt[0]);
scsi_set_resid(cmd, ioaccel2_resid);
@@ -2575,7 +2575,7 @@ static void complete_scsi_command(struct CommandList *cp)
 
cmd->result = 0;
set_host_byte(cmd, DID_OK); /* host byte */
-   cmd->result |= (COMMAND_COMPLETE << 8); /* msg byte */
+   set_msg_byte(cmd, COMMAND_COMPLETE);/* msg byte */
 
if (cp->cmd_type == CMD_IOACCEL2 || cp->cmd_type == CMD_IOACCEL1) {
if (dev->physical_device && dev->expose_device &&
diff --git a/drivers/scsi/mesh.c b/drivers/scsi/mesh.c
index 97c6eb8e5ae8..fee7ada36d52 100644
--- a/drivers/scsi/mesh.c
+++ b/drivers/scsi/mesh.c
@@ -596,7 +596,7 @@ static void mesh_done(struct mesh_state *ms, int start_next)
if (cmd) {
set_scsi_result(cmd, 0, ms->stat, 0, cmd->SCp.Status);
if (ms->stat == DID_OK)
-   cmd->result += (cmd->SCp.Message << 8);
+   set_msg_byte(cmd, cmd->SCp.Message);
if (DEBUG_TARGET(cmd)) {
printk(KERN_DEBUG "mesh_done: result = %x, data_ptr=%d, 
buflen=%d\n",
   cmd->result, ms->data_ptr, scsi_bufflen(cmd));
-- 
2.16.3

[VERY EARLY RFC 01/13] scsi: use host_byte() accessor

[Johannes Thumshirn]

Use the host_byte() accessor for getting the SCSI host byte.

Signed-off-by: Johannes Thumshirn 
---
 drivers/scsi/aic7xxx/aic79xx_osm.h | 2 +-
 drivers/scsi/aic7xxx/aic7xxx_osm.h | 2 +-
 drivers/scsi/dc395x.c  | 2 +-
 drivers/scsi/imm.c | 4 ++--
 drivers/scsi/ppa.c | 4 ++--
 5 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/aic7xxx/aic79xx_osm.h 
b/drivers/scsi/aic7xxx/aic79xx_osm.h
index 8a8b7ae7aed3..0a84a846ca88 100644
--- a/drivers/scsi/aic7xxx/aic79xx_osm.h
+++ b/drivers/scsi/aic7xxx/aic79xx_osm.h
@@ -550,7 +550,7 @@ void ahd_set_scsi_status(struct scb *scb, uint32_t status)
 static inline
 uint32_t ahd_cmd_get_transaction_status(struct scsi_cmnd *cmd)
 {
-   return ((cmd->result >> 16) & CAM_STATUS_MASK);
+   return host_byte(cmd->result) & CAM_STATUS_MASK;
 }
 
 static inline
diff --git a/drivers/scsi/aic7xxx/aic7xxx_osm.h 
b/drivers/scsi/aic7xxx/aic7xxx_osm.h
index f8489078f003..108b30e92c75 100644
--- a/drivers/scsi/aic7xxx/aic7xxx_osm.h
+++ b/drivers/scsi/aic7xxx/aic7xxx_osm.h
@@ -568,7 +568,7 @@ void ahc_set_scsi_status(struct scb *scb, uint32_t status)
 static inline
 uint32_t ahc_cmd_get_transaction_status(struct scsi_cmnd *cmd)
 {
-   return ((cmd->result >> 16) & CAM_STATUS_MASK);
+   return host_byte(cmd->result) & CAM_STATUS_MASK;
 }
 
 static inline
diff --git a/drivers/scsi/dc395x.c b/drivers/scsi/dc395x.c
index 60ef8df42b95..da28f08ae185 100644
--- a/drivers/scsi/dc395x.c
+++ b/drivers/scsi/dc395x.c
@@ -3473,7 +3473,7 @@ static void srb_done(struct AdapterCtlBlk *acb, struct 
DeviceCtlBlk *dcb,
 
/*if( srb->cmd->cmnd[0] == INQUIRY && */
/*  (host_byte(cmd->result) == DID_OK || status_byte(cmd->result) & 
CHECK_CONDITION) ) */
-   if ((cmd->result == (DID_OK << 16)
+   if ((host_byte(cmd->result) == DID_OK
 || status_byte(cmd->result) &
 CHECK_CONDITION)) {
if (!dcb->init_tcq_flag) {
diff --git a/drivers/scsi/imm.c b/drivers/scsi/imm.c
index 87c94191033b..690c0e25ea51 100644
--- a/drivers/scsi/imm.c
+++ b/drivers/scsi/imm.c
@@ -728,7 +728,7 @@ static void imm_interrupt(struct work_struct *work)
}
/* Command must of completed hence it is safe to let go... */
 #if IMM_DEBUG > 0
-   switch ((cmd->result >> 16) & 0xff) {
+   switch (host_byte(cmd->result)) {
case DID_OK:
break;
case DID_NO_CONNECT:
@@ -757,7 +757,7 @@ static void imm_interrupt(struct work_struct *work)
break;
default:
printk("imm: bad return code (%02x)\n",
-  (cmd->result >> 16) & 0xff);
+  host_byte(cmd->result));
}
 #endif
 
diff --git a/drivers/scsi/ppa.c b/drivers/scsi/ppa.c
index ee86a0c62dbf..f22fe4939a74 100644
--- a/drivers/scsi/ppa.c
+++ b/drivers/scsi/ppa.c
@@ -625,7 +625,7 @@ static void ppa_interrupt(struct work_struct *work)
}
/* Command must of completed hence it is safe to let go... */
 #if PPA_DEBUG > 0
-   switch ((cmd->result >> 16) & 0xff) {
+   switch (host_byte(cmd->result)) {
case DID_OK:
break;
case DID_NO_CONNECT:
@@ -654,7 +654,7 @@ static void ppa_interrupt(struct work_struct *work)
break;
default:
printk(KERN_WARNING "ppa: bad return code (%02x)\n",
-  (cmd->result >> 16) & 0xff);
+  host_byte(cmd->result));
}
 #endif
 
-- 
2.16.3

[VERY EARLY RFC 04/13] scsi: add enum for driver byte codes

[Johannes Thumshirn]

Add enum for driver byte codes and adopt set_driver_byte()'s
signature.

Signed-off-by: Johannes Thumshirn 
Suggested-by: Bart Van Assche 
---
 include/scsi/scsi.h  | 33 -
 include/scsi/scsi_cmnd.h |  3 ++-
 2 files changed, 22 insertions(+), 14 deletions(-)

diff --git a/include/scsi/scsi.h b/include/scsi/scsi.h
index 4dfc5e11a5b2..2694faafaddf 100644
--- a/include/scsi/scsi.h
+++ b/include/scsi/scsi.h
@@ -161,21 +161,24 @@ enum scsi_host_byte {
DID_ALLOC_FAILURE,  /* Space allocation on the device failed */
DID_MEDIUM_ERROR,   /* Medium error */
 };
-#define DRIVER_OK   0x00   /* Driver status   */
 
-/*
- *  These indicate the error that occurred, and what is available.
- */
+enum scsi_driver_byte {
+   DRIVER_OK,  /* Driver status */
 
-#define DRIVER_BUSY 0x01
-#define DRIVER_SOFT 0x02
-#define DRIVER_MEDIA0x03
-#define DRIVER_ERROR0x04
+   /*
+*  These indicate the error that occurred, and what is available.
+*/
 
-#define DRIVER_INVALID  0x05
-#define DRIVER_TIMEOUT  0x06
-#define DRIVER_HARD 0x07
-#define DRIVER_SENSE   0x08
+   DRIVER_BUSY,
+   DRIVER_SOFT,
+   DRIVER_MEDIA,
+   DRIVER_ERROR,
+
+   DRIVER_INVALID,
+   DRIVER_TIMEOUT,
+   DRIVER_HARD,
+   DRIVER_SENSE,
+};
 
 /*
  * Internal return values.
@@ -215,7 +218,11 @@ static inline enum scsi_host_byte host_byte(int result)
 {
return (result >> 16) & 0xff;
 }
-#define driver_byte(result) (((result) >> 24) & 0xff)
+
+static inline enum scsi_driver_byte driver_byte(int result)
+{
+   return (result >> 24) & 0xff;
+}
 
 #define sense_class(sense)  (((sense) >> 4) & 0x7)
 #define sense_error(sense)  ((sense) & 0xf)
diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
index b678cd99b12b..da8ea89ccc0a 100644
--- a/include/scsi/scsi_cmnd.h
+++ b/include/scsi/scsi_cmnd.h
@@ -348,7 +348,8 @@ static inline void set_host_byte(struct scsi_cmnd *cmd,
cmd->result = (cmd->result & 0xff00) | (status << 16);
 }
 
-static inline void set_driver_byte(struct scsi_cmnd *cmd, char status)
+static inline void set_driver_byte(struct scsi_cmnd *cmd,
+  enum scsi_driver_byte status)
 {
cmd->result = (cmd->result & 0x00ff) | (status << 24);
 }
-- 
2.16.3

[Bug 199435] HPSA + P420i resetting logical Direct-Access never complete

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=199435

--- Comment #2 from Anthony Hausman (anthonyhaussm...@gmail.com) ---
I don't have any "Controller lockup detected" message in the Syslog
unfortunately.
On the ilo IML log, the last message was about the cache module:

CAUTION: POST Messages - POST Error: 1792-Slot X Drive Array - Valid Data Found
in Cache Module. Data will automatically be written to drive array..

I have nothing about lockup entries.

Indeed, we use the driver from the last kernel and compiled it for 4.11.
I am ready to test the patch you are proposing. 
Where can I retrieve it?

-- 
You are receiving this mail because:
You are the assignee for the bug.

Re: usercopy whitelist woe in scsi_sense_cache

[Jens Axboe]

On 4/18/18 3:08 AM, Paolo Valente wrote:
> 
> 
>> Il giorno 18 apr 2018, alle ore 00:57, Jens Axboe  ha 
>> scritto:
>>
>> On 4/17/18 3:48 PM, Jens Axboe wrote:
>>> On 4/17/18 3:47 PM, Kees Cook wrote:
 On Tue, Apr 17, 2018 at 2:39 PM, Jens Axboe  wrote:
> On 4/17/18 3:25 PM, Kees Cook wrote:
>> On Tue, Apr 17, 2018 at 1:46 PM, Kees Cook  wrote:
>>> I see elv.priv[1] assignments made in a few places -- is it possible
>>> there is some kind of uninitialized-but-not-NULL state that can leak
>>> in there?
>>
>> Got it. This fixes it for me:
>>
>> diff --git a/block/blk-mq.c b/block/blk-mq.c
>> index 0dc9e341c2a7..859df3160303 100644
>> --- a/block/blk-mq.c
>> +++ b/block/blk-mq.c
>> @@ -363,7 +363,7 @@ static struct request *blk_mq_get_request(struct
>> request_queue *q,
>>
>>rq = blk_mq_rq_ctx_init(data, tag, op);
>>if (!op_is_flush(op)) {
>> -   rq->elv.icq = NULL;
>> +   memset(>elv, 0, sizeof(rq->elv));
>>if (e && e->type->ops.mq.prepare_request) {
>>if (e->type->icq_cache && rq_ioc(bio))
>>blk_mq_sched_assign_ioc(rq, bio);
>> @@ -461,7 +461,7 @@ void blk_mq_free_request(struct request *rq)
>>e->type->ops.mq.finish_request(rq);
>>if (rq->elv.icq) {
>>put_io_context(rq->elv.icq->ioc);
>> -   rq->elv.icq = NULL;
>> +   memset(>elv, 0, sizeof(rq->elv));
>>}
>>}
>
> This looks like a BFQ problem, this should not be necessary. Paolo,
> you're calling your own prepare request handler from the insert
> as well, and your prepare request does nothing if rq->elv.icq == NULL.

 I sent the patch anyway, since it's kind of a robustness improvement,
 I'd hope. If you fix BFQ also, please add:
>>>
>>> It's also a memset() in the hot path, would prefer to avoid that...
>>> The issue here is really the convoluted bfq usage of insert/prepare,
>>> I'm sure Paolo can take it from here.
>>
> 
> Hi,
> I'm very sorry for tuning in very late, but, at the same time, very
> glad to find the problem probably already solved ;) (in this respect, I swear,
> my delay was not intentional)
> 
>> Does this fix it?
>>
>> diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
>> index f0ecd98509d8..d883469a1582 100644
>> --- a/block/bfq-iosched.c
>> +++ b/block/bfq-iosched.c
>> @@ -4934,8 +4934,11 @@ static void bfq_prepare_request(struct request *rq, 
>> struct bio *bio)
>>  bool new_queue = false;
>>  bool bfqq_already_existing = false, split = false;
>>
>> -if (!rq->elv.icq)
>> +if (!rq->elv.icq) {
>> +rq->elv.priv[0] = rq->elv.priv[1] = NULL;
>>  return;
>> +}
>> +
> 
> This does solve the problem at hand.  But it also arouses a question,
> related to a possible subtle bug.
> 
> For BFQ, !rq->elv.icq basically means "this request is not for me, as
> I am an icq-based scheduler".  But, IIUC the main points in this
> thread, then this assumption is false.  If it is actually false, then
> I hope that all requests with !rq->elv.icq that are sent to BFQ do
> verify the condition (at_head || blk_rq_is_passthrough(rq)).  In fact,
> requests that do not verify that condition are those that BFQ must put
> in a bfq_queue.  So, even if this patch makes the crash disappear, we
> drive BFQ completely crazy (and we may expect other strange failures)
> if we send BFQ a request with !((at_head || blk_rq_is_passthrough(rq))
> and !rq->elv.icq.  BFQ has to put that rq into a bfq_queue, but simply
> cannot.
> 
> Jens, or any other, could you please shed a light on this, and explain
> how things are exactly?

Your assumption is correct, however you set ->priv[0] and ->priv[1] for
requests, but only for ->elv.icq != NULL. So let's assume you get a
request and assign those two, request completes. Later on, you get
the same request, bypass insert it. BFQ doesn't clear the bic/bfqq
pointers in the request, since ->elv.icq == NULL. It gets inserted
into the dispatch list.

Then when __bfq_dispatch_request() is called, you do:

bfqq = RQ_BFQQ(rq);
if (bfqq)
bfqq->dispatched++;
[...]

which is wrong, since you don't know if you assigned a bfqq for this
request. The memory that bfqq points to could be long gone, if that
queue is freed. So you could either guard any bfqq/bic retrieval
with ->elv.icq != NULL, or you could just clear the pointers for
the case where the values aren't valid.

-- 
Jens Axboe

[Bug 199435] HPSA + P420i resetting logical Direct-Access never complete

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=199435

Don (don.br...@microsemi.com) changed:

   What|Removed |Added

 CC||don.br...@microsemi.com

--- Comment #1 from Don (don.br...@microsemi.com) ---
Do you see any lockup messages in the console logs?
"Controller lockup detected"...


The driver you used is from 4.16 kernel on a 4.11 kernel? I have not tested
this configuration.

I notice that the driver is still using the kernel work-queue for monitoring. I
will be sending up a patch to change this to local work-queues soon. Perhaps
you can test this patch? It may help to discover more information on what is
happening.

Also, after you rebooted, were there any lockup entries in the ilo IML log?

-- 
You are receiving this mail because:
You are the assignee for the bug.

sr 6:0:0:0: [sr0] tag#0 FAILED Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE

[Cristian]

Hello,

Open bug in launchpad.net
https://bugs.launchpad.net/bugs/1765043


dmesg:
[ 4228.290628] sr 6:0:0:0: [sr0] tag#0 FAILED Result: hostbyte=DID_OK
driverbyte=DRIVER_SENSE
[ 4228.290638] sr 6:0:0:0: [sr0] tag#0 Sense Key : Illegal Request [current]
[ 4228.290645] sr 6:0:0:0: [sr0] tag#0 Add. Sense: Illegal mode for this track
[ 4228.290652] sr 6:0:0:0: [sr0] tag#0 CDB: Read(10) 28 00 00 01 58 96
00 00 02 00 00 00
[ 4228.290657] print_req_error: I/O error, dev sr0, sector 352856
[ 4228.290707] attempt to access beyond end of device
[ 4228.290717] unknown-block(11,0): rw=0, want=352860, limit=352856
[ 4228.290723] Buffer I/O error on dev sr0, logical block 88214, async page read
[ 4228.290733] attempt to access beyond end of device
[ 4228.290738] unknown-block(11,0): rw=0, want=352864, limit=352856
[ 4228.290741] Buffer I/O error on dev sr0, logical block 88215, async page read
[ 4230.210353] ISO 9660 Extensions: Microsoft Joliet Level 3
[ 4230.216636] ISO 9660 Extensions: IEEE_1282
[ 4291.030676] VFS: busy inodes on changed media or resized disk sr0

Regards,
--
Cristian Aravena Romero (caravena)

[PATCH] scsi: mptfc: fix spelling mistake in macro names

[Colin King]

From: Colin Ian King 

Rename macros MPI_FCPORTPAGE0_SUPPORT_SPEED_UKNOWN and
MPI_FCPORTPAGE0_CURRENT_SPEED_UKNOWN to add in missing N in UNKNOWN

Signed-off-by: Colin Ian King 
---
 drivers/message/fusion/lsi/mpi_cnfg.h | 4 ++--
 drivers/message/fusion/mptfc.c| 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/message/fusion/lsi/mpi_cnfg.h 
b/drivers/message/fusion/lsi/mpi_cnfg.h
index 4e9c0ce94f27..059997f8ebce 100644
--- a/drivers/message/fusion/lsi/mpi_cnfg.h
+++ b/drivers/message/fusion/lsi/mpi_cnfg.h
@@ -1802,13 +1802,13 @@ typedef struct _CONFIG_PAGE_FC_PORT_0
 #define MPI_FCPORTPAGE0_SUPPORT_CLASS_2 (0x0002)
 #define MPI_FCPORTPAGE0_SUPPORT_CLASS_3 (0x0004)
 
-#define MPI_FCPORTPAGE0_SUPPORT_SPEED_UKNOWN(0x) /* 
(SNIA)HBA_PORTSPEED_UNKNOWN 0   Unknown - transceiver incapable of reporting */
+#define MPI_FCPORTPAGE0_SUPPORT_SPEED_UNKNOWN   (0x) /* 
(SNIA)HBA_PORTSPEED_UNKNOWN 0   Unknown - transceiver incapable of reporting */
 #define MPI_FCPORTPAGE0_SUPPORT_1GBIT_SPEED (0x0001) /* 
(SNIA)HBA_PORTSPEED_1GBIT   1   1 GBit/sec */
 #define MPI_FCPORTPAGE0_SUPPORT_2GBIT_SPEED (0x0002) /* 
(SNIA)HBA_PORTSPEED_2GBIT   2   2 GBit/sec */
 #define MPI_FCPORTPAGE0_SUPPORT_10GBIT_SPEED(0x0004) /* 
(SNIA)HBA_PORTSPEED_10GBIT  4  10 GBit/sec */
 #define MPI_FCPORTPAGE0_SUPPORT_4GBIT_SPEED (0x0008) /* 
(SNIA)HBA_PORTSPEED_4GBIT   8   4 GBit/sec */
 
-#define MPI_FCPORTPAGE0_CURRENT_SPEED_UKNOWN
MPI_FCPORTPAGE0_SUPPORT_SPEED_UKNOWN
+#define MPI_FCPORTPAGE0_CURRENT_SPEED_UNKNOWN   
MPI_FCPORTPAGE0_SUPPORT_SPEED_UNKNOWN
 #define MPI_FCPORTPAGE0_CURRENT_SPEED_1GBIT 
MPI_FCPORTPAGE0_SUPPORT_1GBIT_SPEED
 #define MPI_FCPORTPAGE0_CURRENT_SPEED_2GBIT 
MPI_FCPORTPAGE0_SUPPORT_2GBIT_SPEED
 #define MPI_FCPORTPAGE0_CURRENT_SPEED_10GBIT
MPI_FCPORTPAGE0_SUPPORT_10GBIT_SPEED
diff --git a/drivers/message/fusion/mptfc.c b/drivers/message/fusion/mptfc.c
index 6d461ca97150..06b175420be9 100644
--- a/drivers/message/fusion/mptfc.c
+++ b/drivers/message/fusion/mptfc.c
@@ -693,7 +693,7 @@ mptfc_display_port_link_speed(MPT_ADAPTER *ioc, int 
portnum, FCPortPage0_t *pp0d
state = pp0dest->PortState;
 
if (state != MPI_FCPORTPAGE0_PORTSTATE_OFFLINE &&
-   new_speed != MPI_FCPORTPAGE0_CURRENT_SPEED_UKNOWN) {
+   new_speed != MPI_FCPORTPAGE0_CURRENT_SPEED_UNKNOWN) {
 
old = old_speed == MPI_FCPORTPAGE0_CURRENT_SPEED_1GBIT ? "1 
Gbps" :
   old_speed == MPI_FCPORTPAGE0_CURRENT_SPEED_2GBIT ? "2 
Gbps" :
-- 
2.17.0

Re: Multi-Actuator SAS HDD First Look

[Tim Walker]

On Sun, Apr 15, 2018 at 10:31 PM, Bart Van Assche
 wrote:
> On Sun, 2018-04-15 at 19:35 -0600, Tim Walker wrote:
>> I also believe the dual-actuator, or any significant HDD parallelism,
>> would map well onto an NVMe target, or NVMe-oF behind nvmet. Maybe a
>> lightweight virtual NVMe controller that would efficiently present the
>> HDD logs/mode pages/etc via the admin queue and the LUNs as fixed
>> namespaces...?
>>
>> Doug, I will flesh your three LUN idea out some more and send it up
>> the flagpole over here. Thanks for the input.
>>
>> I'd like to have a conversation at LSFMM and maybe pull together a
>> fairly well defined consensus recommendation. Is that possible? Can we
>> schedule it?
>
> Hello Tim,
>
> I think that you have to submit a request to the LSF/MM program committee
> (lsf...@lists.linux-foundation.org) to add an item to the official agenda.
> In case this topic wouldn't be added to the official agenda we can still
> discuss this topic in a meeting room at the LSF/MM location.
>
> Bart.
>
>
>

Hello Bart-

It would be good if we could set up an informal meeting time and
location at LSFMM to discuss these dual actuator topics. so far you,
Doug, Hannes, and Christoph have expressed the most interest, plus
Damien. Can we set an hour aside one afternoon?

-Tim

-- 
Tim Walker
Product Design Systems Engineering, Seagate Technology
(303) 775-3770

MegaCli fails to communicate with Raid-Controller

[volker.schwick...@godaddy.com]

Hi all,

after our latest kernel-update from 4.6. to 4.14.14 we are having trouble 
getting data out of our LSI-raid-controllers using the megacli-binary.

For every execution of the megacli-binary a line shows up in the kern.log

###
[547216.425556] megaraid_sas :03:00.0: Failed to alloc kernel SGL buffer 
for IOCTL
###

Stracing a megacli-command shows, that ENOMEM is thrown, but thats expected 
with an error message like above.
###
ioctl(3, MCE_GET_RECORD_LEN or MTRRIOC_SET_ENTRY, 
0x7c98d0) = -1 ENOMEM (Cannot allocate memory)
###

This does not happen on a freshly booted machine. After a reboot it usually 
takes roughly 2-3 days for the error to occur, but then it stays.
After the first occurrence sometimes, and very randomly a megacli-command 
works, but only once, then keeps failing again.

Current hardware is
Dell R710, MegaRAID SAS 1078, Debian Jessie, Xen 4.10, Kernel 4.14.14
- virtual disk 1
— 2x 600gb SEAGATE ST3600057SS raid-1
- virtual disk 2
— 4x 2tb SEAGATE ST32000444SS raid-10

Dell R730xd, MegaRAID SAS-3 3108, Debian Jessie, Xen 4.10, Kernel 4.14.14
- same as above

Megaraid-Driver-Version on new 4.14.14 kernel
###
filename:   
/lib/modules/4.14.14-2-xen0-he+/kernel/drivers/scsi/megaraid/megaraid_sas.ko
description:Avago MegaRAID SAS Driver
author: megaraidlinux@avagotech.com
version:07.702.06.00-rc1
license:GPL
srcversion: 15F82F234414CB9CE82AE3D
###

Megaraid-Driver-Version on current 4.6. kernel
###
filename:   
/lib/modules/4.4.74-1-xen0-he+/kernel/drivers/scsi/megaraid/megaraid_sas.ko
description:Avago MegaRAID SAS Driver
author: megaraidlinux@avagotech.com
version:06.808.16.00-rc1
license:GPL
srcversion: AAF4E2A6BAB0B1254F758CA
###

MegaCli Version
###
$ megacli-perc5 -v
  MegaCLI SAS RAID Management Tool  Ver 8.07.14 Dec 16, 2013
###

It may also be interesting that trying to query all controllers with “-aall” 
does not seem to find any controller while querying a specific controller exits 
with an error, even though its definitely there
###
$ megacli-perc5 - -ldpdinfo -aall
Exit Code: 0x00

$ megacli-perc5 -ldpdinfo -a0
User specified controller is not present.
Failed to get CpController object.
Exit Code: 0x01
###

Our monitoring-script runs the following command sequence every 20 minutes:
###
megacli-perc5 -LDGetNum -a0 -NoLog
megacli-perc5 -adpallinfo -a0 -nolog
megacli-perc5 -adpgettime -a0 -nolog
megacli-perc5 -fwtermlog -bbuon -a0 -silent -nolog
megacli-perc5 -adpbbucmd -getbbucapacityinfo -a0 -nolog
megacli-perc5 -ldpdinfo -a0 -nolog
megacli-perc5 -ldinfo -l0 -a0 -nolog
megacli-perc5 -ldinfo -l1 -a0 -nolog
###

I failed to reproduce this on a secondary machine so im looking for clues on 
how to debug this further. 
I have looked at the kernels git-log, but couldn’t match any change to my 
problem.
I have looked at the fwtermlog of the controller but theres nothing of interest 
in there.

Any ideas?

best regards
volker

Re: usercopy whitelist woe in scsi_sense_cache

[Paolo Valente]

> Il giorno 18 apr 2018, alle ore 00:57, Jens Axboe  ha 
> scritto:
> 
> On 4/17/18 3:48 PM, Jens Axboe wrote:
>> On 4/17/18 3:47 PM, Kees Cook wrote:
>>> On Tue, Apr 17, 2018 at 2:39 PM, Jens Axboe  wrote:
 On 4/17/18 3:25 PM, Kees Cook wrote:
> On Tue, Apr 17, 2018 at 1:46 PM, Kees Cook  wrote:
>> I see elv.priv[1] assignments made in a few places -- is it possible
>> there is some kind of uninitialized-but-not-NULL state that can leak
>> in there?
> 
> Got it. This fixes it for me:
> 
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index 0dc9e341c2a7..859df3160303 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -363,7 +363,7 @@ static struct request *blk_mq_get_request(struct
> request_queue *q,
> 
>rq = blk_mq_rq_ctx_init(data, tag, op);
>if (!op_is_flush(op)) {
> -   rq->elv.icq = NULL;
> +   memset(>elv, 0, sizeof(rq->elv));
>if (e && e->type->ops.mq.prepare_request) {
>if (e->type->icq_cache && rq_ioc(bio))
>blk_mq_sched_assign_ioc(rq, bio);
> @@ -461,7 +461,7 @@ void blk_mq_free_request(struct request *rq)
>e->type->ops.mq.finish_request(rq);
>if (rq->elv.icq) {
>put_io_context(rq->elv.icq->ioc);
> -   rq->elv.icq = NULL;
> +   memset(>elv, 0, sizeof(rq->elv));
>}
>}
 
 This looks like a BFQ problem, this should not be necessary. Paolo,
 you're calling your own prepare request handler from the insert
 as well, and your prepare request does nothing if rq->elv.icq == NULL.
>>> 
>>> I sent the patch anyway, since it's kind of a robustness improvement,
>>> I'd hope. If you fix BFQ also, please add:
>> 
>> It's also a memset() in the hot path, would prefer to avoid that...
>> The issue here is really the convoluted bfq usage of insert/prepare,
>> I'm sure Paolo can take it from here.
> 

Hi,
I'm very sorry for tuning in very late, but, at the same time, very
glad to find the problem probably already solved ;) (in this respect, I swear,
my delay was not intentional)

> Does this fix it?
> 
> diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
> index f0ecd98509d8..d883469a1582 100644
> --- a/block/bfq-iosched.c
> +++ b/block/bfq-iosched.c
> @@ -4934,8 +4934,11 @@ static void bfq_prepare_request(struct request *rq, 
> struct bio *bio)
>   bool new_queue = false;
>   bool bfqq_already_existing = false, split = false;
> 
> - if (!rq->elv.icq)
> + if (!rq->elv.icq) {
> + rq->elv.priv[0] = rq->elv.priv[1] = NULL;
>   return;
> + }
> +

This does solve the problem at hand.  But it also arouses a question,
related to a possible subtle bug.

For BFQ, !rq->elv.icq basically means "this request is not for me, as
I am an icq-based scheduler".  But, IIUC the main points in this
thread, then this assumption is false.  If it is actually false, then
I hope that all requests with !rq->elv.icq that are sent to BFQ do
verify the condition (at_head || blk_rq_is_passthrough(rq)).  In fact,
requests that do not verify that condition are those that BFQ must put
in a bfq_queue.  So, even if this patch makes the crash disappear, we
drive BFQ completely crazy (and we may expect other strange failures)
if we send BFQ a request with !((at_head || blk_rq_is_passthrough(rq))
and !rq->elv.icq.  BFQ has to put that rq into a bfq_queue, but simply
cannot.

Jens, or any other, could you please shed a light on this, and explain
how things are exactly?

Thanks,
Paolo

>   bic = icq_to_bic(rq->elv.icq);
> 
>   spin_lock_irq(>lock);
> 
> -- 
> Jens Axboe

Re: [PATCH] blk-mq: Clear out elevator private data

[Paolo Valente]

> Il giorno 17 apr 2018, alle ore 23:42, Kees Cook  ha 
> scritto:
> 
> Some elevators may not correctly check rq->rq_flags & RQF_ELVPRIV, and
> may attempt to read rq->elv fields. When requests got reused, this
> caused BFQ to think it already had a bfqq (rq->elv.priv[1]) allocated.

Hi Kees,
where does BFQ gets confused and operates on a request not destined to
it?  I'm asking because I paid attention to always avoid such a
mistake.

Thanks,
Paolo

> This could lead to odd behaviors like having the sense buffer address
> slowly start incrementing. This eventually tripped HARDENED_USERCOPY
> and KASAN.
> 
> This patch wipes all of rq->elv instead of just rq->elv.icq. While
> it shouldn't technically be needed, this ends up being a robustness
> improvement that should lead to at least finding bugs in elevators faster.
> 
> Reported-by: Oleksandr Natalenko 
> Fixes: bd166ef183c26 ("blk-mq-sched: add framework for MQ capable IO 
> schedulers")
> Cc: sta...@vger.kernel.org
> Signed-off-by: Kees Cook 
> ---
> In theory, BFQ needs to also check the RQF_ELVPRIV flag, but I'll leave that
> to Paolo to figure out. Also, my Fixes line is kind of a best-guess. This
> is where icq was originally wiped, so it seemed as good a commit as any.
> ---
> block/blk-mq.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index 0dc9e341c2a7..859df3160303 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -363,7 +363,7 @@ static struct request *blk_mq_get_request(struct 
> request_queue *q,
> 
>   rq = blk_mq_rq_ctx_init(data, tag, op);
>   if (!op_is_flush(op)) {
> - rq->elv.icq = NULL;
> + memset(>elv, 0, sizeof(rq->elv));
>   if (e && e->type->ops.mq.prepare_request) {
>   if (e->type->icq_cache && rq_ioc(bio))
>   blk_mq_sched_assign_ioc(rq, bio);
> @@ -461,7 +461,7 @@ void blk_mq_free_request(struct request *rq)
>   e->type->ops.mq.finish_request(rq);
>   if (rq->elv.icq) {
>   put_io_context(rq->elv.icq->ioc);
> - rq->elv.icq = NULL;
> + memset(>elv, 0, sizeof(rq->elv));
>   }
>   }
> 
> -- 
> 2.7.4
> 
> 
> -- 
> Kees Cook
> Pixel Security

Re: [PATCH v3 1/6] ilog2: create truly constant version for sparse

[Luc Van Oostenryck]

On Wed, Apr 18, 2018 at 10:12:54AM +0200, Martin Wilck wrote:
> On Tue, 2018-04-17 at 17:07 -0700, Linus Torvalds wrote:
> > On Tue, Apr 17, 2018 at 4:35 PM, Martin Wilck 
> > wrote:
> > > Sparse emits errors about ilog2() in array indices because of the
> > > use of
> > > __ilog2_32() and __ilog2_64(),
> > 
> > If sparse warns about it, then presumably gcc with -Wvla warns about
> > it too?
> 
> No, it doesn't (gcc 7.3.0). -> https://paste.opensuse.org/27471594
> It doesn't even warn on an expression like this:
> 
>   #define SIZE (1<<10)
>   static int foo[ilog2(SIZE)];
> 
> sparse 0.5.2 doesn't warn about that either. It emits "error: bad
> integer constant expression" only if ilog2 is used in an array

sparse supports VLAs at syntaxic level but not much more. Anything
needing directly or indirectly the array size will give this error.

-- Luc Van Oostenryck

[Bug 199435] New: HPSA + P420i resetting logical Direct-Access never complete

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=199435

Bug ID: 199435
   Summary: HPSA + P420i resetting logical Direct-Access never
complete
   Product: IO/Storage
   Version: 2.5
Kernel Version: 4.11.0-14-generic
  Hardware: All
OS: Linux
  Tree: Mainline
Status: NEW
  Severity: normal
  Priority: P1
 Component: SCSI
  Assignee: linux-scsi@vger.kernel.org
  Reporter: anthonyhaussm...@gmail.com
Regression: No

I'm using the kernel 4.11.0-14-generic with the last hpsa driver compile from
the last commit of torvalds github :

https://github.com/torvalds/linux/commit/8b834bff1b73dce46f4e9f5e84af6f73fed8b0ef#diff-7a84fb366ebc08b575a832f0aeee3434

I'm using a Smart Array P420i, Firmware Version 8.32.
When a resetting logical is triggered, this one never complete and the server
start to have a heavy load (can rise to 3000).
After the reset, some task begin to timout but I think that is just the effect
of the resetting (cmaeventd is the process checking for controller status):

Apr 18 01:28:53 kernel: hpsa :08:00.0: scsi 0:1:0:0: resetting logical 
Direct-Access HP   LOGICAL VOLUME   RAID-0 SSDSmartPathCap- En- Exp=1
Apr 18 01:29:16 kernel: INFO: task cmaeventd:3397 blocked for more than 120
seconds.
Apr 18 01:29:16 kernel:   Tainted: G   OE   4.11.0-14-generic
#20~16.04.1-Ubuntu
Apr 18 01:29:16 kernel: "echo 0 > /proc/sys/kernel/hung_task_timeout_secs"
disables this message.
Apr 18 01:29:16 kernel: cmaeventd   D0  3397  1 0x
Apr 18 01:29:16 kernel: Call Trace:
Apr 18 01:29:16 kernel:  __schedule+0x3b9/0x8f0
Apr 18 01:29:16 kernel:  schedule+0x36/0x80
Apr 18 01:29:16 kernel:  scsi_block_when_processing_errors+0xd5/0x110
Apr 18 01:29:16 kernel:  ? wake_atomic_t_function+0x60/0x60
Apr 18 01:29:16 kernel:  sg_open+0x14a/0x5c0
Apr 18 01:29:16 kernel:  ? lookup_fast+0xd8/0x3b0
Apr 18 01:29:16 kernel:  ? refcount_inc+0x9/0x40
Apr 18 01:29:16 kernel:  chrdev_open+0xbf/0x1b0
Apr 18 01:29:16 kernel:  do_dentry_open+0x208/0x310
Apr 18 01:29:16 kernel:  ? cdev_put+0x30/0x30
Apr 18 01:29:16 kernel:  vfs_open+0x4e/0x80
Apr 18 01:29:16 kernel:  path_openat+0x2ac/0x1450
Apr 18 01:29:16 kernel:  do_filp_open+0x99/0x110
Apr 18 01:29:16 kernel:  ? __check_object_size+0x108/0x19e
Apr 18 01:29:16 kernel:  ? __alloc_fd+0x46/0x170
Apr 18 01:29:16 kernel:  do_sys_open+0x12d/0x280
Apr 18 01:29:16 kernel:  ? do_sys_open+0x12d/0x280
Apr 18 01:29:16 kernel:  ? __put_cred+0x3d/0x50
Apr 18 01:29:16 kernel:  ? SyS_access+0x1e8/0x230
Apr 18 01:29:16 kernel:  SyS_open+0x1e/0x20
Apr 18 01:29:16 kernel:  entry_SYSCALL_64_fastpath+0x1e/0xad
Apr 18 01:29:16 kernel: RIP: 0033:0x7f413c901be0
Apr 18 01:29:16 kernel: RSP: 002b:7ffc0c1cd5b8 EFLAGS: 0246 ORIG_RAX:
0002
Apr 18 01:29:16 kernel: RAX: ffda RBX: 025f7a40 RCX:
7f413c901be0
Apr 18 01:29:16 kernel: RDX: 0008 RSI: 0002 RDI:
7ffc0c1cd5f0
Apr 18 01:29:16 kernel: RBP: 02563b40 R08: 0001 R09:

Apr 18 01:29:16 kernel: R10: 7f413c8ea760 R11: 0246 R12:
7ffc0c1cd7b0
Apr 18 01:29:16 kernel: R13: 0001 R14: 7ffc0c1cd700 R15:
7ffc0c1cd830
Apr 18 01:29:16 kernel: INFO: task cmaidad:3442 blocked for more than 120
seconds.
Apr 18 01:29:16 kernel:   Tainted: G   OE   4.11.0-14-generic
#20~16.04.1-Ubuntu
Apr 18 01:29:16 kernel: "echo 0 > /proc/sys/kernel/hung_task_timeout_secs"
disables this message.
Apr 18 01:29:16 kernel: cmaidad D0  3442  1 0x
Apr 18 01:29:16 kernel: Call Trace:
Apr 18 01:29:16 kernel:  __schedule+0x3b9/0x8f0
Apr 18 01:29:16 kernel:  schedule+0x36/0x80
Apr 18 01:29:16 kernel:  scsi_block_when_processing_errors+0xd5/0x110
Apr 18 01:29:16 kernel:  ? wake_atomic_t_function+0x60/0x60
Apr 18 01:29:16 kernel:  sg_open+0x14a/0x5c0
Apr 18 01:29:16 kernel:  ? lookup_fast+0xd8/0x3b0
Apr 18 01:29:16 kernel:  ? refcount_inc+0x9/0x40
Apr 18 01:29:16 kernel:  chrdev_open+0xbf/0x1b0
Apr 18 01:29:16 kernel:  do_dentry_open+0x208/0x310
Apr 18 01:29:16 kernel:  ? cdev_put+0x30/0x30
Apr 18 01:29:16 kernel:  vfs_open+0x4e/0x80
Apr 18 01:29:16 kernel:  path_openat+0x2ac/0x1450
Apr 18 01:29:16 kernel:  do_filp_open+0x99/0x110
Apr 18 01:29:16 kernel:  ? ipcperms+0x94/0x100
Apr 18 01:29:16 kernel:  ? __check_object_size+0x108/0x19e
Apr 18 01:29:16 kernel:  ? __alloc_fd+0x46/0x170
Apr 18 01:29:16 kernel:  do_sys_open+0x12d/0x280
Apr 18 01:29:16 kernel:  ? do_sys_open+0x12d/0x280
Apr 18 01:29:16 kernel:  ? __put_cred+0x3d/0x50
Apr 18 01:29:16 kernel:  ? SyS_access+0x1e8/0x230
Apr 18 01:29:16 kernel:  SyS_open+0x1e/0x20
Apr 18 01:29:16 kernel:  entry_SYSCALL_64_fastpath+0x1e/0xad
Apr 18 01:29:16 kernel: RIP: 0033:0x7ff5af4cdbe0
Apr 18 01:29:16 kernel: RSP: 002b:7fff8eac8818 EFLAGS: 0246 ORIG_RAX:
0002
Apr 18 01:29:16 kernel: 

Re: [PATCH v3 1/6] ilog2: create truly constant version for sparse

[Martin Wilck]

On Tue, 2018-04-17 at 17:07 -0700, Linus Torvalds wrote:
> On Tue, Apr 17, 2018 at 4:35 PM, Martin Wilck 
> wrote:
> > Sparse emits errors about ilog2() in array indices because of the
> > use of
> > __ilog2_32() and __ilog2_64(),
> 
> If sparse warns about it, then presumably gcc with -Wvla warns about
> it too?

No, it doesn't (gcc 7.3.0). -> https://paste.opensuse.org/27471594
It doesn't even warn on an expression like this:

  #define SIZE (1<<10)
  static int foo[ilog2(SIZE)];

sparse 0.5.2 doesn't warn about that either. It emits "error: bad
integer constant expression" only if ilog2 is used in an array
initializer, like this:

  #define SIZE (1<<10)
  #define SUBS (1<<5)
  static int foo [ilog2(SIZE)] = {
  [ilog2(SUBS)] = 0,
  };

So maybe I was wrong, and this is actually a false positive in sparse.

> So I suspect that what you'd want is
> 
>   #define ilog2(n) \
> __builtin_choose_expr(__is_constexpr(n), \
> const_ilog2(n), \
> __builtin_choose_expr(sizeof(n) <= 4, \
> __ilog2_u32(n), \
> __ilog2_u64(n)))
> 
> or something. Hmm?

Do you want me to convert the patch to your approach anyway?
Or should I throw this away and report to sparse?

Regards and thanks,
Martin

PS: apologies to all recipients for the broken cc list in my post.
-- 
Dr. Martin Wilck , Tel. +49 (0)911 74053 2107
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)