Re: [PATCH 1/2] virtio: Reduce BUG if total_sg > virtqueue size to WARN.

[Paolo Bonzini]

On 11/08/2017 19:23, Michael S. Tsirkin wrote:
> On Fri, Aug 11, 2017 at 04:09:26PM +0200, Paolo Bonzini wrote:
>> On 10/08/2017 23:41, Michael S. Tsirkin wrote:
> Then we probably should fail probe if vq size is too small.
 What does this mean?
>>>
>>> We must prevent driver from submitting s/g lists > vq size to device.
>>
>> What is the rationale for the limit?
> 
> So the host knows what it needs to support.
> 
>> both virtio-blk and virtio-scsi transmit their own value for the
>> maximum sg list size (max_seg in virtio-scsi, seg_max in virtio-blk).
> 
> No other device has it, and it seemed like a good idea to
> limit it generally at the time.
> 
> we can fix the spec to relax the requirement for blk and scsi -
> want to submit a proposal? Alternatively, add a generic field
> for that.

Yes, I can submit a proposal.  blk and scsi are the ones that are most
likely to have very long sg lists.

When I was designing scsi I just copied that field from blk. :)

Paolo

Re: [PATCH 1/2] virtio: Reduce BUG if total_sg > virtqueue size to WARN.

[Michael S. Tsirkin]

On Fri, Aug 11, 2017 at 04:09:26PM +0200, Paolo Bonzini wrote:
> On 10/08/2017 23:41, Michael S. Tsirkin wrote:
> >>> Then we probably should fail probe if vq size is too small.
> >> What does this mean?
> > 
> > We must prevent driver from submitting s/g lists > vq size to device.
> 
> What is the rationale for the limit?

So the host knows what it needs to support.

>  It makes no sense if indirect
> descriptors are available, especially because...
> 
> > Either tell linux to avoid s/g lists that are too long, or
> > simply fail request if this happens, or refuse to attach driver to device.
> > 
> > Later option would look something like this within probe:
> > 
> > for (i = VIRTIO_SCSI_VQ_BASE; i < num_vqs; i++)
> > if (vqs[i]->num < MAX_SG_USED_BY_LINUX)
> > goto err;
> > 
> > 
> > I don't know what's MAX_SG_USED_BY_LINUX though.
> > 
> 
> ... both virtio-blk and virtio-scsi transmit their own value for the
> maximum sg list size (max_seg in virtio-scsi, seg_max in virtio-blk).
> 
> Paolo

No other device has it, and it seemed like a good idea to
limit it generally at the time.

we can fix the spec to relax the requirement for blk and scsi -
want to submit a proposal? Alternatively, add a generic field
for that.

For a quick fix, make sure vq size is >= max sg.

-- 
MST

Assalamu`Alaikum.

[Mr Abdul Karim]

Greetings from Mr. Abdul Karim

 Assalamu`Alaikum.

My Name is Mr. Abdul Karim I am a banker by profession. I'm from
Ouagadougou, Burkina Faso, West Africa. My reason for contacting you
is to transfer an abandoned $15.5M to your account.

The owner of this fund died since 2004 with his Next Of Kin. I want to
present you to the bank as the Next of Kin/beneficiary of this fund.

Further details of the transaction shall be forwarded to you as soon
as I receive your return mail indicating your interest.

 1) YOUR FULL NAME...
 (2) YOUR AGE AND SEX
 (3) YOUR CONTACT ADDRESS..
 (4) YOUR PRIVATE PHONE N0..
 (5) FAX NUMBER..
 (6) YOUR COUNTRY OF ORIGIN..
 (7) YOUR OCCUPATION.

 Have a great day,
Mr. Abdul Karim.

RE: [PATCH RESEND 0/6] hpsa: support legacy boards

[Don Brace]

> -Original Message-
> From: Hannes Reinecke [mailto:h...@suse.de]
> Sent: Friday, August 11, 2017 1:43 AM
> To: Don Brace ; James Bottomley
> ; Christoph Hellwig
> 
> Cc: Martin K. Petersen ; Meelis Roos
> ; linux-scsi@vger.kernel.org
> Subject: Re: [PATCH RESEND 0/6] hpsa: support legacy boards
> 
> EXTERNAL EMAIL

...
> >
> > Ok, but to clarify...
> >  * Will the cciss driver be removed when these patches are applied?
> Otherwise
> > what will prevent the cciss driver from loading over these devices?
> > i.e. how often will users need to be reminded to add
> cciss.cciss_allow_hpsa flag?
> >
> >
> The idea is to remove the cciss driver completely once these changes are
> in. Preferably with the same patchset to avoid any timing issues.
> 
> 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)

OK, then I ACK the series once the reviews by others have been resolved.
Thank-you and  everyone else for your patches and attention to this issue.
And thanks to everyone for your input.

Acked-by: Don Brace 

Re: [RFC PATCH 1/6] bsg: fix kernel panic resulting from missing allocation of a reply-buffer

[Benjamin Block]

On Fri, Aug 11, 2017 at 05:35:53PM +0200, Christoph Hellwig wrote:
> On Fri, Aug 11, 2017 at 05:32:03PM +0200, Benjamin Block wrote:
> > So when the bsg interface is used with something different than the
> > bsg-lib request queue?
> 
> Yes.
> 
> > I haven't actually thought about that (presuming
> > the bsg-lib queue was the only one being used). Fair enough, I haven't
> > completely read that code now, but that seems bad then, to reassign a
> > space allocated in someone else's request queue. 
> > 
> > That still leaves open that we now over-allocate space in bsg-lib, or?
> 
> Which space do we over-allocate?

When the BSG interface is used with bsg-lib, and the user sends a
Bidirectional command - so when he gives an input- and output-buffer
(most users of our interface will likely do that, if they wanna get the
transport-level response data) - bsg will allocate two requests from the
queue. The first request's bio is used to map the input and the second
request's bio for the output (see bsg_map_hdr() in the if-statement with
(op == REQ_OP_SCSI_OUT && hdr->din_xfer_len)).

When we now allocate the full space of bsg_job, sense, dd_data for each
request, these will be wasted on the (linked) second request. They will
go unused all the time, as only the first request's bsg_job, sense and
dd_data is used by the LLDs and BSG itself.

Right now, because we don't allocate this on each request, those spaces
are only allocated for the first request in bsg-lib.

Maybe we can ignore this, if it gets to complicated, I don't wanne
prolong this unnecessary.

> 
> > My diff tells that this was the same patch as before.
> 
> Next try:

I will have a look when I am back in the office next week.


Beste Grüße / Best regards,
  - Benjamin Block
