Re: [PATCH v2 3/3] scsi: reduce CONFIG_SCSI_CONSTANTS=y impact by 8k

[Douglas Gilbert]

On 15-11-24 04:42 AM, Rasmus Villemoes wrote:

On 64 bit, struct error_info has 6 bytes of padding, which amounts to
over 4k of wasted space in the additional[] array. We could easily get
rid of that by instead using separate arrays for the codes and the
pointers. However, we can do even better than that and save an
additional 6 bytes per entry: In the table, just store the sizeof()
the corresponding string literal. The cumulative sum of these is then
the appropriate offset into additional_text, which is built from the
concatenation (with '\0's inbetween) of the strings.

$ scripts/bloat-o-meter /tmp/vmlinux vmlinux
add/remove: 0/0 grow/shrink: 1/1 up/down: 24/-8488 (-8464)
function old new   delta
scsi_extd_sense_format   136 160 +24
additional 113122824   -8488

Signed-off-by: Rasmus Villemoes 


Tested-by: Douglas Gilbert 


---
  drivers/scsi/constants.c | 26 +-
  1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
index 6e813eec4f8d..83458f7a2824 100644
--- a/drivers/scsi/constants.c
+++ b/drivers/scsi/constants.c
@@ -292,17 +292,30 @@ bool scsi_opcode_sa_name(int opcode, int service_action,

  struct error_info {
unsigned short code12;  /* 0x0302 looks better than 0x03,0x02 */
-   const char * text;
+   unsigned short size;
  };

+/*
+ * There are 700+ entries in this table. To save space, we don't store
+ * (code, pointer) pairs, which would make sizeof(struct
+ * error_info)==16 on 64 bits. Rather, the second element just stores
+ * the size (including \0) of the corresponding string, and we use the
+ * sum of these to get the appropriate offset into additional_text
+ * defined below. This approach saves 12 bytes per entry.
+ */
  static const struct error_info additional[] =
  {
-#define SENSE_CODE(c, s) {c, s},
+#define SENSE_CODE(c, s) {c, sizeof(s)},
  #include "sense_codes.h"
  #undef SENSE_CODE
-   {0, NULL}
  };

+static const char *additional_text =
+#define SENSE_CODE(c, s) s "\0"
+#include "sense_codes.h"
+#undef SENSE_CODE
+   ;
+
  struct error_info2 {
unsigned char code1, code2_min, code2_max;
const char * str;
@@ -364,11 +377,14 @@ scsi_extd_sense_format(unsigned char asc, unsigned char 
ascq, const char **fmt)
  {
int i;
unsigned short code = ((asc << 8) | ascq);
+   unsigned offset = 0;

*fmt = NULL;
-   for (i = 0; additional[i].text; i++)
+   for (i = 0; i < ARRAY_SIZE(additional); i++) {
if (additional[i].code12 == code)
-   return additional[i].text;
+   return additional_text + offset;
+   offset += additional[i].size;
+   }
for (i = 0; additional2[i].fmt; i++) {
if (additional2[i].code1 == asc &&
ascq >= additional2[i].code2_min &&



--
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/3] scsi: make some Additional Sense strings more grep'able

[Douglas Gilbert]

On 15-11-24 04:42 AM, Rasmus Villemoes wrote:

There's little point in breaking these strings over multiple lines.

Signed-off-by: Rasmus Villemoes 


Tested-by: Douglas Gilbert 


---
  drivers/scsi/constants.c | 27 +--
  1 file changed, 9 insertions(+), 18 deletions(-)

diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
index fa09d4be2b53..58d94e3c3713 100644
--- a/drivers/scsi/constants.c
+++ b/drivers/scsi/constants.c
@@ -346,11 +346,9 @@ static const struct error_info additional[] =
{0x0407, "Logical unit not ready, operation in progress"},
{0x0408, "Logical unit not ready, long write in progress"},
{0x0409, "Logical unit not ready, self-test in progress"},
-   {0x040A, "Logical unit not accessible, asymmetric access state "
-"transition"},
+   {0x040A, "Logical unit not accessible, asymmetric access state 
transition"},
{0x040B, "Logical unit not accessible, target port in standby state"},
-   {0x040C, "Logical unit not accessible, target port in unavailable "
-"state"},
+   {0x040C, "Logical unit not accessible, target port in unavailable 
state"},
{0x040D, "Logical unit not ready, structure check required"},
{0x040E, "Logical unit not ready, security session in progress"},
{0x0410, "Logical unit not ready, auxiliary memory not accessible"},
@@ -363,11 +361,9 @@ static const struct error_info additional[] =
{0x0417, "Logical unit not ready, calibration required"},
{0x0418, "Logical unit not ready, a door is open"},
{0x0419, "Logical unit not ready, operating in sequential mode"},
-   {0x041A, "Logical unit not ready, start stop unit command in "
-"progress"},
+   {0x041A, "Logical unit not ready, start stop unit command in progress"},
{0x041B, "Logical unit not ready, sanitize in progress"},
-   {0x041C, "Logical unit not ready, additional power use not yet "
-"granted"},
+   {0x041C, "Logical unit not ready, additional power use not yet 
granted"},
{0x041D, "Logical unit not ready, configuration in progress"},
{0x041E, "Logical unit not ready, microcode activation required"},
{0x041F, "Logical unit not ready, microcode download required"},
@@ -559,8 +555,7 @@ static const struct error_info additional[] =
{0x2300, "Invalid token operation, cause not reportable"},
{0x2301, "Invalid token operation, unsupported token type"},
{0x2302, "Invalid token operation, remote token usage not supported"},
-   {0x2303, "Invalid token operation, remote rod token creation not "
-"supported"},
+   {0x2303, "Invalid token operation, remote rod token creation not 
supported"},
{0x2304, "Invalid token operation, token unknown"},
{0x2305, "Invalid token operation, token corrupt"},
{0x2306, "Invalid token operation, token revoked"},
@@ -641,8 +636,7 @@ static const struct error_info additional[] =
{0x2A0D, "Data encryption capabilities changed"},
{0x2A10, "Timestamp changed"},
{0x2A11, "Data encryption parameters changed by another i_t nexus"},
-   {0x2A12, "Data encryption parameters changed by vendor specific "
-"event"},
+   {0x2A12, "Data encryption parameters changed by vendor specific event"},
{0x2A13, "Data encryption key instance counter has changed"},
{0x2A14, "SA creation capabilities data has changed"},
{0x2A15, "Medium removal prevention preempted"},
@@ -759,8 +753,7 @@ static const struct error_info additional[] =
{0x3B19, "Element enabled"},
{0x3B1A, "Data transfer device removed"},
{0x3B1B, "Data transfer device inserted"},
-   {0x3B1C, "Too many logical objects on partition to support "
-"operation"},
+   {0x3B1C, "Too many logical objects on partition to support operation"},

{0x3D00, "Invalid bits in identify message"},

@@ -957,8 +950,7 @@ static const struct error_info additional[] =
{0x5D39, "Data channel impending failure throughput performance"},
{0x5D3A, "Data channel impending failure seek time performance"},
{0x5D3B, "Data channel impending failure spin-up retry count"},
-   {0x5D3C, "Data channel impending failure drive calibration retry "
-"count"},
+   {0x5D3C, "Data channel impending failure drive calibration retry 
count"},
{0x5D40, "Servo impending failure general hard drive failure"},
{0x5D41, "Servo impending failure drive error rate too high"},
{0x5D42, "Servo impending failure data error rate too high"},
@@ -1070,8 +1062,7 @@ static const struct error_info additional[] =

{0x6E00, "Command to logical unit failed"},

-   {0x6F00, "Copy protection key exchange failure - authentication "
-"failure"},
+   {0x6F00, "Copy protection key exchange failure - 

re: qla2xxx: Delete session if initiator is gone from FW

[Dan Carpenter]

Hello Alexei Potashnik,

The patch b93bb8ecc389: "qla2xxx: Delete session if initiator is gone
from FW" from Dec 17, 2015, leads to the following static checker
warning:

drivers/scsi/qla2xxx/qla_target.c:3587 qlt_do_ctio_completion()
warn: impossible condition '(logged_out == 41) => (0-1 == 41)'

drivers/scsi/qla2xxx/qla_target.c
  3580  case CTIO_PORT_LOGGED_OUT:
  3581  case CTIO_PORT_UNAVAILABLE:
  3582  {
  3583  bool logged_out = (status & 0x);
  3584  ql_dbg(ql_dbg_tgt_mgt, vha, 0xf059,
  3585  "qla_target(%d): CTIO with %s status %x "
  3586  "received (state %x, se_cmd %p)\n", 
vha->vp_idx,
  3587  (logged_out == CTIO_PORT_LOGGED_OUT) ?
 ^^
Bool cannot equal 0x26.

  3588  "PORT LOGGED OUT" : "PORT UNAVAILABLE",
  3589  status, cmd->state, se_cmd);
  3590  
  3591  if (logged_out && cmd->sess) {
^^
== CTIO_PORT_LOGGED_OUT here?

  3592  /*
  3593   * Session is already logged out, but 
we need
  3594   * to notify initiator, who's not aware 
of this
  3595   */
  3596  cmd->sess->logout_on_delete = 0;
  3597  cmd->sess->send_els_logo = 1;
  3598  
qlt_schedule_sess_for_deletion(cmd->sess, true);
  3599  }
  3600  break;
  3601  }

regards,
dan carpenter
--
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 00/77] More fixes, cleanup and modernization for NCR5380 drivers

[Ondrej Zary]

On Tuesday 22 December 2015 02:17:38 Finn Thain wrote:
> 
> Like my previous work on the NCR5380 drivers, this patch series has bug
> fixes, code cleanup and modernization. These drivers suffer from mistakes,
> poor style and neglect and this long series addresses the worst of it,
> covering all ten wrapper drivers and both of the core driver forks. The
> combined size of the drivers is reduced by over 700 LoC.
> 
> This series continues to reduce divergence between the two core driver
> forks, often by copying a bug fix from one to the other. Most patches are
> larger for having to keep the two forks in sync. Making the same change to
> both is churn if one of them is to be removed but neither can be as yet.
> By the end of this series the diff between the two forks is minimal, so it
> becomes clear what caused the fork and what can be done about it.
> 
> This patch series did benefit from scripts/checkpatch.pl but not too much.
> Decades ago, these drivers started out with 4-space tabs and if the 80
> column limit were to be strictly enforced now, it would require adding new
> functions and shortening identifiers. I would defer this sort of activity
> until after the fork has been resolved.
> 
> All patches to all NCR5380 drivers (x86, ARM, m68k) have been compile-
> tested. The mac_scsi, dmx3191d, g_NCR5380 and atari_scsi modules were
> regression tested on suitable hardware.

Tested on HP C2502 (53C400A chip), Canon FG2-5202 (53C400 chip) and DTC-3181L 
(DTCT-436P chip) ISA cards - everything works fine!

Thanks.

Tested-by: Ondrej Zary 

-- 
Ondrej Zary
--
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 66/77] ncr5380: Fix soft lockups

[Michael Schmitz]

I'd like to think that, too - probably true for the Atari TT SCSI case
(can do scatter-gather, can do more than one command per LUN). Worse
for the Falcon SCSI which is the only one I can test (no
scatter-gather, one command per LUN, interrupt shared with IDE and IDE
driver locked out while SCSI command handled).

But that only affects balancing of I/O between IDE and SCSI drivers.
Is that what you are worried about, Alan?

Happy to test whether limiting max_sectors makes a difference in the DMA case.

Cheers,

  Michael



On Wed, Dec 23, 2015 at 2:47 AM, Finn Thain  wrote:
>
> On Tue, 22 Dec 2015, One Thousand Gnomes wrote:
>
>> On Tue, 22 Dec 2015 12:18:44 +1100 Finn Thain
>>  wrote:
>>
>> > Because of the rudimentary design of the chip, it is necessary to poll
>> > the SCSI bus signals during PIO and this tends to hog the CPU. The
>> > driver will accept new commands while others execute, and this causes
>> > a soft lockup because the workqueue item will not terminate until the
>> > issue queue is emptied.
>> >
>> > When exercising dmx3191d using sequential IO from dd, the driver is
>> > sent 512 KiB WRITE commands and 128 KiB READs. For a PIO transfer, the
>> > rate is is only about 300 KiB/s, so these are long-running commands.
>> > And although PDMA may run at several MiB/s, interrupts are disabled
>> > for the duration of the transfer.
>> >
>> > Fix the unresponsiveness and soft lockup issues by calling
>> > cond_resched() after each command is completed and by limiting
>> > max_sectors for drivers that don't implement real DMA.
>>
>> Is there a reason for not doing some limiting in the DMA case too. A
>> 512K write command even with DMA on a low end 68K box introduces a
>> second of latency before another I/O can be scheduled ?
>
> The DMA case is the atari_scsi case. I'd like to think that atari_scsi
> would have only the latency issues that might be expected from any SCSI-2
> host adapter driver.
>
> Unlike PDMA, interrupts are not disabled for these DMA transfers. Note
> that this patch isn't really relevant to DMA, because the main loop
> iterates only when done == 0, that is, !hostdata->dmalen.
>
> --
>
>>
>> Alan
>
--
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 68/77] ncr5380: Fix whitespace issues using regexp

[Joe Perches]

On Wed, 2015-12-23 at 13:03 +1100, Finn Thain wrote:
> On Tue, 22 Dec 2015, Joe Perches wrote:
> 
> > On Wed, 2015-12-23 at 11:56 +1100, Finn Thain wrote:
> > > On Tue, 22 Dec 2015, Joe Perches wrote:
> > > 
> > > > On Tue, 2015-12-22 at 12:18 +1100, Finn Thain wrote:
> > > > > This patch is just the result of two substitutions. The first 
> > > > > removes any tabs and spaces at the end of the line. The second 
> > > > > replaces runs of tabs and spaces at the beginning of comment lines 
> > > > > with a single space.
> > > > 
> > > > I think the second of these isn't done well.
> > > 
> > > The aim of this patch is not to fix code style, but to make it 
> > > possible to compare these two files so that the fork can be repaired. 
> > > Regexp is very helpful in creating uniformity (and a minimal diff).
> > > 
> > > If this was a coding style issue, we would be discussing the use of 
> > > kernel-doc format for the affected comments, not whitespace.
> > > 
> > > > Many of these comments post reformatting are much worse to read 
> > > > because of lost alignment.
> > > 
> > > You exaggerate a very trivial point.
> > 
> > 
> > 
> > I prefer that all patches be improvements.
> > 
> 
> Agreed. But the example you cited is an improvement, in that it creates 
> consistency.

I think "consistency" isn't a useful argument.
The kernel code doesn't care about any other
external code bases.

> Like you, I prefer to see formal parameters aligned when wrapped. But this 
> isn't a formal parameter list, it is a comment, and no comment should 
> duplicate code.
> 
> Can you suggest a better regexp? Since this is patch 68 in the series, 
> there is a good chance that it will need to be regenerated.

I suggest you do 2 patches here.  One that removes
unnecessary trailing spaces and converts multiple
leading spaces to tabs where appropriate and a
second patch that fixes whatever odd indentation
that does exist after comment leading *.  I think
there aren't many instances of those and I think
those should be done by hand rather than regex.

--
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 68/77] ncr5380: Fix whitespace issues using regexp

[Finn Thain]

On Tue, 22 Dec 2015, James Bottomley wrote:

> I don't think it is trivial.  I can't actually find a single instance in 
> this patch where collapsing the space at the start of the comment looks 
> justified; most of the time it eliminates intended formatting.

The present formatting is broken. It differs between the two core driver 
forks. One uses spaces, the other tabs. For example, line 3.

$ grep -c "^ [*] *\t" drivers/scsi/{atari_,}NCR5380.c 
drivers/scsi/atari_NCR5380.c:14
drivers/scsi/NCR5380.c:23

This patch resolves the issue by deliberately adopting an easy and 
foolproof formatting convention.

But clearly there are different views as to what convention should be used 
here. It would be great if you would indicate an acceptable convention so 
we don't have to bikeshed the use of whitespace in comments.

To set an example, would you be kind enough to reformat, say, the comment 
block at the top of the two files? Or some other comment where kernel-doc 
is not appropriate, and the comment isn't merely duplicating actual code?

Thanks.

> Even if there's an odd one I've missed where space at the beginning of a 
> comment is a problem, I think not doing that part of the regexp and just 
> correcting the odd missed case by hand later will be much better.
> 
> James

-- 
--
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 68/77] ncr5380: Fix whitespace issues using regexp

[James Bottomley]

On Wed, 2015-12-23 at 11:56 +1100, Finn Thain wrote:
> On Tue, 22 Dec 2015, Joe Perches wrote:
> 
> > On Tue, 2015-12-22 at 12:18 +1100, Finn Thain wrote:
> > > This patch is just the result of two substitutions. The first
> removes 
> > > any tabs and spaces at the end of the line. The second replaces
> runs 
> > > of tabs and spaces at the beginning of comment lines with a
> single 
> > > space.
> > 
> > I think the second of these isn't done well.
> 
> The aim of this patch is not to fix code style, but to make it 
> possible to compare these two files so that the fork can be repaired. 
> Regexp is very helpful in creating uniformity (and a minimal diff).
> 
> If this was a coding style issue, we would be discussing the use of 
> kernel-doc format for the affected comments, not whitespace.
> 
> > Many of these comments post reformatting are
> > much worse to read because of lost alignment.
> 
> You exaggerate a very trivial point.
> 
> I admit that a small proportion of comments are slightly less 
> readable. But it is the diff that needs to be readable in order to 
> resolve the fork.
> 
> As I said in patch 0, I am aware that this patch series can be 
> faulted on trivial grounds but in order to avoid churn I don't wish 
> to address those issues until the fork has been resolved.

I don't think it is trivial.  I can't actually find a single instance
in this patch where collapsing the space at the start of the comment
looks justified; most of the time it eliminates intended formatting.
Even if there's an odd one I've missed where space at the beginning of
a comment is a problem, I think not doing that part of the regexp and
just correcting the odd missed case by hand later will be much better.

James


--
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/3] badblocks: Add core badblock management code

[Verma, Vishal L]

On Wed, 2015-12-23 at 10:06 +1100, NeilBrown wrote:
> On Wed, Dec 23 2015, Verma, Vishal L wrote:
> 
> > On Tue, 2015-12-22 at 16:34 +1100, NeilBrown wrote:
> > > On Sat, Dec 05 2015, Verma, Vishal L wrote:
> > > 
> > > > On Fri, 2015-12-04 at 15:30 -0800, James Bottomley wrote:
> > > > [...]
> > > > > > +ssize_t badblocks_store(struct badblocks *bb, const char
> > > > > > *page,
> > > > > > size_t len,
> > > > > > +   int unack)
> > > > > [...]
> > > > > > +int badblocks_init(struct badblocks *bb, int enable)
> > > > > > +{
> > > > > > +   bb->count = 0;
> > > > > > +   if (enable)
> > > > > > +   bb->shift = 0;
> > > > > > +   else
> > > > > > +   bb->shift = -1;
> > > > > > +   bb->page = kmalloc(PAGE_SIZE, GFP_KERNEL);
> > > > > 
> > > > > Why not __get_free_page(GFP_KERNEL)?  The problem with kmalloc
> > > > > of
> > > > > an
> > > > > exactly known page sized quantity is that the slab tracker for
> > > > > this
> > > > > requires two contiguous pages for each page because of the
> > > > > overhead.
> > > > 
> > > > Cool, I didn't know about __get_free_page - I can fix this up
> > > > too.
> > > > 
> > > 
> > > I was reminded of this just recently I thought I should clear up
> > > the
> > > misunderstanding.
> > > 
> > > kmalloc(PAGE_SIZE) does *not* incur significant overhead and
> > > certainly
> > > does not require two contiguous free pages.
> > > If you "grep kmalloc-4096 /proc/slabinfo" you will note that both
> > > objperslab and pagesperslab are 1.  So one page is used to store
> > > each
> > > 4096 byte allocation.
> > > 
> > > To quote the email from Linus which reminded me about this
> > > 
> > > > If you
> > > > want to allocate a page, and get a pointer, just use
> > > > "kmalloc()".
> > > > Boom, done!
> > > 
> > > https://lkml.org/lkml/2015/12/21/605
> > > 
> > > There probably is a small CPU overhead from using kmalloc, but no
> > > memory
> > > overhead.
> > 
> > Thanks Neil.
> > I just read the rest of that thread - and I'm wondering if we should
> > change back to kzalloc here.
> > 
> > The one thing __get_free_page gets us is PAGE_SIZE-aligned memory.
> > Do
> > you think that would be better for this use? (I can't think of any).
> > If
> > not, I can send out a new version reverting back to kzalloc.
> 
> kzalloc(PAGE_SIZE) will also always return page-aligned memory.
> kzalloc returns a void*, __get_free_page returns unsigned long.  For
> that reason alone I would prefer kzalloc.
> 
> But I'm not necessarily suggesting you change the code.  I just wanted
> to clarify a misunderstanding.  You should produce the
> code that you are
> most happy with.


I agree, the typecasting with __get_free_page is pretty ugly. I'll
change it back to kzalloc.

Thanks,
-Vishal

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

Re: [PATCH v3 68/77] ncr5380: Fix whitespace issues using regexp

[Finn Thain]

On Tue, 22 Dec 2015, Joe Perches wrote:

> On Tue, 2015-12-22 at 12:18 +1100, Finn Thain wrote:
> > This patch is just the result of two substitutions. The first removes 
> > any tabs and spaces at the end of the line. The second replaces runs 
> > of tabs and spaces at the beginning of comment lines with a single 
> > space.
> 
> I think the second of these isn't done well.

The aim of this patch is not to fix code style, but to make it possible to 
compare these two files so that the fork can be repaired. Regexp is very 
helpful in creating uniformity (and a minimal diff).

If this was a coding style issue, we would be discussing the use of 
kernel-doc format for the affected comments, not whitespace.

> Many of these comments post reformatting are
> much worse to read because of lost alignment.

You exaggerate a very trivial point.

I admit that a small proportion of comments are slightly less readable. 
But it is the diff that needs to be readable in order to resolve the fork.

As I said in patch 0, I am aware that this patch series can be faulted on 
trivial grounds but in order to avoid churn I don't wish to address those 
issues until the fork has been resolved.

Thanks for your review.

> 
> For instance:
> 
> > +/*
> >   * Function : int NCR5380_select(struct Scsi_Host *instance,
> > - *   struct scsi_cmnd *cmd)
> > + * struct scsi_cmnd *cmd)
> >   *
> >   * Purpose : establishes I_T_L or I_T_L_Q nexus for new or existing 
> > command,
> > - *  including ARBITRATION, SELECTION, and initial message out for 
> > - *  IDENTIFY and queue messages. 
> > + * including ARBITRATION, SELECTION, and initial message out for
> > + * IDENTIFY and queue messages.
> 

-- 

Re: [PATCH v3 68/77] ncr5380: Fix whitespace issues using regexp

[Joe Perches]

On Wed, 2015-12-23 at 11:56 +1100, Finn Thain wrote:
> On Tue, 22 Dec 2015, Joe Perches wrote:
> 
> > On Tue, 2015-12-22 at 12:18 +1100, Finn Thain wrote:
> > > This patch is just the result of two substitutions. The first removes 
> > > any tabs and spaces at the end of the line. The second replaces runs 
> > > of tabs and spaces at the beginning of comment lines with a single 
> > > space.
> > 
> > I think the second of these isn't done well.
> 
> The aim of this patch is not to fix code style, but to make it possible to 
> compare these two files so that the fork can be repaired. Regexp is very 
> helpful in creating uniformity (and a minimal diff).
> 
> If this was a coding style issue, we would be discussing the use of 
> kernel-doc format for the affected comments, not whitespace.
> 
> > Many of these comments post reformatting are
> > much worse to read because of lost alignment.
> 
> You exaggerate a very trivial point.



I prefer that all patches be improvements.

> I admit that a small proportion of comments are slightly less readable. 
> But it is the diff that needs to be readable in order to resolve the fork.

diff -w works well.

--
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 68/77] ncr5380: Fix whitespace issues using regexp

[Finn Thain]

On Tue, 22 Dec 2015, Joe Perches wrote:

> On Wed, 2015-12-23 at 11:56 +1100, Finn Thain wrote:
> > On Tue, 22 Dec 2015, Joe Perches wrote:
> > 
> > > On Tue, 2015-12-22 at 12:18 +1100, Finn Thain wrote:
> > > > This patch is just the result of two substitutions. The first 
> > > > removes any tabs and spaces at the end of the line. The second 
> > > > replaces runs of tabs and spaces at the beginning of comment lines 
> > > > with a single space.
> > > 
> > > I think the second of these isn't done well.
> > 
> > The aim of this patch is not to fix code style, but to make it 
> > possible to compare these two files so that the fork can be repaired. 
> > Regexp is very helpful in creating uniformity (and a minimal diff).
> > 
> > If this was a coding style issue, we would be discussing the use of 
> > kernel-doc format for the affected comments, not whitespace.
> > 
> > > Many of these comments post reformatting are much worse to read 
> > > because of lost alignment.
> > 
> > You exaggerate a very trivial point.
> 
> 
> 
> I prefer that all patches be improvements.
> 

Agreed. But the example you cited is an improvement, in that it creates 
consistency.

Like you, I prefer to see formal parameters aligned when wrapped. But this 
isn't a formal parameter list, it is a comment, and no comment should 
duplicate code.

Can you suggest a better regexp? Since this is patch 68 in the series, 
there is a good chance that it will need to be regenerated.

> > I admit that a small proportion of comments are slightly less 
> > readable. But it is the diff that needs to be readable in order to 
> > resolve the fork.
> 
> diff -w works well.
> 

Yes, it works well at times. For this I prefer to use meld. It does have 
text filters that allow the user to elide differences that match a given 
regexp. But I don't want every meld user to have to write those regexps.

-- 

Re: [PATCH v2 1/3] badblocks: Add core badblock management code

[NeilBrown]

On Wed, Dec 23 2015, Verma, Vishal L wrote:

> On Tue, 2015-12-22 at 16:34 +1100, NeilBrown wrote:
>> On Sat, Dec 05 2015, Verma, Vishal L wrote:
>> 
>> > On Fri, 2015-12-04 at 15:30 -0800, James Bottomley wrote:
>> > [...]
>> > > > +ssize_t badblocks_store(struct badblocks *bb, const char *page,
>> > > > size_t len,
>> > > > +  int unack)
>> > > [...]
>> > > > +int badblocks_init(struct badblocks *bb, int enable)
>> > > > +{
>> > > > +  bb->count = 0;
>> > > > +  if (enable)
>> > > > +  bb->shift = 0;
>> > > > +  else
>> > > > +  bb->shift = -1;
>> > > > +  bb->page = kmalloc(PAGE_SIZE, GFP_KERNEL);
>> > > 
>> > > Why not __get_free_page(GFP_KERNEL)?  The problem with kmalloc of
>> > > an
>> > > exactly known page sized quantity is that the slab tracker for
>> > > this
>> > > requires two contiguous pages for each page because of the
>> > > overhead.
>> > 
>> > Cool, I didn't know about __get_free_page - I can fix this up too.
>> > 
>> 
>> I was reminded of this just recently I thought I should clear up the
>> misunderstanding.
>> 
>> kmalloc(PAGE_SIZE) does *not* incur significant overhead and certainly
>> does not require two contiguous free pages.
>> If you "grep kmalloc-4096 /proc/slabinfo" you will note that both
>> objperslab and pagesperslab are 1.  So one page is used to store each
>> 4096 byte allocation.
>> 
>> To quote the email from Linus which reminded me about this
>> 
>> > If you
>> > want to allocate a page, and get a pointer, just use "kmalloc()".
>> > Boom, done!
>> 
>> https://lkml.org/lkml/2015/12/21/605
>> 
>> There probably is a small CPU overhead from using kmalloc, but no
>> memory
>> overhead.
>
> Thanks Neil.
> I just read the rest of that thread - and I'm wondering if we should
> change back to kzalloc here.
>
> The one thing __get_free_page gets us is PAGE_SIZE-aligned memory. Do
> you think that would be better for this use? (I can't think of any). If
> not, I can send out a new version reverting back to kzalloc.

kzalloc(PAGE_SIZE) will also always return page-aligned memory.
kzalloc returns a void*, __get_free_page returns unsigned long.  For
that reason alone I would prefer kzalloc.

But I'm not necessarily suggesting you change the code.  I just wanted
to clarify a misunderstanding.  You should produce the code that you are
most happy with.

NeilBrown


signature.asc
Description: PGP signature

Re: [PATCH v2 1/3] badblocks: Add core badblock management code

[Verma, Vishal L]

On Tue, 2015-12-22 at 16:34 +1100, NeilBrown wrote:
> On Sat, Dec 05 2015, Verma, Vishal L wrote:
> 
> > On Fri, 2015-12-04 at 15:30 -0800, James Bottomley wrote:
> > [...]
> > > > +ssize_t badblocks_store(struct badblocks *bb, const char *page,
> > > > size_t len,
> > > > +   int unack)
> > > [...]
> > > > +int badblocks_init(struct badblocks *bb, int enable)
> > > > +{
> > > > +   bb->count = 0;
> > > > +   if (enable)
> > > > +   bb->shift = 0;
> > > > +   else
> > > > +   bb->shift = -1;
> > > > +   bb->page = kmalloc(PAGE_SIZE, GFP_KERNEL);
> > > 
> > > Why not __get_free_page(GFP_KERNEL)?  The problem with kmalloc of
> > > an
> > > exactly known page sized quantity is that the slab tracker for
> > > this
> > > requires two contiguous pages for each page because of the
> > > overhead.
> > 
> > Cool, I didn't know about __get_free_page - I can fix this up too.
> > 
> 
> I was reminded of this just recently I thought I should clear up the
> misunderstanding.
> 
> kmalloc(PAGE_SIZE) does *not* incur significant overhead and certainly
> does not require two contiguous free pages.
> If you "grep kmalloc-4096 /proc/slabinfo" you will note that both
> objperslab and pagesperslab are 1.  So one page is used to store each
> 4096 byte allocation.
> 
> To quote the email from Linus which reminded me about this
> 
> > If you
> > want to allocate a page, and get a pointer, just use "kmalloc()".
> > Boom, done!
> 
> https://lkml.org/lkml/2015/12/21/605
> 
> There probably is a small CPU overhead from using kmalloc, but no
> memory
> overhead.

Thanks Neil.
I just read the rest of that thread - and I'm wondering if we should
change back to kzalloc here.

The one thing __get_free_page gets us is PAGE_SIZE-aligned memory. Do
you think that would be better for this use? (I can't think of any). If
not, I can send out a new version reverting back to kzalloc.

-Vishal



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

Re: [PATCH v3 68/77] ncr5380: Fix whitespace issues using regexp

[Finn Thain]

On Tue, 22 Dec 2015, Joe Perches wrote:

> On Wed, 2015-12-23 at 13:03 +1100, Finn Thain wrote:
> > On Tue, 22 Dec 2015, Joe Perches wrote:
> > 
> > > On Wed, 2015-12-23 at 11:56 +1100, Finn Thain wrote:
> > > > On Tue, 22 Dec 2015, Joe Perches wrote:
> > > > 
> > > > > On Tue, 2015-12-22 at 12:18 +1100, Finn Thain wrote:
> > > > > > This patch is just the result of two substitutions. The first 
> > > > > > removes any tabs and spaces at the end of the line. The second 
> > > > > > replaces runs of tabs and spaces at the beginning of comment lines 
> > > > > > with a single space.
> > > > > 
> > > > > I think the second of these isn't done well.
> > > > 
> > > > The aim of this patch is not to fix code style, but to make it 
> > > > possible to compare these two files so that the fork can be repaired. 
> > > > Regexp is very helpful in creating uniformity (and a minimal diff).
> > > > 
> > > > If this was a coding style issue, we would be discussing the use of 
> > > > kernel-doc format for the affected comments, not whitespace.
> > > > 
> > > > > Many of these comments post reformatting are much worse to read 
> > > > > because of lost alignment.
> > > > 
> > > > You exaggerate a very trivial point.
> > > 
> > > 
> > > 
> > > I prefer that all patches be improvements.
> > > 
> > 
> > Agreed. But the example you cited is an improvement, in that it creates 
> > consistency.
> 
> I think "consistency" isn't a useful argument.
> The kernel code doesn't care about any other
> external code bases.

I prefer that the drivers I maintain be self-consistent.

> 
> > Like you, I prefer to see formal parameters aligned when wrapped. But this 
> > isn't a formal parameter list, it is a comment, and no comment should 
> > duplicate code.
> > 
> > Can you suggest a better regexp? Since this is patch 68 in the series, 
> > there is a good chance that it will need to be regenerated.
> 
> I suggest you do 2 patches here.  One that removes
> unnecessary trailing spaces

Those are resolved by this patch.

> and converts multiple leading spaces to tabs where appropriate

As I said, trivial cleanups are better done after the fork is resolved, to 
avoid churn. To assist with resolving the fork, this patch addresses 
inconsistencies.

> and a
> second patch that fixes whatever odd indentation
> that does exist after comment leading *.

Those are resolved by this patch.

> I think
> there aren't many instances of those and I think
> those should be done by hand rather than regex.

I don't know why a regexp wouldn't work and I don't know what you have in 
mind when you say "[fix] odd indentation". Is there some kind of style 
guide applicable here, which this patch violates?

Upon re-reading this patch, I did find a table where I think the regexp is 
detrimental.

@@ -2096,7 +2096,7 @@ static void NCR5380_information_transfer
 * Byte
 * 0EXTENDED_MESSAGE == 1
 * 1length (includes one 
byte for code, doesn't
-*  include first two bytes)
+* include first two bytes)
 * 2code
 * 3..length+1  arguments
 *
This table is interesting. Even though the author took the trouble to
duplicate a portion of the SCSI spec in the source, they were still able 
to stuff it up. See patch 44/77 in this series, "ncr5380: Fix off-by-one 
bug in extended_msg[] bounds check".

So how about I remove this table in patch 67, along with the other dud 
comments, and then regenerate this patch. That way, perhaps we can all 
agree that the regexp is not actually detrimental?

-- 

Re: [PATCH v3 67/77] ncr5380: Cleanup comments

[Hannes Reinecke]

On 12/22/2015 02:18 AM, Finn Thain wrote:

The CVS revision log is not nearly as useful as the history/history.git
repo, so remove it. Roman's commentary at the top of his driver repeats
the same information elsewhere in the file so remove it. Also remove
some other redundant or obsolete comments.

Both the driver and the datasheets confusingly refer to a DMA access
for a SCSI WRITE command as a "DMA write". Similarly a SCSI READ command
is called a "DMA read". This is the opposite of the usual convention.
Thankfully, the chip documentation and driver code also use "DMA send" and
"DMA receive", so adopt this terminology.

This removes some unimportant discrepancies between the two core driver
forks so that 'diff' can be used to reveal the important ones, to
facilitate reunification.

Signed-off-by: Finn Thain 

---
  drivers/scsi/NCR5380.c   |  154 
++-
  drivers/scsi/atari_NCR5380.c |   97 +++
  2 files changed, 48 insertions(+), 203 deletions(-)


Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
--
Dr. Hannes Reinecke   zSeries & Storage
h...@suse.de  +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
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 66/77] ncr5380: Fix soft lockups

[Hannes Reinecke]

On 12/22/2015 02:18 AM, Finn Thain wrote:

Because of the rudimentary design of the chip, it is necessary to poll the
SCSI bus signals during PIO and this tends to hog the CPU. The driver will
accept new commands while others execute, and this causes a soft lockup
because the workqueue item will not terminate until the issue queue is
emptied.

When exercising dmx3191d using sequential IO from dd, the driver is sent
512 KiB WRITE commands and 128 KiB READs. For a PIO transfer, the rate is
is only about 300 KiB/s, so these are long-running commands. And although
PDMA may run at several MiB/s, interrupts are disabled for the duration
of the transfer.

Fix the unresponsiveness and soft lockup issues by calling cond_resched()
after each command is completed and by limiting max_sectors for drivers
that don't implement real DMA.

Signed-off-by: Finn Thain 

---

Changed since v2:
- Moved max_sectors initialization to wrapper drivers. It isn't really
   relevant to the core driver and compile-time configuration using macros
   like REAL_DMA should be avoided.

---
  drivers/scsi/NCR5380.c   |6 --
  drivers/scsi/arm/cumana_1.c  |1 +
  drivers/scsi/arm/oak.c   |1 +
  drivers/scsi/atari_NCR5380.c |6 --
  drivers/scsi/dmx3191d.c  |1 +
  drivers/scsi/dtc.c   |1 +
  drivers/scsi/g_NCR5380.c |1 +
  drivers/scsi/mac_scsi.c  |1 +
  drivers/scsi/pas16.c |1 +
  drivers/scsi/t128.c  |1 +
  10 files changed, 16 insertions(+), 4 deletions(-)


Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
--
Dr. Hannes Reinecke   zSeries & Storage
h...@suse.de  +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
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 65/77] atari_scsi, sun3_scsi: Remove global Scsi_Host pointer

[Hannes Reinecke]

On 12/22/2015 02:18 AM, Finn Thain wrote:

This refactoring removes two global Scsi_Host pointers. This
improves consistency with other ncr5380 drivers. Adopting the same
conventions as the other drivers makes them easier to read.

Signed-off-by: Finn Thain 

---
  drivers/scsi/atari_NCR5380.c |5 +-
  drivers/scsi/atari_scsi.c|   29 -
  drivers/scsi/sun3_scsi.c |   72 
++-
  3 files changed, 36 insertions(+), 70 deletions(-)


Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
--
Dr. Hannes Reinecke   zSeries & Storage
h...@suse.de  +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
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: st driver doesn't seem to grok LTO partitioning

[Emmanuel Florac]

Le Tue, 22 Dec 2015 05:51:30 +
"Seymour, Shane M"  écrivait:

> If you need help with anything please let me know I'd be more than
> happy to contribute (with testing and a review if you want). I have a
> system with an older LTO-3 tape drive that I can use any time (it
> doesn’t support partitioning so if nothing else I can make sure
> partitioning attempts fail gracefully). I may be able to beg, borrow,
> or steal access to an LTO 5 or 6 drive though to help out in testing.

ATM I have an HP LTO-5 and IBM LTO-6 drives (there are real differences
in behaviour between these at times). I need to check what media I have
available for tests though.

> Kai, for the source of the HPE EverStore software should be available
> here:
> 
> http://h20564.www2.hpe.com/hpsc/swd/public/readIndex?sp4ts.oid=5111617

The IBM driver should be here:
http://www-933.ibm.com/support/fixcentral/swg/quickorder?parent=ibm~ST~Tapedevicedriversandsoftware=ibm/Storage_Tape/Tape+device+drivers=1.0=Linux=all=fc

When you have selected RH as a platform, the source rpm is also
available to download.

> 
> This seems to be the relevant code from the driver though (the same
> download has the ibm tape driver as well). You'll need to look at the
> following:
> 
> ltotape.c - ltotape_readposition to determine the current partition
> ltotape.c - ltotape_locate - to move to a position on tape (includes
> setting the partition and has a special flag for changing partitions
> between the two it supports if required) ltotape.c - ltotape_format -
> for creating and destroying partitions ltotape.c -
> ltotape_remaining_capacity - will get you the remaining and maximum
> capacity for the partitions
> 
> When you look at those functions you'll see TC_FORMAT_TYPE referenced
> this is the enum referred it is in src/libltfs/tape_ops.h:
> 
> typedef enum {
> TC_FORMAT_DEFAULT   = 0x00,   /* Make 1 partition medium */
> TC_FORMAT_PARTITION = 0x01,   /* Make 2 partition medium */
> TC_FORMAT_DEST_PART = 0x02,   /* Destroy all data and make 2
> partition medium */ TC_FORMAT_MAX   = 0x03
> } TC_FORMAT_TYPE;/* Space command operations */
 
> The driver at that download looks like it only supports two
> partitions though and if you go looking around the code (grep for
> partition) some LTO drives (probably older ones) look like they may
> be partition aware but may not actually support partitions, see this
> comment:
> 
> ltotape_platform.c:  * For an LTO drive, need to determine
> whether it is partition-capable or only partition-aware:

The IBM code however looks like it supports any number of partitions
(though LTO-6/7 support only 4,so MAX_SUPPORTED_PARTITIONS is set to 4):

if(copy_from_user((void*)usr_part, (void*)arg,
sizeof(struct tape_partition))) {
printk("lin_tape_create_partition: cannot copy from user\n");
rc = -EFAULT;
goto EXIT_LABEL;
} /* if */

if(usr_part->number_of_partitions > MAX_SUPPORTED_PARTITIONS) {
DbgPrint(("lin_tape: too many partitions %d\n",
usr_part->number_of_partitions));
rc = -EINVAL;
goto EXIT_LABEL;
} /* if */


Interestingly it seems to support both wrap partitioning and
longitudinal partitioning (on 3592 drives).

-- 

Emmanuel Florac |   Direction technique
|   Intellique
|   
|   +33 1 78 94 84 02

--
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: st driver doesn't seem to grok LTO partitioning

[Laurence Oberman]

I am just waiting on some LTO5 tape cartridges and then will start
working on this.
I only have LTO cartridges so had to order a couple of LTO5's

On Tue, Dec 22, 2015 at 5:04 AM, Emmanuel Florac  wrote:
> Le Tue, 22 Dec 2015 02:20:31 -0500
> Laurence Oberman  écrivait:
>
>> I also have access to newer hardware if needed. I have started
>> reviewing all of this and will post back to this thread.
>> Emmanuel can you summarize what you would like to achieve and we will
>> all work on this together.
>
> I'd like to be able to partition LTO media through standard commands,
> like "mt mkpartition", mostly to be able to create LTFS tapes without
> relying on hard to compile code from IBM/HP/Quantum/Oracle.
>
> --
> 
> Emmanuel Florac |   Direction technique
> |   Intellique
> |   
> |   +33 1 78 94 84 02
> 
--
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: st driver doesn't seem to grok LTO partitioning

[Emmanuel Florac]

Le Tue, 22 Dec 2015 02:20:31 -0500
Laurence Oberman  écrivait:

> I also have access to newer hardware if needed. I have started
> reviewing all of this and will post back to this thread.
> Emmanuel can you summarize what you would like to achieve and we will
> all work on this together.

I'd like to be able to partition LTO media through standard commands,
like "mt mkpartition", mostly to be able to create LTFS tapes without
relying on hard to compile code from IBM/HP/Quantum/Oracle.

-- 

Emmanuel Florac |   Direction technique
|   Intellique
|   
|   +33 1 78 94 84 02

--
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 68/77] ncr5380: Fix whitespace issues using regexp

[Hannes Reinecke]

On 12/22/2015 02:18 AM, Finn Thain wrote:

This patch is just the result of two substitutions. The first removes any
tabs and spaces at the end of the line. The second replaces runs of
tabs and spaces at the beginning of comment lines with a single space.

perl -i -pe 's,[\t ]+$,,; s,^(\t*[/ ]\*)[ \t]+,$1 ,' 
drivers/scsi/{atari_,}NCR5380.c

This removes some unimportant discrepancies between the two core driver
forks so that 'diff' can be used to reveal the important ones, to
facilitate reunification.

Signed-off-by: Finn Thain 

---
  drivers/scsi/NCR5380.c   |  550 
+--
  drivers/scsi/atari_NCR5380.c |  110 
  2 files changed, 330 insertions(+), 330 deletions(-)


Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
--
Dr. Hannes Reinecke   zSeries & Storage
h...@suse.de  +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
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 76/77] ncr5380: Fix wait for 53C80 registers registers after PDMA

[Hannes Reinecke]

On 12/22/2015 02:18 AM, Finn Thain wrote:

From: Ondrej Zary 

The check for 53C80 registers accessibility was commented out because
it was broken (inverted). Fix and enable it.

Signed-off-by: Ondrej Zary 
Signed-off-by: Finn Thain 

---
  drivers/scsi/g_NCR5380.c |   37 ++---
  1 file changed, 6 insertions(+), 31 deletions(-)


Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
--
Dr. Hannes Reinecke   zSeries & Storage
h...@suse.de  +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
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 77/77] ncr5380: Add support for HP C2502

[Hannes Reinecke]

On 12/22/2015 02:18 AM, Finn Thain wrote:

From: Ondrej Zary 

HP C2502 cards (based on 53C400A chips) use different magic numbers for
software-based I/O address configuration than other cards.
The configuration is also extended to allow setting the IRQ.

Move the configuration to a new function magic_configure() and move
magic the magic numbers into an array. Add new magic numbers for these
HP cards and hp_c2502 module parameter to use them, e.g.:
modprobe g_NCR5380 ncr_irq=7 ncr_addr=0x280 hp_c2502=1

Tested with HP C2502 and DTCT-436P.

Signed-off-by: Ondrej Zary 
Signed-off-by: Finn Thain 


Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
--
Dr. Hannes Reinecke   zSeries & Storage
h...@suse.de  +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
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 74/77] ncr5380: Enable PDMA for NCR53C400A

[Hannes Reinecke]

On 12/22/2015 02:18 AM, Finn Thain wrote:

From: Ondrej Zary 

Add I/O register mapping for NCR53C400A and enable PDMA mode to
improve performance and fix non-working IRQ.

Tested with HP C2502 (and user-space enabler).

Signed-off-by: Ondrej Zary 
Signed-off-by: Finn Thain 

---

Changes to Ondrej's version:
- An 'if' statement is now a 'switch' statement.
- Throw an error if MMIO register locations are not known.

---
  drivers/scsi/g_NCR5380.c |   23 +++
  1 file changed, 19 insertions(+), 4 deletions(-)


Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
--
Dr. Hannes Reinecke   zSeries & Storage
h...@suse.de  +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
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 75/77] ncr5380: Enable PDMA for DTC chips

