[RFC PATCH mkp-scsi] scsi: libsas: sas_destruct_ports() can be static

[kbuild test robot]

Fixes: 99cfdc04c12e ("scsi: libsas: direct call probe and destruct")
Signed-off-by: Fengguang Wu 
---
 sas_discover.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/libsas/sas_discover.c 
b/drivers/scsi/libsas/sas_discover.c
index 190108e..e4fd078 100644
--- a/drivers/scsi/libsas/sas_discover.c
+++ b/drivers/scsi/libsas/sas_discover.c
@@ -361,7 +361,7 @@ void sas_destruct_devices(struct asd_sas_port *port)
}
 }
 
-void sas_destruct_ports(struct asd_sas_port *port)
+static void sas_destruct_ports(struct asd_sas_port *port)
 {
struct sas_port *sas_port, *p;
 

[mkp-scsi:for-next 237/263] drivers/scsi/libsas/sas_discover.c:364:6: sparse: symbol 'sas_destruct_ports' was not declared. Should it be static?

[kbuild test robot]

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next
head:   f7c3d0667404295422c9a5d71fd87ca20c770f49
commit: 99cfdc04c12ea1bc5719bff15f24dae8b5b90da9 [237/263] scsi: libsas: direct 
call probe and destruct
reproduce:
# apt-get install sparse
git checkout 99cfdc04c12ea1bc5719bff15f24dae8b5b90da9
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

>> drivers/scsi/libsas/sas_discover.c:364:6: sparse: symbol 
>> 'sas_destruct_ports' was not declared. Should it be

Please review and possibly fold the followup patch.

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation

Re: [PATCHv2] libsas: Check for completed commands before calling lldd_abort_task()

[Hannes Reinecke]

On 01/09/2018 05:09 AM, Jason Yan wrote:
> Hannes,
> 
> On 2018/1/8 20:04, Hannes Reinecke wrote:
>> The abort handler might be racing with command completion, so the
>> task might already be NULL by the time the abort handler is called.
>>
>> Signed-off-by: Hannes Reinecke 
>> ---
>>   drivers/scsi/libsas/sas_scsi_host.c | 22 +++---
>>   1 file changed, 19 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/scsi/libsas/sas_scsi_host.c
>> b/drivers/scsi/libsas/sas_scsi_host.c
>> index 58476b7..08203fb 100644
>> --- a/drivers/scsi/libsas/sas_scsi_host.c
>> +++ b/drivers/scsi/libsas/sas_scsi_host.c
>> @@ -486,18 +486,34 @@ static int sas_queue_reset(struct domain_device
>> *dev, int reset_type,
>>
>>   int sas_eh_abort_handler(struct scsi_cmnd *cmd)
>>   {
>> -    int res;
>> -    struct sas_task *task = TO_SAS_TASK(cmd);
>> +    int res = TMF_RESP_FUNC_COMPLETE;
>> +    struct sas_task *task;
>>   struct Scsi_Host *host = cmd->device->host;
>>   struct sas_internal *i = to_sas_internal(host->transportt);
>> +    struct domain_device *dev = cmd_to_domain_dev(cmd);
>> +    struct sas_ha_struct *ha = dev->port->ha;
>> +    unsigned long flags;
>>
>>   if (!i->dft->lldd_abort_task)
>>   return FAILED;
>>
>> -    res = i->dft->lldd_abort_task(task);
>> +    /* Avoid sas_scsi_task_done() interfering */
>> +    spin_lock_irqsave(>done_lock, flags);
>> +    task = TO_SAS_TASK(cmd);
>> +    if (test_bit(SAS_HA_FROZEN, >state)) {
>> +    res = TMF_RESP_FUNC_FAILED;
>> +    task = NULL;
>> +    } else
>> +    ASSIGN_SAS_TASK(cmd, NULL);
>> +    spin_unlock_irqrestore(>done_lock, flags);
>> +    if (task)
>> +    res = i->dft->lldd_abort_task(task);
>>   if (res == TMF_RESP_FUNC_SUCC || res == TMF_RESP_FUNC_COMPLETE)
>>   return SUCCESS;
>>
>> +    spin_lock_irqsave(>done_lock, flags);
>> +    ASSIGN_SAS_TASK(cmd, task);
>> +    spin_unlock_irqrestore(>done_lock, flags);
> 
> Why do you assign task back? As I remember, when this cmd dispatch
> again, we will create a new task and assign to it again. So should we
> end this task here?
> 
We only will create a new task if we decide to retry the command.
But if we return FAILED here the command is not retried but rather the
SCSI EH is invoked.

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.com  +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

Re: [PATCH 4/4] IB/srp: Fix a sleep-in-invalid-context bug

[Hannes Reinecke]

On 01/08/2018 07:50 PM, Bart Van Assche wrote:
> The previous two patches guarantee that srp_queuecommand() does not get
> invoked while reconnecting occurs. Hence remove the code from
> srp_queuecommand() that prevents command queueing while reconnecting.
> This patch avoids that the following can appear in the kernel log:
> 
> BUG: sleeping function called from invalid context at 
> kernel/locking/mutex.c:747
> in_atomic(): 1, irqs_disabled(): 0, pid: 5600, name: scsi_eh_9
> 1 lock held by scsi_eh_9/5600:
>  #0:  (rcu_read_lock){}, at: [] 
> __blk_mq_run_hw_queue+0xf1/0x1e0
> Preemption disabled at:
> [<139badf2>] __blk_mq_delay_run_hw_queue+0x78/0xf0
> CPU: 9 PID: 5600 Comm: scsi_eh_9 Tainted: GW4.15.0-rc4-dbg+ #1
> Hardware name: Dell Inc. PowerEdge R720/0VWT90, BIOS 2.5.4 01/22/2016
> Call Trace:
>  dump_stack+0x67/0x99
>  ___might_sleep+0x16a/0x250 [ib_srp]
>  __mutex_lock+0x46/0x9d0
>  srp_queuecommand+0x356/0x420 [ib_srp]
>  scsi_dispatch_cmd+0xf6/0x3f0
>  scsi_queue_rq+0x4a8/0x5f0
>  blk_mq_dispatch_rq_list+0x73/0x440
>  blk_mq_sched_dispatch_requests+0x109/0x1a0
>  __blk_mq_run_hw_queue+0x131/0x1e0
>  __blk_mq_delay_run_hw_queue+0x9a/0xf0
>  blk_mq_run_hw_queue+0xc0/0x1e0
>  blk_mq_start_hw_queues+0x2c/0x40
>  scsi_run_queue+0x18e/0x2d0
>  scsi_run_host_queues+0x22/0x40
>  scsi_error_handler+0x18d/0x5f0
>  kthread+0x11c/0x140
>  ret_from_fork+0x24/0x30
> 
> Signed-off-by: Bart Van Assche 
> Cc: Jason Gunthorpe 
> Cc: Doug Ledford 
> ---
>  drivers/infiniband/ulp/srp/ib_srp.c | 21 ++---
>  1 file changed, 2 insertions(+), 19 deletions(-)
> 
Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

Re: [PATCH 3/4] scsi: Avoid that .queuecommand() gets called for a quiesced SCSI device

[Hannes Reinecke]

On 01/08/2018 07:50 PM, Bart Van Assche wrote:
> Several SCSI transport and LLD drivers surround code that does not
> tolerate concurrent calls of .queuecommand() with scsi_target_block() /
> scsi_target_unblock(). These last two functions use
> blk_mq_quiesce_queue() / blk_mq_unquiesce_queue() for scsi-mq request
> queues to prevent concurrent .queuecommand() calls. However, that is
> not sufficient to prevent .queuecommand() calls from scsi_send_eh_cmnd().
> Hence surround the .queuecommand() call from the SCSI error handler with
> blk_wait_if_quiesced() / blk_finish_wait_if_quiesced().
> 
> Note: converting the .queuecommand() call in scsi_send_eh_cmnd() into
> code that calls blk_get_request(), e.g. scsi_execute_req(), is not an
> option since scsi_send_eh_cmnd() can be called if all requests are
> allocated and if no requests will make progress without aborting any
> of these requests.
> 
> Signed-off-by: Bart Van Assche 
> Cc: Martin K. Petersen 
> Cc: Christoph Hellwig 
> Cc: Hannes Reinecke 
> Cc: Johannes Thumshirn 
> Cc: Ming Lei 
> ---
>  drivers/scsi/scsi_error.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
> index 62b56de38ae8..5af3de1731a5 100644
> --- a/drivers/scsi/scsi_error.c
> +++ b/drivers/scsi/scsi_error.c
> @@ -1016,6 +1016,7 @@ static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, 
> unsigned char *cmnd,
>  {
>   struct scsi_device *sdev = scmd->device;
>   struct Scsi_Host *shost = sdev->host;
> + struct request_queue *q = sdev->request_queue;
>   DECLARE_COMPLETION_ONSTACK(done);
>   unsigned long timeleft = timeout;
>   struct scsi_eh_save ses;
> @@ -1028,7 +1029,9 @@ static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, 
> unsigned char *cmnd,
>  
>   scsi_log_send(scmd);
>   scmd->scsi_done = scsi_eh_done;
> + blk_wait_if_quiesced(q);
>   rtn = shost->hostt->queuecommand(shost, scmd);
> + blk_finish_wait_if_quiesced(q);
>   if (rtn) {
>   if (timeleft > stall_for) {
>   scsi_eh_restore_cmnd(scmd, );
> 
Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

Re: [PATCH 2/4] block: Introduce blk_wait_if_quiesced() and blk_finish_wait_if_quiesced()

[Hannes Reinecke]

On 01/08/2018 07:50 PM, Bart Van Assche wrote:
> Introduce functions that allow block drivers to wait while a request
> queue is in the quiesced state (blk-mq) or in the stopped state (legacy
> block layer). The next patch will add calls to these functions in the
> SCSI core.
> 
> Signed-off-by: Bart Van Assche 
> Cc: Martin K. Petersen 
> Cc: Christoph Hellwig 
> Cc: Hannes Reinecke 
> Cc: Johannes Thumshirn 
> Cc: Ming Lei 
> ---
>  block/blk-core.c   |  1 +
>  block/blk-mq.c | 61 
> ++
>  include/linux/blk-mq.h |  2 ++
>  3 files changed, 64 insertions(+)
> 
> diff --git a/block/blk-core.c b/block/blk-core.c
> index 605599a2ab3b..d70ff53e6505 100644
> --- a/block/blk-core.c
> +++ b/block/blk-core.c
> @@ -285,6 +285,7 @@ void blk_start_queue(struct request_queue *q)
>   WARN_ON_ONCE(q->mq_ops);
>  
>   queue_flag_clear(QUEUE_FLAG_STOPPED, q);
> + wake_up_all(>mq_wq);
>   __blk_run_queue(q);
>  }
>  EXPORT_SYMBOL(blk_start_queue);
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index 8118890fb66f..c79b102680fe 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -248,11 +248,72 @@ void blk_mq_unquiesce_queue(struct request_queue *q)
>   queue_flag_clear(QUEUE_FLAG_QUIESCED, q);
>   spin_unlock_irqrestore(q->queue_lock, flags);
>  
> + wake_up_all(>mq_wq);
> +
>   /* dispatch requests which are inserted during quiescing */
>   blk_mq_run_hw_queues(q, true);
>  }
>  EXPORT_SYMBOL_GPL(blk_mq_unquiesce_queue);
>  
> +/**
> + * blk_wait_if_quiesced() - wait if a queue is quiesced (blk-mq) or stopped 
> (legacy block layer)
> + * @q: Request queue pointer.
> + *
> + * Some block drivers, e.g. the SCSI core, can bypass the block layer core
> + * request submission mechanism. Surround such code with 
> blk_wait_if_quiesced()
> + * / blk_finish_wait_if_quiesced() to avoid that request submission can 
> happen
> + * while a queue is quiesced or stopped.
> + *
> + * Returns with the RCU read lock held (blk-mq) or with q->queue_lock held
> + * (legacy block layer).
> + *
> + * Note: this function does not support block drivers whose .queue_rq()
> + * implementation can sleep (BLK_MQ_F_BLOCKING).
> + */
> +int blk_wait_if_quiesced(struct request_queue *q)
> +{
> + struct blk_mq_hw_ctx *hctx;
> + unsigned int i;
> +
> + might_sleep();
> +
> + if (q->mq_ops) {
> + queue_for_each_hw_ctx(q, hctx, i)
> + WARN_ON(hctx->flags & BLK_MQ_F_BLOCKING);
> +
> + rcu_read_lock();
> + while (!blk_queue_dying(q) && blk_queue_quiesced(q)) {
> + rcu_read_unlock();
> + wait_event(q->mq_wq, blk_queue_dying(q) ||
> +!blk_queue_quiesced(q));
> + rcu_read_lock();
> + }
> + } else {
> + spin_lock_irq(q->queue_lock);
> + wait_event_lock_irq(q->mq_wq,
> + blk_queue_dying(q) || !blk_queue_stopped(q),
> + *q->queue_lock);
> + q->request_fn_active++;
> + }
> + return blk_queue_dying(q) ? -ENODEV : 0;
> +}
> +EXPORT_SYMBOL(blk_wait_if_quiesced);
> +
> +/**
> + * blk_finish_wait_if_quiesced() - counterpart of blk_wait_if_quiesced()
> + * @q: Request queue pointer.
> + */
> +void blk_finish_wait_if_quiesced(struct request_queue *q)
> +{
> + if (q->mq_ops) {
> + rcu_read_unlock();
> + } else {
> + q->request_fn_active--;
> + spin_unlock_irq(q->queue_lock);
> + }
> +}
> +EXPORT_SYMBOL(blk_finish_wait_if_quiesced);
> +
>  void blk_mq_wake_waiters(struct request_queue *q)
>  {
>   struct blk_mq_hw_ctx *hctx;
> diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
> index 95c9a5c862e2..f6b787bd244e 100644
> --- a/include/linux/blk-mq.h
> +++ b/include/linux/blk-mq.h
> @@ -266,6 +266,8 @@ void blk_mq_start_stopped_hw_queue(struct blk_mq_hw_ctx 
> *hctx, bool async);
>  void blk_mq_start_stopped_hw_queues(struct request_queue *q, bool async);
>  void blk_mq_quiesce_queue(struct request_queue *q);
>  void blk_mq_unquiesce_queue(struct request_queue *q);
> +int blk_wait_if_quiesced(struct request_queue *q);
> +void blk_finish_wait_if_quiesced(struct request_queue *q);
>  void blk_mq_delay_run_hw_queue(struct blk_mq_hw_ctx *hctx, unsigned long 
> msecs);
>  bool blk_mq_run_hw_queue(struct blk_mq_hw_ctx *hctx, bool async);
>  void blk_mq_run_hw_queues(struct request_queue *q, bool async);
> 
I'm always a bit cautious when having rcu_read_lock() and
rcu_read_unlock() in two separate functions.
Can we make this dependency more explicit by renaming the first function
to blk_start_wait_if_quiesced() and updating the comment to the effect
that both functions must be used in tandem?

Cheers,

Hannes
-- 
Dr. 

Re: [PATCH 1/4] blk-mq: Rename request_queue.mq_freeze_wq into mq_wq

[Hannes Reinecke]

On 01/08/2018 07:50 PM, Bart Van Assche wrote:
> Rename a waitqueue in struct request_queue since the next patch will
> add code that uses this waitqueue outside the request queue freezing
> implementation.
> 
> Signed-off-by: Bart Van Assche 
> Cc: Christoph Hellwig 
> Cc: Hannes Reinecke 
> Cc: Johannes Thumshirn 
> Cc: Ming Lei 
> ---
>  block/blk-core.c   | 10 +-
>  block/blk-mq.c | 10 +-
>  include/linux/blkdev.h |  2 +-
>  3 files changed, 11 insertions(+), 11 deletions(-)
> 
Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

Re: [PATCHv2] libsas: Check for completed commands before calling lldd_abort_task()

[Jason Yan]

Hannes,

On 2018/1/8 20:04, Hannes Reinecke wrote:

The abort handler might be racing with command completion, so the
task might already be NULL by the time the abort handler is called.

Signed-off-by: Hannes Reinecke 
---
  drivers/scsi/libsas/sas_scsi_host.c | 22 +++---
  1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/libsas/sas_scsi_host.c 
b/drivers/scsi/libsas/sas_scsi_host.c
index 58476b7..08203fb 100644
--- a/drivers/scsi/libsas/sas_scsi_host.c
+++ b/drivers/scsi/libsas/sas_scsi_host.c
@@ -486,18 +486,34 @@ static int sas_queue_reset(struct domain_device *dev, int 
reset_type,

  int sas_eh_abort_handler(struct scsi_cmnd *cmd)
  {
-   int res;
-   struct sas_task *task = TO_SAS_TASK(cmd);
+   int res = TMF_RESP_FUNC_COMPLETE;
+   struct sas_task *task;
struct Scsi_Host *host = cmd->device->host;
struct sas_internal *i = to_sas_internal(host->transportt);
+   struct domain_device *dev = cmd_to_domain_dev(cmd);
+   struct sas_ha_struct *ha = dev->port->ha;
+   unsigned long flags;

if (!i->dft->lldd_abort_task)
return FAILED;

-   res = i->dft->lldd_abort_task(task);
+   /* Avoid sas_scsi_task_done() interfering */
+   spin_lock_irqsave(>done_lock, flags);
+   task = TO_SAS_TASK(cmd);
+   if (test_bit(SAS_HA_FROZEN, >state)) {
+   res = TMF_RESP_FUNC_FAILED;
+   task = NULL;
+   } else
+   ASSIGN_SAS_TASK(cmd, NULL);
+   spin_unlock_irqrestore(>done_lock, flags);
+   if (task)
+   res = i->dft->lldd_abort_task(task);
if (res == TMF_RESP_FUNC_SUCC || res == TMF_RESP_FUNC_COMPLETE)
return SUCCESS;

+   spin_lock_irqsave(>done_lock, flags);
+   ASSIGN_SAS_TASK(cmd, task);
+   spin_unlock_irqrestore(>done_lock, flags);


Why do you assign task back? As I remember, when this cmd dispatch
again, we will create a new task and assign to it again. So should we
end this task here?



return FAILED;
  }
  EXPORT_SYMBOL_GPL(sas_eh_abort_handler);

Re: [PATCH 0/2] Change frame type for SET MAX commands

[Martin K. Petersen]

chenxiang,

> According to ATA protocol, there are two other values for SET MAX commands' 
> feature field. So definite them. Also those SET MAX commands belong to 
> different frame types,and judge feature field of SET MAX commands to decide
> which frame type they belongs to.

Applied to 4.16/scsi-queue.

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH 1/2] ata: enhance the definition of SET MAX feature field value

[Tejun Heo]

On Mon, Jan 08, 2018 at 10:16:55PM -0500, Martin K. Petersen wrote:
> 
> Tejun,
> 
> Are you OK with me pulling this change through the SCSI tree?

Sure, yeah, please go ahead.

Thanks.

-- 
tejun

Re: [PATCH 7/9] scsi: bnx2fc: Use zeroing allocator rather than allocator/memset

[Martin K. Petersen]

Himanshu,

> scsi: qedi: Use zeroing allocator instead of allocator/memset --Resend
> scsi: mvsas: Use zeroing allocator rather than allocator/memset
> scsi: fnic: Use zeroing allocator rather than allocator/memset
> scsi: dpt_i2o: Use zeroing allocator rather than allocator/memset
> scsi: bnx2fc: Use zeroing allocator rather than allocator/memset --Resend
>
> But do I also need to send those mentioned above, which didn't receive
> any response from you ?

I typically only merge cleanup patches if the driver maintainer acks
them. So you should poke the relevant maintainers.

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH] scsi: smartpqi: allow static build ("built-in")

[Martin K. Petersen]

Steffen,

> If CONFIG_SCSI_SMARTPQI=y then don't build this driver as a module.

Applied to 4.16/scsi-queue. Thanks!

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH v3 0/5] scsi_debug: add write scattered support

[Martin K. Petersen]

Douglas,

> While testing the WRITE SCATTERED command support in a new sg3_utils
> utility (sg_write_x) it was helpful to have a target that supported
> this command. This command might be attractive to other kernel
> subsystems. Even if end devices don't support this command yet, it
> would most likely be a performance win if SCSI LLDs supported it (by
> breaking it down to a series of WRITE commands), as that would cut
> down the overhead of the block layer, the ULD (e.g. sd) and the SCSI
> midlevel.

Applied to 4.16/scsi-queue, thanks!

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH 1/2] ata: enhance the definition of SET MAX feature field value

[Martin K. Petersen]

Tejun,

Are you OK with me pulling this change through the SCSI tree?

> There are two other values for SET MAX feature field according to ata
> protocol. So definite them.
>
> Signed-off-by: Xiang Chen 
> Signed-off-by: John Garry 
> ---
>  include/linux/ata.h | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/include/linux/ata.h b/include/linux/ata.h
> index c7a3538..40d150a 100644
> --- a/include/linux/ata.h
> +++ b/include/linux/ata.h
> @@ -448,6 +448,8 @@ enum {
>   ATA_SET_MAX_LOCK= 0x02,
>   ATA_SET_MAX_UNLOCK  = 0x03,
>   ATA_SET_MAX_FREEZE_LOCK = 0x04,
> + ATA_SET_MAX_PASSWD_DMA  = 0x05,
> + ATA_SET_MAX_UNLOCK_DMA  = 0x06,
>  
>   /* feature values for DEVICE CONFIGURATION OVERLAY */
>   ATA_DCO_RESTORE = 0xC0,

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH V9 0/7] blk-mq support for ZBC disks

[Martin K. Petersen]

Jens,

> Completely up to you - I already have 1-5, I can add 6/7 as well, or
> just can do it in your tree. Let me know what you prefer.

Started my 4.16/scsi-fixes branch early based on your tree.

I queued these two up.

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH v2 3/3] scsi-mq-debugfs: Show more information

[Martin K. Petersen]

Bart,

> Show the request result, request timeout and SCSI command flags.
> This information is very helpful when trying to figure out why a
> queue got stuck. An example of the information that is exported
> through debugfs:

Applied to 4.16/scsi-fixes, thanks.

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [Resend PATCH 00/11] mpt3sas: Enable scsi MQ & lockless command submission

[Martin K. Petersen]

Suganath,

> This patch set is initially posted by Hannes Reinecke to enable scsi
> multiqueue for the mpt3sas driver.  While the HBA only has a single
> mailbox register for submitting commands, it does have individual
> receive queues per MSI-X interrupt and as such does benefit from
> converting it to full multiqueue support.

Applied to 4.16/scsi-queue. Thank you!

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH 0/6] cxlflash: Miscellaneous patches