> 
> ---
> From f5b03b82df0569c035022c1c2535696186907f1a Mon Sep 17 00:00:00 2001
> From: Christoph Hellwig 
> Date: Fri, 11 Aug 2017 11:03:29 +0200
> Subject: bsg-lib: allocate sense data for each request
> 
> Since we split the scsi_request out of the request the driver is supposed
> to provide storage for the sense buffer.  The bsg-lib code failed to do so,
> though and will crash anytime it is used.
> 
> This patch moves bsg-lib to allocate and setup the bsg_job ahead of time,
> and allocate the sense data, which is used as reply buffer in bsg.
> 
> Reported-by: Steffen Maier 
> Signed-off-by: Benjamin Block 
> Fixes: 82ed4db499b8 ("block: split scsi_request out of struct request")
> Cc:  #4.11+
> ---
>  block/bsg-lib.c | 51 
> +++--
>  include/linux/blkdev.h  |  1 -
>  include/linux/bsg-lib.h |  2 ++
>  3 files changed, 30 insertions(+), 24 deletions(-)
> 
> diff --git a/block/bsg-lib.c b/block/bsg-lib.c
> index c4513b23f57a..c07333c3b785 100644
> --- a/block/bsg-lib.c
> +++ b/block/bsg-lib.c
> @@ -37,13 +37,11 @@ static void bsg_destroy_job(struct kref *kref)
>   struct bsg_job *job = container_of(kref, struct bsg_job, kref);
>   struct request *rq = job->req;
> 
> - blk_end_request_all(rq, BLK_STS_OK);
> -
>   put_device(job->dev);   /* release reference for the request */
> 
>   kfree(job->request_payload.sg_list);
>   kfree(job->reply_payload.sg_list);
> - kfree(job);
> + blk_end_request_all(rq, BLK_STS_OK);
>  }
> 
>  void bsg_job_put(struct bsg_job *job)
> @@ -100,7 +98,7 @@ EXPORT_SYMBOL_GPL(bsg_job_done);
>   */
>  static void bsg_softirq_done(struct request *rq)
>  {
> - struct bsg_job *job = rq->special;
> + struct bsg_job *job = blk_mq_rq_to_pdu(rq);
> 
>   bsg_job_put(job);
>  }
> @@ -129,24 +127,11 @@ static int bsg_map_buffer(struct bsg_buffer *buf, 
> struct request *req)
>  static int bsg_create_job(struct device *dev, struct request *req)
>  {
>   struct request *rsp = req->next_rq;
> - struct request_queue *q = req->q;
> + struct bsg_job *job = blk_mq_rq_to_pdu(req);
>   struct scsi_request *rq = scsi_req(req);
> - struct bsg_job *job;
>   int ret;
> 
> - BUG_ON(req->special);
> -
> - job = kzalloc(sizeof(struct bsg_job) + q->bsg_job_size, GFP_KERNEL);
> - if (!job)
> - return -ENOMEM;
> -
> - req->special = job;
> - job->req = req;
> - if (q->bsg_job_size)
> - job->dd_data = (void *)[1];
> - job->request = rq->cmd;
>   job->request_len = rq->cmd_len;
> - job->reply = rq->sense;
>   job->reply_len = SCSI_SENSE_BUFFERSIZE; /* Size of sense buffer
>* allocated */
>   if (req->bio) {
> @@ -187,7 +172,6 @@ static void bsg_request_fn(struct request_queue *q)
>  {
>   struct device *dev = q->queuedata;
>   struct request *req;
> - struct bsg_job *job;
>   int ret;
> 
>   if (!get_device(dev))
> @@ -207,8 +191,7 @@ static 

Re: [PATCH] scsi-mq: Always unprepare before requeuing a request

[Bart Van Assche]

On Fri, 2017-08-11 at 11:05 +1000, Michael Ellerman wrote:
> kworker/u193:0  D12736 6  2 0x0800
> Workqueue: events_unbound async_run_entry_fn
> Call Trace:
> [c003f7597410] [c0150d00] console_unlock+0x330/0x770 (unreliable)
> [c003f75975e0] [c001b3b8] __switch_to+0x2a8/0x460
> [c003f7597640] [c09abc60] __schedule+0x320/0xaa0
> [c003f7597710] [c09ac420] schedule+0x40/0xb0
> [c003f7597740] [c09b09d4] schedule_timeout+0x254/0x440
> [c003f7597820] [c09aca80] io_schedule_timeout+0x30/0x60
> [c003f7597850] [c09ad75c] 
> wait_for_common_io.constprop.2+0xbc/0x1e0
> [c003f75978d0] [c0509e6c] blk_execute_rq+0x4c/0x70
> [c003f7597920] [c0654abc] scsi_execute+0xfc/0x260
> [c003f7597990] [c0654f98] scsi_mode_sense+0x218/0x410
> [c003f7597aa0] [c068ee68] sd_revalidate_disk+0x908/0x1cd0
> [c003f7597be0] [c0690434] sd_probe_async+0xb4/0x220
> [c003f7597c60] [c0110a20] async_run_entry_fn+0x70/0x170
> [c003f7597ca0] [c0103904] process_one_work+0x2b4/0x560
> [c003f7597d30] [c0103c38] worker_thread+0x88/0x5a0
> [c003f7597dc0] [c010bfcc] kthread+0x15c/0x1a0
> [c003f7597e30] [c000ba1c] ret_from_kernel_thread+0x5c/0xc0

Hello Michael,

It is suspicious that entries like the above appear in the SysRq-w output.
Every time I saw this in the past it was caused by a block driver not having
called blk_end_request() or a SCSI LLD not having called .scsi_done().
Additionally, it is unlikely that the patch at the start of this thread
introduced this issue. Can you have another look at the ipr driver? If a
shell is available at the time this lockup occurs, it will probably be
helpful to have a look at the debugfs entries under /sys/kernel/debug/block/.

Thanks,

Bart.

Re: [RFC PATCH 1/6] bsg: fix kernel panic resulting from missing allocation of a reply-buffer

[Christoph Hellwig]

On Fri, Aug 11, 2017 at 05:32:03PM +0200, Benjamin Block wrote:
> So when the bsg interface is used with something different than the
> bsg-lib request queue?

Yes.

> I haven't actually thought about that (presuming
> the bsg-lib queue was the only one being used). Fair enough, I haven't
> completely read that code now, but that seems bad then, to reassign a
> space allocated in someone else's request queue. 
> 
> That still leaves open that we now over-allocate space in bsg-lib, or?

Which space do we over-allocate?

> My diff tells that this was the same patch as before.

Next try:

---
>From f5b03b82df0569c035022c1c2535696186907f1a Mon Sep 17 00:00:00 2001
From: Christoph Hellwig 
Date: Fri, 11 Aug 2017 11:03:29 +0200
Subject: bsg-lib: allocate sense data for each request

Since we split the scsi_request out of the request the driver is supposed
to provide storage for the sense buffer.  The bsg-lib code failed to do so,
though and will crash anytime it is used.

This patch moves bsg-lib to allocate and setup the bsg_job ahead of time,
and allocate the sense data, which is used as reply buffer in bsg.

Reported-by: Steffen Maier 
Signed-off-by: Benjamin Block 
Fixes: 82ed4db499b8 ("block: split scsi_request out of struct request")
Cc:  #4.11+
---
 block/bsg-lib.c | 51 +++--
 include/linux/blkdev.h  |  1 -
 include/linux/bsg-lib.h |  2 ++
 3 files changed, 30 insertions(+), 24 deletions(-)

diff --git a/block/bsg-lib.c b/block/bsg-lib.c
index c4513b23f57a..c07333c3b785 100644
--- a/block/bsg-lib.c
+++ b/block/bsg-lib.c
@@ -37,13 +37,11 @@ static void bsg_destroy_job(struct kref *kref)
struct bsg_job *job = container_of(kref, struct bsg_job, kref);
struct request *rq = job->req;
 
-   blk_end_request_all(rq, BLK_STS_OK);
-
put_device(job->dev);   /* release reference for the request */
 
kfree(job->request_payload.sg_list);
kfree(job->reply_payload.sg_list);
-   kfree(job);
+   blk_end_request_all(rq, BLK_STS_OK);
 }
 
 void bsg_job_put(struct bsg_job *job)
@@ -100,7 +98,7 @@ EXPORT_SYMBOL_GPL(bsg_job_done);
  */
 static void bsg_softirq_done(struct request *rq)
 {
-   struct bsg_job *job = rq->special;
+   struct bsg_job *job = blk_mq_rq_to_pdu(rq);
 
bsg_job_put(job);
 }
@@ -129,24 +127,11 @@ static int bsg_map_buffer(struct bsg_buffer *buf, struct 
request *req)
 static int bsg_create_job(struct device *dev, struct request *req)
 {
struct request *rsp = req->next_rq;
-   struct request_queue *q = req->q;
+   struct bsg_job *job = blk_mq_rq_to_pdu(req);
struct scsi_request *rq = scsi_req(req);
-   struct bsg_job *job;
int ret;
 
-   BUG_ON(req->special);
-
-   job = kzalloc(sizeof(struct bsg_job) + q->bsg_job_size, GFP_KERNEL);
-   if (!job)
-   return -ENOMEM;
-
-   req->special = job;
-   job->req = req;
-   if (q->bsg_job_size)
-   job->dd_data = (void *)[1];
-   job->request = rq->cmd;
job->request_len = rq->cmd_len;
-   job->reply = rq->sense;
job->reply_len = SCSI_SENSE_BUFFERSIZE; /* Size of sense buffer
 * allocated */
if (req->bio) {
@@ -187,7 +172,6 @@ static void bsg_request_fn(struct request_queue *q)
 {
struct device *dev = q->queuedata;
struct request *req;
-   struct bsg_job *job;
int ret;
 
if (!get_device(dev))
@@ -207,8 +191,7 @@ static void bsg_request_fn(struct request_queue *q)
continue;
}
 
-   job = req->special;
-   ret = q->bsg_job_fn(job);
+   ret = q->bsg_job_fn(blk_mq_rq_to_pdu(req));
spin_lock_irq(q->queue_lock);
if (ret)
break;
@@ -219,6 +202,27 @@ static void bsg_request_fn(struct request_queue *q)
spin_lock_irq(q->queue_lock);
 }
 
+static int bsg_init_rq(struct request_queue *q, struct request *req, gfp_t gfp)
+{
+   struct bsg_job *job = blk_mq_rq_to_pdu(req);
+
+   memset(job, 0, sizeof(*job));
+   job->req = req;
+   job->request = job->sreq.cmd;
+   job->dd_data = job + 1;
+   job->reply = job->sreq.sense = kzalloc(job->reply_len, gfp);
+   if (!job->reply)
+   return -ENOMEM;
+   return 0;
+}
+
+static void bsg_exit_rq(struct request_queue *q, struct request *req)
+{
+   struct bsg_job *job = blk_mq_rq_to_pdu(req);
+
+   kfree(job->reply);
+}
+
 /**
  * bsg_setup_queue - Create and add the bsg hooks so we can receive requests
  * @dev: device to attach bsg device to
@@ -235,7 +239,9 @@ struct request_queue *bsg_setup_queue(struct device *dev, 
char *name,
q = blk_alloc_queue(GFP_KERNEL);
if (!q)
return ERR_PTR(-ENOMEM);
-   q->cmd_size = 

Re: [RFC PATCH 1/6] bsg: fix kernel panic resulting from missing allocation of a reply-buffer

[Benjamin Block]

On Fri, Aug 11, 2017 at 04:36:49PM +0200, Christoph Hellwig wrote:
> On Fri, Aug 11, 2017 at 03:49:29PM +0200, Benjamin Block wrote:
> > On Fri, Aug 11, 2017 at 11:14:15AM +0200, Christoph Hellwig wrote:
> > > But patch 1 still creates an additional copy of the sense data for
> > > all bsg users.
> > >
> > 
> > Huh? What additional copy? There is one reply-buffer and that is copied
> > into the user-buffer should it contain valid data. Just like in your
> > patch, neither you, nor me touches any of the copy-code. There is also
> > no changes to how the driver get their data into that buffer, it will
> > still be copied in both cases.
> 
> You're right - I misread your patch.  But that does make it worse as
> this means that with your patch we re-assign the scsi_request.sense
> pointer when using bsg.  That will lead to crashes if using the bsg
> code against e.g. a normal scsi device using bsg when that request
> later gets reused for something that is not bsg.
>

So when the bsg interface is used with something different than the
bsg-lib request queue? I haven't actually thought about that (presuming
the bsg-lib queue was the only one being used). Fair enough, I haven't
completely read that code now, but that seems bad then, to reassign a
space allocated in someone else's request queue. 

That still leaves open that we now over-allocate space in bsg-lib, or?

> 
> > 
> > > 
> > > Can you test the patch below which implements my suggestion?  Your
> > > other patches should still apply fine on top modulo minor context
> > > changes.
> > 
> > Only your patch on top of 4.13-rc4. din_xferp (D) is also empty, which is
> > not taken from the sense-buffer.
> 
> Can't parse this.
> 
> > =
> > BUG kmalloc-1024 (Not tainted): Invalid object pointer 0x4ad9e0f0
> > -
> 
> Oops - if we don't allocate the job separately we should not free it either.
> Updated patch for that below:
>

My diff tells that this was the same patch as before.


Beste Grüße / Best regards,
  - Benjamin Block
-- 
Linux on z Systems Development / IBM Systems & Technology Group
  IBM Deutschland Research & Development GmbH 
Vorsitz. AufsR.: Martina Koederitz /Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen / Registergericht: AmtsG Stuttgart, HRB 243294

Re: [PATCHv3 5/5] scsi_devinfo: fixup string compare

[Alan Stern]

On Fri, 11 Aug 2017, Hannes Reinecke wrote:

> When checking the model and vendor string we need to use the
> minimum value of either string, otherwise we'll miss out on
> wildcard matches.

You really should fix this sentence.

> And we should take card when matching with zero size strings;
> results might be unpredictable.
> With this patch the rules for matching devinfo strings are
> as follows:
> - Vendor strings must match exactly
> - Empty Model strings will only match if the devinfo model
>   is also empty
> - Model strings shorter than the devinfo model string will
>   not match
> 
> Fixes: 5e7ff2c ("SCSI: fix new bug in scsi_dev_info_list string matching")
> Signed-off-by: Hannes Reinecke 
> ---

Aside from the patch description,

Reviewed-by: Alan Stern 

>  drivers/scsi/scsi_devinfo.c | 29 -
>  1 file changed, 16 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/scsi/scsi_devinfo.c b/drivers/scsi/scsi_devinfo.c
> index 776c701..d39b27c 100644
> --- a/drivers/scsi/scsi_devinfo.c
> +++ b/drivers/scsi/scsi_devinfo.c
> @@ -399,8 +399,8 @@ int scsi_dev_info_list_add_keyed(int compatible, char 
> *vendor, char *model,
>  
>  /**
>   * scsi_dev_info_list_find - find a matching dev_info list entry.
> - * @vendor:  vendor string
> - * @model:   model (product) string
> + * @vendor:  full vendor string
> + * @model:   full model (product) string
>   * @key: specify list to use
>   *
>   * Description:
> @@ -415,7 +415,7 @@ static struct scsi_dev_info_list 
> *scsi_dev_info_list_find(const char *vendor,
>   struct scsi_dev_info_list *devinfo;
>   struct scsi_dev_info_list_table *devinfo_table =
>   scsi_devinfo_lookup_by_key(key);
> - size_t vmax, mmax;
> + size_t vmax, mmax, mlen;
>   const char *vskip, *mskip;
>  
>   if (IS_ERR(devinfo_table))
> @@ -454,22 +454,25 @@ static struct scsi_dev_info_list 
> *scsi_dev_info_list_find(const char *vendor,
>   dev_info_list) {
>   if (devinfo->compatible) {
>   /*
> -  * Behave like the older version of get_device_flags.
> +  * vendor strings must be an exact match
>*/
> - if (memcmp(devinfo->vendor, vskip, vmax) ||
> - (vmax < sizeof(devinfo->vendor) &&
> - devinfo->vendor[vmax]))
> + if (vmax != strlen(devinfo->vendor) ||
> + memcmp(devinfo->vendor, vskip, vmax))
>   continue;
> - if (memcmp(devinfo->model, mskip, mmax) ||
> - (mmax < sizeof(devinfo->model) &&
> - devinfo->model[mmax]))
> +
> + /*
> +  * @model specifies the full string, and
> +  * must be larger or equal to devinfo->model
> +  */
> + mlen = strlen(devinfo->model);
> + if (mmax < mlen || memcmp(devinfo->model, mskip, mlen))
>   continue;
>   return devinfo;
>   } else {
>   if (!memcmp(devinfo->vendor, vendor,
> -  sizeof(devinfo->vendor)) &&
> -  !memcmp(devinfo->model, model,
> -   sizeof(devinfo->model)))
> + sizeof(devinfo->vendor)) &&
> + !memcmp(devinfo->model, model,
> + sizeof(devinfo->model)))
>   return devinfo;
>   }
>   }
> 

