[PATCH] IB/srp: Fix a memory leak

2015-11-18 Thread Bart Van Assche
If srp_connect_ch() returns a positive value then that is considered
by its caller as a connection failure but this does not result in a
scsi_host_put() call and additionally causes the srp_create_target()
function to return a positive value while it should return a negative
value. Avoid all this confusion and additionally fix a memory leak by
ensuring that srp_connect_ch() always returns a value that is <= 0.
This patch avoids that a rejected login triggers the following memory
leak:

unreferenced object 0x88021b24a220 (size 8):
  comm "srp_daemon", pid 56421, jiffies 4295006762 (age 4240.750s)
  hex dump (first 8 bytes):
68 6f 73 74 35 38 00 a5  host58..
  backtrace:
[] kmemleak_alloc+0x7a/0xc0
[] __kmalloc_track_caller+0xfe/0x160
[] kvasprintf+0x5b/0x90
[] kvasprintf_const+0x8d/0xb0
[] kobject_set_name_vargs+0x3c/0xa0
[] dev_set_name+0x3c/0x40
[] scsi_host_alloc+0x327/0x4b0
[] srp_create_target+0x4e/0x8a0 [ib_srp]
[] dev_attr_store+0x1b/0x20
[] sysfs_kf_write+0x4a/0x60
[] kernfs_fop_write+0x14e/0x180
[] __vfs_write+0x2f/0xf0
[] vfs_write+0xa4/0x100
[] SyS_write+0x54/0xc0
[] entry_SYSCALL_64_fastpath+0x12/0x6f

Signed-off-by: Bart Van Assche 
Cc: Sagi Grimberg 
Cc: Sebastian Parschauer 
Cc: Christoph Hellwig 
Cc: stable 
---
 drivers/infiniband/ulp/srp/ib_srp.c | 22 +-
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/drivers/infiniband/ulp/srp/ib_srp.c 
b/drivers/infiniband/ulp/srp/ib_srp.c
index eda427f8..3f4786a 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -993,16 +993,16 @@ static int srp_connect_ch(struct srp_rdma_ch *ch, bool 
multich)
 
ret = srp_lookup_path(ch);
if (ret)
-   return ret;
+   goto out;
 
while (1) {
init_completion(&ch->done);
ret = srp_send_req(ch, multich);
if (ret)
-   return ret;
+   goto out;
ret = wait_for_completion_interruptible(&ch->done);
if (ret < 0)
-   return ret;
+   goto out;
 
/*
 * The CM event handling code will set status to
@@ -1010,15 +1010,16 @@ static int srp_connect_ch(struct srp_rdma_ch *ch, bool 
multich)
 * back, or SRP_DLID_REDIRECT if we get a lid/qp
 * redirect REJ back.
 */
-   switch (ch->status) {
+   ret = ch->status;
+   switch (ret) {
case 0:
ch->connected = true;
-   return 0;
+   goto out;
 
case SRP_PORT_REDIRECT:
ret = srp_lookup_path(ch);
if (ret)
-   return ret;
+   goto out;
break;
 
case SRP_DLID_REDIRECT:
@@ -1027,13 +1028,16 @@ static int srp_connect_ch(struct srp_rdma_ch *ch, bool 
multich)
case SRP_STALE_CONN:
shost_printk(KERN_ERR, target->scsi_host, PFX
 "giving up on stale connection\n");
-   ch->status = -ECONNRESET;
-   return ch->status;
+   ret = -ECONNRESET;
+   goto out;
 
default:
-   return ch->status;
+   goto out;
}
}
+
+out:
+   return ret <= 0 ? ret : -ENODEV;
 }
 
 static int srp_inv_rkey(struct srp_rdma_ch *ch, u32 rkey, u32 send_flags)
-- 
2.1.4

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


Re: [PATCH v2] Separate target visibility from reaped state information

2015-11-18 Thread Bart Van Assche

On 11/16/2015 10:22 PM, James Bottomley wrote:

OK, could you justify this, please ... like with traces and things.
[ ... ]
If that isn't the case, we can fix it, but I'd like to see the evidence.