[Martin K. Petersen]

Uma,

> This patch series contains miscellaneous fixes. The first patch fixes
> a bug while the rest improve the code structure and prepare the code
> for future enhancements.

Added stable tag to first patch and applied series to
4.16/scsi-queue. Thank you!

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH] scsi: mptfusion: use strlcpy() instead of strncpy()

[Martin K. Petersen]

Xiongfeng,

> gcc-8 reports
>
> drivers/message/fusion/mptbase.c: In function 'mpt_display_event_info':
> ./include/linux/string.h:245:9: warning: '__builtin_strncpy' specified
> bound 100 equals destination size [-Wstringop-truncation]
>
> We need to use strlcpy() to make sure the dest string is
> nul-terminated.

Applied to 4.16/scsi-queue. Thanks!

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH][scsi-next] scsi: aacraid: remove redundant setting of variable c

[Martin K. Petersen]

Colin,

> A previous commit no longer stores the contents of c, so we now have a
> situation where c is being updated but the value is never read. Clean
> up the code by removing the now redundant setting of variable c.

Applied to 4.16/scsi-queue. Thanks!

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH] scsi: libcxgbi: use GFP_ATOMIC in cxgbi_conn_alloc_pdu()

[Martin K. Petersen]

Varun,

> For mgmt cmds ->alloc_pdu() can be called from atomic
> context so use GFP_ATOMIC instead of GFP_KERNEL.

