Re: [dm-devel] split scsi passthrough fields out of struct request V2

2017-01-26 Thread Jens Axboe 
On 01/26/2017 06:22 PM, Jens Axboe wrote:
> On 01/26/2017 06:15 PM, Bart Van Assche wrote:
>> On Thu, 2017-01-26 at 17:41 -0700, Jens Axboe wrote:
>>> On 01/26/2017 05:38 PM, Bart Van Assche wrote:
 I see similar behavior with the blk-mq-sched branch of
 git://git.kernel.dk/linux-block.git (git commit ID 0efe27068ecf):
 booting happens much slower than usual and I/O hangs if I run the
 srp-test software.
>>>
>>> Please don't run that, run for-4.11/block and merge it to master.
>>> Same behavior?
>>
>> I have not yet had the chance to run the srp-test software against that
>> kernel. But I already see that booting takes more than ten times longer
>> than usual. Note: as far as I know the dm-mpath driver is not involved
>> in the boot process of my test system.
> 
> What's your boot device? I've been booting this on a variety of setups,
> no problems observed. It's booting my laptop, and on SCSI and SATA as
> well. What is your root drive? What is the queue depth of it?
> Controller?

Are you using dm for your root device?

I think I see what is going on. The scheduler framework put the
insertion of flushes on the side, whereas it's integrated "nicely"
on the legacy side.

Can you try with this applied? This is on top of the previous two that
we already went through. Or, you can just pull:

git://git.kernel.dk/linux-block for-4.11/next

which is for-4.11/block with the next set of fixes on top that I haven't
pulled in yet.


commit 995447bfd14dd871e0c8771261ed7d1f2b5b4c86
Author: Jens Axboe 
Date:   Thu Jan 26 23:34:56 2017 -0700

blk-mq-sched: integrate flush insertion into blk_mq_sched_get_request()

Instead of letting the caller check this and handle the details
of inserting a flush request, put the logic in the scheduler
insertion function.

Outside of cleaning up the code, this handles the case where
outside callers insert a flush, like through
blk_insert_cloned_request().

Signed-off-by: Jens Axboe 

diff --git a/block/blk-core.c b/block/blk-core.c
index a61f1407f4f6..78daf5b6d7cb 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -2129,7 +2129,7 @@ int blk_insert_cloned_request(struct request_queue *q, 
struct request *rq)
if (q->mq_ops) {
if (blk_queue_io_stat(q))
blk_account_io_start(rq, true);
-   blk_mq_sched_insert_request(rq, false, true, false);
+   blk_mq_sched_insert_request(rq, false, true, false, false);
return 0;
}
 
diff --git a/block/blk-exec.c b/block/blk-exec.c
index 86656fdfa637..ed1f10165268 100644
--- a/block/blk-exec.c
+++ b/block/blk-exec.c
@@ -66,7 +66,7 @@ void blk_execute_rq_nowait(struct request_queue *q, struct 
gendisk *bd_disk,
 * be reused after dying flag is set
 */
if (q->mq_ops) {
-   blk_mq_sched_insert_request(rq, at_head, true, false);
+   blk_mq_sched_insert_request(rq, at_head, true, false, false);
return;
}
 
diff --git a/block/blk-flush.c b/block/blk-flush.c
index d7de34ee39c2..4427896641ac 100644
--- a/block/blk-flush.c
+++ b/block/blk-flush.c
@@ -456,7 +456,7 @@ void blk_insert_flush(struct request *rq)
if ((policy & REQ_FSEQ_DATA) &&
!(policy & (REQ_FSEQ_PREFLUSH | REQ_FSEQ_POSTFLUSH))) {
if (q->mq_ops)
-   blk_mq_sched_insert_request(rq, false, true, false);
+   blk_mq_sched_insert_request(rq, false, true, false, 
false);
else
list_add_tail(>queuelist, >queue_head);
return;
diff --git a/block/blk-mq-sched.c b/block/blk-mq-sched.c
index c27613de80c5..fa2ff0f458fa 100644
--- a/block/blk-mq-sched.c
+++ b/block/blk-mq-sched.c
@@ -336,6 +336,64 @@ void blk_mq_sched_restart_queues(struct blk_mq_hw_ctx 
*hctx)
}
 }
 
+/*
+ * Add flush/fua to the queue. If we fail getting a driver tag, then
+ * punt to the requeue list. Requeue will re-invoke us from a context
+ * that's safe to block from.
+ */
+static void blk_mq_sched_insert_flush(struct blk_mq_hw_ctx *hctx,
+ struct request *rq, bool can_block)
+{
+   if (blk_mq_get_driver_tag(rq, , can_block)) {
+   blk_insert_flush(rq);
+   blk_mq_run_hw_queue(hctx, !can_block);
+   } else
+   blk_mq_add_to_requeue_list(rq, true, true);
+}
+
+void blk_mq_sched_insert_request(struct request *rq, bool at_head,
+bool run_queue, bool async, bool can_block)
+{
+   struct request_queue *q = rq->q;
+   struct elevator_queue *e = q->elevator;
+   struct blk_mq_ctx *ctx = rq->mq_ctx;
+   struct blk_mq_hw_ctx *hctx = blk_mq_map_queue(q, ctx->cpu);
+
+   if (rq->tag == -1 && (rq->cmd_flags & (REQ_PREFLUSH | REQ_FUA))) {
+   blk_mq_sched_insert_flush(hctx, rq, can_block);
+   return;
+

Re: [dm-devel] split scsi passthrough fields out of struct request V2

2017-01-26 Thread Jens Axboe 
On 01/26/2017 06:15 PM, Bart Van Assche wrote:
> On Thu, 2017-01-26 at 17:41 -0700, Jens Axboe wrote:
>> On 01/26/2017 05:38 PM, Bart Van Assche wrote:
>>> I see similar behavior with the blk-mq-sched branch of
>>> git://git.kernel.dk/linux-block.git (git commit ID 0efe27068ecf):
>>> booting happens much slower than usual and I/O hangs if I run the
>>> srp-test software.
>>
>> Please don't run that, run for-4.11/block and merge it to master.
>> Same behavior?
> 
> I have not yet had the chance to run the srp-test software against that
> kernel. But I already see that booting takes more than ten times longer
> than usual. Note: as far as I know the dm-mpath driver is not involved
> in the boot process of my test system.

What's your boot device? I've been booting this on a variety of setups,
no problems observed. It's booting my laptop, and on SCSI and SATA as
well. What is your root drive? What is the queue depth of it?
Controller?

-- 
Jens Axboe

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [dm-devel] split scsi passthrough fields out of struct request V2

2017-01-26 Thread Bart Van Assche 
On Thu, 2017-01-26 at 17:41 -0700, Jens Axboe wrote:
> On 01/26/2017 05:38 PM, Bart Van Assche wrote:
> > I see similar behavior with the blk-mq-sched branch of
> > git://git.kernel.dk/linux-block.git (git commit ID 0efe27068ecf):
> > booting happens much slower than usual and I/O hangs if I run the
> > srp-test software.
> 
> Please don't run that, run for-4.11/block and merge it to master.
> Same behavior?

I have not yet had the chance to run the srp-test software against that
kernel. But I already see that booting takes more than ten times longer
than usual. Note: as far as I know the dm-mpath driver is not involved
in the boot process of my test system.

> > Regarding creating a similar dm setup: I hope that in the future it
> > will become possible to run the srp-test software without any special
> > hardware and with in-tree drivers. Today running the srp-test software
> > with in-tree drivers namely requires IB hardware. This is how to run the
> > srp-test software today with in-tree drivers:
> > * Find a system with at least two InfiniBand ports.
> > * Make sure that the appropriate IB driver in the kernel is enabled and
> >   also that LIO (CONFIG_TARGET_CORE=m and CONFIG_TCM_FILEIO=m), ib_srp,
> >   ib_srpt and dm-mpath are built as kernel modules.
> > * If none of the IB ports are connected to an IB switch, connect the
> >   two ports to each other and configure and start the opensm software
> >   such that the port states change from "Initializing" to "Active".
> > * Check with "ibstat | grep State: Active" that at least one port is
> >   in the active state.
> > * Configure multipathd as explained in
> >   https://github.com/bvanassche/srp-test/blob/master/README.md.
> > * Restart multipathd to make sure it picks up /etc/multipath.conf.
> > * Clone https://github.com/bvanassche/srp-test and start it as follows:
> >   srp-test/run_tests -t 02-mq
> 
> I can't run that. Any chance of a test case that doesn't require IB?

It is possible to run that test on top of the SoftRoCE driver. I will first
check myself whether the latest version of the SoftRoCE driver is stable
enough to run srp-test on top of it (see also
https://github.com/dledford/linux/commits/k.o/for-4.11).

Bart.--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [dm-devel] split scsi passthrough fields out of struct request V2

2017-01-26 Thread Jens Axboe 
On 01/26/2017 05:38 PM, Bart Van Assche wrote:
> On Thu, 2017-01-26 at 16:50 -0700, Jens Axboe wrote:
>> Clearly we are missing some requests. How do I setup dm similarly to
>> you?
>>
>> Does it reproduce without Christoph's patchset?
> 
> Hello Jens,
> 
> I see similar behavior with the blk-mq-sched branch of
> git://git.kernel.dk/linux-block.git (git commit ID 0efe27068ecf):
> booting happens much slower than usual and I/O hangs if I run the
> srp-test software.

Please don't run that, run for-4.11/block and merge it to master.
Same behavior?

> Regarding creating a similar dm setup: I hope that in the future it
> will become possible to run the srp-test software without any special
> hardware and with in-tree drivers. Today running the srp-test software
> with in-tree drivers namely requires IB hardware. This is how to run the
> srp-test software today with in-tree drivers:
> * Find a system with at least two InfiniBand ports.
> * Make sure that the appropriate IB driver in the kernel is enabled and
>   also that LIO (CONFIG_TARGET_CORE=m and CONFIG_TCM_FILEIO=m), ib_srp,
>   ib_srpt and dm-mpath are built as kernel modules.
> * If none of the IB ports are connected to an IB switch, connect the
>   two ports to each other and configure and start the opensm software
>   such that the port states change from "Initializing" to "Active".
> * Check with "ibstat | grep State: Active" that at least one port is
>   in the active state.
> * Configure multipathd as explained in
>   https://github.com/bvanassche/srp-test/blob/master/README.md.
> * Restart multipathd to make sure it picks up /etc/multipath.conf.
> * Clone https://github.com/bvanassche/srp-test and start it as follows:
>   srp-test/run_tests -t 02-mq

I can't run that. Any chance of a test case that doesn't require IB?

-- 
Jens Axboe

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [dm-devel] split scsi passthrough fields out of struct request V2

2017-01-26 Thread Bart Van Assche 
On Thu, 2017-01-26 at 16:50 -0700, Jens Axboe wrote:
> Clearly we are missing some requests. How do I setup dm similarly to
> you?
> 
> Does it reproduce without Christoph's patchset?

Hello Jens,

I see similar behavior with the blk-mq-sched branch of
git://git.kernel.dk/linux-block.git (git commit ID 0efe27068ecf):
booting happens much slower than usual and I/O hangs if I run the
srp-test software.

Regarding creating a similar dm setup: I hope that in the future it
will become possible to run the srp-test software without any special
hardware and with in-tree drivers. Today running the srp-test software
with in-tree drivers namely requires IB hardware. This is how to run the
srp-test software today with in-tree drivers:
* Find a system with at least two InfiniBand ports.
* Make sure that the appropriate IB driver in the kernel is enabled and
  also that LIO (CONFIG_TARGET_CORE=m and CONFIG_TCM_FILEIO=m), ib_srp,
  ib_srpt and dm-mpath are built as kernel modules.
* If none of the IB ports are connected to an IB switch, connect the
  two ports to each other and configure and start the opensm software
  such that the port states change from "Initializing" to "Active".
* Check with "ibstat | grep State: Active" that at least one port is
  in the active state.
* Configure multipathd as explained in
  https://github.com/bvanassche/srp-test/blob/master/README.md.
* Restart multipathd to make sure it picks up /etc/multipath.conf.
* Clone https://github.com/bvanassche/srp-test and start it as follows:
  srp-test/run_tests -t 02-mq

Bart.--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [dm-devel] split scsi passthrough fields out of struct request V2

2017-01-26 Thread Jens Axboe 
On 01/26/2017 04:50 PM, Jens Axboe wrote:
> On 01/26/2017 04:47 PM, Bart Van Assche wrote:
>> On Thu, 2017-01-26 at 16:26 -0700, Jens Axboe wrote:
>>> What device is stuck? Is it running with an mq scheduler attached, or
>>> with "none"?
>>>
>>> Would also be great to see the output of /sys/block/*/mq/*/tags and
>>> sched_tags so we can see if they have anything pending.
>>>
>>> From a quick look at the below, it looks like a request leak. Bisection
>>> would most likely be very helpful.
>>
>> Hello Jens,
>>
>> This happens with and without scheduler attached. The most recent test I ran
>> was with the deadline scheduler configured as default scheduler for all 
>> blk-mq
>> devices (CONFIG_DEFAULT_SQ_IOSCHED="mq-deadline" and
>> CONFIG_DEFAULT_MQ_IOSCHED="mq-deadline"). The block devices that hang are
>> /dev/dm-0 and /dev/dm-1. The tags and sched_tags data is as follows:
>>
>> # (cd /sys/class/block && grep -aH '' dm*/mq/*/tags)
>> dm-0/mq/0/tags:nr_tags=2048, reserved_tags=0, bits_per_word=64
>> dm-0/mq/0/tags:nr_free=1795, nr_reserved=0
>> dm-0/mq/0/tags:active_queues=0
>> dm-1/mq/0/tags:nr_tags=2048, reserved_tags=0, bits_per_word=64
>> dm-1/mq/0/tags:nr_free=2047, nr_reserved=0
>> dm-1/mq/0/tags:active_queues=0
>> # (cd /sys/class/block && grep -aH '' dm*/mq/*/sched_tags)
>> dm-0/mq/0/sched_tags:nr_tags=256, reserved_tags=0, bits_per_word=64
>> dm-0/mq/0/sched_tags:nr_free=0, nr_reserved=0
>> dm-0/mq/0/sched_tags:active_queues=0
>> dm-1/mq/0/sched_tags:nr_tags=256, reserved_tags=0, bits_per_word=64
>> dm-1/mq/0/sched_tags:nr_free=254, nr_reserved=0
>> dm-1/mq/0/sched_tags:active_queues=0
> 
> Clearly we are missing some requests. How do I setup dm similarly to
> you?
> 
> Does it reproduce without Christoph's patchset?

I have dm-mpath running using blk_mq and with mq-deadline on both dm and
the lower level device, and it seems to be running just fine here.
Note, this is without Christoph's patchset, I'll try that next once
xfstest finishes.

-- 
Jens Axboe

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [dm-devel] split scsi passthrough fields out of struct request V2

2017-01-26 Thread Jens Axboe 
On 01/26/2017 04:47 PM, Bart Van Assche wrote:
> On Thu, 2017-01-26 at 16:26 -0700, Jens Axboe wrote:
>> What device is stuck? Is it running with an mq scheduler attached, or
>> with "none"?
>>
>> Would also be great to see the output of /sys/block/*/mq/*/tags and
>> sched_tags so we can see if they have anything pending.
>>
>> From a quick look at the below, it looks like a request leak. Bisection
>> would most likely be very helpful.
> 
> Hello Jens,
> 
> This happens with and without scheduler attached. The most recent test I ran
> was with the deadline scheduler configured as default scheduler for all blk-mq
> devices (CONFIG_DEFAULT_SQ_IOSCHED="mq-deadline" and
> CONFIG_DEFAULT_MQ_IOSCHED="mq-deadline"). The block devices that hang are
> /dev/dm-0 and /dev/dm-1. The tags and sched_tags data is as follows:
> 
> # (cd /sys/class/block && grep -aH '' dm*/mq/*/tags)
> dm-0/mq/0/tags:nr_tags=2048, reserved_tags=0, bits_per_word=64
> dm-0/mq/0/tags:nr_free=1795, nr_reserved=0
> dm-0/mq/0/tags:active_queues=0
> dm-1/mq/0/tags:nr_tags=2048, reserved_tags=0, bits_per_word=64
> dm-1/mq/0/tags:nr_free=2047, nr_reserved=0
> dm-1/mq/0/tags:active_queues=0
> # (cd /sys/class/block && grep -aH '' dm*/mq/*/sched_tags)
> dm-0/mq/0/sched_tags:nr_tags=256, reserved_tags=0, bits_per_word=64
> dm-0/mq/0/sched_tags:nr_free=0, nr_reserved=0
> dm-0/mq/0/sched_tags:active_queues=0
> dm-1/mq/0/sched_tags:nr_tags=256, reserved_tags=0, bits_per_word=64
> dm-1/mq/0/sched_tags:nr_free=254, nr_reserved=0
> dm-1/mq/0/sched_tags:active_queues=0

Clearly we are missing some requests. How do I setup dm similarly to
you?

Does it reproduce without Christoph's patchset?

-- 
Jens Axboe

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [dm-devel] split scsi passthrough fields out of struct request V2

2017-01-26 Thread Bart Van Assche 
On Thu, 2017-01-26 at 16:26 -0700, Jens Axboe wrote:
> What device is stuck? Is it running with an mq scheduler attached, or
> with "none"?
> 
> Would also be great to see the output of /sys/block/*/mq/*/tags and
> sched_tags so we can see if they have anything pending.
> 
> From a quick look at the below, it looks like a request leak. Bisection
> would most likely be very helpful.

Hello Jens,

This happens with and without scheduler attached. The most recent test I ran
was with the deadline scheduler configured as default scheduler for all blk-mq
devices (CONFIG_DEFAULT_SQ_IOSCHED="mq-deadline" and
CONFIG_DEFAULT_MQ_IOSCHED="mq-deadline"). The block devices that hang are
/dev/dm-0 and /dev/dm-1. The tags and sched_tags data is as follows:

# (cd /sys/class/block && grep -aH '' dm*/mq/*/tags)
dm-0/mq/0/tags:nr_tags=2048, reserved_tags=0, bits_per_word=64
dm-0/mq/0/tags:nr_free=1795, nr_reserved=0
dm-0/mq/0/tags:active_queues=0
dm-1/mq/0/tags:nr_tags=2048, reserved_tags=0, bits_per_word=64
dm-1/mq/0/tags:nr_free=2047, nr_reserved=0
dm-1/mq/0/tags:active_queues=0
# (cd /sys/class/block && grep -aH '' dm*/mq/*/sched_tags)
dm-0/mq/0/sched_tags:nr_tags=256, reserved_tags=0, bits_per_word=64
dm-0/mq/0/sched_tags:nr_free=0, nr_reserved=0
dm-0/mq/0/sched_tags:active_queues=0
dm-1/mq/0/sched_tags:nr_tags=256, reserved_tags=0, bits_per_word=64
dm-1/mq/0/sched_tags:nr_free=254, nr_reserved=0
dm-1/mq/0/sched_tags:active_queues=0

Bart.--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 04/18] block: simplify blk_init_allocated_queue

2017-01-26 Thread Bart Van Assche 
On Wed, 2017-01-25 at 18:25 +0100, Christoph Hellwig wrote:
> Return an errno value instead of the passed in queue so that the callers
> don't have to keep track of two queues, and move the assignment of the
> request_fn and lock to the caller as passing them as argument doesn't
> simplify anything.  While we're at it also remove two pointless NULL
> assignments, given that the request structure is zeroed on allocation.

Reviewed-by: Bart Van Assche --
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [dm-devel] split scsi passthrough fields out of struct request V2

2017-01-26 Thread Jens Axboe 
On 01/26/2017 04:14 PM, Bart Van Assche wrote:
> On Thu, 2017-01-26 at 14:51 -0700, Jens Axboe wrote:
>> That is exactly what it means, looks like that one path doesn't handle
>> that.  You'd have to exhaust the pool with atomic allocs for this to
>> trigger, we don't do that at all in the normal IO path. So good catch,
>> must be the dm part that enables this since it does NOWAIT allocations.
>>
>>
>> diff --git a/block/blk-mq-sched.c b/block/blk-mq-sched.c
>> index 3136696f4991..c27613de80c5 100644
>> --- a/block/blk-mq-sched.c
>> +++ b/block/blk-mq-sched.c
>> @@ -134,7 +134,8 @@ struct request *blk_mq_sched_get_request(struct 
>> request_queue *q,
>>  rq = __blk_mq_alloc_request(data, op);
>>  } else {
>>  rq = __blk_mq_alloc_request(data, op);
>> -data->hctx->tags->rqs[rq->tag] = rq;
>> +if (rq)
>> +data->hctx->tags->rqs[rq->tag] = rq;
>>  }
>>  
>>  if (rq) {
> 
> Hello Jens,
> 
> With these two patches applied the scheduling-while-atomic complaint and
> the oops are gone. However, some tasks get stuck. Is the console output
> below enough to figure out what is going on or do you want me to bisect
> this? I don't think that any requests got stuck since no pending requests
> are shown in /sys/block/*/mq/*/{pending,*/rq_list}.

What device is stuck? Is it running with an mq scheduler attached, or
with "none"?

Would also be great to see the output of /sys/block/*/mq/*/tags and
sched_tags so we can see if they have anything pending.

>From a quick look at the below, it looks like a request leak. Bisection
would most likely be very helpful.

-- 
Jens Axboe

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 03/18] block: fix elevator init check

2017-01-26 Thread Bart Van Assche 
On Wed, 2017-01-25 at 18:25 +0100, Christoph Hellwig wrote:
> We can't initalize the elevator fields for flushes as flush share space
> in struct request with the elevator data.  But currently we can't
> commnicate that a request is a flush through blk_get_request as we
> can only pass READ or WRITE, and the low-level code looks at the
> possible NULL bio to check for a flush.
> 
> Fix this by allowing to pass any block op and flags, and by checking for
> the flush flags in __get_request.

Reviewed-by: Bart Van Assche --
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 02/18] md: cleanup bio op / flags handling in raid1_write_request

2017-01-26 Thread Bart Van Assche 
On Wed, 2017-01-25 at 18:25 +0100, Christoph Hellwig wrote:
> No need for the local variables, the bio is still live and we can just
> assigned the bits we want directly.  Make me wonder why we can't assign
> all the bio flags to start with.

I assume that you ment "assign" in the patch description? Anyway:

Reviewed-by: Bart Van Assche --
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 01/18] block: add a op_is_flush helper