[Hannes Reinecke]

On 12/22/2015 02:18 AM, Finn Thain wrote:

From: Ondrej Zary 

Add I/O register mapping for DTC chips and enable PDMA mode.

These chips have 16-bit wide HOST BUFFER register and it must be read
by 16-bit accesses (we lose data otherwise).

Large PIO transfers crash at least the DTCT-436P chip (all reads result
in 0xFF) so this patch actually makes it work.

The chip also crashes when we bang on the C400 host status register too
heavily after PDMA write - a small udelay is needed.

Tested on DTCT-436P and verified that it does not break 53C400A.

Signed-off-by: Ondrej Zary 
Signed-off-by: Finn Thain 


Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
--
Dr. Hannes Reinecke   zSeries & Storage
h...@suse.de  +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
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 73/77] ncr5380: Use runtime register mapping

[Hannes Reinecke]

On 12/22/2015 02:18 AM, Finn Thain wrote:

From: Ondrej Zary 

Convert compile-time C400_ register mapping to runtime mapping.
This removes the weird negative register offsets and allows adding
additional mappings.

While at it, convert read/write loops into insb/outsb.

Signed-off-by: Ondrej Zary 
Signed-off-by: Finn Thain 


Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
--
Dr. Hannes Reinecke   zSeries & Storage
h...@suse.de  +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
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 71/77] ncr5380: Cleanup whitespace and parentheses