Applied to 4.16/scsi-queue, thank you!

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH v3 1/3] scsi: libsas: fix memory leak in sas_smp_get_phy_events()

[Martin K. Petersen]

Jason,

> We've got a memory leak with the following producer:

Applied patches 1-3 to 4.16/scsi-queue. Thanks!

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH 2/2] Fix a race condition between SPI domain validation and system suspend

[Rafael J. Wysocki]

On Monday, January 8, 2018 5:02:53 PM CET Martin K. Petersen wrote:
> 
> Bart,
> 
> > Avoid that the following warning is reported when suspending a system
> > that is using the mptspi driver:
> 
> Acked-by: Martin K. Petersen 

Thanks!

Let me take both patches to the linux-pm tree then.

Thanks,
Rafael

[PATCH] scsi: fnic: use kzalloc in fnic_fcoe_process_vlan_resp

[Rasmus Villemoes]

This saves a little .text and gets rid of the unmotivated line break and
the sizeof(...) style inconsistency.

Signed-off-by: Rasmus Villemoes 
---
 drivers/scsi/fnic/fnic_fcs.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/scsi/fnic/fnic_fcs.c b/drivers/scsi/fnic/fnic_fcs.c
index 999fc7547560..c7bf316d8e83 100644
--- a/drivers/scsi/fnic/fnic_fcs.c
+++ b/drivers/scsi/fnic/fnic_fcs.c
@@ -442,15 +442,13 @@ static void fnic_fcoe_process_vlan_resp(struct fnic 
*fnic, struct sk_buff *skb)
vid = ntohs(((struct fip_vlan_desc *)desc)->fd_vlan);
shost_printk(KERN_INFO, fnic->lport->host,
  "process_vlan_resp: FIP VLAN %d\n", vid);
-   vlan = kmalloc(sizeof(*vlan),
-   GFP_ATOMIC);
+   vlan = kzalloc(sizeof(*vlan), GFP_ATOMIC);
if (!vlan) {
/* retry from timer */
spin_unlock_irqrestore(>vlans_lock,
flags);
goto out;
}
-   memset(vlan, 0, sizeof(struct fcoe_vlan));
vlan->vid = vid & 0x0fff;
vlan->state = FIP_VLAN_AVAIL;
list_add_tail(>list, >vlans);
-- 
2.15.1

[PATCH 2/4] block: Introduce blk_wait_if_quiesced() and blk_finish_wait_if_quiesced()

[Bart Van Assche]

Introduce functions that allow block drivers to wait while a request
queue is in the quiesced state (blk-mq) or in the stopped state (legacy
block layer). The next patch will add calls to these functions in the
SCSI core.

Signed-off-by: Bart Van Assche 
Cc: Martin K. Petersen 
Cc: Christoph Hellwig 
Cc: Hannes Reinecke 
Cc: Johannes Thumshirn 
Cc: Ming Lei 
---
 block/blk-core.c   |  1 +
 block/blk-mq.c | 61 ++
 include/linux/blk-mq.h |  2 ++
 3 files changed, 64 insertions(+)