2017-01-26 Thread Bart Van Assche 
On Wed, 2017-01-25 at 18:25 +0100, Christoph Hellwig wrote:
> This centralizes the checks for bios that needs to be go into the flush
> state machine.

Reviewed-by: Bart Van Assche 
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/2] virtio_scsi: Implement fc_host

2017-01-26 Thread Michael S. Tsirkin 
On Thu, Jan 26, 2017 at 11:41:09AM +0800, Fam Zheng wrote:
> This implements the VIRTIO_SCSI_F_FC_HOST feature by reading the config
> fields and presenting them as sysfs fc_host attributes. The config
> change handler is added here because primary_active will toggle during
> migration.

Looks like there's active discussion on virtio tc mailing list.
It's ok to post patches meanwhile but best as RFC,
and repost after controversy is resolved.

> 
> Signed-off-by: Fam Zheng 
> ---
>  drivers/scsi/virtio_scsi.c | 60 
> +-
>  1 file changed, 59 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
> index ec91bd0..1bb330c 100644
> --- a/drivers/scsi/virtio_scsi.c
> +++ b/drivers/scsi/virtio_scsi.c
> @@ -28,6 +28,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  
>  #define VIRTIO_SCSI_MEMPOOL_SZ 64
> @@ -795,6 +796,15 @@ static struct scsi_host_template 
> virtscsi_host_template_multi = {
>   .track_queue_depth = 1,
>  };
>  
> +static struct fc_function_template virtscsi_fc_template = {
> + .show_host_node_name = 1,
> + .show_host_port_name = 1,
> + .show_host_port_type = 1,
> + .show_host_port_state = 1,
> +};
> +
> +static struct scsi_transport_template *virtscsi_fc_transport_template;
> +
>  #define virtscsi_config_get(vdev, fld) \
>   ({ \
>   typeof(((struct virtio_scsi_config *)0)->fld) __val; \
> @@ -956,15 +966,42 @@ static int virtscsi_init(struct virtio_device *vdev,
>   return err;
>  }
>  
> +static void virtscsi_update_fc_host_attrs(struct virtio_device *vdev)
> +{
> + struct Scsi_Host *shost = vdev->priv;
> + u8 node_name[8], port_name[8];
> +
> + if (virtscsi_config_get(vdev, primary_active)) {
> + virtio_cread_bytes(vdev,
> + offsetof(struct virtio_scsi_config, primary_wwnn),
> + _name, 8);
> + virtio_cread_bytes(vdev,
> + offsetof(struct virtio_scsi_config, primary_wwpn),
> + _name, 8);
> + } else {
> + virtio_cread_bytes(vdev,
> + offsetof(struct virtio_scsi_config, secondary_wwnn),
> + _name, 8);
> + virtio_cread_bytes(vdev,
> + offsetof(struct virtio_scsi_config, secondary_wwpn),
> + _name, 8);
> + }

This is racy, isn't it? You need to wrap this in a generation check
otherwise read can race with primary_active changing.
And you might want a wrapper to virtio_cread_bytes that does not
include generation check.

> + fc_host_node_name(shost) = wwn_to_u64(node_name);
> + fc_host_port_name(shost) = wwn_to_u64(port_name);
> + fc_host_port_type(shost) = FC_PORTTYPE_NPORT;
> + fc_host_port_state(shost) = FC_PORTSTATE_ONLINE;
> +}
> +
>  static int virtscsi_probe(struct virtio_device *vdev)
>  {
> - struct Scsi_Host *shost;
> + struct Scsi_Host *shost = NULL;
>   struct virtio_scsi *vscsi;
>   int err;
>   u32 sg_elems, num_targets;
>   u32 cmd_per_lun;
>   u32 num_queues;
>   struct scsi_host_template *hostt;
> + bool fc_host_enabled;
>  
>   if (!vdev->config->get) {
>   dev_err(>dev, "%s failure: config access disabled\n",
> @@ -987,6 +1024,9 @@ static int virtscsi_probe(struct virtio_device *vdev)
>   if (!shost)
>   return -ENOMEM;
>  
> + fc_host_enabled = virtio_has_feature(vdev, VIRTIO_SCSI_F_FC_HOST);
> + if (fc_host_enabled)
> + shost->transportt = virtscsi_fc_transport_template;
>   sg_elems = virtscsi_config_get(vdev, seg_max) ?: 1;
>   shost->sg_tablesize = sg_elems;
>   vscsi = shost_priv(shost);
> @@ -1032,6 +1072,9 @@ static int virtscsi_probe(struct virtio_device *vdev)
>   if (err)
>   goto scsi_add_host_failed;
>  
> + if (fc_host_enabled)
> + virtscsi_update_fc_host_attrs(vdev);
> +
>   virtio_device_ready(vdev);
>  
>   if (virtio_has_feature(vdev, VIRTIO_SCSI_F_HOTPLUG))
> @@ -1098,6 +1141,11 @@ static int virtscsi_restore(struct virtio_device *vdev)
>  }
>  #endif
>  
> +static void virtscsi_config_changed(struct virtio_device *vdev)
> +{

This is called unconditionally here, will access
invalid config fields if feature is off.
In fact this is wasting an MSIX vector when feature is not
negotiated. No easy way not to, but best document this
in a code comment.

> + virtscsi_update_fc_host_attrs(vdev);
> +}
> +
>  static struct virtio_device_id id_table[] = {
>   { VIRTIO_ID_SCSI, VIRTIO_DEV_ANY_ID },
>   { 0 },
> @@ -1109,6 +1157,7 @@ static unsigned int features[] = {
>  #ifdef CONFIG_BLK_DEV_INTEGRITY
>   VIRTIO_SCSI_F_T10_PI,
>  #endif
> + VIRTIO_SCSI_F_FC_HOST,
>  };
>  
>  static struct virtio_driver virtio_scsi_driver = {
> @@ -1123,12 +1172,20 @@ static struct virtio_driver virtio_scsi_driver = {
>

Re: [dm-devel] split scsi passthrough fields out of struct request V2

2017-01-26 Thread Jens Axboe 
On 01/26/2017 02:47 PM, Bart Van Assche wrote:
> (gdb) list *(blk_mq_sched_get_request+0x310)
> 0x8132dcf0 is in blk_mq_sched_get_request (block/blk-mq-sched.c:136).
> 131 rq->rq_flags |= RQF_QUEUED;
> 132 } else
> 133 rq = __blk_mq_alloc_request(data, op);
> 134 } else {
> 135 rq = __blk_mq_alloc_request(data, op);
> 136 data->hctx->tags->rqs[rq->tag] = rq;
> 137 }
> 138
> 139 if (rq) {
> 140 if (!op_is_flush(op)) {
> 
> (gdb) disas blk_mq_sched_get_request
> [ ... ]
>0x8132dce3 <+771>:   callq  0x81324ab0 
> <__blk_mq_alloc_request>
>0x8132dce8 <+776>:   mov%rax,%rcx
>0x8132dceb <+779>:   mov0x18(%r12),%rax
>0x8132dcf0 <+784>:   movslq 0x5c(%rcx),%rdx
> [ ... ]
> (gdb) print &((struct request *)0)->tag
> $1 = (int *) 0x5c 
> 
> I think this means that rq == NULL and that a test for rq is missing after the
> __blk_mq_alloc_request() call?

That is exactly what it means, looks like that one path doesn't handle
that.  You'd have to exhaust the pool with atomic allocs for this to
trigger, we don't do that at all in the normal IO path. So good catch,
must be the dm part that enables this since it does NOWAIT allocations.


diff --git a/block/blk-mq-sched.c b/block/blk-mq-sched.c
index 3136696f4991..c27613de80c5 100644
--- a/block/blk-mq-sched.c
+++ b/block/blk-mq-sched.c
@@ -134,7 +134,8 @@ struct request *blk_mq_sched_get_request(struct 
request_queue *q,
rq = __blk_mq_alloc_request(data, op);
} else {
rq = __blk_mq_alloc_request(data, op);
-   data->hctx->tags->rqs[rq->tag] = rq;
+   if (rq)
+   data->hctx->tags->rqs[rq->tag] = rq;
}
 
if (rq) {

-- 
Jens Axboe

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/2] virtio_scsi: Add fc_host definitions

2017-01-26 Thread Michael S. Tsirkin 
On Thu, Jan 26, 2017 at 11:41:08AM +0800, Fam Zheng wrote:
> Signed-off-by: Fam Zheng 
> ---

I pefer combining this with implementation,
hard to reason about interface alone.

>  include/uapi/linux/virtio_scsi.h | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/include/uapi/linux/virtio_scsi.h 
> b/include/uapi/linux/virtio_scsi.h
> index cc18ef8..a26fb31 100644
> --- a/include/uapi/linux/virtio_scsi.h
> +++ b/include/uapi/linux/virtio_scsi.h
> @@ -113,6 +113,11 @@ struct virtio_scsi_config {
>   __u16 max_channel;
>   __u16 max_target;
>   __u32 max_lun;
> + __u8  primary_wwpn[8];
> + __u8  primary_wwnn[8];
> + __u8  secondary_wwpn[8];
> + __u8  secondary_wwnn[8];
> + __u8  primary_active;

Is this in fact a binary value?
Also pls add padding to align on 8 byte boundary.

>  } __attribute__((packed));
>  
>  /* Feature Bits */
> @@ -120,6 +125,7 @@ struct virtio_scsi_config {
>  #define VIRTIO_SCSI_F_HOTPLUG  1
>  #define VIRTIO_SCSI_F_CHANGE   2
>  #define VIRTIO_SCSI_F_T10_PI   3
> +#define VIRTIO_SCSI_F_FC_HOST  4
>  
>  /* Response codes */
>  #define VIRTIO_SCSI_S_OK   0
> -- 
> 2.9.3
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [dm-devel] split scsi passthrough fields out of struct request V2

2017-01-26 Thread Bart Van Assche 
On Thu, 2017-01-26 at 14:12 -0700, Jens Axboe wrote:
> On 01/26/2017 02:01 PM, Bart Van Assche wrote:
> > On Thu, 2017-01-26 at 13:54 -0700, Jens Axboe wrote:
> > > Your call path has blk_get_request() in it, I don't have
> > > that in my tree. Is it passing in the right mask?
> > 
> > Hello Jens,
> > 
> > There is only one blk_get_request() call in drivers/md/dm-mpath.c
> > and it looks as follows:
> > 
> > clone = blk_get_request(bdev_get_queue(bdev),
> > rq->cmd_flags | REQ_NOMERGE,
> > GFP_ATOMIC);
> 
> Yeah, I found it in the dm patch. Looks fine to me, since
> blk_mq_alloc_request() checks for __GFP_DIRECT_RECLAIM. Weird, it all
> looks fine to me. Are you sure you tested with the patch? Either that,
> or I'm smoking crack.

Hello Jens,

After I received your e-mail I noticed that there was a local
modification on the test system that was responsible for the schedule-
while-atomic complaint. Sorry for that. Anyway, I undid the merge with
the v4.10-rc5 code and repeated my test. This time the following call
stack appeared:

BUG: unable to handle kernel NULL pointer dereference at 005c
IP: blk_mq_sched_get_request+0x310/0x350
PGD 34bd9c067 
PUD 346b37067 
PMD 0 

Oops:  [#1] SMP
Modules linked in: dm_service_time ib_srp scsi_transport_srp target_core_user 
uio target_core_pscsi target_core_file ib_srpt target_core_iblock 
target_core_mod brd netconsole xt_CHECKSUM iptable_mangle ipt_MASQUERADE 
nf_nat_masquerade_ipv4 iptable_nat nf_nat_ipv4 nf_nat libcrc32c 
nf_conntrack_ipv4 nf_defrag_ipv4 xt_conntrack nf_conntrack ipt_REJECT 
nf_reject_ipv4 xt_tcpudp tun bridge stp llc ebtable_filter ebtables 
ip6table_filter ip6_tables iptable_filter ip_tables x_tables af_packet ib_ipoib 
rdma_ucm ib_ucm ib_uverbs ib_umad rdma_cm configfs ib_cm iw_cm msr mlx4_ib 
ib_core sb_edac edac_core x86_pkg_temp_thermal intel_powerclamp coretemp 
ipmi_ssif kvm_intel kvm irqbypass crct10dif_pclmul crc32_pclmul mlx4_core 
crc32c_intel ghash_clmulni_intel pcbc aesni_intel aes_x86_64 tg3 iTCO_wdt 
crypto_simd dcdbas iTCO_vendor_support ptp glue_helper ipmi_si cryptd 
ipmi_devintf pps_core fjes devlink ipmi_msghandler pcspkr libphy tpm_tis 
tpm_tis_core tpm button mei_me lpc_ich wmi mei mfd_core shpchp hid_generic 
usbhid mgag200 i2c_algo_bit drm_kms_helper syscopyarea sysfillrect sysimgblt 
fb_sys_fops ttm sr_mod drm cdrom ehci_pci ehci_hcd usbcore usb_common sg 
dm_multipath dm_mod scsi_dh_rdac scsi_dh_emc scsi_dh_alua autofs4
CPU: 0 PID: 9231 Comm: fio Not tainted 4.10.0-rc4-dbg+ #1
Hardware name: Dell Inc. PowerEdge R430/03XKDV, BIOS 1.0.2 11/17/2014
task: 88034c8c3140 task.stack: c90005698000
RIP: 0010:blk_mq_sched_get_request+0x310/0x350
RSP: 0018:c9000569bac8 EFLAGS: 00010246
RAX: 88034f430958 RBX: 88045ed2cef0 RCX: 
RDX: 001f RSI: 8803507bdcf8 RDI: 001f
RBP: c9000569bb00 R08: 0001 R09: 
R10: 0001 R11:  R12: c9000569bb18
R13: c801 R14:  R15: 
FS:  7f65ca054700() GS:88046f20() knlGS:
CS:  0010 DS:  ES:  CR0: 80050033
CR2: 005c CR3: 00034b0ed000 CR4: 001406f0
Call Trace:
 blk_mq_alloc_request+0x5e/0xb0
 blk_get_request+0x2f/0x110
 multipath_clone_and_map+0xcd/0x140 [dm_multipath]
 map_request+0x3c/0x290 [dm_mod]
 dm_mq_queue_rq+0x77/0x100 [dm_mod]
 blk_mq_dispatch_rq_list+0x1ff/0x320
 blk_mq_sched_dispatch_requests+0xa9/0xe0
 __blk_mq_run_hw_queue+0x122/0x1c0
 blk_mq_run_hw_queue+0x84/0x90
 blk_mq_flush_plug_list+0x39f/0x480
 blk_flush_plug_list+0xee/0x270
 blk_finish_plug+0x27/0x40
 do_io_submit+0x475/0x900
 SyS_io_submit+0xb/0x10
 entry_SYSCALL_64_fastpath+0x18/0xad
RIP: 0033:0x7f65e4d05787
RSP: 002b:7f65ca051948 EFLAGS: 0202 ORIG_RAX: 00d1
RAX: ffda RBX: 0046 RCX: 7f65e4d05787
RDX: 7f65a404f158 RSI: 0001 RDI: 7f65f6bfd000
RBP: 0815 R08: 0001 R09: 7f65a404e3e0
R10: 7f65a404 R11: 0202 R12: 06d0
R13: 7f65a404e930 R14: 1000 R15: 0830
Code: 67 ff ff ff e9 80 fe ff ff 48 89 df e8 ba c4 fe ff 31 c9 e9 60 ff ff ff 
44 89 ee 4c 89 e7 e8 c8 6d ff ff 48 89 c1 49 8b 44 24 18 <48> 63 51 5c 48 8b 80 
20 01 00 00 48 8b 80 80 00 00 00 48 89 0c 
RIP: blk_mq_sched_get_request+0x310/0x350 RSP: c9000569bac8
CR2: 005c

(gdb) list *(blk_mq_sched_get_request+0x310)
0x8132dcf0 is in blk_mq_sched_get_request (block/blk-mq-sched.c:136).
131 rq->rq_flags |= RQF_QUEUED;
132 } else
133 rq = __blk_mq_alloc_request(data, op);
134 } else {
135 rq = __blk_mq_alloc_request(data, op);
136 data->hctx->tags->rqs[rq->tag] = rq;
137 }

Re: [dm-devel] split scsi passthrough fields out of struct request V2

2017-01-26 Thread Jens Axboe 
On 01/26/2017 02:01 PM, Bart Van Assche wrote:
> On Thu, 2017-01-26 at 13:54 -0700, Jens Axboe wrote:
>> Your call path has blk_get_request() in it, I don't have
>> that in my tree. Is it passing in the right mask?
> 
> Hello Jens,
> 
> There is only one blk_get_request() call in drivers/md/dm-mpath.c
> and it looks as follows:
> 
>   clone = blk_get_request(bdev_get_queue(bdev),
>   rq->cmd_flags | REQ_NOMERGE,
>   GFP_ATOMIC);

Yeah, I found it in the dm patch. Looks fine to me, since
blk_mq_alloc_request() checks for __GFP_DIRECT_RECLAIM. Weird, it all
looks fine to me. Are you sure you tested with the patch? Either that,
or I'm smoking crack.

-- 
Jens Axboe

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [dm-devel] split scsi passthrough fields out of struct request V2

2017-01-26 Thread Bart Van Assche 
On Thu, 2017-01-26 at 13:54 -0700, Jens Axboe wrote:
> Your call path has blk_get_request() in it, I don't have
> that in my tree. Is it passing in the right mask?

Hello Jens,

There is only one blk_get_request() call in drivers/md/dm-mpath.c
and it looks as follows:

clone = blk_get_request(bdev_get_queue(bdev),
rq->cmd_flags | REQ_NOMERGE,
GFP_ATOMIC);

Bart.--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH V2 05/24] aacraid: Retrieve and update the device types

2017-01-26 Thread Raghava Aditya Renukunta 


> -Original Message-
> From: linux-scsi-ow...@vger.kernel.org [mailto:linux-scsi-
> ow...@vger.kernel.org] On Behalf Of Raghava Aditya Renukunta
> Sent: Thursday, January 26, 2017 10:44 AM
> To: Johannes Thumshirn 
> Cc: j...@linux.vnet.ibm.com; martin.peter...@oracle.com; linux-
> s...@vger.kernel.org; Dave Carroll ; Gana
> Sridaran ; Scott Benesh
> 
> Subject: RE: [PATCH V2 05/24] aacraid: Retrieve and update the device types
> 
> EXTERNAL EMAIL
> 
> 
> > -Original Message-
> > From: Johannes Thumshirn [mailto:jthumsh...@suse.de]
> > Sent: Thursday, January 26, 2017 1:00 AM
> > To: Raghava Aditya Renukunta
> > 
> > Cc: j...@linux.vnet.ibm.com; martin.peter...@oracle.com; linux-
> > s...@vger.kernel.org; Dave Carroll ; Gana
> > Sridaran ; Scott Benesh
> > 
> > Subject: Re: [PATCH V2 05/24] aacraid: Retrieve and update the device
> types
> >
> > EXTERNAL EMAIL
> >
> >
> > On Wed, Jan 25, 2017 at 10:00:52AM -0800, Raghava Aditya Renukunta
> wrote:
> > > This patch adds support to retrieve the type of each adapter connected
> > > device. Applicable to HBA1000 and SmartIOC2000 products
> > >
> > > Signed-off-by: Raghava Aditya Renukunta
> > 
> > > Signed-off-by: Dave Carroll 
> > >
> > > ---
> >
> > [...]
> >
> > >  /*
> > >   *   Adapter Information Block
> > >   *
> > > @@ -1056,7 +1091,28 @@ struct aac_supplement_adapter_info
> > >   /* StructExpansion == 1 */
> > >   __le32  FeatureBits3;
> > >   __le32  SupportedPerformanceModes;
> > > - __le32  ReservedForFutureGrowth[80];
> > > + u8  HostBusType;/* uses HOST_BUS_TYPE_xxx defines */
> > > + u8  HostBusWidth;   /* actual width in bits or links */
> > > + u16 HostBusSpeed;   /* actual bus speed/link rate in 
> > > MHz */
> > > + u8  MaxRRCDrives;   /* max. number of ITP-RRC 
> > > drives/pool */
> > > + u8  MaxDiskXtasks;  /* max. possible num of DiskX Tasks 
> > > */
> > > +
> > > + u8  CpldVerLoaded;
> > > + u8  CpldVerInFlash;
> > > +
> > > + __le64  MaxRRCCapacity;
> > > + __le32  CompiledMaxHistLogLevel;
> > > + u8  CustomBoardName[12];
> > > + u16 SupportedCntlrMode; /* identify supported controller 
> > > mode
> */
> > > + u16 ReservedForFuture16;
> > > + __le32  SupportedOptions3;  /* reserved for future options */
> > > +
> > > + __le16  VirtDeviceBus;  /* virt. SCSI device for Thor */
> > > + __le16  VirtDeviceTarget;
> > > + __le16  VirtDeviceLUN;
> > > + __le16  Unused;
> > > + __le32  ReservedForFutureGrowth[68];
> > > +
> 
> Same here.

On second thought changing all of the variables here will open up 
Pandoras  box. I will leave them as it is for now  and change the whole
 structure and anything attached it in one of my next patch submission series.

Will that be ok?
 

> > Appart from that,
> > 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
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [dm-devel] split scsi passthrough fields out of struct request V2

2017-01-26 Thread Jens Axboe 
On 01/26/2017 01:47 PM, Bart Van Assche wrote:
> On 01/26/2017 11:01 AM, Jens Axboe wrote:
>> On 01/26/2017 11:59 AM, h...@lst.de wrote:
>>> On Thu, Jan 26, 2017 at 11:57:36AM -0700, Jens Axboe wrote:
 It's against my for-4.11/block, which you were running under Christoph's
 patches. Maybe he's using an older version? In any case, should be
 pretty trivial for you to hand apply. Just ensure that .flags is set to
 0 for the common cases, and inherit 'flags' when it is passed in.
>>>
>>> No, the flush op cleanups you asked for last round create a conflict
>>> with your patch.  They should be trivial to fix, though.
>>
>> Ah, makes sense. And yes, as I said, should be trivial to hand apply the
>> hunk that does fail.
> 
> Hello Jens and Christoph,
> 
> With the below patch applied the test got a little further but did not
> pass unfortunately. I tried to analyze the new call stack but it's not yet
> clear to me what is going on.
>  
> The patch I had applied on Christoph's tree:
> 
> ---
>  block/blk-mq-sched.c | 2 +-
>  block/blk-mq.c   | 6 +++---
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/block/blk-mq-sched.c b/block/blk-mq-sched.c
> index 3bd66e50ec84..7c9318755fab 100644
> --- a/block/blk-mq-sched.c
> +++ b/block/blk-mq-sched.c
> @@ -116,7 +116,7 @@ struct request *blk_mq_sched_get_request(struct 
> request_queue *q,
>   ctx = blk_mq_get_ctx(q);
>   hctx = blk_mq_map_queue(q, ctx->cpu);
>  
> - blk_mq_set_alloc_data(data, q, 0, ctx, hctx);
> + blk_mq_set_alloc_data(data, q, data->flags, ctx, hctx);
>  
>   if (e) {
>   data->flags |= BLK_MQ_REQ_INTERNAL;
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index 83640869d9e4..6697626e5d32 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -248,7 +248,7 @@ EXPORT_SYMBOL_GPL(__blk_mq_alloc_request);
>  struct request *blk_mq_alloc_request(struct request_queue *q, int rw,
>   unsigned int flags)
>  {
> - struct blk_mq_alloc_data alloc_data;
> + struct blk_mq_alloc_data alloc_data = { .flags = flags };
>   struct request *rq;
>   int ret;
>  
> @@ -1369,7 +1369,7 @@ static blk_qc_t blk_mq_make_request(struct 
> request_queue *q, struct bio *bio)
>  {
>   const int is_sync = op_is_sync(bio->bi_opf);
>   const int is_flush_fua = op_is_flush(bio->bi_opf);
> - struct blk_mq_alloc_data data;
> + struct blk_mq_alloc_data data = { };
>   struct request *rq;
>   unsigned int request_count = 0, srcu_idx;
>   struct blk_plug *plug;
> @@ -1491,7 +1491,7 @@ static blk_qc_t blk_sq_make_request(struct 
> request_queue *q, struct bio *bio)
>   const int is_flush_fua = op_is_flush(bio->bi_opf);
>   struct blk_plug *plug;
>   unsigned int request_count = 0;
> - struct blk_mq_alloc_data data;
> + struct blk_mq_alloc_data data = { };
>   struct request *rq;
>   blk_qc_t cookie;
>   unsigned int wb_acct;

Looks correct to me. Your call path has blk_get_request() in it, I don't have
that in my tree. Is it passing in the right mask?

-- 
Jens Axboe

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [dm-devel] split scsi passthrough fields out of struct request V2

2017-01-26 Thread Bart Van Assche 
On 01/26/2017 11:01 AM, Jens Axboe wrote:
> On 01/26/2017 11:59 AM, h...@lst.de wrote:
>> On Thu, Jan 26, 2017 at 11:57:36AM -0700, Jens Axboe wrote:
>>> It's against my for-4.11/block, which you were running under Christoph's
>>> patches. Maybe he's using an older version? In any case, should be
>>> pretty trivial for you to hand apply. Just ensure that .flags is set to
>>> 0 for the common cases, and inherit 'flags' when it is passed in.
>>
>> No, the flush op cleanups you asked for last round create a conflict
>> with your patch.  They should be trivial to fix, though.
> 
> Ah, makes sense. And yes, as I said, should be trivial to hand apply the
> hunk that does fail.

Hello Jens and Christoph,

With the below patch applied the test got a little further but did not
pass unfortunately. I tried to analyze the new call stack but it's not yet
clear to me what is going on.
 
The patch I had applied on Christoph's tree:

---
 block/blk-mq-sched.c | 2 +-
 block/blk-mq.c   | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/block/blk-mq-sched.c b/block/blk-mq-sched.c
index 3bd66e50ec84..7c9318755fab 100644
--- a/block/blk-mq-sched.c
+++ b/block/blk-mq-sched.c
@@ -116,7 +116,7 @@ struct request *blk_mq_sched_get_request(struct 
request_queue *q,
ctx = blk_mq_get_ctx(q);
hctx = blk_mq_map_queue(q, ctx->cpu);
 
-   blk_mq_set_alloc_data(data, q, 0, ctx, hctx);
+   blk_mq_set_alloc_data(data, q, data->flags, ctx, hctx);
 
if (e) {
data->flags |= BLK_MQ_REQ_INTERNAL;
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 83640869d9e4..6697626e5d32 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -248,7 +248,7 @@ EXPORT_SYMBOL_GPL(__blk_mq_alloc_request);
 struct request *blk_mq_alloc_request(struct request_queue *q, int rw,
unsigned int flags)
 {
-   struct blk_mq_alloc_data alloc_data;
+   struct blk_mq_alloc_data alloc_data = { .flags = flags };
struct request *rq;
int ret;
 
@@ -1369,7 +1369,7 @@ static blk_qc_t blk_mq_make_request(struct request_queue 
*q, struct bio *bio)
 {
const int is_sync = op_is_sync(bio->bi_opf);
const int is_flush_fua = op_is_flush(bio->bi_opf);
-   struct blk_mq_alloc_data data;
+   struct blk_mq_alloc_data data = { };
struct request *rq;
unsigned int request_count = 0, srcu_idx;
struct blk_plug *plug;
@@ -1491,7 +1491,7 @@ static blk_qc_t blk_sq_make_request(struct request_queue 
*q, struct bio *bio)
const int is_flush_fua = op_is_flush(bio->bi_opf);
struct blk_plug *plug;
unsigned int request_count = 0;
-   struct blk_mq_alloc_data data;
+   struct blk_mq_alloc_data data = { };
struct request *rq;
blk_qc_t cookie;
unsigned int wb_acct;
-- 
2.11.0


The new call trace:

[ 4277.729785] BUG: scheduling while atomic: mount/9209/0x0004
[ 4277.729824] 2 locks held by mount/9209:
[ 4277.729846]  #0:  (>s_umount_key#25/1){+.+.+.}, at: 
[] sget_userns+0x2bd/0x500
[ 4277.729881]  #1:  (rcu_read_lock){..}, at: [] 
__blk_mq_run_hw_queue+0xde/0x1c0
[ 4277.729911] Modules linked in: dm_service_time ib_srp scsi_transport_srp 
target_core_user uio target_core_pscsi target_core_file ib_srpt 
target_core_iblock target_core_mod brd netconsole xt_CHECKSUM iptable_mangle 
ipt_MASQUERADE nf_nat_masquerade_ipv4 iptable_nat nf_nat_
ipv4 nf_nat libcrc32c nf_conntrack_ipv4 nf_defrag_ipv4 xt_conntrack 
nf_conntrack ipt_REJECT nf_reject_ipv4 xt_tcpudp tun bridge stp llc 
ebtable_filter ebtables ip6table_filter ip6_tables iptable_filter ip_tables 
x_tables af_packet ib_ipoib rdma_ucm ib_ucm ib_uverbs ib_umad 
rdma_cm configfs ib_cm iw_cm msr mlx4_ib ib_core sb_edac edac_core 
x86_pkg_temp_thermal intel_powerclamp coretemp kvm_intel ipmi_ssif kvm 
irqbypass crct10dif_pclmul crc32_pclmul crc32c_intel ghash_clmulni_intel pcbc 
aesni_intel aes_x86_64 mlx4_core crypto_simd iTCO_wdt
[ 4277.730048]  tg3 iTCO_vendor_support dcdbas glue_helper ptp ipmi_si pcspkr 
pps_core devlink ipmi_devintf cryptd libphy fjes ipmi_msghandler tpm_tis mei_me 
tpm_tis_core lpc_ich mfd_core shpchp mei tpm wmi button hid_generic usbhid 
mgag200 i2c_algo_bit drm_kms_helper sysco
pyarea sysfillrect sysimgblt fb_sys_fops ttm sr_mod cdrom drm ehci_pci ehci_hcd 
usbcore usb_common sg dm_multipath dm_mod scsi_dh_rdac scsi_dh_emc scsi_dh_alua 
autofs4
[ 4277.730135] CPU: 11 PID: 9209 Comm: mount Not tainted 4.10.0-rc5-dbg+ #2
[ 4277.730159] Hardware name: Dell Inc. PowerEdge R430/03XKDV, BIOS 1.0.2 
11/17/2014
[ 4277.730187] Call Trace:
[ 4277.730212]  dump_stack+0x68/0x93
[ 4277.730236]  __schedule_bug+0x5b/0x80
[ 4277.730259]  __schedule+0x762/0xb00
[ 4277.730281]  schedule+0x38/0x90
[ 4277.730302]  schedule_timeout+0x2fe/0x640
[ 4277.730324]  ? mark_held_locks+0x6f/0xa0
[ 4277.730349]  ? ktime_get+0x74/0x130
[ 4277.730370]  ? trace_hardirqs_on_caller+0xf9/0x1b0
[ 4277.730391]  ? trace_hardirqs_on+0xd/0x10
[ 4277.730418]  ?

Re: split scsi passthrough fields out of struct request V2

2017-01-26 Thread Jens Axboe 
On 01/26/2017 11:59 AM, h...@lst.de wrote:
> On Thu, Jan 26, 2017 at 11:57:36AM -0700, Jens Axboe wrote:
>> It's against my for-4.11/block, which you were running under Christoph's
>> patches. Maybe he's using an older version? In any case, should be
>> pretty trivial for you to hand apply. Just ensure that .flags is set to
>> 0 for the common cases, and inherit 'flags' when it is passed in.
> 
> No, the flush op cleanups you asked for last round create a conflict
> with your patch.  They should be trivial to fix, though.

Ah, makes sense. And yes, as I said, should be trivial to hand apply the
hunk that does fail.

-- 
Jens Axboe

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 15/16] block: split scsi_request out of struct request

2017-01-26 Thread Jens Axboe 
On 01/26/2017 12:37 PM, Christoph Hellwig wrote:
> On Thu, Jan 26, 2017 at 11:12:51AM -0800, Bart Van Assche wrote:
>> Where does the '* 3' come from? I think that deserves a comment.
>> Additionally, this patch introduces a new warning when building with W=1:
> 
> It's a magic factor copied from the old code :(
> 
> That beeing said I really wonder if we should keep the tracing for
> the scsi_requests fields at all.  We have tracing in SCSI, and poking
> into these SCSI fields from block code is painful and just screams
> layering violations.
> 
> Jens, any opinion on just removing the printout of the SCSI CDB
> for blktrace?

Kill it with fire, I don't think there's much value to having it
there to begin with.

-- 
Jens Axboe

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 15/16] block: split scsi_request out of struct request

2017-01-26 Thread Christoph Hellwig 
On Thu, Jan 26, 2017 at 11:12:51AM -0800, Bart Van Assche wrote:
> Where does the '* 3' come from? I think that deserves a comment.
> Additionally, this patch introduces a new warning when building with W=1:

It's a magic factor copied from the old code :(

That beeing said I really wonder if we should keep the tracing for
the scsi_requests fields at all.  We have tracing in SCSI, and poking
into these SCSI fields from block code is painful and just screams
layering violations.

Jens, any opinion on just removing the printout of the SCSI CDB
for blktrace?
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 15/16] block: split scsi_request out of struct request

2017-01-26 Thread Bart Van Assche 
On 01/23/2017 07:29 AM, Christoph Hellwig wrote:
> +int scsi_cmd_buf_len(struct request *rq)
> +{
> + return scsi_req(rq)->cmd_len * 3;
> +}
> +EXPORT_SYMBOL(scsi_cmd_buf_len);

Hello Christoph,

Where does the '* 3' come from? I think that deserves a comment.
Additionally, this patch introduces a new warning when building with W=1:

block/scsi_ioctl.c:761:5: warning: no previous prototype for
‘scsi_cmd_buf_len’ [-Wmissing-prototypes]

Please consider adding #include  to the source
file in which scsi_cmd_buf_len() is added.

Thanks,

Bart.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH V2 05/24] aacraid: Retrieve and update the device types

2017-01-26 Thread Raghava Aditya Renukunta 


> -Original Message-
> From: Johannes Thumshirn [mailto:jthumsh...@suse.de]
> Sent: Thursday, January 26, 2017 1:00 AM
> To: Raghava Aditya Renukunta
> 
> Cc: j...@linux.vnet.ibm.com; martin.peter...@oracle.com; linux-
> s...@vger.kernel.org; Dave Carroll ; Gana
> Sridaran ; Scott Benesh
> 
> Subject: Re: [PATCH V2 05/24] aacraid: Retrieve and update the device types
> 
> EXTERNAL EMAIL
> 
> 
> On Wed, Jan 25, 2017 at 10:00:52AM -0800, Raghava Aditya Renukunta wrote:
> > This patch adds support to retrieve the type of each adapter connected
> > device. Applicable to HBA1000 and SmartIOC2000 products
> >
> > Signed-off-by: Raghava Aditya Renukunta
> 
> > Signed-off-by: Dave Carroll 
> >
> > ---
> 
> [...]
> 
> >  /*
> >   *   Adapter Information Block
> >   *
> > @@ -1056,7 +1091,28 @@ struct aac_supplement_adapter_info
> >   /* StructExpansion == 1 */
> >   __le32  FeatureBits3;
> >   __le32  SupportedPerformanceModes;
> > - __le32  ReservedForFutureGrowth[80];
> > + u8  HostBusType;/* uses HOST_BUS_TYPE_xxx defines */
> > + u8  HostBusWidth;   /* actual width in bits or links */
> > + u16 HostBusSpeed;   /* actual bus speed/link rate in MHz 
> > */
> > + u8  MaxRRCDrives;   /* max. number of ITP-RRC drives/pool 
> > */
> > + u8  MaxDiskXtasks;  /* max. possible num of DiskX Tasks */
> > +
> > + u8  CpldVerLoaded;
> > + u8  CpldVerInFlash;
> > +
> > + __le64  MaxRRCCapacity;
> > + __le32  CompiledMaxHistLogLevel;
> > + u8  CustomBoardName[12];
> > + u16 SupportedCntlrMode; /* identify supported controller mode 
> > */
> > + u16 ReservedForFuture16;
> > + __le32  SupportedOptions3;  /* reserved for future options */
> > +
> > + __le16  VirtDeviceBus;  /* virt. SCSI device for Thor */
> > + __le16  VirtDeviceTarget;
> > + __le16  VirtDeviceLUN;
> > + __le16  Unused;
> > + __le32  ReservedForFutureGrowth[68];
> > +

Same here.

> Appart from that,
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: split scsi passthrough fields out of struct request V2

2017-01-26 Thread h...@lst.de 
On Thu, Jan 26, 2017 at 11:57:36AM -0700, Jens Axboe wrote:
> It's against my for-4.11/block, which you were running under Christoph's
> patches. Maybe he's using an older version? In any case, should be
> pretty trivial for you to hand apply. Just ensure that .flags is set to
> 0 for the common cases, and inherit 'flags' when it is passed in.

No, the flush op cleanups you asked for last round create a conflict
with your patch.  They should be trivial to fix, though.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: split scsi passthrough fields out of struct request V2

2017-01-26 Thread Jens Axboe 
On 01/26/2017 11:52 AM, Bart Van Assche wrote:
> On Thu, 2017-01-26 at 11:44 -0700, Jens Axboe wrote:
>> I think this may be my bug - does the below help?
> 
> Hello Jens,
> 
> What tree has that patch been generated against? It does not apply
> cleanly on top of Christoph's tree:
> 
> $ git checkout hch-block-pc-refactor
> $ patch -p1 --dry-run -f -s < 
> ~/Re\:_split_scsi_passthrough_fields_out_of_struct_request_V2.mbox
> 1 out of 3 hunks FAILED

It's against my for-4.11/block, which you were running under Christoph's
patches. Maybe he's using an older version? In any case, should be
pretty trivial for you to hand apply. Just ensure that .flags is set to
0 for the common cases, and inherit 'flags' when it is passed in.

-- 
Jens Axboe

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH V2 06/24] aacraid: Reworked scsi command submission path

2017-01-26 Thread Raghava Aditya Renukunta 
Hi Johannes,

> -Original Message-
> From: Johannes Thumshirn [mailto:jthumsh...@suse.de]
> Sent: Thursday, January 26, 2017 1:06 AM
> To: Raghava Aditya Renukunta
> 
> Cc: j...@linux.vnet.ibm.com; martin.peter...@oracle.com; linux-
> s...@vger.kernel.org; Dave Carroll ; Gana
> Sridaran ; Scott Benesh
> 
> Subject: Re: [PATCH V2 06/24] aacraid: Reworked scsi command submission
> path
> 
> EXTERNAL EMAIL
> 
> 
> On Wed, Jan 25, 2017 at 10:00:53AM -0800, Raghava Aditya Renukunta wrote:
> > Moved the READ and WRITE switch cases to the top. Added a  default
> > case to the switch case and replaced duplicate scsi result value with a
> > macro.
> 
> Can you please explain why you did the changes as well?

The idea is that since most of commands that we care performance wise, are
read and writes, we wanted to process them first and then the others later.

Internally as I understood it , a switch case is either converted into a jump
Table or a bunch of if else conditions, so placing the often used read,  write 
cases at top was an effort in optimization.  

Regards,
Raghava Aditya
> --
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: split scsi passthrough fields out of struct request V2

2017-01-26 Thread Bart Van Assche 
On Thu, 2017-01-26 at 11:44 -0700, Jens Axboe wrote:
> I think this may be my bug - does the below help?

Hello Jens,

What tree has that patch been generated against? It does not apply
cleanly on top of Christoph's tree:

$ git checkout hch-block-pc-refactor
$ patch -p1 --dry-run -f -s < 
~/Re\:_split_scsi_passthrough_fields_out_of_struct_request_V2.mbox
1 out of 3 hunks FAILED

Thanks,

Bart.--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: split scsi passthrough fields out of struct request V2

2017-01-26 Thread Jens Axboe 
On 01/26/2017 11:29 AM, Bart Van Assche wrote:
> On Wed, 2017-01-25 at 18:25 +0100, Christoph Hellwig wrote:
>> Hi all,
>>
>> this series splits the support for SCSI passthrough commands from the
>> main struct request used all over the block layer into a separate
>> scsi_request structure that drivers that want to support SCSI passthough
>> need to embedded as the first thing into their request-private data,
>> similar to how we handle NVMe passthrough commands.
>>
>> To support this I've added support for that the private data after
>> request structure to the legacy request path instead, so that it can
>> be treated the same way as the blk-mq path.  Compare to the current
>> scsi_cmnd allocator that actually is a major simplification.
>>
>> Changes since V1:
>>  - fix handling of a NULL sense pointer in __scsi_execute
>>  - clean up handling of the flush flags in the block layer and MD
>>  - additional small cleanup in dm-rq
> 
> Hello Christoph,
> 
> Thanks for having fixed the NULL pointer issue I had reported for v1.
> However, if I try to run my srp-test testsuite on top of your
> hch-block/block-pc-refactor branch (commit ID a07dc3521034) merged
> with v4.10-rc5 the following appears on the console:

I think this may be my bug - does the below help?


diff --git a/block/blk-mq-sched.c b/block/blk-mq-sched.c
index d05061f27bb1..56b92db944ae 100644
--- a/block/blk-mq-sched.c
+++ b/block/blk-mq-sched.c
@@ -117,7 +117,7 @@ struct request *blk_mq_sched_get_request(struct 
request_queue *q,
ctx = blk_mq_get_ctx(q);
hctx = blk_mq_map_queue(q, ctx->cpu);
 
-   blk_mq_set_alloc_data(data, q, 0, ctx, hctx);
+   blk_mq_set_alloc_data(data, q, data->flags, ctx, hctx);
 
if (e) {
data->flags |= BLK_MQ_REQ_INTERNAL;
diff --git a/block/blk-mq.c b/block/blk-mq.c
index dcb567642db7..9e4ed04f398c 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -253,7 +253,7 @@ EXPORT_SYMBOL_GPL(__blk_mq_alloc_request);
 struct request *blk_mq_alloc_request(struct request_queue *q, int rw,
unsigned int flags)
 {
-   struct blk_mq_alloc_data alloc_data;
+   struct blk_mq_alloc_data alloc_data = { .flags = flags };
struct request *rq;
int ret;
 
@@ -1382,7 +1382,7 @@ static blk_qc_t blk_mq_make_request(struct request_queue 
*q, struct bio *bio)
 {
const int is_sync = op_is_sync(bio->bi_opf);
const int is_flush_fua = bio->bi_opf & (REQ_PREFLUSH | REQ_FUA);
-   struct blk_mq_alloc_data data;
+   struct blk_mq_alloc_data data = { 0, };
struct request *rq;
unsigned int request_count = 0, srcu_idx;
struct blk_plug *plug;
@@ -1504,7 +1504,7 @@ static blk_qc_t blk_sq_make_request(struct request_queue 
*q, struct bio *bio)
const int is_flush_fua = bio->bi_opf & (REQ_PREFLUSH | REQ_FUA);
struct blk_plug *plug;
unsigned int request_count = 0;
-   struct blk_mq_alloc_data data;
+   struct blk_mq_alloc_data data = { 0, };
struct request *rq;
blk_qc_t cookie;
unsigned int wb_acct;

-- 
Jens Axboe

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH V2 04/24] aacraid: Added sa firmware support

2017-01-26 Thread Raghava Aditya Renukunta 


> -Original Message-
> From: Johannes Thumshirn [mailto:jthumsh...@suse.de]
> Sent: Thursday, January 26, 2017 12:55 AM
> To: Raghava Aditya Renukunta
> 
> Cc: j...@linux.vnet.ibm.com; martin.peter...@oracle.com; linux-
> s...@vger.kernel.org; Dave Carroll ; Gana
> Sridaran ; Scott Benesh
> 
> Subject: Re: [PATCH V2 04/24] aacraid: Added sa firmware support
> 
> EXTERNAL EMAIL
> 
> 
> On Wed, Jan 25, 2017 at 10:00:51AM -0800, Raghava Aditya Renukunta wrote:
> > sa_firmware adds the capability to differentiate the new SmartIOC family
> > of adapters from the series 8 and below.
> >
> > Signed-off-by: Raghava Aditya Renukunta
> 
> > Signed-off-by: Dave Carroll 
> >
> > ---
> 
> [...]
> 
> > @@ -465,9 +473,13 @@ void aac_define_int_mode(struct aac_dev *dev)
> >   if (dev->max_msix > msi_count)
> >   dev->max_msix = msi_count;
> >   }
> > - dev->vector_cap =
> > - (dev->scsi_host_ptr->can_queue + AAC_NUM_MGT_FIB) /
> > - msi_count;
> > + if (dev->comm_interface == AAC_COMM_MESSAGE_TYPE3 && dev-
> >sa_firmware)
> > + dev->vector_cap = (dev->scsi_host_ptr->can_queue +
> > + AAC_NUM_MGT_FIB);
> 
> '+' has precedence before '=' and you do no multiplication here, so no
> brackets
> needed.

Noted , will remove it in the next patch series. 
 

> [...]
> 
> 
> --
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH V2 03/24] aacraid: added support for init_struct_8

2017-01-26 Thread Raghava Aditya Renukunta 


> -Original Message-
> From: Johannes Thumshirn [mailto:jthumsh...@suse.de]
> Sent: Thursday, January 26, 2017 12:37 AM
> To: Raghava Aditya Renukunta
> 
> Cc: j...@linux.vnet.ibm.com; martin.peter...@oracle.com; linux-
> s...@vger.kernel.org; Dave Carroll ; Gana
> Sridaran ; Scott Benesh
> 
> Subject: Re: [PATCH V2 03/24] aacraid: added support for init_struct_8
> 
> EXTERNAL EMAIL
> 
> 
> On Wed, Jan 25, 2017 at 10:00:50AM -0800, Raghava Aditya Renukunta wrote:
> > This  patch lays the groundwork for supporting the new HBA-1000
> controller
> > family.A new INIT structure INIT_STRUCT_8 has been added which allows
> for a
> > variable size for MSI-x vectors among other things,  and is used for both
> > Series-8, HBA-1000 and SmartIOC-2000.
> >
> > Signed-off-by: Raghava Aditya Renukunta
> 
> > Signed-off-by: Dave Carroll 
> >
> > ---
> 
> [...]
> 
> > -struct aac_init
> > +union aac_init
> >  {
> > - __le32  InitStructRevision;
> > - __le32  Sa_MSIXVectors;
> > - __le32  fsrev;
> > - __le32  CommHeaderAddress;
> > - __le32  FastIoCommAreaAddress;
> > - __le32  AdapterFibsPhysicalAddress;
> > - __le32  AdapterFibsVirtualAddress;
> > - __le32  AdapterFibsSize;
> > - __le32  AdapterFibAlign;
> > - __le32  printfbuf;
> > - __le32  printfbufsiz;
> > - __le32  HostPhysMemPages;   /* number of 4k pages of host
> > -physical memory */
> > - __le32  HostElapsedSeconds; /* number of seconds since 1970. */
> > - /*
> > -  * ADAPTER_INIT_STRUCT_REVISION_4 begins here
> > -  */
> > - __le32  InitFlags;  /* flags for supported features */
> > + struct _r7 {
> > + __le32  InitStructRevision;
> > + __le32  NoOfMSIXVectors;
> > + __le32  fsrev;
> > + __le32  CommHeaderAddress;
> > + __le32  FastIoCommAreaAddress;
> > + __le32  AdapterFibsPhysicalAddress;
> > + __le32  AdapterFibsVirtualAddress;
> > + __le32  AdapterFibsSize;
> > + __le32  AdapterFibAlign;
> > + __le32  printfbuf;
> > + __le32  printfbufsiz;
> > + /* number of 4k pages of host phys. mem. */
> > + __le32  HostPhysMemPages;
> > + /* number of seconds since 1970. */
> > + __le32  HostElapsedSeconds;
> > + /* ADAPTER_INIT_STRUCT_REVISION_4 begins here */
> > + __le32  InitFlags;  /* flags for supported features */
> >  #define INITFLAGS_NEW_COMM_SUPPORTED 0x0001
> >  #define INITFLAGS_DRIVER_USES_UTC_TIME   0x0010
> >  #define INITFLAGS_DRIVER_SUPPORTS_PM 0x0020
> >  #define INITFLAGS_NEW_COMM_TYPE1_SUPPORTED   0x0040
> >  #define INITFLAGS_FAST_JBOD_SUPPORTED0x0080
> >  #define INITFLAGS_NEW_COMM_TYPE2_SUPPORTED   0x0100
> > - __le32  MaxIoCommands;  /* max outstanding commands */
> > - __le32  MaxIoSize;  /* largest I/O command */
> > - __le32  MaxFibSize; /* largest FIB to adapter */
> > - /* ADAPTER_INIT_STRUCT_REVISION_5 begins here */
> > - __le32  MaxNumAif;  /* max number of aif */
> > - /* ADAPTER_INIT_STRUCT_REVISION_6 begins here */
> > - __le32  HostRRQ_AddrLow;
> > - __le32  HostRRQ_AddrHigh;   /* Host RRQ (response queue) for SRC
> */
> > +#define INITFLAGS_DRIVER_SUPPORTS_HBA_MODE  0x0400
> > + __le32  MaxIoCommands;  /* max outstanding commands */
> > + __le32  MaxIoSize;  /* largest I/O command */
> > + __le32  MaxFibSize; /* largest FIB to adapter */
> > + /* ADAPTER_INIT_STRUCT_REVISION_5 begins here */
> > + __le32  MaxNumAif;  /* max number of aif */
> > + /* ADAPTER_INIT_STRUCT_REVISION_6 begins here */
> > + /* Host RRQ (response queue) for SRC */
> > + __le32  HostRRQ_AddrLow;
> > + __le32  HostRRQ_AddrHigh;
> > + } r7;
> > + struct _r8 {
> > + /* ADAPTER_INIT_STRUCT_REVISION_8 */
> > + __le32  InitStructRevision;
> > + __le32  RRQueueCount;
> > + __le32  HostElapsedSeconds; /* number of seconds since 1970. 
> > */
> > + __le32  InitFlags;
> > + __le32  MaxIoSize;  /* largest I/O command */
> > + __le32  MaxNumAif;  /* max number of aif */
> > + __le32  Reserved1;
> > + __le32  Reserved2;
> > + struct _rrq {
> > + __le32  Host_AddrLow;
> > + __le32  Host_AddrHigh;
> > + __le16  MSIX_Id;
> > + __le16  ElementCount;
> > + __le16  Comp_Thresh;
> > + __le16  Unused;
> > +

Re: [PATCH v2] mpt3sas: Force request partial completion alignment

2017-01-26 Thread Guilherme G. Piccoli 
On 01/26/2017 03:02 PM, Ram Pai wrote:
> On Thu, Jan 26, 2017 at 11:31:53AM -0200, Guilherme G. Piccoli wrote:
>> On 01/25/2017 09:46 PM, Martin K. Petersen wrote:
 "Guilherme" == Guilherme G Piccoli  
 writes:
>>>
>>> Hi Guilherme,
>>
>> Hi Martin, thanks for the review!
>>
>>
>>>
>>> diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c 
>>> b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
>>> index 75f3fce..e52c942 100644
>>> --- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
>>> +++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
>>> @@ -4657,6 +4657,8 @@ _scsih_io_done(struct MPT3SAS_ADAPTER *ioc, u16 smid, 
>>> u8 msix_index, u32 reply)
>>> struct MPT3SAS_DEVICE *sas_device_priv_data;
>>> u32 response_code = 0;
>>> unsigned long flags;
>>> +   unsigned int sector_sz;
>>> +   struct request *req;
>>>
>>> mpi_reply = mpt3sas_base_get_reply_virt_addr(ioc, reply);
>>> scmd = _scsih_scsi_lookup_get_clear(ioc, smid);
>>> @@ -4715,6 +4717,21 @@ _scsih_io_done(struct MPT3SAS_ADAPTER *ioc, u16 
>>> smid, u8 msix_index, u32 reply)
>>> }
>>>
>>> xfer_cnt = le32_to_cpu(mpi_reply->TransferCount);
>>> +
>>> +   /* In case of bogus fw or device, we could end up having
>>> +* unaligned partial completion. We can force alignment here,
>>> +* then scsi-ml does not need to handle this misbehavior.
>>> +*/
>>> +   sector_sz = scmd->device->sector_size;
>>> +   req = scmd->request;
>>> +   if (unlikely(sector_sz && req && (req->cmd_type == REQ_TYPE_FS) &&
>>> +   (xfer_cnt % sector_sz))) {
>>>
>>> Maybe a bit zealous on the sanity checking...
>>
>> A bit...? heheh
>> Too much I'd say. Since this is dealing with a bogus FW scenario, I
>> found more safe to check everything...of course we can remove checks if
>> it's sure req isn't NULL ever. The sector_sz check is avoiding
>> degenerate cases, since our division below.
>>
>>
>>>
>>> +   sdev_printk(KERN_INFO, scmd->device,
>>> +   "unaligned partial completion avoided (xfer_cnt=%u, 
>>> sector_sz=%u)\n",
>>> +   xfer_cnt, sector_sz);
>>> +   xfer_cnt = (xfer_cnt / sector_sz) * sector_sz;
>>>
>>> Not so keen on divisions. xfer_cnt = round_down(xfer_cnt, sector_sz), maybe?
>>>
>>
>> Martin, I might be completely wrong here (please correct me if this is
>> the case), but isn't C standard integer division a truncation that acts
>> like a round down? I checked (what I think is) the specification of C
>> language (ISO/IEC 9899:1999), and it seems the division proposed by Ram
>> Pai is accurate in this case. Also, both variables are unsigned.
> 
> Guilherme,  Its better to use round_down() instead of division. Among
> other things it saves a few nanoseconds.

Thanks Ram and Martin for the suggestion and explanation. I just sent a V3.

Cheers,


Guilherme

> 
> RP
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3] mpt3sas: Force request partial completion alignment

2017-01-26 Thread Guilherme G. Piccoli 
From: Ram Pai 

The firmware or device, possibly under a heavy I/O load, can return
on a partial unaligned boundary. Scsi-ml expects these requests to be
completed on an alignment boundary. Scsi-ml blindly requeues the I/O
without checking the alignment boundary of the I/O request for the
remaining bytes. This leads to errors, since devices cannot perform
non-aligned read/write operations.

This patch fixes the issue in the driver. It aligns unaligned
completions of FS requests, by truncating them to the nearest
alignment boundary.

Reported-by: Mauricio Faria De Oliveira 
Signed-off-by: Guilherme G. Piccoli 
Signed-off-by: Ram Pai 
Acked-by: Sreekanth Reddy 
---
v2->v3:
  * Changed division to round_down() [suggestion by Martin].

 drivers/scsi/mpt3sas/mpt3sas_scsih.c | 17 +
 1 file changed, 17 insertions(+)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c 
b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
index 75f3fce..a6b6f58 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
@@ -4657,6 +4657,8 @@ _scsih_io_done(struct MPT3SAS_ADAPTER *ioc, u16 smid, u8 
msix_index, u32 reply)
struct MPT3SAS_DEVICE *sas_device_priv_data;
u32 response_code = 0;
unsigned long flags;
+   unsigned int sector_sz;
+   struct request *req;
 
mpi_reply = mpt3sas_base_get_reply_virt_addr(ioc, reply);
scmd = _scsih_scsi_lookup_get_clear(ioc, smid);
@@ -4715,6 +4717,21 @@ _scsih_io_done(struct MPT3SAS_ADAPTER *ioc, u16 smid, u8 
msix_index, u32 reply)
}
 
xfer_cnt = le32_to_cpu(mpi_reply->TransferCount);
+
+   /* In case of bogus fw or device, we could end up having
+* unaligned partial completion. We can force alignment here,
+* then scsi-ml does not need to handle this misbehavior.
+*/
+   sector_sz = scmd->device->sector_size;
+   req = scmd->request;
+   if (unlikely(sector_sz && req && (req->cmd_type == REQ_TYPE_FS) &&
+   (xfer_cnt % sector_sz))) {
+   sdev_printk(KERN_INFO, scmd->device,
+   "unaligned partial completion avoided (xfer_cnt=%u, 
sector_sz=%u)\n",
+   xfer_cnt, sector_sz);
+   xfer_cnt = round_down(xfer_cnt, sector_sz);
+   }
+
scsi_set_resid(scmd, scsi_bufflen(scmd) - xfer_cnt);
if (ioc_status & MPI2_IOCSTATUS_FLAG_LOG_INFO_AVAILABLE)
log_info =  le32_to_cpu(mpi_reply->IOCLogInfo);
-- 
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: split scsi passthrough fields out of struct request V2

2017-01-26 Thread Bart Van Assche 
On Wed, 2017-01-25 at 18:25 +0100, Christoph Hellwig wrote:
> Hi all,
> 
> this series splits the support for SCSI passthrough commands from the
> main struct request used all over the block layer into a separate
> scsi_request structure that drivers that want to support SCSI passthough
> need to embedded as the first thing into their request-private data,
> similar to how we handle NVMe passthrough commands.
> 
> To support this I've added support for that the private data after
> request structure to the legacy request path instead, so that it can
> be treated the same way as the blk-mq path.  Compare to the current
> scsi_cmnd allocator that actually is a major simplification.
> 
> Changes since V1:
>  - fix handling of a NULL sense pointer in __scsi_execute
>  - clean up handling of the flush flags in the block layer and MD
>  - additional small cleanup in dm-rq

Hello Christoph,

Thanks for having fixed the NULL pointer issue I had reported for v1.
However, if I try to run my srp-test testsuite on top of your
hch-block/block-pc-refactor branch (commit ID a07dc3521034) merged
with v4.10-rc5 the following appears on the console:

[  707.317403] BUG: scheduling while atomic: fio/9073/0x0003
[  707.317404] 1 lock held by fio/9073:
[  707.317404]  #0:  (rcu_read_lock){..}, at: [] 
__blk_mq_run_hw_queue+0xde/0x1c0
[  707.317409] Modules linked in: dm_service_time ib_srp scsi_transport_srp 
target_core_user uio target_core_pscsi target_core_file ib_srpt 
target_core_iblock target_core_mod brd netconsole xt_CHECKSUM iptable_mangle 
ipt_MASQUERADE nf_nat_masquerade_ipv4 iptable_nat nf_nat_ipv4 nf_nat libcrc32c 
nf_conntrack_ipv4 nf_defrag_ipv4 xt_conntrack nf_conntrack ipt_REJECT 
nf_reject_ipv4 xt_tcpudp tun bridge stp llc ebtable_filter ebtables 
ip6table_filter ip6_tables iptable_filter ip_tables x_tables af_packet ib_ipoib 
rdma_ucm ib_ucm ib_uverbs ib_umad rdma_cm configfs ib_cm iw_cm msr mlx4_ib 
ib_core sb_edac edac_core x86_pkg_temp_thermal intel_powerclamp coretemp 
ipmi_ssif kvm_intel kvm irqbypass crct10dif_pclmul crc32_pclmul mlx4_core 
crc32c_intel ghash_clmulni_intel hid_generic pcbc usbhid iTCO_wdt tg3 
aesni_intel
[  707.317445]  ptp iTCO_vendor_support aes_x86_64 crypto_simd pps_core 
glue_helper dcdbas ipmi_si ipmi_devintf libphy devlink lpc_ich cryptd pcspkr 
ipmi_msghandler mfd_core fjes mei_me tpm_tis button tpm_tis_core shpchp mei tpm 
wmi mgag200 i2c_algo_bit drm_kms_helper syscopyarea sysfillrect sysimgblt 
fb_sys_fops ttm sr_mod cdrom drm ehci_pci ehci_hcd usbcore usb_common sg 
dm_multipath dm_mod scsi_dh_rdac scsi_dh_emc scsi_dh_alua autofs4
[  707.317469] CPU: 6 PID: 9073 Comm: fio Tainted: GW   
4.10.0-rc5-dbg+ #1
[  707.317470] Hardware name: Dell Inc. PowerEdge R430/03XKDV, BIOS 1.0.2 
11/17/2014
[  707.317470] Call Trace:
[  707.317473]  dump_stack+0x68/0x93
[  707.317475]  __schedule_bug+0x5b/0x80
[  707.317477]  __schedule+0x762/0xb00
[  707.317479]  schedule+0x38/0x90
[  707.317481]  schedule_timeout+0x2fe/0x640
[  707.317491]  io_schedule_timeout+0x9f/0x110
[  707.317493]  blk_mq_get_tag+0x158/0x260
[  707.317496]  __blk_mq_alloc_request+0x16/0xe0
[  707.317498]  blk_mq_sched_get_request+0x30d/0x360
[  707.317502]  blk_mq_alloc_request+0x3b/0x90
[  707.317505]  blk_get_request+0x2f/0x110
[  707.317507]  multipath_clone_and_map+0xcd/0x140 [dm_multipath]
[  707.317512]  map_request+0x3c/0x290 [dm_mod]
[  707.317517]  dm_mq_queue_rq+0x77/0x100 [dm_mod]
[  707.317519]  blk_mq_dispatch_rq_list+0x1ff/0x320
[  707.317521]  blk_mq_sched_dispatch_requests+0xa9/0xe0
[  707.317523]  __blk_mq_run_hw_queue+0x122/0x1c0
[  707.317528]  blk_mq_run_hw_queue+0x84/0x90
[  707.317530]  blk_mq_flush_plug_list+0x39f/0x480
[  707.317531]  blk_flush_plug_list+0xee/0x270
[  707.317533]  blk_finish_plug+0x27/0x40
[  707.317534]  do_io_submit+0x475/0x900
[  707.317537]  SyS_io_submit+0xb/0x10
[  707.317539]  entry_SYSCALL_64_fastpath+0x18/0xad

Bart.--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] mpt3sas: Force request partial completion alignment

2017-01-26 Thread Ram Pai 
On Thu, Jan 26, 2017 at 11:31:53AM -0200, Guilherme G. Piccoli wrote:
> On 01/25/2017 09:46 PM, Martin K. Petersen wrote:
> >> "Guilherme" == Guilherme G Piccoli  
> >> writes:
> > 
> > Hi Guilherme,
> 
> Hi Martin, thanks for the review!
> 
> 
> > 
> > diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c 
> > b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> > index 75f3fce..e52c942 100644
> > --- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> > +++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> > @@ -4657,6 +4657,8 @@ _scsih_io_done(struct MPT3SAS_ADAPTER *ioc, u16 smid, 
> > u8 msix_index, u32 reply)
> > struct MPT3SAS_DEVICE *sas_device_priv_data;
> > u32 response_code = 0;
> > unsigned long flags;
> > +   unsigned int sector_sz;
> > +   struct request *req;
> > 
> > mpi_reply = mpt3sas_base_get_reply_virt_addr(ioc, reply);
> > scmd = _scsih_scsi_lookup_get_clear(ioc, smid);
> > @@ -4715,6 +4717,21 @@ _scsih_io_done(struct MPT3SAS_ADAPTER *ioc, u16 
> > smid, u8 msix_index, u32 reply)
> > }
> > 
> > xfer_cnt = le32_to_cpu(mpi_reply->TransferCount);
> > +
> > +   /* In case of bogus fw or device, we could end up having
> > +* unaligned partial completion. We can force alignment here,
> > +* then scsi-ml does not need to handle this misbehavior.
> > +*/
> > +   sector_sz = scmd->device->sector_size;
> > +   req = scmd->request;
> > +   if (unlikely(sector_sz && req && (req->cmd_type == REQ_TYPE_FS) &&
> > +   (xfer_cnt % sector_sz))) {
> > 
> > Maybe a bit zealous on the sanity checking...
> 
> A bit...? heheh
> Too much I'd say. Since this is dealing with a bogus FW scenario, I
> found more safe to check everything...of course we can remove checks if
> it's sure req isn't NULL ever. The sector_sz check is avoiding
> degenerate cases, since our division below.
> 
> 
> > 
> > +   sdev_printk(KERN_INFO, scmd->device,
> > +   "unaligned partial completion avoided (xfer_cnt=%u, 
> > sector_sz=%u)\n",
> > +   xfer_cnt, sector_sz);
> > +   xfer_cnt = (xfer_cnt / sector_sz) * sector_sz;
> > 
> > Not so keen on divisions. xfer_cnt = round_down(xfer_cnt, sector_sz), maybe?
> >
> 
> Martin, I might be completely wrong here (please correct me if this is
> the case), but isn't C standard integer division a truncation that acts
> like a round down? I checked (what I think is) the specification of C
> language (ISO/IEC 9899:1999), and it seems the division proposed by Ram
> Pai is accurate in this case. Also, both variables are unsigned.

Guilherme,  Its better to use round_down() instead of division. Among
other things it saves a few nanoseconds.

RP

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Revert "sd: remove __data_len hack for WRITE SAME"

2017-01-26 Thread Jens Axboe 
On 01/25/2017 07:56 PM, Martin K. Petersen wrote:
>> "Christoph" == Christoph Hellwig  writes:
> 
> Christoph> Thanks Bart, and sorry for messing this up.  Acked-by:
> Christoph> Christoph Hellwig 
> 
> Jens: Since this came in via your tree would you mind reverting it? I'd
> prefer to avoid rebasing my fixes branch.
> 
> Acked-by: Martin K. Petersen 

Yep, I have added it, thanks.

-- 
Jens Axboe

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] scsi: srp_transport: Fix 'always false comparison' in srp_tmo_valid()

2017-01-26 Thread Bart Van Assche 
On Thu, 2017-01-26 at 11:17 +, Augusto Mecking Caringi wrote:
> In a 64bit system (where long is 64bit wide), even dividing LONG_MAX by
> HZ will always be bigger than the max value that an int variable can
> hold.
> 
> This has been detected by building the driver with W=1:
> 
> drivers/scsi/scsi_transport_srp.c: In function ‘srp_tmo_valid’:
> drivers/scsi/scsi_transport_srp.c:92:19: warning: comparison is always
> false due to limited range of data type [-Wtype-limits]
> if (dev_loss_tmo >= LONG_MAX / HZ)
>^
> 
> Signed-off-by: Augusto Mecking Caringi 
> ---
>  drivers/scsi/scsi_transport_srp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/scsi_transport_srp.c 
> b/drivers/scsi/scsi_transport_srp.c
> index b87a786..d8c83f4 100644
> --- a/drivers/scsi/scsi_transport_srp.c
> +++ b/drivers/scsi/scsi_transport_srp.c
> @@ -89,7 +89,7 @@ int srp_tmo_valid(int reconnect_delay, int 
> fast_io_fail_tmo, int dev_loss_tmo)
>   if (fast_io_fail_tmo < 0 &&
>   dev_loss_tmo > SCSI_DEVICE_BLOCK_MAX_TIMEOUT)
>   return -EINVAL;
> - if (dev_loss_tmo >= LONG_MAX / HZ)
> + if (dev_loss_tmo >= INT_MAX / HZ)
>   return -EINVAL;
>   if (fast_io_fail_tmo >= 0 && dev_loss_tmo >= 0 &&
>   fast_io_fail_tmo >= dev_loss_tmo)

This patch is wrong. The purpose of the dev_loss_tmo >= LONG_MAX / HZ check
is to avoid that the expression 1UL * dev_loss_tmo * HZ further down
overflows. Can you check whether changing the if-statement into if (1UL *
dev_loss_tmo >= LONG_MAX / HZ) also suppresses the compiler warning?

Bart.N�r��yb�X��ǧv�^�)޺{.n�+{���"�{ay�ʇڙ�,j��f���h���z��w���
���j:+v���w�j�mzZ+�ݢj"��!�i

Re: [PATCH v2 0/2] scsi: storvsc: Add support for FC lightweight host.

2017-01-26 Thread Christoph Hellwig 
On Thu, Jan 26, 2017 at 08:38:58AM -0500, Cathy Avery wrote:
> Included in the current storvsc driver for Hyper-V is the ability
> to access luns on an FC fabric via a virtualized fiber channel
> adapter exposed by the Hyper-V host. This was done to provide an
> interface for existing customer tools that was more consistent with
> a conventional FC device. The driver attaches to the FC transport 
> to allow host and port names to be published under 
> /sys/class/fc_host/hostX.
> 
> A problem arose when attaching to the FC transport. The scsi_scan code
> attempts to call fc_user_scan which has basically become a no-op 
> due to the virtualized nature of the FC host 
> ( missing rports, vports, etc ). At this point you cannot refresh
> the scsi bus after mapping or unmapping luns on the SAN without
> a reboot.

I don't think a device without rports or vports is a FC device, plain and
simple.  So as far as I'm concerned we should remove the code from storvsc
that pretends to be FC, and not add it to virtio to start with.

And again I think leightweight is a very confusing name -
what exactly is leight or heavy?   It's really fake or dummy
in the current version.

> 
> 2) Removes an original workaround dealing with replacing
> the eh_timed_out function. Patch 1 will not set the 
> scsi_transport_template.eh_timed_out function directly during
> lightweight fc_attach_transport(). It instead relies on
> whatever was indicated as the scsi_host_template timeout handler
> during scsi_times_out() scsi_error.c. So the workaround is 
> no longer necessary.

Can you send a patch that gets rid of the transport class timeout handler
entirely?  I think it's simply the wrong layering we have here - the
driver needs to be in control of timeouts, and if it wants it can
optionally call into library code in the transport class.


FYI, all the long-term relevant explanation need to go into the patches
themselves (patch description or code comments), not in the cover
letter.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] qla2xxx: Fix a recently introduced memory leak

2017-01-26 Thread h...@lst.de 
On Wed, Jan 25, 2017 at 03:47:20PM +, Bart Van Assche wrote:
> =
> BUG kmalloc-16 (Not tainted): Redzone overwritten
> -
> 
> Disabling lock debugging due to kernel taint
> INFO: 0x880030bacc78-0x880030bacc7f. First byte 0xf instead of 0xcc
> INFO: Allocated in irq_create_affinity_masks+0x5f/0x260 age=0 cpu=3 pid=812
>   ___slab_alloc.constprop.79+0x482/0x4f0
>   __slab_alloc.isra.75.constprop.78+0x55/0xa0
>   __kmalloc+0x27c/0x310
>   irq_create_affinity_masks+0x5f/0x260

This is the normal affinity mask allocation.

>   __pci_enable_msix+0x314/0x4c0
>   pci_alloc_irq_vectors_affinity+0xb7/0x140
>   qla2x00_request_irqs+0xa6/0x6d0 [qla2xxx]
>   qla2x00_probe_one+0xc2e/0x25f0 [qla2xxx]
>   pci_device_probe+0x8a/0xf0
>   driver_probe_device+0x1f5/0x450
>   __driver_attach+0xe3/0xf0
>   bus_for_each_dev+0x66/0xa0
>   driver_attach+0x1e/0x20
>   bus_add_driver+0x200/0x270
>   driver_register+0x60/0xe0
>   __pci_register_driver+0x5d/0x60
> INFO: Freed in acpi_ns_get_node_unlocked+0x90/0xa4 age=0 cpu=3 pid=812
>   __slab_free+0x176/0x310
>   kfree+0x25e/0x2d0
>   acpi_ns_get_node_unlocked+0x90/0xa4
>   acpi_ns_get_node+0x3d/0x52
>   acpi_get_handle+0x82/0x96

This on the other hand I don't understand acpi_ns_get_node_unlocked
only frees the object it allocated in the ACPI code using
acpi_ns_internalize_name.  I can't really see any relation to the
affinity mask allocation.

>   acpi_pci_irq_find_prt_entry+0x26e/0x2ae
>   acpi_pci_irq_lookup+0x28/0x135
>   acpi_pci_irq_enable+0x60/0x1f8
>   pcibios_enable_device+0x2d/0x30
>   do_pci_enable_device+0x64/0xf0
>   pci_enable_device_flags+0xc5/0x110
>   pci_enable_device_mem+0x13/0x20
>   qla2x00_probe_one+0x14b/0x25f0 [qla2xxx]
>   pci_device_probe+0x8a/0xf0
>   driver_probe_device+0x1f5/0x450
>   __driver_attach+0xe3/0xf0
> INFO: Slab 0xeac2eb00 objects=23 used=21 fp=0x880030bacdc8 
> flags=0x40008101
> INFO: Object 0x880030bacc68 @offset=3176 fp=0x880030bacf28
> 
> Redzone 880030bacc60: cc cc cc cc cc cc cc cc  
> 
> Object 880030bacc68: ff 00 00 00 00 00 00 00 ff 00 00 00 00 00 00 00  
> 
> Redzone 880030bacc78: 0f 00 00 00 00 00 00 00  
> 
> Padding 880030bacdb8: 5a 5a 5a 5a 5a 5a 5a 5a  
> 
> CPU: 3 PID: 812 Comm: modprobe Tainted: GB   4.10.0-rc5-dbg+ #9
> Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
> Call Trace:
>  dump_stack+0x85/0xc2
>  print_trailer+0x162/0x260
>  check_bytes_and_report+0xc5/0x110
>  check_object+0x1da/0x2a0
>  free_debug_processing+0x161/0x3d0
>  ? debug_lockdep_rcu_enabled+0x1d/0x20
>  ? __pci_enable_msix+0x41c/0x4c0
>  __slab_free+0x176/0x310
>  ? __pci_enable_msix+0x41c/0x4c0
>  ? call_rcu+0x17/0x20
>  ? kfree+0xe7/0x2d0
>  ? __pci_enable_msix+0x41c/0x4c0
>  ? __pci_enable_msix+0x41c/0x4c0
>  kfree+0x25e/0x2d0
>  __pci_enable_msix+0x41c/0x4c0
>  pci_alloc_irq_vectors_affinity+0xb7/0x140
>  qla2x00_request_irqs+0xa6/0x6d0 [qla2xxx]
>  qla2x00_probe_one+0xc2e/0x25f0 [qla2xxx]
>  ? __pm_runtime_resume+0x40/0x80
>  ? trace_hardirqs_on_caller+0x128/0x1b0
>  ? trace_hardirqs_on+0xd/0x10
>  ? _raw_spin_unlock_irqrestore+0x4a/0x80
>  pci_device_probe+0x8a/0xf0
>  driver_probe_device+0x1f5/0x450
>  __driver_attach+0xe3/0xf0
>  ? driver_probe_device+0x450/0x450
>  bus_for_each_dev+0x66/0xa0
>  driver_attach+0x1e/0x20
>  bus_add_driver+0x200/0x270
>  ? 0xa04eb000
>  driver_register+0x60/0xe0
>  ? 0xa04eb000
>  __pci_register_driver+0x5d/0x60
>  qla2x00_module_init+0x1c9/0x217 [qla2xxx]
>  do_one_initcall+0x44/0x180
>  ? rcu_read_lock_sched_held+0x72/0x80
>  ? kmem_cache_alloc_trace+0x25b/0x2c0
>  ? do_init_module+0x27/0x1f9
>  do_init_module+0x5f/0x1f9
>  load_module+0x2582/0x2a00
>  ? __symbol_put+0x70/0x70
>  ? kernel_read_file+0x10a/0x1a0
>  ? kernel_read_file_from_fd+0x49/0x80
>  SYSC_finit_module+0xbc/0xf0
>  SyS_finit_module+0xe/0x10
>  entry_SYSCALL_64_fastpath+0x23/0xc6
> RIP: 0033:0x7f05711388e9
> RSP: 002b:7fff51d4a0f8 EFLAGS: 0246 ORIG_RAX: 0139
> RAX: ffda RBX: 0003 RCX: 7f05711388e9
> RDX:  RSI: 55c17ab4f720 RDI: 0004
> RBP: 7fff51d49100 R08:  R09: 0019
> R10: 0004 R11: 0246 R12: 55c17ab4f570
> R13: 7fff51d490e0 R14: 0005 R15: 0004
> FIX kmalloc-16: Restoring 0x880030bacc78-0x880030bacc7f=0xcc
> 
> FIX kmalloc-16: Object at 0x880030bacc68 not freed
> scsi host2: qla2xxx
> qla2xxx [:00:09.0]-00fb:2: QLogic QLE2460 - QLogic 4GB FC Single-Port 
> PCI-E HBA for IBM System x.
>

[PATCH v2 1/2] scsi: scsi_transport_fc: Provide a lightweight option for Virtual FC Hosts.

2017-01-26 Thread Cathy Avery 
This patch provides a means to offer a lightweight option to the
current FC transport class. This new transport option is more
suitable for FC transports running on a VM that virtualizes their
FCs and that do not possess rports or vports whereas the heavyweight
option maintains the standard physical FC hba attuibute set.

The patch performs the following:

1) Adds the lightweight_transport option to fc_function_template. Based
on this selection the transport will either be lightweight or default
to heavyweight.

2) Divides the applicable export functions into two sets. The lightweight
functions involve FC attributes port_name and node name. The functions
that deal with targets, rports, etc are not used. The heavyweight default
contains the original standard physical FC hba attribute set.

3) All top level FC class directories such fc_remote_ports, fc_transport,
and fc_vports are still created when the transport driver loads.
They are just not populated when running in lightweight mode. Conceptually
both lightweight and heavyweight clients could coexist.

4) fc_transport_template->user_scan is now null and the bus can be scanned.

Signed-off-by: Cathy Avery 
---
 drivers/scsi/scsi_transport_fc.c | 144 +--
 include/scsi/scsi_transport_fc.h |   2 +
 2 files changed, 142 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/scsi_transport_fc.c
index 03577bd..615057c 100644
--- a/drivers/scsi/scsi_transport_fc.c
+++ b/drivers/scsi/scsi_transport_fc.c
@@ -52,6 +52,25 @@ static void fc_bsg_remove(struct request_queue *);
 static void fc_bsg_goose_queue(struct fc_rport *);
 
 /*
+ * These functions define the actions taken on behalf of either
+ * a virtualized client which is considered to be lightweigth
+ * having only port name and node name as attributes in sysfs and
+ * which does not possess rports or vports vs the heavyweigth
+ * mode which is intended for fully functional clients such as
+ * physical HBAs.
+ */
+
+static int fc_host_lw_setup(struct Scsi_Host *, struct fc_host_attrs *);
+static int fc_host_hw_setup(struct Scsi_Host *, struct fc_host_attrs *);
+static int fc_host_hw_remove(struct fc_host_attrs *);
+static struct scsi_transport_template *
+   fc_attach_lw_transport(struct fc_function_template *);
+static struct scsi_transport_template *
+   fc_attach_hw_transport(struct fc_function_template *);
+static void fc_remove_lw_host(struct Scsi_Host *);
+static void fc_remove_hw_host(struct Scsi_Host *, struct fc_host_attrs *);
+
+/*
  * Module Parameters
  */
 
@@ -352,6 +371,10 @@ struct fc_internal {
 
 #define to_fc_internal(tmpl)   container_of(tmpl, struct fc_internal, t)
 
+
+static void fc_release_lw_transport(struct fc_internal *);
+static void fc_release_hw_transport(struct fc_internal *);
+
 static int fc_target_setup(struct transport_container *tc, struct device *dev,
   struct device *cdev)
 {
@@ -387,7 +410,33 @@ static int fc_host_setup(struct transport_container *tc, 
struct device *dev,
 {
struct Scsi_Host *shost = dev_to_shost(dev);
struct fc_host_attrs *fc_host = shost_to_fc_host(shost);
+   struct fc_internal *i = to_fc_internal(shost->transportt);
+
+   /*
+* Check if the client driver has selected a minimum set
+* of fc transport attributes to initialize. Otherwise
+* initialize all currently available attributes.
+*/
+
+   if (i->f->lightweight_transport)
+   return fc_host_lw_setup(shost, fc_host);
+
+   return fc_host_hw_setup(shost, fc_host);
+}
+
+static int fc_host_lw_setup(struct Scsi_Host *shost,
+   struct fc_host_attrs *fc_host)
+{
+   /* Only node_name and port_name are used */
+   fc_host->node_name = -1;
+   fc_host->port_name = -1;
+
+   return 0;
+}
 
+static int fc_host_hw_setup(struct Scsi_Host *shost,
+   struct fc_host_attrs *fc_host)
+{
/*
 * Set default values easily detected by the midlayer as
 * failure cases.  The scsi lldd is responsible for initializing
@@ -468,7 +517,16 @@ static int fc_host_remove(struct transport_container *tc, 
struct device *dev,
 {
struct Scsi_Host *shost = dev_to_shost(dev);
struct fc_host_attrs *fc_host = shost_to_fc_host(shost);
+   struct fc_internal *i = to_fc_internal(shost->transportt);
+
+   if (i->f->lightweight_transport)
+   return 0;
 
+   return fc_host_hw_remove(fc_host);
+}
+
+static int fc_host_hw_remove(struct fc_host_attrs *fc_host)
+{
fc_bsg_remove(fc_host->rqst_q);
return 0;
 }
@@ -2175,6 +2233,51 @@ static int fc_it_nexus_response(struct Scsi_Host *shost, 
u64 nexus, int result)
 struct scsi_transport_template *
 fc_attach_transport(struct fc_function_template *ft)
 {
+   if (ft->lightweight_transport)
+   return fc_attach_lw_transport(ft);
+
+

[PATCH v2 0/2] scsi: storvsc: Add support for FC lightweight host.

2017-01-26 Thread Cathy Avery 
This patch set is based on the following patch submission
and email exchange:

[PATCH 1/1] scsi: storvsc: Support manual scan of FC hosts on Hyper-V
K. Y. Srinivasan kys at microsoft.com
Sat Mar 12 21:52:48 UTC 2016

http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2016-March/087116.html

Included in the current storvsc driver for Hyper-V is the ability
to access luns on an FC fabric via a virtualized fiber channel
adapter exposed by the Hyper-V host. This was done to provide an
interface for existing customer tools that was more consistent with
a conventional FC device. The driver attaches to the FC transport 
to allow host and port names to be published under 
/sys/class/fc_host/hostX.

A problem arose when attaching to the FC transport. The scsi_scan code
attempts to call fc_user_scan which has basically become a no-op 
due to the virtualized nature of the FC host 
( missing rports, vports, etc ). At this point you cannot refresh
the scsi bus after mapping or unmapping luns on the SAN without
a reboot.

The patch above attempted to address the problem of not being
able to scan FC hosts on a Hyper-V guest by setting
fc_transport_template->user_scan = NULL but it was rejected
in favor of a new "lightweight" version of the FC transport that
only provides the bare minimum functionality of the standard FC model.
This new transport option would be more suitable for FC transports
running on a VM and provide some flexibility in the future.

The patches below offer a method to incorporate the new 
lightweight FC option into the existing transport
and storvsc drivers.

Patch 1: scsi_transport_fc.h, scsi_transport_fc.c

1) Adds the lightweight_transport option to fc_function_template.
Based on this selection the transport will either be lightweight
or default to heavyweight.

2) Divides the applicable export functions into two sets.
The lightweight functions involve FC attributes port_name and 
node name. The functions that deal with targets, rports, etc
are not used. The heavyweight default contains the original
standard physical FC hba attribute set. 

3) All top level FC class directories such fc_remote_ports,
fc_transport, and fc_vports are still created when the transport
driver is loaded. They are just not populated when running in
lightweight mode. Conceptually both lightweight and heavyweight
clients could coexist.

4) fc_transport_template->user_scan is now null and the bus 
can be scanned.

Patch 2: storvsc.c

1) storvsc elects to use the new lightweight FC host option 
by enabling it in fc_function_template. 

2) Removes an original workaround dealing with replacing
the eh_timed_out function. Patch 1 will not set the 
scsi_transport_template.eh_timed_out function directly during
lightweight fc_attach_transport(). It instead relies on
whatever was indicated as the scsi_host_template timeout handler
during scsi_times_out() scsi_error.c. So the workaround is 
no longer necessary.


It has been suggested that the word lightweight may not be
the best choice of terms when describing the new FC transport
option.  I can offer a few new ones but I am not particularly
imaginative. 

Virtual FC
Mini FC
Host only FC 


Changes from V1:

Added more comments and documentation in the code regarding 
the lightweight feature.


Cathy Avery (2):
  scsi: scsi_transport_fc: Provide a lightweight option for Virtual FC
Hosts.
  scsi: storvsc: Add support for FC lightweight host.

 drivers/scsi/scsi_transport_fc.c | 144 +--
 drivers/scsi/storvsc_drv.c   |  12 ++--
 include/scsi/scsi_transport_fc.h |   2 +
 3 files changed, 149 insertions(+), 9 deletions(-)

-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 2/2] scsi: storvsc: Add support for FC lightweight host.

2017-01-26 Thread Cathy Avery 
Included in the current storvsc driver for Hyper-V is the ability
to access luns on an FC fabric via a virtualized fiber channel
adapter exposed by the Hyper-V host. This was done to provide an
interface for existing customer tools that was more consistent
with a conventional FC device. The driver attaches to the FC
transport to allow host and port names to be published
under /sys/class/fc_host/hostX.

A problem arose when attaching to the FC transport. The scsi_scan
code attempts to call fc_user_scan which has basically become a no-op
due to the virtualized nature of the FC host ( missing rports,
vports, etc ).  At this point you cannot refresh the scsi bus after
mapping or unmapping luns on the SAN without a reboot.

This patch consists of:

1) storvsc elects to use the new lightweight FC host option by enabling it
in fc_function_template facilitating a fuctional scsi_scan.

2) Removes an original workaround dealing with replacing the eh_timed_out
function.  Patch 1 will not set the scsi_transport_template.eh_timed_out
function directly during lightweight fc_attach_transport(). It instead
relies on whatever was indicated as the scsi_host_template timeout handler
during scsi_times_out() scsi_error.c. So the workaround is no longer
necessary.

Signed-off-by: Cathy Avery 
---
 drivers/scsi/storvsc_drv.c | 12 +++-
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index 888e16e..d487e00 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -1878,10 +1878,17 @@ static struct hv_driver storvsc_drv = {
.remove = storvsc_remove,
 };
 
+/*
+ * The driver cannot functionally back all transport
+ * attributes so select only those pertinent including
+ * the lightweight model.
+ */
+
 #if IS_ENABLED(CONFIG_SCSI_FC_ATTRS)
 static struct fc_function_template fc_transport_functions = {
.show_host_node_name = 1,
.show_host_port_name = 1,
+   .lightweight_transport = 1,
 };
 #endif
 
@@ -1906,11 +1913,6 @@ static int __init storvsc_drv_init(void)
fc_transport_template = fc_attach_transport(_transport_functions);
if (!fc_transport_template)
return -ENODEV;
-
-   /*
-* Install Hyper-V specific timeout handler.
-*/
-   fc_transport_template->eh_timed_out = storvsc_eh_timed_out;
 #endif
 
ret = vmbus_driver_register(_drv);
-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] mpt3sas: Force request partial completion alignment

2017-01-26 Thread Guilherme G. Piccoli 
On 01/25/2017 09:46 PM, Martin K. Petersen wrote:
>> "Guilherme" == Guilherme G Piccoli  writes:
> 
> Hi Guilherme,

Hi Martin, thanks for the review!


> 
> diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c 
> b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> index 75f3fce..e52c942 100644
> --- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> +++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> @@ -4657,6 +4657,8 @@ _scsih_io_done(struct MPT3SAS_ADAPTER *ioc, u16 smid, 
> u8 msix_index, u32 reply)
>   struct MPT3SAS_DEVICE *sas_device_priv_data;
>   u32 response_code = 0;
>   unsigned long flags;
> + unsigned int sector_sz;
> + struct request *req;
> 
>   mpi_reply = mpt3sas_base_get_reply_virt_addr(ioc, reply);
>   scmd = _scsih_scsi_lookup_get_clear(ioc, smid);
> @@ -4715,6 +4717,21 @@ _scsih_io_done(struct MPT3SAS_ADAPTER *ioc, u16 smid, 
> u8 msix_index, u32 reply)
>   }
> 
>   xfer_cnt = le32_to_cpu(mpi_reply->TransferCount);
> +
> + /* In case of bogus fw or device, we could end up having
> +  * unaligned partial completion. We can force alignment here,
> +  * then scsi-ml does not need to handle this misbehavior.
> +  */
> + sector_sz = scmd->device->sector_size;
> + req = scmd->request;
> + if (unlikely(sector_sz && req && (req->cmd_type == REQ_TYPE_FS) &&
> + (xfer_cnt % sector_sz))) {
> 
> Maybe a bit zealous on the sanity checking...

A bit...? heheh
Too much I'd say. Since this is dealing with a bogus FW scenario, I
found more safe to check everything...of course we can remove checks if
it's sure req isn't NULL ever. The sector_sz check is avoiding
degenerate cases, since our division below.


> 
> + sdev_printk(KERN_INFO, scmd->device,
> + "unaligned partial completion avoided (xfer_cnt=%u, 
> sector_sz=%u)\n",
> + xfer_cnt, sector_sz);
> + xfer_cnt = (xfer_cnt / sector_sz) * sector_sz;
> 
> Not so keen on divisions. xfer_cnt = round_down(xfer_cnt, sector_sz), maybe?
>

Martin, I might be completely wrong here (please correct me if this is
the case), but isn't C standard integer division a truncation that acts
like a round down? I checked (what I think is) the specification of C
language (ISO/IEC 9899:1999), and it seems the division proposed by Ram
Pai is accurate in this case. Also, both variables are unsigned.

Let me know your thoughts.
Thanks,


Guilherme

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: mvsas escalated kernel crash and ata mapping mvsas driver question

2017-01-26 Thread Jack Wang 
2017-01-26 12:53 GMT+01:00 Jelle de Jong :
> Beste Jack,
>
> I set the queue_depth to 1 and timeout to 300 for all SATA disk connected to
> the mvsas controller [ARC-1300ix-16].
>
> Does this mean that ata21 is mapped to /dev/sdq!
>
> root@sweeney:~# dmesg | grep ata21 | grep device
> [4.788568] sas: ata21: end_device-0:0:26: dev error handler
>
> root@sweeney:~# lsscsi -v | grep end_device-0:0:26
>   dir: /sys/bus/scsi/devices/0:0:14:0
> [/sys/devices/pci:00/:00:1c.0/:08:00.0/host0/port-0:0/expander-0:0/port-0:0:26/end_device-0:0:26/target0:0:14/0:0:14:0]
>
> root@sweeney:~# lsscsi -v | grep 0:0:14:0
> [0:0:14:0]   diskATA  WDC WD1003FBYX-0 1V01  /dev/sdq
>   dir: /sys/bus/scsi/devices/0:0:14:0
> [/sys/devices/pci:00/:00:1c.0/:08:00.0/host0/port-0:0/expander-0:0/port-0:0:26/end_device-0:0:26/target0:0:14/0:0:14:0]
>
> I added the following to my rc.local
>
> vim /etc/rc.local
>
> for disk in sd{c..r}; do
> echo deadline > /sys/block/$disk/queue/scheduler
> echo 0 > /sys/block/$disk/queue/iosched/front_merges
> echo 150 > /sys/block/$disk/queue/iosched/read_expire
> echo 1500 > /sys/block/$disk/queue/iosched/write_expire
> echo 1 > /sys/block/$disk/device/queue_depth;
> echo 300 > /sys/block/$disk/device/timeout;
> done
>
> I hope the performance impact of queue_depth = 1 is not to much

Yes, sdq maps to ata21.

Regards,
Jack


>
> Kind regards,
>
> Jelle de Jong
>
> On 26/01/17 11:17, Jack Wang wrote:
>>
>> 2017-01-26 10:51 GMT+01:00 Jelle de Jong :
>>>
>>> Hello everybody,
>>>
>>> I got a server that seemingly random gets kernel crashes, due to an
>>> escalation of events from most likely the mvsas based disk controller.
>>>
>>> The harddisk should be okay, I replaced a whole bunch to be sure, but the
>>> server does not get stable. I can not seem to figure out how to map for
>>> example ata21.00 to an disk so I can do a deep badblock check.
>>>
>>> I have to complete boot with kernel crash log saved with additional
>>> information.
>>>
>>> http://paste.debian.net/plainh/96325d89
>>>
>>> Can somebody take a look and maybe help?
>>>
>>>
>>> 08:00.0 SCSI storage controller: Areca Technology Corp. ARC-1300ix-16
>>> 16-Port PCI-Express to SAS Non-RAID Host Adapter (rev 02)
>>>
>>> Kind regards,
>>>
>>> Jelle de Jong
>>
>>
>> Your IO error seems related to NCQ, have you tried to disable NCQ?
>>
>> echo 1 > /sys/block/sdX/device/queue_depth
>>
>> Maybe try 4.10-rc5 is also a option?
>>
>> Regards,
>> Jack
>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
>>> the body of a message to majord...@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: mvsas escalated kernel crash and ata mapping mvsas driver question

2017-01-26 Thread Jelle de Jong 

Beste Jack,

I set the queue_depth to 1 and timeout to 300 for all SATA disk 
connected to the mvsas controller [ARC-1300ix-16].


Does this mean that ata21 is mapped to /dev/sdq!

root@sweeney:~# dmesg | grep ata21 | grep device
[4.788568] sas: ata21: end_device-0:0:26: dev error handler

root@sweeney:~# lsscsi -v | grep end_device-0:0:26
  dir: /sys/bus/scsi/devices/0:0:14:0 
[/sys/devices/pci:00/:00:1c.0/:08:00.0/host0/port-0:0/expander-0:0/port-0:0:26/end_device-0:0:26/target0:0:14/0:0:14:0]


root@sweeney:~# lsscsi -v | grep 0:0:14:0
[0:0:14:0]   diskATA  WDC WD1003FBYX-0 1V01  /dev/sdq
  dir: /sys/bus/scsi/devices/0:0:14:0 
[/sys/devices/pci:00/:00:1c.0/:08:00.0/host0/port-0:0/expander-0:0/port-0:0:26/end_device-0:0:26/target0:0:14/0:0:14:0]


I added the following to my rc.local

vim /etc/rc.local

for disk in sd{c..r}; do
echo deadline > /sys/block/$disk/queue/scheduler
echo 0 > /sys/block/$disk/queue/iosched/front_merges
echo 150 > /sys/block/$disk/queue/iosched/read_expire
echo 1500 > /sys/block/$disk/queue/iosched/write_expire
echo 1 > /sys/block/$disk/device/queue_depth;
echo 300 > /sys/block/$disk/device/timeout;
done

I hope the performance impact of queue_depth = 1 is not to much

Kind regards,

Jelle de Jong

On 26/01/17 11:17, Jack Wang wrote:

2017-01-26 10:51 GMT+01:00 Jelle de Jong :

Hello everybody,

I got a server that seemingly random gets kernel crashes, due to an
escalation of events from most likely the mvsas based disk controller.

The harddisk should be okay, I replaced a whole bunch to be sure, but the
server does not get stable. I can not seem to figure out how to map for
example ata21.00 to an disk so I can do a deep badblock check.

I have to complete boot with kernel crash log saved with additional
information.

http://paste.debian.net/plainh/96325d89

Can somebody take a look and maybe help?


08:00.0 SCSI storage controller: Areca Technology Corp. ARC-1300ix-16
16-Port PCI-Express to SAS Non-RAID Host Adapter (rev 02)

Kind regards,

Jelle de Jong


Your IO error seems related to NCQ, have you tried to disable NCQ?

echo 1 > /sys/block/sdX/device/queue_depth

Maybe try 4.10-rc5 is also a option?

Regards,
Jack


--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] scsi: srp_transport: Fix 'always false comparison' in srp_tmo_valid()

2017-01-26 Thread Augusto Mecking Caringi 
In a 64bit system (where long is 64bit wide), even dividing LONG_MAX by
HZ will always be bigger than the max value that an int variable can
hold.

This has been detected by building the driver with W=1:

drivers/scsi/scsi_transport_srp.c: In function ‘srp_tmo_valid’:
drivers/scsi/scsi_transport_srp.c:92:19: warning: comparison is always
false due to limited range of data type [-Wtype-limits]
if (dev_loss_tmo >= LONG_MAX / HZ)
   ^

Signed-off-by: Augusto Mecking Caringi 
---
 drivers/scsi/scsi_transport_srp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/scsi_transport_srp.c 
b/drivers/scsi/scsi_transport_srp.c
index b87a786..d8c83f4 100644
--- a/drivers/scsi/scsi_transport_srp.c
+++ b/drivers/scsi/scsi_transport_srp.c
@@ -89,7 +89,7 @@ int srp_tmo_valid(int reconnect_delay, int fast_io_fail_tmo, 
int dev_loss_tmo)
if (fast_io_fail_tmo < 0 &&
dev_loss_tmo > SCSI_DEVICE_BLOCK_MAX_TIMEOUT)
return -EINVAL;
-   if (dev_loss_tmo >= LONG_MAX / HZ)
+   if (dev_loss_tmo >= INT_MAX / HZ)
return -EINVAL;
if (fast_io_fail_tmo >= 0 && dev_loss_tmo >= 0 &&
fast_io_fail_tmo >= dev_loss_tmo)
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: mvsas escalated kernel crash and ata mapping mvsas driver question

2017-01-26 Thread Jack Wang 
2017-01-26 10:51 GMT+01:00 Jelle de Jong :
> Hello everybody,
>
> I got a server that seemingly random gets kernel crashes, due to an
> escalation of events from most likely the mvsas based disk controller.
>
> The harddisk should be okay, I replaced a whole bunch to be sure, but the
> server does not get stable. I can not seem to figure out how to map for
> example ata21.00 to an disk so I can do a deep badblock check.
>
> I have to complete boot with kernel crash log saved with additional
> information.
>
> http://paste.debian.net/plainh/96325d89
>
> Can somebody take a look and maybe help?
>
>
> 08:00.0 SCSI storage controller: Areca Technology Corp. ARC-1300ix-16
> 16-Port PCI-Express to SAS Non-RAID Host Adapter (rev 02)
>
> Kind regards,
>
> Jelle de Jong

Your IO error seems related to NCQ, have you tried to disable NCQ?

echo 1 > /sys/block/sdX/device/queue_depth

Maybe try 4.10-rc5 is also a option?

Regards,
Jack

> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

mvsas escalated kernel crash and ata mapping mvsas driver question

2017-01-26 Thread Jelle de Jong 

Hello everybody,

I got a server that seemingly random gets kernel crashes, due to an 
escalation of events from most likely the mvsas based disk controller.


The harddisk should be okay, I replaced a whole bunch to be sure, but 
the server does not get stable. I can not seem to figure out how to map 
for example ata21.00 to an disk so I can do a deep badblock check.


I have to complete boot with kernel crash log saved with additional 
information.


http://paste.debian.net/plainh/96325d89

Can somebody take a look and maybe help?


08:00.0 SCSI storage controller: Areca Technology Corp. ARC-1300ix-16 
16-Port PCI-Express to SAS Non-RAID Host Adapter (rev 02)


Kind regards,

Jelle de Jong
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 07/24] aacraid: Process Error for response I/O

2017-01-26 Thread Johannes Thumshirn 
On Wed, Jan 25, 2017 at 10:00:54AM -0800, Raghava Aditya Renukunta wrote:
> Make sure that the driver processes error conditions even in the fast
> response path for response from the adapter.
> 
> Signed-off-by: Raghava Aditya Renukunta 
> 
> Signed-off-by: Dave Carroll 
> 
> ---

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
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 06/24] aacraid: Reworked scsi command submission path