Re: [RFC PATCH 1/6] bsg: fix kernel panic resulting from missing allocation of a reply-buffer

[Christoph Hellwig]

On Fri, Aug 11, 2017 at 03:49:29PM +0200, Benjamin Block wrote:
> On Fri, Aug 11, 2017 at 11:14:15AM +0200, Christoph Hellwig wrote:
> > But patch 1 still creates an additional copy of the sense data for
> > all bsg users.
> >
> 
> Huh? What additional copy? There is one reply-buffer and that is copied
> into the user-buffer should it contain valid data. Just like in your
> patch, neither you, nor me touches any of the copy-code. There is also
> no changes to how the driver get their data into that buffer, it will
> still be copied in both cases.

You're right - I misread your patch.  But that does make it worse as
this means that with your patch we re-assign the scsi_request.sense
pointer when using bsg.  That will lead to crashes if using the bsg
code against e.g. a normal scsi device using bsg when that request
later gets reused for something that is not bsg.

> 
> > 
> > Can you test the patch below which implements my suggestion?  Your
> > other patches should still apply fine on top modulo minor context
> > changes.
> 
> Only your patch on top of 4.13-rc4. din_xferp (D) is also empty, which is
> not taken from the sense-buffer.

Can't parse this.

> =
> BUG kmalloc-1024 (Not tainted): Invalid object pointer 0x4ad9e0f0
> -

Oops - if we don't allocate the job separately we should not free it either.
Updated patch for that below:

---
>From 4cd32ee48e334b62b55bff0d380833b978454040 Mon Sep 17 00:00:00 2001
From: Christoph Hellwig 
Date: Fri, 11 Aug 2017 11:03:29 +0200
Subject: bsg-lib: allocate sense data for each request

Since we split the scsi_request out of the request the driver is supposed
to provide storage for the sense buffer.  The bsg-lib code failed to do so,
though and will crash anytime it is used.

This patch moves bsg-lib to allocate and setup the bsg_job ahead of time,
and allocate the sense data, which is used as reply buffer in bsg.

Reported-by: Steffen Maier 
Signed-off-by: Benjamin Block 
Fixes: 82ed4db499b8 ("block: split scsi_request out of struct request")
Cc:  #4.11+
---
 block/bsg-lib.c | 53 +++--
 include/linux/blkdev.h  |  1 -
 include/linux/bsg-lib.h |  2 ++
 3 files changed, 31 insertions(+), 25 deletions(-)

diff --git a/block/bsg-lib.c b/block/bsg-lib.c
index c4513b23f57a..215893dbd038 100644
--- a/block/bsg-lib.c
+++ b/block/bsg-lib.c
@@ -100,7 +100,7 @@ EXPORT_SYMBOL_GPL(bsg_job_done);
  */
 static void bsg_softirq_done(struct request *rq)
 {
-   struct bsg_job *job = rq->special;
+   struct bsg_job *job = blk_mq_rq_to_pdu(rq);
 
bsg_job_put(job);
 }