diff --git a/block/blk-core.c b/block/blk-core.c
index 605599a2ab3b..d70ff53e6505 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -285,6 +285,7 @@ void blk_start_queue(struct request_queue *q)
WARN_ON_ONCE(q->mq_ops);
 
queue_flag_clear(QUEUE_FLAG_STOPPED, q);
+   wake_up_all(>mq_wq);
__blk_run_queue(q);
 }
 EXPORT_SYMBOL(blk_start_queue);
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 8118890fb66f..c79b102680fe 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -248,11 +248,72 @@ void blk_mq_unquiesce_queue(struct request_queue *q)
queue_flag_clear(QUEUE_FLAG_QUIESCED, q);
spin_unlock_irqrestore(q->queue_lock, flags);
 
+   wake_up_all(>mq_wq);
+
/* dispatch requests which are inserted during quiescing */
blk_mq_run_hw_queues(q, true);
 }
 EXPORT_SYMBOL_GPL(blk_mq_unquiesce_queue);
 
+/**
+ * blk_wait_if_quiesced() - wait if a queue is quiesced (blk-mq) or stopped 
(legacy block layer)
+ * @q: Request queue pointer.
+ *
+ * Some block drivers, e.g. the SCSI core, can bypass the block layer core
+ * request submission mechanism. Surround such code with blk_wait_if_quiesced()
+ * / blk_finish_wait_if_quiesced() to avoid that request submission can happen
+ * while a queue is quiesced or stopped.
+ *
+ * Returns with the RCU read lock held (blk-mq) or with q->queue_lock held
+ * (legacy block layer).
+ *
+ * Note: this function does not support block drivers whose .queue_rq()
+ * implementation can sleep (BLK_MQ_F_BLOCKING).
+ */
+int blk_wait_if_quiesced(struct request_queue *q)
+{
+   struct blk_mq_hw_ctx *hctx;
+   unsigned int i;
+
+   might_sleep();
+
+   if (q->mq_ops) {
+   queue_for_each_hw_ctx(q, hctx, i)
+   WARN_ON(hctx->flags & BLK_MQ_F_BLOCKING);
+
+   rcu_read_lock();
+   while (!blk_queue_dying(q) && blk_queue_quiesced(q)) {
+   rcu_read_unlock();
+   wait_event(q->mq_wq, blk_queue_dying(q) ||
+  !blk_queue_quiesced(q));
+   rcu_read_lock();
+   }
+   } else {
+   spin_lock_irq(q->queue_lock);
+   wait_event_lock_irq(q->mq_wq,
+   blk_queue_dying(q) || !blk_queue_stopped(q),
+   *q->queue_lock);
+   q->request_fn_active++;
+   }
+   return blk_queue_dying(q) ? -ENODEV : 0;
+}
+EXPORT_SYMBOL(blk_wait_if_quiesced);
+
+/**
+ * blk_finish_wait_if_quiesced() - counterpart of blk_wait_if_quiesced()
+ * @q: Request queue pointer.
+ */
+void blk_finish_wait_if_quiesced(struct request_queue *q)
+{
+   if (q->mq_ops) {
+   rcu_read_unlock();
+   } else {
+   q->request_fn_active--;
+   spin_unlock_irq(q->queue_lock);
+   }
+}
+EXPORT_SYMBOL(blk_finish_wait_if_quiesced);
+
 void blk_mq_wake_waiters(struct request_queue *q)
 {
struct blk_mq_hw_ctx *hctx;
diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index 95c9a5c862e2..f6b787bd244e 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -266,6 +266,8 @@ void blk_mq_start_stopped_hw_queue(struct blk_mq_hw_ctx 
*hctx, bool async);
 void blk_mq_start_stopped_hw_queues(struct request_queue *q, bool async);
 void blk_mq_quiesce_queue(struct request_queue *q);
 void blk_mq_unquiesce_queue(struct request_queue *q);
+int blk_wait_if_quiesced(struct request_queue *q);
+void blk_finish_wait_if_quiesced(struct request_queue *q);
 void blk_mq_delay_run_hw_queue(struct blk_mq_hw_ctx *hctx, unsigned long 
msecs);
 bool blk_mq_run_hw_queue(struct blk_mq_hw_ctx *hctx, bool async);
 void blk_mq_run_hw_queues(struct request_queue *q, bool async);
-- 
2.15.1

[PATCH 4/4] IB/srp: Fix a sleep-in-invalid-context bug

[Bart Van Assche]

The previous two patches guarantee that srp_queuecommand() does not get
invoked while reconnecting occurs. Hence remove the code from
srp_queuecommand() that prevents command queueing while reconnecting.
This patch avoids that the following can appear in the kernel log:

BUG: sleeping function called from invalid context at kernel/locking/mutex.c:747
in_atomic(): 1, irqs_disabled(): 0, pid: 5600, name: scsi_eh_9
1 lock held by scsi_eh_9/5600:
 #0:  (rcu_read_lock){}, at: [] 
__blk_mq_run_hw_queue+0xf1/0x1e0
Preemption disabled at:
[<139badf2>] __blk_mq_delay_run_hw_queue+0x78/0xf0
CPU: 9 PID: 5600 Comm: scsi_eh_9 Tainted: GW4.15.0-rc4-dbg+ #1
Hardware name: Dell Inc. PowerEdge R720/0VWT90, BIOS 2.5.4 01/22/2016
Call Trace:
 dump_stack+0x67/0x99
 ___might_sleep+0x16a/0x250 [ib_srp]
 __mutex_lock+0x46/0x9d0
 srp_queuecommand+0x356/0x420 [ib_srp]
 scsi_dispatch_cmd+0xf6/0x3f0
 scsi_queue_rq+0x4a8/0x5f0
 blk_mq_dispatch_rq_list+0x73/0x440
 blk_mq_sched_dispatch_requests+0x109/0x1a0
 __blk_mq_run_hw_queue+0x131/0x1e0
 __blk_mq_delay_run_hw_queue+0x9a/0xf0
 blk_mq_run_hw_queue+0xc0/0x1e0
 blk_mq_start_hw_queues+0x2c/0x40
 scsi_run_queue+0x18e/0x2d0
 scsi_run_host_queues+0x22/0x40
 scsi_error_handler+0x18d/0x5f0
 kthread+0x11c/0x140
 ret_from_fork+0x24/0x30

Signed-off-by: Bart Van Assche 
Cc: Jason Gunthorpe 
Cc: Doug Ledford 
---
 drivers/infiniband/ulp/srp/ib_srp.c | 21 ++---
 1 file changed, 2 insertions(+), 19 deletions(-)

diff --git a/drivers/infiniband/ulp/srp/ib_srp.c 
b/drivers/infiniband/ulp/srp/ib_srp.c
index 972d4b3c5223..670f187ecb91 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -2149,7 +2149,6 @@ static void srp_handle_qp_err(struct ib_cq *cq, struct 
ib_wc *wc,
 static int srp_queuecommand(struct Scsi_Host *shost, struct scsi_cmnd *scmnd)
 {
struct srp_target_port *target = host_to_target(shost);
-   struct srp_rport *rport = target->rport;
struct srp_rdma_ch *ch;
struct srp_request *req;
struct srp_iu *iu;
@@ -2159,16 +2158,6 @@ static int srp_queuecommand(struct Scsi_Host *shost, 
struct scsi_cmnd *scmnd)
u32 tag;
u16 idx;
int len, ret;
-   const bool in_scsi_eh = !in_interrupt() && current == shost->ehandler;
-
-   /*
-* The SCSI EH thread is the only context from which srp_queuecommand()
-* can get invoked for blocked devices (SDEV_BLOCK /
-* SDEV_CREATED_BLOCK). Avoid racing with srp_reconnect_rport() by
-* locking the rport mutex if invoked from inside the SCSI EH.
-*/
-   if (in_scsi_eh)
-   mutex_lock(>mutex);
 
scmnd->result = srp_chkready(target->rport);
if (unlikely(scmnd->result))
@@ -2230,13 +2219,7 @@ static int srp_queuecommand(struct Scsi_Host *shost, 
struct scsi_cmnd *scmnd)
goto err_unmap;
}
 
-   ret = 0;
-
-unlock_rport:
-   if (in_scsi_eh)
-   mutex_unlock(>mutex);
-
-   return ret;
+   return 0;
 
 err_unmap:
srp_unmap_data(scmnd, ch, req);
@@ -2258,7 +2241,7 @@ static int srp_queuecommand(struct Scsi_Host *shost, 
struct scsi_cmnd *scmnd)
ret = SCSI_MLQUEUE_HOST_BUSY;
}
 
-   goto unlock_rport;
+   return ret;
 }
 
 /*
-- 
2.15.1

[PATCH 3/4] scsi: Avoid that .queuecommand() gets called for a quiesced SCSI device

[Bart Van Assche]

Several SCSI transport and LLD drivers surround code that does not
tolerate concurrent calls of .queuecommand() with scsi_target_block() /
scsi_target_unblock(). These last two functions use
blk_mq_quiesce_queue() / blk_mq_unquiesce_queue() for scsi-mq request
queues to prevent concurrent .queuecommand() calls. However, that is
not sufficient to prevent .queuecommand() calls from scsi_send_eh_cmnd().
Hence surround the .queuecommand() call from the SCSI error handler with
blk_wait_if_quiesced() / blk_finish_wait_if_quiesced().

Note: converting the .queuecommand() call in scsi_send_eh_cmnd() into
code that calls blk_get_request(), e.g. scsi_execute_req(), is not an
option since scsi_send_eh_cmnd() can be called if all requests are
allocated and if no requests will make progress without aborting any
of these requests.

Signed-off-by: Bart Van Assche 
Cc: Martin K. Petersen 
Cc: Christoph Hellwig 
Cc: Hannes Reinecke 
Cc: Johannes Thumshirn 
Cc: Ming Lei 
---
 drivers/scsi/scsi_error.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index 62b56de38ae8..5af3de1731a5 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -1016,6 +1016,7 @@ static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, 
unsigned char *cmnd,
 {
struct scsi_device *sdev = scmd->device;
struct Scsi_Host *shost = sdev->host;
+   struct request_queue *q = sdev->request_queue;
DECLARE_COMPLETION_ONSTACK(done);
unsigned long timeleft = timeout;
struct scsi_eh_save ses;
@@ -1028,7 +1029,9 @@ static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, 
unsigned char *cmnd,
 
scsi_log_send(scmd);
scmd->scsi_done = scsi_eh_done;
+   blk_wait_if_quiesced(q);
rtn = shost->hostt->queuecommand(shost, scmd);
+   blk_finish_wait_if_quiesced(q);
if (rtn) {
if (timeleft > stall_for) {
scsi_eh_restore_cmnd(scmd, );
-- 
2.15.1

[PATCH 0/4] Make SCSI transport recovery more robust

[Bart Van Assche]

Hello Jens,

A longstanding issue with the SCSI core is that several SCSI transport drivers
use scsi_target_block() and scsi_target_unblock() to avoid concurrent
.queuecommand() calls during e.g. transport recovery but that this is not
sufficient to protect from such calls. Hence this patch series. Please
consider this patch series for kernel v4.16.

Thanks,

Bart.

Bart Van Assche (4):
  blk-mq: Rename request_queue.mq_freeze_wq into mq_wq
  block: Introduce blk_wait_if_quiesced() and
blk_finish_wait_if_quiesced()
  scsi: Avoid that .queuecommand() gets called for a quiesced SCSI
device
  IB/srp: Fix a sleep-in-invalid-context bug

 block/blk-core.c| 11 +++---
 block/blk-mq.c  | 71 ++---
 drivers/infiniband/ulp/srp/ib_srp.c | 21 ++-
 drivers/scsi/scsi_error.c   |  3 ++
 include/linux/blk-mq.h  |  2 ++
 include/linux/blkdev.h  |  2 +-
 6 files changed, 80 insertions(+), 30 deletions(-)

-- 
2.15.1

[PATCH 1/4] blk-mq: Rename request_queue.mq_freeze_wq into mq_wq

[Bart Van Assche]

Rename a waitqueue in struct request_queue since the next patch will
add code that uses this waitqueue outside the request queue freezing
implementation.

Signed-off-by: Bart Van Assche 
Cc: Christoph Hellwig 
Cc: Hannes Reinecke 
Cc: Johannes Thumshirn 
Cc: Ming Lei 
---
 block/blk-core.c   | 10 +-
 block/blk-mq.c | 10 +-
 include/linux/blkdev.h |  2 +-
 3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index 8f27ab14abce..605599a2ab3b 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -375,7 +375,7 @@ void blk_clear_preempt_only(struct request_queue *q)
 
spin_lock_irqsave(q->queue_lock, flags);
queue_flag_clear(QUEUE_FLAG_PREEMPT_ONLY, q);
-   wake_up_all(>mq_freeze_wq);
+   wake_up_all(>mq_wq);
spin_unlock_irqrestore(q->queue_lock, flags);
 }
 EXPORT_SYMBOL_GPL(blk_clear_preempt_only);
@@ -651,7 +651,7 @@ void blk_set_queue_dying(struct request_queue *q)
}
 
/* Make blk_queue_enter() reexamine the DYING flag. */
-   wake_up_all(>mq_freeze_wq);
+   wake_up_all(>mq_wq);
 }
 EXPORT_SYMBOL_GPL(blk_set_queue_dying);
 