2017-01-26 Thread Johannes Thumshirn 
On Wed, Jan 25, 2017 at 10:00:53AM -0800, Raghava Aditya Renukunta wrote:
> Moved the READ and WRITE switch cases to the top. Added a  default
> case to the switch case and replaced duplicate scsi result value with a
> macro.

Can you please explain why you did the changes as well?

-- 
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
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] scsi_debug: Add OPTIMAL TRANSFER LENGTH GRANULARITY option.

2017-01-26 Thread Lukas Herbolt 
Signed-off-by: Lukas Herbolt 
---
 drivers/scsi/scsi_debug.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index cf04a36..6910ea1 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -125,6 +125,7 @@ static const char *sdebug_version_date = "20160430";
 #define DEF_OPTS   0
 #define DEF_OPT_BLKS 1024
 #define DEF_PHYSBLK_EXP 0
+#define DEF_OPT_XFERLEN_EXP 0
 #define DEF_PTYPE   TYPE_DISK
 #define DEF_REMOVABLE false
 #define DEF_SCSI_LEVEL   7/* INQUIRY, byte2 [6->SPC-4; 7->SPC-5] */
@@ -590,6 +591,7 @@ static int sdebug_num_tgts = DEF_NUM_TGTS; /* targets per 
host */
 static int sdebug_opt_blks = DEF_OPT_BLKS;
 static int sdebug_opts = DEF_OPTS;
 static int sdebug_physblk_exp = DEF_PHYSBLK_EXP;