Hello James,

To my own surprise so far I have not yet been able to trigger a lockup 
without this patch. After I had inserted an msleep() call at the start 
of scsi_target_reap_ref_release() to delay the starget->state = 
STARGET_DEL assignment I rebuilt and rebooted the kernel, started 
multipathd and held a SCSI device open by creating a filesystem on it 
and by mounting it. Next I removed all SRP targets (for p in 
/sys/class/srp_remote_ports/*; do echo 1 >$p/delete & done). During this 
test no lockup was reported.


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


Re: kernel BUG at drivers/scsi/scsi_lib.c:1096!

2015-11-18 Thread Michael Ellerman
On Wed, 2015-11-18 at 09:03 -0500, Mark Salter wrote:
> On Wed, 2015-11-18 at 20:18 +1100, Michael Ellerman wrote:
> > Hi folks,
> > 
> > I'm intermittently seeing the following oops on at least one powerpc box.
> > 
> > The BUG_ON() is from:
> > 
> > static int scsi_init_sgtable(struct request *req, struct scsi_data_buffer 
> > *sdb)
> > {
> > ...
> > count = blk_rq_map_sg(req->q, req, sdb->table.sgl);
> > BUG_ON(count > sdb->table.nents);
> > 
> > Looking at the dump it looks like count was 2, I can't work out what nents 
> > was.
> > 
> > The machine's just a fairly boring bare metal setup, with a single IPR 
> > adapter:
> > 
> > 0001:08:00.0 RAID bus controller: IBM PCI-E IPR SAS Adapter (ASIC) (rev 02)
> > Subsystem: IBM PCIe3 x8 SAS RAID Internal Adapter 6Gb (57D7)
> > Flags: bus master, fast devsel, latency 0
> > Kernel driver in use: ipr
> > 
> > 
> > Anyone seen it before or have any ideas?
> 
> I'm also seeing it on arm64 in 4.4-rc1

Ah thanks, that's a good data point. I was assuming it was a driver bug, but I
assume you're not using IPR :)

> [6.859003] Call trace:
>   
> [6.861439] [] scsi_init_sgtable+0x84/0x88   
>   
> [6.867072] [] scsi_init_io+0x4c/0x1ac   
>   
> [6.872358] [] sd_setup_read_write_cmnd+0x44/0x844   
>   
> [6.878682] [] sd_init_command+0x38/0xb0 
>   
> [6.884141] [] scsi_setup_cmnd+0xd8/0x13c
>   
> [6.889686] [] scsi_prep_fn+0xc0/0x140   
>   
> [6.894973] [] blk_peek_request+0x148/0x24c  
>   
> [6.900692] [] scsi_request_fn+0x58/0x648
>   
> [6.906237] [] __blk_run_queue+0x40/0x58 
>   
> [6.911696] [] blk_run_queue+0x30/0x48   
>   
> [6.916983] [] scsi_run_queue+0x204/0x294
>   
> [6.922528] [] scsi_end_request+0x13c/0x1a0  
>   
> [6.928247] [] scsi_io_completion+0xf0/0x564 
>   
> [6.934052] [] scsi_finish_command+0xe4/0x144
>   
> [6.939943] [] scsi_softirq_done+0x148/0x178 
>   
> [6.945748] [] blk_done_softirq+0x7c/0x94
>   
> [6.951295] [] __do_softirq+0x114/0x2a0  
>   
> [6.956667] [] irq_exit+0x8c/0xe4
>   
> [6.961522] [] handle_IPI+0x170/0x228
>   
> [6.966721] [] gic_handle_irq+0xa0/0xb8  
>   
> [6.972093] Exception stack(0xfe03dc143de0 to 0xfe03dc143f00)  
>   

cheers

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

2015-11-18 Thread Finn Thain

On Wed, 18 Nov 2015, Ondrej Zary wrote:

> On Wednesday 18 November 2015, 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 about 750 
> > 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.
> >
> > I have compile-tested all patches to all NCR5380 drivers (x86, ARM, 
> > m68k) and regression tested mac_scsi and dmx3191d modules on suitable 
> > hardware. Testing the mac_scsi and dmx3191d modules provides only 
> > limited coverage. It would be good to see some testing of ISA cards 
> > and Sun 3 and Atari hardware too (I don't have any).
> 
> I have some NCR5380 ISA cards and can test them.
> 

Thanks Ondrej. I've no idea which ISA drivers are presently working in 
mainline. Finding regressions may be more difficult than usual ;-)

Michael, Sam: only atari_scsi and sun3_scsi implement DMA support, so some
testing of either driver would be helpful.

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

2015-11-18 Thread Michael Schmitz
Hi Finn,

>>>
>>> I have compile-tested all patches to all NCR5380 drivers (x86, ARM, 
>>> m68k) and regression tested mac_scsi and dmx3191d modules on suitable 
>>> hardware. Testing the mac_scsi and dmx3191d modules provides only 
>>> limited coverage. It would be good to see some testing of ISA cards 
>>> and Sun 3 and Atari hardware too (I don't have any).
>>
>> I have some NCR5380 ISA cards and can test them.
>>
> 
> Thanks Ondrej. I've no idea which ISA drivers are presently working in 
> mainline. Finding regressions may be more difficult than usual ;-)
> 
> Michael, Sam: only atari_scsi and sun3_scsi implement DMA support, so some
> testing of either driver would be helpful.

One way or another, I'll test this series or get someone to test a
kernel I built.

Cheers,

Michael



--
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 10/71] atari_NCR5380: Remove RESET_BOOT, CONFIG_ATARI_SCSI_TOSHIBA_DELAY and CONFIG_ATARI_SCSI_RESET_BOOT

2015-11-18 Thread Michael Schmitz
Hi Finn,

Am 18.11.2015 um 21:35 schrieb Finn Thain:

> The bus reset may raise an interrupt. That would be new behaviour for
> atari_scsi only when CONFIG_ATARI_SCSI_RESET_BOOT=n. The ST DMA interrupt
> is not assigned to atari_scsi at this stage, so
> CONFIG_ATARI_SCSI_RESET_BOOT=y may well be problematic already.

I can confirm that the bus reset at boot has never been problematic in
the past. It's been enabled in my kernels as long as I've used the
driver (must be getting close to 20 years now).

Cheers,


Michael


> Regardless, do_reset() now raises and clears the interrupt within
> local_irq_save/restore which should avoid problems.
> 
> Signed-off-by: Finn Thain 
> 
> ---
>  drivers/scsi/Kconfig |   17 ---
>  drivers/scsi/NCR5380.c   |   17 +--
>  drivers/scsi/NCR5380.h   |1 
>  drivers/scsi/atari_NCR5380.c |   22 +-
>  drivers/scsi/atari_scsi.c|   60 +--
>  drivers/scsi/mac_scsi.c  |   65 
> ++-
>  drivers/scsi/sun3_scsi.c |   47 ---
>  7 files changed, 51 insertions(+), 178 deletions(-)
> 
> Index: linux/drivers/scsi/Kconfig
> ===
> --- linux.orig/drivers/scsi/Kconfig   2015-11-18 19:25:56.0 +1100
> +++ linux/drivers/scsi/Kconfig2015-11-18 19:33:15.0 +1100
> @@ -1624,23 +1624,6 @@ config ATARI_SCSI
> ST-DMA, replacing ACSI).  It does NOT support other schemes, like
> in the Hades (without DMA).
>  
> -config ATARI_SCSI_TOSHIBA_DELAY
> - bool "Long delays for Toshiba CD-ROMs"
> - depends on ATARI_SCSI
> - help
> -   This option increases the delay after a SCSI arbitration to
> -   accommodate some flaky Toshiba CD-ROM drives. Say Y if you intend to
> -   use a Toshiba CD-ROM drive; otherwise, the option is not needed and
> -   would impact performance a bit, so say N.
> -
> -config ATARI_SCSI_RESET_BOOT
> - bool "Reset SCSI-devices at boottime"
> - depends on ATARI_SCSI
> - help
> -   Reset the devices on your Atari whenever it boots.  This makes the
> -   boot process fractionally longer but may assist recovery from errors
> -   that leave the devices with SCSI operations partway completed.
> -
>  config MAC_SCSI
>   tristate "Macintosh NCR5380 SCSI"
>   depends on MAC && SCSI=y
> Index: linux/drivers/scsi/atari_NCR5380.c
> ===
> --- linux.orig/drivers/scsi/atari_NCR5380.c   2015-11-18 19:33:13.0 
> +1100
> +++ linux/drivers/scsi/atari_NCR5380.c2015-11-18 19:33:15.0 
> +1100
> @@ -674,13 +674,14 @@ static void prepare_info(struct Scsi_Hos
>"base 0x%lx, irq %d, "
>"can_queue %d, cmd_per_lun %d, "
>"sg_tablesize %d, this_id %d, "
> -  "flags { %s}, "
> +  "flags { %s%s}, "
>"options { %s} ",
>instance->hostt->name, instance->io_port, instance->n_io_port,
>instance->base, instance->irq,
>instance->can_queue, instance->cmd_per_lun,
>instance->sg_tablesize, instance->this_id,
>hostdata->flags & FLAG_TAGGED_QUEUING ? "TAGGED_QUEUING " : "",
> +  hostdata->flags & FLAG_TOSHIBA_DELAY  ? "TOSHIBA_DELAY "  : "",
>  #ifdef DIFFERENTIAL
>"DIFFERENTIAL "
>  #endif
> @@ -860,6 +861,7 @@ static int __init NCR5380_init(struct Sc
>  
>  static int NCR5380_maybe_reset_bus(struct Scsi_Host *instance)
>  {
> + struct NCR5380_hostdata *hostdata = shost_priv(instance);
>   int pass;
>  
>   for (pass = 1; (NCR5380_read(STATUS_REG) & SR_BSY) && pass <= 6; 
> ++pass) {
> @@ -878,6 +880,14 @@ static int NCR5380_maybe_reset_bus(struc
>   case 4:
>   shost_printk(KERN_ERR, instance, "bus busy, attempting 
> reset\n");
>   do_reset(instance);
> + /* Wait after a reset; the SCSI standard calls for
> +  * 250ms, we wait 500ms to be on the safe side.
> +  * But some Toshiba CD-ROMs need ten times that.
> +  */
> + if (hostdata->flags & FLAG_TOSHIBA_DELAY)
> + msleep(2500);
> + else
> + msleep(500);
>   break;
>   case 6:
>   shost_printk(KERN_ERR, instance, "bus locked solid\n");
> @@ -1493,12 +1503,10 @@ static int NCR5380_select(struct Scsi_Ho
>* a minimum so we'll udelay ceil(1.2)
>*/
>  
> -#ifdef CONFIG_ATARI_SCSI_TOSHIBA_DELAY
> - /* ++roman: But some targets (see above :-) seem to need a bit more... 
> */
> - udelay(15);
> -#else
> - udelay(2);
> -#endif
> + if (hostdata->flags & FLAG_TOSHIBA_D

Re: [PATCH v5 00/32] HiSilicon SAS driver

2015-11-18 Thread Martin K. Petersen
> "John" == John Garry  writes:

John> thanks, please note that we still have the dependency on
John> http://www.spinics.net/lists/arm-kernel/msg452833.html

John> Without it the driver can only be built into the kernel, and not
John> as a module.

I have your driver in a staging branch rather than the main 4.5 SCSI
queue because I wanted to see what kind of additional fallout I'd get
from the zeroday testing.

It's not a problem for me to wait for that patch to go in (or take it
through SCSI if that makes things easier).

-- 
Martin K. Petersen  Oracle Linux Engineering
--
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 10/71] atari_NCR5380: Remove RESET_BOOT, CONFIG_ATARI_SCSI_TOSHIBA_DELAY and CONFIG_ATARI_SCSI_RESET_BOOT

2015-11-18 Thread Finn Thain
w
On Thu, 19 Nov 2015, Michael Schmitz wrote:

> Hi Finn,
> 
> Am 18.11.2015 um 21:35 schrieb Finn Thain:
> 
> > The bus reset may raise an interrupt. That would be new behaviour for 
> > atari_scsi only when CONFIG_ATARI_SCSI_RESET_BOOT=n. The ST DMA 
> > interrupt is not assigned to atari_scsi at this stage, so 
> > CONFIG_ATARI_SCSI_RESET_BOOT=y may well be problematic already.
> 
> I can confirm that the bus reset at boot has never been problematic in 
> the past. It's been enabled in my kernels as long as I've used the 
> driver (must be getting close to 20 years now).

That's good to know. I'm not sure why it was configurable in the first 
place (long delays?). The new algorithm (the one I copied from NCR5380.c) 
does not allow the user to prevent a possible scsi bus reset at driver 
init time. The scsi bus reset only takes place if the driver discovers 
that the bus was already wedged when it started. (It proved useful when I 
was introducing faults for EH testing, BTW.)

-- 

> 
> Cheers,
> 
> 
>   Michael
> 
> 
> > Regardless, do_reset() now raises and clears the interrupt within
> > local_irq_save/restore which should avoid problems.
> > 
> > Signed-off-by: Finn Thain 
> > 
> > ---
> >  drivers/scsi/Kconfig |   17 ---
> >  drivers/scsi/NCR5380.c   |   17 +--
> >  drivers/scsi/NCR5380.h   |1 
> >  drivers/scsi/atari_NCR5380.c |   22 +-
> >  drivers/scsi/atari_scsi.c|   60 +--
> >  drivers/scsi/mac_scsi.c  |   65 
> > ++-
> >  drivers/scsi/sun3_scsi.c |   47 ---
> >  7 files changed, 51 insertions(+), 178 deletions(-)
> > 
> > Index: linux/drivers/scsi/Kconfig
> > ===
> > --- linux.orig/drivers/scsi/Kconfig 2015-11-18 19:25:56.0 +1100
> > +++ linux/drivers/scsi/Kconfig  2015-11-18 19:33:15.0 +1100
> > @@ -1624,23 +1624,6 @@ config ATARI_SCSI
> >   ST-DMA, replacing ACSI).  It does NOT support other schemes, like
> >   in the Hades (without DMA).
> >  
> > -config ATARI_SCSI_TOSHIBA_DELAY
> > -   bool "Long delays for Toshiba CD-ROMs"
> > -   depends on ATARI_SCSI
> > -   help
> > - This option increases the delay after a SCSI arbitration to
> > - accommodate some flaky Toshiba CD-ROM drives. Say Y if you intend to
> > - use a Toshiba CD-ROM drive; otherwise, the option is not needed and
> > - would impact performance a bit, so say N.
> > -
> > -config ATARI_SCSI_RESET_BOOT
> > -   bool "Reset SCSI-devices at boottime"
> > -   depends on ATARI_SCSI
> > -   help
> > - Reset the devices on your Atari whenever it boots.  This makes the
> > - boot process fractionally longer but may assist recovery from errors
> > - that leave the devices with SCSI operations partway completed.
> > -
> >  config MAC_SCSI
> > tristate "Macintosh NCR5380 SCSI"
> > depends on MAC && SCSI=y
> > Index: linux/drivers/scsi/atari_NCR5380.c
> > ===
> > --- linux.orig/drivers/scsi/atari_NCR5380.c 2015-11-18 19:33:13.0 
> > +1100
> > +++ linux/drivers/scsi/atari_NCR5380.c  2015-11-18 19:33:15.0 
> > +1100
> > @@ -674,13 +674,14 @@ static void prepare_info(struct Scsi_Hos
> >  "base 0x%lx, irq %d, "
> >  "can_queue %d, cmd_per_lun %d, "
> >  "sg_tablesize %d, this_id %d, "
> > -"flags { %s}, "
> > +"flags { %s%s}, "
> >  "options { %s} ",
> >  instance->hostt->name, instance->io_port, instance->n_io_port,
> >  instance->base, instance->irq,
> >  instance->can_queue, instance->cmd_per_lun,
> >  instance->sg_tablesize, instance->this_id,
> >  hostdata->flags & FLAG_TAGGED_QUEUING ? "TAGGED_QUEUING " : "",
> > +hostdata->flags & FLAG_TOSHIBA_DELAY  ? "TOSHIBA_DELAY "  : "",
> >  #ifdef DIFFERENTIAL
> >  "DIFFERENTIAL "
> >  #endif
> > @@ -860,6 +861,7 @@ static int __init NCR5380_init(struct Sc
> >  
> >  static int NCR5380_maybe_reset_bus(struct Scsi_Host *instance)
> >  {
> > +   struct NCR5380_hostdata *hostdata = shost_priv(instance);
> > int pass;
> >  
> > for (pass = 1; (NCR5380_read(STATUS_REG) & SR_BSY) && pass <= 6; 
> > ++pass) {
> > @@ -878,6 +880,14 @@ static int NCR5380_maybe_reset_bus(struc
> > case 4:
> > shost_printk(KERN_ERR, instance, "bus busy, attempting 
> > reset\n");
> > do_reset(instance);
> > +   /* Wait after a reset; the SCSI standard calls for
> > +* 250ms, we wait 500ms to be on the safe side.
> > +* But some Toshiba CD-ROMs need ten times that.
> > +*/
> > +   if (hostdata->flags & FLAG_TOSHIBA_DELAY)
> > +   

Re: [PATCH 10/71] atari_NCR5380: Remove RESET_BOOT, CONFIG_ATARI_SCSI_TOSHIBA_DELAY and CONFIG_ATARI_SCSI_RESET_BOOT

2015-11-18 Thread Michael Schmitz
Hi Finn,

Am 19.11.2015 um 17:05 schrieb Finn Thain:
> w
> On Thu, 19 Nov 2015, Michael Schmitz wrote:
> 
>> Hi Finn,
>>
>> Am 18.11.2015 um 21:35 schrieb Finn Thain:
>>
>>> The bus reset may raise an interrupt. That would be new behaviour for 
>>> atari_scsi only when CONFIG_ATARI_SCSI_RESET_BOOT=n. The ST DMA 
>>> interrupt is not assigned to atari_scsi at this stage, so 
>>> CONFIG_ATARI_SCSI_RESET_BOOT=y may well be problematic already.
>>
>> I can confirm that the bus reset at boot has never been problematic in 
>> the past. It's been enabled in my kernels as long as I've used the 
>> driver (must be getting close to 20 years now).
> 
> That's good to know. I'm not sure why it was configurable in the first 
> place (long delays?). The new algorithm (the one I copied from NCR5380.c) 

The longer delays (and possibly a reset at boot) were only necessary for
certain CD-ROM drives. I don't think I have ever seen such a device, and
it's a bit unlikely any of these still survive. Reset at boot before
proper driver init can probably go away now.

> does not allow the user to prevent a possible scsi bus reset at driver 
> init time. The scsi bus reset only takes place if the driver discovers 
> that the bus was already wedged when it started. (It proved useful when I 
> was introducing faults for EH testing, BTW.)

Much saner approach, I'm sure. Don't forget the driver was written
before sophisticated error handling came along. Reset at boot and keep
your fingers crossed was the strategy in these days.

Cheers,

Michael

--
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 for-next 05/10] iser: Have initiator and target to share protocol structures and definitions

2015-11-18 Thread Or Gerlitz

On 11/16/2015 6:37 PM, Sagi Grimberg wrote:

+/**
+ * struct iser_hello - iSER Hello header
+ *
+ * @opcode:   opcode (must be set to ISER_HELLO)
+ * @max_min_ver:  maximum and minimum iser versions
+ * @iser_ird: iSER IRD
+ * @rsvd: reserved
+ */
+struct iser_hello {
+   u8  opcode;
+   u8  max_min_ver;
+   u16 iser_ird;
+   u8  rsvd[20];
+} __packed;
+
+/**
+ * struct iser_hello_rep - iSER Hello reply header
+ *
+ * @opcode_rej:   opcode (must be set to ISER_HELLORPLY)
+ *lower bit is reject bit
+ * @max_cur_ver:  maximum and current iser versions
+ * @iser_ord: iSER ORD
+ * @rsvd: reserved
+ */
+struct iser_hello_rep {
+   u8  opcode_rej;
+   u8  max_cur_ver;
+   u16 iser_ord;
+   u8  rsvd[20];
+} __packed;
+


I don't see the point to include these two defs, we don't use them and 
Steve even got iser to work

over iwarp without them, so why care? we should only leave


+#define ISER_HELLO 0x20
+#define ISER_HELLORPLY 0x30

to allow warnings on them if we get such packets
--
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] lpfc: replaced kmalloc + memset with kzalloc

2015-11-18 Thread Saurabh Sengar
replacing kmalloc and memset by a single call of kzalloc

Signed-off-by: Saurabh Sengar 
---
v2 : I didn't got any response for my initial patch,
I am sending it again on top of latest kernel(today's)

 drivers/scsi/lpfc/lpfc_els.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_els.c b/drivers/scsi/lpfc/lpfc_els.c
index b6fa257..92dd204 100644
--- a/drivers/scsi/lpfc/lpfc_els.c
+++ b/drivers/scsi/lpfc/lpfc_els.c
@@ -4956,13 +4956,12 @@ lpfc_els_rcv_rdp(struct lpfc_vport *vport, struct 
lpfc_iocbq *cmdiocb,
if (RDP_NPORT_ID_SIZE !=
be32_to_cpu(rdp_req->nport_id_desc.length))
goto rjt_logerr;
-   rdp_context = kmalloc(sizeof(struct lpfc_rdp_context), GFP_KERNEL);
+   rdp_context = kzalloc(sizeof(struct lpfc_rdp_context), GFP_KERNEL);
if (!rdp_context) {
rjt_err = LSRJT_UNABLE_TPC;
goto error;
}
 
-   memset(rdp_context, 0, sizeof(struct lpfc_rdp_context));
cmd = &cmdiocb->iocb;
rdp_context->ndlp = lpfc_nlp_get(ndlp);
rdp_context->ox_id = cmd->unsli3.rcvsli3.ox_id;
-- 
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


Re: [PATCH 01/71] atari_scsi: Fix SCSI host ID setting

2015-11-18 Thread Hannes Reinecke

On 11/18/2015 09:34 AM, Finn Thain wrote:

The NVRAM location of this byte is 16, as documented in
http://toshyp.atari.org/en/004009.html

This was confirmed by Michael Schmitz, by setting the SCSI host ID
under EmuTOS and then checking the value in /proc/driver/nvram and
/dev/nvram under Linux.

Signed-off-by: Finn Thain 


Reviewed-by: Hannes Reinecke 

Cheers,

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

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


Re: [PATCH 03/71] ncr5380: Eliminate PDEBUG*, TDEBUG* and DTCDEBUG* macros

2015-11-18 Thread Hannes Reinecke

On 11/18/2015 09:34 AM, Finn Thain wrote:

Replace {P,T,DTC}DEBUG_INIT with NDEBUG_INIT. Remove dead debugging
code, including code that's conditional upon *DEBUG_TRANSFER.

Signed-off-by: Finn Thain 

---
  drivers/scsi/dtc.c   |   18 ++
  drivers/scsi/dtc.h   |   27 ---
  drivers/scsi/pas16.c |   21 +++--
  drivers/scsi/pas16.h |   16 
  drivers/scsi/t128.c  |   18 ++
  drivers/scsi/t128.h  |   16 
  6 files changed, 19 insertions(+), 97 deletions(-)


Reviewed-by: Hannes Reinecke 

Cheers,

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

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


Re: [PATCH 02/71] ncr5380: Remove redundant static variable initializers

2015-11-18 Thread Hannes Reinecke

On 11/18/2015 09:34 AM, Finn Thain wrote:

Signed-off-by: Finn Thain 

---
  drivers/scsi/NCR5380.c   |2 +-
  drivers/scsi/dtc.c   |4 ++--
  drivers/scsi/g_NCR5380.c |4 ++--
  drivers/scsi/pas16.c |   10 +-
  drivers/scsi/sun3_scsi.c |8 
  drivers/scsi/t128.c  |4 ++--
  6 files changed, 16 insertions(+), 16 deletions(-)


Reviewed-by: Hannes Reinecke 

Cheers,

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

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


Re: [PATCH 04/71] ncr5380: Remove more pointless macros

2015-11-18 Thread Hannes Reinecke

On 11/18/2015 09:34 AM, Finn Thain wrote:

ASM macro is never defined. rtrc in pas16.c is not used.
NCR5380_map_config, do_NCR5380_intr, do_t128_intr and do_pas16_intr
are unused. NCR_NOT_SET harms readability. Remove them.

Signed-off-by: Finn Thain 

---
  drivers/scsi/NCR5380.h   |3 ---
  drivers/scsi/g_NCR5380.c |   29 ++---
  drivers/scsi/g_NCR5380.h |5 -
  drivers/scsi/pas16.c |   16 
  drivers/scsi/pas16.h |5 -
  drivers/scsi/t128.h  |4 
  6 files changed, 14 insertions(+), 48 deletions(-)


Reviewed-by: Hannes Reinecke 

Cheers,

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

--
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 05/71] ncr5380: Remove NCR5380_local_declare and NCR5380_setup macros

2015-11-18 Thread Hannes Reinecke

On 11/18/2015 09:35 AM, Finn Thain wrote:

The NCR5380_local_declare and NCR5380_setup macros exist to define and
initialize a particular local variable, to provide the address of the
chip registers needed for the driver's implementation of its
NCR5380_read/write register access macros.

In cumana_1 and macscsi, these macros generate pointless code like this,
struct Scsi_Host *_instance;
_instance = instance;

In pas16, the use of NCR5380_read/write in pas16_hw_detect() requires that
the io_port local variable has been defined and initialized, but the
NCR5380_local_declare and NCR5380_setup macros can't be used for that
purpose because the Scsi_Host struct has not yet been instantiated.

Moreover, these macros were removed from atari_NCR5380.c long ago and
now they constitute yet another discrepancy between the two core driver
forks.

Remove these "optimizations".

Signed-off-by: Finn Thain 

---

Reviewed-by: Hannes Reinecke 

Cheers,

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

--
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 06/71] ncr5380: Remove NCR5380_instance_name macro

2015-11-18 Thread Hannes Reinecke

On 11/18/2015 09:35 AM, Finn Thain wrote:

This macro makes the code cryptic. Remove it.

Signed-off-by: Finn Thain 

---
  drivers/scsi/NCR5380.c   |2 +-
  drivers/scsi/g_NCR5380.c |7 ---
  drivers/scsi/g_NCR5380.h |2 --
  3 files changed, 5 insertions(+), 6 deletions(-)


Reviewed-by: Hannes Reinecke 

Cheers,

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

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

2015-11-18 Thread Ondrej Zary
On Thursday 19 November 2015, Finn Thain wrote:
> On Wed, 18 Nov 2015, Ondrej Zary wrote:
> > On Wednesday 18 November 2015, 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 about 750
> > > 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.
> > >
> > > I have compile-tested all patches to all NCR5380 drivers (x86, ARM,
> > > m68k) and regression tested mac_scsi and dmx3191d modules on suitable
> > > hardware. Testing the mac_scsi and dmx3191d modules provides only
> > > limited coverage. It would be good to see some testing of ISA cards
> > > and Sun 3 and Atari hardware too (I don't have any).
> >
> > I have some NCR5380 ISA cards and can test them.
>
> Thanks Ondrej. I've no idea which ISA drivers are presently working in
> mainline. Finding regressions may be more difficult than usual ;-)

I remember that at least one of them never worked in Linux - HP C2502 card 
with 53C400A chip with no jumpers (magic-numbers-based configuration).

The memory-mapped Canon FG2-5202 (53C400) did not work properly either.

At least DTCT-436P used to work.

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


<    1   2