@@ -854,7 +854,7 @@ int blk_queue_enter(struct request_queue *q, 
blk_mq_req_flags_t flags)
 */
smp_rmb();
 
-   ret = wait_event_interruptible(q->mq_freeze_wq,
+   ret = wait_event_interruptible(q->mq_wq,
(atomic_read(>mq_freeze_depth) == 0 &&
 (preempt || !blk_queue_preempt_only(q))) ||
blk_queue_dying(q));
@@ -875,7 +875,7 @@ static void blk_queue_usage_counter_release(struct 
percpu_ref *ref)
struct request_queue *q =
container_of(ref, struct request_queue, q_usage_counter);
 
-   wake_up_all(>mq_freeze_wq);
+   wake_up_all(>mq_wq);
 }
 
 static void blk_rq_timed_out_timer(struct timer_list *t)
@@ -951,7 +951,7 @@ struct request_queue *blk_alloc_queue_node(gfp_t gfp_mask, 
int node_id)
q->bypass_depth = 1;
__set_bit(QUEUE_FLAG_BYPASS, >queue_flags);
 
-   init_waitqueue_head(>mq_freeze_wq);
+   init_waitqueue_head(>mq_wq);
 
/*
 * Init percpu_ref in atomic mode so that it's faster to shutdown.
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 21c21ca13072..8118890fb66f 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -138,16 +138,16 @@ EXPORT_SYMBOL_GPL(blk_freeze_queue_start);
 
 void blk_mq_freeze_queue_wait(struct request_queue *q)
 {
-   wait_event(q->mq_freeze_wq, percpu_ref_is_zero(>q_usage_counter));
+   wait_event(q->mq_wq, percpu_ref_is_zero(>q_usage_counter));
 }
 EXPORT_SYMBOL_GPL(blk_mq_freeze_queue_wait);
 
 int blk_mq_freeze_queue_wait_timeout(struct request_queue *q,
 unsigned long timeout)
 {
-   return wait_event_timeout(q->mq_freeze_wq,
-   percpu_ref_is_zero(>q_usage_counter),
-   timeout);
+   return wait_event_timeout(q->mq_wq,
+ percpu_ref_is_zero(>q_usage_counter),
+ timeout);
 }
 EXPORT_SYMBOL_GPL(blk_mq_freeze_queue_wait_timeout);
 
@@ -186,7 +186,7 @@ void blk_mq_unfreeze_queue(struct request_queue *q)
WARN_ON_ONCE(freeze_depth < 0);
if (!freeze_depth) {
percpu_ref_reinit(>q_usage_counter);
-   wake_up_all(>mq_freeze_wq);
+   wake_up_all(>mq_wq);
}
 }
 EXPORT_SYMBOL_GPL(blk_mq_unfreeze_queue);
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 742c0d9aed6b..26c4d4343edb 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -613,7 +613,7 @@ struct request_queue {
struct throtl_data *td;
 #endif
struct rcu_head rcu_head;
-   wait_queue_head_t   mq_freeze_wq;
+   wait_queue_head_t   mq_wq;
struct percpu_ref   q_usage_counter;
struct list_headall_q_node;
 
-- 
2.15.1

Re: [PATCH 00/18] prevent bounds-check bypass via speculative execution

[Ingo Molnar]

* Alan Cox  wrote:

> On Mon, 8 Jan 2018 11:08:36 +0100
> Peter Zijlstra  wrote:
> 
> > On Fri, Jan 05, 2018 at 10:30:16PM -0800, Dan Williams wrote:
> > > On Fri, Jan 5, 2018 at 6:22 PM, Eric W. Biederman  
> > > wrote:  
> > > > In at least one place (mpls) you are patching a fast path.  Compile out
> > > > or don't load mpls by all means.  But it is not acceptable to change the
> > > > fast path without even considering performance.  
> > > 
> > > Performance matters greatly, but I need help to identify a workload
> > > that is representative for this fast path to see what, if any, impact
> > > is incurred. Even better is a review that says "nope, 'index' is not
> > > subject to arbitrary userspace control at this point, drop the patch."  
> > 
> > I think we're focussing a little too much on pure userspace. That is, we
> > should be saying under the attackers control. Inbound network packets
> > could equally be under the attackers control.
> 
> Inbound network packets don't come with a facility to read back and do
> cache timimg. [...]

But the reply packets can be measured on the sending side, and the total delay 
timing would thus carry the timing information.

Yes, a lot of noise gets added that way if we think 'packet goes through the 
Internet' - but with gigabit local network access or even through localhost
access a lot of noise can be removed as well.

It's not as dangerous as a near instantaneous local attack, but 'needs a day of 
runtime to brute-force through localhost or 10GigE' is still worrying in many 
real-world security contexts.

So I concur with Peter that we should generally consider making all of our 
responses to external data (maybe with the exception of pigeon post messages) 
Spectre-safe.

Thanks,

Ingo

[Bug 151661] Adaptec 3405 3805 prints "AAC: Host adapter dead -1" every 10 seconds but works fine anyway

[bugzilla-daemon]

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

--- Comment #22 from Matthias (hm10...@gmail.com) ---
with my Adaptec 3405 and Kernel of Ubuntu 16.04.3 (Kernel 4.4.0-104-generic)
and also with self-compiled 4.13.0, the message does not appear anymore.

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

Re: [PATCHv2] libsas: Check for completed commands before calling lldd_abort_task()

[Yves-Alexis Perez]

On Mon, 2018-01-08 at 13:04 +0100, Hannes Reinecke wrote:
> The abort handler might be racing with command completion, so the
> task might already be NULL by the time the abort handler is called.

Hi,

I tried the patch on top of 4.15-rc7 (and without the revert of 90965761). I
don't have the NULL pointer deref anymore, but the box timeouts after SCSI
init. It's not completely dead, but I don't have any meaningful log from the
kernel.

Regards,
-- 
Yves-Alexis

signature.asc
Description: This is a digitally signed message part

Re: [PATCH 1/6] cxlflash: Reset command ioasc

[Uma Krishnan]

> On Jan 7, 2018, at 1:36 PM, Matthew R. Ochs  wrote:
> 
> On Thu, Jan 04, 2018 at 05:33:48PM +1100, Andrew Donnellan wrote:
>> On 04/01/18 09:54, Uma Krishnan wrote:
>>> In the event of a command failure, cxlflash returns the failure to the
>>> upper layers to process. After processing the error, when the command is
>>> queued again, the private command structure will not be zeroed and the
>>> ioasc could be stale. Per the SISLite specification, the AFU only sets the
>>> ioasc in the presence of a failure. Thus, even though the original command
>>> succeeds the second time, the command is considered a failure due to stale
>>> ioasc. This cycle repeats indefinitely and can cause a hang or IO failure.
>>> 
>>> To fix the issue, clear the ioasc before queuing any command.
>>> 
>>> Fixes: 479ad8e9d48c ("scsi: cxlflash: Remove zeroing of private command
>>> data")
>>> Signed-off-by: Uma Krishnan 
>> 
>> Should this go to stable?
> 
> Not a bad idea.
> 

I will forward the patch to stable once the patch is in next/master. 
Unless there is a need to send a V2. Thanks for pointing out Andrew !!

Re: [PATCH] scsi: mptfusion: use strlcpy() instead of strncpy()

[Arnd Bergmann]

On Mon, Jan 8, 2018 at 1:49 PM, Xiongfeng Wang
 wrote:
> From: Xiongfeng Wang 
>
> gcc-8 reports
>
> drivers/message/fusion/mptbase.c: In function 'mpt_display_event_info':
> ./include/linux/string.h:245:9: warning: '__builtin_strncpy' specified
> bound 100 equals destination size [-Wstringop-truncation]
>
> We need to use strlcpy() to make sure the dest string is
> nul-terminated.
>
> Signed-off-by: Xiongfeng Wang 

Looks correct to me,

Acked-by: Arnd Bergmann 

Re: [PATCH 00/18] prevent bounds-check bypass via speculative execution

[Bart Van Assche]

On 01/05/18 22:30, Dan Williams wrote:

On Fri, Jan 5, 2018 at 6:22 PM, Eric W. Biederman  wrote:

Please expand this.

It is not clear what the static analysis is looking for.  Have a clear
description of what is being fixed is crucial for allowing any of these
changes.

For the details given in the change description what I read is magic
changes because a magic process says this code is vulnerable.


Yes, that was my first reaction to the patches as well, I try below to
add some more background and guidance, but in the end these are static
analysis reports across a wide swath of sub-systems. It's going to
take some iteration with domain experts to improve the patch
descriptions, and that's the point of this series, to get the better
trained eyes from the actual sub-system owners to take a look at these
reports.


More information about what the static analysis is looking for would 
definitely be welcome.


Additionally, since the analysis tool is not publicly available, how are 
authors of new kernel code assumed to verify whether or not their code 
needs to use nospec_array_ptr()? How are reviewers of kernel code 
assumed to verify whether or not nospec_array_ptr() is missing where it 
should be used?


Since this patch series only modifies the upstream kernel, how will 
out-of-tree drivers be fixed, e.g. the nVidia driver and the Android 
drivers?


Thanks,

Bart.

Re: [PATCH V9 0/7] blk-mq support for ZBC disks

[Jens Axboe]

On 1/8/18 8:52 AM, Martin K. Petersen wrote:
> 
> Jens,
> 
>> This looks OK for me for 4.16. I can grab all of them, or I can leave
>> the last two for Martin to apply if he prefers that, though that will
>> add a block tree dependency for SCSI.
> 
> I already have a block dependency for 4.16. But it doesn't matter much.

Completely up to you - I already have 1-5, I can add 6/7 as well, or
just can do it in your tree. Let me know what you prefer.


-- 
Jens Axboe

Re: [PATCH 2/2] Fix a race condition between SPI domain validation and system suspend

[Martin K. Petersen]

Bart,

> Avoid that the following warning is reported when suspending a system
> that is using the mptspi driver:

Acked-by: Martin K. Petersen 

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH v1] scsi: hpsa: Use vsnprintf extension %phN

[Martin K. Petersen]

Andy,

> Using this extension reduces the object size.

Applied to 4.16/scsi-queue, thanks!

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH V9 0/7] blk-mq support for ZBC disks

[Martin K. Petersen]

Jens,

> This looks OK for me for 4.16. I can grab all of them, or I can leave
> the last two for Martin to apply if he prefers that, though that will
> add a block tree dependency for SCSI.

I already have a block dependency for 4.16. But it doesn't matter much.

-- 
Martin K. Petersen  Oracle Linux Engineering

[PATCH] scsi: mptfusion: use strlcpy() instead of strncpy()

[Xiongfeng Wang]

From: Xiongfeng Wang 

gcc-8 reports

drivers/message/fusion/mptbase.c: In function 'mpt_display_event_info':
./include/linux/string.h:245:9: warning: '__builtin_strncpy' specified
bound 100 equals destination size [-Wstringop-truncation]

We need to use strlcpy() to make sure the dest string is
nul-terminated.

Signed-off-by: Xiongfeng Wang 
---
 drivers/message/fusion/mptbase.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/message/fusion/mptbase.c b/drivers/message/fusion/mptbase.c
index 7a93400..3c47888 100644
--- a/drivers/message/fusion/mptbase.c
+++ b/drivers/message/fusion/mptbase.c
@@ -7697,7 +7697,7 @@ static void seq_mpt_print_ioc_summary(MPT_ADAPTER *ioc, 
struct seq_file *m, int
break;
}
if (ds)
-   strncpy(evStr, ds, EVENT_DESCR_STR_SZ);
+   strlcpy(evStr, ds, EVENT_DESCR_STR_SZ);
 
 
devtprintk(ioc, printk(MYIOC_s_DEBUG_FMT
-- 
1.8.3.1

[PATCHv2] libsas: Check for completed commands before calling lldd_abort_task()

[Hannes Reinecke]

The abort handler might be racing with command completion, so the
task might already be NULL by the time the abort handler is called.

Signed-off-by: Hannes Reinecke 
---
 drivers/scsi/libsas/sas_scsi_host.c | 22 +++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/libsas/sas_scsi_host.c 
b/drivers/scsi/libsas/sas_scsi_host.c
index 58476b7..08203fb 100644
--- a/drivers/scsi/libsas/sas_scsi_host.c
+++ b/drivers/scsi/libsas/sas_scsi_host.c
@@ -486,18 +486,34 @@ static int sas_queue_reset(struct domain_device *dev, int 
reset_type,
 
 int sas_eh_abort_handler(struct scsi_cmnd *cmd)
 {
-   int res;
-   struct sas_task *task = TO_SAS_TASK(cmd);
+   int res = TMF_RESP_FUNC_COMPLETE;
+   struct sas_task *task;
struct Scsi_Host *host = cmd->device->host;
struct sas_internal *i = to_sas_internal(host->transportt);
+   struct domain_device *dev = cmd_to_domain_dev(cmd);
+   struct sas_ha_struct *ha = dev->port->ha;
+   unsigned long flags;
 
if (!i->dft->lldd_abort_task)
return FAILED;
 
-   res = i->dft->lldd_abort_task(task);
+   /* Avoid sas_scsi_task_done() interfering */
+   spin_lock_irqsave(>done_lock, flags);
+   task = TO_SAS_TASK(cmd);
+   if (test_bit(SAS_HA_FROZEN, >state)) {
+   res = TMF_RESP_FUNC_FAILED;
+   task = NULL;
+   } else
+   ASSIGN_SAS_TASK(cmd, NULL);
+   spin_unlock_irqrestore(>done_lock, flags);
+   if (task)
+   res = i->dft->lldd_abort_task(task);
if (res == TMF_RESP_FUNC_SUCC || res == TMF_RESP_FUNC_COMPLETE)
return SUCCESS;
 