+static int sdebug_opt_xferlen_exp = DEF_OPT_XFERLEN_EXP;
 static int sdebug_ptype = DEF_PTYPE; /* SCSI peripheral device type */
 static int sdebug_scsi_level = DEF_SCSI_LEVEL;
 static int sdebug_sector_size = DEF_SECTOR_SIZE;
@@ -1205,7 +1207,10 @@ static int inquiry_vpd_b0(unsigned char *arr)
memcpy(arr, vpdb0_data, sizeof(vpdb0_data));
 
/* Optimal transfer length granularity */
-   gran = 1 << sdebug_physblk_exp;
+   if (sdebug_opt_xferlen_exp != 0 && sdebug_physblk_exp < 
sdebug_opt_xferlen_exp)
+   gran = 1 <<  sdebug_opt_xferlen_exp;
+   else
+   gran = 1 << sdebug_physblk_exp;
put_unaligned_be16(gran, arr + 2);
 
/* Maximum Transfer Length */
@@ -4161,6 +4166,7 @@ module_param_named(num_tgts, sdebug_num_tgts, int, 
S_IRUGO | S_IWUSR);
 module_param_named(opt_blks, sdebug_opt_blks, int, S_IRUGO);
 module_param_named(opts, sdebug_opts, int, S_IRUGO | S_IWUSR);
 module_param_named(physblk_exp, sdebug_physblk_exp, int, S_IRUGO);