[Hannes Reinecke]

On 12/22/2015 02:18 AM, Finn Thain wrote:

Signed-off-by: Finn Thain 

---
  drivers/scsi/NCR5380.c   |   30 +++---
  drivers/scsi/atari_NCR5380.c |   26 +-
  2 files changed, 32 insertions(+), 24 deletions(-)


Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
--
Dr. Hannes Reinecke   zSeries & Storage
h...@suse.de  +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
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 72/77] ncr5380: Fix pseudo DMA transfers on 53C400

[Hannes Reinecke]

On 12/22/2015 02:18 AM, Finn Thain wrote:

From: Ondrej Zary 

Pseudo-DMA (PDMA) has been broken for ages, resulting in hangs on
53C400-based cards.

According to 53C400 datasheet, PDMA transfer length must be a multiple
of 128. Check if that's true and use PIO if it's not.

This makes PDMA work on 53C400 (Canon FG2-5202).

Signed-off-by: Ondrej Zary 
Signed-off-by: Finn Thain 


Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
--
Dr. Hannes Reinecke   zSeries & Storage
h...@suse.de  +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
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 70/77] atari_NCR5380: Merge changes from NCR5380.c

[Hannes Reinecke]

On 12/22/2015 02:18 AM, Finn Thain wrote:

In the past, atari_NCR5380.c was overlooked by those working on NCR5380.c
and this caused needless divergence. All of the changes in this patch were
taken from NCR5380.c.

This removes some unimportant discrepancies between the two core driver
forks so that 'diff' can be used to reveal the important ones, to
facilitate reunification.

Signed-off-by: Finn Thain 

---
  drivers/scsi/atari_NCR5380.c |  108 
+--
  1 file changed, 64 insertions(+), 44 deletions(-)


Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
--
Dr. Hannes Reinecke   zSeries & Storage
h...@suse.de  +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
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 69/77] ncr5380: Merge changes from atari_NCR5380.c

[Hannes Reinecke]

On 12/22/2015 02:18 AM, Finn Thain wrote:

In the past, NCR5380.c was overlooked by those working on atari_NCR5380.c
and this caused needless divergence. All of the changes in this patch were
taken from atari_NCR5380.c.

This removes some unimportant discrepancies between the two core driver
forks so that 'diff' can be used to reveal the important ones, to
facilitate reunification.

Signed-off-by: Finn Thain 

---
  drivers/scsi/NCR5380.c |  155 
+++--
  1 file changed, 87 insertions(+), 68 deletions(-)


Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
--
Dr. Hannes Reinecke   zSeries & Storage
h...@suse.de  +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
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 63/77] atari_NCR5380: Remove HOSTNO macro from printk() and seq_printf() calls

[Hannes Reinecke]

On 12/22/2015 02:18 AM, Finn Thain wrote:

Remove the HOSTNO macro that is peculiar to atari_NCR5380.c and
contributes to the problem of divergence of the NCR5380 core drivers.
Keep NCR5380.c in sync.

Signed-off-by: Finn Thain 


Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
--
Dr. Hannes Reinecke   zSeries & Storage
h...@suse.de  +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
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 64/77] atari_NCR5380: Eliminate HOSTNO macro

[Hannes Reinecke]

On 12/22/2015 02:18 AM, Finn Thain wrote:

Keep the two core driver forks in sync.

Signed-off-by: Finn Thain 

---
  drivers/scsi/NCR5380.c   |   71 +++--
  drivers/scsi/atari_NCR5380.c |  102 
+++
  2 files changed, 84 insertions(+), 89 deletions(-)


Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
--
Dr. Hannes Reinecke   zSeries & Storage
h...@suse.de  +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
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 62/77] ncr5380: Implement new eh_bus_reset_handler

[Hannes Reinecke]

On 12/22/2015 02:18 AM, Finn Thain wrote:

NCR5380.c lacks a sane eh_bus_reset_handler. The atari_NCR5380.c code is
much better but it should not throw out the issue queue (that would be
a host reset) and it neglects to set the result code for commands that it
throws out. Fix these bugs and keep the two core drivers in sync.

Signed-off-by: Finn Thain 

---
  drivers/scsi/NCR5380.c   |   50 
++-
  drivers/scsi/atari_NCR5380.c |   39 +++--
  2 files changed, 72 insertions(+), 17 deletions(-)



Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
--
Dr. Hannes Reinecke   zSeries & Storage
h...@suse.de  +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
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 61/77] ncr5380: Fix EH during arbitration and selection

[Hannes Reinecke]

On 12/22/2015 02:18 AM, Finn Thain wrote:

During arbitration and selection, the relevant command is invisible to
exception handlers and can be found only in a pointer on the stack of a
different thread.

When eh_abort_handler can't find a given command, it can't decide whether
that command was completed already or is still in arbitration or selection
phase. But it must return either SUCCESS (e.g. command completed earlier)
or FAILED (could not abort the nexus, try bus reset).

The solution is to make sure all commands belonging to the LLD are always
visible to exception handlers. Add another scsi_cmnd pointer to the
hostdata struct to track the command in arbitration or selection phase.

Replace 'retain_dma_irq' with the new 'selecting' pointer, to bring
atari_NCR5380.c into line with NCR5380.c.

Signed-off-by: Finn Thain 

---
  drivers/scsi/NCR5380.c   |   76 +--
  drivers/scsi/NCR5380.h   |4 +-
  drivers/scsi/atari_NCR5380.c |   82 
+++
  3 files changed, 119 insertions(+), 43 deletions(-)


Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
--
Dr. Hannes Reinecke   zSeries & Storage
h...@suse.de  +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
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 66/77] ncr5380: Fix soft lockups

[One Thousand Gnomes]

On Tue, 22 Dec 2015 12:18:44 +1100
Finn Thain  wrote:

> Because of the rudimentary design of the chip, it is necessary to poll the
> SCSI bus signals during PIO and this tends to hog the CPU. The driver will
> accept new commands while others execute, and this causes a soft lockup
> because the workqueue item will not terminate until the issue queue is
> emptied.
> 
> When exercising dmx3191d using sequential IO from dd, the driver is sent
> 512 KiB WRITE commands and 128 KiB READs. For a PIO transfer, the rate is
> is only about 300 KiB/s, so these are long-running commands. And although
> PDMA may run at several MiB/s, interrupts are disabled for the duration
> of the transfer.
> 
> Fix the unresponsiveness and soft lockup issues by calling cond_resched()
> after each command is completed and by limiting max_sectors for drivers
> that don't implement real DMA.

Is there a reason for not doing some limiting in the DMA case too. A 512K
write command even with DMA on a low end 68K box introduces a second of
latency before another I/O can be scheduled ?

Alan
--
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 18/77] ncr5380: Eliminate USLEEP_WAITLONG delay

[Finn Thain]

On Tue, 22 Dec 2015, Hannes Reinecke wrote:

> On 12/22/2015 02:17 AM, Finn Thain wrote:
> > Linux 2.1.105 introduced the USLEEP_WAITLONG delay, apparently "needed for
> > Mustek scanners". It is intended to stall the issue queue for 5 seconds.
> > There are a number of problems with this.
> >
> > 1. Only g_NCR5380 enables the delay, which implies that the other five
> > drivers using the NCR5380.c core driver remain incompatible with
> > Mustek scanners.
> >
> > 2. The delay is not implemented by atari_NCR5380.c, which is problematic
> > for re-unifying the two core driver forks.
> >
> > 3. The delay is implemented using NCR5380_set_timer() which makes it
> > unreliable. A new command queued by the mid-layer cancels the delay.
> >
> > 4. The delay is applied indiscriminately in several situations in which
> > NCR5380_select() returns -1. These are-- reselection by the target,
> > failure of the target to assert BSY, and failure of the target to
> > assert REQ. It's clear from the comments that USLEEP_WAITLONG is not
> > relevant to the reselection case. And reportedly, these scanners do
> > not disconnect.
> >
> > 5. atari_NCR5380.c was forked before Linux 2.1.105, so it was spared some
> > of the damage done to NCR5380.c. In this case, the atari_NCR5380.c core
> > driver was more standard-compliant and may not have needed any
> > workaround like the USLEEP_WAITLONG kludge. The compliance issue was
> > addressed in the previous patch.
> >
> > If these scanners still don't work, we need a better solution. Retrying
> > selection until EH aborts a command offers equivalent robustness. Bugs in
> > the existing driver prevent EH working correctly but this is addressed in
> > a subsequent patch. Remove USLEEP_WAITLONG.
> >
> > Signed-off-by: Finn Thain 
> >
> > ---
> >   drivers/scsi/NCR5380.c   |   19 +--
> >   drivers/scsi/g_NCR5380.c |1 -
> >   2 files changed, 5 insertions(+), 15 deletions(-)
> >
> > Index: linux/drivers/scsi/NCR5380.c
> > ===
> > --- linux.orig/drivers/scsi/NCR5380.c   2015-12-22 12:15:51.0 
> > +1100
> > +++ linux/drivers/scsi/NCR5380.c2015-12-22 12:15:52.0 +1100
> > @@ -468,10 +468,6 @@ static void NCR5380_print_phase(struct S
> >   #ifndef USLEEP_POLL
> >   #define USLEEP_POLL msecs_to_jiffies(200)
> >   #endif
> > -#ifndef USLEEP_WAITLONG
> > -/* RvC: (reasonable time to wait on select error) */
> > -#define USLEEP_WAITLONG USLEEP_SLEEP
> > -#endif
> >
> >   /*
> >* Function : int should_disconnect (unsigned char cmd)
> > @@ -619,8 +615,8 @@ static void prepare_info(struct Scsi_Hos
> > "can_queue %d, cmd_per_lun %d, "
> > "sg_tablesize %d, this_id %d, "
> > "flags { %s%s%s%s}, "
> > -#if defined(USLEEP_POLL) && defined(USLEEP_WAITLONG)
> > -"USLEEP_POLL %lu, USLEEP_WAITLONG %lu, "
> > +#if defined(USLEEP_POLL) && defined(USLEEP_SLEEP)
> > +"USLEEP_POLL %lu, USLEEP_SLEEP %lu, "
> >   #endif
> > "options { %s} ",
> > instance->hostt->name, instance->io_port, instance->n_io_port,
> > @@ -631,8 +627,8 @@ static void prepare_info(struct Scsi_Hos
> > hostdata->flags & FLAG_DTC3181E  ? "DTC3181E "  : "",
> > hostdata->flags & FLAG_NO_PSEUDO_DMA ? "NO_PSEUDO_DMA " : "",
> > hostdata->flags & FLAG_TOSHIBA_DELAY ? "TOSHIBA_DELAY "  : "",
> > -#if defined(USLEEP_POLL) && defined(USLEEP_WAITLONG)
> > -USLEEP_POLL, USLEEP_WAITLONG,
> > +#if defined(USLEEP_POLL) && defined(USLEEP_SLEEP)
> > +USLEEP_POLL, USLEEP_SLEEP,
> >   #endif
> >   #ifdef AUTOPROBE_IRQ
> > "AUTOPROBE_IRQ "
> Wouldn't it make more sense to remove the USLEEP_WAITLONG completely?
> From what I can see it is meant to indicate that WAITLONG is enabled,
> but we've just removed that functionality...

Actually, this patch does remove USLEEP_WAITLONG completely. It does not 
remove USLEEP_POLL and USLEEP_SLEEP. In patch 25, the USLEEP_POLL and 
USLEEP_SLEEP stuff is replaced by an algorithm that sleeps while polling.

You are right that adding USLEEP_SLEEP to this snprintf() is a change that 
doesn't really belong here. But since I was changing those lines anyway, 
it seemed like a good time to fix a mistake I made when I first added the 
snprintf() and wrote "USLEEP_POLL, USLEEP_WAITLONG" instead of 
"USLEEP_POLL, USLEEP_SLEEP".

Shall I revise this patch? That will affect patch 25. Or perhaps I should 
add the snprintf() change in the commit log?

Thanks for your review.

-- 

> 
> Cheers,
> 
> Hannes
> 
--
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 20/77] ncr5380: Introduce unbound workqueue

[Finn Thain]

On Tue, 22 Dec 2015, Hannes Reinecke wrote:

> On 12/22/2015 02:17 AM, Finn Thain wrote:
> > Allocate a work queue that will permit busy waiting and sleeping. This
> > means NCR5380_init() can potentially fail, so add this error path.
> >
> > Signed-off-by: Finn Thain 
> >
> > ---
> >
> > In subsequent patches, the work function adopts this work queue so it
> > can sleep while polling, which allows the removal of some flawed and
> > complicated code in NCR5380_select() in NCR5380.c.
> >
> > Changed since v1:
> > - Dropped WQ_CPU_INTENSIVE flag because Documentation/workqueue.txt says it
> >"is meaningless for unbound wq".
> >
> > ---
> >   drivers/scsi/NCR5380.c   |   15 +++
> >   drivers/scsi/NCR5380.h   |1 +
> >   drivers/scsi/arm/cumana_1.c  |8 ++--
> >   drivers/scsi/arm/oak.c   |8 ++--
> >   drivers/scsi/atari_NCR5380.c |8 +++-
> >   drivers/scsi/atari_scsi.c|5 -
> >   drivers/scsi/dmx3191d.c  |   17 +++--
> >   drivers/scsi/dtc.c   |   11 +--
> >   drivers/scsi/g_NCR5380.c |   31 +++
> >   drivers/scsi/mac_scsi.c  |5 -
> >   drivers/scsi/pas16.c |   10 --
> >   drivers/scsi/sun3_scsi.c |5 -
> >   drivers/scsi/t128.c  |   13 ++---
> >   13 files changed, 96 insertions(+), 41 deletions(-)
> >
> > Index: linux/drivers/scsi/NCR5380.c
> > ===
> > --- linux.orig/drivers/scsi/NCR5380.c   2015-12-22 12:15:52.0 
> > +1100
> > +++ linux/drivers/scsi/NCR5380.c2015-12-22 12:15:56.0 +1100
> > @@ -514,7 +514,7 @@ static int should_disconnect(unsigned ch
> >   static void NCR5380_set_timer(struct NCR5380_hostdata *hostdata, unsigned
> >   long timeout)
> >   {
> > hostdata->time_expires = jiffies + timeout;
> > -   schedule_delayed_work(>coroutine, timeout);
> > +   queue_delayed_work(hostdata->work_q, >coroutine, timeout);
> >   }
> >
> >
> > @@ -791,7 +791,12 @@ static int NCR5380_init(struct Scsi_Host
> >hostdata->disconnected_queue = NULL;
> >
> >INIT_DELAYED_WORK(>coroutine, NCR5380_main);
> > -   
> > +   hostdata->work_q = alloc_workqueue("ncr5380_%d",
> > +   WQ_UNBOUND | WQ_MEM_RECLAIM,
> > +   1, instance->host_no);
> > +   if (!hostdata->work_q)
> > +   return -ENOMEM;
> > +
> >/* The CHECK code seems to break the 53C400. Will check it later maybe */
> >if (flags & FLAG_NCR53C400)
> > hostdata->flags = FLAG_HAS_LAST_BYTE_SENT | flags;
> 
> Wouldn't it be better to use a normal (ie bound) workqueue here?

The polling algorithm I've used requires that the workqueue item is free 
to busy-wait and sleep. Perhaps a kthread_worker would be better?

> SCSI-2 is pretty much single-threaded, so shifting things onto arbitrary 
> CPUs don't sound very appealing.

Most of these drivers only run on UP systems. For the x86 drivers, I 
suspect that the cache miss penalty would be insignificant compared to 
some of the other overheads. The 5380 chip requires that the CPU is 
involved in SCSI bus signalling and merely accessing a chip register 
takes over a microsecond.

-- 

> 
> Cheers,
> 
> Hannes
> 
--
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 2/3] hpsa: change SAS transport devices to bus 0.

[Don Brace]

sas transport places devices on bus 0 but driver was setting
the bus to 3.

Reviewed-by: Justin Lindley 
Reviewed-by: Kevin Barnett 
Reviewed-by: Scott Teel 
Reviewed-by: Matthew R. Ochs 
Reviewed-by: Hannes Reinecke 
Signed-off-by: Don Brace 
---
 drivers/scsi/hpsa.h |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/hpsa.h b/drivers/scsi/hpsa.h