+   spin_lock_irqsave(>done_lock, flags);
+   ASSIGN_SAS_TASK(cmd, task);
+   spin_unlock_irqrestore(>done_lock, flags);
return FAILED;
 }
 EXPORT_SYMBOL_GPL(sas_eh_abort_handler);
-- 
1.8.5.6

Re: [PATCH 00/18] prevent bounds-check bypass via speculative execution

[Peter Zijlstra]

On Mon, Jan 08, 2018 at 11:43:42AM +, Alan Cox wrote:
> On Mon, 8 Jan 2018 11:08:36 +0100
> Peter Zijlstra  wrote:
> 
> > On Fri, Jan 05, 2018 at 10:30:16PM -0800, Dan Williams wrote:
> > > On Fri, Jan 5, 2018 at 6:22 PM, Eric W. Biederman  
> > > wrote:  
> > > > In at least one place (mpls) you are patching a fast path.  Compile out
> > > > or don't load mpls by all means.  But it is not acceptable to change the
> > > > fast path without even considering performance.  
> > > 
> > > Performance matters greatly, but I need help to identify a workload
> > > that is representative for this fast path to see what, if any, impact
> > > is incurred. Even better is a review that says "nope, 'index' is not
> > > subject to arbitrary userspace control at this point, drop the patch."  
> > 
> > I think we're focussing a little too much on pure userspace. That is, we
> > should be saying under the attackers control. Inbound network packets
> > could equally be under the attackers control.
> 
> Inbound network packets don't come with a facility to read back and do
> cache timimg. 

