Re: [PATCH 1/2] block: Implement global tagset

[Arun Easi]

On Thu, 6 Apr 2017, 1:49am, Hannes Reinecke wrote:

> On 04/06/2017 08:27 AM, Arun Easi wrote:
> > Hi Hannes,
> > 
> > Thanks for taking a crack at the issue. My comments below..
> > 
> > On Tue, 4 Apr 2017, 5:07am, Hannes Reinecke wrote:
> > 
> >> Most legacy HBAs have a tagset per HBA, not per queue. To map
> >> these devices onto block-mq this patch implements a new tagset
> >> flag BLK_MQ_F_GLOBAL_TAGS, which will cause the tag allocator
> >> to use just one tagset for all hardware queues.
> >>
> >> Signed-off-by: Hannes Reinecke <h...@suse.com>
> >> ---
> >>  block/blk-mq-tag.c | 12 
> >>  block/blk-mq.c | 10 --
> >>  include/linux/blk-mq.h |  1 +
> >>  3 files changed, 17 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/block/blk-mq-tag.c b/block/blk-mq-tag.c
> >> index e48bc2c..a14e76c 100644
> >> --- a/block/blk-mq-tag.c
> >> +++ b/block/blk-mq-tag.c
> >> @@ -276,9 +276,11 @@ static void blk_mq_all_tag_busy_iter(struct 
> >> blk_mq_tags *tags,
> >>  void blk_mq_tagset_busy_iter(struct blk_mq_tag_set *tagset,
> >>busy_tag_iter_fn *fn, void *priv)
> >>  {
> >> -  int i;
> >> +  int i, lim = tagset->nr_hw_queues;
> >>  
> >> -  for (i = 0; i < tagset->nr_hw_queues; i++) {
> >> +  if (tagset->flags & BLK_MQ_F_GLOBAL_TAGS)
> >> +  lim = 1;
> >> +  for (i = 0; i < lim; i++) {
> >>if (tagset->tags && tagset->tags[i])
> >>blk_mq_all_tag_busy_iter(tagset->tags[i], fn, priv);
> >>}
> >> @@ -287,12 +289,14 @@ void blk_mq_tagset_busy_iter(struct blk_mq_tag_set 
> >> *tagset,
> >>  
> >>  int blk_mq_reinit_tagset(struct blk_mq_tag_set *set)
> >>  {
> >> -  int i, j, ret = 0;
> >> +  int i, j, ret = 0, lim = set->nr_hw_queues;
> >>  
> >>if (!set->ops->reinit_request)
> >>goto out;
> >>  
> >> -  for (i = 0; i < set->nr_hw_queues; i++) {
> >> +  if (set->flags & BLK_MQ_F_GLOBAL_TAGS)
> >> +  lim = 1;
> >> +  for (i = 0; i < lim; i++) {
> >>struct blk_mq_tags *tags = set->tags[i];
> >>  
> >>for (j = 0; j < tags->nr_tags; j++) {
> >> diff --git a/block/blk-mq.c b/block/blk-mq.c
> >> index 159187a..db96ed0 100644
> >> --- a/block/blk-mq.c
> >> +++ b/block/blk-mq.c
> >> @@ -2061,6 +2061,10 @@ static bool __blk_mq_alloc_rq_map(struct 
> >> blk_mq_tag_set *set, int hctx_idx)
> >>  {
> >>int ret = 0;
> >>  
> >> +  if ((set->flags & BLK_MQ_F_GLOBAL_TAGS) && hctx_idx != 0) {
> >> +  set->tags[hctx_idx] = set->tags[0];
> >> +  return true;
> >> +  }
> > 
:
> 
> > BTW, if you would like me to try out this patch on my setup, please let me 
> > know.
> > 
> Oh, yes. Please do.
> 

Ran the tests on my setup (Dell R730, 2 Node). This change did not drop 
any IOPs (got ~2M 512b). The cache miss percentage was varying based on if 
the tests were running on one node or both (latter yperformed worse). All 
interrupts were directed to only 1 node. Interestingly, the cache miss 
percentage was lowest when MQ was off.

I hit a fdisk hang (open path), btw, not sure if it has anything todo with 
this change, though.

Notes and hang stack attached.

Let me know if you are interested in any specific perf event/command-line.

Regards,
-Arunperf stat, ran on a short 10 second load.

---1port-1node-new-mq
 Performance counter stats for 'CPU(s) 2':

 188,642,696  LLC-loads(66.66%)
   3,615,142  LLC-load-misses  #1.92% of all LL-cache hits (66.67%)
  86,488,341  LLC-stores   (33.34%)
  10,820,977  LLC-store-misses (33.33%)
 391,370,104  cache-references (49.99%)
  14,498,491  cache-misses #3.705 % of all cache refs  (66.66%)

---1port-1node-mq---
 Performance counter stats for 'CPU(s) 2':

 145,025,999  LLC-loads(66.67%)
   3,793,427  LLC-load-misses  #2.62% of all LL-cache hits (66.67%)
  60,878,939  LLC-stores   (33.33%)
   8,044,714  LLC-store-misses (33.33%)
 294,713,070  cache-references 

Re: [PATCH 1/2] block: Implement global tagset

[Arun Easi]

Hi Hannes,

Thanks for taking a crack at the issue. My comments below..

On Tue, 4 Apr 2017, 5:07am, Hannes Reinecke wrote:

> Most legacy HBAs have a tagset per HBA, not per queue. To map
> these devices onto block-mq this patch implements a new tagset
> flag BLK_MQ_F_GLOBAL_TAGS, which will cause the tag allocator
> to use just one tagset for all hardware queues.
> 
> Signed-off-by: Hannes Reinecke 
> ---
>  block/blk-mq-tag.c | 12 
>  block/blk-mq.c | 10 --
>  include/linux/blk-mq.h |  1 +
>  3 files changed, 17 insertions(+), 6 deletions(-)
> 
> diff --git a/block/blk-mq-tag.c b/block/blk-mq-tag.c
> index e48bc2c..a14e76c 100644
> --- a/block/blk-mq-tag.c
> +++ b/block/blk-mq-tag.c
> @@ -276,9 +276,11 @@ static void blk_mq_all_tag_busy_iter(struct blk_mq_tags 
> *tags,
>  void blk_mq_tagset_busy_iter(struct blk_mq_tag_set *tagset,
>   busy_tag_iter_fn *fn, void *priv)
>  {
> - int i;
> + int i, lim = tagset->nr_hw_queues;
>  
> - for (i = 0; i < tagset->nr_hw_queues; i++) {
> + if (tagset->flags & BLK_MQ_F_GLOBAL_TAGS)
> + lim = 1;
> + for (i = 0; i < lim; i++) {
>   if (tagset->tags && tagset->tags[i])
>   blk_mq_all_tag_busy_iter(tagset->tags[i], fn, priv);
>   }
> @@ -287,12 +289,14 @@ void blk_mq_tagset_busy_iter(struct blk_mq_tag_set 
> *tagset,
>  
>  int blk_mq_reinit_tagset(struct blk_mq_tag_set *set)
>  {
> - int i, j, ret = 0;
> + int i, j, ret = 0, lim = set->nr_hw_queues;
>  
>   if (!set->ops->reinit_request)
>   goto out;
>  
> - for (i = 0; i < set->nr_hw_queues; i++) {
> + if (set->flags & BLK_MQ_F_GLOBAL_TAGS)
> + lim = 1;
> + for (i = 0; i < lim; i++) {
>   struct blk_mq_tags *tags = set->tags[i];
>  
>   for (j = 0; j < tags->nr_tags; j++) {
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index 159187a..db96ed0 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -2061,6 +2061,10 @@ static bool __blk_mq_alloc_rq_map(struct 
> blk_mq_tag_set *set, int hctx_idx)
>  {
>   int ret = 0;
>  
> + if ((set->flags & BLK_MQ_F_GLOBAL_TAGS) && hctx_idx != 0) {
> + set->tags[hctx_idx] = set->tags[0];
> + return true;
> + }

So, this effectively make all request allocations to the same NUMA node 
locality of the hctx_idx 0, correct? Is the performance hit you were 
talking about in the cover letter?

Do you have any other alternatives in mind? Dynamic growing/shrinking 
tags/request-pool in hctx with a fixed base as start?

One alternative that comes to my mind is to move the divvy up logic to 
SCSI (instead of LLD doing it), IOW:

1. Have SCSI set tag_set.queue_depth to can_queue/nr_hw_queues
2. Have blk_mq_unique_tag() (or a new i/f) returning "hwq * nr_hw_queue + 
   rq->tag"

That would make the tags linear in the can_queue space, but could result 
in poor use of LLD resource if a given hctx has used up all it's tags.

On a related note, would not the current use of can_queue in SCSI lead to 
poor resource utilization in MQ cases? Like, block layer allocating 
nr_hw_queues * tags+request+driver_data.etc * can_queue, but SCSI limiting 
the number of requests to can_queue.

BTW, if you would like me to try out this patch on my setup, please let me 
know.

Regards,
-Arun

>   set->tags[hctx_idx] = blk_mq_alloc_rq_map(set, hctx_idx,
>   set->queue_depth, set->reserved_tags);
>   if (!set->tags[hctx_idx])
> @@ -2080,8 +2084,10 @@ static void blk_mq_free_map_and_requests(struct 
> blk_mq_tag_set *set,
>unsigned int hctx_idx)
>  {
>   if (set->tags[hctx_idx]) {
> - blk_mq_free_rqs(set, set->tags[hctx_idx], hctx_idx);
> - blk_mq_free_rq_map(set->tags[hctx_idx]);
> + if (!(set->flags & BLK_MQ_F_GLOBAL_TAGS) || hctx_idx == 0) {
> + blk_mq_free_rqs(set, set->tags[hctx_idx], hctx_idx);
> + blk_mq_free_rq_map(set->tags[hctx_idx]);
> + }
>   set->tags[hctx_idx] = NULL;
>   }
>  }
> diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
> index b296a90..eee27b016 100644
> --- a/include/linux/blk-mq.h
> +++ b/include/linux/blk-mq.h
> @@ -155,6 +155,7 @@ enum {
>   BLK_MQ_F_DEFER_ISSUE= 1 << 4,
>   BLK_MQ_F_BLOCKING   = 1 << 5,
>   BLK_MQ_F_NO_SCHED   = 1 << 6,
> + BLK_MQ_F_GLOBAL_TAGS= 1 << 7,
>   BLK_MQ_F_ALLOC_POLICY_START_BIT = 8,
>   BLK_MQ_F_ALLOC_POLICY_BITS = 1,
>  
> 

Re: scsi-mq - tag# and can_queue, performance.

[Arun Easi]

On Mon, 3 Apr 2017, 9:47am, Jens Axboe wrote:

> On 04/03/2017 10:41 AM, Arun Easi wrote:
> > On Mon, 3 Apr 2017, 8:20am, Bart Van Assche wrote:
> > 
> >> On Mon, 2017-04-03 at 09:29 +0200, Hannes Reinecke wrote:
> >>> On 04/03/2017 08:37 AM, Arun Easi wrote:
> >>>> If the above is true, then for a LLD to get tag# within it's max-tasks 
> >>>> range, it has to report max-tasks / number-of-hw-queues in can_queue, 
> >>>> and 
> >>>> in the I/O path, use the tag and hwq# to arrive at a index# to use. 
> >>>> This, 
> >>>> though, leads to a poor use of tag resources -- queue reaching it's 
> >>>> capacity while LLD can still take it.
> >>>
> >>> Shared tag sets continue to dog the block-mq on 'legacy' (ie non-NVMe)
> >>> HBAs. ATM the only 'real' solution to this problem is indeed have a
> >>> static split of the entire tag space by the number of hardware queues.
> >>> With the mentioned tag-starvation problem.
> >>
> >> Hello Arun and Hannes,
> >>
> >> Apparently the current blk_mq_alloc_tag_set() implementation is well suited
> >> for drivers like NVMe and ib_srp but not for traditional SCSI HBA drivers.
> >> How about adding a BLK_MQ_F_* flag that tells __blk_mq_alloc_rq_maps() to
> >> allocate a single set of tags for all hardware queues and also to add a 
> >> flag
> >> to struct scsi_host_template such that SCSI LLDs can enable this behavior?
> >>
> > 
> > Hi Bart,
> > 
> > This would certainly be beneficial in my case. Moreover, it certainly 
> > makes sense to move the logic up where multiple drivers can leverage. 
> > 
> > Perhaps, use percpu_ida* interfaces to do that, but I think I read 
> > somewhere that, it is not efficient (enough?) and is the reason to go the 
> > current way for block tags.
> 
> You don't have to change the underlying tag generation to solve this
> problem, Bart already pretty much outlined a fix that would work.
> percpu_ida works fine if you never use more than roughly half the
> available space, it's a poor fit for request tags where we want to
> retain good behavior and scaling at or near tag exhaustion. That's why
> blk-mq ended up rolling its own, which is now generically available as
> lib/sbitmap.c.
> 

Sounds good. Thanks for the education, Jens.

Regards,
-Arun

Re: scsi-mq - tag# and can_queue, performance.

[Arun Easi]

On Mon, 3 Apr 2017, 8:20am, Bart Van Assche wrote:

> On Mon, 2017-04-03 at 09:29 +0200, Hannes Reinecke wrote:
> > On 04/03/2017 08:37 AM, Arun Easi wrote:
> > > If the above is true, then for a LLD to get tag# within it's max-tasks 
> > > range, it has to report max-tasks / number-of-hw-queues in can_queue, and 
> > > in the I/O path, use the tag and hwq# to arrive at a index# to use. This, 
> > > though, leads to a poor use of tag resources -- queue reaching it's 
> > > capacity while LLD can still take it.
> >
> > Shared tag sets continue to dog the block-mq on 'legacy' (ie non-NVMe)
> > HBAs. ATM the only 'real' solution to this problem is indeed have a
> > static split of the entire tag space by the number of hardware queues.
> > With the mentioned tag-starvation problem.
> 
> Hello Arun and Hannes,
> 
> Apparently the current blk_mq_alloc_tag_set() implementation is well suited
> for drivers like NVMe and ib_srp but not for traditional SCSI HBA drivers.
> How about adding a BLK_MQ_F_* flag that tells __blk_mq_alloc_rq_maps() to
> allocate a single set of tags for all hardware queues and also to add a flag
> to struct scsi_host_template such that SCSI LLDs can enable this behavior?
> 

Hi Bart,

This would certainly be beneficial in my case. Moreover, it certainly 
makes sense to move the logic up where multiple drivers can leverage. 

Perhaps, use percpu_ida* interfaces to do that, but I think I read 
somewhere that, it is not efficient (enough?) and is the reason to go the 
current way for block tags.

Regards,
-Arun

Re: scsi-mq - tag# and can_queue, performance.

[Arun Easi]

On Mon, 3 Apr 2017, 12:29am, Hannes Reinecke wrote:

> On 04/03/2017 08:37 AM, Arun Easi wrote:
> > Hi Folks,
> > 
> > I would like to seek your input on a few topics on SCSI / block 
> > multi-queue.
> > 
> > 1. Tag# generation.
> > 
> > The context is with SCSI MQ on. My question is, what should a LLD do to 
> > get request tag values in the range 0 through can_queue - 1 across *all* 
> > of the queues. In our QLogic 41XXX series of adapters, we have a per 
> > session submit queue, a shared task memory (shared across all queues) and 
> > N completion queues (separate MSI-X vectors). We report N as the 
> > nr_hw_queues. I would like to, if possible, use the block layer tags to 
> > index into the above shared task memory area.
> > 
> > From looking at the scsi/block source, it appears that when a LLD reports 
> > a value say #C, in can_queue (via scsi_host_template), that value is used 
> > as the max depth when corresponding block layer queues are created. So, 
> > while SCSI restricts the number of commands to LLD at #C, the request tag 
> > generated across any of the queues can range from 0..#C-1. Please correct 
> > me if I got this wrong.
> > 
> > If the above is true, then for a LLD to get tag# within it's max-tasks 
> > range, it has to report max-tasks / number-of-hw-queues in can_queue, and 
> > in the I/O path, use the tag and hwq# to arrive at a index# to use. This, 
> > though, leads to a poor use of tag resources -- queue reaching it's 
> > capacity while LLD can still take it.
> > 
> Yep.
> 
> > blk_mq_unique_tag() would not work here, because it just puts the hwq# in 
> > the upper 16 bits, which need not fall in the max-tasks range.
> > 
> > Perhaps the current MQ model is to cater to a queue pair 
> > (submit/completion) kind of hardware model; nevertheless I would like to 
> > know how other hardware variants can makes use of it.
> > 
> He. Welcome to the club.
> 
> Shared tag sets continue to dog the block-mq on 'legacy' (ie non-NVMe)
> HBAs. ATM the only 'real' solution to this problem is indeed have a
> static split of the entire tag space by the number of hardware queues.
> With the mentioned tag-starvation problem.
> 
> If we were to continue with the tag to hardware ID mapping, we would
> need to implement a dynamic tag space mapping onto hardware queues.
> My idea to that would be to map the entire tag space, but rather the
> individual bit words onto the hardware queue. Then we could make the
> mapping dynamic, where there individual words are mapped onto the queues
> only as needed.
> However, the _one_ big problem we're facing here is timeouts.
> With the 1:1 mapping between tags and hardware IDs we can only re-use
> the tag once the timeout is _definitely_ resolved. But this means
> the command will be active, and we cannot return blk_mq_complete() until
> the timeout itself has been resolved.
> With FC this essentially means until the corresponding XIDs are safe to
> re-use, ie after all ABRT/RRQ etc processing has been completed.
> Hence we totally lose the ability to return the command itself with
> -ETIMEDOUT and continue with I/O processing even though the original XID
> is still being held by firmware.
> 
> In the light of this I wonder if it wouldn't be better to completely
> decouple block-layer tags and hardware IDs, and have an efficient
> algorithm mapping the block-layer tags onto hardware IDs.
> That should avoid the arbitrary tag starvation problem, and would allow
> us to handle timeouts efficiently.
> Of course, we don't _have_ such an efficient algorithm; maybe it's time
> to have a generic one within the kernel as quite some drivers would
> _love_ to just use the generic implementation here.
> (qla2xxx, lpfc, fcoe, mpt3sas etc all suffer from the same problem)
> 
> > 2. mq vs non-mq performance gain.
> > 
> > This is more like a poll, I guess. I was wondering what performance gains 
> > folks are observing with SCSI MQ on. I saw Christoph H.'s slide deck that 
> > has one slide that shows a 200k IOPS gain.
> > 
> > From my testing, though, I was not lucky to observe that big of a change. 
> > In fact, the difference was not even noticeable(*). For e.g., for 512 
> > bytes random read test, in both cases, gave me in the vicinity of 2M IOPs. 
> > When I say both cases, I meant, one with scsi_mod's use_blk_mq set to 0 
> > and another with 1 (LLD is reloaded when it is done). I only used one NUMA 
> > node for this run. The test was run on a x86_64 setup.
> > 
> You _really_ should have listened to my talk at VAULT.

Would you have a slide deck / minutes that 

scsi-mq - tag# and can_queue, performance.

[Arun Easi]

Hi Folks,

I would like to seek your input on a few topics on SCSI / block 
multi-queue.

1. Tag# generation.

The context is with SCSI MQ on. My question is, what should a LLD do to 
get request tag values in the range 0 through can_queue - 1 across *all* 
of the queues. In our QLogic 41XXX series of adapters, we have a per 
session submit queue, a shared task memory (shared across all queues) and 
N completion queues (separate MSI-X vectors). We report N as the 
nr_hw_queues. I would like to, if possible, use the block layer tags to 
index into the above shared task memory area.

>From looking at the scsi/block source, it appears that when a LLD reports 
a value say #C, in can_queue (via scsi_host_template), that value is used 
as the max depth when corresponding block layer queues are created. So, 
while SCSI restricts the number of commands to LLD at #C, the request tag 
generated across any of the queues can range from 0..#C-1. Please correct 
me if I got this wrong.

If the above is true, then for a LLD to get tag# within it's max-tasks 
range, it has to report max-tasks / number-of-hw-queues in can_queue, and 
in the I/O path, use the tag and hwq# to arrive at a index# to use. This, 
though, leads to a poor use of tag resources -- queue reaching it's 
capacity while LLD can still take it.

blk_mq_unique_tag() would not work here, because it just puts the hwq# in 
the upper 16 bits, which need not fall in the max-tasks range.

Perhaps the current MQ model is to cater to a queue pair 
(submit/completion) kind of hardware model; nevertheless I would like to 
know how other hardware variants can makes use of it.

2. mq vs non-mq performance gain.

This is more like a poll, I guess. I was wondering what performance gains 
folks are observing with SCSI MQ on. I saw Christoph H.'s slide deck that 
has one slide that shows a 200k IOPS gain.

>From my testing, though, I was not lucky to observe that big of a change. 
In fact, the difference was not even noticeable(*). For e.g., for 512 
bytes random read test, in both cases, gave me in the vicinity of 2M IOPs. 
When I say both cases, I meant, one with scsi_mod's use_blk_mq set to 0 
and another with 1 (LLD is reloaded when it is done). I only used one NUMA 
node for this run. The test was run on a x86_64 setup.

* See item 3 for a special handling.

3. add_random slowness.

One thing I observed with MQ on and off was this block layer tunable, 
add_random, which as I understand is to tune disk entropy contribution. 
With non-MQ, it is turned on, and with MQ, it is turned off by default.

This got noticed because, when I was running multi-port testing, there was 
a big drop in IOPs with and without MQ (~200K IOPS to 1M+ IOPs when the 
test ran on same NUMA node / across NUMA nodes).

Just wondering why we have it ON on one setting and OFF on another.

Sorry for the rather long e-mail, but your comments/thoughts are much 
appreciated.

Regards,
-Arun

Re: [PATCH V2 net-next 1/2] qed: Add support for hardware offloaded FCoE.

[Arun Easi]

On Mon, 30 Jan 2017, 1:44am, Hannes Reinecke wrote:

> On 01/25/2017 09:33 PM, Dupuis, Chad wrote:
> > From: Arun Easi <arun.e...@qlogic.com>
> > 
> > This adds the backbone required for the various HW initalizations
> > which are necessary for the FCoE driver (qedf) for QLogic FastLinQ
> > 4 line of adapters - FW notification, resource initializations, etc.
> > 
> > Signed-off-by: Arun Easi <arun.e...@cavium.com>
> > Signed-off-by: Yuval Mintz <yuval.mi...@cavium.com>
> > ---
> >  drivers/net/ethernet/qlogic/Kconfig   |   3 +
> >  drivers/net/ethernet/qlogic/qed/Makefile  |   1 +
> >  drivers/net/ethernet/qlogic/qed/qed.h |  11 +
> >  drivers/net/ethernet/qlogic/qed/qed_cxt.c |  98 ++-
> >  drivers/net/ethernet/qlogic/qed/qed_cxt.h |   3 +
> >  drivers/net/ethernet/qlogic/qed/qed_dcbx.c|  13 +-
> >  drivers/net/ethernet/qlogic/qed/qed_dcbx.h|   5 +-
> >  drivers/net/ethernet/qlogic/qed/qed_dev.c | 205 -
> >  drivers/net/ethernet/qlogic/qed/qed_dev_api.h |  42 +
> >  drivers/net/ethernet/qlogic/qed/qed_fcoe.c| 990 
> > ++
> >  drivers/net/ethernet/qlogic/qed/qed_fcoe.h|  52 ++
> >  drivers/net/ethernet/qlogic/qed/qed_hsi.h | 781 -
> >  drivers/net/ethernet/qlogic/qed/qed_hw.c  |   3 +
> >  drivers/net/ethernet/qlogic/qed/qed_ll2.c |  25 +
> >  drivers/net/ethernet/qlogic/qed/qed_ll2.h |   2 +-
> >  drivers/net/ethernet/qlogic/qed/qed_main.c|   7 +
> >  drivers/net/ethernet/qlogic/qed/qed_mcp.c |   3 +
> >  drivers/net/ethernet/qlogic/qed/qed_mcp.h |   1 +
> >  drivers/net/ethernet/qlogic/qed/qed_reg_addr.h|   8 +
> >  drivers/net/ethernet/qlogic/qed/qed_sp.h  |   4 +
> >  drivers/net/ethernet/qlogic/qed/qed_sp_commands.c |   3 +
> >  include/linux/qed/common_hsi.h|  10 +-
> >  include/linux/qed/fcoe_common.h   | 715 
> >  include/linux/qed/qed_fcoe_if.h   | 145 
> >  include/linux/qed/qed_if.h|  41 +-
> >  25 files changed, 3152 insertions(+), 19 deletions(-)
> >  create mode 100644 drivers/net/ethernet/qlogic/qed/qed_fcoe.c
> >  create mode 100644 drivers/net/ethernet/qlogic/qed/qed_fcoe.h
> >  create mode 100644 include/linux/qed/fcoe_common.h
> >  create mode 100644 include/linux/qed/qed_fcoe_if.h
> > 
> [ .. ]
> > diff --git a/drivers/net/ethernet/qlogic/qed/qed_dcbx.h 
> > b/drivers/net/ethernet/qlogic/qed/qed_dcbx.h
> > index d70300f..0fabe97 100644
> > --- a/drivers/net/ethernet/qlogic/qed/qed_dcbx.h
> > +++ b/drivers/net/ethernet/qlogic/qed/qed_dcbx.h
> > @@ -57,7 +57,6 @@ struct qed_dcbx_app_data {
> > u8 tc;  /* Traffic Class */
> >  };
> >  
> > -#ifdef CONFIG_DCB
> >  #define QED_DCBX_VERSION_DISABLED   0
> >  #define QED_DCBX_VERSION_IEEE   1
> >  #define QED_DCBX_VERSION_CEE2
> > @@ -73,7 +72,6 @@ struct qed_dcbx_set {
> > struct qed_dcbx_admin_params config;
> > u32 ver_num;
> >  };
> > -#endif
> >  
> >  struct qed_dcbx_results {
> > bool dcbx_enabled;
> > @@ -97,9 +95,8 @@ struct qed_dcbx_info {
> > struct qed_dcbx_results results;
> > struct dcbx_mib operational;
> > struct dcbx_mib remote;
> > -#ifdef CONFIG_DCB
> > struct qed_dcbx_set set;
> > -#endif
> > +   struct qed_dcbx_get get;
> > u8 dcbx_cap;
> >  };
> >  
> Why did you remove the dependency on 'CONFIG_DCB'?

Thanks Hannes for the review.

The offloaded functions are not dependent on CONFIG_DCB; the DCB
functionality is handled by the firmware for them. The above '#ifdef'-ed 
part is shared by offloaded functions, and should be present regardless of 
the CONFIG_DCB setting.

Regards,
-Arun

> 
> Other than that:
> 
> Reviewed-by: Hannes Reinecke <h...@suse.com>
--
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] qed: Add support for hardware offloaded FCoE.

[Arun Easi]

This will be addressed in v2 of the series. The warning pops up when 
CONFIG_DCB is not set. Thanks.

Regards,
-Arun

On Tue, 17 Jan 2017, 6:35am, kbuild test robot wrote:

> Hi Arun,
> 
> [auto build test WARNING on net-next/master]
> [also build test WARNING on v4.10-rc4 next-20170117]
> [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/Dupuis-Chad/Add-QLogic-FastLinQ-FCoE-qedf-driver/20170117-052438
> config: i386-randconfig-c0-01172134 (attached as .config)
> compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
> reproduce:
> # save the attached .config to linux build tree
> make ARCH=i386 
> 
> All warnings (new ones prefixed by >>):
> 
>In file included from drivers/net/ethernet/qlogic/qed/qed.h:49:0,
> from drivers/net/ethernet/qlogic/qed/qed_cxt.c:44:
> >> include/linux/qed/qed_if.h:428:37: warning: 'struct qed_dcbx_get' declared 
> >> inside parameter list
>  void (*dcbx_aen)(void *dev, struct qed_dcbx_get *get, u32 mib_type);
> ^
> >> include/linux/qed/qed_if.h:428:37: warning: its scope is only this 
> >> definition or declaration, which is probably not what you want
> --
>In file included from drivers/net/ethernet/qlogic/qed/qed.h:49:0,
> from drivers/net/ethernet/qlogic/qed/qed_dcbx.c:41:
> >> include/linux/qed/qed_if.h:428:37: warning: 'struct qed_dcbx_get' declared 
> >> inside parameter list
>  void (*dcbx_aen)(void *dev, struct qed_dcbx_get *get, u32 mib_type);
> ^
> >> include/linux/qed/qed_if.h:428:37: warning: its scope is only this 
> >> definition or declaration, which is probably not what you want
>drivers/net/ethernet/qlogic/qed/qed_dcbx.c: In function 'qed_dcbx_aen':
>drivers/net/ethernet/qlogic/qed/qed_dcbx.c:873:42: error: 'struct 
> qed_dcbx_info' has no member named 'get'
>   op->dcbx_aen(cookie, >p_dcbx_info->get, mib_type);
>  ^
>drivers/net/ethernet/qlogic/qed/qed_dcbx.c: In function 
> 'qed_dcbx_mib_update_event':
>drivers/net/ethernet/qlogic/qed/qed_dcbx.c:902:2: error: implicit 
> declaration of function 'qed_dcbx_get_params' 
> [-Werror=implicit-function-declaration]
>  qed_dcbx_get_params(p_hwfn, p_ptt, _hwfn->p_dcbx_info->get, type);
>  ^
>drivers/net/ethernet/qlogic/qed/qed_dcbx.c:902:57: error: 'struct 
> qed_dcbx_info' has no member named 'get'
>  qed_dcbx_get_params(p_hwfn, p_ptt, _hwfn->p_dcbx_info->get, type);
> ^
>cc1: some warnings being treated as errors
> 
> vim +428 include/linux/qed/qed_if.h
> 
>412u8  name[QED_DRV_VER_STR_SIZE];
>413};
>414
>415#define ILT_PAGE_SIZE_TCFC 0x8000 /* 32KB */
>416
>417struct qed_int_info {
>418struct msix_entry   *msix;
>419u8  msix_cnt;
>420
>421/* This should be updated by the protocol driver */
>422u8  used_cnt;
>423};
>424
>425struct qed_common_cb_ops {
>426void(*link_update)(void *dev,
>427   struct qed_link_output   *link);
>  > 428void(*dcbx_aen)(void *dev, struct qed_dcbx_get 
> *get, u32 mib_type);
>429};
>430
>431struct qed_selftest_ops {
>432/**
>433 * @brief selftest_interrupt - Perform interrupt test
>434 *
>435 * @param cdev
>436 *
> 
> ---
> 0-DAY kernel test infrastructureOpen Source Technology Center
> https://lists.01.org/pipermail/kbuild-all   Intel Corporation
> 
--
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: [Open-FCoE] [PATCH RFC net-next 1/5] qed: Add support for hardware offloaded FCoE.

[Arun Easi]

Hi Hannes,

Thank you for the review. Please see my comment inline..

On Wed, 28 Dec 2016, 12:41am, Hannes Reinecke wrote:

> On 12/23/2016 08:17 PM, Dupuis, Chad wrote:
> > From: Arun Easi <arun.e...@qlogic.com>
> > 
> > This adds the backbone required for the various HW initalizations
> > which are necessary for the FCoE driver (qedf) for QLogic FastLinQ
> > 4 line of adapters - FW notification, resource initializations, etc.
> > 
> > Signed-off-by: Arun Easi <arun.e...@cavium.com>
> > Signed-off-by: Yuval Mintz <yuval.mi...@cavium.com>
> > ---

--8<-- snipped --

> > diff --git a/include/linux/qed/qed_fcoe_if.h 
> > b/include/linux/qed/qed_fcoe_if.h
> > new file mode 100644
> > index 000..bd6bcb8
> > --- /dev/null
> > +++ b/include/linux/qed/qed_fcoe_if.h
> > @@ -0,0 +1,145 @@
> > +#ifndef _QED_FCOE_IF_H
> > +#define _QED_FCOE_IF_H
> > +#include 
> > +#include 
> > +struct qed_fcoe_stats {
> > +   u64 fcoe_rx_byte_cnt;
> > +   u64 fcoe_rx_data_pkt_cnt;
> > +   u64 fcoe_rx_xfer_pkt_cnt;
> > +   u64 fcoe_rx_other_pkt_cnt;
> > +   u32 fcoe_silent_drop_pkt_cmdq_full_cnt;
> > +   u32 fcoe_silent_drop_pkt_rq_full_cnt;
> > +   u32 fcoe_silent_drop_pkt_crc_error_cnt;
> > +   u32 fcoe_silent_drop_pkt_task_invalid_cnt;
> > +   u32 fcoe_silent_drop_total_pkt_cnt;
> > +
> > +   u64 fcoe_tx_byte_cnt;
> > +   u64 fcoe_tx_data_pkt_cnt;
> > +   u64 fcoe_tx_xfer_pkt_cnt;
> > +   u64 fcoe_tx_other_pkt_cnt;
> > +};
> > +
> > +struct qed_dev_fcoe_info {
> > +   struct qed_dev_info common;
> > +
> > +   void __iomem *primary_dbq_rq_addr;
> > +   void __iomem *secondary_bdq_rq_addr;
> > +};
> > +
> > +struct qed_fcoe_params_offload {
> > +   dma_addr_t sq_pbl_addr;
> > +   dma_addr_t sq_curr_page_addr;
> > +   dma_addr_t sq_next_page_addr;
> > +
> > +   u8 src_mac[ETH_ALEN];
> > +   u8 dst_mac[ETH_ALEN];
> > +
> > +   u16 tx_max_fc_pay_len;
> > +   u16 e_d_tov_timer_val;
> > +   u16 rec_tov_timer_val;
> > +   u16 rx_max_fc_pay_len;
> > +   u16 vlan_tag;
> > +
> > +   struct fc_addr_nw s_id;
> > +   u8 max_conc_seqs_c3;
> > +   struct fc_addr_nw d_id;
> > +   u8 flags;
> > +   u8 def_q_idx;
> > +};
> > +
> > +#define MAX_TID_BLOCKS_FCOE (512)
> > +struct qed_fcoe_tid {
> > +   u32 size;   /* In bytes per task */
> > +   u32 num_tids_per_block;
> > +   u8 *blocks[MAX_TID_BLOCKS_FCOE];
> > +};
> > +
> > +struct qed_fcoe_cb_ops {
> > +   struct qed_common_cb_ops common;
> > +u32 (*get_login_failures)(void *cookie);
> > +};
> > +
> > +void qed_fcoe_set_pf_params(struct qed_dev *cdev,
> > +   struct qed_fcoe_pf_params *params);
> > +
> > +/**
> > + * struct qed_fcoe_ops - qed FCoE operations.
> > + * @common:common operations pointer
> > + * @fill_dev_info: fills FCoE specific information
> > + * @param cdev
> > + * @param info
> > + * @return 0 on sucesss, otherwise error value.
> > + * @register_ops:  register FCoE operations
> > + * @param cdev
> > + * @param ops - specified using qed_iscsi_cb_ops
> > + * @param cookie - driver private
> > + * @ll2:   light L2 operations pointer
> > + * @start: fcoe in FW
> > + * @param cdev
> > + * @param tasks - qed will fill information about tasks
> > + * return 0 on success, otherwise error value.
> > + * @stop:  stops fcoe in FW
> > + * @param cdev
> > + * return 0 on success, otherwise error value.
> > + * @acquire_conn:  acquire a new fcoe connection
> > + * @param cdev
> > + * @param handle - qed will fill handle that should be
> > + * used henceforth as identifier of the
> > + * connection.
> > + * @param p_doorbell - qed will fill the address of the
> > + * doorbell.
> > + * return 0 on sucesss, otherwise error value.
> > + * @release_conn:  release a previously acquired fcoe connection
> > + * @param cdev
> > + * @param handle - the connection handle.
> > + * return 0 on success, otherwise error value.
> > + * @offload_conn:  configures an offloaded connection

Re: qed, qedi patchset submission

[Arun Easi]

Thanks for the response, Martin.

On Wed, 30 Nov 2016, 8:45am, Martin K. Petersen wrote:

> >>>>> "Arun" == Arun Easi <arun.e...@cavium.com> writes:
> 
> Arun,
> 
> Arun> So far, we have been posting qedi changes split into functional
> Arun> blocks, for review, but was not bisectable. With Martin ok to our
> Arun> request to squash all patches while committing to tree, we were
> Arun> wondering if we should post the qedi patches squashed, with all
> Arun> the Reviewed-by added, or continue to post as before?
> 
> I guess it depends how things can be split up in a bisectable fashion.

These were the patches that was sent:
  1 qed: Add support for hardware offloaded iSCSI.
  2 qed: Add iSCSI out of order packet handling.
  3 qedi: Add QLogic FastLinQ offload iSCSI driver framework.
  4 qedi: Add LL2 iSCSI interface for offload iSCSI.
  5 qedi: Add support for iSCSI session management.
  6 qedi: Add support for data path.

> 
> If the net/ pieces can be completely separated from the scsi/ pieces
> maybe it would be best to have two patches?
> 

Yes, those pieces can be completely separated; there are actually 3 parts, 
2 that goes to net/ and 1 to scsi/.

In the list above, 1 & 2 goes to net/ and are bisectable. 3 through 6 are 
scsi/ pieces, which are the ones requested for collapsing.

We will post these pieces for V3, then:

  1 [PATCH v3 net-next 1/3] qed: Add support for hardware offloaded iSCSI.
  2 [PATCH v3 net-next 2/3] qed: Add iSCSI out of order packet handling.
  3 [PATCH v3 3/3] qedi: Add QLogic FastLinQ offload iSCSI driver framework.

Regards,
-Arun
--
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: qed, qedi patchset submission

[Arun Easi]

Hi David,

As we are preparing to post V3 of the qed (drivers/net/) and qedi 
(drivers/scsi/), I would like to get your opinion on the "qedi" part. As 
qedi (the iSCSI driver) is dependent on the accompanying "qed" changes, 
qedi changes have to follow or be together with qed changes. Martin is ok 
in having you taking the qedi part as well (see below discussion) or wait 
until qed is in, and then take qedi through SCSI tree.

Hi David, Martin,

So far, we have been posting qedi changes split into functional blocks, 
for review, but was not bisectable. With Martin ok to our request to 
squash all patches while committing to tree, we were wondering if we 
should post the qedi patches squashed, with all the Reviewed-by added, or 
continue to post as before?

Regards,
-Arun

On Mon, 14 Nov 2016, 3:47pm, Martin K. Petersen wrote:

> >>>>> "Arun" == Arun Easi <arun.e...@cavium.com> writes:
> 
> Arun,
> 
> Arun> Do you have any preference or thoughts on how the "qed" patches be
> Arun> approached? Just as a reference, our rdma driver "qedr" went
> Arun> through something similar[1], and eventually "qed" patches were
> Arun> taken by David in the net tree and "qedr", in the rdma tree
> Arun> (obviously) by Doug L.
> 
> DaveM can queue the whole series or I can hold the SCSI pieces for
> rc1. Either way works for me.
> 
> However, I would like to do a review of the driver first. I will try to
> get it done this week.
> 
> 
--
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

qed, qedi patchset submission

[Arun Easi]

Hi Martin, David,

This is regarding the submission of the recent patch series we have posted
to linux-scsi and netdev:

[PATCH v2 0/6] Add QLogic FastLinQ iSCSI (qedi) driver.
[PATCH v2 1/6] qed: Add support for hardware offloaded iSCSI.
[PATCH v2 2/6] qed: Add iSCSI out of order packet handling.
[PATCH v2 3/6] qedi: Add QLogic FastLinQ offload iSCSI driver framework.
[PATCH v2 4/6] qedi: Add LL2 iSCSI interface for offload iSCSI.
[PATCH v2 5/6] qedi: Add support for iSCSI session management.
[PATCH v2 6/6] qedi: Add support for data path.

Patches 1 & 2 are "qed" module patches that goes under
drivers/net/ethernet/qlogic/qed/ and include/linux/qed/ directory.
- These are the iSCSI enablement changes to the common "qed"
  module. There is no dependency for these patches and so
  can go independently.

Patches 3, 4, 5 & 6 are the qedi patches that is aimed towards
drivers/scsi/qedi/ directory.
- These are the core qedi changes and is dependent on the
  qed changes (invokes qed_XXX functions).

As qed sits in the net tree, the patches are usually submitted via netdev.

Do you have any preference or thoughts on how the "qed" patches be 
approached? Just as a reference, our rdma driver "qedr" went through 
something similar[1], and eventually "qed" patches were taken by David 
in the net tree and "qedr", in the rdma tree (obviously) by Doug L.

Hi David,

For the "qed" enablement sent with the v2 series, we did not prefix the 
qed patches with "[PATCH net-next]" prefix, so netdev folks may have 
failed to notice/review that, sorry about that. We will send the next (v3) 
series with that corrected.

Right now, we are basing the "qed" patches on top of latest net + net-next 
tree. FYI, I tried a test merge of net-next/master + qed patches with 
"net/master" and I see no conflict in qed.

Regards,
-Arun

[1] http://marc.info/?l=linux-rdma=147509152719831=2
--
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/6] qed: Add support for hardware offloaded iSCSI.

[Arun Easi]

On Fri, 11 Nov 2016, 7:57am, Hannes Reinecke wrote:

> On 11/08/2016 07:56 AM, Manish Rangankar wrote:
> > From: Yuval Mintz <yuval.mi...@cavium.com>
> > 
> > This adds the backbone required for the various HW initalizations
> > which are necessary for the iSCSI driver (qedi) for QLogic FastLinQ
> > 4 line of adapters - FW notification, resource initializations, etc.
> > 
> > Signed-off-by: Arun Easi <arun.e...@cavium.com>
> > Signed-off-by: Yuval Mintz <yuval.mi...@cavium.com>
> > ---
> >  drivers/net/ethernet/qlogic/Kconfig|   15 +
> >  drivers/net/ethernet/qlogic/qed/Makefile   |1 +
> >  drivers/net/ethernet/qlogic/qed/qed.h  |7 +-
> >  drivers/net/ethernet/qlogic/qed/qed_dev.c  |   12 +
> >  drivers/net/ethernet/qlogic/qed/qed_int.h  |1 -
> >  drivers/net/ethernet/qlogic/qed/qed_iscsi.c| 1276
> > 
> >  drivers/net/ethernet/qlogic/qed/qed_iscsi.h|   52 +
> >  drivers/net/ethernet/qlogic/qed/qed_l2.c   |1 -
> >  drivers/net/ethernet/qlogic/qed/qed_ll2.c  |4 +-
> >  drivers/net/ethernet/qlogic/qed/qed_reg_addr.h |2 +
> >  drivers/net/ethernet/qlogic/qed/qed_spq.c  |   15 +
> >  include/linux/qed/qed_if.h |2 +
> >  include/linux/qed/qed_iscsi_if.h   |  229 +
> >  13 files changed, 1613 insertions(+), 4 deletions(-)
> >  create mode 100644 drivers/net/ethernet/qlogic/qed/qed_iscsi.c
> >  create mode 100644 drivers/net/ethernet/qlogic/qed/qed_iscsi.h
> >  create mode 100644 include/linux/qed/qed_iscsi_if.h
> > 
> > diff --git a/drivers/net/ethernet/qlogic/Kconfig
> > b/drivers/net/ethernet/qlogic/Kconfig
> > index 32f2a45..2832570 100644
> > --- a/drivers/net/ethernet/qlogic/Kconfig
> > +++ b/drivers/net/ethernet/qlogic/Kconfig
> > @@ -110,4 +110,19 @@ config QEDE
> >  config QED_RDMA
> > bool
> > 
> > +config QED_ISCSI
> > +   bool
> > +
> > +config QEDI
> > +   tristate "QLogic QED 25/40/100Gb iSCSI driver"
> > +   depends on QED
> > +   select QED_LL2
> > +   select QED_ISCSI
> > +   default n
> > +   ---help---
> > + This provides a temporary node that allows the compilation
> > + and logical testing of the hardware offload iSCSI support
> > + for QLogic QED. This would be replaced by the 'real' option
> > + once the QEDI driver is added [+relocated].
> > +
> >  endif # NET_VENDOR_QLOGIC
> > diff --git a/drivers/net/ethernet/qlogic/qed/Makefile
> > b/drivers/net/ethernet/qlogic/qed/Makefile
> > index 967acf3..597e15c 100644
> > --- a/drivers/net/ethernet/qlogic/qed/Makefile
> > +++ b/drivers/net/ethernet/qlogic/qed/Makefile
> > @@ -6,3 +6,4 @@ qed-y := qed_cxt.o qed_dev.o qed_hw.o qed_init_fw_funcs.o
> > qed_init_ops.o \
> >  qed-$(CONFIG_QED_SRIOV) += qed_sriov.o qed_vf.o
> >  qed-$(CONFIG_QED_LL2) += qed_ll2.o
> >  qed-$(CONFIG_QED_RDMA) += qed_roce.o
> > +qed-$(CONFIG_QED_ISCSI) += qed_iscsi.o
> > diff --git a/drivers/net/ethernet/qlogic/qed/qed.h
> > b/drivers/net/ethernet/qlogic/qed/qed.h
> > index 50b8a01..15286c1 100644
> > --- a/drivers/net/ethernet/qlogic/qed/qed.h
> > +++ b/drivers/net/ethernet/qlogic/qed/qed.h
> > @@ -35,6 +35,7 @@
> > 
> >  #define QED_WFQ_UNIT   100
> > 
> > +#define ISCSI_BDQ_ID(_port_id) (_port_id)
> >  #define QED_WID_SIZE(1024)
> >  #define QED_PF_DEMS_SIZE(4)
> > 
> > @@ -392,6 +393,7 @@ struct qed_hwfn {
> > boolusing_ll2;
> > struct qed_ll2_info *p_ll2_info;
> > struct qed_rdma_info*p_rdma_info;
> > +   struct qed_iscsi_info   *p_iscsi_info;
> > struct qed_pf_paramspf_params;
> > 
> > bool b_rdma_enabled_in_prs;
> > @@ -593,6 +595,8 @@ struct qed_dev {
> > /* Linux specific here */
> > struct  qede_dev*edev;
> > struct  pci_dev *pdev;
> > +   u32 flags;
> > +#define QED_FLAG_STORAGE_STARTED   (BIT(0))
> > int msg_enable;
> > 
> > struct pci_params   pci_params;
> > @@ -606,6 +610,7 @@ struct qed_dev {
> > union {
> > struct qed_common_cb_ops*common;
> > struct qed_eth_cb_ops   *eth;
> > +   struct qed_iscsi_cb_ops *iscsi;
> > } protocol_ops;
> > void*ops_cookie;
> > 
> >

Re: [PATCH v2 3/6] qedi: Add QLogic FastLinQ offload iSCSI driver framework.

[Arun Easi]

Martin,

On Tue, 8 Nov 2016, 3:49pm -, Martin K. Petersen wrote:

> >>>>> "Arun" == Arun Easi <arun.e...@cavium.com> writes:
> 
> Arun,
> 
> Arun> qedi is the new iSCSI driver, which we are trying to submit, for
> Arun> our 41000 series CNA. This patch series were broken up into
> Arun> logical blocks for review purpose, but were not made to compile
> Arun> individually. It is our impression that this is acceptable for
> Arun> SCSI and all the initial "qedi" patches will be squashed and
> Arun> committed as a single commit. Please let us know if we are
> Arun> mistaken, and if so, we will post another series with this taken
> Arun> care of.
> 
> It's fine to post the patches split up to ease the review process. But
> whatever we commit must obviously be bisectable.
> 

If it is alright with you, we would like to have all of our initial 
patches for the driver (qedi) squashed as a single commit to the tree. We 
will ensure that this single combined commit compiles clean.

Regards,
-Arun
--
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 3/6] qedi: Add QLogic FastLinQ offload iSCSI driver framework.

[Arun Easi]

[ Sending on behalf of Manish to cover for the time difference. ]

Hi Martin, James,

I would like to request your input on this kbuild test error on the 
series, where they compile fine together, but is not bisectable.

qedi is the new iSCSI driver, which we are trying to submit, for our 41000 
series CNA. This patch series were broken up into logical blocks for 
review purpose, but were not made to compile individually. It is our 
impression that this is acceptable for SCSI and all the initial "qedi" 
patches will be squashed and committed as a single commit. Please let us 
know if we are mistaken, and if so, we will post another series 
with this taken care of.

FYI, this series accompany additions to the common core module, "qed", 
that goes under drivers/net/. The patches for the qed module compiles fine 
individually and so is bisectable.

In regards to the additional warnings brought out by kbuild test on 
"PATCH v2 6/6" and "PATCH v2 3/6", we will post a v3 with the fixes.

Regards,
-Arun

On Tue, 8 Nov 2016, 2:52am -, kbuild test robot wrote:

> Hi Manish,
> 
> [auto build test ERROR on net-next/master]
> [also build test ERROR on v4.9-rc4]
> [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/Manish-Rangankar/qed-Add-support-for-hardware-offloaded-iSCSI/20161108-180027
> config: ia64-allmodconfig (attached as .config)
> compiler: ia64-linux-gcc (GCC) 6.2.0
> reproduce:
> wget 
> https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross
>  -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # save the attached .config to linux build tree
> make.cross ARCH=ia64 
> 
> Note: the 
> linux-review/Manish-Rangankar/qed-Add-support-for-hardware-offloaded-iSCSI/20161108-180027
>  HEAD dd4d1d0e0785d20cdcfdf9b2c792c564a79b2de2 builds fine.
>   It only hurts bisectibility.
> 

--
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: [RFC 2/6] qed: Add iSCSI out of order packet handling.

[Arun Easi]

On Wed, 19 Oct 2016, 2:39am, Johannes Thumshirn wrote:

> On Wed, Oct 19, 2016 at 01:01:09AM -0400, manish.rangan...@cavium.com wrote:
> > From: Yuval Mintz <yuval.mi...@qlogic.com>
> > 
> > This patch adds out of order packet handling for hardware offloaded
> > iSCSI. Out of order packet handling requires driver buffer allocation
> > and assistance.
> > 
> > Signed-off-by: Arun Easi <arun.e...@cavium.com>
> > Signed-off-by: Yuval Mintz <yuval.mi...@cavium.com>
> > ---
> 
> [...]
> 
> > +   if (IS_ENABLED(CONFIG_QEDI) &&
> > +   p_ll2_conn->conn_type == QED_LL2_TYPE_ISCSI_OOO) {
> 
> If you're going to implement the qed_is_iscsi_personallity() helper, please
> consider a qed_ll2_is_iscsi_() as well.

I see that I can avoid the IS_ENABLED() here as well. I will fix this
in the next revision.

> 
> > +   struct qed_ooo_buffer *p_buffer;
> 
> [...]
> 
> > +   while (cq_new_idx != cq_old_idx) {
> > +   struct core_rx_fast_path_cqe *p_cqe_fp;
> > +
> > +   cqe = qed_chain_consume(_rx->rcq_chain);
> > +   cq_old_idx = qed_chain_get_cons_idx(_rx->rcq_chain);
> > +   cqe_type = cqe->rx_cqe_sp.type;
> > +
> > +   if (cqe_type != CORE_RX_CQE_TYPE_REGULAR) {
> > +   DP_NOTICE(p_hwfn,
> > + "Got a non-regular LB LL2 completion [type 
> > 0x%02x]\n",
> > + cqe_type);
> > +   return -EINVAL;
> > +   }
> > +   p_cqe_fp = >rx_cqe_fp;
> > +
> > +   placement_offset = p_cqe_fp->placement_offset;
> > +   parse_flags = le16_to_cpu(p_cqe_fp->parse_flags.flags);
> > +   packet_length = le16_to_cpu(p_cqe_fp->packet_length);
> > +   vlan = le16_to_cpu(p_cqe_fp->vlan);
> > +   iscsi_ooo = (struct ooo_opaque *)_cqe_fp->opaque_data;
> > +   qed_ooo_save_history_entry(p_hwfn, p_hwfn->p_ooo_info,
> > +  iscsi_ooo);
> > +   cid = le32_to_cpu(iscsi_ooo->cid);
> > +
> > +   /* Process delete isle first */
> > +   if (iscsi_ooo->drop_size)
> > +   qed_ooo_delete_isles(p_hwfn, p_hwfn->p_ooo_info, cid,
> > +iscsi_ooo->drop_isle,
> > +iscsi_ooo->drop_size);
> > +
> > +   if (iscsi_ooo->ooo_opcode == TCP_EVENT_NOP)
> > +   continue;
> > +
> > +   /* Now process create/add/join isles */
> > +   if (list_empty(_rx->active_descq)) {
> > +   DP_NOTICE(p_hwfn,
> > + "LL2 OOO RX chain has no submitted 
> > buffers\n");
> > +   return -EIO;
> > +   }
> > +
> > +   p_pkt = list_first_entry(_rx->active_descq,
> > +struct qed_ll2_rx_packet, list_entry);
> > +
> > +   if ((iscsi_ooo->ooo_opcode == TCP_EVENT_ADD_NEW_ISLE) ||
> > +   (iscsi_ooo->ooo_opcode == TCP_EVENT_ADD_ISLE_RIGHT) ||
> > +   (iscsi_ooo->ooo_opcode == TCP_EVENT_ADD_ISLE_LEFT) ||
> > +   (iscsi_ooo->ooo_opcode == TCP_EVENT_ADD_PEN) ||
> > +   (iscsi_ooo->ooo_opcode == TCP_EVENT_JOIN)) {
> > +   if (!p_pkt) {
> > +   DP_NOTICE(p_hwfn,
> > + "LL2 OOO RX packet is not valid\n");
> > +   return -EIO;
> > +   }
> > +   list_del(_pkt->list_entry);
> > +   p_buffer = (struct qed_ooo_buffer *)p_pkt->cookie;
> > +   p_buffer->packet_length = packet_length;
> > +   p_buffer->parse_flags = parse_flags;
> > +   p_buffer->vlan = vlan;
> > +   p_buffer->placement_offset = placement_offset;
> > +   qed_chain_consume(_rx->rxq_chain);
> > +   list_add_tail(_pkt->list_entry, _rx->free_descq);
> > +
> > +   switch (iscsi_ooo->ooo_opcode) {
> > +   case TCP_EVENT_ADD_NEW_ISLE:
> > +   qed_ooo_add_new_isle(p_hwfn,
> > +p_hwfn->p_ooo_info,
> > +cid,
>

Re: [RFC 1/6] qed: Add support for hardware offloaded iSCSI.

[Arun Easi]

Thanks Johannes for the review, please see my response below.

On Wed, 19 Oct 2016, 2:09am, Johannes Thumshirn wrote:

> Hi Manish,
> 
> Some initital comments
> 
> On Wed, Oct 19, 2016 at 01:01:08AM -0400, manish.rangan...@cavium.com wrote:
> > From: Yuval Mintz <yuval.mi...@qlogic.com>
> > 
> > This adds the backbone required for the various HW initalizations
> > which are necessary for the iSCSI driver (qedi) for QLogic FastLinQ
> > 4 line of adapters - FW notification, resource initializations, etc.
> > 
> > Signed-off-by: Arun Easi <arun.e...@cavium.com>
> > Signed-off-by: Yuval Mintz <yuval.mi...@cavium.com>
> > ---
> >  drivers/net/ethernet/qlogic/Kconfig|   15 +
> >  drivers/net/ethernet/qlogic/qed/Makefile   |1 +
> >  drivers/net/ethernet/qlogic/qed/qed.h  |8 +-
> >  drivers/net/ethernet/qlogic/qed/qed_dev.c  |   15 +
> >  drivers/net/ethernet/qlogic/qed/qed_int.h  |1 -
> >  drivers/net/ethernet/qlogic/qed/qed_iscsi.c| 1310 
> > 
> >  drivers/net/ethernet/qlogic/qed/qed_iscsi.h|   52 +
> >  drivers/net/ethernet/qlogic/qed/qed_l2.c   |1 -
> >  drivers/net/ethernet/qlogic/qed/qed_ll2.c  |   35 +-
> >  drivers/net/ethernet/qlogic/qed/qed_main.c |2 -
> >  drivers/net/ethernet/qlogic/qed/qed_mcp.h  |6 -
> >  drivers/net/ethernet/qlogic/qed/qed_reg_addr.h |2 +
> >  drivers/net/ethernet/qlogic/qed/qed_spq.c  |   15 +
> >  include/linux/qed/qed_if.h |2 +
> >  include/linux/qed/qed_iscsi_if.h   |  249 +
> >  15 files changed, 1692 insertions(+), 22 deletions(-)
> >  create mode 100644 drivers/net/ethernet/qlogic/qed/qed_iscsi.c
> >  create mode 100644 drivers/net/ethernet/qlogic/qed/qed_iscsi.h
> >  create mode 100644 include/linux/qed/qed_iscsi_if.h
> > 
> > diff --git a/drivers/net/ethernet/qlogic/Kconfig 
> > b/drivers/net/ethernet/qlogic/Kconfig
> > index 0df1391f9..bad4fae 100644
> > --- a/drivers/net/ethernet/qlogic/Kconfig
> > +++ b/drivers/net/ethernet/qlogic/Kconfig
> > @@ -118,4 +118,19 @@ config INFINIBAND_QEDR
> >   for QLogic QED. This would be replaced by the 'real' option
> >   once the QEDR driver is added [+relocated].
> >  
> > +config QED_ISCSI
> > +   bool
> > +
> > +config QEDI
> > +   tristate "QLogic QED 25/40/100Gb iSCSI driver"
> > +   depends on QED
> > +   select QED_LL2
> > +   select QED_ISCSI
> > +   default n
> > +   ---help---
> > + This provides a temporary node that allows the compilation
> > + and logical testing of the hardware offload iSCSI support
> > + for QLogic QED. This would be replaced by the 'real' option
> > + once the QEDI driver is added [+relocated].
> > +
> >  endif # NET_VENDOR_QLOGIC
> > diff --git a/drivers/net/ethernet/qlogic/qed/Makefile 
> > b/drivers/net/ethernet/qlogic/qed/Makefile
> > index cda0af7..b76669c 100644
> > --- a/drivers/net/ethernet/qlogic/qed/Makefile
> > +++ b/drivers/net/ethernet/qlogic/qed/Makefile
> > @@ -6,3 +6,4 @@ qed-y := qed_cxt.o qed_dev.o qed_hw.o qed_init_fw_funcs.o 
> > qed_init_ops.o \
> >  qed-$(CONFIG_QED_SRIOV) += qed_sriov.o qed_vf.o
> >  qed-$(CONFIG_QED_LL2) += qed_ll2.o
> >  qed-$(CONFIG_INFINIBAND_QEDR) += qed_roce.o
> > +qed-$(CONFIG_QED_ISCSI) += qed_iscsi.o
> > diff --git a/drivers/net/ethernet/qlogic/qed/qed.h 
> > b/drivers/net/ethernet/qlogic/qed/qed.h
> > index 653bb57..a61b1c0 100644
> > --- a/drivers/net/ethernet/qlogic/qed/qed.h
> > +++ b/drivers/net/ethernet/qlogic/qed/qed.h
> > @@ -35,6 +35,7 @@
> >  
> >  #define QED_WFQ_UNIT   100
> >  
> > +#define ISCSI_BDQ_ID(_port_id) (_port_id)
> 
> This looks a bit odd to me.
> 
> [...]
> 
> >  #endif
> > +   if (IS_ENABLED(CONFIG_QEDI) &&
> > +   p_hwfn->hw_info.personality == QED_PCI_ISCSI)
> > +   qed_iscsi_free(p_hwfn, p_hwfn->p_iscsi_info);
> 
> 
> Why not introduce a small helper like:
> static inline bool qed_is_iscsi_personality()
> {
>   return IS_ENABLED(CONFIG_QEDI) && p_hwfn->hw_info.personality ==
>   QED_PCI_ISCSI;
> }

I think I can remove the IS_ENABLED() check in places like this
and have the check contained in header file. qed_iscsi_free()
already is taken care, if I do the same fore qed_ooo*, I think
the check would just be "p_hwfn->hw_info.personality ==
QED_PCI_ISCSI", which would keep it consistent with th

Re: [RFC 1/6] qed: Add support for hardware offloaded iSCSI.

[Arun Easi]

Thanks Hannes for the review. Please see my comments inline..

On Wed, 19 Oct 2016, 12:31am, Hannes Reinecke wrote:

> On 10/19/2016 07:01 AM, manish.rangan...@cavium.com wrote:
> > From: Yuval Mintz <yuval.mi...@qlogic.com>
> > 
> > This adds the backbone required for the various HW initalizations
> > which are necessary for the iSCSI driver (qedi) for QLogic FastLinQ
> > 4 line of adapters - FW notification, resource initializations, etc.
> > 
> > Signed-off-by: Arun Easi <arun.e...@cavium.com>
> > Signed-off-by: Yuval Mintz <yuval.mi...@cavium.com>
> > ---
> >  drivers/net/ethernet/qlogic/Kconfig|   15 +
> >  drivers/net/ethernet/qlogic/qed/Makefile   |1 +
> >  drivers/net/ethernet/qlogic/qed/qed.h  |8 +-
> >  drivers/net/ethernet/qlogic/qed/qed_dev.c  |   15 +
> >  drivers/net/ethernet/qlogic/qed/qed_int.h  |1 -
> >  drivers/net/ethernet/qlogic/qed/qed_iscsi.c| 1310 
> > 
> >  drivers/net/ethernet/qlogic/qed/qed_iscsi.h|   52 +
> >  drivers/net/ethernet/qlogic/qed/qed_l2.c   |1 -
> >  drivers/net/ethernet/qlogic/qed/qed_ll2.c  |   35 +-
> >  drivers/net/ethernet/qlogic/qed/qed_main.c |2 -
> >  drivers/net/ethernet/qlogic/qed/qed_mcp.h  |6 -
> >  drivers/net/ethernet/qlogic/qed/qed_reg_addr.h |2 +
> >  drivers/net/ethernet/qlogic/qed/qed_spq.c  |   15 +
> >  include/linux/qed/qed_if.h |2 +
> >  include/linux/qed/qed_iscsi_if.h   |  249 +
> >  15 files changed, 1692 insertions(+), 22 deletions(-)
> >  create mode 100644 drivers/net/ethernet/qlogic/qed/qed_iscsi.c
> >  create mode 100644 drivers/net/ethernet/qlogic/qed/qed_iscsi.h
> >  create mode 100644 include/linux/qed/qed_iscsi_if.h
> > 

-- snipped --

> > diff --git a/drivers/net/ethernet/qlogic/qed/qed_iscsi.c 
> > b/drivers/net/ethernet/qlogic/qed/qed_iscsi.c
> > new file mode 100644
> > index 000..cb22dad
> > --- /dev/null
> > +++ b/drivers/net/ethernet/qlogic/qed/qed_iscsi.c
> > @@ -0,0 +1,1310 @@
> > +/* QLogic qed NIC Driver
> 
> Shouldn't that be qedi iSCSI Driver?

Actually, this is the common module under drivers/net/, which was 
submitted along with the NIC driver, qede, so the comment stayed.

In this driver architecture, for all protocols, there is this
common module, qed, as well as a protocol module (qede, qedr, qedi
etc.).

This comment needs to be changed in all files under qed/. We will submit 
another patch to do that.

> > +static int qed_sp_iscsi_conn_offload(struct qed_hwfn *p_hwfn,
> > +struct qed_iscsi_conn *p_conn,
> > +enum spq_mode comp_mode,
> > +struct qed_spq_comp_cb *p_comp_addr)
> > +{
> > +   struct iscsi_spe_conn_offload *p_ramrod = NULL;
> > +   struct tcp_offload_params_opt2 *p_tcp2 = NULL;
> > +   struct tcp_offload_params *p_tcp = NULL;
> > +   struct qed_spq_entry *p_ent = NULL;
> > +   struct qed_sp_init_data init_data;
> > +   union qed_qm_pq_params pq_params;
> > +   u16 pq0_id = 0, pq1_id = 0;
> > +   dma_addr_t r2tq_pbl_addr;
> > +   dma_addr_t xhq_pbl_addr;
> > +   dma_addr_t uhq_pbl_addr;
> > +   int rc = 0;
> > +   u32 dval;
> > +   u16 wval;
> > +   u8 ucval;
> > +   u8 i;
> > +
> > +   /* Get SPQ entry */
> > +   memset(_data, 0, sizeof(init_data));
> > +   init_data.cid = p_conn->icid;
> > +   init_data.opaque_fid = p_hwfn->hw_info.opaque_fid;
> > +   init_data.comp_mode = comp_mode;
> > +   init_data.p_comp_data = p_comp_addr;
> > +
> > +   rc = qed_sp_init_request(p_hwfn, _ent,
> > +ISCSI_RAMROD_CMD_ID_OFFLOAD_CONN,
> > +PROTOCOLID_ISCSI, _data);
> > +   if (rc)
> > +   return rc;
> > +
> > +   p_ramrod = _ent->ramrod.iscsi_conn_offload;
> > +
> > +   /* Transmission PQ is the first of the PF */
> > +   memset(_params, 0, sizeof(pq_params));
> > +   pq0_id = qed_get_qm_pq(p_hwfn, PROTOCOLID_ISCSI, _params);
> > +   p_conn->physical_q0 = cpu_to_le16(pq0_id);
> > +   p_ramrod->iscsi.physical_q0 = cpu_to_le16(pq0_id);
> > +
> > +   /* iSCSI Pure-ACK PQ */
> > +   pq_params.iscsi.q_idx = 1;
> > +   pq1_id = qed_get_qm_pq(p_hwfn, PROTOCOLID_ISCSI, _params);
> > +   p_conn->physical_q1 = cpu_to_le16(pq1_id);
> > +   p_ramrod->iscsi.physical_q1 = cpu_to_le16(pq1_id);
> > +
> > +   p_ramrod->hdr.op_code = ISCSI_RAMROD_CMD_ID_OF

Re: [PATCH] target/qla2xxx: Honor max_data_sg_nents I/O transfer limit

[Arun Easi]

On Thu, 10 Sep 2015, 11:36pm -0700, Nicholas A. Bellinger wrote:


On Thu, 2015-09-10 at 22:16 +, Himanshu Madhani wrote:

Hi Nic,

Sorry about the delay in response.

I have tested with RHEL 6.5 and noticed that IO were not able to complete
with 16M and 32M read/write. IO¹s were getting error due to Mid-layer
underflow

With initiator running upstream kernel version 4.2 as well I was seeing
error with Mid-layer reporting under-run.

I made change in the driver to report DID_OK to Mid-layer with residual
count and I was able to see IO completed without any error.
Basically, overriding driver¹s logic of failing an I/O, when residual
makes it an error with scsi_cmnd->underflow set.
In reality, though, with scsi_cmnd->underflow being set, Linux Qlogic
initiator will always see this as an error.
Maybe this is useful in other OSes. We have verified both read and write
data on FC trace and it looks good.



Great.  Thanks for the update.


Here¹s diff that was done in qla2xxx driver to report response to
mid-layer as DID_OK.


# git diff
diff --git a/drivers/scsi/qla2xxx/qla_isr.c
b/drivers/scsi/qla2xxx/qla_isr.c
index ccf6a7f..fc7b6a2 100644
--- a/drivers/scsi/qla2xxx/qla_isr.c
+++ b/drivers/scsi/qla2xxx/qla_isr.c
@@ -2257,7 +2257,7 @@ qla2x00_status_entry(scsi_qla_host_t *vha, struct
rsp_que *rsp, void *pkt)
"detected (0x%x of 0x%x bytes).\n",
resid, scsi_bufflen(cp));

-   res = DID_ERROR << 16;
+   res = DID_OK << 16;
break;
}
} else if (lscsi_status != SAM_STAT_TASK_SET_FULL &&
(END)


Please go ahead and add patch to mainline kernel.



This missed the last linux-next build by a day, but with your Tested-by
+ Signed-off-by on this patch, I think it's still safe enough to queue
for v4.3-rc1 code.  (Adding JEJB CC')

qla2xxx: Allow DID_OK status for residual under/overflow completion
https://git.kernel.org/cgit/linux/kernel/git/nab/target-pending.git/commit/?h=for-next=46376bdc1e8a77c6d2cc65b77a1e340d4b4c9f1c

The target-pending/for-next PULL request will be going out in the next
48 hours.

Please let me know ASAP if you hit any regressions.



With existing scsi_cmnd->underflow semantics, LLDs are required (I 
believe) to treat the I/O as error if data transferred is less than 
underflow. "underflow", I think, is set to scsi_bufflen() today making any 
non-zero residual cases an error. So the above patch would break that 
semantic. We were not planning to push that change when we made that 
internally for testing.



From scsi_cmnd.h:

--8<-- struct scsi_cmnd --
unsigned underflow; /* Return error if less than
   this amount is transferred */
--8<--

That said, I think moving the check (of scsi_cmnd->underflow) to upper 
layers make more sense to me. LLDs would continue to set residual, if 
there is a residual. SCSI M/L or sd/st can do the enforcement of 
"scsi_cmnd->underflow" requirement.


Regards,
-Arun

Re: [PATCH] target/qla2xxx: Honor max_data_sg_nents I/O transfer limit

[Arun Easi]

Hi nab,

On Thu, 13 Aug 2015, 1:45am -0700, Nicholas A. Bellinger wrote:


From: Nicholas Bellinger n...@linux-iscsi.org

Hi Arun, Roland  Co,

Based upon the feedback from last week, here is a proper
patch for target-core to honor a fabric provided SGL limit
using residual count plus underflow response bit.

Everything appears to be working as expected with tcm-loop
LUNs with basic I/O and sg_raw to test CDB underflow, but
still needs to be verified on real qla2xxx hardware over
the next days for v4.2.0 code.

Please review + test.


Changes look good. I could not test the changes, though. I have requested 
internally to test this patch. Himanshu (copied) will get back with the 
test results (Thanks Himanshu).


Regards,
-Arun



--nab

This patch adds an optional fabric driver provided SGL limit
that target-core will honor as it's own internal I/O maximum
transfer length limit, as exposed by EVPD=0xb0 block limits
parameters.

This is required for handling cases when host I/O transfer
length exceeds the requested EVPD block limits maximum
transfer length. The initial user of this logic is qla2xxx,
so that we can avoid having to reject I/Os from some legacy
FC hosts where EVPD=0xb0 parameters are not honored.

When se_cmd payload length exceeds the provided limit in
target_check_max_data_sg_nents() code, se_cmd-data_length +
se_cmd-prot_length are reset with se_cmd-residual_count
plus underflow bit for outgoing TFO response callbacks.
It also checks for existing CDB level underflow + overflow
and recalculates final residual_count as necessary.

Note this patch currently assumes 1:1 mapping of PAGE_SIZE
per struct scatterlist entry.

Reported-by: Craig Watson craig.wat...@vanguard-rugged.com
Cc: Craig Watson craig.wat...@vanguard-rugged.com
Cc: Roland Dreier rol...@purestorage.com
Cc: Arun Easi arun.e...@qlogic.com
Cc: Giridhar Malavali giridhar.malav...@qlogic.com
Cc: Andrew Vasquez andrew.vasq...@qlogic.com
Cc: Christoph Hellwig h...@lst.de
Cc: Hannes Reinecke h...@suse.de
Cc: Martin K. Petersen martin.peter...@oracle.com
Signed-off-by: Nicholas Bellinger n...@linux-iscsi.org
---
drivers/scsi/qla2xxx/tcm_qla2xxx.c |  5 
drivers/target/target_core_spc.c   | 13 +++--
drivers/target/target_core_transport.c | 51 +-
include/target/target_core_fabric.h| 13 +
4 files changed, 78 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/qla2xxx/tcm_qla2xxx.c 
b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
index 9224a06..6649809 100644
--- a/drivers/scsi/qla2xxx/tcm_qla2xxx.c
+++ b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
@@ -1804,6 +1804,11 @@ static const struct target_core_fabric_ops 
tcm_qla2xxx_ops = {
.module = THIS_MODULE,
.name   = qla2xxx,
.node_acl_size  = sizeof(struct tcm_qla2xxx_nacl),
+   /*
+* XXX: Limit assumes single page per scatter-gather-list entry.
+* Current maximum is ~4.9 MB per se_cmd-t_data_sg with PAGE_SIZE=4096
+*/
+   .max_data_sg_nents  = 1200,
.get_fabric_name= tcm_qla2xxx_get_fabric_name,
.tpg_get_wwn= tcm_qla2xxx_get_fabric_wwn,
.tpg_get_tag= tcm_qla2xxx_get_tag,
diff --git a/drivers/target/target_core_spc.c b/drivers/target/target_core_spc.c
index 556ea1b..da6130a 100644
--- a/drivers/target/target_core_spc.c
+++ b/drivers/target/target_core_spc.c
@@ -484,8 +484,8 @@ static sense_reason_t
spc_emulate_evpd_b0(struct se_cmd *cmd, unsigned char *buf)
{
struct se_device *dev = cmd-se_dev;
-   int have_tp = 0;
-   int opt, min;
+   u32 mtl = 0;
+   int have_tp = 0, opt, min;

/*
 * Following spc3r22 section 6.5.3 Block Limits VPD page, when
@@ -516,8 +516,15 @@ spc_emulate_evpd_b0(struct se_cmd *cmd, unsigned char *buf)

/*
 * Set MAXIMUM TRANSFER LENGTH
+*
+* XXX: Currently assumes single PAGE_SIZE per scatterlist for fabrics
+* enforcing maximum HW scatter-gather-list entry limit
 */
-   put_unaligned_be32(dev-dev_attrib.hw_max_sectors, buf[8]);
+   if (cmd-se_tfo-max_data_sg_nents) {
+   mtl = (cmd-se_tfo-max_data_sg_nents * PAGE_SIZE) /
+  dev-dev_attrib.block_size;
+   }
+   put_unaligned_be32(min_not_zero(mtl, dev-dev_attrib.hw_max_sectors), 
buf[8]);

/*
 * Set OPTIMAL TRANSFER LENGTH
diff --git a/drivers/target/target_core_transport.c 
b/drivers/target/target_core_transport.c
index ce8574b..652471a1 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -1074,6 +1074,55 @@ transport_set_vpd_ident(struct t10_vpd *vpd, unsigned 
char *page_83)
}
EXPORT_SYMBOL(transport_set_vpd_ident);

+static sense_reason_t
+target_check_max_data_sg_nents(struct se_cmd *cmd, struct se_device *dev,
+  unsigned int size

Re: [PATCH] [RESEND] qla2xxx: fix potential deadlock on ha-hardware_lock

[Arun Easi]

Hi Nick,

On Tue, 9 Oct 2012, 11:47am -0700, Nicholas A. Bellinger wrote:


Hi Jiri, Andrew, Arun  Co,


--8-- snipped --


Also please have a look below for a few more related items I noticed
while reviewing this patch..


 drivers/scsi/qla2xxx/qla_init.c |5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c
index 799a58b..48fca47 100644
--- a/drivers/scsi/qla2xxx/qla_init.c
+++ b/drivers/scsi/qla2xxx/qla_init.c
@@ -2080,6 +2080,7 @@ qla2x00_configure_hba(scsi_qla_host_t *vha)
 uint8_t   domain;
 charconnect_type[22];
 struct qla_hw_data *ha = vha-hw;
+unsigned long flags;

 /* Get host addresses. */
 rval = qla2x00_get_adapter_id(vha,
@@ -2154,9 +2155,9 @@ qla2x00_configure_hba(scsi_qla_host_t *vha)
 vha-d_id.b.area = area;
 vha-d_id.b.al_pa = al_pa;

-spin_lock(ha-vport_slock);
+spin_lock_irqsave(ha-vport_slock, flags);
 qlt_update_vp_map(vha, SET_AL_PA);
-spin_unlock(ha-vport_slock);
+spin_unlock_irqrestore(ha-vport_slock, flags);

 if (!vha-flags.init_done)
 ql_log(ql_log_info, vha, 0x2010,



So while looking at other -vport_slock + qlt_update_vp_map() usage, two
more items caught my eye:

In qla_mid.c:qla24xx_disable_vp() code:

   ret = qla24xx_control_vp(vha, VCE_COMMAND_DISABLE_VPS_LOGO_ALL);
   atomic_set(vha-loop_state, LOOP_DOWN);
   atomic_set(vha-loop_down_timer, LOOP_DOWN_TIME);

   /* Remove port id from vp target map */
   qlt_update_vp_map(vha, RESET_AL_PA);

   qla2x00_mark_vp_devices_dead(vha);
   atomic_set(vha-vp_state, VP_FAILED);

AFAICT all callers of qlt_update_vp_map() into qla_target.c code should
be holding -vport_slock.  I'll send out a separate patch for this
shortly.


Makes sense.



And in qla_init.c:qla2x00_init_rings() code:

   for (que = 0; que  ha-max_rsp_queues; que++) {
   rsp = ha-rsp_q_map[que];
   if (!rsp)
   continue;
   /* Initialize response queue entries */
   qla2x00_init_response_q_entries(rsp);
   }

   spin_lock(ha-vport_slock);

   spin_unlock(ha-vport_slock);

   ha-tgt.atio_ring_ptr = ha-tgt.atio_ring;
   ha-tgt.atio_ring_index = 0;
   /* Initialize ATIO queue entries */
   qlt_init_atio_q_entries(vha);

The usage of -vport_slock seems to be now either unnecessary, or a
result of some bad merge outside of qla2xxx target mode.

Qlogic folks, can this (leftover..?) usage of -vport_slock now be
safety removed..?



Yes, this is a left over of code removal and can be safely removed. We
already have a patch queued internally for this.

--Arun

This message and any attached documents contain information from QLogic 
Corporation or its wholly-owned subsidiaries that may be confidential. If you 
are not the intended recipient, you may not read, copy, distribute, or use this 
information. If you have received this transmission in error, please notify the 
sender immediately by reply e-mail and then delete this message.

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