+module_param_named(opt_xferlen_exp, sdebug_opt_xferlen_exp, int, S_IRUGO);
 module_param_named(ptype, sdebug_ptype, int, S_IRUGO | S_IWUSR);
 module_param_named(removable, sdebug_removable, bool, S_IRUGO | S_IWUSR);
 module_param_named(scsi_level, sdebug_scsi_level, int, S_IRUGO);
@@ -4212,6 +4218,7 @@ MODULE_PARM_DESC(num_tgts, "number of targets per host to 
simulate(def=1)");
 MODULE_PARM_DESC(opt_blks, "optimal transfer length in blocks (def=1024)");
 MODULE_PARM_DESC(opts, "1->noise, 2->medium_err, 4->timeout, 
8->recovered_err... (def=0)");
 MODULE_PARM_DESC(physblk_exp, "physical block exponent (def=0)");
+MODULE_PARM_DESC(opt_xferlen_exp, "optimal transfer length granularity 
exponent (def=physblk_exp)");
 MODULE_PARM_DESC(ptype, "SCSI peripheral type(def=0[disk])");
 MODULE_PARM_DESC(removable, "claim to have removable media (def=0)");
 MODULE_PARM_DESC(scsi_level, "SCSI level to simulate(def=7[SPC-5])");
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 05/24] aacraid: Retrieve and update the device types

2017-01-26 Thread Johannes Thumshirn 
On Wed, Jan 25, 2017 at 10:00:52AM -0800, Raghava Aditya Renukunta wrote:
> This patch adds support to retrieve the type of each adapter connected
> device. Applicable to HBA1000 and SmartIOC2000 products
> 
> Signed-off-by: Raghava Aditya Renukunta 
> 
> Signed-off-by: Dave Carroll 
> 
> ---

[...]

>  /*
>   *   Adapter Information Block
>   *
> @@ -1056,7 +1091,28 @@ struct aac_supplement_adapter_info
>   /* StructExpansion == 1 */
>   __le32  FeatureBits3;
>   __le32  SupportedPerformanceModes;
> - __le32  ReservedForFutureGrowth[80];
> + u8  HostBusType;/* uses HOST_BUS_TYPE_xxx defines */
> + u8  HostBusWidth;   /* actual width in bits or links */
> + u16 HostBusSpeed;   /* actual bus speed/link rate in MHz */
> + u8  MaxRRCDrives;   /* max. number of ITP-RRC drives/pool */
> + u8  MaxDiskXtasks;  /* max. possible num of DiskX Tasks */
> +
> + u8  CpldVerLoaded;
> + u8  CpldVerInFlash;
> +
> + __le64  MaxRRCCapacity;
> + __le32  CompiledMaxHistLogLevel;
> + u8  CustomBoardName[12];
> + u16 SupportedCntlrMode; /* identify supported controller mode */
> + u16 ReservedForFuture16;
> + __le32  SupportedOptions3;  /* reserved for future options */
> +
> + __le16  VirtDeviceBus;  /* virt. SCSI device for Thor */
> + __le16  VirtDeviceTarget;
> + __le16  VirtDeviceLUN;
> + __le16  Unused;
> + __le32  ReservedForFutureGrowth[68];
> +

:-(

Appart from that,
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
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/2] virtio_scsi: Implement fc_host

2017-01-26 Thread kbuild test robot 
Hi Fam,

[auto build test ERROR on linus/master]
[also build test ERROR on v4.10-rc5 next-20170125]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Fam-Zheng/virtio-scsi-Implement-FC_HOST-feature/20170126-114655
config: i386-randconfig-h0-01261251 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=i386 

All errors (new ones prefixed by >>):

>> ERROR: "fc_release_transport" [drivers/scsi/virtio_scsi.ko] undefined!
>> ERROR: "fc_attach_transport" [drivers/scsi/virtio_scsi.ko] undefined!

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


.config.gz
Description: application/gzip

Re: [PATCH V2 04/24] aacraid: Added sa firmware support

2017-01-26 Thread Johannes Thumshirn 
On Wed, Jan 25, 2017 at 10:00:51AM -0800, Raghava Aditya Renukunta wrote:
> sa_firmware adds the capability to differentiate the new SmartIOC family
> of adapters from the series 8 and below.
> 
> Signed-off-by: Raghava Aditya Renukunta 
> 
> Signed-off-by: Dave Carroll 
> 
> ---

[...]

> @@ -465,9 +473,13 @@ void aac_define_int_mode(struct aac_dev *dev)
>   if (dev->max_msix > msi_count)
>   dev->max_msix = msi_count;
>   }
> - dev->vector_cap =
> - (dev->scsi_host_ptr->can_queue + AAC_NUM_MGT_FIB) /
> - msi_count;
> + if (dev->comm_interface == AAC_COMM_MESSAGE_TYPE3 && dev->sa_firmware)
> + dev->vector_cap = (dev->scsi_host_ptr->can_queue +
> + AAC_NUM_MGT_FIB);

'+' has precedence before '=' and you do no multiplication here, so no brackets
needed.

[...]


-- 
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
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 03/24] aacraid: added support for init_struct_8