But could they not be used in conjunction with a local task to prime the
stuff?

Re: [PATCH 00/18] prevent bounds-check bypass via speculative execution

[Alan Cox]

On Mon, 8 Jan 2018 11:08:36 +0100
Peter Zijlstra  wrote:

> On Fri, Jan 05, 2018 at 10:30:16PM -0800, Dan Williams wrote:
> > On Fri, Jan 5, 2018 at 6:22 PM, Eric W. Biederman  
> > wrote:  
> > > In at least one place (mpls) you are patching a fast path.  Compile out
> > > or don't load mpls by all means.  But it is not acceptable to change the
> > > fast path without even considering performance.  
> > 
> > Performance matters greatly, but I need help to identify a workload
> > that is representative for this fast path to see what, if any, impact
> > is incurred. Even better is a review that says "nope, 'index' is not
> > subject to arbitrary userspace control at this point, drop the patch."  
> 
> I think we're focussing a little too much on pure userspace. That is, we
> should be saying under the attackers control. Inbound network packets
> could equally be under the attackers control.

Inbound network packets don't come with a facility to read back and do
cache timimg. For the more general case, timing attacks on network
activity are not exactly new, and you have to mitigate them in user space
because most of them are about how many instructions you execute on each
path. The ancient classic being telling if a user exists by seeing if the
password was actually checked.

Alan

Re: [PATCH] libsas: Check for completed commands before calling lldd_abort_task()

[Hannes Reinecke]

On 01/08/2018 12:23 PM, Christoph Hellwig wrote:
> On Mon, Jan 08, 2018 at 11:51:56AM +0100, Hannes Reinecke wrote:
>> The abort handler might be racing with command completion, so the
>> task might already be NULL by the time the abort handler is called.
> 
> But without taking dev_done_lock (or using cmpxchg) what prevents
> use from still hitting this, although in a more narrow window?
> 
Good point.
Will be updating the patch.

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

Re: [PATCH 0/4] mylex: Replace DAC960 block driver

[Christoph Hellwig]

Btw, did you manage to get any further with these new drivers?

Re: [PATCH 00/47] SCSI EH argument reshuffle part II

[Christoph Hellwig]

On Wed, Jun 28, 2017 at 10:32:26AM +0200, Hannes Reinecke wrote:
> Hi all,
> finally here's the patchset to revamp the SCSI EH callback arguments
> which I promised to do (some years ago ...).

What happened to this series?

Re: [PATCH] libsas: Check for completed commands before calling lldd_abort_task()

[Christoph Hellwig]

On Mon, Jan 08, 2018 at 11:51:56AM +0100, Hannes Reinecke wrote:
> The abort handler might be racing with command completion, so the
> task might already be NULL by the time the abort handler is called.

But without taking dev_done_lock (or using cmpxchg) what prevents
use from still hitting this, although in a more narrow window?

Re: Oops: NULL pointer dereference - RIP: isci_task_abort_task+0x30/0x3e0 [isci]

[Hannes Reinecke]

On 01/08/2018 11:11 AM, Christoph Hellwig wrote:
> Hannes said he was going to look into this, which makes sense
> given that he designed the async abort code.
> 
> On Fri, Jan 05, 2018 at 01:13:48PM +0100, Yves-Alexis Perez wrote:
>> Hi,
>>
>> since kernel 4.11 (sorry it took so long to report) I have a box failing to
>> boot with a NULL pointer dereference (the box is stuck there afterwards).
>>
>> The bug has also been reported to the Debian BTS 
>> (https://bugs.debian.org/cgi-
>> bin/bugreport.cgi?bug=882414) and a suggestion to revert 90965761 has been
>> made. I can confirm it fix the boot issue.
>>
>> I don't have the complete stack trace at hand but there's an example in the
>> Debian bug. The machine is a Dell Precision T5600 with the following SATA
>> controllers:
>>
>> 00:1f.2 SATA controller: Intel Corporation C600/X79 series chipset 6-Port 
>> SATA
>> AHCI Controller (rev 05)
>> 05:00.0 Serial Attached SCSI controller: Intel Corporation C602 chipset 
>> 4-Port 
>> SATA Storage Control Unit (rev 05)
>>
>> If you need more information or need me to test something, please ask.
>>
>> Regards,
>> -- 
>> Yves-Alexis
> 
> ---end quoted text---
> 
Looks like we're calling lldd_abort_task() with a NULL argument.
Will be sending a patch.

Cheers,

Hannes
-- 
Dr. Hannes ReineckezSeries & Storage
h...@suse.com  +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

[PATCH] libsas: Check for completed commands before calling lldd_abort_task()

[Hannes Reinecke]

The abort handler might be racing with command completion, so the
task might already be NULL by the time the abort handler is called.

Signed-off-by: Hannes Reinecke 
---
 drivers/scsi/libsas/sas_scsi_host.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/libsas/sas_scsi_host.c 
