Re: [PATCH v3 1/6] ilog2: create truly constant version for sparse
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)
[Bug 199435] New: HPSA + P420i resetting logical Direct-Access never complete
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
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
Re: [PATCH] blk-mq: Clear out elevator private data
> 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(&rq->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(&rq->elv, 0, sizeof(rq->elv)); > } > } > > -- > 2.7.4 > > > -- > Kees Cook > Pixel Security
Re: usercopy whitelist woe in scsi_sense_cache
> 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(&rq->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(&rq->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(&bfqd->lock); > > -- > Jens Axboe
MegaCli fails to communicate with Raid-Controller
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: Multi-Actuator SAS HDD First Look
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
[PATCH] scsi: mptfc: fix spelling mistake in macro names
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
sr 6:0:0:0: [sr0] tag#0 FAILED Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE
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)
[Bug 199435] HPSA + P420i resetting logical Direct-Access never complete
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.
Re: usercopy whitelist woe in scsi_sense_cache
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(&rq->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(&rq->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
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.
[VERY EARLY RFC 01/13] scsi: use host_byte() accessor
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
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
[VERY EARLY RFC 03/13] scsi: add enum for host byte codes
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), &adapter->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), &adapter->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, /* Parity error
[VERY EARLY RFC 02/13] scsi: remove Scsi_Cmnd typedef
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, &tw_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) +static inline str
[VERY EARLY RFC 07/13] scsi: use set_driver_byte
@@ 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(&cmd->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 *fusion,
[VERY EARLY RFC 00/13] Rework SCSI results handling
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
@@ 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( &c2->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 05/13] scsi: add enum for message byte codes
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
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)) { switch
[VERY EARLY RFC 13/13] scsi: introduce struct scsi_result
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); - hdr->driver_status = driver_byte(job->result); +
[VERY EARLY RFC 06/13] scsi: introduce set_scsi_result
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 = scsi_sglist(cmd);
[VERY EARLY RFC 10/13] scsi: introduce set_status_byte and convert LLDDs to use it
@@ 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, &qc->result_tf); @@ -2131,7 +2131,7 @@ static void ata_scsi_rbuf_fill(struct ata_scsi_args *args, ata_scsi_rbuf_put(cmd, rc == 0, &flags); 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 failure path */
Re: [VERY EARLY RFC 00/13] Rework SCSI results handling
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.
Re: Multi-Actuator SAS HDD First Look
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: Multi-Actuator SAS HDD First Look
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: MegaCli fails to communicate with Raid-Controller
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
[Bug 199435] HPSA + P420i resetting logical Direct-Access never complete
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&action=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.
[Bug 199435] HPSA + P420i resetting logical Direct-Access never complete
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.
Re: [VERY EARLY RFC 00/13] Rework SCSI results handling
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
Re: [PATCH] Waiting for scsi_host_template release
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 >>
scsi sd driver is not emitting a CHANGE uevent for online disk resizes
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 v3 1/6] ilog2: create truly constant version for sparse
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
RE: [PATCH v2] storvsc: Set up correct queue depth values for IDE devices
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: [VERY EARLY RFC 01/13] scsi: use host_byte() accessor
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: [VERY EARLY RFC 08/13] treewide: use set_host_byte
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, &phase, &len, &data); > 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, &phase, &len, > &data); > 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(&hostdata->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(&hostdata->unissued, cmd
Re: [PATCH 1/1] scsi: sd: improved drive sanitize error handling
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: [PATCH] target: fix crash with iscsi target and dvd
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 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? -- Martin K. Petersen Oracle Linux Engineering
Re: [PATCH] target: fix crash with iscsi target and dvd
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 v9] scsi: new zorro_esp.c for Amiga Zorro NCR53C9x boards
Michael, > Anything else needed for linux-scsi? 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
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] scsi: mptfc: fix spelling mistake in macro names
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: scsi sd driver is not emitting a CHANGE uevent for online disk resizes
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 v2 00/14] mpt3sas: Enhancements and Defect fixes.
> 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: [PATCH] scsi: Change return type to vm_fault_t
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 02/12] storsvc: don't set a bounce limit
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 01/12] iscsi_tcp: don't set a bounce limit
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
[PATCH] scsi_debug: replace SDEG_RES_IMMED_MASK with cmnd flag
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(&devip->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] target: fix crash with iscsi target and dvd
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
Re: [PATCHv2] target: prefer dbroot of /etc/target over /var/target
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: Change return type to vm_fault_t
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: [PATCH v9] scsi: new zorro_esp.c for Amiga Zorro NCR53C9x boards
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 v2] storvsc: Set up correct queue depth values for IDE devices
> 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: [VERY EARLY RFC 01/13] scsi: use host_byte() accessor
On Thu, 19 Apr 2018 09:46:30 +1000 (AEST) Finn Thain wrote: > 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? > Indeed, as it's actually wrong. The status byte is not a bitfield, but rather a value, and as such the '&' check is wrong; it should be '=='. Cheers, Hannes
Re: [VERY EARLY RFC 12/13] reewide: introduce clear_scsi_result() and convert drivers
On Wed, 18 Apr 2018 17:01:15 +0200 Johannes Thumshirn wrote: > Introduce clear_scsi_result() and convert drivers over to use the new > function. > [ .. ] Why not use 'set_scsi_result' here? Basically everywhere the usage pattern is clear_scsi_result() set_host_byte() so they can be collapsed into set_scsi_result(cmd, 0, host_byte, 0, 0) and this helper can be dropped. Cheers, Hannes
RE: MegaCli fails to communicate with Raid-Controller
> -Original Message- > From: Martin K. Petersen [mailto:martin.peter...@oracle.com] > Sent: Wednesday, April 18, 2018 10:43 PM > To: volker.schwick...@godaddy.com > Cc: linux-scsi@vger.kernel.org; Kashyap Desai; Sumit Saxena > Subject: Re: MegaCli fails to communicate with Raid-Controller > > > 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. I think you may see issue with 4.6 kernel as well. This is run time memory allocation failure. Older controller used 32 bit consistence DMA mask, so possibility of memory allocation failure is high compare to 64 bit consistence DMA mask. Newer controller has fix in this area, but you are using gen-1 controller. ("Dell R710, MegaRAID SAS 1078"). There can be a two possibilities. 1. This is actual memory allocation failure due to system resource issue. 2. IOCLT provided large memory length in iov and dma buffer allocation from below API failed due to large memory chunk requested. kbuff_arr[i] = dma_alloc_coherent(&instance->pdev->dev, ioc->sgl[i].iov_len, &buf_handle, GFP_KERNEL); Can you change driver code *printk* to dump iov_len ? Just to confirm. One wild guess - You are using Xen flavor, which will reserve less memory for Dom0 and there may be some way to increase dom0 memory. Can you tune that as well and see ? I am not sure how to do that in your case, but in Citrix we used to see such issue frequently compare to *default* Linux. Providing some tuning in grub increase the dom0 memory and that make things better compare to default settings. > > Kashyap? Sumit? > > -- > Martin K. PetersenOracle Linux Engineering