2017-01-26 Thread Johannes Thumshirn 
On Wed, Jan 25, 2017 at 10:00:50AM -0800, Raghava Aditya Renukunta wrote:
> This  patch lays the groundwork for supporting the new HBA-1000 controller
> family.A new INIT structure INIT_STRUCT_8 has been added which allows for a
> variable size for MSI-x vectors among other things,  and is used for both
> Series-8, HBA-1000 and SmartIOC-2000.
> 
> Signed-off-by: Raghava Aditya Renukunta 
> 
> Signed-off-by: Dave Carroll 
> 
> ---

[...]

> -struct aac_init
> +union aac_init
>  {
> - __le32  InitStructRevision;
> - __le32  Sa_MSIXVectors;
> - __le32  fsrev;
> - __le32  CommHeaderAddress;
> - __le32  FastIoCommAreaAddress;
> - __le32  AdapterFibsPhysicalAddress;
> - __le32  AdapterFibsVirtualAddress;
> - __le32  AdapterFibsSize;
> - __le32  AdapterFibAlign;
> - __le32  printfbuf;
> - __le32  printfbufsiz;
> - __le32  HostPhysMemPages;   /* number of 4k pages of host
> -physical memory */
> - __le32  HostElapsedSeconds; /* number of seconds since 1970. */
> - /*
> -  * ADAPTER_INIT_STRUCT_REVISION_4 begins here
> -  */
> - __le32  InitFlags;  /* flags for supported features */
> + struct _r7 {
> + __le32  InitStructRevision;
> + __le32  NoOfMSIXVectors;
> + __le32  fsrev;
> + __le32  CommHeaderAddress;
> + __le32  FastIoCommAreaAddress;
> + __le32  AdapterFibsPhysicalAddress;
> + __le32  AdapterFibsVirtualAddress;
> + __le32  AdapterFibsSize;
> + __le32  AdapterFibAlign;
> + __le32  printfbuf;
> + __le32  printfbufsiz;
> + /* number of 4k pages of host phys. mem. */
> + __le32  HostPhysMemPages;
> + /* number of seconds since 1970. */
> + __le32  HostElapsedSeconds;
> + /* ADAPTER_INIT_STRUCT_REVISION_4 begins here */
> + __le32  InitFlags;  /* flags for supported features */
>  #define INITFLAGS_NEW_COMM_SUPPORTED 0x0001
>  #define INITFLAGS_DRIVER_USES_UTC_TIME   0x0010
>  #define INITFLAGS_DRIVER_SUPPORTS_PM 0x0020
>  #define INITFLAGS_NEW_COMM_TYPE1_SUPPORTED   0x0040
>  #define INITFLAGS_FAST_JBOD_SUPPORTED0x0080
>  #define INITFLAGS_NEW_COMM_TYPE2_SUPPORTED   0x0100
> - __le32  MaxIoCommands;  /* max outstanding commands */
> - __le32  MaxIoSize;  /* largest I/O command */
> - __le32  MaxFibSize; /* largest FIB to adapter */
> - /* ADAPTER_INIT_STRUCT_REVISION_5 begins here */
> - __le32  MaxNumAif;  /* max number of aif */
> - /* ADAPTER_INIT_STRUCT_REVISION_6 begins here */
> - __le32  HostRRQ_AddrLow;
> - __le32  HostRRQ_AddrHigh;   /* Host RRQ (response queue) for SRC */
> +#define INITFLAGS_DRIVER_SUPPORTS_HBA_MODE  0x0400
> + __le32  MaxIoCommands;  /* max outstanding commands */
> + __le32  MaxIoSize;  /* largest I/O command */
> + __le32  MaxFibSize; /* largest FIB to adapter */
> + /* ADAPTER_INIT_STRUCT_REVISION_5 begins here */
> + __le32  MaxNumAif;  /* max number of aif */
> + /* ADAPTER_INIT_STRUCT_REVISION_6 begins here */
> + /* Host RRQ (response queue) for SRC */
> + __le32  HostRRQ_AddrLow;
> + __le32  HostRRQ_AddrHigh;
> + } r7;
> + struct _r8 {
> + /* ADAPTER_INIT_STRUCT_REVISION_8 */
> + __le32  InitStructRevision;
> + __le32  RRQueueCount;
> + __le32  HostElapsedSeconds; /* number of seconds since 1970. */
> + __le32  InitFlags;
> + __le32  MaxIoSize;  /* largest I/O command */
> + __le32  MaxNumAif;  /* max number of aif */
> + __le32  Reserved1;
> + __le32  Reserved2;
> + struct _rrq {
> + __le32  Host_AddrLow;
> + __le32  Host_AddrHigh;
> + __le16  MSIX_Id;
> + __le16  ElementCount;
> + __le16  Comp_Thresh;
> + __le16  Unused;
> + } rrq[1];   /* up to 64 RRQ addresses */
> + } r8;
>  };