b/drivers/scsi/libsas/sas_scsi_host.c
index 58476b7..ae2ae3c 100644
--- a/drivers/scsi/libsas/sas_scsi_host.c
+++ b/drivers/scsi/libsas/sas_scsi_host.c
@@ -486,7 +486,7 @@ static int sas_queue_reset(struct domain_device *dev, int 
reset_type,
 
 int sas_eh_abort_handler(struct scsi_cmnd *cmd)
 {
-   int res;
+   int res = TMF_RESP_FUNC_COMPLETE;
struct sas_task *task = TO_SAS_TASK(cmd);
struct Scsi_Host *host = cmd->device->host;
struct sas_internal *i = to_sas_internal(host->transportt);
@@ -494,7 +494,8 @@ int sas_eh_abort_handler(struct scsi_cmnd *cmd)
if (!i->dft->lldd_abort_task)
return FAILED;
 
-   res = i->dft->lldd_abort_task(task);
+   if (task)
+   res = i->dft->lldd_abort_task(task);
if (res == TMF_RESP_FUNC_SUCC || res == TMF_RESP_FUNC_COMPLETE)
return SUCCESS;
 
-- 
1.8.5.6

Re: Oops: NULL pointer dereference - RIP: isci_task_abort_task+0x30/0x3e0 [isci]

[Christoph Hellwig]

Hannes said he was going to look into this, which makes sense
given that he designed the async abort code.

On Fri, Jan 05, 2018 at 01:13:48PM +0100, Yves-Alexis Perez wrote:
> Hi,
> 
> since kernel 4.11 (sorry it took so long to report) I have a box failing to
> boot with a NULL pointer dereference (the box is stuck there afterwards).
> 
> The bug has also been reported to the Debian BTS (https://bugs.debian.org/cgi-
> bin/bugreport.cgi?bug=882414) and a suggestion to revert 90965761 has been
> made. I can confirm it fix the boot issue.
> 
> I don't have the complete stack trace at hand but there's an example in the
> Debian bug. The machine is a Dell Precision T5600 with the following SATA
> controllers:
> 
> 00:1f.2 SATA controller: Intel Corporation C600/X79 series chipset 6-Port SATA
> AHCI Controller (rev 05)
> 05:00.0 Serial Attached SCSI controller: Intel Corporation C602 chipset 
> 4-Port 
> SATA Storage Control Unit (rev 05)
> 
> If you need more information or need me to test something, please ask.
> 
> Regards,
> -- 
> Yves-Alexis

---end quoted text---

Re: [PATCH 00/18] prevent bounds-check bypass via speculative execution

[Peter Zijlstra]

On Fri, Jan 05, 2018 at 10:30:16PM -0800, Dan Williams wrote:
> On Fri, Jan 5, 2018 at 6:22 PM, Eric W. Biederman  
> wrote:
> > In at least one place (mpls) you are patching a fast path.  Compile out
> > or don't load mpls by all means.  But it is not acceptable to change the
> > fast path without even considering performance.
> 
> Performance matters greatly, but I need help to identify a workload
> that is representative for this fast path to see what, if any, impact
> is incurred. Even better is a review that says "nope, 'index' is not
> subject to arbitrary userspace control at this point, drop the patch."

I think we're focussing a little too much on pure userspace. That is, we
should be saying under the attackers control. Inbound network packets
could equally be under the attackers control.

Sure, userspace is the most direct and highest bandwidth one, but I
think we should treat all (kernel) external values with the same
paranoia.

Re: [PATCH 13/14] megaraid_sas: NVME passthru command support

[Christoph Hellwig]

NAK.  Please implement the same ioctl interfaces as the nvme driver
instead of inventing your own incomaptible one.

Re: [PATCH v3 3/3] scsi: libsas: initialize sas_phy status according to response of DISCOVER

[Hannes Reinecke]

On 01/04/2018 02:04 PM, Jason Yan wrote:
> From: chenxiang 
> 
> The status of SAS PHY is in sas_phy->enabled. There is an issue that the
> status of a remote SAS PHY may be initialized incorrectly: if disable remote
> SAS PHY through sysfs interface (such as echo 0 > 
> /sys/class/sas_phy/phy-1:0:0/enable),
> then reboot the system, and we will find the status of remote SAS PHY which is
> disabled before is 1 (cat /sys/class/sas_phy/phy-1:0:0/enable). But actually
> the status of remote SAS PHY is disabled and the device attached is not found.
> 
> In SAS protocol, NEGOTIATED LOGICAL LINK RATE field of DISCOVER response is 
> 0x1
> when remote SAS PHY is disabled. So initialize sas_phy->enabled according to
> the value of NEGOTIATED LOGICAL LINK RATE field.
> 
> Signed-off-by: chenxiang 
> Reviewed-by: John Garry 
> Signed-off-by: Jason Yan 
> ---
>  drivers/scsi/libsas/sas_expander.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/scsi/libsas/sas_expander.c 
> b/drivers/scsi/libsas/sas_expander.c
> index 6eab487..c79cfd1 100644
> --- a/drivers/scsi/libsas/sas_expander.c
> +++ b/drivers/scsi/libsas/sas_expander.c
> @@ -293,6 +293,7 @@ static void sas_set_ex_phy(struct domain_device *dev, int 
> phy_id, void *rsp)
>   phy->phy->minimum_linkrate = dr->pmin_linkrate;
>   phy->phy->maximum_linkrate = dr->pmax_linkrate;
>   phy->phy->negotiated_linkrate = phy->linkrate;
> + phy->phy->enabled = (phy->linkrate != SAS_PHY_DISABLED);
>  
>   skip:
>   if (new_phy)
> 
Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckezSeries & Storage
h...@suse.com  +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

Re: [PATCH v3 2/3] scsi: libsas: fix error when getting phy events

[Hannes Reinecke]

On 01/04/2018 02:04 PM, Jason Yan wrote:
> The intend purpose here was to goto out if smp_execute_task() returned
> error. Obviously something got screwed up. We will never get these link
> error statistics below:
> 
> ~:/sys/class/sas_phy/phy-1:0:12 # cat invalid_dword_count
> 0
> ~:/sys/class/sas_phy/phy-1:0:12 # cat running_disparity_error_count
> 0
> ~:/sys/class/sas_phy/phy-1:0:12 # cat loss_of_dword_sync_count
> 0
> ~:/sys/class/sas_phy/phy-1:0:12 # cat phy_reset_problem_count
> 0
> 
> Obviously we should goto error handler if smp_execute_task() returns
> non-zero.
> 
> Fixes: 2908d778ab3e ("[SCSI] aic94xx: new driver")
> Signed-off-by: Jason Yan 
> CC: John Garry 
> CC: chenqilin 
> CC: chenxiang 
> Reviewed-by: Christoph Hellwig 
> ---
>  drivers/scsi/libsas/sas_expander.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/libsas/sas_expander.c 
> b/drivers/scsi/libsas/sas_expander.c
> index 4b0c67f..6eab487 100644
> --- a/drivers/scsi/libsas/sas_expander.c
> +++ b/drivers/scsi/libsas/sas_expander.c
> @@ -686,7 +686,7 @@ int sas_smp_get_phy_events(struct sas_phy *phy)
>   res = smp_execute_task(dev, req, RPEL_REQ_SIZE,
>   resp, RPEL_RESP_SIZE);
>  
> - if (!res)
> + if (res)
>   goto out;
>  
>   phy->invalid_dword_count = scsi_to_u32([12]);
> 
Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckezSeries & Storage
h...@suse.com  +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

Re: [PATCH v3 1/3] scsi: libsas: fix memory leak in sas_smp_get_phy_events()

[Hannes Reinecke]

On 01/04/2018 02:04 PM, Jason Yan wrote:
> We've got a memory leak with the following producer:
> 
> while true;
> do cat /sys/class/sas_phy/phy-1:0:12/invalid_dword_count >/dev/null;
> done
> 
> The buffer req is allocated and not freed after we return. Fix it.
> 
> Fixes: 2908d778ab3e ("[SCSI] aic94xx: new driver")
> Signed-off-by: Jason Yan 
> CC: John Garry 
> CC: chenqilin 
> CC: chenxiang 
> Reviewed-by: Christoph Hellwig 
> ---
>  drivers/scsi/libsas/sas_expander.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/scsi/libsas/sas_expander.c 
> b/drivers/scsi/libsas/sas_expander.c
> index 3183d63..4b0c67f 100644
> --- a/drivers/scsi/libsas/sas_expander.c
> +++ b/drivers/scsi/libsas/sas_expander.c
> @@ -695,6 +695,7 @@ int sas_smp_get_phy_events(struct sas_phy *phy)
>   phy->phy_reset_problem_count = scsi_to_u32([24]);
>  
>   out:
> + kfree(req);
>   kfree(resp);
>   return res;
>  
> 
Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckezSeries & Storage
h...@suse.com  +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

Re: [PATCH v2 4/4] scsi: pm8001: fix dma_unmap_sg() parameter

[chenxiang (M)]

+cc Jack Wang 

在 2018/1/4 10:36, chenxiang 写道:

For function dma_unmap_sg(), the  parameter should be number of
elements in the scatterlist prior to the mapping, not after the mapping.
Fix this usage.

Cc: Jack Wang 
Cc: lindar_...@usish.com
Fixes: dbf9bfe6("[SCSI]pm8001: add SAS/SATA/HBA driver")
Signed-off-by: Xiang Chen 
---
  drivers/scsi/pm8001/pm8001_sas.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/pm8001/pm8001_sas.c b/drivers/scsi/pm8001/pm8001_sas.c
index 947d601..576a0f0 100644
--- a/drivers/scsi/pm8001/pm8001_sas.c
+++ b/drivers/scsi/pm8001/pm8001_sas.c
@@ -466,7 +466,7 @@ static int pm8001_task_exec(struct sas_task *task,
dev_printk(KERN_ERR, pm8001_ha->dev, "pm8001 exec failed[%d]!\n", rc);
if (!sas_protocol_ata(t->task_proto))
if (n_elem)
-   dma_unmap_sg(pm8001_ha->dev, t->scatter, n_elem,
+   dma_unmap_sg(pm8001_ha->dev, t->scatter, t->num_scatter,
t->data_dir);
  out_done:
spin_unlock_irqrestore(_ha->lock, flags);

Re: [PATCH] scsi: remove extra white space at the end of the line

[Johannes Thumshirn]

"Martin K. Petersen"  writes:
> I am thoroughly annoyed by all the legacy whitespace problems. I've been
> working on two different sd patch series over the holidays and both
> caused me no end of grief due to legacy formatting issues.
>
> I have had an unbreak-sd patch sitting in my queue for several years but
> never pulled the trigger on it. For the usual reasons.

So we all share the same common pain points.

> I'm not particularly worried about bisection. But fixing whitespace does
> make it harder on the distro backporting front (Very pleased that you
> have now inadvertently volunteered to deal with all the issues that may
> arise at SUSE from such a subsystem-wide cleanup :).
>
> Anyway. I'm OK with fixing up the core pieces since they are the ones
> that annoy me the most. But I'm not sure we should enforce cleanups on
> drivers without an ack from the relevant maintainer. And for the
> unmaintained legacy baggage, I'm just not sure it's worth the hassle to
> clean things up. Fixing the crufty old things gives an illusion of the
> driver being actively worked on. I'd rather see dead code being left as
> such. Gives us a good indication of when it's safe to drop.

Well the drivers are more of a personal pain pain point but yes cleaning
up the core would be very much appreciated.

> One thing I specifically don't want is to open the flood gates for
> drive-by whitespace patches. I have no interest in wasting cycles on
> that. I generally only take arbitrary 3rd party cleanups if a driver is
> actively maintained and the maintainer specifically acks the change.

Agreed

> PS. I'll at least partially unbreak sd.c as part of the series I'll be
> posting shortly.

:-)

Byte,
Johannes

-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850