index ae5beda..fdd39fc 100644
--- a/drivers/scsi/hpsa.h
+++ b/drivers/scsi/hpsa.h
@@ -400,7 +400,7 @@ struct offline_device_entry {
 #define HPSA_PHYSICAL_DEVICE_BUS   0
 #define HPSA_RAID_VOLUME_BUS   1
 #define HPSA_EXTERNAL_RAID_VOLUME_BUS  2
-#define HPSA_HBA_BUS   3
+#define HPSA_HBA_BUS   0
 
 /*
Send the command to the hardware

--
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 3/3] hpsa: add box and bay information for enclosure devices

[Don Brace]

Adding a new method to display enclosure device information.

Reviewed-by: Justin Lindley 
Reviewed-by: Kevin Barnett 
Reviewed-by: Scott Teel 
Reviewed-by: Hannes Reinecke 
Signed-off-by: Don Brace 
---
 drivers/scsi/hpsa.c |  107 ---
 drivers/scsi/hpsa_cmd.h |   13 ++
 2 files changed, 113 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index f8370b8..38ce0e3 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -750,7 +750,6 @@ static ssize_t host_show_hp_ssd_smart_path_enabled(struct 
device *dev,
 }
 
 #define MAX_PATHS 8
-
 static ssize_t path_info_show(struct device *dev,
 struct device_attribute *attr, char *buf)
 {
@@ -792,9 +791,7 @@ static ssize_t path_info_show(struct device *dev,
hdev->bus, hdev->target, hdev->lun,
scsi_device_type(hdev->devtype));
 
-   if (hdev->external ||
-   hdev->devtype == TYPE_RAID ||
-   is_logical_device(hdev)) {
+   if (hdev->devtype == TYPE_RAID || is_logical_device(hdev)) {
output_len += scnprintf(buf + output_len,
PAGE_SIZE - output_len,
"%s\n", active);
@@ -808,8 +805,7 @@ static ssize_t path_info_show(struct device *dev,
phys_connector[0] = '0';
if (phys_connector[1] < '0')
phys_connector[1] = '0';
-   if (hdev->phys_connector[i] > 0)
-   output_len += scnprintf(buf + output_len,
+   output_len += scnprintf(buf + output_len,
PAGE_SIZE - output_len,
"PORT: %.2s ",
phys_connector);
@@ -3191,6 +3187,87 @@ out:
return rc;
 }
 
+/*
+ * get enclosure information
+ * struct ReportExtendedLUNdata *rlep - Used for BMIC drive number
+ * struct hpsa_scsi_dev_t *encl_dev - device entry for enclosure
+ * Uses id_physical_device to determine the box_index.
+ */
+static void hpsa_get_enclosure_info(struct ctlr_info *h,
+   unsigned char *scsi3addr,
+   struct ReportExtendedLUNdata *rlep, int rle_index,
+   struct hpsa_scsi_dev_t *encl_dev)
+{
+   int rc = -1;
+   struct CommandList *c = NULL;
+   struct ErrorInfo *ei = NULL;
+   struct bmic_sense_storage_box_params *bssbp = NULL;
+   struct bmic_identify_physical_device *id_phys = NULL;
+   struct ext_report_lun_entry *rle = >LUN[rle_index];
+   u16 bmic_device_index = 0;
+
+   bmic_device_index = GET_BMIC_DRIVE_NUMBER(>lunid[0]);
+
+   if (bmic_device_index == 0xFF00)
+   goto out;
+
+   bssbp = kzalloc(sizeof(*bssbp), GFP_KERNEL);
+   if (!bssbp)
+   goto out;
+
+   id_phys = kzalloc(sizeof(*id_phys), GFP_KERNEL);
+   if (!id_phys)
+   goto out;
+
+   rc = hpsa_bmic_id_physical_device(h, scsi3addr, bmic_device_index,
+   id_phys, sizeof(*id_phys));
+   if (rc) {
+   dev_warn(>pdev->dev, "%s: id_phys failed %d bdi[0x%x]\n",
+   __func__, encl_dev->external, bmic_device_index);
+   goto out;
+   }
+
+   c = cmd_alloc(h);
+
+   rc = fill_cmd(c, BMIC_SENSE_STORAGE_BOX_PARAMS, h, bssbp,
+   sizeof(*bssbp), 0, RAID_CTLR_LUNID, TYPE_CMD);
+
+   if (rc)
+   goto out;
+
+   if (id_phys->phys_connector[1] == 'E')
+   c->Request.CDB[5] = id_phys->box_index;
+   else
+   c->Request.CDB[5] = 0;
+
+   rc = hpsa_scsi_do_simple_cmd_with_retry(h, c, PCI_DMA_FROMDEVICE,
+   NO_TIMEOUT);
+   if (rc)
+   goto out;
+
+   ei = c->err_info;
+   if (ei->CommandStatus != 0 && ei->CommandStatus != CMD_DATA_UNDERRUN) {
+   rc = -1;
+   goto out;
+   }
+
+   encl_dev->box[id_phys->active_path_number] = bssbp->phys_box_on_port;
+   memcpy(_dev->phys_connector[id_phys->active_path_number],
+   bssbp->phys_connector, sizeof(bssbp->phys_connector));
+
+   rc = IO_OK;
+out:
+   kfree(bssbp);
+   kfree(id_phys);
+
+   if (c)
+   cmd_free(h, c);
+
+   if (rc != IO_OK)
+   hpsa_show_dev_msg(KERN_INFO, h, encl_dev,
+   "Error, could not get enclosure information\n");
+}
+
 static u64 hpsa_get_sas_address_from_report_physical(struct ctlr_info *h,
unsigned char *scsi3addr)
 {
@@ -4032,7 +4109,8 @@ static void hpsa_update_scsi_devices(struct ctlr_info *h)

[PATCH v3 1/3] hpsa: fix path_info_show

[Don Brace]

left off some changes from Rasmus Villemoes where he changed
snprintf to scnprintf

Suggested-by: Rasmus Villemoes 
Reviewed-by: Justin Lindley 
Reviewed-by: Kevin Barnett 
Reviewed-by: Scott Teel 
Reviewed-by: Rasmus Villemoes 
Reviewed-by: Hannes Reinecke 
Signed-off-by: Don Brace 
---
 drivers/scsi/hpsa.c |   12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index a386036..f8370b8 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -795,7 +795,7 @@ static ssize_t path_info_show(struct device *dev,
if (hdev->external ||
hdev->devtype == TYPE_RAID ||
is_logical_device(hdev)) {
-   output_len += snprintf(buf + output_len,
+   output_len += scnprintf(buf + output_len,
PAGE_SIZE - output_len,
"%s\n", active);
continue;
@@ -809,28 +809,28 @@ static ssize_t path_info_show(struct device *dev,
if (phys_connector[1] < '0')
phys_connector[1] = '0';
if (hdev->phys_connector[i] > 0)
-   output_len += snprintf(buf + output_len,
+   output_len += scnprintf(buf + output_len,
PAGE_SIZE - output_len,
"PORT: %.2s ",
phys_connector);
if (hdev->devtype == TYPE_DISK && hdev->expose_device) {
if (box == 0 || box == 0xFF) {
-   output_len += snprintf(buf + output_len,
+   output_len += scnprintf(buf + output_len,
PAGE_SIZE - output_len,
"BAY: %hhu %s\n",
bay, active);
} else {
-   output_len += snprintf(buf + output_len,
+   output_len += scnprintf(buf + output_len,
PAGE_SIZE - output_len,
"BOX: %hhu BAY: %hhu %s\n",
box, bay, active);
}
} else if (box != 0 && box != 0xFF) {
-   output_len += snprintf(buf + output_len,
+   output_len += scnprintf(buf + output_len,
PAGE_SIZE - output_len, "BOX: %hhu %s\n",
box, active);
} else
-   output_len += snprintf(buf + output_len,
+   output_len += scnprintf(buf + output_len,
PAGE_SIZE - output_len, "%s\n", active);
}
 

--
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 0/3] hpsa update

[Don Brace]

These patches are based on Linus's tree

The changes are:
 - correct missing changes from snprintf to scnprintf
   in path_info_show by Rasmus Villemoes
 - fix reported bus for SAS transport devices
 - add in enclosure information

Changes from initial upload
 - added short patch description for patch
   hpsa-add-enclosure-connection-box-bay-information
 - kept hpsa-change-hba-controller-to-bus-0 the same to
   minimize changes.

Changes from V1
 - modified add box and bay information for enclosure
   devices based on Matthew R. Ochs review.
   - moved memory allocation lower in function
 hpsa_get_enclosure_info
   - removed memset on a kzalloc'ed buffer
   - do not fill out enclosure info on tape and medium
 changer devices.
   - changed hpsa_get_enclosure_info to type void.

Changes from V2
 - Added missing '{' that kdiff3 seemed to have removed.
   sorry about that.

---

Don Brace (3):
  hpsa: fix path_info_show
  hpsa: change SAS transport devices to bus 0.
  hpsa: add box and bay information for enclosure devices


 drivers/scsi/hpsa.c |  117 ++-
 drivers/scsi/hpsa.h |2 -
 drivers/scsi/hpsa_cmd.h |   13 +
 3 files changed, 119 insertions(+), 13 deletions(-)

--
Signature
--
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 18/77] ncr5380: Eliminate USLEEP_WAITLONG delay

[Hannes Reinecke]

On 12/22/2015 01:38 PM, Finn Thain wrote:


On Tue, 22 Dec 2015, Hannes Reinecke wrote:


On 12/22/2015 02:17 AM, Finn Thain wrote:

Linux 2.1.105 introduced the USLEEP_WAITLONG delay, apparently "needed for
Mustek scanners". It is intended to stall the issue queue for 5 seconds.
There are a number of problems with this.

1. Only g_NCR5380 enables the delay, which implies that the other five
 drivers using the NCR5380.c core driver remain incompatible with
 Mustek scanners.

2. The delay is not implemented by atari_NCR5380.c, which is problematic
 for re-unifying the two core driver forks.

3. The delay is implemented using NCR5380_set_timer() which makes it
 unreliable. A new command queued by the mid-layer cancels the delay.

4. The delay is applied indiscriminately in several situations in which
 NCR5380_select() returns -1. These are-- reselection by the target,
 failure of the target to assert BSY, and failure of the target to
 assert REQ. It's clear from the comments that USLEEP_WAITLONG is not
 relevant to the reselection case. And reportedly, these scanners do
 not disconnect.

5. atari_NCR5380.c was forked before Linux 2.1.105, so it was spared some
 of the damage done to NCR5380.c. In this case, the atari_NCR5380.c core
 driver was more standard-compliant and may not have needed any
 workaround like the USLEEP_WAITLONG kludge. The compliance issue was
 addressed in the previous patch.

If these scanners still don't work, we need a better solution. Retrying
selection until EH aborts a command offers equivalent robustness. Bugs in
the existing driver prevent EH working correctly but this is addressed in
a subsequent patch. Remove USLEEP_WAITLONG.

Signed-off-by: Finn Thain 

---
   drivers/scsi/NCR5380.c   |   19 +--
   drivers/scsi/g_NCR5380.c |1 -
   2 files changed, 5 insertions(+), 15 deletions(-)

Index: linux/drivers/scsi/NCR5380.c
===
--- linux.orig/drivers/scsi/NCR5380.c   2015-12-22 12:15:51.0 +1100
+++ linux/drivers/scsi/NCR5380.c2015-12-22 12:15:52.0 +1100
@@ -468,10 +468,6 @@ static void NCR5380_print_phase(struct S
   #ifndef USLEEP_POLL
   #define USLEEP_POLL msecs_to_jiffies(200)
   #endif
-#ifndef USLEEP_WAITLONG
-/* RvC: (reasonable time to wait on select error) */
-#define USLEEP_WAITLONG USLEEP_SLEEP
-#endif

   /*
* Function : int should_disconnect (unsigned char cmd)
@@ -619,8 +615,8 @@ static void prepare_info(struct Scsi_Hos
 "can_queue %d, cmd_per_lun %d, "
 "sg_tablesize %d, this_id %d, "
 "flags { %s%s%s%s}, "
-#if defined(USLEEP_POLL) && defined(USLEEP_WAITLONG)
-"USLEEP_POLL %lu, USLEEP_WAITLONG %lu, "
+#if defined(USLEEP_POLL) && defined(USLEEP_SLEEP)
+"USLEEP_POLL %lu, USLEEP_SLEEP %lu, "
   #endif
 "options { %s} ",
 instance->hostt->name, instance->io_port, instance->n_io_port,
@@ -631,8 +627,8 @@ static void prepare_info(struct Scsi_Hos
 hostdata->flags & FLAG_DTC3181E  ? "DTC3181E "  : "",
 hostdata->flags & FLAG_NO_PSEUDO_DMA ? "NO_PSEUDO_DMA " : "",
 hostdata->flags & FLAG_TOSHIBA_DELAY ? "TOSHIBA_DELAY "  : "",
-#if defined(USLEEP_POLL) && defined(USLEEP_WAITLONG)
-USLEEP_POLL, USLEEP_WAITLONG,
+#if defined(USLEEP_POLL) && defined(USLEEP_SLEEP)
+USLEEP_POLL, USLEEP_SLEEP,
   #endif
   #ifdef AUTOPROBE_IRQ
 "AUTOPROBE_IRQ "

Wouldn't it make more sense to remove the USLEEP_WAITLONG completely?
 From what I can see it is meant to indicate that WAITLONG is enabled,
but we've just removed that functionality...


Actually, this patch does remove USLEEP_WAITLONG completely. It does not
remove USLEEP_POLL and USLEEP_SLEEP. In patch 25, the USLEEP_POLL and
USLEEP_SLEEP stuff is replaced by an algorithm that sleeps while polling.

You are right that adding USLEEP_SLEEP to this snprintf() is a change that
doesn't really belong here. But since I was changing those lines anyway,
it seemed like a good time to fix a mistake I made when I first added the
snprintf() and wrote "USLEEP_POLL, USLEEP_WAITLONG" instead of
"USLEEP_POLL, USLEEP_SLEEP".

Shall I revise this patch? That will affect patch 25. Or perhaps I should
add the snprintf() change in the commit log?

Thanks for your review.

Na, that's fine. The entire thing got removed in a later patch, so no 
worries.


Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
--
Dr. Hannes Reinecke   zSeries & Storage
h...@suse.de  +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
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  

Re: [PATCH v3 68/77] ncr5380: Fix whitespace issues using regexp

[Joe Perches]

On Tue, 2015-12-22 at 12:18 +1100, Finn Thain wrote:
> This patch is just the result of two substitutions. The first removes any
> tabs and spaces at the end of the line. The second replaces runs of
> tabs and spaces at the beginning of comment lines with a single space.

I think the second of these isn't done well.
Many of these comments post reformatting are
much worse to read because of lost alignment.

For instance:

> +/*
>   * Function : int NCR5380_select(struct Scsi_Host *instance,
> - *   struct scsi_cmnd *cmd)
> + * struct scsi_cmnd *cmd)
>   *
>   * Purpose : establishes I_T_L or I_T_L_Q nexus for new or existing command,
> - *  including ARBITRATION, SELECTION, and initial message out for 
> - *  IDENTIFY and queue messages. 
> + * including ARBITRATION, SELECTION, and initial message out for
> + * IDENTIFY and queue messages.

--
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 66/77] ncr5380: Fix soft lockups

[Finn Thain]

On Tue, 22 Dec 2015, One Thousand Gnomes wrote:

> On Tue, 22 Dec 2015 12:18:44 +1100 Finn Thain 
>  wrote:
> 
> > Because of the rudimentary design of the chip, it is necessary to poll 
> > the SCSI bus signals during PIO and this tends to hog the CPU. The 
> > driver will accept new commands while others execute, and this causes 
> > a soft lockup because the workqueue item will not terminate until the 
> > issue queue is emptied.
> > 
> > When exercising dmx3191d using sequential IO from dd, the driver is 
> > sent 512 KiB WRITE commands and 128 KiB READs. For a PIO transfer, the 
> > rate is is only about 300 KiB/s, so these are long-running commands. 
> > And although PDMA may run at several MiB/s, interrupts are disabled 
> > for the duration of the transfer.
> > 
> > Fix the unresponsiveness and soft lockup issues by calling 
> > cond_resched() after each command is completed and by limiting 
> > max_sectors for drivers that don't implement real DMA.
> 
> Is there a reason for not doing some limiting in the DMA case too. A 
> 512K write command even with DMA on a low end 68K box introduces a 
> second of latency before another I/O can be scheduled ?

The DMA case is the atari_scsi case. I'd like to think that atari_scsi 
would have only the latency issues that might be expected from any SCSI-2 
host adapter driver.

Unlike PDMA, interrupts are not disabled for these DMA transfers. Note 
that this patch isn't really relevant to DMA, because the main loop 
iterates only when done == 0, that is, !hostdata->dmalen.

-- 

> 
> Alan

--
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 20/77] ncr5380: Introduce unbound workqueue

[Hannes Reinecke]

On 12/22/2015 01:44 PM, Finn Thain wrote:


On Tue, 22 Dec 2015, Hannes Reinecke wrote:


On 12/22/2015 02:17 AM, Finn Thain wrote:

Allocate a work queue that will permit busy waiting and sleeping. This
means NCR5380_init() can potentially fail, so add this error path.

Signed-off-by: Finn Thain 

---

In subsequent patches, the work function adopts this work queue so it
can sleep while polling, which allows the removal of some flawed and
complicated code in NCR5380_select() in NCR5380.c.

Changed since v1:
- Dropped WQ_CPU_INTENSIVE flag because Documentation/workqueue.txt says it
"is meaningless for unbound wq".

---
   drivers/scsi/NCR5380.c   |   15 +++
   drivers/scsi/NCR5380.h   |1 +
   drivers/scsi/arm/cumana_1.c  |8 ++--
   drivers/scsi/arm/oak.c   |8 ++--
   drivers/scsi/atari_NCR5380.c |8 +++-
   drivers/scsi/atari_scsi.c|5 -
   drivers/scsi/dmx3191d.c  |   17 +++--
   drivers/scsi/dtc.c   |   11 +--
   drivers/scsi/g_NCR5380.c |   31 +++
   drivers/scsi/mac_scsi.c  |5 -
   drivers/scsi/pas16.c |   10 --
   drivers/scsi/sun3_scsi.c |5 -
   drivers/scsi/t128.c  |   13 ++---
   13 files changed, 96 insertions(+), 41 deletions(-)

Index: linux/drivers/scsi/NCR5380.c
===
--- linux.orig/drivers/scsi/NCR5380.c   2015-12-22 12:15:52.0 +1100
+++ linux/drivers/scsi/NCR5380.c2015-12-22 12:15:56.0 +1100
@@ -514,7 +514,7 @@ static int should_disconnect(unsigned ch
   static void NCR5380_set_timer(struct NCR5380_hostdata *hostdata, unsigned
   long timeout)
   {
hostdata->time_expires = jiffies + timeout;
-   schedule_delayed_work(>coroutine, timeout);
+   queue_delayed_work(hostdata->work_q, >coroutine, timeout);
   }


@@ -791,7 +791,12 @@ static int NCR5380_init(struct Scsi_Host
hostdata->disconnected_queue = NULL;

INIT_DELAYED_WORK(>coroutine, NCR5380_main);
-   
+   hostdata->work_q = alloc_workqueue("ncr5380_%d",
+   WQ_UNBOUND | WQ_MEM_RECLAIM,
+   1, instance->host_no);
+   if (!hostdata->work_q)
+   return -ENOMEM;
+
/* The CHECK code seems to break the 53C400. Will check it later maybe */
if (flags & FLAG_NCR53C400)
 hostdata->flags = FLAG_HAS_LAST_BYTE_SENT | flags;


Wouldn't it be better to use a normal (ie bound) workqueue here?


The polling algorithm I've used requires that the workqueue item is free
to busy-wait and sleep. Perhaps a kthread_worker would be better?


SCSI-2 is pretty much single-threaded, so shifting things onto arbitrary
CPUs don't sound very appealing.


Most of these drivers only run on UP systems. For the x86 drivers, I
suspect that the cache miss penalty would be insignificant compared to
some of the other overheads. The 5380 chip requires that the CPU is
involved in SCSI bus signalling and merely accessing a chip register
takes over a microsecond.


I know.
But using a bound workqueue would mean you could use 
'create_workqueue()' instead of open-coding it :-)


But in the end it's up to you. If the thing works I'm not that concerned.

Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
--
Dr. Hannes Reinecke   zSeries & Storage
h...@suse.de  +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
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 11/17] be2iscsi: Fix return value for MCC completion

[Jitendra Bhivare]

Change return value of completed MCC EBUSY to EINVAL.

Signed-off-by: Jitendra Bhivare 
Reviewed-by: Hannes Reinecke 
---
 drivers/scsi/be2iscsi/be_cmds.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/be2iscsi/be_cmds.c b/drivers/scsi/be2iscsi/be_cmds.c
index 533060e..dbe62c0 100644
--- a/drivers/scsi/be2iscsi/be_cmds.c
+++ b/drivers/scsi/be2iscsi/be_cmds.c
@@ -323,7 +323,7 @@ static int be_mcc_compl_process(struct be_ctrl_info *ctrl,
if (resp_hdr->response_length)
return 0;
}
-   return -EBUSY;
+   return -EINVAL;
}
return 0;
 }
-- 
1.9.1

--
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 10/17] be2iscsi: Add FW config validation

[Jitendra Bhivare]

System crash in I+T card personality

Fix to add validation for ULP in initiator mode, physical port number,
and supported queue, icd, cid counts.

Signed-off-by: Jitendra Bhivare 
---
 drivers/scsi/be2iscsi/be_main.c |   2 +-
 drivers/scsi/be2iscsi/be_main.h |   2 +
 drivers/scsi/be2iscsi/be_mgmt.c | 188 ++--
 3 files changed, 125 insertions(+), 67 deletions(-)

diff --git a/drivers/scsi/be2iscsi/be_main.c b/drivers/scsi/be2iscsi/be_main.c
index 8967e05..a665e6a 100644
--- a/drivers/scsi/be2iscsi/be_main.c
+++ b/drivers/scsi/be2iscsi/be_main.c
@@ -5670,6 +5670,7 @@ static int beiscsi_dev_probe(struct pci_dev *pcidev,
goto free_port;
}
mgmt_get_port_name(>ctrl, phba);
+   beiscsi_get_params(phba);
 
if (enable_msix)
find_num_cpus(phba);
@@ -5687,7 +5688,6 @@ static int beiscsi_dev_probe(struct pci_dev *pcidev,
}
 
phba->shost->max_id = phba->params.cxns_per_ctrl;
-   beiscsi_get_params(phba);
phba->shost->can_queue = phba->params.ios_per_ctrl;
ret = beiscsi_init_port(phba);
if (ret < 0) {
diff --git a/drivers/scsi/be2iscsi/be_main.h b/drivers/scsi/be2iscsi/be_main.h
index c09082a..f89861b 100644
--- a/drivers/scsi/be2iscsi/be_main.h
+++ b/drivers/scsi/be2iscsi/be_main.h
@@ -397,7 +397,9 @@ struct beiscsi_hba {
 * group together since they are used most frequently
 * for cid to cri conversion
 */
+#define BEISCSI_PHYS_PORT_MAX  4
unsigned int phys_port;
+   /* valid values of phys_port id are 0, 1, 2, 3 */
unsigned int eqid_count;
unsigned int cqid_count;
unsigned int iscsi_cid_start[BEISCSI_ULP_COUNT];
diff --git a/drivers/scsi/be2iscsi/be_mgmt.c b/drivers/scsi/be2iscsi/be_mgmt.c
index 10a8364..60140e2 100644
--- a/drivers/scsi/be2iscsi/be_mgmt.c
+++ b/drivers/scsi/be2iscsi/be_mgmt.c
@@ -373,90 +373,146 @@ int mgmt_get_fw_config(struct be_ctrl_info *ctrl,
struct beiscsi_hba *phba)
 {
struct be_mcc_wrb *wrb = wrb_from_mbox(>mbox_mem);
-   struct be_fw_cfg *req = embedded_payload(wrb);
-   int status = 0;
+   struct be_fw_cfg *pfw_cfg = embedded_payload(wrb);
+   uint32_t cid_count, icd_count;
+   int status = -EINVAL;
+   uint8_t ulp_num = 0;
 
mutex_lock(>mbox_lock);
memset(wrb, 0, sizeof(*wrb));
+   be_wrb_hdr_prepare(wrb, sizeof(*pfw_cfg), true, 0);
 
-   be_wrb_hdr_prepare(wrb, sizeof(*req), true, 0);
-
-   be_cmd_hdr_prepare(>hdr, CMD_SUBSYSTEM_COMMON,
+   be_cmd_hdr_prepare(_cfg->hdr, CMD_SUBSYSTEM_COMMON,
   OPCODE_COMMON_QUERY_FIRMWARE_CONFIG,
   EMBED_MBX_MAX_PAYLOAD_SIZE);
-   status = be_mbox_notify(ctrl);
-   if (!status) {
-   uint8_t ulp_num = 0;
-   struct be_fw_cfg *pfw_cfg;
-   pfw_cfg = req;
 
-   if (!is_chip_be2_be3r(phba)) {
-   phba->fw_config.eqid_count = pfw_cfg->eqid_count;
-   phba->fw_config.cqid_count = pfw_cfg->cqid_count;
+   if (be_mbox_notify(ctrl)) {
+   beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_INIT,
+   "BG_%d : Failed in mgmt_get_fw_config\n");
+   goto fail_init;
+   }
 
-   beiscsi_log(phba, KERN_INFO,
-   BEISCSI_LOG_INIT,
-   "BG_%d : EQ_Count : %d CQ_Count : %d\n",
-   phba->fw_config.eqid_count,
+   /* FW response formats depend on port id */
+   phba->fw_config.phys_port = pfw_cfg->phys_port;
+   if (phba->fw_config.phys_port >= BEISCSI_PHYS_PORT_MAX) {
+   beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_INIT,
+   "BG_%d : invalid physical port id %d\n",
+   phba->fw_config.phys_port);
+   goto fail_init;
+   }
+
+   /* populate and check FW config against min and max values */
+   if (!is_chip_be2_be3r(phba)) {
+   phba->fw_config.eqid_count = pfw_cfg->eqid_count;
+   phba->fw_config.cqid_count = pfw_cfg->cqid_count;
+   if (phba->fw_config.eqid_count == 0 ||
+   phba->fw_config.eqid_count > 2048) {
+   beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_INIT,
+   "BG_%d : invalid EQ count %d\n",
+   phba->fw_config.eqid_count);
+   goto fail_init;
+   }
+   if (phba->fw_config.cqid_count == 0 ||
+   phba->fw_config.cqid_count > 4096) {
+   beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_INIT,
+   "BG_%d : invalid CQ count %d\n",

[PATCH v2 04/17] be2iscsi: Fix to synchronize tag allocation using spin_lock

[Jitendra Bhivare]

alloc_mcc_tag/free_mcc_tag is now done under mcc_lock spin_lock

Signed-off-by: Jitendra Bhivare 
Reviewed-by: Hannes Reinecke 
---
 drivers/scsi/be2iscsi/be_cmds.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/be2iscsi/be_cmds.c b/drivers/scsi/be2iscsi/be_cmds.c
index 1913e9e..db03149 100644
--- a/drivers/scsi/be2iscsi/be_cmds.c
+++ b/drivers/scsi/be2iscsi/be_cmds.c
@@ -121,6 +121,7 @@ unsigned int alloc_mcc_tag(struct beiscsi_hba *phba)
 {
unsigned int tag = 0;
 
+   spin_lock(>ctrl.mcc_lock);
if (phba->ctrl.mcc_tag_available) {
tag = phba->ctrl.mcc_tag[phba->ctrl.mcc_alloc_index];
phba->ctrl.mcc_tag[phba->ctrl.mcc_alloc_index] = 0;
@@ -134,6 +135,7 @@ unsigned int alloc_mcc_tag(struct beiscsi_hba *phba)
else
phba->ctrl.mcc_alloc_index++;
}
+   spin_unlock(>ctrl.mcc_lock);
return tag;
 }
 
@@ -254,7 +256,7 @@ int beiscsi_mccq_compl(struct beiscsi_hba *phba,
 
 void free_mcc_tag(struct be_ctrl_info *ctrl, unsigned int tag)
 {
-   spin_lock(>mbox_lock);
+   spin_lock(>mcc_lock);
tag = tag & 0x00FF;
ctrl->mcc_tag[ctrl->mcc_free_index] = tag;
if (ctrl->mcc_free_index == (MAX_MCC_CMD - 1))
@@ -262,7 +264,7 @@ void free_mcc_tag(struct be_ctrl_info *ctrl, unsigned int 
tag)
else
ctrl->mcc_free_index++;
ctrl->mcc_tag_available++;
-   spin_unlock(>mbox_lock);
+   spin_unlock(>mcc_lock);
 }
 
 bool is_link_state_evt(u32 trailer)
-- 
1.9.1

--
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 09/17] be2iscsi: Fix to handle misconfigured optics events

[Jitendra Bhivare]

Log messages for misconfigured transceivers reported by FW.

Register async events that driver handles using MCC_CREATE_EXT ioctl.
Errors messages for faulted/uncertified/unqualified optics are logged.
Added IOCTL to get port_name to be displayed in error message.

Signed-off-by: Jitendra Bhivare 
Reviewed-by: Hannes Reinecke 
---
 drivers/scsi/be2iscsi/be_cmds.c | 164 +---
 drivers/scsi/be2iscsi/be_cmds.h |  47 ++--
 drivers/scsi/be2iscsi/be_main.c |  19 +
 drivers/scsi/be2iscsi/be_main.h |  10 ++-
 drivers/scsi/be2iscsi/be_mgmt.c |  42 ++
 drivers/scsi/be2iscsi/be_mgmt.h |   2 +
 6 files changed, 199 insertions(+), 85 deletions(-)

diff --git a/drivers/scsi/be2iscsi/be_cmds.c b/drivers/scsi/be2iscsi/be_cmds.c
index 66a1fc3..533060e 100644
--- a/drivers/scsi/be2iscsi/be_cmds.c
+++ b/drivers/scsi/be2iscsi/be_cmds.c
@@ -267,26 +267,6 @@ void free_mcc_tag(struct be_ctrl_info *ctrl, unsigned int 
tag)
spin_unlock(>mcc_lock);
 }
 
-bool is_link_state_evt(u32 trailer)
-{
-   return (((trailer >> ASYNC_TRAILER_EVENT_CODE_SHIFT) &
- ASYNC_TRAILER_EVENT_CODE_MASK) ==
- ASYNC_EVENT_CODE_LINK_STATE);
-}
-
-static bool is_iscsi_evt(u32 trailer)
-{
-   return ((trailer >> ASYNC_TRAILER_EVENT_CODE_SHIFT) &
- ASYNC_TRAILER_EVENT_CODE_MASK) ==
- ASYNC_EVENT_CODE_ISCSI;
-}
-
-static int iscsi_evt_type(u32 trailer)
-{
-   return (trailer >> ASYNC_TRAILER_EVENT_TYPE_SHIFT) &
-ASYNC_TRAILER_EVENT_TYPE_MASK;
-}
-
 static inline bool be_mcc_compl_is_new(struct be_mcc_compl *compl)
 {
if (compl->flags != 0) {
@@ -425,7 +405,7 @@ void beiscsi_fail_session(struct iscsi_cls_session 
*cls_session)
iscsi_session_failure(cls_session->dd_data, ISCSI_ERR_CONN_FAILED);
 }
 
-void beiscsi_async_link_state_process(struct beiscsi_hba *phba,
+static void beiscsi_async_link_state_process(struct beiscsi_hba *phba,
struct be_async_event_link_state *evt)
 {
if ((evt->port_link_status == ASYNC_EVENT_LINK_DOWN) ||
@@ -453,6 +433,100 @@ void beiscsi_async_link_state_process(struct beiscsi_hba 
*phba,
}
 }
 
+static char *beiscsi_port_misconf_event_msg[] = {
+   "Physical Link is functional.",
+   "Optics faulted/incorrectly installed/not installed - Reseat optics, if 
issue not resolved, replace.",
+   "Optics of two types installed - Remove one optic or install matching 
pair of optics.",
+   "Incompatible optics - Replace with compatible optics for card to 
function.",
+   "Unqualified optics - Replace with Avago optics for Warranty and 
Technical Support.",
+   "Uncertified optics - Replace with Avago Certified optics to enable 
link operation."
+};
+
+static void beiscsi_process_async_sli(struct beiscsi_hba *phba,
+ struct be_mcc_compl *compl)
+{
+   struct be_async_event_sli *async_sli;
+   u8 evt_type, state, old_state, le;
+   char *sev = KERN_WARNING;
+   char *msg = NULL;
+
+   evt_type = compl->flags >> ASYNC_TRAILER_EVENT_TYPE_SHIFT;
+   evt_type &= ASYNC_TRAILER_EVENT_TYPE_MASK;
+
+   /* processing only MISCONFIGURED physical port event */
+   if (evt_type != ASYNC_SLI_EVENT_TYPE_MISCONFIGURED)
+   return;
+
+   async_sli = (struct be_async_event_sli *)compl;
+   state = async_sli->event_data1 >>
+(phba->fw_config.phys_port * 8) & 0xff;
+   le = async_sli->event_data2 >>
+(phba->fw_config.phys_port * 8) & 0xff;
+
+   old_state = phba->optic_state;
+   phba->optic_state = state;
+
+   if (state >= ARRAY_SIZE(beiscsi_port_misconf_event_msg)) {
+   /* fw is reporting a state we don't know, log and return */
+   __beiscsi_log(phba, KERN_ERR,
+   "BC_%d : Port %c: Unrecognized optic state 0x%x\n",
+   phba->port_name, async_sli->event_data1);
+   return;
+   }
+
+   if (ASYNC_SLI_LINK_EFFECT_VALID(le)) {
+   /* log link effect for unqualified-4, uncertified-5 optics */
+   if (state > 3)
+   msg = (ASYNC_SLI_LINK_EFFECT_STATE(le)) ?
+   " Link is non-operational." :
+   " Link is operational.";
+   /* 1 - info */
+   if (ASYNC_SLI_LINK_EFFECT_SEV(le) == 1)
+   sev = KERN_INFO;
+   /* 2 - error */
+   if (ASYNC_SLI_LINK_EFFECT_SEV(le) == 2)
+   sev = KERN_ERR;
+   }
+
+   if (old_state != phba->optic_state)
+   __beiscsi_log(phba, sev, "BC_%d : Port %c: %s%s\n",
+ phba->port_name,
+ beiscsi_port_misconf_event_msg[state],
+ !msg ? "" : msg);
+}
+
+void 

[PATCH v2 14/17] be2iscsi: Fix to process 25G link speed info from FW

[Jitendra Bhivare]

Async link event provides port_speed info. Cache the port_speed info
and use the same to report in ISCSI_HOST_PARAM_PORT_SPEED query.

Removed link status query IOCTL used to do the same.

Signed-off-by: Jitendra Bhivare 
---
 drivers/scsi/be2iscsi/be_cmds.c  |  1 +
 drivers/scsi/be2iscsi/be_cmds.h  | 35 ++-
 drivers/scsi/be2iscsi/be_iscsi.c | 39 +--
 drivers/scsi/be2iscsi/be_main.h  |  1 +
 drivers/scsi/be2iscsi/be_mgmt.c  | 28 
 5 files changed, 21 insertions(+), 83 deletions(-)

diff --git a/drivers/scsi/be2iscsi/be_cmds.c b/drivers/scsi/be2iscsi/be_cmds.c
index 14a1c71..ce82f4d 100644
--- a/drivers/scsi/be2iscsi/be_cmds.c
+++ b/drivers/scsi/be2iscsi/be_cmds.c
@@ -408,6 +408,7 @@ void beiscsi_fail_session(struct iscsi_cls_session 
*cls_session)
 static void beiscsi_async_link_state_process(struct beiscsi_hba *phba,
struct be_async_event_link_state *evt)
 {
+   phba->port_speed = evt->port_speed;
if ((evt->port_link_status == ASYNC_EVENT_LINK_DOWN) ||
((evt->port_link_status & ASYNC_EVENT_LOGICAL) &&
 (evt->port_fault != BEISCSI_PHY_LINK_FAULT_NONE))) {
diff --git a/drivers/scsi/be2iscsi/be_cmds.h b/drivers/scsi/be2iscsi/be_cmds.h
index 724974e..a194066 100644
--- a/drivers/scsi/be2iscsi/be_cmds.h
+++ b/drivers/scsi/be2iscsi/be_cmds.h
@@ -153,12 +153,21 @@ struct be_async_event_link_state {
u8 physical_port;
u8 port_link_status;
u8 port_duplex;
+/* BE2ISCSI_LINK_SPEED_ZERO0x00 - no link */
+#define BE2ISCSI_LINK_SPEED_10MBPS 0x01
+#define BE2ISCSI_LINK_SPEED_100MBPS0x02
+#define BE2ISCSI_LINK_SPEED_1GBPS  0x03
+#define BE2ISCSI_LINK_SPEED_10GBPS 0x04
+#define BE2ISCSI_LINK_SPEED_25GBPS 0x06
+#define BE2ISCSI_LINK_SPEED_40GBPS 0x07
u8 port_speed;
 #define BEISCSI_PHY_LINK_FAULT_NONE0x00
 #define BEISCSI_PHY_LINK_FAULT_LOCAL   0x01
 #define BEISCSI_PHY_LINK_FAULT_REMOTE  0x02
u8 port_fault;
-   u8 rsvd0[7];
+   u8 event_reason;
+   u16 qos_link_speed;
+   u32 event_tag;
struct be_async_event_trailer trailer;
 } __packed;
 
@@ -711,29 +720,6 @@ struct be_cmd_hba_name {
u8 initiator_alias[BEISCSI_ALIAS_LEN];
 } __packed;
 
-struct be_cmd_ntwk_link_status_req {
-   struct be_cmd_req_hdr hdr;
-   u32 rsvd0;
-} __packed;
-
-/*** Port Speed Values ***/
-#define BE2ISCSI_LINK_SPEED_ZERO   0x00
-#define BE2ISCSI_LINK_SPEED_10MBPS 0x01
-#define BE2ISCSI_LINK_SPEED_100MBPS0x02
-#define BE2ISCSI_LINK_SPEED_1GBPS  0x03
-#define BE2ISCSI_LINK_SPEED_10GBPS 0x04
-struct be_cmd_ntwk_link_status_resp {
-   struct be_cmd_resp_hdr hdr;
-   u8 phys_port;
-   u8 mac_duplex;
-   u8 mac_speed;
-   u8 mac_fault;
-   u8 mgmt_mac_duplex;
-   u8 mgmt_mac_speed;
-   u16 qos_link_speed;
-   u32 logical_link_speed;
-} __packed;
-
 int beiscsi_cmd_eq_create(struct be_ctrl_info *ctrl,
  struct be_queue_info *eq, int eq_delay);
 
@@ -752,7 +738,6 @@ int be_poll_mcc(struct be_ctrl_info *ctrl);
 int mgmt_check_supported_fw(struct be_ctrl_info *ctrl,
  struct beiscsi_hba *phba);
 unsigned int be_cmd_get_initname(struct beiscsi_hba *phba);
-unsigned int be_cmd_get_port_speed(struct beiscsi_hba *phba);
 
 void free_mcc_tag(struct be_ctrl_info *ctrl, unsigned int tag);
 
diff --git a/drivers/scsi/be2iscsi/be_iscsi.c b/drivers/scsi/be2iscsi/be_iscsi.c
index 3545721..a3bc5e4 100644
--- a/drivers/scsi/be2iscsi/be_iscsi.c
+++ b/drivers/scsi/be2iscsi/be_iscsi.c
@@ -766,34 +766,13 @@ static void beiscsi_get_port_state(struct Scsi_Host 
*shost)
  * beiscsi_get_port_speed  - Get the Port Speed from Adapter
  * @shost : pointer to scsi_host structure
  *
- * returns Success/Failure
  */
-static int beiscsi_get_port_speed(struct Scsi_Host *shost)
+static void beiscsi_get_port_speed(struct Scsi_Host *shost)
 {
-   int rc;
-   unsigned int tag;
-   struct be_mcc_wrb *wrb;
-   struct be_cmd_ntwk_link_status_resp *resp;
struct beiscsi_hba *phba = iscsi_host_priv(shost);
struct iscsi_cls_host *ihost = shost->shost_data;
 
-   tag = be_cmd_get_port_speed(phba);
-   if (!tag) {
-   beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_CONFIG,
-   "BS_%d : Getting Port Speed Failed\n");
-
-return -EBUSY;
-   }
-   rc = beiscsi_mccq_compl(phba, tag, , NULL);
-   if (rc) {
-   beiscsi_log(phba, KERN_ERR,
-   BEISCSI_LOG_CONFIG | BEISCSI_LOG_MBOX,
-   "BS_%d : Port Speed MBX Failed\n");
-   return rc;
-   }
-   resp = embedded_payload(wrb);
-
-   switch (resp->mac_speed) {
+   switch (phba->port_speed) {
case BE2ISCSI_LINK_SPEED_10MBPS:
ihost->port_speed = 

[PATCH v2 17/17] be2iscsi: Update the driver version

[Jitendra Bhivare]

Driver version: 11.0.0.0

Signed-off-by: Jitendra Bhivare 
---
 drivers/scsi/be2iscsi/be_main.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/be2iscsi/be_main.h b/drivers/scsi/be2iscsi/be_main.h
index 41c708c..16a6fd0 100644
--- a/drivers/scsi/be2iscsi/be_main.h
+++ b/drivers/scsi/be2iscsi/be_main.h
@@ -36,7 +36,7 @@
 #include 
 
 #define DRV_NAME   "be2iscsi"
-#define BUILD_STR  "10.6.0.1"
+#define BUILD_STR  "11.0.0.0"
 #define BE_NAME"Emulex OneConnect" \
"Open-iSCSI Driver version" BUILD_STR
 #define DRV_DESC   BE_NAME " " "Driver"
-- 
1.9.1

--
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 13/17] scsi_transport_iscsi: Add 25G and 40G speed definition

[Jitendra Bhivare]

iscsi_port_speed and iscsi_port_speed_names have new entries for
25Gbps and 40Gbps link speeds.

Signed-off-by: Jitendra Bhivare 
---
 drivers/scsi/scsi_transport_iscsi.c | 2 ++
 include/scsi/iscsi_if.h | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/drivers/scsi/scsi_transport_iscsi.c 
b/drivers/scsi/scsi_transport_iscsi.c
index e4b3d8f..4414816 100644
--- a/drivers/scsi/scsi_transport_iscsi.c
+++ b/drivers/scsi/scsi_transport_iscsi.c
@@ -4308,6 +4308,8 @@ static const struct {
{ISCSI_PORT_SPEED_100MBPS,  "100 Mbps" },
{ISCSI_PORT_SPEED_1GBPS,"1 Gbps" },
{ISCSI_PORT_SPEED_10GBPS,   "10 Gbps" },
+   {ISCSI_PORT_SPEED_25GBPS,   "25 Gbps" },
+   {ISCSI_PORT_SPEED_40GBPS,   "40 Gbps" },
 };
 
 char *iscsi_get_port_speed_name(struct Scsi_Host *shost)
diff --git a/include/scsi/iscsi_if.h b/include/scsi/iscsi_if.h
index 95ed942..d66c070 100644
--- a/include/scsi/iscsi_if.h
+++ b/include/scsi/iscsi_if.h
@@ -724,6 +724,8 @@ enum iscsi_port_speed {
ISCSI_PORT_SPEED_100MBPS= 0x4,
ISCSI_PORT_SPEED_1GBPS  = 0x8,
ISCSI_PORT_SPEED_10GBPS = 0x10,
+   ISCSI_PORT_SPEED_25GBPS = 0x20,
+   ISCSI_PORT_SPEED_40GBPS = 0x40,
 };
 
 /* iSCSI port state */
-- 
1.9.1

--
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 08/17] be2iscsi: Fix VLAN support for IPv6 network

[Jitendra Bhivare]

Configuring VLAN parameters through IPv6 interface was not supported in driver.

Signed-off-by: Jitendra Bhivare 
Reviewed-by: Hannes Reinecke 
---
 drivers/scsi/be2iscsi/be_iscsi.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/scsi/be2iscsi/be_iscsi.c b/drivers/scsi/be2iscsi/be_iscsi.c
index 188d83f..c89a025 100644
--- a/drivers/scsi/be2iscsi/be_iscsi.c
+++ b/drivers/scsi/be2iscsi/be_iscsi.c
@@ -466,6 +466,10 @@ beiscsi_set_ipv6(struct Scsi_Host *shost,
ret = mgmt_set_ip(phba, iface_param, NULL,
  ISCSI_BOOTPROTO_STATIC);
break;
+   case ISCSI_NET_PARAM_VLAN_ENABLED:
+   case ISCSI_NET_PARAM_VLAN_TAG:
+   ret = beiscsi_set_vlan_tag(shost, iface_param);
+   break;
default:
beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_CONFIG,
"BS_%d : Param %d not supported\n",
-- 
1.9.1

--
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 12/17] be2iscsi: Fix IOPOLL implementation

[Jitendra Bhivare]

OS not responding when running 2 port traffic on 72 CPUs system.

be2iscsi IRQs gets affined to CPU0 when irqbalancer is disabled.
be_iopoll processing completions in BLOCK_IOPOLL_SOFTIRQ hogged CPU0.

1. Use budget to exit the polling loop. beiscsi_process_cq didn't honour it.
2. Rearming of EQ is done only after iopoll completes.

Signed-off-by: Jitendra Bhivare 
Reviewed-by: Hannes Reinecke 
---
 drivers/scsi/be2iscsi/be_cmds.c  |  2 +-
 drivers/scsi/be2iscsi/be_iscsi.c |  2 +-
 drivers/scsi/be2iscsi/be_main.c  | 91 ++--
 drivers/scsi/be2iscsi/be_main.h  |  5 ++-
 4 files changed, 56 insertions(+), 44 deletions(-)

diff --git a/drivers/scsi/be2iscsi/be_cmds.c b/drivers/scsi/be2iscsi/be_cmds.c
index dbe62c0..14a1c71 100644
--- a/drivers/scsi/be2iscsi/be_cmds.c
+++ b/drivers/scsi/be2iscsi/be_cmds.c
@@ -546,7 +546,7 @@ int beiscsi_process_mcc(struct beiscsi_hba *phba)
}
 
if (num)
-   hwi_ring_cq_db(phba, phba->ctrl.mcc_obj.cq.id, num, 1, 0);
+   hwi_ring_cq_db(phba, phba->ctrl.mcc_obj.cq.id, num, 1);
 
spin_unlock_bh(>ctrl.mcc_cq_lock);
return status;
diff --git a/drivers/scsi/be2iscsi/be_iscsi.c b/drivers/scsi/be2iscsi/be_iscsi.c
index c89a025..3545721 100644
--- a/drivers/scsi/be2iscsi/be_iscsi.c
+++ b/drivers/scsi/be2iscsi/be_iscsi.c
@@ -1298,7 +1298,7 @@ static void beiscsi_flush_cq(struct beiscsi_hba *phba)
for (i = 0; i < phba->num_cpus; i++) {
pbe_eq = _context->be_eq[i];
blk_iopoll_disable(_eq->iopoll);
-   beiscsi_process_cq(pbe_eq);
+   beiscsi_process_cq(pbe_eq, BE2_MAX_NUM_CQ_PROC);
blk_iopoll_enable(_eq->iopoll);
}
 }
diff --git a/drivers/scsi/be2iscsi/be_main.c b/drivers/scsi/be2iscsi/be_main.c
index a665e6a..9a86044 100644
--- a/drivers/scsi/be2iscsi/be_main.c
+++ b/drivers/scsi/be2iscsi/be_main.c
@@ -895,32 +895,21 @@ static irqreturn_t be_isr_mcc(int irq, void *dev_id)
 static irqreturn_t be_isr_msix(int irq, void *dev_id)
 {
struct beiscsi_hba *phba;
-   struct be_eq_entry *eqe = NULL;
struct be_queue_info *eq;
-   struct be_queue_info *cq;
-   unsigned int num_eq_processed;
struct be_eq_obj *pbe_eq;
 
pbe_eq = dev_id;
eq = _eq->q;
-   cq = pbe_eq->cq;
-   eqe = queue_tail_node(eq);
 
phba = pbe_eq->phba;
-   num_eq_processed = 0;
-   while (eqe->dw[offsetof(struct amap_eq_entry, valid) / 32]
-   & EQE_VALID_MASK) {
-   if (!blk_iopoll_sched_prep(_eq->iopoll))
-   blk_iopoll_sched(_eq->iopoll);
-
-   AMAP_SET_BITS(struct amap_eq_entry, valid, eqe, 0);
-   queue_tail_inc(eq);
-   eqe = queue_tail_node(eq);
-   num_eq_processed++;
-   }
-
-   if (num_eq_processed)
-   hwi_ring_eq_db(phba, eq->id, 1, num_eq_processed, 0, 1);
+   /* disable interrupt till iopoll completes */
+   hwi_ring_eq_db(phba, eq->id, 1, 0, 0, 1);
+   if (!blk_iopoll_sched_prep(_eq->iopoll))
+   blk_iopoll_sched(_eq->iopoll);
+   else
+   beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_IO,
+   "BM_%d: received event while polling eq %d cq %d\n",
+   eq->id, pbe_eq->cq->id);
 
return IRQ_HANDLED;
 }
@@ -998,6 +987,7 @@ static irqreturn_t be_isr(int irq, void *dev_id)
return IRQ_NONE;
 }
 
+
 static int beiscsi_init_irqs(struct beiscsi_hba *phba)
 {
struct pci_dev *pcidev = phba->pcidev;
@@ -1072,7 +1062,7 @@ free_msix_irqs:
 
 void hwi_ring_cq_db(struct beiscsi_hba *phba,
   unsigned int id, unsigned int num_processed,
-  unsigned char rearm, unsigned char event)
+  unsigned char rearm)
 {
u32 val = 0;
 
@@ -2044,7 +2034,7 @@ static void  beiscsi_process_mcc_isr(struct beiscsi_hba 
*phba)
 
if (num_processed >= 32) {
hwi_ring_cq_db(phba, mcc_cq->id,
-   num_processed, 0, 0);
+   num_processed, 0);
num_processed = 0;
}
if (mcc_compl->flags & CQE_FLAGS_ASYNC_MASK) {
@@ -2062,24 +2052,25 @@ static void  beiscsi_process_mcc_isr(struct beiscsi_hba 
*phba)
}
 
if (num_processed > 0)
-   hwi_ring_cq_db(phba, mcc_cq->id, num_processed, 1, 0);
+   hwi_ring_cq_db(phba, mcc_cq->id, num_processed, 1);
 
 }
 
 /**
  * beiscsi_process_cq()- Process the Completion Queue
  * @pbe_eq: Event Q on which the Completion has come
+ * @budget: Max number of events to processed
  *
  * return
  * Number of Completion Entries processed.
  **/
-unsigned int beiscsi_process_cq(struct be_eq_obj *pbe_eq)
+unsigned int 

[PATCH v2 07/17] be2iscsi: Fix to remove shutdown entry point

[Jitendra Bhivare]

Null pointer dereference in shutdown path after taking dump.

Shutdown path is not needed as FW comes up clean every time during probe
after issuing FUNCTION reset MBOX command.

Signed-off-by: Jitendra Bhivare 
Reviewed-by: Hannes Reinecke 
---
 drivers/scsi/be2iscsi/be_cmds.c | 17 +++--
 drivers/scsi/be2iscsi/be_cmds.h |  2 +-
 drivers/scsi/be2iscsi/be_main.c | 24 
 drivers/scsi/be2iscsi/be_main.h |  3 +--
 4 files changed, 9 insertions(+), 37 deletions(-)

diff --git a/drivers/scsi/be2iscsi/be_cmds.c b/drivers/scsi/be2iscsi/be_cmds.c
index f93ab6d..66a1fc3 100644
--- a/drivers/scsi/be2iscsi/be_cmds.c
+++ b/drivers/scsi/be2iscsi/be_cmds.c
@@ -417,22 +417,11 @@ static struct be_mcc_compl *be_mcc_compl_get(struct 
beiscsi_hba *phba)
 }
 
 /**
- * be2iscsi_fail_session(): Closing session with appropriate error
+ * beiscsi_fail_session(): Closing session with appropriate error
  * @cls_session: ptr to session
- *
- * Depending on adapter state appropriate error flag is passed.
  **/
-void be2iscsi_fail_session(struct iscsi_cls_session *cls_session)
+void beiscsi_fail_session(struct iscsi_cls_session *cls_session)
 {
-   struct Scsi_Host *shost = iscsi_session_to_shost(cls_session);
-   struct beiscsi_hba *phba = iscsi_host_priv(shost);
-   uint32_t iscsi_err_flag;
-
-   if (phba->state & BE_ADAPTER_STATE_SHUTDOWN)
-   iscsi_err_flag = ISCSI_ERR_INVALID_HOST;
-   else
-   iscsi_err_flag = ISCSI_ERR_CONN_FAILED;
-
iscsi_session_failure(cls_session->dd_data, ISCSI_ERR_CONN_FAILED);
 }
 
@@ -450,7 +439,7 @@ void beiscsi_async_link_state_process(struct beiscsi_hba 
*phba,
evt->physical_port);
 
iscsi_host_for_each_session(phba->shost,
-   be2iscsi_fail_session);
+   beiscsi_fail_session);
} else if ((evt->port_link_status & ASYNC_EVENT_LINK_UP) ||
((evt->port_link_status & ASYNC_EVENT_LOGICAL) &&
 (evt->port_fault == BEISCSI_PHY_LINK_FAULT_NONE))) {
diff --git a/drivers/scsi/be2iscsi/be_cmds.h b/drivers/scsi/be2iscsi/be_cmds.h
index 1883d32..f988164 100644
--- a/drivers/scsi/be2iscsi/be_cmds.h
+++ b/drivers/scsi/be2iscsi/be_cmds.h
@@ -1367,5 +1367,5 @@ void be_wrb_hdr_prepare(struct be_mcc_wrb *wrb, int 
payload_len,
 void be_cmd_hdr_prepare(struct be_cmd_req_hdr *req_hdr,
u8 subsystem, u8 opcode, int cmd_len);
 
-void be2iscsi_fail_session(struct iscsi_cls_session *cls_session);
+void beiscsi_fail_session(struct iscsi_cls_session *cls_session);
 #endif /* !BEISCSI_CMDS_H */
diff --git a/drivers/scsi/be2iscsi/be_main.c b/drivers/scsi/be2iscsi/be_main.c
index 61ce86b..2f3e118 100644
--- a/drivers/scsi/be2iscsi/be_main.c
+++ b/drivers/scsi/be2iscsi/be_main.c
@@ -5315,7 +5315,6 @@ static void beiscsi_quiesce(struct beiscsi_hba *phba,
 
 static void beiscsi_remove(struct pci_dev *pcidev)
 {
-
struct beiscsi_hba *phba = NULL;
 
phba = pci_get_drvdata(pcidev);
@@ -5325,9 +5324,9 @@ static void beiscsi_remove(struct pci_dev *pcidev)
}
 
beiscsi_destroy_def_ifaces(phba);
-   beiscsi_quiesce(phba, BEISCSI_CLEAN_UNLOAD);
iscsi_boot_destroy_kset(phba->boot_kset);
iscsi_host_remove(phba->shost);
+   beiscsi_quiesce(phba, BEISCSI_CLEAN_UNLOAD);
pci_dev_put(phba->pcidev);
iscsi_host_free(phba->shost);
pci_disable_pcie_error_reporting(pcidev);
@@ -5336,23 +5335,6 @@ static void beiscsi_remove(struct pci_dev *pcidev)
pci_disable_device(pcidev);
 }
 
-static void beiscsi_shutdown(struct pci_dev *pcidev)
-{
-
-   struct beiscsi_hba *phba = NULL;
-
-   phba = (struct beiscsi_hba *)pci_get_drvdata(pcidev);
-   if (!phba) {
-   dev_err(>dev, "beiscsi_shutdown called with no phba\n");
-   return;
-   }
-
-   phba->state = BE_ADAPTER_STATE_SHUTDOWN;
-   iscsi_host_for_each_session(phba->shost, be2iscsi_fail_session);
-   beiscsi_quiesce(phba, BEISCSI_CLEAN_UNLOAD);
-   pci_disable_device(pcidev);
-}
-
 static void beiscsi_msix_enable(struct beiscsi_hba *phba)
 {
int i, status;
@@ -5673,6 +5655,9 @@ static int beiscsi_dev_probe(struct pci_dev *pcidev,
goto hba_free;
}
 
+   /*
+* FUNCTION_RESET should clean up any stale info in FW for this fn
+*/
ret = beiscsi_cmd_reset_function(phba);
if (ret) {
beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_INIT,
@@ -5861,7 +5846,6 @@ static struct pci_driver beiscsi_pci_driver = {
.name = DRV_NAME,
.probe = beiscsi_dev_probe,
.remove = beiscsi_remove,
-   .shutdown = beiscsi_shutdown,
.id_table = beiscsi_pci_id_table,
.err_handler = _eeh_handlers
 };
diff --git a/drivers/scsi/be2iscsi/be_main.h 

[PATCH v2 02/17] be2iscsi: Fix mbox synchronization replacing spinlock with mutex

[Jitendra Bhivare]

This is second part of actual fix for soft lockup.

All mbox cmds issued using BMBX and MCC are synchronized using mutex
mbox_lock instead of spin_lock. Used mutex_lock_interruptible where
ever possible.

Signed-off-by: Jitendra Bhivare 
---
 drivers/scsi/be2iscsi/be.h  |   2 +-
 drivers/scsi/be2iscsi/be_cmds.c |  73 ++--
 drivers/scsi/be2iscsi/be_main.c |   2 +-
 drivers/scsi/be2iscsi/be_mgmt.c | 105 +---
 4 files changed, 94 insertions(+), 88 deletions(-)

diff --git a/drivers/scsi/be2iscsi/be.h b/drivers/scsi/be2iscsi/be.h
index 77f992e..cf19bce 100644
--- a/drivers/scsi/be2iscsi/be.h
+++ b/drivers/scsi/be2iscsi/be.h
@@ -124,7 +124,7 @@ struct be_ctrl_info {
struct pci_dev *pdev;
 
/* Mbox used for cmd request/response */
-   spinlock_t mbox_lock;   /* For serializing mbox cmds to BE card */
+   struct mutex mbox_lock; /* For serializing mbox cmds to BE card */
struct be_dma_mem mbox_mem;
/* Mbox mem is adjusted to align to 16 bytes. The allocated addr
 * is stored for freeing purpose */
diff --git a/drivers/scsi/be2iscsi/be_cmds.c b/drivers/scsi/be2iscsi/be_cmds.c
index cd50e3c..6fabded 100644
--- a/drivers/scsi/be2iscsi/be_cmds.c
+++ b/drivers/scsi/be2iscsi/be_cmds.c
@@ -164,9 +164,9 @@ int beiscsi_mccq_compl(struct beiscsi_hba *phba,
}
 
/* Set MBX Tag state to Active */
-   spin_lock(>ctrl.mbox_lock);
+   mutex_lock(>ctrl.mbox_lock);
phba->ctrl.ptag_state[tag].tag_state = MCC_TAG_STATE_RUNNING;
-   spin_unlock(>ctrl.mbox_lock);
+   mutex_unlock(>ctrl.mbox_lock);
 
/* wait for the mccq completion */
rc = wait_event_interruptible_timeout(
@@ -178,9 +178,9 @@ int beiscsi_mccq_compl(struct beiscsi_hba *phba,
if (rc <= 0) {
struct be_dma_mem *tag_mem;
/* Set MBX Tag state to timeout */
-   spin_lock(>ctrl.mbox_lock);
+   mutex_lock(>ctrl.mbox_lock);
phba->ctrl.ptag_state[tag].tag_state = MCC_TAG_STATE_TIMEOUT;
-   spin_unlock(>ctrl.mbox_lock);
+   mutex_unlock(>ctrl.mbox_lock);
 
/* Store resource addr to be freed later */
tag_mem = >ctrl.ptag_state[tag].tag_mem_state;
@@ -199,9 +199,9 @@ int beiscsi_mccq_compl(struct beiscsi_hba *phba,
} else {
rc = 0;
/* Set MBX Tag state to completed */
-   spin_lock(>ctrl.mbox_lock);
+   mutex_lock(>ctrl.mbox_lock);
phba->ctrl.ptag_state[tag].tag_state = MCC_TAG_STATE_COMPLETED;
-   spin_unlock(>ctrl.mbox_lock);
+   mutex_unlock(>ctrl.mbox_lock);
}
 
mcc_tag_response = phba->ctrl.mcc_numtag[tag];
@@ -390,9 +390,9 @@ int be_mcc_compl_process_isr(struct be_ctrl_info *ctrl,
tag_mem->va, tag_mem->dma);
 
/* Change tag state */
-   spin_lock(>ctrl.mbox_lock);
+   mutex_lock(>ctrl.mbox_lock);
ctrl->ptag_state[tag].tag_state = MCC_TAG_STATE_COMPLETED;
-   spin_unlock(>ctrl.mbox_lock);
+   mutex_unlock(>ctrl.mbox_lock);
 
/* Free MCC Tag */
free_mcc_tag(ctrl, tag);
@@ -831,7 +831,7 @@ int beiscsi_cmd_eq_create(struct be_ctrl_info *ctrl,
struct be_dma_mem *q_mem = >dma_mem;
int status;
 
-   spin_lock(>mbox_lock);
+   mutex_lock(>mbox_lock);
memset(wrb, 0, sizeof(*wrb));
 
be_wrb_hdr_prepare(wrb, sizeof(*req), true, 0);
@@ -858,7 +858,7 @@ int beiscsi_cmd_eq_create(struct be_ctrl_info *ctrl,
eq->id = le16_to_cpu(resp->eq_id);
eq->created = true;
}
-   spin_unlock(>mbox_lock);
+   mutex_unlock(>mbox_lock);
return status;
 }
 
@@ -879,7 +879,7 @@ int be_cmd_fw_initialize(struct be_ctrl_info *ctrl)
int status;
u8 *endian_check;
 
-   spin_lock(>mbox_lock);
+   mutex_lock(>mbox_lock);
memset(wrb, 0, sizeof(*wrb));
 
endian_check = (u8 *) wrb;
@@ -898,7 +898,7 @@ int be_cmd_fw_initialize(struct be_ctrl_info *ctrl)
beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_INIT,
"BC_%d : be_cmd_fw_initialize Failed\n");
 
-   spin_unlock(>mbox_lock);
+   mutex_unlock(>mbox_lock);
return status;
 }
 
@@ -919,7 +919,7 @@ int be_cmd_fw_uninit(struct be_ctrl_info *ctrl)
int status;
u8 *endian_check;
 
-   spin_lock(>mbox_lock);
+   mutex_lock(>mbox_lock);
memset(wrb, 0, sizeof(*wrb));
 
endian_check = (u8 *) wrb;
@@ -939,7 +939,7 @@ int be_cmd_fw_uninit(struct be_ctrl_info *ctrl)
beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_INIT,
"BC_%d : be_cmd_fw_uninit Failed\n");
 
-   spin_unlock(>mbox_lock);
+   

[PATCH v2 00/17] be2iscsi: driver update 11.0.0.0

[Jitendra Bhivare]

This patch is generated against for-next branch.

Jitendra Bhivare (17):
  be2iscsi: Fix soft lockup in mgmt_get_all_if_id path using bmbx
  be2iscsi: Fix mbox synchronization replacing spinlock with mutex
  be2iscsi: Fix to synchronize tag allocation using spin_lock
  be2iscsi: Set mbox timeout to 30s
  be2iscsi: Added return value check for mgmt_get_all_if_id
  be2iscsi: Fix to remove shutdown entry point
  be2iscsi: Fix VLAN support for IPv6 network
  be2iscsi: Fix to handle misconfigured optics events
  be2iscsi: Fix return value for MCC completion
  be2iscsi: Fix IOPOLL implementation
  be2iscsi: Fix WRB leak in login/logout path
  be2iscsi: Update the driver version

  be2iscsi: Fix to use atomic bit operations for tag_state
  be2iscsi: Add FW config validation
  scsi_transport_iscsi: Add 25G and 40G speed definition
  be2iscsi: Fix to process 25G link speed info from FW
  be2iscsi: Fix async link event processing

 drivers/scsi/be2iscsi/be.h  |   9 +-
 drivers/scsi/be2iscsi/be_cmds.c | 453 +++-
 drivers/scsi/be2iscsi/be_cmds.h | 118 +-
 drivers/scsi/be2iscsi/be_iscsi.c|  54 ++---
 drivers/scsi/be2iscsi/be_main.c | 210 +
 drivers/scsi/be2iscsi/be_main.h |  23 +-
 drivers/scsi/be2iscsi/be_mgmt.c | 425 -
 drivers/scsi/be2iscsi/be_mgmt.h |   2 +
 drivers/scsi/scsi_transport_iscsi.c |   2 +
 include/scsi/iscsi_if.h |   2 +
 10 files changed, 720 insertions(+), 578 deletions(-)

-- 
1.9.1

--
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 01/17] be2iscsi: Fix soft lockup in mgmt_get_all_if_id path using bmbx

[Jitendra Bhivare]

We are taking mbox_lock spinlock which disables pre-emption before we poll
for mbox completion. Waiting there with spinlock held in excess of 20s will
cause soft lockup.

Actual fix is to change mbox_lock to mutex.
The changes are done in phases. This is the first part.
1. Changed mgmt_get_all_if_id to use MCC where after posting lock is released.
2. Changed be_mbox_db_ready_wait to busy wait for 12s max and removed
wait_event_timeout. Added error handling code for IO reads.
OPCODE_COMMON_QUERY_FIRMWARE_CONFIG mbox command takes 8s time when unreachable
boot targets configured.

Signed-off-by: Jitendra Bhivare 
Reviewed-by: Hannes Reinecke 
---
 drivers/scsi/be2iscsi/be_cmds.c | 60 -
 drivers/scsi/be2iscsi/be_mgmt.c | 32 +++---
 2 files changed, 51 insertions(+), 41 deletions(-)

diff --git a/drivers/scsi/be2iscsi/be_cmds.c b/drivers/scsi/be2iscsi/be_cmds.c
index 2778089..cd50e3c 100644
--- a/drivers/scsi/be2iscsi/be_cmds.c
+++ b/drivers/scsi/be2iscsi/be_cmds.c
@@ -587,47 +587,42 @@ int be_mcc_notify_wait(struct beiscsi_hba *phba)
  **/
 static int be_mbox_db_ready_wait(struct be_ctrl_info *ctrl)
 {
-#define BEISCSI_MBX_RDY_BIT_TIMEOUT4000/* 4sec */
+#define BEISCSI_MBX_RDY_BIT_TIMEOUT12000   /* 12sec */
void __iomem *db = ctrl->db + MPU_MAILBOX_DB_OFFSET;
struct beiscsi_hba *phba = pci_get_drvdata(ctrl->pdev);
unsigned long timeout;
-   bool read_flag = false;
-   int ret = 0, i;
u32 ready;
-   DECLARE_WAIT_QUEUE_HEAD_ONSTACK(rdybit_check_q);
 
-   if (beiscsi_error(phba))
-   return -EIO;
+   /*
+* This BMBX busy wait path is used during init only.
+* For the commands executed during init, 5s should suffice.
+*/
+   timeout = jiffies + msecs_to_jiffies(BEISCSI_MBX_RDY_BIT_TIMEOUT);
+   do {
+   if (beiscsi_error(phba))
+   return -EIO;
 
-   timeout = jiffies + (HZ * 110);
+   ready = ioread32(db);
+   if (ready == 0x)
+   return -EIO;
 
-   do {
-   for (i = 0; i < BEISCSI_MBX_RDY_BIT_TIMEOUT; i++) {
-   ready = ioread32(db) & MPU_MAILBOX_DB_RDY_MASK;
-   if (ready) {
-   read_flag = true;
-   break;
-   }
-   mdelay(1);
-   }
+   ready &= MPU_MAILBOX_DB_RDY_MASK;
+   if (ready)
+   return 0;
 
-   if (!read_flag) {
-   wait_event_timeout(rdybit_check_q,
- (read_flag != true),
-  HZ * 5);
-   }
-   } while ((time_before(jiffies, timeout)) && !read_flag);
+   if (time_after(jiffies, timeout))
+   break;
+   mdelay(1);
+   } while (!ready);
 
-   if (!read_flag) {
-   beiscsi_log(phba, KERN_ERR,
-   BEISCSI_LOG_CONFIG | BEISCSI_LOG_MBOX,
-   "BC_%d : FW Timed Out\n");
-   phba->fw_timeout = true;
-   beiscsi_ue_detect(phba);
-   ret = -EBUSY;
-   }
+   beiscsi_log(phba, KERN_ERR,
+   BEISCSI_LOG_CONFIG | BEISCSI_LOG_MBOX,
+   "BC_%d : FW Timed Out\n");
 
-   return ret;
+   phba->fw_timeout = true;
+   beiscsi_ue_detect(phba);
+
+   return -EBUSY;
 }
 
 /*
@@ -674,6 +669,9 @@ int be_mbox_notify(struct be_ctrl_info *ctrl)
if (status)
return status;
 
+   /* RDY is set; small delay before CQE read. */
+   udelay(1);
+
if (be_mcc_compl_is_new(compl)) {
status = be_mcc_compl_process(ctrl, >compl);
be_mcc_compl_use(compl);
diff --git a/drivers/scsi/be2iscsi/be_mgmt.c b/drivers/scsi/be2iscsi/be_mgmt.c
index aea3e6b..7b54b23 100644
--- a/drivers/scsi/be2iscsi/be_mgmt.c
+++ b/drivers/scsi/be2iscsi/be_mgmt.c
@@ -809,27 +809,39 @@ int mgmt_open_connection(struct beiscsi_hba *phba,
 unsigned int mgmt_get_all_if_id(struct beiscsi_hba *phba)
 {
struct be_ctrl_info *ctrl = >ctrl;
-   struct be_mcc_wrb *wrb = wrb_from_mbox(>mbox_mem);
-   struct be_cmd_get_all_if_id_req *req = embedded_payload(wrb);
-   struct be_cmd_get_all_if_id_req *pbe_allid = req;
+   struct be_mcc_wrb *wrb;
+   struct be_cmd_get_all_if_id_req *req;
+   struct be_cmd_get_all_if_id_req *pbe_allid;
+   unsigned int tag;
int status = 0;
 
-   memset(wrb, 0, sizeof(*wrb));
-
spin_lock(>mbox_lock);
+   tag = alloc_mcc_tag(phba);
+   if (!tag) {
+   spin_unlock(>mbox_lock);
+   return -ENOMEM;
+   }
+
+   wrb = wrb_from_mccq(phba);
+  

[PATCH v2 06/17] be2iscsi: Added return value check for mgmt_get_all_if_id

[Jitendra Bhivare]

Use of mutex_lock_interruptible can return -EINTR, handle and log
the error.

Signed-off-by: Jitendra Bhivare 
Reviewed-by: Hannes Reinecke 
---
 drivers/scsi/be2iscsi/be_iscsi.c |  7 ---
 drivers/scsi/be2iscsi/be_mgmt.c  | 10 ++
 2 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/be2iscsi/be_iscsi.c b/drivers/scsi/be2iscsi/be_iscsi.c
index b7087ba..188d83f 100644
--- a/drivers/scsi/be2iscsi/be_iscsi.c
+++ b/drivers/scsi/be2iscsi/be_iscsi.c
@@ -367,13 +367,14 @@ beiscsi_set_vlan_tag(struct Scsi_Host *shost,
  struct iscsi_iface_param_info *iface_param)
 {
struct beiscsi_hba *phba = iscsi_host_priv(shost);
-   int ret = 0;
+   int ret;
 
/* Get the Interface Handle */
-   if (mgmt_get_all_if_id(phba)) {
+   ret = mgmt_get_all_if_id(phba);
+   if (ret) {
beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_CONFIG,
"BS_%d : Getting Interface Handle Failed\n");
-   return -EIO;
+   return ret;
}
 
switch (iface_param->param) {
diff --git a/drivers/scsi/be2iscsi/be_mgmt.c b/drivers/scsi/be2iscsi/be_mgmt.c
index 6ef5285..9268a70 100644
--- a/drivers/scsi/be2iscsi/be_mgmt.c
+++ b/drivers/scsi/be2iscsi/be_mgmt.c
@@ -1024,8 +1024,9 @@ int mgmt_set_ip(struct beiscsi_hba *phba,
uint32_t ip_type;
int rc;
 
-   if (mgmt_get_all_if_id(phba))
-   return -EIO;
+   rc = mgmt_get_all_if_id(phba);
+   if (rc)
+   return rc;
 
ip_type = (ip_param->param == ISCSI_NET_PARAM_IPV6_ADDR) ?
BE2_IPV6 : BE2_IPV4 ;
@@ -1194,8 +1195,9 @@ int mgmt_get_if_info(struct beiscsi_hba *phba, int 
ip_type,
uint32_t ioctl_size = sizeof(struct be_cmd_get_if_info_resp);
int rc;
 
-   if (mgmt_get_all_if_id(phba))
-   return -EIO;
+   rc = mgmt_get_all_if_id(phba);
+   if (rc)
+   return rc;
 
do {
rc = mgmt_alloc_cmd_data(phba, _cmd,
-- 
1.9.1

--
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 15/17] be2iscsi: Fix async link event processing

[Jitendra Bhivare]

Use only port_link_status only to determine link state change.
Only bit 0 is used to get the state.
Remove code for processing port_fault.

Fixed get_nic_conf structure definition. Removed rsvd[23] field in
be_cmd_get_nic_conf_resp.

Moved defintions of struct field values below the field.

Signed-off-by: Jitendra Bhivare 
---
 drivers/scsi/be2iscsi/be_cmds.c  | 48 +++-
 drivers/scsi/be2iscsi/be_cmds.h  | 32 +--
 drivers/scsi/be2iscsi/be_iscsi.c |  2 +-
 3 files changed, 34 insertions(+), 48 deletions(-)

diff --git a/drivers/scsi/be2iscsi/be_cmds.c b/drivers/scsi/be2iscsi/be_cmds.c
index ce82f4d..34c33d4 100644
--- a/drivers/scsi/be2iscsi/be_cmds.c
+++ b/drivers/scsi/be2iscsi/be_cmds.c
@@ -405,32 +405,31 @@ void beiscsi_fail_session(struct iscsi_cls_session 
*cls_session)
iscsi_session_failure(cls_session->dd_data, ISCSI_ERR_CONN_FAILED);
 }
 
-static void beiscsi_async_link_state_process(struct beiscsi_hba *phba,
-   struct be_async_event_link_state *evt)
+static void beiscsi_process_async_link(struct beiscsi_hba *phba,
+  struct be_mcc_compl *compl)
 {
-   phba->port_speed = evt->port_speed;
-   if ((evt->port_link_status == ASYNC_EVENT_LINK_DOWN) ||
-   ((evt->port_link_status & ASYNC_EVENT_LOGICAL) &&
-(evt->port_fault != BEISCSI_PHY_LINK_FAULT_NONE))) {
-   phba->state = BE_ADAPTER_LINK_DOWN;
+   struct be_async_event_link_state *evt;
 
-   beiscsi_log(phba, KERN_ERR,
-   BEISCSI_LOG_CONFIG | BEISCSI_LOG_INIT,
-   "BC_%d : Link Down on Port %d\n",
-   evt->physical_port);
+   evt = (struct be_async_event_link_state *)compl;
 
-   iscsi_host_for_each_session(phba->shost,
-   beiscsi_fail_session);
-   } else if ((evt->port_link_status & ASYNC_EVENT_LINK_UP) ||
-   ((evt->port_link_status & ASYNC_EVENT_LOGICAL) &&
-(evt->port_fault == BEISCSI_PHY_LINK_FAULT_NONE))) {
+   phba->port_speed = evt->port_speed;
+   /**
+* Check logical link status in ASYNC event.
+* This has been newly introduced in SKH-R Firmware 10.0.338.45.
+**/
+   if (evt->port_link_status & BE_ASYNC_LINK_UP_MASK) {
phba->state = BE_ADAPTER_LINK_UP | BE_ADAPTER_CHECK_BOOT;
phba->get_boot = BE_GET_BOOT_RETRIES;
-
-   beiscsi_log(phba, KERN_ERR,
-   BEISCSI_LOG_CONFIG | BEISCSI_LOG_INIT,
-   "BC_%d : Link UP on Port %d\n",
-   evt->physical_port);
+   __beiscsi_log(phba, KERN_ERR,
+ "BC_%d : Link Up on Port %d tag 0x%x\n",
+ evt->physical_port, evt->event_tag);
+   } else {
+   phba->state = BE_ADAPTER_LINK_DOWN;
+   __beiscsi_log(phba, KERN_ERR,
+ "BC_%d : Link Down on Port %d tag 0x%x\n",
+ evt->physical_port, evt->event_tag);
+   iscsi_host_for_each_session(phba->shost,
+   beiscsi_fail_session);
}
 }
 
@@ -507,8 +506,7 @@ void beiscsi_process_async_event(struct beiscsi_hba *phba,
evt_code &= ASYNC_TRAILER_EVENT_CODE_MASK;
switch (evt_code) {
case ASYNC_EVENT_CODE_LINK_STATE:
-   beiscsi_async_link_state_process(phba,
-   (struct be_async_event_link_state *)compl);
+   beiscsi_process_async_link(phba, compl);
break;
case ASYNC_EVENT_CODE_ISCSI:
phba->state |= BE_ADAPTER_CHECK_BOOT;
@@ -524,8 +522,8 @@ void beiscsi_process_async_event(struct beiscsi_hba *phba,
}
 
beiscsi_log(phba, sev, BEISCSI_LOG_CONFIG | BEISCSI_LOG_MBOX,
-   "BC_%d : ASYNC Event: status 0x%08x flags 0x%08x\n",
-   compl->status, compl->flags);
+   "BC_%d : ASYNC Event %x: status 0x%08x flags 0x%08x\n",
+   evt_code, compl->status, compl->flags);
 }
 
 int beiscsi_process_mcc(struct beiscsi_hba *phba)
diff --git a/drivers/scsi/be2iscsi/be_cmds.h b/drivers/scsi/be2iscsi/be_cmds.h
index a194066..7caf585 100644
--- a/drivers/scsi/be2iscsi/be_cmds.h
+++ b/drivers/scsi/be2iscsi/be_cmds.h
@@ -142,7 +142,6 @@ struct be_async_event_trailer {
 enum {
ASYNC_EVENT_LINK_DOWN = 0x0,
ASYNC_EVENT_LINK_UP = 0x1,
-   ASYNC_EVENT_LOGICAL = 0x2
 };
 
 /**
@@ -152,7 +151,15 @@ enum {
 struct be_async_event_link_state {
u8 physical_port;
u8 port_link_status;
+/**
+ * ASYNC_EVENT_LINK_DOWN   0x0
+ * ASYNC_EVENT_LINK_UP 0x1
+ * ASYNC_EVENT_LINK_LOGICAL_DOWN   0x2
+ * ASYNC_EVENT_LINK_LOGICAL_UP 0x3

[PATCH v2 05/17] be2iscsi: Set mbox timeout to 30s

[Jitendra Bhivare]

FW recommended timeout for all mbox command is 30s.
Use msleep instead mdelay to relinquish CPU when polling for
mbox completion.

Signed-off-by: Jitendra Bhivare 
Reviewed-by: Hannes Reinecke 
---
 drivers/scsi/be2iscsi/be_cmds.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/be2iscsi/be_cmds.c b/drivers/scsi/be2iscsi/be_cmds.c
index db03149..f93ab6d 100644
--- a/drivers/scsi/be2iscsi/be_cmds.c
+++ b/drivers/scsi/be2iscsi/be_cmds.c
@@ -590,7 +590,8 @@ int be_mcc_notify_wait(struct beiscsi_hba *phba, unsigned 
int tag)
  **/
 static int be_mbox_db_ready_wait(struct be_ctrl_info *ctrl)
 {
-#define BEISCSI_MBX_RDY_BIT_TIMEOUT12000   /* 12sec */
+   /* wait 30s for generic non-flash MBOX operation */
+#define BEISCSI_MBX_RDY_BIT_TIMEOUT3
void __iomem *db = ctrl->db + MPU_MAILBOX_DB_OFFSET;
struct beiscsi_hba *phba = pci_get_drvdata(ctrl->pdev);
unsigned long timeout;
@@ -615,7 +616,7 @@ static int be_mbox_db_ready_wait(struct be_ctrl_info *ctrl)
 
if (time_after(jiffies, timeout))
break;
-   mdelay(1);
+   msleep(20);
} while (!ready);
 
beiscsi_log(phba, KERN_ERR,
-- 
1.9.1

--
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 16/17] be2iscsi: Fix WRB leak in login/logout path

[Jitendra Bhivare]

Login/Logout loop was hanging after few hours. /var/log/message showed
that alloc_wrb_handle() function was not able to allocate any new WRB.

Sep 11 11:25:22 Jhelum10 kernel: connection32513:0: Could not send nopout
Sep 11 11:25:22 Jhelum10 kernel: scsi host10: BM_4989 : Alloc of WRB_HANDLE
Failedfor the CID : 384
Sep 11 11:25:22 Jhelum10 kernel: connection32513:0: Could not allocate pdu for
mgmt task.

Driver allocates WRB to pass login negotiated parameters information to FW
in beiscsi_offload_connection(). This allocated WRB was not freed so there
was WRB_Leak happening.

Put WRB used for posting the login-negotiated parameters back in pool.

Signed-off-by: Jitendra Bhivare 
Reviewed-by: Hannes Reinecke 
---
 drivers/scsi/be2iscsi/be_main.c | 72 -
 1 file changed, 49 insertions(+), 23 deletions(-)

diff --git a/drivers/scsi/be2iscsi/be_main.c b/drivers/scsi/be2iscsi/be_main.c
index 9a86044..d8cdb4e 100644
--- a/drivers/scsi/be2iscsi/be_main.c
+++ b/drivers/scsi/be2iscsi/be_main.c
@@ -1184,6 +1184,22 @@ free_io_sgl_handle(struct beiscsi_hba *phba, struct 
sgl_handle *psgl_handle)
phba->io_sgl_free_index++;
 }
 
+static inline struct wrb_handle *
+beiscsi_get_wrb_handle(struct hwi_wrb_context *pwrb_context,
+  unsigned int wrbs_per_cxn)
+{
+   struct wrb_handle *pwrb_handle;
+
+   pwrb_handle = pwrb_context->pwrb_handle_base[pwrb_context->alloc_index];
+   pwrb_context->wrb_handles_available--;
+   if (pwrb_context->alloc_index == (wrbs_per_cxn - 1))
+   pwrb_context->alloc_index = 0;
+   else
+   pwrb_context->alloc_index++;
+
+   return pwrb_handle;
+}
+
 /**
  * alloc_wrb_handle - To allocate a wrb handle
  * @phba: The hba pointer
@@ -1193,30 +1209,30 @@ free_io_sgl_handle(struct beiscsi_hba *phba, struct 
sgl_handle *psgl_handle)
  * This happens under session_lock until submission to chip
  */
 struct wrb_handle *alloc_wrb_handle(struct beiscsi_hba *phba, unsigned int cid,
-struct hwi_wrb_context **pcontext)
+   struct hwi_wrb_context **pcontext)
 {
struct hwi_wrb_context *pwrb_context;
struct hwi_controller *phwi_ctrlr;
-   struct wrb_handle *pwrb_handle;
uint16_t cri_index = BE_GET_CRI_FROM_CID(cid);
 
phwi_ctrlr = phba->phwi_ctrlr;
pwrb_context = _ctrlr->wrb_context[cri_index];
-   if (pwrb_context->wrb_handles_available >= 2) {
-   pwrb_handle = pwrb_context->pwrb_handle_base[
-   pwrb_context->alloc_index];
-   pwrb_context->wrb_handles_available--;
-   if (pwrb_context->alloc_index ==
-   (phba->params.wrbs_per_cxn - 1))
-   pwrb_context->alloc_index = 0;
-   else
-   pwrb_context->alloc_index++;
+   /* return the context address */
+   *pcontext = pwrb_context;
+   return beiscsi_get_wrb_handle(pwrb_context, phba->params.wrbs_per_cxn);
+}
 
-   /* Return the context address */
-   *pcontext = pwrb_context;
-   } else
-   pwrb_handle = NULL;
-   return pwrb_handle;
+static inline void
+beiscsi_put_wrb_handle(struct hwi_wrb_context *pwrb_context,
+  struct wrb_handle *pwrb_handle,
+  unsigned int wrbs_per_cxn)
+{
+   pwrb_context->pwrb_handle_base[pwrb_context->free_index] = pwrb_handle;
+   pwrb_context->wrb_handles_available++;
+   if (pwrb_context->free_index == (wrbs_per_cxn - 1))
+   pwrb_context->free_index = 0;
+   else
+   pwrb_context->free_index++;
 }
 
 /**
@@ -1231,13 +1247,9 @@ static void
 free_wrb_handle(struct beiscsi_hba *phba, struct hwi_wrb_context *pwrb_context,
struct wrb_handle *pwrb_handle)
 {
-   pwrb_context->pwrb_handle_base[pwrb_context->free_index] = pwrb_handle;
-   pwrb_context->wrb_handles_available++;
-   if (pwrb_context->free_index == (phba->params.wrbs_per_cxn - 1))
-   pwrb_context->free_index = 0;
-   else
-   pwrb_context->free_index++;
-
+   beiscsi_put_wrb_handle(pwrb_context,
+  pwrb_handle,
+  phba->params.wrbs_per_cxn);
beiscsi_log(phba, KERN_INFO,
BEISCSI_LOG_IO | BEISCSI_LOG_CONFIG,
"BM_%d : FREE WRB: pwrb_handle=%p free_index=0x%x"
@@ -4715,6 +4727,20 @@ beiscsi_offload_connection(struct beiscsi_conn 
*beiscsi_conn,
doorbell |= 1 << DB_DEF_PDU_NUM_POSTED_SHIFT;
iowrite32(doorbell, phba->db_va +
  beiscsi_conn->doorbell_offset);
+
+   /*
+* There is no completion for CONTEXT_UPDATE. The completion of next
+* WRB posted guarantees FW's processing and