Is this CamelCase pollution really needed? You're touching it anyways, so
wouldn't it be a great chance to run 
sed -e 's/\([A-Z]\)/_\l\1/g' -e 's/^_\([a-z]\)/\1/g'
on the new structures?

[...]

-- 
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
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to

Re: [PATCH v2 2/2] virtio_scsi: Implement fc_host

2017-01-26 Thread Christoph Hellwig 
On Thu, Jan 26, 2017 at 11:41:09AM +0800, Fam Zheng wrote:
> This implements the VIRTIO_SCSI_F_FC_HOST feature by reading the config
> fields and presenting them as sysfs fc_host attributes. The config
> change handler is added here because primary_active will toggle during
> migration.

This needs a way, way better description on why the heck we would
even bother with something odd like this.

Also please coordinate with Cathy's "lightweight" FC attrs for storsvcs
as the code seems to be clearly following the "lead" set by Hyper-V.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 02/24] aacraid: Added aacraid.h include guard

2017-01-26 Thread Johannes Thumshirn 
On Wed, Jan 25, 2017 at 10:00:49AM -0800, Raghava Aditya Renukunta wrote:
> Added aacraid.h include guard
> 
> Signed-off-by: Raghava Aditya Renukunta 
> 
> Signed-off-by: Dave Carroll 
> 
> ---

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
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 01/24] aacraid: Remove duplicate irq management code

