Re: Multi-Actuator SAS HDD First Look
On Fri, Apr 06, 2018 at 08:24:18AM +0200, Hannes Reinecke wrote: > Ah. Far better. > What about delegating FORMAT UNIT to the control LUN, and not > implementing it for the individual disk LUNs? > That would make an even stronger case for having a control LUN; > with that there wouldn't be any problem with having to synchronize > across LUNs etc. It sounds to me like NVMe might be a much better model for this drive than SCSI, btw :)
Re: [PATCH v3 2/3] Rename __scsi_error_from_host_byte() into scsi_result_to_blk_status()
On Thu, 5 Apr 2018 10:33:00 -0700
Bart Van Assche wrote:
> Since the next patch will modify this function such that it
> checks more than just the host byte of the SCSI result, rename
> __scsi_error_from_host_byte() into scsi_result_to_blk_status().
> This patch does not change any functionality.
>
> Signed-off-by: Bart Van Assche
> Cc: Hannes Reinecke
> Cc: Douglas Gilbert
> Cc: Damien Le Moal
> Cc: Christoph Hellwig
> Cc: Lee Duncan
> ---
> drivers/scsi/scsi_lib.c | 18 +-
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index c0e4ae733cce..26d82f323b31 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -723,14 +723,14 @@ static bool scsi_end_request(struct request
> *req, blk_status_t error, }
>
> /**
> - * __scsi_error_from_host_byte - translate SCSI error code into errno
> - * @cmd: SCSI command (unused)
> + * scsi_result_to_blk_status - translate a SCSI result code into
> blk_status_t
> + * @cmd: SCSI command
> * @result: scsi error code
> *
> - * Translate SCSI error code into block errors.
> + * Translate a SCSI result code into a blk_status_t value. May reset
> the host
> + * byte of @cmd->result.
> */
> -static blk_status_t __scsi_error_from_host_byte(struct scsi_cmnd
> *cmd,
> - int result)
> +static blk_status_t scsi_result_to_blk_status(struct scsi_cmnd *cmd,
> int result) {
> switch (host_byte(result)) {
> case DID_TRANSPORT_FAILFAST:
> @@ -810,10 +810,10 @@ void scsi_io_completion(struct scsi_cmnd *cmd,
> unsigned int good_bytes) SCSI_SENSE_BUFFERSIZE);
> }
> if (!sense_deferred)
> - error =
> __scsi_error_from_host_byte(cmd, result);
> + error =
> scsi_result_to_blk_status(cmd, result); }
> /*
> - * __scsi_error_from_host_byte may have reset the
> host_byte
> + * scsi_result_to_blk_status may have reset the
> host_byte */
> scsi_req(req)->result = cmd->result;
> scsi_req(req)->resid_len = scsi_get_resid(cmd);
> @@ -835,7 +835,7 @@ void scsi_io_completion(struct scsi_cmnd *cmd,
> unsigned int good_bytes)
>* good_bytes != blk_rq_bytes(req) as the signal for
> an error.
>* This sets the error explicitly for the problem
> case. */
> - error = __scsi_error_from_host_byte(cmd, result);
> + error = scsi_result_to_blk_status(cmd, result);
> }
>
> /* no bidi support for !blk_rq_is_passthrough yet */
> @@ -905,7 +905,7 @@ void scsi_io_completion(struct scsi_cmnd *cmd,
> unsigned int good_bytes) if (result == 0)
> goto requeue;
>
> - error = __scsi_error_from_host_byte(cmd, result);
> + error = scsi_result_to_blk_status(cmd, result);
>
> if (host_byte(result) == DID_RESET) {
> /* Third party bus reset or reset for error recovery
Reviewed-by: Hannes Reinecke
Cheers,
Hannes
Re: [PATCH v3 3/3] Make scsi_result_to_blk_status() recognize CONDITION MET
On Thu, 5 Apr 2018 10:33:01 -0700
Bart Van Assche wrote:
> Ensure that CONDITION MET and other non-zero status values that
> indicate success are translated into BLK_STS_OK.
>
> Signed-off-by: Bart Van Assche
> Cc: Hannes Reinecke
> Cc: Douglas Gilbert
> Cc: Damien Le Moal
> Cc: Christoph Hellwig
> Cc: Lee Duncan
> ---
> drivers/scsi/scsi_lib.c | 9 +
> 1 file changed, 9 insertions(+)
>
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index 26d82f323b31..3e494a476c50 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -733,6 +733,15 @@ static bool scsi_end_request(struct request
> *req, blk_status_t error, static blk_status_t
> scsi_result_to_blk_status(struct scsi_cmnd *cmd, int result) {
> switch (host_byte(result)) {
> + case DID_OK:
> + /*
> + * Also check the other bytes than the status byte
> in result
> + * to handle the case when a SCSI LLD sets result to
> + * DRIVER_SENSE << 24 without setting
> SAM_STAT_CHECK_CONDITION.
> + */
> + if (scsi_status_is_good(result) && (result & ~0xff)
> == 0)
> + return BLK_STS_OK;
> + return BLK_STS_IOERR;
> case DID_TRANSPORT_FAILFAST:
> return BLK_STS_TRANSPORT;
> case DID_TARGET_FAILURE:
Reviewed-by: Hannes Reinecke
Cheers,
Hannes
Re: [PATCH v3 1/3] Revert "scsi: core: return BLK_STS_OK for DID_OK in __scsi_error_from_host_byte()"
On Thu, 5 Apr 2018 10:32:59 -0700
Bart Van Assche wrote:
> The description of commit e39a97353e53 is wrong: it mentions that
> commit 2a842acab109 introduced a bug in __scsi_error_from_host_byte()
> although that commit did not change the behavior of that function.
> Additionally, commit e39a97353e53 introduced a bug: it causes commands
> that fail with hostbyte=DID_OK and driverbyte=DRIVER_SENSE to be
> completed with BLK_STS_OK. Hence revert that commit.
>
> Fixes: e39a97353e53 ("scsi: core: return BLK_STS_OK for DID_OK in
> __scsi_error_from_host_byte()") Reported-by: Damien Le Moal
> Signed-off-by: Bart Van Assche
> Cc: Hannes Reinecke
> Cc: Douglas Gilbert
> Cc: Damien Le Moal
> Cc: Christoph Hellwig
> Cc: Lee Duncan
> Cc: sta...@vger.kernel.org
> ---
> drivers/scsi/scsi_lib.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index 1d83f29aee74..c0e4ae733cce 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -733,8 +733,6 @@ static blk_status_t
> __scsi_error_from_host_byte(struct scsi_cmnd *cmd, int result)
> {
> switch (host_byte(result)) {
> - case DID_OK:
> - return BLK_STS_OK;
> case DID_TRANSPORT_FAILFAST:
> return BLK_STS_TRANSPORT;
> case DID_TARGET_FAILURE:
Reviewed-by: Hannes Reinecke
Cheers,
Hannes
Re: Multi-Actuator SAS HDD First Look
On Thu, 5 Apr 2018 17:43:46 -0600 Tim Walker wrote: > On Tue, Apr 3, 2018 at 1:46 AM, Christoph Hellwig > wrote: > > On Sat, Mar 31, 2018 at 01:03:46PM +0200, Hannes Reinecke wrote: > >> Actually I would propose to have a 'management' LUN at LUN0, who > >> could handle all the device-wide commands (eg things like START > >> STOP UNIT, firmware update, or even SMART commands), and ignoring > >> them for the remaining LUNs. > > > > That is in fact the only workable option at all. Everything else > > completly breaks the scsi architecture. > > Here's an update: Seagate will eliminate the inter-LU actions from > FORMAT UNIT and SANITIZE. Probably SANITIZE will be per-LUN, but > FORMAT UNIT is trickier due to internal drive architecture, and how > FORMAT UNIT initializes on-disk metadata. Likely it will require some > sort of synchronization across LUNs, such as the command being sent to > both LUNs sequentially or something similar. We are also considering > not supporting FORMAT UNIT at all - would anybody object? Any other > suggestions? > Ah. Far better. What about delegating FORMAT UNIT to the control LUN, and not implementing it for the individual disk LUNs? That would make an even stronger case for having a control LUN; with that there wouldn't be any problem with having to synchronize across LUNs etc. Cheers, Hannes
Re: usercopy whitelist woe in scsi_sense_cache
Hi. 05.04.2018 20:52, Kees Cook wrote: Okay. My qemu gets mad about that and wants the format=raw argument, so I'm using: -drive file=sda.img,format=raw \ -drive file=sdb.img,format=raw \ How are you running your smartctl? I'm doing this now: [1] Running while :; do ( smartctl -a /dev/sda; smartctl -a /dev/sdb ) > /dev/null; done & Yes, so do I. I assume I'm missing something from your .config, but since I don't boot with an initramfs, I had to tweak it a bit. I'll try again... Let me, maybe, describe, what both the VM and the server have in common: 1. have 4 CPUs 2. are EFI-based 3. use blk-mq with BFQ scheduler (it is set up via udev rule during boot) 4. have zswap enabled 5. have 2 SATA disks with RAID10 on top of it (layout f2) 6. have LUKS on top of the RAID, and LVM on top of the LUKS VM has machine type "q35", BTW. Do you think something of what's mentioned above is relevant for the code path in question? Thanks. Regards, Oleksandr
Re: [PATCH 02/10] staging: fnic2 add resource allocation
On Thu, Apr 05, 2018 at 02:17:52PM -0700, Oliver Smith-Denny wrote: > --- /dev/null > +++ b/drivers/staging/fnic2/src/fnic2_isr.c > @@ -0,0 +1,324 @@ > +/* > + * SPDX-License-Identifier: GPL-2.0 > + * Copyright 2018 Cisco Systems, Inc. All rights reserved. > + * > + * This program is free software; you may redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; version 2 of the License. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, > + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF > + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND > + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS > + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN > + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN > + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE > + * SOFTWARE. > + */ No need for the messy boilerplate text if you have a SPDX line, please remove both of those paragraphs. thanks, greg k-h
Re: [PATCH 03/10] staging: fnic2 add fip handling
On Thu, Apr 05, 2018 at 02:18:37PM -0700, Oliver Smith-Denny wrote: > --- /dev/null > +++ b/drivers/staging/fnic2/src/fip.c > @@ -0,0 +1,804 @@ > +/* > + * SPDX-License-Identifier: GPL-2.0 > + * Copyright 2018 Cisco Systems, Inc. All rights reserved. > + * > + * This program is free software; you may redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; version 2 of the License. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, > + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF > + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND > + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS > + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN > + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN > + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE > + * SOFTWARE. > + */ > + > +/*! \file */ What is that line for?
Re: [PATCH 01/10] staging: fnic2 add initialization
On Thu, Apr 05, 2018 at 02:16:45PM -0700, Oliver Smith-Denny wrote: > These files contain module load and unload, global driver context, > PCI registration, PCI probe and remove, and definitions of > the fnic2 global context. > > Signed-off-by: Oliver Smith-Denny > Signed-off-by: Sesidhar Baddela > Signed-off-by: Anil Chintalapati > Signed-off-by: Arulprabhu Ponnusamy > Signed-off-by: Gian Carlo Boffa > Co-Developed-by: Arulprabhu Ponnusamy > Co-Developed-by: Gian Carlo Boffa > Co-Developed-by: Oliver Smith-Denny > --- > drivers/staging/fnic2/src/fnic2.h | 256 > drivers/staging/fnic2/src/fnic2_main.c | 711 > + > 2 files changed, 967 insertions(+) > create mode 100644 drivers/staging/fnic2/src/fnic2.h > create mode 100644 drivers/staging/fnic2/src/fnic2_main.c Why is this a drivers/staging/ driver at all? What is keeping you from getting this merged into the "proper" place in the kernel? If you have a staging driver, you have to have a TODO file in the directory listing what is keeping this in the staging section. Also, one tiny thing to fix up: > --- /dev/null > +++ b/drivers/staging/fnic2/src/fnic2.h > @@ -0,0 +1,256 @@ > +/* > + * SPDX-License-Identifier: GPL-2.0 Please read the documentation on how to properly use SPDX tags on kernel files. This needs to be the first line of the file. thanks, greg k-h
Re: 4.15.14 crash with iscsi target and dvd
On Thu, 2018-04-05 at 22:06 -0400, Wakko Warner wrote: > I know now why scsi_print_command isn't doing anything. cmd->cmnd is null. > I added a dev_printk in scsi_print_command where the 2 if statements return. > Logs: > [ 29.866415] sr 3:0:0:0: cmd->cmnd is NULL That's something that should never happen. As one can see in scsi_setup_scsi_cmnd() and scsi_setup_fs_cmnd() both functions initialize that pointer. Since I have not yet been able to reproduce myself what you reported, would it be possible for you to bisect this issue? You will need to follow something like the following procedure (see also https://git-scm.com/docs/git-bisect): git clone git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git git bisect start git bisect bad v4.10 git bisect good v4.9 and then build the kernel, install it, boot the kernel and test it. Depending on the result, run either git bisect bad or git bisect good and keep going until git bisect comes to a conclusion. This can take an hour or more. Bart.
Re: 4.15.14 crash with iscsi target and dvd
Wakko Warner wrote:
> Bart Van Assche wrote:
> > On Sun, 2018-04-01 at 14:27 -0400, Wakko Warner wrote:
> > > Wakko Warner wrote:
> > > > Wakko Warner wrote:
> > > > > I tested 4.14.32 last night with the same oops. 4.9.91 works fine.
> > > > > From the initiator, if I do cat /dev/sr1 > /dev/null it works. If I
> > > > > mount
> > > > > /dev/sr1 and then do find -type f | xargs cat > /dev/null the target
> > > > > crashes. I'm using the builtin iscsi target with pscsi. I can burn
> > > > > from
> > > > > the initiator with out problems. I'll test other kernels between 4.9
> > > > > and
> > > > > 4.14.
> > > >
> > > > So I've tested 4.x.y where x one of 10 11 12 14 15 and y is the latest
> > > > patch
> > > > (except for 4.15 which was 1 behind)
> > > > Each of these kernels crash within seconds or immediate of doing find
> > > > -type
> > > > f | xargs cat > /dev/null from the initiator.
> > >
> > > I tried 4.10.0. It doesn't completely lockup the system, but the device
> > > that was used hangs. So from the initiator, it's /dev/sr1 and from the
> > > target it's /dev/sr0. Attempting to read /dev/sr0 after the oops causes
> > > the
> > > process to hang in D state.
> >
> > Hello Wakko,
> >
> > Thank you for having narrowed down this further. I think that you
> > encountered
> > a regression either in the block layer core or in the SCSI core.
> > Unfortunately
> > the number of changes between kernel versions v4.9 and v4.10 in these two
> > subsystems is huge. I see two possible ways forward:
> > - Either that you perform a bisect to identify the patch that introduced
> > this
> > regression. However, I'm not sure whether you are familiar with the bisect
> > process.
> > - Or that you identify the command that triggers this crash such that others
> > can reproduce this issue without needing access to your setup.
> >
> > How about reproducing this crash with the below patch applied on top of
> > kernel v4.15.x? The additional output sent by this patch to the system log
> > should allow us to reproduce this issue by submitting the same SCSI command
> > with sg_raw.
>
> Ok, so I tried this, but scsi_print_command doesn't print anything. I added
> a check for !rq and the same thing that blk_rq_nr_phys_segments does in an
> if statement above this thinking it might have crashed during WARN_ON_ONCE.
> It still didn't print anything. My printk shows this:
> [ 36.263193] sr 3:0:0:0: cmd->request->nr_phys_segments is 0
>
> I also had scsi_print_command in the same if block which again didn't print
> anything. Is there some debug option I need to turn on to make it print? I
> tried looking through the code for this and following some of the function
> calls but didn't see any config options.
I know now why scsi_print_command isn't doing anything. cmd->cmnd is null.
I added a dev_printk in scsi_print_command where the 2 if statements return.
Logs:
[ 29.866415] sr 3:0:0:0: cmd->cmnd is NULL
> > Subject: [PATCH] Report commands with no physical segments in the system log
> >
> > ---
> > drivers/scsi/scsi_lib.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> > index 6b6a6705f6e5..74a39db57d49 100644
> > --- a/drivers/scsi/scsi_lib.c
> > +++ b/drivers/scsi/scsi_lib.c
> > @@ -1093,8 +1093,10 @@ int scsi_init_io(struct scsi_cmnd *cmd)
> > bool is_mq = (rq->mq_ctx != NULL);
> > int error = BLKPREP_KILL;
> >
> > - if (WARN_ON_ONCE(!blk_rq_nr_phys_segments(rq)))
> > + if (WARN_ON_ONCE(!blk_rq_nr_phys_segments(rq))) {
> > + scsi_print_command(cmd);
> > goto err_exit;
> > + }
> >
> > error = scsi_init_sgtable(rq, &cmd->sdb);
> > if (error)
> --
> Microsoft has beaten Volkswagen's world record. Volkswagen only created 22
> million bugs.
--
Microsoft has beaten Volkswagen's world record. Volkswagen only created 22
million bugs.
Re: 4.15.14 crash with iscsi target and dvd
Bart Van Assche wrote:
> On Sun, 2018-04-01 at 14:27 -0400, Wakko Warner wrote:
> > Wakko Warner wrote:
> > > Wakko Warner wrote:
> > > > I tested 4.14.32 last night with the same oops. 4.9.91 works fine.
> > > > From the initiator, if I do cat /dev/sr1 > /dev/null it works. If I
> > > > mount
> > > > /dev/sr1 and then do find -type f | xargs cat > /dev/null the target
> > > > crashes. I'm using the builtin iscsi target with pscsi. I can burn
> > > > from
> > > > the initiator with out problems. I'll test other kernels between 4.9
> > > > and
> > > > 4.14.
> > >
> > > So I've tested 4.x.y where x one of 10 11 12 14 15 and y is the latest
> > > patch
> > > (except for 4.15 which was 1 behind)
> > > Each of these kernels crash within seconds or immediate of doing find
> > > -type
> > > f | xargs cat > /dev/null from the initiator.
> >
> > I tried 4.10.0. It doesn't completely lockup the system, but the device
> > that was used hangs. So from the initiator, it's /dev/sr1 and from the
> > target it's /dev/sr0. Attempting to read /dev/sr0 after the oops causes the
> > process to hang in D state.
>
> Hello Wakko,
>
> Thank you for having narrowed down this further. I think that you encountered
> a regression either in the block layer core or in the SCSI core. Unfortunately
> the number of changes between kernel versions v4.9 and v4.10 in these two
> subsystems is huge. I see two possible ways forward:
> - Either that you perform a bisect to identify the patch that introduced this
> regression. However, I'm not sure whether you are familiar with the bisect
> process.
> - Or that you identify the command that triggers this crash such that others
> can reproduce this issue without needing access to your setup.
>
> How about reproducing this crash with the below patch applied on top of
> kernel v4.15.x? The additional output sent by this patch to the system log
> should allow us to reproduce this issue by submitting the same SCSI command
> with sg_raw.
Ok, so I tried this, but scsi_print_command doesn't print anything. I added
a check for !rq and the same thing that blk_rq_nr_phys_segments does in an
if statement above this thinking it might have crashed during WARN_ON_ONCE.
It still didn't print anything. My printk shows this:
[ 36.263193] sr 3:0:0:0: cmd->request->nr_phys_segments is 0
I also had scsi_print_command in the same if block which again didn't print
anything. Is there some debug option I need to turn on to make it print? I
tried looking through the code for this and following some of the function
calls but didn't see any config options.
> Subject: [PATCH] Report commands with no physical segments in the system log
>
> ---
> drivers/scsi/scsi_lib.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index 6b6a6705f6e5..74a39db57d49 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -1093,8 +1093,10 @@ int scsi_init_io(struct scsi_cmnd *cmd)
> bool is_mq = (rq->mq_ctx != NULL);
> int error = BLKPREP_KILL;
>
> - if (WARN_ON_ONCE(!blk_rq_nr_phys_segments(rq)))
> + if (WARN_ON_ONCE(!blk_rq_nr_phys_segments(rq))) {
> + scsi_print_command(cmd);
> goto err_exit;
> + }
>
> error = scsi_init_sgtable(rq, &cmd->sdb);
> if (error)
--
Microsoft has beaten Volkswagen's world record. Volkswagen only created 22
million bugs.
Re: Multi-Actuator SAS HDD First Look
On 2018-04-05 07:43 PM, Tim Walker wrote: On Tue, Apr 3, 2018 at 1:46 AM, Christoph Hellwig wrote: On Sat, Mar 31, 2018 at 01:03:46PM +0200, Hannes Reinecke wrote: Actually I would propose to have a 'management' LUN at LUN0, who could handle all the device-wide commands (eg things like START STOP UNIT, firmware update, or even SMART commands), and ignoring them for the remaining LUNs. That is in fact the only workable option at all. Everything else completly breaks the scsi architecture. Here's an update: Seagate will eliminate the inter-LU actions from FORMAT UNIT and SANITIZE. Probably SANITIZE will be per-LUN, but FORMAT UNIT is trickier due to internal drive architecture, and how FORMAT UNIT initializes on-disk metadata. Likely it will require some sort of synchronization across LUNs, such as the command being sent to both LUNs sequentially or something similar. We are also considering not supporting FORMAT UNIT at all - would anybody object? Any other suggestions? Good, that is progress. [But you still only have one spindle.] If Protection Information (PI) or changing the logical block size between 512 and 4096 bytes per block are options, then you need FU for that. But does it need to take 900 minutes like one I got recently from S..? Couldn't the actual reformatting of a track be deferred until the first block written to that track? Doug Gilbert
Re: Multi-Actuator SAS HDD First Look
On Tue, Apr 3, 2018 at 1:46 AM, Christoph Hellwig wrote: > On Sat, Mar 31, 2018 at 01:03:46PM +0200, Hannes Reinecke wrote: >> Actually I would propose to have a 'management' LUN at LUN0, who could >> handle all the device-wide commands (eg things like START STOP UNIT, >> firmware update, or even SMART commands), and ignoring them for the >> remaining LUNs. > > That is in fact the only workable option at all. Everything else > completly breaks the scsi architecture. Here's an update: Seagate will eliminate the inter-LU actions from FORMAT UNIT and SANITIZE. Probably SANITIZE will be per-LUN, but FORMAT UNIT is trickier due to internal drive architecture, and how FORMAT UNIT initializes on-disk metadata. Likely it will require some sort of synchronization across LUNs, such as the command being sent to both LUNs sequentially or something similar. We are also considering not supporting FORMAT UNIT at all - would anybody object? Any other suggestions? -- Tim Walker Product Design Systems Engineering, Seagate Technology (303) 775-3770
Re: [PATCH] target: change default dbroot to /etc/target
On 04/05/2018 10:57 AM, Christoph Hellwig wrote: > On Thu, Apr 05, 2018 at 10:06:35AM -0700, Lee Duncan wrote: >> Also, I think it's a bit unlikely that anyone will still be using >> /var/target, since targetcli-fb has been setting the target root to >> /etc/target for a while now, and the old targetcli has been deprecated. >> (It's the only app I know that hard-codes the old location.) > > That is a a good point. How about having a search path in the kernel? > Try the configs attribute first if one is set, then /etc/target, then > /var/target? > Good suggestion. But the configfs path is only passed in optionally, and possibly much after initialization, so there's no way the code can check the configfs path at initialization time, when this decision is first made. How about if the code looks for /etc/target, then uses /var/target if /etc/target is not present? Then users can use the current sysfs logic to set the path to any other directory, if neither of these paths are to their liking? And users can always find out the current dbroot with the sysfs attribute. -- Lee Duncan SUSE Labs
RE: [PATCH] scsi: devinfo: Add Microsoft iSCSI target to 1024 sector blacklist
+Matt
> -Original Message-
> From: Ross Lagerwall
> Sent: Thursday, April 5, 2018 9:58 AM
> To: Long Li ; Martin K. Petersen
> ; KY Srinivasan
> Cc: James E.J. Bottomley ; linux-
> s...@vger.kernel.org; linux-ker...@vger.kernel.org
> Subject: Re: [PATCH] scsi: devinfo: Add Microsoft iSCSI target to 1024 sector
> blacklist
>
> On 03/28/2018 11:33 PM, Long Li wrote:
> >> Subject: Re: [PATCH] scsi: devinfo: Add Microsoft iSCSI target to 1024
> sector
> >> blacklist
> >>
> >>
> >> Long, KY: Please confirm.
> >>
> >>> The Windows Server 2016 iSCSI target doesn't work with the Linux
> >>> kernel initiator since the kernel started sending larger requests by
> >>> default, nor does it implement the block limits VPD page. Apply the
> >>> sector limit workaround for these targets.
> >>>
> >>> Signed-off-by: Ross Lagerwall
> >>> ---
> >>> drivers/scsi/scsi_devinfo.c | 2 +-
> >>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/scsi/scsi_devinfo.c b/drivers/scsi/scsi_devinfo.c
> >>> index f3b1172..5cb748a 100644
> >>> --- a/drivers/scsi/scsi_devinfo.c
> >>> +++ b/drivers/scsi/scsi_devinfo.c
> >>> @@ -213,7 +213,7 @@ static struct {
> >>> {"Medion", "Flash XL MMC/SD", "2.6D", BLIST_FORCELUN},
> >>> {"MegaRAID", "LD", NULL, BLIST_FORCELUN},
> >>> {"MICROP", "4110", NULL, BLIST_NOTQ},
> >>> - {"MSFT", "Virtual HD", NULL, BLIST_NO_RSOC},
> >>> + {"MSFT", "Virtual HD", NULL, BLIST_MAX_1024 | BLIST_NO_RSOC},
> >
> > Ross,
> >
> > What about storage_channel_properties.max_transfer_bytes returned
> from VSTOR_OPERATION_QUERY_PROPERTIES (in storvsc_channel_init())
> >
> > Does it return correctly the maximum transfer size for iSCSI?
> >
>
> I presume you're referring to the Hyper-V virtual storage driver? This
> has nothing to do with that module -- I don't even have it compiled in.
> It's just simply the Linux kernel initiator connecting over plain
> software iSCSI to a Windows Server 2016 iSCSI target.
>
> This is easy enough to reproduce. Just set up a Windows Server 2016
> target and try and use it from Linux. You get I/O errors as soon as you
> try and format the disk.
Adding Matt for visibility. Ross, we are fine with blacklisting for now.
K. Y
>
> Cheers,
> --
> Ross Lagerwall
Re: usercopy whitelist woe in scsi_sense_cache
[forcing non-HTML and resending...] On Thu, Apr 5, 2018 at 7:33 AM, Oleksandr Natalenko wrote: > > 05.04.2018 16:32, Oleksandr Natalenko wrote: >> >> "-hda sda.img -hdb sda.img" > > > "-hda sda.img -hdb sdb.img", of course, I don't pass the same disk twice Okay. My qemu gets mad about that and wants the format=raw argument, so I'm using: -drive file=sda.img,format=raw \ -drive file=sdb.img,format=raw \ How are you running your smartctl? I'm doing this now: [1] Running while :; do ( smartctl -a /dev/sda; smartctl -a /dev/sdb ) > /dev/null; done & I assume I'm missing something from your .config, but since I don't boot with an initramfs, I had to tweak it a bit. I'll try again... -Kees -- Kees Cook Pixel Security
Re: [PATCH] target: change default dbroot to /etc/target
On Thu, Apr 05, 2018 at 10:06:35AM -0700, Lee Duncan wrote: > Also, I think it's a bit unlikely that anyone will still be using > /var/target, since targetcli-fb has been setting the target root to > /etc/target for a while now, and the old targetcli has been deprecated. > (It's the only app I know that hard-codes the old location.) That is a a good point. How about having a search path in the kernel? Try the configs attribute first if one is set, then /etc/target, then /var/target?
Re: [PATCH v3 0/3] Report all request failures again to user space
Bart, > A recent change in the SCSI core caused certain request failures no > longer to be reported to user space. Damien noticed this by sending a > write request that is not aligned to the write pointer to an SMR drive > from user space. Such non-aligned write requests are failed by the > drive and such failures should be reported to user space. This patch > series makes sure that all SCSI request failures are again reported to > user space. This patch series also makes the SCSI core recognize > status codes like CONDITION MET as not being an error. Please > consider at least the first patch in this series for the rc stage of > kernel v4.17. Looks good to me. Longer term I'd really like to see the command result integer host/driver/msg/status stuff cleaned up. It's super arcane and the associated naming schemes make it a very error-prone interface. -- Martin K. Petersen Oracle Linux Engineering
Re: [PATCH v3 3/3] Make scsi_result_to_blk_status() recognize CONDITION MET
On Thu, Apr 05, 2018 at 10:33:01AM -0700, Bart Van Assche wrote: > Ensure that CONDITION MET and other non-zero status values that > indicate success are translated into BLK_STS_OK. > > Signed-off-by: Bart Van Assche Looks good, Reviewed-by: Christoph Hellwig
Re: [PATCH v3 2/3] Rename __scsi_error_from_host_byte() into scsi_result_to_blk_status()
On Thu, Apr 05, 2018 at 10:33:00AM -0700, Bart Van Assche wrote: > Since the next patch will modify this function such that it > checks more than just the host byte of the SCSI result, rename > __scsi_error_from_host_byte() into scsi_result_to_blk_status(). > This patch does not change any functionality. > > Signed-off-by: Bart Van Assche Looks good, Reviewed-by: Christoph Hellwig
Re: [PATCH v3 1/3] Revert "scsi: core: return BLK_STS_OK for DID_OK in __scsi_error_from_host_byte()"
Looks good: Reviewed-by: Christoph Hellwig
[PATCH v3 3/3] Make scsi_result_to_blk_status() recognize CONDITION MET
Ensure that CONDITION MET and other non-zero status values that
indicate success are translated into BLK_STS_OK.
Signed-off-by: Bart Van Assche
Cc: Hannes Reinecke
Cc: Douglas Gilbert
Cc: Damien Le Moal
Cc: Christoph Hellwig
Cc: Lee Duncan
---
drivers/scsi/scsi_lib.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 26d82f323b31..3e494a476c50 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -733,6 +733,15 @@ static bool scsi_end_request(struct request *req,
blk_status_t error,
static blk_status_t scsi_result_to_blk_status(struct scsi_cmnd *cmd, int
result)
{
switch (host_byte(result)) {
+ case DID_OK:
+ /*
+* Also check the other bytes than the status byte in result
+* to handle the case when a SCSI LLD sets result to
+* DRIVER_SENSE << 24 without setting SAM_STAT_CHECK_CONDITION.
+*/
+ if (scsi_status_is_good(result) && (result & ~0xff) == 0)
+ return BLK_STS_OK;
+ return BLK_STS_IOERR;
case DID_TRANSPORT_FAILFAST:
return BLK_STS_TRANSPORT;
case DID_TARGET_FAILURE:
--
2.16.2
[PATCH v3 2/3] Rename __scsi_error_from_host_byte() into scsi_result_to_blk_status()
Since the next patch will modify this function such that it
checks more than just the host byte of the SCSI result, rename
__scsi_error_from_host_byte() into scsi_result_to_blk_status().
This patch does not change any functionality.
Signed-off-by: Bart Van Assche
Cc: Hannes Reinecke
Cc: Douglas Gilbert
Cc: Damien Le Moal
Cc: Christoph Hellwig
Cc: Lee Duncan
---
drivers/scsi/scsi_lib.c | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index c0e4ae733cce..26d82f323b31 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -723,14 +723,14 @@ static bool scsi_end_request(struct request *req,
blk_status_t error,
}
/**
- * __scsi_error_from_host_byte - translate SCSI error code into errno
- * @cmd: SCSI command (unused)
+ * scsi_result_to_blk_status - translate a SCSI result code into blk_status_t
+ * @cmd: SCSI command
* @result:scsi error code
*
- * Translate SCSI error code into block errors.
+ * Translate a SCSI result code into a blk_status_t value. May reset the host
+ * byte of @cmd->result.
*/
-static blk_status_t __scsi_error_from_host_byte(struct scsi_cmnd *cmd,
- int result)
+static blk_status_t scsi_result_to_blk_status(struct scsi_cmnd *cmd, int
result)
{
switch (host_byte(result)) {
case DID_TRANSPORT_FAILFAST:
@@ -810,10 +810,10 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned
int good_bytes)
SCSI_SENSE_BUFFERSIZE);
}
if (!sense_deferred)
- error = __scsi_error_from_host_byte(cmd,
result);
+ error = scsi_result_to_blk_status(cmd, result);
}
/*
-* __scsi_error_from_host_byte may have reset the host_byte
+* scsi_result_to_blk_status may have reset the host_byte
*/
scsi_req(req)->result = cmd->result;
scsi_req(req)->resid_len = scsi_get_resid(cmd);
@@ -835,7 +835,7 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int
good_bytes)
* good_bytes != blk_rq_bytes(req) as the signal for an error.
* This sets the error explicitly for the problem case.
*/
- error = __scsi_error_from_host_byte(cmd, result);
+ error = scsi_result_to_blk_status(cmd, result);
}
/* no bidi support for !blk_rq_is_passthrough yet */
@@ -905,7 +905,7 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int
good_bytes)
if (result == 0)
goto requeue;
- error = __scsi_error_from_host_byte(cmd, result);
+ error = scsi_result_to_blk_status(cmd, result);
if (host_byte(result) == DID_RESET) {
/* Third party bus reset or reset for error recovery
--
2.16.2
[PATCH v3 1/3] Revert "scsi: core: return BLK_STS_OK for DID_OK in __scsi_error_from_host_byte()"
The description of commit e39a97353e53 is wrong: it mentions that
commit 2a842acab109 introduced a bug in __scsi_error_from_host_byte()
although that commit did not change the behavior of that function.
Additionally, commit e39a97353e53 introduced a bug: it causes commands
that fail with hostbyte=DID_OK and driverbyte=DRIVER_SENSE to be
completed with BLK_STS_OK. Hence revert that commit.
Fixes: e39a97353e53 ("scsi: core: return BLK_STS_OK for DID_OK in
__scsi_error_from_host_byte()")
Reported-by: Damien Le Moal
Signed-off-by: Bart Van Assche
Cc: Hannes Reinecke
Cc: Douglas Gilbert
Cc: Damien Le Moal
Cc: Christoph Hellwig
Cc: Lee Duncan
Cc: sta...@vger.kernel.org
---
drivers/scsi/scsi_lib.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 1d83f29aee74..c0e4ae733cce 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -733,8 +733,6 @@ static blk_status_t __scsi_error_from_host_byte(struct
scsi_cmnd *cmd,
int result)
{
switch (host_byte(result)) {
- case DID_OK:
- return BLK_STS_OK;
case DID_TRANSPORT_FAILFAST:
return BLK_STS_TRANSPORT;
case DID_TARGET_FAILURE:
--
2.16.2
[PATCH v3 0/3] Report all request failures again to user space
Hello Martin, A recent change in the SCSI core caused certain request failures no longer to be reported to user space. Damien noticed this by sending a write request that is not aligned to the write pointer to an SMR drive from user space. Such non-aligned write requests are failed by the drive and such failures should be reported to user space. This patch series makes sure that all SCSI request failures are again reported to user space. This patch series also makes the SCSI core recognize status codes like CONDITION MET as not being an error. Please consider at least the first patch in this series for the rc stage of kernel v4.17. Thanks, Bart. Changes compared to v2: - Split the single v2 patch into two patches: one revert with a cc: stable tag and another patch that translates CONDITION MET into BLK_STS_OK. - Added a third patch that renames __scsi_error_from_host_byte() into scsi_result_to_blk_status(). Changes compared to v1: - Modified __scsi_error_from_host_byte() such that it again returns BLK_STS_OK for CONDITION MET and other result codes that represent success. Bart Van Assche (3): Revert "scsi: core: return BLK_STS_OK for DID_OK in __scsi_error_from_host_byte()" Rename __scsi_error_from_host_byte() into scsi_result_to_blk_status() Make scsi_result_to_blk_status() recognize CONDITION MET drivers/scsi/scsi_lib.c | 27 +-- 1 file changed, 17 insertions(+), 10 deletions(-) -- 2.16.2
Re: [PATCH] scsi: devinfo: Add Microsoft iSCSI target to 1024 sector blacklist
On 03/28/2018 11:33 PM, Long Li wrote:
Subject: Re: [PATCH] scsi: devinfo: Add Microsoft iSCSI target to 1024 sector
blacklist
Long, KY: Please confirm.
The Windows Server 2016 iSCSI target doesn't work with the Linux
kernel initiator since the kernel started sending larger requests by
default, nor does it implement the block limits VPD page. Apply the
sector limit workaround for these targets.
Signed-off-by: Ross Lagerwall
---
drivers/scsi/scsi_devinfo.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/scsi_devinfo.c b/drivers/scsi/scsi_devinfo.c
index f3b1172..5cb748a 100644
--- a/drivers/scsi/scsi_devinfo.c
+++ b/drivers/scsi/scsi_devinfo.c
@@ -213,7 +213,7 @@ static struct {
{"Medion", "Flash XL MMC/SD", "2.6D", BLIST_FORCELUN},
{"MegaRAID", "LD", NULL, BLIST_FORCELUN},
{"MICROP", "4110", NULL, BLIST_NOTQ},
- {"MSFT", "Virtual HD", NULL, BLIST_NO_RSOC},
+ {"MSFT", "Virtual HD", NULL, BLIST_MAX_1024 | BLIST_NO_RSOC},
Ross,
What about storage_channel_properties.max_transfer_bytes returned from
VSTOR_OPERATION_QUERY_PROPERTIES (in storvsc_channel_init())
Does it return correctly the maximum transfer size for iSCSI?
I presume you're referring to the Hyper-V virtual storage driver? This
has nothing to do with that module -- I don't even have it compiled in.
It's just simply the Linux kernel initiator connecting over plain
software iSCSI to a Windows Server 2016 iSCSI target.
This is easy enough to reproduce. Just set up a Windows Server 2016
target and try and use it from Linux. You get I/O errors as soon as you
try and format the disk.
Cheers,
--
Ross Lagerwall
Re: [PATCH] target: change default dbroot to /etc/target
On 04/05/2018 12:17 AM, Christoph Hellwig wrote: > On Wed, Apr 04, 2018 at 12:47:03PM -0700, Lee Duncan wrote: >> The dbroot (target PR database root directory) is >> configurable but default to /var/target, a historic >> value. But the reason for adding configurability >> was to move the target directory out of /var. This >> is because the File Hierarchy Standard v3.0 mandates >> that this "target" directory not be in /var. See >> https://refspecs.linuxfoundation.org/FHS_3.0/fhs-3.0.pdf >> >> This change moves the default from /var/target to >> /etc/target, but this value is still configurable, >> so those wishing to continue to use /var/target >> can still do so. > > This will break upgrades of all existing setups, so NAK. > Hi Christoph: I half expected this response, but here's the problem with the current approach ... The "dbroot" attribute is managed by target_core_mod, but this module is generally never loaded directly. Usually it is loaded because some other module (like ib_isert) is loaded, and it depends on target_core_mod. So if a user starts up rdma services, for example, they end up with a target_core_mod that will not allow dbroot to be changed. This is because the dbroot change rules will not allow a change when there are any children modules. [This problem will continue to grow as we get other target-mode drivers, such as nvme?] So setting the dbroot for all target driver requires some entity to load target_core_mod and set dbroot before any other module that uses target_core_mod loads. This means either we need a special service, at boot time, to load the core driver and set dbroot, so that all target drivers are treated equally, or we put the burden on every target driver to do this setup, which seems like an unreasonable duplication of effort. Bottom line, we need a better way to set dbroot. This is true no matter what the default value is for this attribute. I understand your reluctance to change dbroot, but I also see this leading to /var/target being the default location from now on unless we change it some time. I'm open to suggestions on how we could update this value and not break existing systems. A simple shell script could move things from /var/target to /etc/target, but bare kernels have no way to invoke "upgrade" scripts that I know of. Also, I think it's a bit unlikely that anyone will still be using /var/target, since targetcli-fb has been setting the target root to /etc/target for a while now, and the old targetcli has been deprecated. (It's the only app I know that hard-codes the old location.) For now, I believe a workable solution may be to add "dbroot" as a module parameter? If dbroot was a module param for target_core_mod, then dbroot=/etc/target could be passed in by default (via modprobe.d) for distros that wish to change the location. I will work on such a patch, but if you dislike this idea please feel free to save me some work, and let me know. -- Lee Duncan SUSE Labs
[PATCH v1 15/15] mpt3sas: Update driver version "25.100.00.00"
Update driver version to match OOB/internal driver version. Signed-off-by: Chaitra P B Signed-off-by: Suganath Prabu S --- drivers/scsi/mpt3sas/mpt3sas_base.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.h b/drivers/scsi/mpt3sas/mpt3sas_base.h index 6a9657e..693b04b 100644 --- a/drivers/scsi/mpt3sas/mpt3sas_base.h +++ b/drivers/scsi/mpt3sas/mpt3sas_base.h @@ -74,8 +74,8 @@ #define MPT3SAS_DRIVER_NAME"mpt3sas" #define MPT3SAS_AUTHOR "Avago Technologies " #define MPT3SAS_DESCRIPTION"LSI MPT Fusion SAS 3.0 Device Driver" -#define MPT3SAS_DRIVER_VERSION "17.100.00.00" -#define MPT3SAS_MAJOR_VERSION 17 +#define MPT3SAS_DRIVER_VERSION "25.100.00.00" +#define MPT3SAS_MAJOR_VERSION 25 #define MPT3SAS_MINOR_VERSION 100 #define MPT3SAS_BUILD_VERSION 0 #define MPT3SAS_RELEASE_VERSION00 -- 1.8.3.1
[PATCH v1 14/15] mpt3sas: fix possible memory leak.
In ioctl exit path driver refers ioc_list to free memory associated with
diag buffers and event_log pointer used to save events by driver.
If ctl_exit() func is called after unregistering driver, then ioc_list will
be empty and hence driver will not be able to free the allocated memory
which in turn causes memory leak.
So call ctl_exit() function before unregistering mpt3sas driver.
Signed-off-by: Chaitra P B
Signed-off-by: Suganath Prabu S
---
drivers/scsi/mpt3sas/mpt3sas_scsih.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
index f3e1dc7..f8b5248 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
@@ -11283,10 +11283,10 @@ _mpt3sas_exit(void)
pr_info("mpt3sas version %s unloading\n",
MPT3SAS_DRIVER_VERSION);
- pci_unregister_driver(&mpt3sas_driver);
-
mpt3sas_ctl_exit(hbas_to_enumerate);
+ pci_unregister_driver(&mpt3sas_driver);
+
scsih_exit();
}
--
1.8.3.1
[PATCH v1 10/15] mpt3sas: Cache enclosure pages during enclosure add.
In function _scsih_add_device,
for each device connected to an enclosure, driver reads the
enclosure page(To get details like enclosure handle,
enclosure logical ID, enclosure level etc.)
With this patch, instead of reading enclosure page everytime,
driver maintains a list for enclosure device(During enclosure
add event, enclosure device is added to the list and removed
from the list on delete events) and uses the enclosure page
from the list.
Signed-off-by: Chaitra P B
Signed-off-by: Suganath Prabu S
---
drivers/scsi/mpt3sas/mpt3sas_base.c | 22 +++
drivers/scsi/mpt3sas/mpt3sas_base.h | 14 ++
drivers/scsi/mpt3sas/mpt3sas_scsih.c | 296 +++
3 files changed, 236 insertions(+), 96 deletions(-)
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c
b/drivers/scsi/mpt3sas/mpt3sas_base.c
index f9bacd2..fccf5d1 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -4087,6 +4087,27 @@ _base_static_config_pages(struct MPT3SAS_ADAPTER *ioc)
}
/**
+ * mpt3sas_free_enclosure_list - release memory
+ * @ioc: per adapter object
+ *
+ * Free memory allocated during encloure add.
+ *
+ * Return nothing.
+ */
+void
+mpt3sas_free_enclosure_list(struct MPT3SAS_ADAPTER *ioc)
+{
+ struct _enclosure_node *enclosure_dev, *enclosure_dev_next;
+
+ /* Free enclosure list */
+ list_for_each_entry_safe(enclosure_dev,
+ enclosure_dev_next, &ioc->enclosure_list, list) {
+ list_del(&enclosure_dev->list);
+ kfree(enclosure_dev);
+ }
+}
+
+/**
* _base_release_memory_pools - release memory
* @ioc: per adapter object
*
@@ -,6 +6687,7 @@ mpt3sas_base_detach(struct MPT3SAS_ADAPTER *ioc)
mpt3sas_base_stop_watchdog(ioc);
mpt3sas_base_free_resources(ioc);
_base_release_memory_pools(ioc);
+ mpt3sas_free_enclosure_list(ioc);
pci_set_drvdata(ioc->pdev, NULL);
kfree(ioc->cpu_msix_table);
if (ioc->is_warpdrive)
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.h
b/drivers/scsi/mpt3sas/mpt3sas_base.h
index a0fca8a..6f3329e 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.h
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.h
@@ -741,6 +741,17 @@ struct _sas_node {
struct list_head sas_port_list;
};
+
+/**
+ * struct _enclosure_node - enclosure information
+ * @list: list of enclosures
+ * @pg0: enclosure pg0;
+ */
+struct _enclosure_node {
+ struct list_head list;
+ Mpi2SasEnclosurePage0_t pg0;
+};
+
/**
* enum reset_type - reset state
* @FORCE_BIG_HAMMER: issue diagnostic reset
@@ -1013,6 +1024,7 @@ typedef void (*MPT3SAS_FLUSH_RUNNING_CMDS)(struct
MPT3SAS_ADAPTER *ioc);
* @iounit_pg8: static iounit page 8
* @sas_hba: sas host object
* @sas_expander_list: expander object list
+ * @enclosure_list: enclosure object list
* @sas_node_lock:
* @sas_device_list: sas device object list
* @sas_device_init_list: sas device object list (used only at init time)
@@ -1218,6 +1230,7 @@ struct MPT3SAS_ADAPTER {
/* sas hba, expander, and device list */
struct _sas_node sas_hba;
struct list_head sas_expander_list;
+ struct list_head enclosure_list;
spinlock_t sas_node_lock;
struct list_head sas_device_list;
struct list_head sas_device_init_list;
@@ -1391,6 +1404,7 @@ int mpt3sas_base_attach(struct MPT3SAS_ADAPTER *ioc);
void mpt3sas_base_detach(struct MPT3SAS_ADAPTER *ioc);
int mpt3sas_base_map_resources(struct MPT3SAS_ADAPTER *ioc);
void mpt3sas_base_free_resources(struct MPT3SAS_ADAPTER *ioc);
+void mpt3sas_free_enclosure_list(struct MPT3SAS_ADAPTER *ioc);
int mpt3sas_base_hard_reset_handler(struct MPT3SAS_ADAPTER *ioc,
enum reset_type type);
diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
index f6861a6..2f9121e 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
@@ -1362,6 +1362,30 @@ mpt3sas_scsih_expander_find_by_handle(struct
MPT3SAS_ADAPTER *ioc, u16 handle)
}
/**
+ * mpt3sas_scsih_enclosure_find_by_handle - exclosure device search
+ * @ioc: per adapter object
+ * @handle: enclosure handle (assigned by firmware)
+ * Context: Calling function should acquire ioc->sas_device_lock
+ *
+ * This searches for enclosure device based on handle, then returns the
+ * enclosure object.
+ */
+static struct _enclosure_node *
+mpt3sas_scsih_enclosure_find_by_handle(struct MPT3SAS_ADAPTER *ioc, u16 handle)
+{
+ struct _enclosure_node *enclosure_dev, *r;
+
+ r = NULL;
+ list_for_each_entry(enclosure_dev, &ioc->enclosure_list, list) {
+ if (le16_to_cpu(enclosure_dev->pg0.EnclosureHandle) != handle)
+ continue;
+ r = enclosure_dev;
+ goto out;
+ }
+out:
+ return r;
+}
+/**
* mpt3sas_scsih_expander_find_by_sas_address - expander device search
* @ioc: per adapter object
* @sas_address
[PATCH v1 11/15] mpt3sas: Report Firmware Package Version from HBA Driver.
Added function _base_display_fwpkg_version, which sends FWUpload
request to pull FW package version from FW Image Header.
Now driver prints FW package version in addition to FW
version if the PackageVersion is valid.
Signed-off-by: Chaitra P B
Signed-off-by: Suganath Prabu S
---
drivers/scsi/mpt3sas/mpt3sas_base.c | 109 +++-
drivers/scsi/mpt3sas/mpt3sas_base.h | 1 +
2 files changed, 108 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c
b/drivers/scsi/mpt3sas/mpt3sas_base.c
index fccf5d1..7f438be 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -3825,6 +3825,105 @@ _base_display_OEMs_branding(struct MPT3SAS_ADAPTER *ioc)
}
/**
+ * _base_display_fwpkg_version - sends FWUpload request to pull FWPkg
+ * version from FW Image Header.
+ * @ioc: per adapter object
+ *
+ * Returns 0 for success, non-zero for failure.
+ */
+ static int
+_base_display_fwpkg_version(struct MPT3SAS_ADAPTER *ioc)
+{
+ Mpi2FWImageHeader_t *FWImgHdr;
+ Mpi25FWUploadRequest_t *mpi_request;
+ Mpi2FWUploadReply_t mpi_reply;
+ int r = 0;
+ void *fwpkg_data = NULL;
+ dma_addr_t fwpkg_data_dma;
+ u16 smid, ioc_status;
+ size_t data_length;
+
+ dinitprintk(ioc, pr_info(MPT3SAS_FMT "%s\n", ioc->name,
+ __func__));
+
+ if (ioc->base_cmds.status & MPT3_CMD_PENDING) {
+ pr_err(MPT3SAS_FMT "%s: internal command already in use\n",
+ ioc->name, __func__);
+ return -EAGAIN;
+ }
+
+ data_length = sizeof(Mpi2FWImageHeader_t);
+ fwpkg_data = pci_alloc_consistent(ioc->pdev, data_length,
+ &fwpkg_data_dma);
+ if (!fwpkg_data) {
+ pr_err(MPT3SAS_FMT "failure at %s:%d/%s()!\n",
+ ioc->name, __FILE__, __LINE__, __func__);
+ return -ENOMEM;
+ }
+
+ smid = mpt3sas_base_get_smid(ioc, ioc->base_cb_idx);
+ if (!smid) {
+ pr_err(MPT3SAS_FMT "%s: failed obtaining a smid\n",
+ ioc->name, __func__);
+ r = -EAGAIN;
+ goto out;
+ }
+
+ ioc->base_cmds.status = MPT3_CMD_PENDING;
+ mpi_request = mpt3sas_base_get_msg_frame(ioc, smid);
+ ioc->base_cmds.smid = smid;
+ memset(mpi_request, 0, sizeof(Mpi25FWUploadRequest_t));
+ mpi_request->Function = MPI2_FUNCTION_FW_UPLOAD;
+ mpi_request->ImageType = MPI2_FW_UPLOAD_ITYPE_FW_FLASH;
+ mpi_request->ImageSize = cpu_to_le32(data_length);
+ ioc->build_sg(ioc, &mpi_request->SGL, 0, 0, fwpkg_data_dma,
+ data_length);
+ init_completion(&ioc->base_cmds.done);
+ mpt3sas_base_put_smid_default(ioc, smid);
+ /* Wait for 15 seconds */
+ wait_for_completion_timeout(&ioc->base_cmds.done,
+ FW_IMG_HDR_READ_TIMEOUT*HZ);
+ pr_info(MPT3SAS_FMT "%s: complete\n",
+ ioc->name, __func__);
+ if (!(ioc->base_cmds.status & MPT3_CMD_COMPLETE)) {
+ pr_err(MPT3SAS_FMT "%s: timeout\n",
+ ioc->name, __func__);
+ _debug_dump_mf(mpi_request,
+ sizeof(Mpi25FWUploadRequest_t)/4);
+ r = -ETIME;
+ } else {
+ memset(&mpi_reply, 0, sizeof(Mpi2FWUploadReply_t));
+ if (ioc->base_cmds.status & MPT3_CMD_REPLY_VALID) {
+ memcpy(&mpi_reply, ioc->base_cmds.reply,
+ sizeof(Mpi2FWUploadReply_t));
+ ioc_status = le16_to_cpu(mpi_reply.IOCStatus) &
+ MPI2_IOCSTATUS_MASK;
+ if (ioc_status == MPI2_IOCSTATUS_SUCCESS) {
+ FWImgHdr = (Mpi2FWImageHeader_t *)fwpkg_data;
+ if (FWImgHdr->PackageVersion.Word) {
+ pr_info(MPT3SAS_FMT "FW Package Version"
+ "(%02d.%02d.%02d.%02d)\n",
+ ioc->name,
+ FWImgHdr->PackageVersion.Struct.Major,
+ FWImgHdr->PackageVersion.Struct.Minor,
+ FWImgHdr->PackageVersion.Struct.Unit,
+ FWImgHdr->PackageVersion.Struct.Dev);
+ }
+ } else {
+ _debug_dump_mf(&mpi_reply,
+ sizeof(Mpi2FWUploadReply_t)/4);
+ }
+ }
+ }
+ ioc->base_cmds.status = MPT3_CMD_NOT_USED;
+out:
+ if (fwpkg_data)
+ pci_free_consistent(ioc->pdev, data_length, fwp
[PATCH v1 13/15] mpt3sas: For NVME device, issue a protocol level reset instead of hot reset and use TM timeout value exposed in PCIe Device Page 2.
1)Manufacturing Page 11 contains parameters to control
internal firmware behavior. Based on AddlFlags2 field
FW/Driver behaviour can be changed, (flag tm_custom_handling
is used for this)
a) For PCIe device, protocol level reset should be used if
flag tm_custom_handling is 0.
Since Abort Task Set, LUN reset and Target reset will result
in a protocol level reset. Drivers should issue only one type
of this reset, if that fails then it should escalate to a controller
reset (diag reset/OCR).
b) If the driver has control over the TM reset timeout value, then
driver should use the value exposed in PCIe Device Page 2 for pcie device
(field ControllerResetTO).
Signed-off-by: Chaitra P B
Signed-off-by: Suganath Prabu S
---
drivers/scsi/mpt3sas/mpt3sas_base.c | 13 ++
drivers/scsi/mpt3sas/mpt3sas_base.h | 26 ++--
drivers/scsi/mpt3sas/mpt3sas_ctl.c | 22 +--
drivers/scsi/mpt3sas/mpt3sas_scsih.c | 76 ++--
4 files changed, 116 insertions(+), 21 deletions(-)
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c
b/drivers/scsi/mpt3sas/mpt3sas_base.c
index 7f438be..eaeace0 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -4139,6 +4139,7 @@ _base_static_config_pages(struct MPT3SAS_ADAPTER *ioc)
Mpi2ConfigReply_t mpi_reply;
u32 iounit_pg1_flags;
+ ioc->nvme_abort_timeout = 30;
mpt3sas_config_get_manufacturing_pg0(ioc, &mpi_reply, &ioc->manu_pg0);
if (ioc->ir_firmware)
mpt3sas_config_get_manufacturing_pg10(ioc, &mpi_reply,
@@ -4157,6 +4158,18 @@ _base_static_config_pages(struct MPT3SAS_ADAPTER *ioc)
mpt3sas_config_set_manufacturing_pg11(ioc, &mpi_reply,
&ioc->manu_pg11);
}
+ if (ioc->manu_pg11.AddlFlags2 & NVME_TASK_MNGT_CUSTOM_MASK)
+ ioc->tm_custom_handling = 1;
+ else {
+ ioc->tm_custom_handling = 0;
+ if (ioc->manu_pg11.NVMeAbortTO < NVME_TASK_ABORT_MIN_TIMEOUT)
+ ioc->nvme_abort_timeout = NVME_TASK_ABORT_MIN_TIMEOUT;
+ else if (ioc->manu_pg11.NVMeAbortTO >
+ NVME_TASK_ABORT_MAX_TIMEOUT)
+ ioc->nvme_abort_timeout = NVME_TASK_ABORT_MAX_TIMEOUT;
+ else
+ ioc->nvme_abort_timeout = ioc->manu_pg11.NVMeAbortTO;
+ }
mpt3sas_config_get_bios_pg2(ioc, &mpi_reply, &ioc->bios_pg2);
mpt3sas_config_get_bios_pg3(ioc, &mpi_reply, &ioc->bios_pg3);
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.h
b/drivers/scsi/mpt3sas/mpt3sas_base.h
index 20fe90d..6a9657e 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.h
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.h
@@ -146,8 +146,12 @@
#defineNVME_CMD_PRP1_OFFSET24 /* PRP1 offset in NVMe
cmd */
#defineNVME_CMD_PRP2_OFFSET32 /* PRP2 offset in NVMe
cmd */
#defineNVME_ERROR_RESPONSE_SIZE16 /* Max NVME Error
Response */
+#define NVME_TASK_ABORT_MIN_TIMEOUT6
+#define NVME_TASK_ABORT_MAX_TIMEOUT60
+#define NVME_TASK_MNGT_CUSTOM_MASK (0x0010)
#defineNVME_PRP_PAGE_SIZE 4096/* Page size */
+
/*
* reset phases
*/
@@ -363,7 +367,15 @@ struct Mpi2ManufacturingPage11_t {
u8 EEDPTagMode;/* 09h */
u8 Reserved3; /* 0Ah */
u8 Reserved4; /* 0Bh */
- __le32 Reserved5[23]; /* 0Ch-60h*/
+ __le32 Reserved5[8]; /* 0Ch-2Ch */
+ u16 AddlFlags2; /* 2Ch */
+ u8 AddlFlags3; /* 2Eh */
+ u8 Reserved6; /* 2Fh */
+ __le32 Reserved7[7]; /* 30h - 4Bh */
+ u8 NVMeAbortTO;/* 4Ch */
+ u8 Reserved8; /* 4Dh */
+ u16 Reserved9; /* 4Eh */
+ __le32 Reserved10[4]; /* 50h - 60h */
};
/**
@@ -573,6 +585,7 @@ struct _pcie_device {
u8 enclosure_level;
u8 connector_name[4];
u8 *serial_number;
+ u8 reset_timeout;
struct kref refcount;
};
/**
@@ -1211,6 +1224,10 @@ struct MPT3SAS_ADAPTER {
void*event_log;
u32 event_masks[MPI2_EVENT_NOTIFY_EVENTMASK_WORDS];
+ u8 tm_custom_handling;
+ u8 nvme_abort_timeout;
+
+
/* static config pages */
struct mpt3sas_facts facts;
struct mpt3sas_port_facts *pfacts;
@@ -1470,10 +1487,11 @@ u8 mpt3sas_scsih_event_callback(struct MPT3SAS_ADAPTER
*ioc, u8 msix_index,
u32 reply);
void mpt3sas_scsih_reset_handler(struct MPT3SAS_ADAPTER *ioc, int reset_phase);
-int mpt3sas_scsih_issue_tm(struct MPT3SAS_ADAPTER *ioc, u16 handle,
- u64 lun, u8 ty
[PATCH v1 09/15] mpt3sas: Allow processing of events during driver unload.
Events were not processed during driver unload, hence unloading of driver
doesn't complete when drives are disconnected while unloading of driver.
So don't block events in ISR path, i,e., remove the flag ioc->remove_host
so that events are getting processed during driver unload.
Thus allowing driver unload to complete by processing drive removal events
during driver unload.
Signed-off-by: Chaitra P B
Signed-off-by: Suganath Prabu S
---
drivers/scsi/mpt3sas/mpt3sas_scsih.c | 16 +---
1 file changed, 5 insertions(+), 11 deletions(-)
diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
index 0e06d19..f6861a6 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
@@ -3677,11 +3677,7 @@ _scsih_tm_tr_complete(struct MPT3SAS_ADAPTER *ioc, u16
smid, u8 msix_index,
u32 ioc_state;
struct _sc_list *delayed_sc;
- if (ioc->remove_host) {
- dewtprintk(ioc, pr_info(MPT3SAS_FMT
- "%s: host has been removed\n", __func__, ioc->name));
- return 1;
- } else if (ioc->pci_error_recovery) {
+ if (ioc->pci_error_recovery) {
dewtprintk(ioc, pr_info(MPT3SAS_FMT
"%s: host in pci error recovery\n", __func__,
ioc->name));
@@ -3803,8 +3799,7 @@ _scsih_tm_tr_volume_send(struct MPT3SAS_ADAPTER *ioc, u16
handle)
u16 smid;
struct _tr_list *delayed_tr;
- if (ioc->shost_recovery || ioc->remove_host ||
- ioc->pci_error_recovery) {
+ if (ioc->pci_error_recovery) {
dewtprintk(ioc, pr_info(MPT3SAS_FMT
"%s: host reset in progress!\n",
__func__, ioc->name));
@@ -3857,8 +3852,7 @@ _scsih_tm_volume_tr_complete(struct MPT3SAS_ADAPTER *ioc,
u16 smid,
Mpi2SCSITaskManagementReply_t *mpi_reply =
mpt3sas_base_get_reply_virt_addr(ioc, reply);
- if (ioc->shost_recovery || ioc->remove_host ||
- ioc->pci_error_recovery) {
+ if (ioc->shost_recovery || ioc->pci_error_recovery) {
dewtprintk(ioc, pr_info(MPT3SAS_FMT
"%s: host reset in progress!\n",
__func__, ioc->name));
@@ -9475,8 +9469,8 @@ mpt3sas_scsih_event_callback(struct MPT3SAS_ADAPTER *ioc,
u8 msix_index,
u16 sz;
Mpi26EventDataActiveCableExcept_t *ActiveCableEventData;
- /* events turned off due to host reset or driver unloading */
- if (ioc->remove_host || ioc->pci_error_recovery)
+ /* events turned off due to host reset */
+ if (ioc->pci_error_recovery)
return 1;
mpi_reply = mpt3sas_base_get_reply_virt_addr(ioc, reply);
--
1.8.3.1
[PATCH v1 12/15] mpt3sas: Update MPI Headers
Update MPI Files to support protocol level reset for NVMe device.
Signed-off-by: Chaitra P B
Signed-off-by: Suganath Prabu S
---
drivers/scsi/mpt3sas/mpi/mpi2.h | 9 ++---
drivers/scsi/mpt3sas/mpi/mpi2_cnfg.h | 30 --
drivers/scsi/mpt3sas/mpi/mpi2_ioc.h | 7 ++-
3 files changed, 36 insertions(+), 10 deletions(-)
diff --git a/drivers/scsi/mpt3sas/mpi/mpi2.h b/drivers/scsi/mpt3sas/mpi/mpi2.h
index b015c30..1e45268 100644
--- a/drivers/scsi/mpt3sas/mpi/mpi2.h
+++ b/drivers/scsi/mpt3sas/mpi/mpi2.h
@@ -9,7 +9,7 @@
* scatter/gather formats.
* Creation Date: June 21, 2006
*
- * mpi2.h Version: 02.00.48
+ * mpi2.h Version: 02.00.50
*
* NOTE: Names (typedefs, defines, etc.) beginning with an MPI25 or Mpi25
* prefix are for use only on MPI v2.5 products, and must not be used
@@ -114,6 +114,8 @@
* 09-02-16 02.00.46 Bumped MPI2_HEADER_VERSION_UNIT.
* 11-23-16 02.00.47 Bumped MPI2_HEADER_VERSION_UNIT.
* 02-03-17 02.00.48 Bumped MPI2_HEADER_VERSION_UNIT.
+ * 06-13-17 02.00.49 Bumped MPI2_HEADER_VERSION_UNIT.
+ * 09-29-17 02.00.50 Bumped MPI2_HEADER_VERSION_UNIT.
* --
*/
@@ -152,8 +154,9 @@
MPI26_VERSION_MINOR)
#define MPI2_VERSION_02_06 (0x0206)
-/*Unit and Dev versioning for this MPI header set */
-#define MPI2_HEADER_VERSION_UNIT(0x30)
+
+/* Unit and Dev versioning for this MPI header set */
+#define MPI2_HEADER_VERSION_UNIT(0x32)
#define MPI2_HEADER_VERSION_DEV (0x00)
#define MPI2_HEADER_VERSION_UNIT_MASK (0xFF00)
#define MPI2_HEADER_VERSION_UNIT_SHIFT (8)
diff --git a/drivers/scsi/mpt3sas/mpi/mpi2_cnfg.h
b/drivers/scsi/mpt3sas/mpi/mpi2_cnfg.h
index 0ad88de..5122920 100644
--- a/drivers/scsi/mpt3sas/mpi/mpi2_cnfg.h
+++ b/drivers/scsi/mpt3sas/mpi/mpi2_cnfg.h
@@ -7,7 +7,7 @@
* Title: MPI Configuration messages and pages
* Creation Date: November 10, 2006
*
- * mpi2_cnfg.h Version: 02.00.40
+ *mpi2_cnfg.h Version: 02.00.42
*
* NOTE: Names (typedefs, defines, etc.) beginning with an MPI25 or Mpi25
* prefix are for use only on MPI v2.5 products, and must not be used
@@ -219,6 +219,18 @@
* Added ChassisSlot field to SAS Enclosure Page 0.
* Added ChassisSlot Valid bit (bit 5) to the Flags field
* in SAS Enclosure Page 0.
+ * 06-13-17 02.00.41 Added MPI26_MFGPAGE_DEVID_SAS3816 and
+ * MPI26_MFGPAGE_DEVID_SAS3916 defines.
+ * Removed MPI26_MFGPAGE_DEVID_SAS4008 define.
+ * Added MPI26_PCIEIOUNIT1_LINKFLAGS_SRNS_EN define.
+ * Renamed PI26_PCIEIOUNIT1_LINKFLAGS_EN_SRIS to
+ * PI26_PCIEIOUNIT1_LINKFLAGS_SRIS_EN.
+ * Renamed MPI26_PCIEIOUNIT1_LINKFLAGS_DIS_SRIS to
+ * MPI26_PCIEIOUNIT1_LINKFLAGS_DIS_SEPARATE_REFCLK.
+ * 09-29-17 02.00.42 Added ControllerResetTO field to PCIe Device Page 2.
+ * Added NOIOB field to PCIe Device Page 2.
+ * Added MPI26_PCIEDEV2_CAP_DATA_BLK_ALIGN_AND_GRAN to
+ * the Capabilities field of PCIe Device Page 2.
* --
*/
@@ -556,7 +568,8 @@ typedef struct _MPI2_CONFIG_REPLY {
#define MPI26_MFGPAGE_DEVID_SAS3616 (0x00D1)
#define MPI26_MFGPAGE_DEVID_SAS3708 (0x00D2)
-#define MPI26_MFGPAGE_DEVID_SAS4008 (0x00A1)
+#define MPI26_MFGPAGE_DEVID_SAS3816 (0x00A1)
+#define MPI26_MFGPAGE_DEVID_SAS3916 (0x00A0)
/*Manufacturing Page 0 */
@@ -3864,20 +3877,25 @@ typedef struct _MPI26_CONFIG_PAGE_PCIEDEV_0 {
typedef struct _MPI26_CONFIG_PAGE_PCIEDEV_2 {
MPI2_CONFIG_EXTENDED_PAGE_HEADERHeader; /*0x00 */
U16 DevHandle; /*0x08 */
- U16 Reserved1; /*0x0A */
- U32 MaximumDataTransferSize;/*0x0C */
+ U8 ControllerResetTO; /* 0x0A */
+ U8 Reserved1; /* 0x0B */
+ U32 MaximumDataTransferSize;/*0x0C */
U32 Capabilities; /*0x10 */
- U32 Reserved2; /*0x14 */
+ U16 NOIOB; /* 0x14 */
+ U16 Reserved2; /* 0x16 */
} MPI26_CONFIG_PAGE_PCIEDEV_2, *PTR_MPI26_CONFIG_PAGE_PCIEDEV_2,
Mpi26PCIeDevicePage2_t, *pMpi26PCIeDevicePage2_t;
-#define MPI26_PCIEDEVICE2_PAGEVERSION (0x00)
+#define MPI26_PCIEDEVICE2_PAGEVERSION (0x01)
/*defines for PCIe Device Page 2 Capabilities field */
+#define MPI26_PCIEDEV2_CAP_DATA_BLK_ALIGN_AND_GRAN (0x0008)
#define MPI26_PCIEDEV2_CAP_SGL_FORMAT (0x0004)
#define MPI26_PCIEDEV2_CAP_BIT
[PATCH v1 07/15] mpt3sas: Added support for SAS Device Discovery Error Event.
The SAS Device Discovery Error Event is sent to the host when
discovery for a particular device is failed during discovery,
even after maximum retries by the IOC.
Signed-off-by: Chaitra P B
Signed-off-by: Suganath Prabu S
---
drivers/scsi/mpt3sas/mpt3sas_base.c | 4
drivers/scsi/mpt3sas/mpt3sas_scsih.c | 42
2 files changed, 46 insertions(+)
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c
b/drivers/scsi/mpt3sas/mpt3sas_base.c
index bd1beaf..f9bacd2 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -1029,6 +1029,9 @@ _base_display_event_data(struct MPT3SAS_ADAPTER *ioc,
case MPI2_EVENT_ACTIVE_CABLE_EXCEPTION:
desc = "Cable Event";
break;
+ case MPI2_EVENT_SAS_DEVICE_DISCOVERY_ERROR:
+ desc = "SAS Device Discovery Error";
+ break;
case MPI2_EVENT_PCIE_DEVICE_STATUS_CHANGE:
desc = "PCIE Device Status Change";
break;
@@ -6596,6 +6599,7 @@ mpt3sas_base_attach(struct MPT3SAS_ADAPTER *ioc)
_base_unmask_events(ioc, MPI2_EVENT_LOG_ENTRY_ADDED);
_base_unmask_events(ioc, MPI2_EVENT_TEMP_THRESHOLD);
_base_unmask_events(ioc, MPI2_EVENT_ACTIVE_CABLE_EXCEPTION);
+ _base_unmask_events(ioc, MPI2_EVENT_SAS_DEVICE_DISCOVERY_ERROR);
if (ioc->hba_mpi_version_belonged == MPI26_VERSION) {
if (ioc->is_gen35_ioc) {
_base_unmask_events(ioc,
diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
index 6b1aaa0..0e06d19 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
@@ -7531,6 +7531,44 @@ _scsih_sas_discovery_event(struct MPT3SAS_ADAPTER *ioc,
}
/**
+ * _scsih_sas_device_discovery_error_event - display SAS device discovery error
+ * events
+ * @ioc: per adapter object
+ * @fw_event: The fw_event_work object
+ * Context: user.
+ *
+ * Return nothing.
+ */
+static void
+_scsih_sas_device_discovery_error_event(struct MPT3SAS_ADAPTER *ioc,
+ struct fw_event_work *fw_event)
+{
+ Mpi25EventDataSasDeviceDiscoveryError_t *event_data =
+ (Mpi25EventDataSasDeviceDiscoveryError_t *)fw_event->event_data;
+
+ switch (event_data->ReasonCode) {
+ case MPI25_EVENT_SAS_DISC_ERR_SMP_FAILED:
+ pr_warn(MPT3SAS_FMT "SMP command sent to the expander"
+ "(handle:0x%04x, sas_address:0x%016llx,"
+ "physical_port:0x%02x) has failed",
+ ioc->name, le16_to_cpu(event_data->DevHandle),
+ (unsigned long long)le64_to_cpu(event_data->SASAddress),
+ event_data->PhysicalPort);
+ break;
+ case MPI25_EVENT_SAS_DISC_ERR_SMP_TIMEOUT:
+ pr_warn(MPT3SAS_FMT "SMP command sent to the expander"
+ "(handle:0x%04x, sas_address:0x%016llx,"
+ "physical_port:0x%02x) has timed out",
+ ioc->name, le16_to_cpu(event_data->DevHandle),
+ (unsigned long long)le64_to_cpu(event_data->SASAddress),
+ event_data->PhysicalPort);
+ break;
+ default:
+ break;
+ }
+}
+
+/**
* _scsih_pcie_enumeration_event - handle enumeration events
* @ioc: per adapter object
* @fw_event: The fw_event_work object
@@ -9357,6 +9395,9 @@ _mpt3sas_fw_work(struct MPT3SAS_ADAPTER *ioc, struct
fw_event_work *fw_event)
case MPI2_EVENT_SAS_DISCOVERY:
_scsih_sas_discovery_event(ioc, fw_event);
break;
+ case MPI2_EVENT_SAS_DEVICE_DISCOVERY_ERROR:
+ _scsih_sas_device_discovery_error_event(ioc, fw_event);
+ break;
case MPI2_EVENT_SAS_BROADCAST_PRIMITIVE:
_scsih_sas_broadcast_primitive_event(ioc, fw_event);
break;
@@ -9541,6 +9582,7 @@ mpt3sas_scsih_event_callback(struct MPT3SAS_ADAPTER *ioc,
u8 msix_index,
case MPI2_EVENT_SAS_DEVICE_STATUS_CHANGE:
case MPI2_EVENT_IR_OPERATION_STATUS:
case MPI2_EVENT_SAS_DISCOVERY:
+ case MPI2_EVENT_SAS_DEVICE_DISCOVERY_ERROR:
case MPI2_EVENT_SAS_ENCL_DEVICE_STATUS_CHANGE:
case MPI2_EVENT_IR_PHYSICAL_DISK:
case MPI2_EVENT_PCIE_ENUMERATION:
--
1.8.3.1
[PATCH v1 06/15] mpt3sas: Enhanced handling of Sense Buffer.
Enhanced DMA allocation for Sense Buffer, if the allocation does not fit
within same 4GB.Introduced is_MSB_are_same function to check if allocted
buffer within 4GB range or not.
Signed-off-by: Chaitra P B
Signed-off-by: Suganath Prabu S
---
drivers/scsi/mpt3sas/mpt3sas_base.c | 56 +
1 file changed, 56 insertions(+)
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c
b/drivers/scsi/mpt3sas/mpt3sas_base.c
index 701e1e7..bd1beaf 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -4205,6 +4205,31 @@ _base_release_memory_pools(struct MPT3SAS_ADAPTER *ioc)
}
/**
+ * is_MSB_are_same - checks whether all reply queues in a set are
+ * having same upper 32bits in their base memory address.
+ * @reply_pool_start_address: Base address of a reply queue set
+ * @pool_sz: Size of single Reply Descriptor Post Queues pool size
+ *
+ * Returns 1 if reply queues in a set have a same upper 32bits
+ * in their base memory address,
+ * else 0
+ */
+
+static int
+is_MSB_are_same(long reply_pool_start_address, u32 pool_sz)
+{
+ long reply_pool_end_address;
+
+ reply_pool_end_address = reply_pool_start_address + pool_sz;
+
+ if (upper_32_bits(reply_pool_start_address) ==
+ upper_32_bits(reply_pool_end_address))
+ return 1;
+ else
+ return 0;
+}
+
+/**
* _base_allocate_memory_pools - allocate start of day memory pools
* @ioc: per adapter object
*
@@ -4664,6 +4689,37 @@ _base_allocate_memory_pools(struct MPT3SAS_ADAPTER *ioc)
ioc->name);
goto out;
}
+ /* sense buffer requires to be in same 4 gb region.
+* Below function will check the same.
+* In case of failure, new pci pool will be created with updated
+* alignment. Older allocation and pool will be destroyed.
+* Alignment will be used such a way that next allocation if
+* success, will always meet same 4gb region requirement.
+* Actual requirement is not alignment, but we need start and end of
+* DMA address must have same upper 32 bit address.
+*/
+ if (!is_MSB_are_same((long)ioc->sense, sz)) {
+ //Release Sense pool & Reallocate
+ dma_pool_free(ioc->sense_dma_pool, ioc->sense, ioc->sense_dma);
+ dma_pool_destroy(ioc->sense_dma_pool);
+ ioc->sense = NULL;
+
+ ioc->sense_dma_pool =
+ dma_pool_create("sense pool", &ioc->pdev->dev, sz,
+ roundup_pow_of_two(sz), 0);
+ if (!ioc->sense_dma_pool) {
+ pr_err(MPT3SAS_FMT "sense pool: pci_pool_create
failed\n",
+ ioc->name);
+ goto out;
+ }
+ ioc->sense = dma_pool_alloc(ioc->sense_dma_pool, GFP_KERNEL,
+ &ioc->sense_dma);
+ if (!ioc->sense) {
+ pr_err(MPT3SAS_FMT "sense pool: pci_pool_alloc
failed\n",
+ ioc->name);
+ goto out;
+ }
+ }
dinitprintk(ioc, pr_info(MPT3SAS_FMT
"sense pool(0x%p): depth(%d), element_size(%d), pool_size"
"(%d kB)\n", ioc->name, ioc->sense, ioc->scsiio_depth,
--
1.8.3.1
[PATCH v1 08/15] mpt3sas: Increase event log buffer to support 24 port HBA's.
For 24 port HBA's events generated by IOC are more in certain cases and
the current circular buffer may be overwritten.Hence increased the event
log buffer to accommodate more events.
Signed-off-by: Chaitra P B
Signed-off-by: Suganath Prabu S
---
drivers/scsi/mpt3sas/mpt3sas_ctl.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/mpt3sas/mpt3sas_ctl.h
b/drivers/scsi/mpt3sas/mpt3sas_ctl.h
index a44046c..18b46fa 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_ctl.h
+++ b/drivers/scsi/mpt3sas/mpt3sas_ctl.h
@@ -184,7 +184,7 @@ struct mpt3_ioctl_iocinfo {
/* number of event log entries */
-#define MPT3SAS_CTL_EVENT_LOG_SIZE (50)
+#define MPT3SAS_CTL_EVENT_LOG_SIZE (200)
/**
* struct mpt3_ioctl_eventquery - query event count and type
--
1.8.3.1
[PATCH v1 03/15] mpt3sas: Add sanity checks for scsi tracker before accessing it.
Check scsi tracker 'st' for NULL and st->smid for zero (as driver uses smid
starting from one) before accessing it.
These checks are added as there are possibilities for getting valid
scsi_cmd when driver calls scsi_host_find_tag() API when it loops using
smid(i.e tag) from one to hba queue depth but still scsi tracker st for
this corresponding scsi_cmd is not yet initialized.
For example below are such scenario:
Sometimes it is possible that scsi_cmd might have created at SML but it
might not be issued to the driver (or driver might have returned the
command with Host busy status) as the host reset operation / TMs is in
progress.In such case where the scsi_cmd is not yet processed by driver
then the scsi tracker 'st' of that scsi_cmd & the fields of this 'st' will
be uninitialized.
And hence this patch add checks for 'st' in IOCTL path for TMs issued from
applications and also in host reset path where driver flushes all the
outstanding commands as part of host reset operation.
Signed-off-by: Chaitra P B
Signed-off-by: Suganath Prabu S
---
drivers/scsi/mpt3sas/mpt3sas_ctl.c | 5 -
drivers/scsi/mpt3sas/mpt3sas_scsih.c | 9 -
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/mpt3sas/mpt3sas_ctl.c
b/drivers/scsi/mpt3sas/mpt3sas_ctl.c
index c1b17d6..2f27d5c 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_ctl.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_ctl.c
@@ -590,7 +590,8 @@ _ctl_set_task_mid(struct MPT3SAS_ADAPTER *ioc, struct
mpt3_ioctl_command *karg,
struct scsiio_tracker *st;
scmd = mpt3sas_scsih_scsi_lookup_get(ioc, smid);
- if (!scmd)
+ if (scmd == NULL || scmd->device == NULL ||
+ scmd->device->hostdata == NULL)
continue;
if (lun != scmd->device->lun)
continue;
@@ -600,6 +601,8 @@ _ctl_set_task_mid(struct MPT3SAS_ADAPTER *ioc, struct
mpt3_ioctl_command *karg,
if (priv_data->sas_target->handle != handle)
continue;
st = scsi_cmd_priv(scmd);
+ if ((!st) || (st->smid == 0))
+ continue;
tm_request->TaskMID = cpu_to_le16(st->smid);
found = 1;
}
diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
index c9cce65..6b1aaa0 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
@@ -1465,7 +1465,7 @@ mpt3sas_scsih_scsi_lookup_get(struct MPT3SAS_ADAPTER
*ioc, u16 smid)
scmd = scsi_host_find_tag(ioc->shost, unique_tag);
if (scmd) {
st = scsi_cmd_priv(scmd);
- if (st->cb_idx == 0xFF)
+ if ((!st) || (st->cb_idx == 0xFF) || (st->smid == 0))
scmd = NULL;
}
}
@@ -4451,6 +4451,13 @@ _scsih_flush_running_cmds(struct MPT3SAS_ADAPTER *ioc)
count++;
_scsih_set_satl_pending(scmd, false);
st = scsi_cmd_priv(scmd);
+ /*
+* It may be possible that SCSI scmd got prepared by SML
+* but it has not issued to the driver, for these type of
+* scmd's don't do anything"
+*/
+ if (st && st->smid == 0)
+ continue;
mpt3sas_base_clear_st(ioc, st);
scsi_dma_unmap(scmd);
if (ioc->pci_error_recovery)
--
1.8.3.1
[PATCH v1 05/15] mpt3sas: Optimize I/O memory consumption in driver.
For every IO, memory of PAGE size is allocated for handling NVMe native
PRPS. And in addition to that for every IO (chains need per IO * chain
buffer size, e.g. 38 * 128byte) amount of memory is allocated for chain
buffers.
However, at any point of time; the IO request can be for NVMe target device
(where PRP's page is used for framing PRP's) or can be for SCSI target
device (where chain buffers are used for framing chain SGE's). This patch
modifies the driver to reuse same pre-allocated PRP page buffers as a chain
buffer for IO's targeted for SCSI target devices. No need to allocate
separate buffers for chain SGE's buffers.
Suppose if the number of chain buffers need for IO doesn't fit in the PRP
Page size then driver maintain's separate buffers for those extra chain
buffers that exceeds the PRP page size. For example consider PRP page size
as 4K and chain buffer size as 128 bytes, then number of chain buffers that
can fit in PRP page is 4096/128 => 32. if the number of chain buffer need
per IO exceeds 32; for example consider number of chains need per IO is 36
then for remaining 4 chain buffer's driver allocates them individual.
Signed-off-by: Chaitra P B
Signed-off-by: Suganath Prabu S
---
drivers/scsi/mpt3sas/mpt3sas_base.c | 80 +++--
1 file changed, 51 insertions(+), 29 deletions(-)
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c
b/drivers/scsi/mpt3sas/mpt3sas_base.c
index 1e8e399..701e1e7 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -4188,7 +4188,8 @@ _base_release_memory_pools(struct MPT3SAS_ADAPTER *ioc)
kfree(ioc->internal_lookup);
if (ioc->chain_lookup) {
for (i = 0; i < ioc->scsiio_depth; i++) {
- for (j = 0; j < ioc->chains_needed_per_io; j++) {
+ for (j = ioc->chains_per_prp_buffer;
+ j < ioc->chains_needed_per_io; j++) {
ct = &ioc->chain_lookup[i].chains_per_smid[j];
if (ct && ct->chain_buffer)
dma_pool_free(ioc->chain_dma_pool,
@@ -4506,7 +4507,7 @@ _base_allocate_memory_pools(struct MPT3SAS_ADAPTER *ioc)
ioc->chain_lookup = kzalloc(sz, GFP_KERNEL);
if (!ioc->chain_lookup) {
pr_err(MPT3SAS_FMT "chain_lookup: __get_free_pages "
- "failed\n", ioc->name);
+ "failed\n", ioc->name);
goto out;
}
@@ -4520,33 +4521,6 @@ _base_allocate_memory_pools(struct MPT3SAS_ADAPTER *ioc)
}
}
- ioc->chain_dma_pool = dma_pool_create("chain pool", &ioc->pdev->dev,
- ioc->chain_segment_sz, 16, 0);
- if (!ioc->chain_dma_pool) {
- pr_err(MPT3SAS_FMT "chain_dma_pool: dma_pool_create failed\n",
- ioc->name);
- goto out;
- }
- for (i = 0; i < ioc->scsiio_depth; i++) {
- for (j = 0; j < ioc->chains_needed_per_io; j++) {
- ct = &ioc->chain_lookup[i].chains_per_smid[j];
- ct->chain_buffer = dma_pool_alloc(
- ioc->chain_dma_pool , GFP_KERNEL,
- &ct->chain_buffer_dma);
- if (!ct->chain_buffer) {
- pr_err(MPT3SAS_FMT "chain_lookup: "
- " pci_pool_alloc failed\n", ioc->name);
- goto out;
- }
- }
- total_sz += ioc->chain_segment_sz;
- }
-
- dinitprintk(ioc, pr_info(MPT3SAS_FMT
- "chain pool depth(%d), frame_size(%d), pool_size(%d kB)\n",
- ioc->name, ioc->chain_depth, ioc->chain_segment_sz,
- ((ioc->chain_depth * ioc->chain_segment_sz))/1024));
-
/* initialize hi-priority queue smid's */
ioc->hpr_lookup = kcalloc(ioc->hi_priority_depth,
sizeof(struct request_tracker), GFP_KERNEL);
@@ -4587,6 +4561,7 @@ _base_allocate_memory_pools(struct MPT3SAS_ADAPTER *ioc)
* be required for NVMe PRP's, only each set of NVMe blocks will be
* contiguous, so a new set is allocated for each possible I/O.
*/
+ ioc->chains_per_prp_buffer = 0;
if (ioc->facts.ProtocolFlags & MPI2_IOCFACTS_PROTOCOL_NVME_DEVICES) {
nvme_blocks_needed =
(ioc->shost->sg_tablesize * NVME_PRP_SIZE) - 1;
@@ -4609,6 +4584,11 @@ _base_allocate_memory_pools(struct MPT3SAS_ADAPTER *ioc)
ioc->name);
goto out;
}
+
+ ioc->chains_per_prp_buffer = sz/ioc->chain_segment_sz;
+ ioc->chains_per_prp_buffer = min(ioc->chains_per_prp_buffer,
+ ioc->chains_needed_per_io);
+
for (i = 0; i < ioc->scsiio_depth; i++) {
[PATCH v1 00/15] mpt3sas: Enhancements and Defect fixes.
Chaitra P B (15): mpt3sas: Bug fix for big endian systems. mpt3sas: Pre-allocate RDPQ Array at driver boot time. mpt3sas: Add sanity checks for scsi tracker before accessing it. 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" drivers/scsi/mpt3sas/mpi/mpi2.h | 9 +- drivers/scsi/mpt3sas/mpi/mpi2_cnfg.h | 30 +- drivers/scsi/mpt3sas/mpi/mpi2_init.h | 2 +- drivers/scsi/mpt3sas/mpi/mpi2_ioc.h | 7 +- drivers/scsi/mpt3sas/mpt3sas_base.c | 477 ++--- drivers/scsi/mpt3sas/mpt3sas_base.h | 62 +++- drivers/scsi/mpt3sas/mpt3sas_ctl.c | 38 ++- drivers/scsi/mpt3sas/mpt3sas_ctl.h | 2 +- drivers/scsi/mpt3sas/mpt3sas_scsih.c | 500 +-- drivers/scsi/mpt3sas/mpt3sas_warpdrive.c | 3 +- 10 files changed, 830 insertions(+), 300 deletions(-) -- 1.8.3.1
[PATCH v1 04/15] mpt3sas: Lockless access for chain buffers.
Introduces Chain lookup table/tracker and implements accessing chain buffer
using smid.
Removed link list based access of chain buffer which requires lock and
allocated as many chains needed.
Signed-off-by: Chaitra P B
Signed-off-by: Suganath Prabu S
---
drivers/scsi/mpt3sas/mpt3sas_base.c | 111 +++-
drivers/scsi/mpt3sas/mpt3sas_base.h | 8 ++-
2 files changed, 65 insertions(+), 54 deletions(-)
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c
b/drivers/scsi/mpt3sas/mpt3sas_base.c
index a79c6df..1e8e399 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -297,12 +297,15 @@ static void *
_base_get_chain_buffer_dma_to_chain_buffer(struct MPT3SAS_ADAPTER *ioc,
dma_addr_t chain_buffer_dma)
{
- u16 index;
-
- for (index = 0; index < ioc->chain_depth; index++) {
- if (ioc->chain_lookup[index].chain_buffer_dma ==
- chain_buffer_dma)
- return ioc->chain_lookup[index].chain_buffer;
+ u16 index, j;
+ struct chain_tracker *ct;
+
+ for (index = 0; index < ioc->scsiio_depth; index++) {
+ for (j = 0; j < ioc->chains_needed_per_io; j++) {
+ ct = &ioc->chain_lookup[index].chains_per_smid[j];
+ if (ct && ct->chain_buffer_dma == chain_buffer_dma)
+ return ct->chain_buffer;
+ }
}
pr_info(MPT3SAS_FMT
"Provided chain_buffer_dma address is not in the lookup list\n",
@@ -1678,7 +1681,8 @@ _base_add_sg_single_64(void *paddr, u32 flags_length,
dma_addr_t dma_addr)
* @ioc: per adapter object
* @scmd: SCSI commands of the IO request
*
- * Returns chain tracker(from ioc->free_chain_list)
+ * Returns chain tracker from chain_lookup table using key as
+ * smid and smid's chain_offset.
*/
static struct chain_tracker *
_base_get_chain_buffer_tracker(struct MPT3SAS_ADAPTER *ioc,
@@ -1686,20 +1690,15 @@ _base_get_chain_buffer_tracker(struct MPT3SAS_ADAPTER
*ioc,
{
struct chain_tracker *chain_req;
struct scsiio_tracker *st = scsi_cmd_priv(scmd);
- unsigned long flags;
+ u16 smid = st->smid;
+ u8 chain_offset =
+ atomic_read(&ioc->chain_lookup[smid - 1].chain_offset);
- spin_lock_irqsave(&ioc->scsi_lookup_lock, flags);
- if (list_empty(&ioc->free_chain_list)) {
- spin_unlock_irqrestore(&ioc->scsi_lookup_lock, flags);
- dfailprintk(ioc, pr_warn(MPT3SAS_FMT
- "chain buffers not available\n", ioc->name));
+ if (chain_offset == ioc->chains_needed_per_io)
return NULL;
- }
- chain_req = list_entry(ioc->free_chain_list.next,
- struct chain_tracker, tracker_list);
- list_del_init(&chain_req->tracker_list);
- list_add_tail(&chain_req->tracker_list, &st->chain_list);
- spin_unlock_irqrestore(&ioc->scsi_lookup_lock, flags);
+
+ chain_req = &ioc->chain_lookup[smid - 1].chains_per_smid[chain_offset];
+ atomic_inc(&ioc->chain_lookup[smid - 1].chain_offset);
return chain_req;
}
@@ -3278,13 +3277,7 @@ void mpt3sas_base_clear_st(struct MPT3SAS_ADAPTER *ioc,
return;
st->cb_idx = 0xFF;
st->direct_io = 0;
- if (!list_empty(&st->chain_list)) {
- unsigned long flags;
-
- spin_lock_irqsave(&ioc->scsi_lookup_lock, flags);
- list_splice_init(&st->chain_list, &ioc->free_chain_list);
- spin_unlock_irqrestore(&ioc->scsi_lookup_lock, flags);
- }
+ atomic_set(&ioc->chain_lookup[st->smid - 1].chain_offset, 0);
}
/**
@@ -4102,6 +4095,8 @@ static void
_base_release_memory_pools(struct MPT3SAS_ADAPTER *ioc)
{
int i = 0;
+ int j = 0;
+ struct chain_tracker *ct;
struct reply_post_struct *rps;
dexitprintk(ioc, pr_info(MPT3SAS_FMT "%s\n", ioc->name,
@@ -4192,14 +4187,18 @@ _base_release_memory_pools(struct MPT3SAS_ADAPTER *ioc)
kfree(ioc->hpr_lookup);
kfree(ioc->internal_lookup);
if (ioc->chain_lookup) {
- for (i = 0; i < ioc->chain_depth; i++) {
- if (ioc->chain_lookup[i].chain_buffer)
- dma_pool_free(ioc->chain_dma_pool,
- ioc->chain_lookup[i].chain_buffer,
- ioc->chain_lookup[i].chain_buffer_dma);
+ for (i = 0; i < ioc->scsiio_depth; i++) {
+ for (j = 0; j < ioc->chains_needed_per_io; j++) {
+ ct = &ioc->chain_lookup[i].chains_per_smid[j];
+ if (ct && ct->chain_buffer)
+ dma_pool_free(ioc->chain_dma_pool,
+ ct->chain_buffer,
+ ct->
[PATCH v1 02/15] mpt3sas: Pre-allocate RDPQ Array at driver boot time.
Instead of allocating RDPQ array (This stores the address's of each RDPQ
pools) at run time, now it will be allocated once during driver load time
and same will be reused during host reset operation also (instead of
allocating & freeing this buffer on the fly during every host reset
operation) and then freed during driver unload.
Signed-off-by: Chaitra P B
Signed-off-by: Suganath Prabu S
---
drivers/scsi/mpt3sas/mpt3sas_base.c | 57 +++--
drivers/scsi/mpt3sas/mpt3sas_base.h | 3 ++
2 files changed, 38 insertions(+), 22 deletions(-)
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c
b/drivers/scsi/mpt3sas/mpt3sas_base.c
index 4767690..a79c6df 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -4159,7 +4159,14 @@ _base_release_memory_pools(struct MPT3SAS_ADAPTER *ioc)
}
} while (ioc->rdpq_array_enable &&
(++i < ioc->reply_queue_count));
-
+ if (ioc->reply_post_free_array &&
+ ioc->rdpq_array_enable) {
+ dma_pool_free(ioc->reply_post_free_array_dma_pool,
+ ioc->reply_post_free_array,
+ ioc->reply_post_free_array_dma);
+ ioc->reply_post_free_array = NULL;
+ }
+ dma_pool_destroy(ioc->reply_post_free_array_dma_pool);
dma_pool_destroy(ioc->reply_post_free_dma_pool);
kfree(ioc->reply_post);
}
@@ -4209,7 +4216,7 @@ _base_allocate_memory_pools(struct MPT3SAS_ADAPTER *ioc)
struct mpt3sas_facts *facts;
u16 max_sge_elements;
u16 chains_needed_per_io;
- u32 sz, total_sz, reply_post_free_sz;
+ u32 sz, total_sz, reply_post_free_sz, reply_post_free_array_sz;
u32 retry_sz;
u16 max_request_credit, nvme_blocks_needed;
unsigned short sg_tablesize;
@@ -4681,6 +4688,28 @@ _base_allocate_memory_pools(struct MPT3SAS_ADAPTER *ioc)
ioc->name, (unsigned long long)ioc->reply_free_dma));
total_sz += sz;
+ if (ioc->rdpq_array_enable) {
+ reply_post_free_array_sz = ioc->reply_queue_count *
+ sizeof(Mpi2IOCInitRDPQArrayEntry);
+ ioc->reply_post_free_array_dma_pool =
+ dma_pool_create("reply_post_free_array pool",
+ &ioc->pdev->dev, reply_post_free_array_sz, 16, 0);
+ if (!ioc->reply_post_free_array_dma_pool) {
+ dinitprintk(ioc,
+ pr_info(MPT3SAS_FMT "reply_post_free_array pool: "
+ "dma_pool_create failed\n", ioc->name));
+ goto out;
+ }
+ ioc->reply_post_free_array =
+ dma_pool_alloc(ioc->reply_post_free_array_dma_pool,
+ GFP_KERNEL, &ioc->reply_post_free_array_dma);
+ if (!ioc->reply_post_free_array) {
+ dinitprintk(ioc,
+ pr_info(MPT3SAS_FMT "reply_post_free_array pool: "
+ "dma_pool_alloc failed\n", ioc->name));
+ goto out;
+ }
+ }
ioc->config_page_sz = 512;
ioc->config_page = pci_alloc_consistent(ioc->pdev,
ioc->config_page_sz, &ioc->config_page_dma);
@@ -5487,8 +5516,6 @@ _base_send_ioc_init(struct MPT3SAS_ADAPTER *ioc)
ktime_t current_time;
u16 ioc_status;
u32 reply_post_free_array_sz = 0;
- Mpi2IOCInitRDPQArrayEntry *reply_post_free_array = NULL;
- dma_addr_t reply_post_free_array_dma;
dinitprintk(ioc, pr_info(MPT3SAS_FMT "%s\n", ioc->name,
__func__));
@@ -5522,23 +5549,14 @@ _base_send_ioc_init(struct MPT3SAS_ADAPTER *ioc)
if (ioc->rdpq_array_enable) {
reply_post_free_array_sz = ioc->reply_queue_count *
sizeof(Mpi2IOCInitRDPQArrayEntry);
- reply_post_free_array = pci_alloc_consistent(ioc->pdev,
- reply_post_free_array_sz, &reply_post_free_array_dma);
- if (!reply_post_free_array) {
- pr_err(MPT3SAS_FMT
- "reply_post_free_array: pci_alloc_consistent failed\n",
- ioc->name);
- r = -ENOMEM;
- goto out;
- }
- memset(reply_post_free_array, 0, reply_post_free_array_sz);
+ memset(ioc->reply_post_free_array, 0, reply_post_free_array_sz);
for (i = 0; i < ioc->reply_queue_count; i++)
- reply_post_free_array[i].RDPQBaseAddress =
+ ioc->reply_post_free_array[i].RDPQBaseAddress =
cpu_to_le64(
(u64)ioc->reply_post[i].reply_post_free_dma);
mpi_request.MsgFlags = MPI2_
[PATCH v1 01/15] mpt3sas: Bug fix for big endian systems.
This patch fixes bug for big endian systems.
Signed-off-by: Chaitra P B
Signed-off-by: Suganath Prabu S
---
drivers/scsi/mpt3sas/mpi/mpi2_init.h | 2 +-
drivers/scsi/mpt3sas/mpt3sas_base.c | 57 +-
drivers/scsi/mpt3sas/mpt3sas_base.h | 6 ++--
drivers/scsi/mpt3sas/mpt3sas_ctl.c | 11 +++---
drivers/scsi/mpt3sas/mpt3sas_scsih.c | 59 +++-
drivers/scsi/mpt3sas/mpt3sas_warpdrive.c | 3 +-
6 files changed, 73 insertions(+), 65 deletions(-)
diff --git a/drivers/scsi/mpt3sas/mpi/mpi2_init.h
b/drivers/scsi/mpt3sas/mpi/mpi2_init.h
index 948a3ba..6213ce6 100644
--- a/drivers/scsi/mpt3sas/mpi/mpi2_init.h
+++ b/drivers/scsi/mpt3sas/mpi/mpi2_init.h
@@ -75,7 +75,7 @@
typedef struct _MPI2_SCSI_IO_CDB_EEDP32 {
U8 CDB[20]; /*0x00 */
- U32 PrimaryReferenceTag;/*0x14 */
+ __be32 PrimaryReferenceTag; /*0x14 */
U16 PrimaryApplicationTag; /*0x18 */
U16 PrimaryApplicationTagMask; /*0x1A */
U32 TransferLength; /*0x1C */
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c
b/drivers/scsi/mpt3sas/mpt3sas_base.c
index 0a0e7aa..4767690 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -394,13 +394,14 @@ static void _clone_sg_entries(struct MPT3SAS_ADAPTER *ioc,
buff_ptr_phys = buffer_iomem_phys;
WARN_ON(buff_ptr_phys > U32_MAX);
- if (sgel->FlagsLength &
+ if (le32_to_cpu(sgel->FlagsLength) &
(MPI2_SGE_FLAGS_HOST_TO_IOC << MPI2_SGE_FLAGS_SHIFT))
is_write = 1;
for (i = 0; i < MPT_MIN_PHYS_SEGMENTS + ioc->facts.MaxChainDepth; i++) {
- sgl_flags = (sgel->FlagsLength >> MPI2_SGE_FLAGS_SHIFT);
+ sgl_flags =
+ (le32_to_cpu(sgel->FlagsLength) >> MPI2_SGE_FLAGS_SHIFT);
switch (sgl_flags & MPI2_SGE_FLAGS_ELEMENT_MASK) {
case MPI2_SGE_FLAGS_CHAIN_ELEMENT:
@@ -411,7 +412,7 @@ static void _clone_sg_entries(struct MPT3SAS_ADAPTER *ioc,
*/
sgel_next =
_base_get_chain_buffer_dma_to_chain_buffer(ioc,
- sgel->Address);
+ le32_to_cpu(sgel->Address));
if (sgel_next == NULL)
return;
/*
@@ -426,7 +427,7 @@ static void _clone_sg_entries(struct MPT3SAS_ADAPTER *ioc,
dst_addr_phys = _base_get_chain_phys(ioc,
smid, sge_chain_count);
WARN_ON(dst_addr_phys > U32_MAX);
- sgel->Address = (u32)dst_addr_phys;
+ sgel->Address = cpu_to_le32((u32)dst_addr_phys);
sgel = sgel_next;
sge_chain_count++;
break;
@@ -435,22 +436,28 @@ static void _clone_sg_entries(struct MPT3SAS_ADAPTER *ioc,
if (is_scsiio_req) {
_base_clone_to_sys_mem(buff_ptr,
sg_virt(sg_scmd),
- (sgel->FlagsLength & 0x00ff));
+ (le32_to_cpu(sgel->FlagsLength) &
+ 0x00ff));
/*
* FIXME: this relies on a a zero
* PCI mem_offset.
*/
- sgel->Address = (u32)buff_ptr_phys;
+ sgel->Address =
+ cpu_to_le32((u32)buff_ptr_phys);
} else {
_base_clone_to_sys_mem(buff_ptr,
ioc->config_vaddr,
- (sgel->FlagsLength & 0x00ff));
- sgel->Address = (u32)buff_ptr_phys;
+ (le32_to_cpu(sgel->FlagsLength) &
+ 0x00ff));
+ sgel->Address =
+ cpu_to_le32((u32)buff_ptr_phys);
}
}
- buff_ptr += (sgel->FlagsLength & 0x00ff);
- buff_ptr_phys += (sgel->FlagsLength & 0x00ff);
- if ((sgel->FlagsLength &
+ buff_ptr += (le32_to_cpu(sgel->FlagsLength) &
+ 0x00ff);
+ buff_ptr_phys += (le32_to_cpu(sgel->FlagsLength) &
+
Re: usercopy whitelist woe in scsi_sense_cache
05.04.2018 16:32, Oleksandr Natalenko wrote: "-hda sda.img -hdb sda.img" "-hda sda.img -hdb sdb.img", of course, I don't pass the same disk twice ☺
Re: usercopy whitelist woe in scsi_sense_cache
Hi. 05.04.2018 16:21, Kees Cook wrote: I had a VM running over night with: [1] Running while :; do smartctl -a /dev/sda > /dev/null; done & [2]- Running while :; do ls --color=auto -lR / > /dev/null 2> /dev/null; done & [3]+ Running while :; do sleep $(( $RANDOM % 100 )); sync; echo 3 > /proc/sys/vm/drop_caches; done & and I haven't seen the issue. :( FWIW, I'm using the ahci qemu driver: -drive file=disk-image.raw,if=none,id=drive0,format=raw \ -device ahci,id=bus0 \ -device ide-drive,bus=bus0.0,drive=drive0 Does this match your qemu instance? Well, not really. I just pass 2 raw disks as "-hda sda.img -hdb sda.img" (it is a playground VM for me with RAID10, LVM and LUKS inside, but I hope this doesn't matter). Does passing "-hda" differ from your approach? Regards, Oleksandr
Re: usercopy whitelist woe in scsi_sense_cache
On Thu, Apr 5, 2018 at 2:56 AM, Oleksandr Natalenko wrote: > Hi. > > 04.04.2018 23:25, Kees Cook wrote: >> >> Thanks for the report! I hope someone more familiar with sg_io() can >> help explain the changing buffer offset... :P > > > Also, FYI, I kept the server running with smartctl periodically invoked, and > it was still triggering BUGs, however, I consider them to be more or less > harmless until the server got stuck with high I/O wait this morning after > next smartctl invocation. So, it isn't harmless, it seems… > > It could be unrelated, of course, since the journal didn't give me any hint > (or a stack trace) on what happened, thus I'll monitor how things behave > without smartctl too. I had a VM running over night with: [1] Running while :; do smartctl -a /dev/sda > /dev/null; done & [2]- Running while :; do ls --color=auto -lR / > /dev/null 2> /dev/null; done & [3]+ Running while :; do sleep $(( $RANDOM % 100 )); sync; echo 3 > /proc/sys/vm/drop_caches; done & and I haven't seen the issue. :( FWIW, I'm using the ahci qemu driver: -drive file=disk-image.raw,if=none,id=drive0,format=raw \ -device ahci,id=bus0 \ -device ide-drive,bus=bus0.0,drive=drive0 Does this match your qemu instance? -Kees -- Kees Cook Pixel Security
Re: *** SPAM *** Re: [RFC PATCH] mpt3sas: mpt3sas_scsih_enclosure_find_by_handle can be static
Hi,
Further to that, in the second last hunk there is a very clear
functionality change:
@@ -8756,12 +8859,12 @@ _scsih_mark_responding_expander(struct
MPT3SAS_ADAPTER *ioc,
continue;
sas_expander->responding = 1;
- if (!encl_pg0_rc)
+ if (enclosure_dev) {
sas_expander->enclosure_logical_id =
- le64_to_cpu(enclosure_pg0.EnclosureLogicalID);
-
- sas_expander->enclosure_handle =
- le16_to_cpu(expander_pg0->EnclosureHandle);
+ le64_to_cpu(enclosure_dev->pg
0.EnclosureLogicalID);
+ sas_expander->enclosure_handle =
+ le16_to_cpu(expander_pg0->EnclosureHandle);
+ }
if (sas_expander->handle == handle)
goto out;
Note that the assignment to sas_expander->enclosure_handle is now
dependent on enclosure_dev being non-NULL.
Busy applying the patch to 4.16 - and now I have no idea whether that
functionality change should be part of the change or not. Having worked
through the rest of the patch it seems good otherwise (Keeping in mind
that I'm not familiar with the code in question, nor do I normally work
on kernel code, and this is definitely the first time I took a peek
anywhere near the IO subsystem).
Kind Regards,
Jaco
On 28/03/2018 23:54, Martin K. Petersen wrote:
> Bart,
>
>> Are you aware that if the 0-day test infrastructure suggests an improvement
>> for a patch that the patch that that improvement applies to gets ignored
>> unless either the patch is reposted with the improvement applied or that it
>> is explained why the suggested improvement is inappropriate?
> Correct. I don't apply anything that causes a 0-day warning. The patch
> will be closed with "Changes Required" status in patchwork.
>
> Always build patch submissions to linux-scsi with:
>
> make C=1 CF="-D__CHECK_ENDIAN__"
>
Re: [PATCH 1/1] target:separate tx/rx cmd_puds
Hi, The commit summary has a typo (cmd_puds). That said, this change isn't iSCSI specific, so using "pdu" here doesn't make much sense IMO. On Wed, 21 Mar 2018 17:52:43 +0800, Zhang Zhuoyu wrote: > Separate tx/rx cmd_pdus in order to distinguish LUN read/write IOPS. > > Signed-off-by: Zhang Zhuoyu > --- > drivers/target/target_core_stat.c | 26 ++ > drivers/target/target_core_transport.c | 11 ++- > include/target/target_core_base.h | 3 ++- > 3 files changed, 34 insertions(+), 6 deletions(-) > > diff --git a/drivers/target/target_core_stat.c > b/drivers/target/target_core_stat.c > index f0db91e..9099494 100644 > --- a/drivers/target/target_core_stat.c > +++ b/drivers/target/target_core_stat.c > @@ -706,7 +722,8 @@ static ssize_t > target_stat_tgt_port_hs_in_cmds_show(struct config_item *item, > CONFIGFS_ATTR_RO(target_stat_tgt_port_, indx); > CONFIGFS_ATTR_RO(target_stat_tgt_port_, name); > CONFIGFS_ATTR_RO(target_stat_tgt_port_, port_index); > -CONFIGFS_ATTR_RO(target_stat_tgt_port_, in_cmds); > +CONFIGFS_ATTR_RO(target_stat_tgt_port_, tx_cmds); > +CONFIGFS_ATTR_RO(target_stat_tgt_port_, rx_cmds); I don't think the in_cmds metric should be deleted here. It could be calculated on the fly via tx_cmds + rx_cmds + nodata_cmds. Cheers, David
Re: usercopy whitelist woe in scsi_sense_cache
Hi. 04.04.2018 23:25, Kees Cook wrote: Thanks for the report! I hope someone more familiar with sg_io() can help explain the changing buffer offset... :P Also, FYI, I kept the server running with smartctl periodically invoked, and it was still triggering BUGs, however, I consider them to be more or less harmless until the server got stuck with high I/O wait this morning after next smartctl invocation. So, it isn't harmless, it seems… It could be unrelated, of course, since the journal didn't give me any hint (or a stack trace) on what happened, thus I'll monitor how things behave without smartctl too. Regards, Oleksandr
Re: [PATCH v2] Fix DID_OK handling in __scsi_error_from_host_byte()
On Thu, Apr 05, 2018 at 08:43:18AM +0200, Hannes Reinecke wrote: > And a further nit-pick: the function is called > __scsi_error_from_host_byte(), so it's only logical that it would only > check the host_byte(). > What's wrong is the _usage_ here; after calling > __scsi_error_from_host_byte() we need to check if the _other_ bits of > the results are non-zero to end up with a valid result. > > Hence I would advocate to either rename this function > (__scsi_error_from_result() ?) or evaluate the remaining bits outside > this function. > > But I would advocate for the former; otherwise the same issue will crop > up again in the future. Please also drop the pointless double underscore prefix while at it, otherwise fully agreed.
Re: [PATCH v2] Fix DID_OK handling in __scsi_error_from_host_byte()
On Wed, Apr 04, 2018 at 10:53:55AM -0700, Bart Van Assche wrote: > + /* > + * Also check the other bytes than the status byte in result > + * to handle the case when a SCSI LLD sets result to > + * DRIVER_SENSE << 24 without setting SAM_STAT_CHECK_CONDITION. > + */ > + return scsi_status_is_good(result) && (result & ~0xff) == 0 ? > + BLK_STS_OK : BLK_STS_IOERR; How about an readable version of the statement above? if (scsi_status_is_good(result) && (result & ~0xff)) return BLK_STS_OK; return BLK_STS_IOERR;
Re: [PATCH] target: change default dbroot to /etc/target
On Wed, Apr 04, 2018 at 12:47:03PM -0700, Lee Duncan wrote: > The dbroot (target PR database root directory) is > configurable but default to /var/target, a historic > value. But the reason for adding configurability > was to move the target directory out of /var. This > is because the File Hierarchy Standard v3.0 mandates > that this "target" directory not be in /var. See > https://refspecs.linuxfoundation.org/FHS_3.0/fhs-3.0.pdf > > This change moves the default from /var/target to > /etc/target, but this value is still configurable, > so those wishing to continue to use /var/target > can still do so. This will break upgrades of all existing setups, so NAK.