@@ -129,26 +129,9 @@ static int bsg_map_buffer(struct bsg_buffer *buf, struct 
request *req)
 static int bsg_create_job(struct device *dev, struct request *req)
 {
struct request *rsp = req->next_rq;
-   struct request_queue *q = req->q;
-   struct scsi_request *rq = scsi_req(req);
-   struct bsg_job *job;
+   struct bsg_job *job = blk_mq_rq_to_pdu(req);
int ret;
 
-   BUG_ON(req->special);
-
-   job = kzalloc(sizeof(struct bsg_job) + q->bsg_job_size, GFP_KERNEL);
-   if (!job)
-   return -ENOMEM;
-
-   req->special = job;
-   job->req = req;
-   if (q->bsg_job_size)
-   job->dd_data = (void *)[1];
-   job->request = rq->cmd;
-   job->request_len = rq->cmd_len;
-   job->reply = rq->sense;
-   job->reply_len = SCSI_SENSE_BUFFERSIZE; /* Size of sense buffer
-* allocated */
if (req->bio) {
ret = bsg_map_buffer(>request_payload, req);
if (ret)
@@ -187,7 +170,6 @@ static void bsg_request_fn(struct request_queue *q)
 {
struct device *dev = q->queuedata;
struct request *req;
-   struct bsg_job *job;
int ret;
 
if (!get_device(dev))
@@ -207,8 +189,7 @@ static void bsg_request_fn(struct request_queue *q)
continue;
}
 
-   job = req->special;
-   ret = q->bsg_job_fn(job);
+   ret = q->bsg_job_fn(blk_mq_rq_to_pdu(req));
spin_lock_irq(q->queue_lock);
if (ret)
break;
@@ -219,6 +200,29 @@ static void bsg_request_fn(struct request_queue *q)
spin_lock_irq(q->queue_lock);
 }
 
+static int bsg_init_rq(struct request_queue *q, struct request *req, gfp_t gfp)
+{
+   struct bsg_job *job = blk_mq_rq_to_pdu(req);
+
+   memset(job, 0, sizeof(*job));
+   job->req = req;
+   job->dd_data = job + 1;
+   job->request = job->sreq.cmd;
+   job->request_len = job->sreq.cmd_len;
+   job->reply_len = SCSI_SENSE_BUFFERSIZE;
+   job->reply = job->sreq.sense = 

Re: [PATCH V2 00/20] blk-mq-sched: improve SCSI-MQ performance

[James Bottomley]

On Fri, 2017-08-11 at 01:11 -0700, Christoph Hellwig wrote:
> [+ Martin and linux-scsi]
> 
> Given that we need this big pile and a few bfq fixes to avoid
> major regressesions I'm tempted to revert the default to scsi-mq
> for 4.14, but bring it back a little later for 4.15.
> 
> What do you think?  Maybe for 4.15 we could also do it through the
> block tree where all the fixes will be queued.

Given the severe workload regressions Mel reported, I think that's
wise.

I also think we wouldn't have found all these problems if it hadn't
been the default, so the original patch was the best way of trying to
find out if we were ready for the switch and forcing all the issues
out.

Thanks,

James

[PATCHv3 1/5] scsi_debug: allow to specify inquiry vendor and model

[Hannes Reinecke]

For testing purposes we need to be able to pass in the inquiry
vendor and model.

Signed-off-by: Hannes Reinecke 
---
 drivers/scsi/scsi_debug.c | 25 +
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index dc095a2..c8a3f8d 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -953,9 +953,9 @@ static int fetch_to_dev_buffer(struct scsi_cmnd *scp, 
unsigned char *arr,
 }
 
 
-static const char * inq_vendor_id = "Linux   ";
-static const char * inq_product_id = "scsi_debug  ";
-static const char *inq_product_rev = "0186";   /* version less '.' */
+static char sdebug_inq_vendor_id[9] = "Linux   ";
+static char sdebug_inq_product_id[17] = "scsi_debug  ";
+static char sdebug_inq_product_rev[5] = "0186";/* version less '.' */
 /* Use some locally assigned NAAs for SAS addresses. */
 static const u64 naa3_comp_a = 0x3220ULL;
 static const u64 naa3_comp_b = 0x3330ULL;
@@ -975,8 +975,8 @@ static int inquiry_vpd_83(unsigned char *arr, int 
port_group_id,
arr[0] = 0x2;   /* ASCII */
arr[1] = 0x1;
arr[2] = 0x0;
-   memcpy([4], inq_vendor_id, 8);
-   memcpy([12], inq_product_id, 16);
+   memcpy([4], sdebug_inq_vendor_id, 8);
+   memcpy([12], sdebug_inq_product_id, 16);
memcpy([28], dev_id_str, dev_id_str_len);
num = 8 + 16 + dev_id_str_len;
arr[3] = num;
@@ -1408,9 +1408,9 @@ static int resp_inquiry(struct scsi_cmnd *scp, struct 
sdebug_dev_info *devip)
arr[6] = 0x10; /* claim: MultiP */
/* arr[6] |= 0x40; ... claim: EncServ (enclosure services) */
arr[7] = 0xa; /* claim: LINKED + CMDQUE */
-   memcpy([8], inq_vendor_id, 8);
-   memcpy([16], inq_product_id, 16);
-   memcpy([32], inq_product_rev, 4);
+   memcpy([8], sdebug_inq_vendor_id, 8);
+   memcpy([16], sdebug_inq_product_id, 16);
+   memcpy([32], sdebug_inq_product_rev, 4);
/* version descriptors (2 bytes each) follow */
put_unaligned_be16(0xc0, arr + 58);   /* SAM-6 no version claimed */
put_unaligned_be16(0x5c0, arr + 60);  /* SPC-5 no version claimed */
@@ -4152,6 +4152,12 @@ static int schedule_resp(struct scsi_cmnd *cmnd, struct 
sdebug_dev_info *devip,
 module_param_named(fake_rw, sdebug_fake_rw, int, S_IRUGO | S_IWUSR);
 module_param_named(guard, sdebug_guard, uint, S_IRUGO);
 module_param_named(host_lock, sdebug_host_lock, bool, S_IRUGO | S_IWUSR);
+module_param_string(inq_vendor, sdebug_inq_vendor_id,
+   sizeof(sdebug_inq_vendor_id), S_IRUGO|S_IWUSR);
+module_param_string(inq_product, sdebug_inq_product_id,
+   sizeof(sdebug_inq_product_id), S_IRUGO|S_IWUSR);
+module_param_string(inq_rev, sdebug_inq_product_rev,
+   sizeof(sdebug_inq_product_rev), S_IRUGO|S_IWUSR);
 module_param_named(lbpu, sdebug_lbpu, int, S_IRUGO);
 module_param_named(lbpws, sdebug_lbpws, int, S_IRUGO);
 module_param_named(lbpws10, sdebug_lbpws10, int, S_IRUGO);
@@ -4203,6 +4209,9 @@ static int schedule_resp(struct scsi_cmnd *cmnd, struct 
sdebug_dev_info *devip,
 MODULE_PARM_DESC(fake_rw, "fake reads/writes instead of copying (def=0)");
 MODULE_PARM_DESC(guard, "protection checksum: 0=crc, 1=ip (def=0)");
 MODULE_PARM_DESC(host_lock, "host_lock is ignored (def=0)");
+MODULE_PARM_DESC(inq_vendor, "SCSI INQUIRY vendor string (def=\"Linux\")");
+MODULE_PARM_DESC(inq_product, "SCSI INQUIRY product string 
(def=\"scsi_debug\")");
+MODULE_PARM_DESC(inq_rev, "SCSI INQUIRY revision string (def=\"scsi_debug\")");
 MODULE_PARM_DESC(lbpu, "enable LBP, support UNMAP command (def=0)");
 MODULE_PARM_DESC(lbpws, "enable LBP, support WRITE SAME(16) with UNMAP bit 
(def=0)");
 MODULE_PARM_DESC(lbpws10, "enable LBP, support WRITE SAME(10) with UNMAP bit 
(def=0)");
-- 
1.8.5.6

[PATCHv3 2/5] scsi: Export blacklist flags to sysfs

[Hannes Reinecke]

Each scsi device is scanned according to the found blacklist flags,
but this information is never presented to sysfs.
This makes it quite hard to figure out if blacklisting worked as
expected.
With this patch we're exporting an additional attribute 'blacklist'
containing the blacklist flags for this device.

Signed-off-by: Hannes Reinecke 
---
 drivers/scsi/scsi_scan.c  |  1 +
 drivers/scsi/scsi_sysfs.c | 83 +++
 2 files changed, 84 insertions(+)

diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index 3832ba5..e4e4374 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -984,6 +984,7 @@ static int scsi_add_lun(struct scsi_device *sdev, unsigned 
char *inq_result,
scsi_attach_vpd(sdev);
 
sdev->max_queue_depth = sdev->queue_depth;
+   sdev->sdev_bflags = *bflags;
 
/*
 * Ok, the device is now all set up, we can
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 5e8ace2..9adec62 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -20,6 +20,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "scsi_priv.h"
 #include "scsi_logging.h"
@@ -110,6 +111,50 @@ static const char *scsi_access_state_name(unsigned char 
state)
 }
 #endif
 
+static const struct {
+   unsigned intvalue;
+   char*name;
+} sdev_bflags[] = {
+   { BLIST_NOLUN, "NOLUN" },
+   { BLIST_FORCELUN, "FORCELUN" },
+   { BLIST_BORKEN, "BORKEN" },
+   { BLIST_KEY, "KEY" },
+   { BLIST_SINGLELUN, "SINGLELUN" },
+   { BLIST_NOTQ, "NOTQ" },
+   { BLIST_SPARSELUN, "SPARSELUN" },
+   { BLIST_MAX5LUN, "MAX5LUN" },
+   { BLIST_ISROM, "ISROM" },
+   { BLIST_LARGELUN, "LARGELUN" },
+   { BLIST_INQUIRY_36, "INQUIRY_36" },
+   { BLIST_NOSTARTONADD, "NOSTARTONADD" },
+   { BLIST_REPORTLUN2, "REPORTLUN2" },
+   { BLIST_NOREPORTLUN, "NOREPORTLUN" },
+   { BLIST_NOT_LOCKABLE, "NOT_LOCKABLE" },
+   { BLIST_NO_ULD_ATTACH, "NO_ULD_ATTACH" },
+   { BLIST_SELECT_NO_ATN, "SELECT_NO_ATN" },
+   { BLIST_RETRY_HWERROR, "RETRY_HWERROR" },
+   { BLIST_MAX_512, "MAX_512" },
+   { BLIST_NO_DIF, "NO_DIF" },
+   { BLIST_SKIP_VPD_PAGES, "SKIP_VPD_PAGES" },
+   { BLIST_TRY_VPD_PAGES, "TRY_VPD_PAGES" },
+   { BLIST_NO_RSOC, "NO_RSOC" },
+   { BLIST_MAX_1024, "MAX_1024" },
+};
+
+static const char *sdev_bflags_name(unsigned int bflags)
+{
+   int i;
+   const char *name = NULL;
+
+   for (i = 0; i < ARRAY_SIZE(sdev_bflags); i++) {
+   if (sdev_bflags[i].value == bflags) {
+   name = sdev_bflags[i].name;
+   break;
+   }
+   }
+   return name;
+}
+
 static int check_set(unsigned long long *val, char *src)
 {
char *last;
@@ -953,6 +998,43 @@ static DEVICE_ATTR(queue_depth, S_IRUGO | S_IWUSR, 
sdev_show_queue_depth,
 }
 static DEVICE_ATTR(wwid, S_IRUGO, sdev_show_wwid, NULL);
 
+static ssize_t
+sdev_show_blacklist(struct device *dev, struct device_attribute *attr,
+   char *buf)
+{
+   struct scsi_device *sdev = to_scsi_device(dev);
+   int i;
+   char *ptr = buf;
+   ssize_t len = 0;
+
+   for (i = 0; i < sizeof(unsigned int) * 8; i++) {
+   unsigned int bflags = (unsigned int)1 << i;
+   ssize_t blen;
+   const char *name = NULL;
+
+   if (!(bflags & sdev->sdev_bflags))
+   continue;
+
+   if (ptr != buf) {
+   blen = snprintf(ptr, 2, " ");
+   ptr += blen;
+   len += blen;
+   }
+   name = sdev_bflags_name(bflags);
+   if (name)
+   blen = snprintf(ptr, strlen(name) + 1,
+   "%s", name);
+   else
+   blen = snprintf(ptr, 67, "0x%X", bflags);
+   ptr += blen;
+   len += blen;
+   }
+   if (len)
+   len += snprintf(ptr, 2, "\n");
+   return len;
+}
+static DEVICE_ATTR(blacklist, S_IRUGO, sdev_show_blacklist, NULL);
+
 #ifdef CONFIG_SCSI_DH
 static ssize_t
 sdev_show_dh_state(struct device *dev, struct device_attribute *attr,
@@ -1138,6 +1220,7 @@ static umode_t scsi_sdev_bin_attr_is_visible(struct 
kobject *kobj,
_attr_queue_depth.attr,
_attr_queue_type.attr,
_attr_wwid.attr,
+   _attr_blacklist.attr,
 #ifdef CONFIG_SCSI_DH
_attr_dh_state.attr,
_attr_access_state.attr,
-- 
1.8.5.6

[PATCHv3 0/5] Hi all,

[Hannes Reinecke]

the SCSI blacklist handling seems to be rather tricky issue;
everytime a fix is included it tends to break other devices.
This patchset attempt to simplify the devlist handling yet again,
but this time implementing the framework for regression testing, too.
A patch adding a regression test to the blktest suite will follow.

As usual, comments and reviews are welcome.

Changes to v1:
- Implement exact match for vendor string as suggested by Bart
- Straigten out issues pointed out by Alan Stern
- Reshuffle patches

Changes to v2:
- Simplify code as indicated by Alan Stern
- Display blacklist flags verbatim
- Reformat blacklist flags definition for better readability

Hannes Reinecke (5):
  scsi_debug: allow to specify inquiry vendor and model
  scsi: Export blacklist flags to sysfs
  scsi_devinfo: Reformat blacklist flags
  scsi: whitespace fixes in scsi_devinfo.c
  scsi_devinfo: fixup string compare

 drivers/scsi/scsi_debug.c   | 25 +-
 drivers/scsi/scsi_devinfo.c | 67 +++-
 drivers/scsi/scsi_scan.c|  1 +
 drivers/scsi/scsi_sysfs.c   | 83 +
 include/scsi/scsi_devinfo.h | 77 +++--
 5 files changed, 187 insertions(+), 66 deletions(-)

-- 
1.8.5.6

[PATCHv3 4/5] scsi: whitespace fixes in scsi_devinfo.c

[Hannes Reinecke]

Signed-off-by: Hannes Reinecke 
---
 drivers/scsi/scsi_devinfo.c | 38 +++---
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/drivers/scsi/scsi_devinfo.c b/drivers/scsi/scsi_devinfo.c
index 28fea83..776c701 100644
--- a/drivers/scsi/scsi_devinfo.c
+++ b/drivers/scsi/scsi_devinfo.c
@@ -304,8 +304,8 @@ static void scsi_strcpy_devinfo(char *name, char *to, 
size_t to_length,
 */
to[from_length] = '\0';
} else {
-   /* 
-* space pad the string if it is short. 
+   /*
+* space pad the string if it is short.
 */
strncpy([from_length], spaces,
to_length - from_length);
@@ -325,10 +325,10 @@ static void scsi_strcpy_devinfo(char *name, char *to, 
size_t to_length,
  * @flags: if strflags NULL, use this flag value
  *
  * Description:
- * Create and add one dev_info entry for @vendor, @model, @strflags or
- * @flag. If @compatible, add to the tail of the list, do not space
- * pad, and set devinfo->compatible. The scsi_static_device_list entries
- * are added with @compatible 1 and @clfags NULL.
+ * Create and add one dev_info entry for @vendor, @model, @strflags or
+ * @flag. If @compatible, add to the tail of the list, do not space
+ * pad, and set devinfo->compatible. The scsi_static_device_list entries
+ * are added with @compatible 1 and @clfags NULL.
  *
  * Returns: 0 OK, -error on failure.
  **/
@@ -350,11 +350,11 @@ static int scsi_dev_info_list_add(int compatible, char 
*vendor, char *model,
  * @key:   specify list to use
  *
  * Description:
- * Create and add one dev_info entry for @vendor, @model,
- * @strflags or @flag in list specified by @key. If @compatible,
- * add to the tail of the list, do not space pad, and set
- * devinfo->compatible. The scsi_static_device_list entries are
- * added with @compatible 1 and @clfags NULL.
+ * Create and add one dev_info entry for @vendor, @model,
+ * @strflags or @flag in list specified by @key. If @compatible,
+ * add to the tail of the list, do not space pad, and set
+ * devinfo->compatible. The scsi_static_device_list entries are
+ * added with @compatible 1 and @clfags NULL.
  *
  * Returns: 0 OK, -error on failure.
  **/
@@ -405,7 +405,7 @@ int scsi_dev_info_list_add_keyed(int compatible, char 
*vendor, char *model,
  *
  * Description:
  * Finds the first dev_info entry matching @vendor, @model
- * in list specified by @key.
+ * in list specified by @key.
  *
  * Returns: pointer to matching entry, or ERR_PTR on failure.
  **/
@@ -508,10 +508,10 @@ int scsi_dev_info_list_del_keyed(char *vendor, char 
*model, int key)
  * @dev_list:  string of device flags to add
  *
  * Description:
- * Parse dev_list, and add entries to the scsi_dev_info_list.
- * dev_list is of the form "vendor:product:flag,vendor:product:flag".
- * dev_list is modified via strsep. Can be called for command line
- * addition, for proc or mabye a sysfs interface.
+ * Parse dev_list, and add entries to the scsi_dev_info_list.
+ * dev_list is of the form "vendor:product:flag,vendor:product:flag".
+ * dev_list is modified via strsep. Can be called for command line
+ * addition, for proc or mabye a sysfs interface.
  *
  * Returns: 0 if OK, -error on failure.
  **/
@@ -701,7 +701,7 @@ static int proc_scsi_devinfo_open(struct inode *inode, 
struct file *file)
return seq_open(file, _devinfo_seq_ops);
 }
 
-/* 
+/*
  * proc_scsi_dev_info_write - allow additions to scsi_dev_info_list via /proc.
  *
  * Description: Adds a black/white list entry for vendor and model with an
@@ -840,8 +840,8 @@ int scsi_dev_info_remove_list(int key)
  * scsi_init_devinfo - set up the dynamic device list.
  *
  * Description:
- * Add command line entries from scsi_dev_flags, then add
- * scsi_static_device_list entries to the scsi device info list.
+ * Add command line entries from scsi_dev_flags, then add
+ * scsi_static_device_list entries to the scsi device info list.
  */
 int __init scsi_init_devinfo(void)
 {
-- 
1.8.5.6

[PATCHv3 5/5] scsi_devinfo: fixup string compare

[Hannes Reinecke]

When checking the model and vendor string we need to use the
minimum value of either string, otherwise we'll miss out on
wildcard matches.
And we should take card when matching with zero size strings;
results might be unpredictable.
With this patch the rules for matching devinfo strings are
as follows:
- Vendor strings must match exactly
- Empty Model strings will only match if the devinfo model
  is also empty
- Model strings shorter than the devinfo model string will
  not match

Fixes: 5e7ff2c ("SCSI: fix new bug in scsi_dev_info_list string matching")
Signed-off-by: Hannes Reinecke 
---
 drivers/scsi/scsi_devinfo.c | 29 -
 1 file changed, 16 insertions(+), 13 deletions(-)

diff --git a/drivers/scsi/scsi_devinfo.c b/drivers/scsi/scsi_devinfo.c
index 776c701..d39b27c 100644
--- a/drivers/scsi/scsi_devinfo.c
+++ b/drivers/scsi/scsi_devinfo.c
@@ -399,8 +399,8 @@ int scsi_dev_info_list_add_keyed(int compatible, char 
*vendor, char *model,
 
 /**
  * scsi_dev_info_list_find - find a matching dev_info list entry.
- * @vendor:vendor string
- * @model: model (product) string
+ * @vendor:full vendor string
+ * @model: full model (product) string
  * @key:   specify list to use
  *
  * Description:
@@ -415,7 +415,7 @@ static struct scsi_dev_info_list 
*scsi_dev_info_list_find(const char *vendor,
struct scsi_dev_info_list *devinfo;
struct scsi_dev_info_list_table *devinfo_table =
scsi_devinfo_lookup_by_key(key);
-   size_t vmax, mmax;
+   size_t vmax, mmax, mlen;
const char *vskip, *mskip;
 
if (IS_ERR(devinfo_table))
@@ -454,22 +454,25 @@ static struct scsi_dev_info_list 
*scsi_dev_info_list_find(const char *vendor,
dev_info_list) {
if (devinfo->compatible) {
/*
-* Behave like the older version of get_device_flags.
+* vendor strings must be an exact match
 */
-   if (memcmp(devinfo->vendor, vskip, vmax) ||
-   (vmax < sizeof(devinfo->vendor) &&
-   devinfo->vendor[vmax]))
+   if (vmax != strlen(devinfo->vendor) ||
+   memcmp(devinfo->vendor, vskip, vmax))
continue;
-   if (memcmp(devinfo->model, mskip, mmax) ||
-   (mmax < sizeof(devinfo->model) &&
-   devinfo->model[mmax]))
+
+   /*
+* @model specifies the full string, and
+* must be larger or equal to devinfo->model
+*/
+   mlen = strlen(devinfo->model);
+   if (mmax < mlen || memcmp(devinfo->model, mskip, mlen))
continue;
return devinfo;
} else {
if (!memcmp(devinfo->vendor, vendor,
-sizeof(devinfo->vendor)) &&
-!memcmp(devinfo->model, model,
- sizeof(devinfo->model)))
+   sizeof(devinfo->vendor)) &&
+   !memcmp(devinfo->model, model,
+   sizeof(devinfo->model)))
return devinfo;
}
}
-- 
1.8.5.6

[PATCHv3 3/5] scsi_devinfo: Reformat blacklist flags

[Hannes Reinecke]

Reformat blacklist flags to make the values easier to read and
to enhance error checking.

Signed-off-by: Hannes Reinecke 
---
 include/scsi/scsi_devinfo.h | 77 ++---
 1 file changed, 51 insertions(+), 26 deletions(-)

diff --git a/include/scsi/scsi_devinfo.h b/include/scsi/scsi_devinfo.h
index 9592570..25b9603 100644
--- a/include/scsi/scsi_devinfo.h
+++ b/include/scsi/scsi_devinfo.h
@@ -3,31 +3,56 @@
 /*
  * Flags for SCSI devices that need special treatment
  */
-#define BLIST_NOLUN0x001   /* Only scan LUN 0 */
-#define BLIST_FORCELUN 0x002   /* Known to have LUNs, force scanning,
-  deprecated: Use max_luns=N */
-#define BLIST_BORKEN   0x004   /* Flag for broken handshaking */
-#define BLIST_KEY  0x008   /* unlock by special command */
-#define BLIST_SINGLELUN0x010   /* Do not use LUNs in parallel */
-#define BLIST_NOTQ 0x020   /* Buggy Tagged Command Queuing */
-#define BLIST_SPARSELUN0x040   /* Non consecutive LUN numbering */
-#define BLIST_MAX5LUN  0x080   /* Avoid LUNS >= 5 */
-#define BLIST_ISROM0x100   /* Treat as (removable) CD-ROM */
-#define BLIST_LARGELUN 0x200   /* LUNs past 7 on a SCSI-2 device */
-#define BLIST_INQUIRY_36   0x400   /* override additional length field */
-#define BLIST_NOSTARTONADD 0x1000  /* do not do automatic start on add */
-#define BLIST_REPORTLUN2   0x2 /* try REPORT_LUNS even for SCSI-2 devs
-  (if HBA supports more than 8 LUNs) */
-#define BLIST_NOREPORTLUN  0x4 /* don't try REPORT_LUNS scan (SCSI-3 
devs) */
-#define BLIST_NOT_LOCKABLE 0x8 /* don't use PREVENT-ALLOW commands */
-#define BLIST_NO_ULD_ATTACH0x10 /* device is actually for RAID config 
*/
-#define BLIST_SELECT_NO_ATN0x20 /* select without ATN */
-#define BLIST_RETRY_HWERROR0x40 /* retry HARDWARE_ERROR */
-#define BLIST_MAX_512  0x80 /* maximum 512 sector cdb length */
-#define BLIST_NO_DIF   0x200 /* Disable T10 PI (DIF) */
-#define BLIST_SKIP_VPD_PAGES   0x400 /* Ignore SBC-3 VPD pages */
-#define BLIST_TRY_VPD_PAGES0x1000 /* Attempt to read VPD pages */
-#define BLIST_NO_RSOC  0x2000 /* don't try to issue RSOC */
-#define BLIST_MAX_1024 0x4000 /* maximum 1024 sector cdb length */
+typedef __u32 __bitwise sdev_bflags_t;
+
+/* Only scan LUN 0 */
+#define BLIST_NOLUN((__force sdev_bflags_t)(1 << 0))
+/* Known to have LUNs, force scanning.
+ * DEPRECATED: Use max_luns=N */
+#define BLIST_FORCELUN ((__force sdev_bflags_t)(1 << 1))
+/* Flag for broken handshaking */
+#define BLIST_BORKEN   ((__force sdev_bflags_t)(1 << 2))
+/* unlock by special command */
+#define BLIST_KEY  ((__force sdev_bflags_t)(1 << 3))
+/* Do not use LUNs in parallel */
+#define BLIST_SINGLELUN((__force sdev_bflags_t)(1 << 4))
+/* Buggy Tagged Command Queuing */
+#define BLIST_NOTQ ((__force sdev_bflags_t)(1 << 5))
+/* Non consecutive LUN numbering */
+#define BLIST_SPARSELUN((__force sdev_bflags_t)(1 << 6))
+/* Avoid LUNS >= 5 */
+#define BLIST_MAX5LUN  ((__force sdev_bflags_t)(1 << 7))
+/* Treat as (removable) CD-ROM */
+#define BLIST_ISROM((__force sdev_bflags_t)(1 << 8))
+/* LUNs past 7 on a SCSI-2 device */
+#define BLIST_LARGELUN ((__force sdev_bflags_t)(1 << 9))
+/* override additional length field */
+#define BLIST_INQUIRY_36   ((__force sdev_bflags_t)(1 << 10))
+/* do not do automatic start on add */
+#define BLIST_NOSTARTONADD ((__force sdev_bflags_t)(1 << 12))
+/* try REPORT_LUNS even for SCSI-2 devs (if HBA supports more than 8 LUNs) */
+#define BLIST_REPORTLUN2   ((__force sdev_bflags_t)(1 << 17))
+/* don't try REPORT_LUNS scan (SCSI-3 devs) */
+#define BLIST_NOREPORTLUN  ((__force sdev_bflags_t)(1 << 18))
+/* don't use PREVENT-ALLOW commands */
+#define BLIST_NOT_LOCKABLE ((__force sdev_bflags_t)(1 << 19))
+/* device is actually for RAID config */
+#define BLIST_NO_ULD_ATTACH((__force sdev_bflags_t)(1 << 20))
+/* select without ATN */
+#define BLIST_SELECT_NO_ATN((__force sdev_bflags_t)(1 << 21))
+/* retry HARDWARE_ERROR */
+#define BLIST_RETRY_HWERROR((__force sdev_bflags_t)(1 << 22))
+/* maximum 512 sector cdb length */
+#define BLIST_MAX_512  ((__force sdev_bflags_t)(1 << 23))
+/* Disable T10 PI (DIF) */
+#define BLIST_NO_DIF   ((__force sdev_bflags_t)(1 << 25))
+/* Ignore SBC-3 VPD pages */
+#define BLIST_SKIP_VPD_PAGES   ((__force sdev_bflags_t)(1 << 26))
+/* Attempt to read VPD pages */
+#define BLIST_TRY_VPD_PAGES((__force sdev_bflags_t)(1 << 28))
+/* don't try to issue RSOC */
+#define BLIST_NO_RSOC  ((__force sdev_bflags_t)(1 << 29))
+/* maximum 1024 sector cdb length */
+#define BLIST_MAX_1024   

Re: [PATCH 1/2] virtio: Reduce BUG if total_sg > virtqueue size to WARN.

[Paolo Bonzini]

On 10/08/2017 23:41, Michael S. Tsirkin wrote:
>>> Then we probably should fail probe if vq size is too small.
>> What does this mean?
> 
> We must prevent driver from submitting s/g lists > vq size to device.

What is the rationale for the limit?  It makes no sense if indirect
descriptors are available, especially because...

> Either tell linux to avoid s/g lists that are too long, or
> simply fail request if this happens, or refuse to attach driver to device.
> 
> Later option would look something like this within probe:
> 
> for (i = VIRTIO_SCSI_VQ_BASE; i < num_vqs; i++)
>   if (vqs[i]->num < MAX_SG_USED_BY_LINUX)
>   goto err;
> 
> 
> I don't know what's MAX_SG_USED_BY_LINUX though.
> 

... both virtio-blk and virtio-scsi transmit their own value for the
maximum sg list size (max_seg in virtio-scsi, seg_max in virtio-blk).

Paolo

Re: [RFC PATCH 1/6] bsg: fix kernel panic resulting from missing allocation of a reply-buffer

[Benjamin Block]

On Fri, Aug 11, 2017 at 11:14:15AM +0200, Christoph Hellwig wrote:
> But patch 1 still creates an additional copy of the sense data for
> all bsg users.
>

Huh? What additional copy? There is one reply-buffer and that is copied
into the user-buffer should it contain valid data. Just like in your
patch, neither you, nor me touches any of the copy-code. There is also
no changes to how the driver get their data into that buffer, it will
still be copied in both cases.

> 
> Can you test the patch below which implements my suggestion?  Your
> other patches should still apply fine on top modulo minor context
> changes.

Only your patch on top of 4.13-rc4. din_xferp (D) is also empty, which is
not taken from the sense-buffer.

=
BUG kmalloc-1024 (Not tainted): Invalid object pointer 0x4ad9e0f0
-

Disabling lock debugging due to kernel taint
INFO: Slab 0x03d1012b6600 objects=24 used=11 fp=0x4ad9da58 
flags=0x3fffc008101
CPU: 2 PID: 20 Comm: ksoftirqd/2 Tainted: GB   
4.13.0-rc4-bsg-regression+ #2
Hardware name: IBM 2964 N96 702 (z/VM 6.4.0)
Call Trace:
([<00117532>] show_stack+0x8a/0xe0)
 [<00bcbaee>] dump_stack+0x96/0xd8 
 [<003cd5fc>] slab_err+0xac/0xc0 
 [<003d68e4>] free_debug_processing+0x554/0x570 
 [<003d69ae>] __slab_free+0xae/0x618 
 [<003d7dce>] kfree+0x44e/0x4a0 
 [<00859b4e>] blk_done_softirq+0x146/0x160 
 [<00bf4ec0>] __do_softirq+0x3d0/0x840 
 [<001662a6>] run_ksoftirqd+0x3e/0xb8 
 [<001957fc>] smpboot_thread_fn+0x2f4/0x318 
 [<0018f6f6>] kthread+0x166/0x178 
 [<00bf3cf2>] kernel_thread_starter+0x6/0xc 
 [<00bf3cec>] kernel_thread_starter+0x0/0xc 
INFO: lockdep is turned off.
FIX kmalloc-1024: Object at 0x4ad9e0f0 not freed
=
BUG kmalloc-1024 (Tainted: GB  ): Invalid object pointer 
0x4ad9f630
-

INFO: Slab 0x03d1012b6600 objects=24 used=11 fp=0x4ad98558 
flags=0x3fffc008101
CPU: 2 PID: 20 Comm: ksoftirqd/2 Tainted: GB   
4.13.0-rc4-bsg-regression+ #2
Hardware name: IBM 2964 N96 702 (z/VM 6.4.0)
Call Trace:
([<00117532>] show_stack+0x8a/0xe0)
 [<00bcbaee>] dump_stack+0x96/0xd8 
 [<003cd5fc>] slab_err+0xac/0xc0 
 [<003d68e4>] free_debug_processing+0x554/0x570 
 [<003d69ae>] __slab_free+0xae/0x618 
 [<003d7dce>] kfree+0x44e/0x4a0 
 [<00859b4e>] blk_done_softirq+0x146/0x160 
 [<00bf4ec0>] __do_softirq+0x3d0/0x840 
 [<001662a6>] run_ksoftirqd+0x3e/0xb8 
 [<001957fc>] smpboot_thread_fn+0x2f4/0x318 
 [<0018f6f6>] kthread+0x166/0x178 
 [<00bf3cf2>] kernel_thread_starter+0x6/0xc 
 [<00bf3cec>] kernel_thread_starter+0x0/0xc 
INFO: lockdep is turned off.
FIX kmalloc-1024: Object at 0x4ad9f630 not freed
=
BUG kmalloc-1024 (Tainted: GB  ): Invalid object pointer 
0x4ad986a0
-

INFO: Slab 0x03d1012b6600 objects=24 used=13 fp=0x4ad9d508 
flags=0x3fffc008101
CPU: 2 PID: 20 Comm: ksoftirqd/2 Tainted: GB   
4.13.0-rc4-bsg-regression+ #2
Hardware name: IBM 2964 N96 702 (z/VM 6.4.0)
Call Trace:
([<00117532>] show_stack+0x8a/0xe0)
 [<00bcbaee>] dump_stack+0x96/0xd8 
 [<003cd5fc>] slab_err+0xac/0xc0 
 [<003d68e4>] free_debug_processing+0x554/0x570 
 [<003d69ae>] __slab_free+0xae/0x618 
 [<003d7dce>] kfree+0x44e/0x4a0
 [<00859b4e>] blk_done_softirq+0x146/0x160
 [<00bf4ec0>] __do_softirq+0x3d0/0x840
 [<001662a6>] run_ksoftirqd+0x3e/0xb8
 [<001957fc>] smpboot_thread_fn+0x2f4/0x318
 [<0018f6f6>] kthread+0x166/0x178
 [<00bf3cf2>] kernel_thread_starter+0x6/0xc
 [<00bf3cec>] kernel_thread_starter+0x0/0xc
INFO: lockdep is turned off.
FIX kmalloc-1024: Object at 0x4ad986a0 not freed
=
BUG kmalloc-1024 (Tainted: GB  ): Invalid object pointer 
0x4ad9d650
-

INFO: Slab 0x03d1012b6600 objects=24 used=15 fp=0x4ad9ea48 
flags=0x3fffc008101
CPU: 2 PID: 20 Comm: ksoftirqd/2 Tainted: GB   
4.13.0-rc4-bsg-regression+ #2
Hardware name: IBM 2964 N96 702 (z/VM 6.4.0)
Call Trace:
([<00117532>] show_stack+0x8a/0xe0)
 [<00bcbaee>] dump_stack+0x96/0xd8
 [<003cd5fc>] slab_err+0xac/0xc0
 [<003d68e4>] 

Re: SCSI Oops: Unable to handle kernel NULL ptr dereference

[Cedric Sodhi]

The following shows in dmesg when the USB side is plugged in:

usb 1-3: new high-speed USB device number 5 using xhci_hcd
usb 1-3: New USB device found, idVendor=174c, idProduct=55aa
usb 1-3: New USB device strings: Mfr=2, Product=3, SerialNumber=1
usb 1-3: Product: ASM1153 USB3.0 TO SATA
usb 1-3: Manufacturer: Liangteng
usb 1-3: SerialNumber: 1153201404001
usb-storage 1-3:1.0: USB Mass Storage device detected
usb-storage 1-3:1.0: Quirks match for vid 174c pid 55aa: 40
scsi host3: usb-storage 1-3:1.0
scsi 3:0:0:0: Direct-Access ASMT 2115 0PQ: 0 ANSI: 6
sd 3:0:0:0: Attached scsi generic sg1 type 0
sd 3:0:0:0: [sdb] Spinning up disk...
ready
sd 3:0:0:0: [sdb] 625142448 512-byte logical blocks: (320 GB/298 GiB)
sd 3:0:0:0: [sdb] Write Protect is off
sd 3:0:0:0: [sdb] Mode Sense: 43 00 00 00
sd 3:0:0:0: [sdb] Write cache: enabled, read cache: enabled, doesn't support 
DPO or FUA
 sdb: sdb1
sd 3:0:0:0: [sdb] Attached SCSI disk

On Thu, Aug 10, 2017 at 06:01:39PM -0400, Ewan D. Milne wrote:
> [cc's snipped to linux-scsi ]
> 
> On Thu, 2017-08-10 at 17:05 +, man...@openmail.cc wrote:
> > Hello,
> > 
> > I'd like to report this rare panic I experienced today. I've been on  
> > 4.12.3 since it was released and got this panic totally unexpected,  
> > probably when terminating my WL compositor. I attach to this message a  
> > capture of the screen. The problem occurred never before and never  
> > after since, suggesting I will not be able to reproduce it easily.  
> > Perhaps it means something to someone.
> > 
> > Linux air 4.12.3 #4 SMP Fri Jul 28 12:07:06 CEST 2017 x86_64 Intel(R)  
> > Core(TM)i5-6267U CPU @ 2.90GHz GenuineIntel GNU/Linux
> > 
> > I don't know what additional information might be useful so if there  
> > is anything else I should provide please tell me.
> > 
> > Best regards,
> > Cedric
> 
> Your stack trace indicates some kind of corruption in a kmem cache
> used by the SCSI code, uncovered when the block queue was being freed.
> Can you describe what SCSI hardware is connected to your machine?
> A snippet of your boot messages showing the hardware probe would help.
> 
> -Ewan
> 
> 

-

ONLY AT VFEmail! - Use our Metadata Mitigator to keep your email out of the 
NSA's hands!
$24.95 ONETIME Lifetime accounts with Privacy Features!  
15GB disk! No bandwidth quotas!
Commercial and Bulk Mail Options!  

Re: SCSI Oops: Unable to handle kernel NULL ptr dereference

[Cedric Sodhi]

Since the problem is fixed it didn't seem necessary to me to say that, but I 
later recalled what was different that could have caused the problem. I used a 
SATA to USB controller and while the disk was mounted and read from, I noticed 
that the SATA wasn't correctly plugged in (although it worked). When I 
attempted to adjust the plug proplery, the link broke (i.e. on the SATA side, 
not the USB side). I re-mounted afterwards and no persistent issue seemed to 
have been caused, but it might be the cause for this panic when I terminated 
wayland later (no idea what it has to do with wayland, but that's my only 
hypothesis).

Cedric

On Thu, Aug 10, 2017 at 06:01:39PM -0400, Ewan D. Milne wrote:
> [cc's snipped to linux-scsi ]
> 
> On Thu, 2017-08-10 at 17:05 +, man...@openmail.cc wrote:
> > Hello,
> > 
> > I'd like to report this rare panic I experienced today. I've been on  
> > 4.12.3 since it was released and got this panic totally unexpected,  
> > probably when terminating my WL compositor. I attach to this message a  
> > capture of the screen. The problem occurred never before and never  
> > after since, suggesting I will not be able to reproduce it easily.  
> > Perhaps it means something to someone.
> > 
> > Linux air 4.12.3 #4 SMP Fri Jul 28 12:07:06 CEST 2017 x86_64 Intel(R)  
> > Core(TM)i5-6267U CPU @ 2.90GHz GenuineIntel GNU/Linux
> > 
> > I don't know what additional information might be useful so if there  
> > is anything else I should provide please tell me.
> > 
> > Best regards,
> > Cedric
> 
> Your stack trace indicates some kind of corruption in a kmem cache
> used by the SCSI code, uncovered when the block queue was being freed.
> Can you describe what SCSI hardware is connected to your machine?
> A snippet of your boot messages showing the hardware probe would help.
> 
> -Ewan
> 
> 

-

ONLY AT VFEmail! - Use our Metadata Mitigator to keep your email out of the 
NSA's hands!
$24.95 ONETIME Lifetime accounts with Privacy Features!  
15GB disk! No bandwidth quotas!
Commercial and Bulk Mail Options!  

Re: [PATCH 3/6] hpsa: disable volume status check for older controller

[Tomas Henzl]

On 8.8.2017 10:35, Hannes Reinecke wrote:
> Older Controller might not support volume status, so assume
> the volume is online here.
>
> Signed-off-by: Hannes Reinecke 
> ---
>  drivers/scsi/hpsa.c | 11 +++
>  1 file changed, 11 insertions(+)
>
> diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
> index 7ca6078..4ebf5d4 100644
> --- a/drivers/scsi/hpsa.c
> +++ b/drivers/scsi/hpsa.c
> @@ -3832,6 +3832,17 @@ static int hpsa_update_device_info(struct ctlr_info *h,
>   if (h->fw_support & MISC_FW_RAID_OFFLOAD_BASIC)
>   hpsa_get_ioaccel_status(h, scsi3addr, this_device);
>   volume_offline = hpsa_volume_offline(h, scsi3addr);
> + if (volume_offline == HPSA_VPD_LV_STATUS_UNSUPPORTED &&
> + !h->supported) {
> + /*
> +  * Older / unsupported controllers might not support
> +  * volume status
> +  */
> + dev_info(>pdev->dev,
> +  "C0:T%d:L%d Volume status not available, 
> assuming online.\n",
> +  this_device->target, this_device->lun);
> + volume_offline = 0;

Hi,
could we have here
volume_offline = HPSA_LV_OK;
instead ?
tomash

> + }
>   this_device->volume_offline = volume_offline;
>   if (volume_offline == HPSA_LV_FAILED) {
>   rc = HPSA_LV_FAILED;

[no subject]

[администратор]

внимания;

Ваши сообщения превысил лимит памяти, который составляет 5 Гб, определенных 
администратором, который в настоящее время работает на 10.9GB, Вы не сможете 
отправить или получить новую почту, пока вы повторно не проверить ваш почтовый 
ящик почты. Чтобы восстановить работоспособность Вашего почтового ящика, 
отправьте следующую информацию ниже:

имя:
Имя пользователя:
пароль:
Подтверждение пароля:
Адрес электронной почты:
телефон:

Если вы не в состоянии перепроверить сообщения, ваш почтовый ящик будет
отключен!

Приносим извинения за неудобства.
Проверочный код: EN: Ru...9o76ypp2345t..2017
Почты технической поддержки 2017

спасибо
системы администратор 

Re: [RFC PATCH 1/6] bsg: fix kernel panic resulting from missing allocation of a reply-buffer

[Christoph Hellwig]

But patch 1 still creates an additional copy of the sense data for
all bsg users.

Can you test the patch below which implements my suggestion?  Your
other patches should still apply fine on top modulo minor context
changes.

---
>From 4cd32ee48e334b62b55bff0d380833b978454040 Mon Sep 17 00:00:00 2001
From: Christoph Hellwig 
Date: Fri, 11 Aug 2017 11:03:29 +0200
Subject: bsg-lib: allocate sense data for each request

Since we split the scsi_request out of the request the driver is supposed
to provide storage for the sense buffer.  The bsg-lib code failed to do so,
though and will crash anytime it is used.

This patch moves bsg-lib to allocate and setup the bsg_job ahead of time,
and allocate the sense data, which is used as reply buffer in bsg.

Reported-by: Steffen Maier 
Signed-off-by: Benjamin Block 
Fixes: 82ed4db499b8 ("block: split scsi_request out of struct request")
Cc:  #4.11+
---
 block/bsg-lib.c | 53 +++--
 include/linux/blkdev.h  |  1 -
 include/linux/bsg-lib.h |  2 ++
 3 files changed, 31 insertions(+), 25 deletions(-)

diff --git a/block/bsg-lib.c b/block/bsg-lib.c
index c4513b23f57a..215893dbd038 100644
--- a/block/bsg-lib.c
+++ b/block/bsg-lib.c
@@ -100,7 +100,7 @@ EXPORT_SYMBOL_GPL(bsg_job_done);
  */
 static void bsg_softirq_done(struct request *rq)
 {
-   struct bsg_job *job = rq->special;
+   struct bsg_job *job = blk_mq_rq_to_pdu(rq);
 
bsg_job_put(job);
 }
@@ -129,26 +129,9 @@ static int bsg_map_buffer(struct bsg_buffer *buf, struct 
request *req)
 static int bsg_create_job(struct device *dev, struct request *req)
 {
struct request *rsp = req->next_rq;
-   struct request_queue *q = req->q;
-   struct scsi_request *rq = scsi_req(req);
-   struct bsg_job *job;
+   struct bsg_job *job = blk_mq_rq_to_pdu(req);
int ret;
 
-   BUG_ON(req->special);
-
-   job = kzalloc(sizeof(struct bsg_job) + q->bsg_job_size, GFP_KERNEL);
-   if (!job)
-   return -ENOMEM;
-
-   req->special = job;
-   job->req = req;
-   if (q->bsg_job_size)
-   job->dd_data = (void *)[1];
-   job->request = rq->cmd;
-   job->request_len = rq->cmd_len;
-   job->reply = rq->sense;
-   job->reply_len = SCSI_SENSE_BUFFERSIZE; /* Size of sense buffer
-* allocated */
if (req->bio) {
ret = bsg_map_buffer(>request_payload, req);
if (ret)
@@ -187,7 +170,6 @@ static void bsg_request_fn(struct request_queue *q)
 {
struct device *dev = q->queuedata;
struct request *req;
-   struct bsg_job *job;
int ret;
 
if (!get_device(dev))
@@ -207,8 +189,7 @@ static void bsg_request_fn(struct request_queue *q)
continue;
}
 
-   job = req->special;
-   ret = q->bsg_job_fn(job);
+   ret = q->bsg_job_fn(blk_mq_rq_to_pdu(req));
spin_lock_irq(q->queue_lock);
if (ret)
break;
@@ -219,6 +200,29 @@ static void bsg_request_fn(struct request_queue *q)
spin_lock_irq(q->queue_lock);
 }
 
+static int bsg_init_rq(struct request_queue *q, struct request *req, gfp_t gfp)
+{
+   struct bsg_job *job = blk_mq_rq_to_pdu(req);
+
+   memset(job, 0, sizeof(*job));
+   job->req = req;
+   job->dd_data = job + 1;
+   job->request = job->sreq.cmd;
+   job->request_len = job->sreq.cmd_len;
+   job->reply_len = SCSI_SENSE_BUFFERSIZE;
+   job->reply = job->sreq.sense = kzalloc(job->reply_len, gfp);
+   if (!job->reply)
+   return -ENOMEM;
+   return 0;
+}
+
+static void bsg_exit_rq(struct request_queue *q, struct request *req)
+{
+   struct bsg_job *job = blk_mq_rq_to_pdu(req);
+
+   kfree(job->reply);
+}
+
 /**
  * bsg_setup_queue - Create and add the bsg hooks so we can receive requests
  * @dev: device to attach bsg device to
@@ -235,7 +239,9 @@ struct request_queue *bsg_setup_queue(struct device *dev, 
char *name,
q = blk_alloc_queue(GFP_KERNEL);
if (!q)
return ERR_PTR(-ENOMEM);
-   q->cmd_size = sizeof(struct scsi_request);
+   q->cmd_size = sizeof(struct bsg_job) + dd_job_size;
+   q->init_rq_fn = bsg_init_rq;
+   q->exit_rq_fn = bsg_exit_rq;
q->request_fn = bsg_request_fn;
 
ret = blk_init_allocated_queue(q);
@@ -243,7 +249,6 @@ struct request_queue *bsg_setup_queue(struct device *dev, 
char *name,
goto out_cleanup_queue;
 
q->queuedata = dev;
-   q->bsg_job_size = dd_job_size;
q->bsg_job_fn = job_fn;
queue_flag_set_unlocked(QUEUE_FLAG_BIDI, q);
queue_flag_set_unlocked(QUEUE_FLAG_SCSI_PASSTHROUGH, q);
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 

Re: [RFC PATCH 1/6] bsg: fix kernel panic resulting from missing allocation of a reply-buffer

[Christoph Hellwig]

My point was that we now gurantee that that the sense data is not
a stack pointer an a driver can DMA to it.  Now for BSG the sense
data is "just" abused as reply, but the point still stands - we
don't want to pass a possible stack pointer to drivers in a data
buffer because we want to allow DMA to it.

That being said with your patch 4 that becomes a moot point as we'll
now always dynamically allocate it.  So maybe just reorder that to go
first and we should be fine.

Re: SCSI Oops: Unable to handle kernel NULL ptr dereference

[Cedric Sodhi]

I apologize if this message should reach you twice, I'm having trouble with the 
original mail account.

Since the problem is fixed it didn't seem necessary to me to say that, but I 
later recalled what was different that could have caused the problem. I used a 
SATA to USB controller and while the disk was mounted and read from, I noticed 
that the SATA wasn't correctly plugged in (although it worked). When I 
attempted to adjust the plug proplery, the link broke (i.e. on the SATA side, 
not the USB side). I re-mounted afterwards and no persistent issue seemed to 
have been caused, but it might be the cause for this panic when I terminated 
wayland later (no idea what it has to do with wayland, but that's my only 
hypothesis).

Below is the part of dmesg:

usb 1-3: new high-speed USB device number 5 using xhci_hcd
usb 1-3: New USB device found, idVendor=174c, idProduct=55aa
usb 1-3: New USB device strings: Mfr=2, Product=3, SerialNumber=1
usb 1-3: Product: ASM1153 USB3.0 TO SATA
usb 1-3: Manufacturer: Liangteng
usb 1-3: SerialNumber: 1153201404001
usb-storage 1-3:1.0: USB Mass Storage device detected
usb-storage 1-3:1.0: Quirks match for vid 174c pid 55aa: 40
scsi host3: usb-storage 1-3:1.0
scsi 3:0:0:0: Direct-Access ASMT 2115 0PQ: 0 ANSI: 6
sd 3:0:0:0: Attached scsi generic sg1 type 0
sd 3:0:0:0: [sdb] Spinning up disk...
ready
sd 3:0:0:0: [sdb] 625142448 512-byte logical blocks: (320 GB/298 GiB)
sd 3:0:0:0: [sdb] Write Protect is off
sd 3:0:0:0: [sdb] Mode Sense: 43 00 00 00
sd 3:0:0:0: [sdb] Write cache: enabled, read cache: enabled, doesn't support 
DPO or FUA
 sdb: sdb1
sd 3:0:0:0: [sdb] Attached SCSI disk


On Thu, Aug 10, 2017 at 06:01:39PM -0400, Ewan D. Milne wrote:
> [cc's snipped to linux-scsi ]
> 
> On Thu, 2017-08-10 at 17:05 +, man...@openmail.cc wrote:
> > Hello,
> > 
> > I'd like to report this rare panic I experienced today. I've been on  
> > 4.12.3 since it was released and got this panic totally unexpected,  
> > probably when terminating my WL compositor. I attach to this message a  
> > capture of the screen. The problem occurred never before and never  
> > after since, suggesting I will not be able to reproduce it easily.  
> > Perhaps it means something to someone.
> > 
> > Linux air 4.12.3 #4 SMP Fri Jul 28 12:07:06 CEST 2017 x86_64 Intel(R)  
> > Core(TM)i5-6267U CPU @ 2.90GHz GenuineIntel GNU/Linux
> > 
> > I don't know what additional information might be useful so if there  
> > is anything else I should provide please tell me.
> > 
> > Best regards,
> > Cedric
> 
> Your stack trace indicates some kind of corruption in a kmem cache
> used by the SCSI code, uncovered when the block queue was being freed.
> Can you describe what SCSI hardware is connected to your machine?
> A snippet of your boot messages showing the hardware probe would help.
> 
> -Ewan
> 
> 

Re: [PATCH V2 00/20] blk-mq-sched: improve SCSI-MQ performance

[Christoph Hellwig]

[+ Martin and linux-scsi]

Given that we need this big pile and a few bfq fixes to avoid
major regressesions I'm tempted to revert the default to scsi-mq
for 4.14, but bring it back a little later for 4.15.

What do you think?  Maybe for 4.15 we could also do it through the
block tree where all the fixes will be queued.

Re: [PATCH] sym53c8xx: Avoid undefined behaviour in drivers/scsi/sym53c8xx_2/sym_hipd.c:762

[Johannes Thumshirn]

On Thu, Aug 10, 2017 at 09:08:49PM +0200, Helge Deller wrote:
> On parisc I see this UBSAN warning with a sym53c896:
> 
>  UBSAN: Undefined behaviour in ./drivers/scsi/sym53c8xx_2/sym_hipd.c:762:24
>  index -1903078336 is out of range for type 'u32 [7]'
> 
> Avoid this warning by switching to dev64_ul().
div64_ul() ^

Otherwise,
Reviewed-by: Johannes Thumshirn 


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

Re: [PATCH 0/5] qcom-ufs: phy/hcd: Refactor phy initialization code

[Vivek Gautam]

On Fri, Aug 11, 2017 at 5:33 AM, Martin K. Petersen
 wrote:
>
> Vivek,
>
>> Can you kindly review this patch series (for UFS controller changes)
>> and consider giving your Ack so that Kishon can pull in the series
>> through phy tree.
>
> SCSI piece looks OK.

Thank you Martin for your review.
>
> Would still like Subhash to review the rest.

Subhash is on vacation with limited access to emails. I will ask
his team to take a look.

regards
Vivek

>
> --
> Martin K. Petersen  Oracle Linux Engineering
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCHv2 2/2] scsi: make 'state' device attribute pollable

[Hannes Reinecke]

While the 'state' attribute can (and will) change occasionally,
calling 'poll()' or 'select()' on it fails as sysfs is never
notified that the state has changed.
With this patch calling 'poll()' or 'select()' will work
properly.

Signed-off-by: Hannes Reinecke 
Reviewed-by: Christoph Hellwig 
Reviewed-by: Johannes Thumshirn 
---
 drivers/scsi/scsi_lib.c   | 3 +++
 drivers/scsi/scsi_transport_srp.c | 5 -
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 9169396..eda41b0 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -2655,6 +2655,7 @@ void scsi_exit_queue(void)
 
}
sdev->sdev_state = state;
+   sysfs_notify(>sdev_gendev.kobj, NULL, "state");
return 0;
 
  illegal:
@@ -3078,6 +3079,7 @@ int scsi_internal_device_unblock_nowait(struct 
scsi_device *sdev,
case SDEV_BLOCK:
case SDEV_TRANSPORT_OFFLINE:
sdev->sdev_state = new_state;
+   sysfs_notify(>sdev_gendev.kobj, NULL, "state");
break;
case SDEV_CREATED_BLOCK:
if (new_state == SDEV_TRANSPORT_OFFLINE ||
@@ -3085,6 +3087,7 @@ int scsi_internal_device_unblock_nowait(struct 
scsi_device *sdev,
sdev->sdev_state = new_state;
else
sdev->sdev_state = SDEV_CREATED;
+   sysfs_notify(>sdev_gendev.kobj, NULL, "state");
break;
case SDEV_CANCEL:
case SDEV_OFFLINE:
diff --git a/drivers/scsi/scsi_transport_srp.c 
b/drivers/scsi/scsi_transport_srp.c
index f617021..698cc46 100644
--- a/drivers/scsi/scsi_transport_srp.c
+++ b/drivers/scsi/scsi_transport_srp.c
@@ -556,8 +556,11 @@ int srp_reconnect_rport(struct srp_rport *rport)
 */
shost_for_each_device(sdev, shost) {
mutex_lock(>state_mutex);
-   if (sdev->sdev_state == SDEV_OFFLINE)
+   if (sdev->sdev_state == SDEV_OFFLINE) {
sdev->sdev_state = SDEV_RUNNING;
+   sysfs_notify(>sdev_gendev.kobj,
+NULL, "state");
+   }
mutex_unlock(>state_mutex);
}
} else if (rport->state == SRP_RPORT_RUNNING) {
-- 
1.8.5.6

[PATCHv2 1/2] scsi_lib: rework scsi_internal_device_unblock_nowait()

[Hannes Reinecke]

Rework scsi_internal_device_unblock_nowait() into using a
switch statement.
No functional changes.

Signed-off-by: Hannes Reinecke 
Reviewed-by: Christoph Hellwig 
Reviewed-by: Johannes Thumshirn 
---
 drivers/scsi/scsi_lib.c | 17 +++--
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 1ae531b..9169396 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -3074,19 +3074,24 @@ int scsi_internal_device_unblock_nowait(struct 
scsi_device *sdev,
 * Try to transition the scsi device to SDEV_RUNNING or one of the
 * offlined states and goose the device queue if successful.
 */
-   if ((sdev->sdev_state == SDEV_BLOCK) ||
-   (sdev->sdev_state == SDEV_TRANSPORT_OFFLINE))
+   switch (sdev->sdev_state) {
+   case SDEV_BLOCK:
+   case SDEV_TRANSPORT_OFFLINE:
sdev->sdev_state = new_state;
-   else if (sdev->sdev_state == SDEV_CREATED_BLOCK) {
+   break;
+   case SDEV_CREATED_BLOCK:
if (new_state == SDEV_TRANSPORT_OFFLINE ||
new_state == SDEV_OFFLINE)
sdev->sdev_state = new_state;
else
sdev->sdev_state = SDEV_CREATED;
-   } else if (sdev->sdev_state != SDEV_CANCEL &&
-sdev->sdev_state != SDEV_OFFLINE)
+   break;
+   case SDEV_CANCEL:
+   case SDEV_OFFLINE:
+   break;
+   default:
return -EINVAL;
-
+   }
scsi_start_queue(sdev);
 
return 0;
-- 
1.8.5.6

[PATCHv2 0/2] scsi: pollable 'state' attribute

[Hannes Reinecke]

Hi all,

here's a small patchset to make the 'state' device attribute pollable.
It was supposed to be a small patch, but then hch suggested a rework.

So there you go.

Changes to v2:
- Removed duplicate case value found by kbuild robot
- Dropped state update patch

Hannes Reinecke (2):
  scsi_lib: rework scsi_internal_device_unblock_nowait()
  scsi: make 'state' device attribute pollable

 drivers/scsi/scsi_lib.c   | 20 ++--
 drivers/scsi/scsi_transport_srp.c |  5 -
 2 files changed, 18 insertions(+), 7 deletions(-)

-- 
1.8.5.6

Re: [PATCHv2 2/4] scsi: Export blacklist flags to sysfs

[Hannes Reinecke]

On 08/11/2017 02:43 AM, Martin K. Petersen wrote:
> 
> Hannes,
> 
>> Each scsi device is scanned according to the found blacklist flags,
>> but this information is never presented to sysfs.  This makes it quite
>> hard to figure out if blacklisting worked as expected.  With this
>> patch we're exporting an additional attribute 'blacklist' containing
>> the blacklist flags for this device.
> 
> There have been changes to the blacklist values over the years so I'm
> not so keen on exporting them as a numbers.
> 
Ah, good to know.
I was thinking of exporting them as text; 'tis better for a quick check
anyway.
Would that be acceptable?

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 RESEND 0/6] hpsa: support legacy boards

[Hannes Reinecke]

On 08/10/2017 09:15 PM, Don Brace wrote:
>> -Original Message-
>> From: Hannes Reinecke [mailto:h...@suse.de]
>> Sent: Thursday, August 10, 2017 9:11 AM
>> To: James Bottomley ;
>> Christoph Hellwig 
>> Cc: Don Brace ; Martin K. Petersen
>> ; Meelis Roos ; linux-
>> s...@vger.kernel.org
>> Subject: Re: [PATCH RESEND 0/6] hpsa: support legacy boards
>>
>> EXTERNAL EMAIL
>>
>>
>> On 08/10/2017 04:06 PM, James Bottomley wrote:
>>> On Thu, 2017-08-10 at 09:09 +0200, Christoph Hellwig wrote:
 No device support in Linux is unsupported, sorry.  I think we're
 getting into the corporate bullshit game a little too much here.
>>>
>>> I think there are two different definitions of supported here.  To us,
>>> any device to which the driver attaches is "supported".  However, if
>>> it's never been tested before it may not work very well.  In the Linux
>>> way, we'll try to fix the bugs when they're reported and in that sense
>>> we support the device until nothing in the kernel attaches to its ids
>>> anymore.
>>>
>>> In the corporate world "supported" means we'll sell you a contract
>>> giving you certain rights to report bugs and have us fix them.  There
>>> are definite reasons why corporations only support a small range of new
>>> devices, even though devices not on this list may still be attached to
>>> by the driver and thus we (Linux Community) would try to fix the bug
>>> reports for.
>>>
>>> I think what you're basically asking for is a different name for the
>>> flag, which is fine?  how about 'legacy' instead?
>>>
>> Sure, no problem with that.
>>
>> Don?
>>
>> 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)
> 
> Ok, but to clarify...
>  * Will the cciss driver be removed when these patches are applied? Otherwise
> what will prevent the cciss driver from loading over these devices?
> i.e. how often will users need to be reminded to add 
> cciss.cciss_allow_hpsa flag?
> 
> 
The idea is to remove the cciss driver completely once these changes are
in. Preferably with the same patchset to avoid any timing issues.

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)