2017-01-26 Thread Johannes Thumshirn 
On Wed, Jan 25, 2017 at 10:00:48AM -0800, Raghava Aditya Renukunta wrote:
> Removed duplicate code that for acquiring and releasing irqs
> 
> Signed-off-by: Raghava Aditya Renukunta 
> 
> Signed-off-by: Dave Carroll 
> 
> ---

One could argue 'acquiring and releasing irqs' is two different things so it
should be split into two different patches ;-). But if noone else objects:

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
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] SCSI core: Fix WRITE SAME handling

2017-01-26 Thread h...@lst.de 
On Wed, Jan 25, 2017 at 10:44:41PM +, Bart Van Assche wrote:
> On Wed, 2017-01-25 at 11:37 +0100, Christoph Hellwig wrote:
> > > Fixes: f80de881d8df ("sd: remove __data_len hack for WRITE SAME")
> > 
> > I think the better fix is to simply revert this commit.
> 
> Hello Christoph,
> 
> Do you mean reverting commit f80de881d8df only for kernel release v4.10
> or also for future releases? How about resubmitting a reworked version of
> commit f80de881d8df for kernel v4.11?

Let's revert it for good for now.  This was a braino for me as the
real write same requests are very different from discards implemented
as write same.  I have a WIP series that would change a lot in this
area, so additional churn before that would not be productive.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 1/4] mpt3sas: Added print to notify cable running at a degraded speed.

2017-01-26 Thread Johannes Thumshirn 
On Mon, Jan 23, 2017 at 03:26:07PM +0530, Chaitra P B wrote:
> Driver processes the event MPI26_EVENT_ACTIVE_CABLE_DEGRADED
> when a cable is present and is running at a degraded speed
> (below the SAS3 12 Gb/s rate). Prints added
> to inform the user that the cable is not running at
> optimal speed.
> 
> Signed-off-by: Chaitra P B 
> Signed-off-by: Suganath Prabu S 
> ---


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
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/4] mpt3sas: Fix for Crusader to achieve product targets with SAS devices.

2017-01-26 Thread Johannes Thumshirn 
On Mon, Jan 23, 2017 at 03:26:08PM +0530, Chaitra P B wrote:
> Small glitch/degraded performance in Crusader is improved with SAS
> drives by removing unnecessary spinlocks while clearing scsi command
> in drivers internal lookup table.
> 
> Signed-off-by: Chaitra P B 
> Signed-off-by: Suganath Prabu S 
> ---


Sorry for misreading the patch, it's exactly what I've requestd.

Therefore:
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
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Bug 176951] boot fails unless acpi=off Acer Travelmate X-349

2017-01-26 Thread bugzilla-daemon 
https://bugzilla.kernel.org/show_bug.cgi?id=176951

--- Comment #19 from Dmitry Sysoletin  ---
(In reply to Zhang Rui from comment #11)
> please check if the latest upstream kernel works for you or not.
> If yes, I will close this bug as the original bug has been fixed by BIOS
> upgrade, and the arch linux 4.9 kernel issue sounds like a Distro problem to
> me.

I'am checked 4.10.0-rc5, and have about one successful reboot for 10 reboot
attempts. I have 2 different types of fail - most often it happens about at 0.5
second uptime, but I catched another failure on ~24 second (screenshots
attached).
System is 64-bit, booting with legacy mode, BIOS updated to 1.07 (it seems like
switching legacy <-> UEFI don't make sense). Sometimes I able to boot, if I
press F2 at power-up moment, enter EFI-shell, press ctrl+alt+del - seems like
this increasing boot chances.
I tried with acpi=ht also, but this don't changes behaviour.


I'am able to boot ubuntu with 4.2 kernel from liveusb - and I see messages that
some people able to run other distros. That distros use vanilla kernel, or have
some patches? I guess ubuntu not using vanilla kernel - there must be some
patches fixing/masking this issue.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Bug 176951] boot fails unless acpi=off Acer Travelmate X-349

2017-01-26 Thread bugzilla-daemon 
https://bugzilla.kernel.org/show_bug.cgi?id=176951

--- Comment #18 from Dmitry Sysoletin  ---
Created attachment 253191
  --> https://bugzilla.kernel.org/attachment.cgi?id=253191=edit
Stacktrace screenshot 2

Another 4.10.0-rc5 fail

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Bug 176951] boot fails unless acpi=off Acer Travelmate X-349

2017-01-26 Thread bugzilla-daemon 
https://bugzilla.kernel.org/show_bug.cgi?id=176951

Dmitry Sysoletin  changed:

   What|Removed |Added

 CC||dmitry.sysole...@gmail.com

--- Comment #17 from Dmitry Sysoletin  ---
Created attachment 253181
  --> https://bugzilla.kernel.org/attachment.cgi?id=253181=edit
Stacktrace screenshot 1

Screenshot of hung kernel 4.10.0-rc5

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html