Re: [PATCH] iscsi-target: fix panic when add the second TCP connection to iSCSI session

2016-07-11 Thread Feng Li
Add ls...@suse.com

2016-07-11 22:23 GMT+08:00 冯力 :
> This problem exists at least from v3.16.
> The upstream kernel still exists this issue.
>
> I have tested my patch and the following panic is disappeared.
>
> Thanks,
> - Alex
>
> #dmesg
>
> [ 1160.788676] sd 16:0:0:0: [sde] Attached SCSI disk
> [ 1383.962626] target_core_get_fabric() failed for usb_gadget
> [ 1404.788910] TARGET_CORE[iSCSI]: Unsupported SCSI Opcode 0x1b,
> sending CHECK_CONDITION.
> [ 1404.858451] BUG: unable to handle kernel NULL pointer dereference
> at 01f8
> [ 1404.858911] IP: []
> iscsi_target_login_thread+0x728/0x10b0 [iscsi_target_mod]
> [ 1404.859746] PGD 230c29067 PUD 232060067 PMD 0
> [ 1404.859963] Oops:  [#1] SMP
> [ 1404.860154] Modules linked in: crc32c_generic tcm_loop tcm_qla2xxx
> qla2xxx iscsi_target_mod ib_srpt vhost_scsi vhost tcm_fc libfc
> scsi_transport_fc scsi_tgt target_core_file target_core_iblock
> target_core_pscsi target_core_mod configfs nbd
> vmw_vsock_vmci_transport vsock bridge stp llc vmhgfs(O) snd_ens1371
> snd_rawmidi snd_seq_device snd_ac97_codec snd_pcm snd_timer snd
> vmw_balloon evdev soundcore ac97_bus gameport ppdev pcspkr sg psmouse
> serio_raw parport_pc vmwgfx parport battery ttm drm_kms_helper drm
> i2c_piix4 shpchp i2c_core vmw_vmci processor thermal_sys ac button
> binfmt_misc md_mod ib_iser rdma_cm iw_cm ib_cm ib_sa ib_mad ib_core
> ib_addr iscsi_tcp libiscsi_tcp libiscsi scsi_transport_iscsi
> hid_generic usbhid hid ext4 crc16 mbcache jbd2 sd_mod crc_t10dif
> crct10dif_generic crct10dif_common ata_generic
> [ 1404.861278]  mptspi scsi_transport_spi mptscsih ata_piix uhci_hcd
> ehci_pci ehci_hcd mptbase libata usbcore e1000 usb_common scsi_mod
> sunrpc dm_mirror dm_region_hash dm_log dm_mod autofs4
> [ 1404.861742] CPU: 2 PID: 1865 Comm: iscsi_np Tainted: G   O
> 3.16.0-4-amd64 #1 Debian 3.16.7-ckt25-1
> [ 1404.861868] Hardware name: VMware, Inc. VMware Virtual
> Platform/440BX Desktop Reference Platform, BIOS 6.00 07/02/2015
> [ 1404.862033] task: 880234c849a0 ti: 8800b9c2 task.ti:
> 8800b9c2
> [ 1404.862114] RIP: 0010:[]  []
> iscsi_target_login_thread+0x728/0x10b0 [iscsi_target_mod]
> [ 1404.862233] RSP: 0018:8800b9c23e60  EFLAGS: 00010246
> [ 1404.862282] RAX:  RBX: 88023125b800 RCX: 
> 8802b4d8d000
> [ 1404.862344] RDX: 002d RSI: fe01 RDI: 
> 880233ef1800
> [ 1404.862406] RBP: 8800b9651a00 R08: d5a073de R09: 
> 88023487e6c0
> [ 1404.862585] R10: ead0c2e5 R11: d039ef6a R12: 
> 88023125b854
> [ 1404.865580] R13: 88023125b908 R14: 880233ef1800 R15: 
> 880234c849a0
> [ 1404.867732] FS:  7fba67e2fb80() GS:88023e64()
> knlGS:
> [ 1404.869772] CS:  0010 DS:  ES:  CR0: 8005003b
> [ 1404.871850] CR2: 01f8 CR3: 000230ffa000 CR4: 
> 07e0
> [ 1404.874271] Stack:
> [ 1404.877492]  88023125b908 00ff880234c849a0 88023125b858
> 88023481f400
> [ 1404.880162]  81510d00 880230768000 8800b9c23fd8
> 00012f00
> [ 1404.883545]  880234c849a0 880233e9da40 88023125b800
> a05edeb0
> [ 1404.885865] Call Trace:
> [ 1404.888384]  [] ? __schedule+0x250/0x700
> [ 1404.890619]  [] ?
> iscsi_target_login_sess_out+0x240/0x240 [iscsi_target_mod]
> [ 1404.892668]  [] ? kthread+0xbd/0xe0
> [ 1404.894646]  [] ? kthread_create_on_node+0x180/0x180
> [ 1404.896553]  [] ? ret_from_fork+0x58/0x90
> [ 1404.898556]  [] ? kthread_create_on_node+0x180/0x180
> [ 1404.900597] Code: 09 00 00 48 89 ea 4c 89 f6 48 89 df e8 52 1b 00
> 00 85 c0 0f 88 82 02 00 00 0f b6 44 24 0f 4c 89 f7 88 45 09 49 8b 86
> d8 04 00 00 <4c> 8b a8 f8 01 00 00 49 8b 86 a0 04 00 00 ff 90 98 00 00
> 00 41
> [ 1404.906962] RIP  []
> iscsi_target_login_thread+0x728/0x10b0 [iscsi_target_mod]
> [ 1404.908962]  RSP 
> [ 1404.910940] CR2: 01f8
> [ 1404.917670] ---[ end trace d29d8c681ab2bdfb ]---
> [ 1419.859690] iSCSI Login timeout on Network Portal 10.182.70.69:3260
> [ 1479.832056] iSCSI Login timeout on Network Portal 10.182.70.68:3260
> [ 1479.835739] iSCSI Login negotiation failed.
> [ 1549.332632] iSCSI Login timeout on Network Portal 10.182.70.68:3260
> [ 1549.338432] iSCSI Login negotiation failed.
> [ 1564.358423] iSCSI Login timeout on Network Portal 10.182.70.68:3260
> [ 1564.362443] iSCSI Login negotiation failed.
>
> 2016-07-12 6:15 GMT+08:00 Feng Li :
>> From: Feng Li 
>>
>> In MC/S scenario, the conn->sess has been set NULL in
>> iscsi_login_non_zero_tsih_s1 when the second connection comes here,
>> then kernel panic.
>>
>> The conn->sess will be assigned in iscsi_login_non_zero_tsih_s2. So
>> we should check whether it's NULL before calling.
>>
>> Signed-off-by: Feng Li 
>> ---
>>  drivers/target/iscsi/iscsi_target_login.c | 5 +++--
>>  1 file changed, 3 insertions(+), 2 

[PATCH v2] xen_pvscsi: reclaim the ring request when the prepairing failed

2016-07-11 Thread Bin Wu
During scsi command queueing or exception handling, if prepairing
fails, we need to reclaim the failed request. Otherwise, the garbage
request will be pushed into the ring for the backend to work.

Signed-off-by: Bin Wu 
---
 drivers/scsi/xen-scsifront.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/xen-scsifront.c b/drivers/scsi/xen-scsifront.c
index 9dc8687..8646db1 100644
--- a/drivers/scsi/xen-scsifront.c
+++ b/drivers/scsi/xen-scsifront.c
@@ -184,8 +184,6 @@ static struct vscsiif_request *scsifront_pre_req(struct 
vscsifrnt_info *info)
 
ring_req = RING_GET_REQUEST(&(info->ring), ring->req_prod_pvt);
 
-   ring->req_prod_pvt++;
-
ring_req->rqid = (uint16_t)id;
 
return ring_req;
@@ -196,6 +194,8 @@ static void scsifront_do_request(struct vscsifrnt_info 
*info)
struct vscsiif_front_ring *ring = &(info->ring);
int notify;
 
+   ring->req_prod_pvt++;
+
RING_PUSH_REQUESTS_AND_CHECK_NOTIFY(ring, notify);
if (notify)
notify_remote_via_irq(info->irq);
-- 
2.3.2 (Apple Git-55)


--
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: [Xen-devel] [PATCH] xen_pvscsi: reclaim the ring request when mapping data failed

2016-07-11 Thread Bin Wu

On 2016/7/11 17:53, Juergen Gross wrote:

On 11/07/16 11:50, David Vrabel wrote:

On 11/07/16 10:33, Juergen Gross wrote:

On 11/07/16 04:51, Bin Wu wrote:

During scsi command queueing, if mapping data fails, we need to
reclaim the failed request. Otherwise, the garbage request will
be pushed into the ring for the backend to work.

Well spotted. There is another instance of this problem in
scsifront_action_handler(). Would you mind correcting this one, too?

Would it make more sense to advance req_prod_pvt only if the request has
been successfully created?

Yeah, probably as the first action in scsifront_do_request().


Juergen

ok, I will send a new patch : )



David


Signed-off-by: Bin Wu 
---
  drivers/scsi/xen-scsifront.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/drivers/scsi/xen-scsifront.c b/drivers/scsi/xen-scsifront.c
index 9dc8687..655163d 100644
--- a/drivers/scsi/xen-scsifront.c
+++ b/drivers/scsi/xen-scsifront.c
@@ -565,6 +565,7 @@ static int scsifront_queuecommand(struct Scsi_Host
*shost,
  err = map_data_for_request(info, sc, ring_req, shadow);
  if (err < 0) {
  pr_debug("%s: err %d\n", __func__, err);
+info->ring.req_prod_pvt--;
  scsifront_put_rqid(info, rqid);
  scsifront_return(info);
  spin_unlock_irqrestore(shost->host_lock, flags);


___
Xen-devel mailing list
xen-de...@lists.xen.org
https://lists.xen.org/xen-devel





.




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


Re: [PATCH v3 1/7] lib: string: add functions to case-convert strings

2016-07-11 Thread Markus Mayer
On 9 July 2016 at 08:30, Markus Mayer  wrote:
> On 9 July 2016 at 05:04, Luis de Bethencourt  wrote:
>> On 08/07/16 23:43, Markus Mayer wrote:
>>> Add a collection of generic functions to convert strings to lowercase
>>> or uppercase.
>>>
>>> Changing the case of a string (with or without copying it first) seems
>>> to be a recurring requirement in the kernel that is currently being
>>> solved by several duplicated implementations doing the same thing. This
>>> change aims at reducing this code duplication.
>>>
>>> The new functions are
>>> void strlcpytoupper(char *dst, const char *src, size_t len);
>>> void strlcpytolower(char *dst, const char *src, size_t len);
>>> void strcpytoupper(char *dst, const char *src);
>>> void strcpytolower(char *dst, const char *src);
>>> void strtoupper(char *s);
>>> void strtolower(char *s);
>>>
>>> The "str[l]cpyto*" versions of the function take a destination string
>>> and a source string as arguments. The "strlcpyto*" versions additionally
>>> take a length argument like strlcpy() itself. Lastly, the strto*
>>> functions take a single string argument and modify the passed-in string.
>>>
>>> Like strlcpy(), and unlike strncpy(), the functions guarantee NULL
>>> termination of the destination string.
>>>
>>> Signed-off-by: Markus Mayer 
>>> ---
>>>  include/linux/string.h | 40 
>>>  lib/string.c   | 38 ++
>>>  2 files changed, 78 insertions(+)
>>>
>>> diff --git a/include/linux/string.h b/include/linux/string.h
>>> index 26b6f6a..36c9d14 100644
>>> --- a/include/linux/string.h
>>> +++ b/include/linux/string.h
>>> @@ -116,6 +116,8 @@ extern void * memchr(const void *,int,__kernel_size_t);
>>>  #endif
>>>  void *memchr_inv(const void *s, int c, size_t n);
>>>  char *strreplace(char *s, char old, char new);
>>> +extern void strlcpytoupper(char *dst, const char *src, size_t len);
>>> +extern void strlcpytolower(char *dst, const char *src, size_t len);
>>>
>>>  extern void kfree_const(const void *x);
>>>
>>> @@ -169,4 +171,42 @@ static inline const char *kbasename(const char *path)
>>>   return tail ? tail + 1 : path;
>>>  }
>>>
>>> +/**
>>> + * strcpytoupper - Copy string and convert to uppercase.
>>> + * @dst: The buffer to store the result.
>>> + * @src: The string to convert to uppercase.
>>> + */
>>> +static inline void strcpytoupper(char *dst, const char *src)
>>> +{
>>> + strlcpytoupper(dst, src, -1);
>>> +}
>>> +
>>
>> Why not use SIZE_MAX instead of -1?
>
> Sure. I'll change all four of them. Thanks.

Turns out there's actually a circular dependency here. SIZE_MAX is
defined in linux/kernel.h. So, string.h would need to include
kernel.h. But kernel.h, by way of several other headers, includes
string.h.

Attempting to include kernel.h in string.h then leads to something like this:

  CHK include/config/kernel.release
  CHK include/generated/uapi/linux/version.h
  CHK include/generated/utsrelease.h
  CC  scripts/mod/devicetable-offsets.s
  CHK include/generated/timeconst.h
In file included from include/linux/printk.h:289:0,
 from include/linux/kernel.h:13,
 from include/linux/string.h:11,
 from include/uapi/linux/uuid.h:21,
 from include/linux/uuid.h:19,
 from include/linux/mod_devicetable.h:12,
 from scripts/mod/devicetable-offsets.c:2:
include/linux/dynamic_debug.h: In function ‘ddebug_dyndbg_module_param_cb’:
include/linux/dynamic_debug.h:122:2: error: implicit declaration of
function ‘strstr’ [-Werror=implicit-function-declaration]
  if (strstr(param, "dyndbg")) {
  ^
include/linux/dynamic_debug.h:122:6: warning: incompatible implicit
declaration of built-in function ‘strstr’ [enabled by default]
  if (strstr(param, "dyndbg")) {
  ^
Since kernel.h is referencing string.h (which is needed, but not
included a second time due to the include guards), this leads to
undeclared string functions, because we are still in the early stages
of including string.h itself and haven't gotten to the function
declarations yet.

>>> +/**
>>> + * strcpytolower - Copy string and convert to lowercase.
>>> + * @dst: The buffer to store the result.
>>> + * @src: The string to convert to lowercase.
>>> + */
>>> +static inline void strcpytolower(char *dst, const char *src)
>>> +{
>>> + strlcpytolower(dst, src, -1);
>>> +}
>>> +
>>
>> Same here, and the 2 below :)
>>
>> Thanks Markus,
>> Luis
>>
>>> +/**
>>> + * strtoupper - Convert string to uppercase.
>>> + * @s: The string to operate on.
>>> + */
>>> +static inline void strtoupper(char *s)
>>> +{
>>> + strlcpytoupper(s, s, -1);
>>> +}
>>> +
>>> +/**
>>> + * strtolower - Convert string to lowercase.
>>> + * @s: The string to operate on.
>>> + */
>>> +static inline void strtolower(char *s)
>>> +{
>>> + strlcpytolower(s, s, 

RE: [net-next 0/6] common library for Chelsio drivers

2016-07-11 Thread Steve Wise
> > Hi,
> >
> >  This patch series adds common library module(libcxgb.ko)
> > for Chelsio drivers to remove duplicate code.
> >
> > This series moves common iSCSI DDP Page Pod manager
> > code from cxgb4.ko to libcxgb.ko, earlier this code
> > was used by only cxgbit.ko now it is used by
> > three Chelsio iSCSI drivers cxgb3i, cxgb4i, cxgbit.
> >
> > In future this module will have common connection
> > management and hardware specific code that can
> > be shared by multiple Chelsio drivers(cxgb4,
> > csiostor, iw_cxgb4, cxgb4i, cxgbit).
> >
> > Please review.
> 
> As currently implemented the user is prompted for the Kconfig symbol
> for the library.  That really needs to be hidden from the user and
> they should be able to select these drivers without having to know
> about this implementation detail at all.

Maybe adding something like:

select CHELSIO_LIB

to the Kconfig files for the drivers that are depenedent on CHELSIO_LIB?


--
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: [net-next 0/6] common library for Chelsio drivers

2016-07-11 Thread David Miller
From: Varun Prakash 
Date: Fri,  8 Jul 2016 23:03:53 +0530

> Hi,
> 
>  This patch series adds common library module(libcxgb.ko)
> for Chelsio drivers to remove duplicate code.
> 
> This series moves common iSCSI DDP Page Pod manager
> code from cxgb4.ko to libcxgb.ko, earlier this code
> was used by only cxgbit.ko now it is used by
> three Chelsio iSCSI drivers cxgb3i, cxgb4i, cxgbit.
> 
> In future this module will have common connection
> management and hardware specific code that can
> be shared by multiple Chelsio drivers(cxgb4,
> csiostor, iw_cxgb4, cxgb4i, cxgbit).
> 
> Please review.

As currently implemented the user is prompted for the Kconfig symbol
for the library.  That really needs to be hidden from the user and
they should be able to select these drivers without having to know
about this implementation detail at all.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] scsi: introduce a quirk for false cache reporting

2016-07-11 Thread Alan Stern
On Mon, 11 Jul 2016, Oliver Neukum wrote:

> Some SATA to USB bridges fail to cooperate with some
> drives resulting in no cache being present being reported
> to the host. That causes the host to skip sending
> a command to synchronize caches. That causes data loss
> when the drive is powered down.

> @@ -581,6 +582,9 @@ void usb_stor_adjust_quirks(struct usb_device *udev, 
> unsigned long *fflags)
>   case 'w':
>   f |= US_FL_NO_WP_DETECT;
>   break;
> + case 'y':
> + f |= US_FL_ALWAYS_SYNC;
> + break;
>   /* Ignore unrecognized flag characters */
>   }
>   }

You need to update Documentation/kernel-parameters.txt too.

Alan Stern

--
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] iscsi-target: fix panic when add the second TCP connection to iSCSI session

2016-07-11 Thread 冯力
This problem exists at least from v3.16.
The upstream kernel still exists this issue.

I have tested my patch and the following panic is disappeared.

Thanks,
- Alex

#dmesg

[ 1160.788676] sd 16:0:0:0: [sde] Attached SCSI disk
[ 1383.962626] target_core_get_fabric() failed for usb_gadget
[ 1404.788910] TARGET_CORE[iSCSI]: Unsupported SCSI Opcode 0x1b,
sending CHECK_CONDITION.
[ 1404.858451] BUG: unable to handle kernel NULL pointer dereference
at 01f8
[ 1404.858911] IP: []
iscsi_target_login_thread+0x728/0x10b0 [iscsi_target_mod]
[ 1404.859746] PGD 230c29067 PUD 232060067 PMD 0
[ 1404.859963] Oops:  [#1] SMP
[ 1404.860154] Modules linked in: crc32c_generic tcm_loop tcm_qla2xxx
qla2xxx iscsi_target_mod ib_srpt vhost_scsi vhost tcm_fc libfc
scsi_transport_fc scsi_tgt target_core_file target_core_iblock
target_core_pscsi target_core_mod configfs nbd
vmw_vsock_vmci_transport vsock bridge stp llc vmhgfs(O) snd_ens1371
snd_rawmidi snd_seq_device snd_ac97_codec snd_pcm snd_timer snd
vmw_balloon evdev soundcore ac97_bus gameport ppdev pcspkr sg psmouse
serio_raw parport_pc vmwgfx parport battery ttm drm_kms_helper drm
i2c_piix4 shpchp i2c_core vmw_vmci processor thermal_sys ac button
binfmt_misc md_mod ib_iser rdma_cm iw_cm ib_cm ib_sa ib_mad ib_core
ib_addr iscsi_tcp libiscsi_tcp libiscsi scsi_transport_iscsi
hid_generic usbhid hid ext4 crc16 mbcache jbd2 sd_mod crc_t10dif
crct10dif_generic crct10dif_common ata_generic
[ 1404.861278]  mptspi scsi_transport_spi mptscsih ata_piix uhci_hcd
ehci_pci ehci_hcd mptbase libata usbcore e1000 usb_common scsi_mod
sunrpc dm_mirror dm_region_hash dm_log dm_mod autofs4
[ 1404.861742] CPU: 2 PID: 1865 Comm: iscsi_np Tainted: G   O
3.16.0-4-amd64 #1 Debian 3.16.7-ckt25-1
[ 1404.861868] Hardware name: VMware, Inc. VMware Virtual
Platform/440BX Desktop Reference Platform, BIOS 6.00 07/02/2015
[ 1404.862033] task: 880234c849a0 ti: 8800b9c2 task.ti:
8800b9c2
[ 1404.862114] RIP: 0010:[]  []
iscsi_target_login_thread+0x728/0x10b0 [iscsi_target_mod]
[ 1404.862233] RSP: 0018:8800b9c23e60  EFLAGS: 00010246
[ 1404.862282] RAX:  RBX: 88023125b800 RCX: 8802b4d8d000
[ 1404.862344] RDX: 002d RSI: fe01 RDI: 880233ef1800
[ 1404.862406] RBP: 8800b9651a00 R08: d5a073de R09: 88023487e6c0
[ 1404.862585] R10: ead0c2e5 R11: d039ef6a R12: 88023125b854
[ 1404.865580] R13: 88023125b908 R14: 880233ef1800 R15: 880234c849a0
[ 1404.867732] FS:  7fba67e2fb80() GS:88023e64()
knlGS:
[ 1404.869772] CS:  0010 DS:  ES:  CR0: 8005003b
[ 1404.871850] CR2: 01f8 CR3: 000230ffa000 CR4: 07e0
[ 1404.874271] Stack:
[ 1404.877492]  88023125b908 00ff880234c849a0 88023125b858
88023481f400
[ 1404.880162]  81510d00 880230768000 8800b9c23fd8
00012f00
[ 1404.883545]  880234c849a0 880233e9da40 88023125b800
a05edeb0
[ 1404.885865] Call Trace:
[ 1404.888384]  [] ? __schedule+0x250/0x700
[ 1404.890619]  [] ?
iscsi_target_login_sess_out+0x240/0x240 [iscsi_target_mod]
[ 1404.892668]  [] ? kthread+0xbd/0xe0
[ 1404.894646]  [] ? kthread_create_on_node+0x180/0x180
[ 1404.896553]  [] ? ret_from_fork+0x58/0x90
[ 1404.898556]  [] ? kthread_create_on_node+0x180/0x180
[ 1404.900597] Code: 09 00 00 48 89 ea 4c 89 f6 48 89 df e8 52 1b 00
00 85 c0 0f 88 82 02 00 00 0f b6 44 24 0f 4c 89 f7 88 45 09 49 8b 86
d8 04 00 00 <4c> 8b a8 f8 01 00 00 49 8b 86 a0 04 00 00 ff 90 98 00 00
00 41
[ 1404.906962] RIP  []
iscsi_target_login_thread+0x728/0x10b0 [iscsi_target_mod]
[ 1404.908962]  RSP 
[ 1404.910940] CR2: 01f8
[ 1404.917670] ---[ end trace d29d8c681ab2bdfb ]---
[ 1419.859690] iSCSI Login timeout on Network Portal 10.182.70.69:3260
[ 1479.832056] iSCSI Login timeout on Network Portal 10.182.70.68:3260
[ 1479.835739] iSCSI Login negotiation failed.
[ 1549.332632] iSCSI Login timeout on Network Portal 10.182.70.68:3260
[ 1549.338432] iSCSI Login negotiation failed.
[ 1564.358423] iSCSI Login timeout on Network Portal 10.182.70.68:3260
[ 1564.362443] iSCSI Login negotiation failed.

2016-07-12 6:15 GMT+08:00 Feng Li :
> From: Feng Li 
>
> In MC/S scenario, the conn->sess has been set NULL in
> iscsi_login_non_zero_tsih_s1 when the second connection comes here,
> then kernel panic.
>
> The conn->sess will be assigned in iscsi_login_non_zero_tsih_s2. So
> we should check whether it's NULL before calling.
>
> Signed-off-by: Feng Li 
> ---
>  drivers/target/iscsi/iscsi_target_login.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/target/iscsi/iscsi_target_login.c 
> b/drivers/target/iscsi/iscsi_target_login.c
> index b5212f0..adf419f 100644
> --- a/drivers/target/iscsi/iscsi_target_login.c
> +++ b/drivers/target/iscsi/iscsi_target_login.c
> @@ -1371,8 +1371,9 @@ 

Re: [PATCH] scsi: remove current_cmnd field from struct scsi_device

2016-07-11 Thread Ewan D. Milne
On Mon, 2016-07-11 at 13:34 +0900, Christoph Hellwig wrote:
> The field is only used by the 53c700 driver, so move it into the
> driver-private device data instead of having it in the common structure.
> 
> Signed-off-by: Christoph Hellwig 
> ---
>  drivers/scsi/53c700.c  | 8 +---
>  drivers/scsi/53c700.h  | 1 +
>  include/scsi/scsi_device.h | 1 -
>  3 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/scsi/53c700.c b/drivers/scsi/53c700.c
> index 5556201..95e32a4 100644
> --- a/drivers/scsi/53c700.c
> +++ b/drivers/scsi/53c700.c
> @@ -1120,9 +1120,9 @@ process_script_interrupt(__u32 dsps, __u32 dsp, struct 
> scsi_cmnd *SCp,
>   "reselection is tag %d, slot %p(%d)\n",
>   hostdata->msgin[2], slot, slot->tag);
>   } else {
> - struct scsi_cmnd *SCp;
> + struct NCR_700_Device_Parameters *p = SDp->hostdata;
> + struct scsi_cmnd *SCp = p->current_cmnd;
>  
> - SCp = SDp->current_cmnd;
>   if(unlikely(SCp == NULL)) {
>   sdev_printk(KERN_ERR, SDp,
>   "no saved request for untagged cmd\n");
> @@ -1825,9 +1825,11 @@ NCR_700_queuecommand_lck(struct scsi_cmnd *SCp, void 
> (*done)(struct scsi_cmnd *)
>   CDEBUG(KERN_DEBUG, SCp, "sending out tag %d, slot %p\n",
>  slot->tag, slot);
>   } else {
> + struct NCR_700_Device_Parameters *p = SCp->device->hostdata;
> +
>   slot->tag = SCSI_NO_TAG;
>   /* save current command for reselection */
> - SCp->device->current_cmnd = SCp;
> + p->current_cmnd = SCp;
>   }
>   /* sanity check: some of the commands generated by the mid-layer
>* have an eccentric idea of their sc_data_direction */
> diff --git a/drivers/scsi/53c700.h b/drivers/scsi/53c700.h
> index c893a5d..f34c916 100644
> --- a/drivers/scsi/53c700.h
> +++ b/drivers/scsi/53c700.h
> @@ -82,6 +82,7 @@ struct NCR_700_Device_Parameters {
>* cmnd[1], this could be in static storage */
>   unsigned char cmnd[MAX_COMMAND_SIZE];
>   __u8depth;
> + struct scsi_cmnd *current_cmnd; /* currently active command */
>  };
>  
> 
> diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
> index a6c346d..8a95631 100644
> --- a/include/scsi/scsi_device.h
> +++ b/include/scsi/scsi_device.h
> @@ -94,7 +94,6 @@ struct scsi_device {
>   spinlock_t list_lock;
>   struct list_head cmd_list;  /* queue of in use SCSI Command 
> structures */
>   struct list_head starved_entry;
> - struct scsi_cmnd *current_cmnd; /* currently active command */
>   unsigned short queue_depth; /* How deep of a queue we want */
>   unsigned short max_queue_depth; /* max queue depth */
>   unsigned short last_queue_full_depth; /* These two are used by */

Reviewed-by: Ewan D. Milne 


--
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] iscsi-target: fix panic when add the second TCP connection to iSCSI session

2016-07-11 Thread Feng Li
From: Feng Li 

In MC/S scenario, the conn->sess has been set NULL in
iscsi_login_non_zero_tsih_s1 when the second connection comes here,
then kernel panic.

The conn->sess will be assigned in iscsi_login_non_zero_tsih_s2. So
we should check whether it's NULL before calling.

Signed-off-by: Feng Li 
---
 drivers/target/iscsi/iscsi_target_login.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/target/iscsi/iscsi_target_login.c 
b/drivers/target/iscsi/iscsi_target_login.c
index b5212f0..adf419f 100644
--- a/drivers/target/iscsi/iscsi_target_login.c
+++ b/drivers/target/iscsi/iscsi_target_login.c
@@ -1371,8 +1371,9 @@ static int __iscsi_target_login_thread(struct iscsi_np 
*np)
}
login->zero_tsih = zero_tsih;
 
-   conn->sess->se_sess->sup_prot_ops =
-   conn->conn_transport->iscsit_get_sup_prot_ops(conn);
+   if (conn->sess)
+   conn->sess->se_sess->sup_prot_ops =
+   conn->conn_transport->iscsit_get_sup_prot_ops(conn);
 
tpg = conn->tpg;
if (!tpg) {
-- 
2.9.0

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


[PATCH] scsi: introduce a quirk for false cache reporting

2016-07-11 Thread Oliver Neukum
Some SATA to USB bridges fail to cooperate with some
drives resulting in no cache being present being reported
to the host. That causes the host to skip sending
a command to synchronize caches. That causes data loss
when the drive is powered down.

Signed-off-by: Oliver Neukum 
---
 drivers/scsi/sd.c  | 6 +++---
 drivers/usb/storage/scsiglue.c | 4 
 drivers/usb/storage/usb.c  | 6 +-
 include/linux/usb_usual.h  | 2 ++
 include/scsi/scsi_device.h | 1 +
 5 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 60bff78..3e8a6f1 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -139,7 +139,7 @@ static void sd_set_flush_flag(struct scsi_disk *sdkp)
 {
bool wc = false, fua = false;
 
-   if (sdkp->WCE) {
+   if (sdkp->WCE || sdkp->device->always_sync) {
wc = true;
if (sdkp->DPOFUA)
fua = true;
@@ -3228,7 +3228,7 @@ static void sd_shutdown(struct device *dev)
if (pm_runtime_suspended(dev))
return;
 
-   if (sdkp->WCE && sdkp->media_present) {
+   if ((sdkp->WCE || sdkp->device->always_sync) && sdkp->media_present)  {
sd_printk(KERN_NOTICE, sdkp, "Synchronizing SCSI cache\n");
sd_sync_cache(sdkp);
}
@@ -3247,7 +3247,7 @@ static int sd_suspend_common(struct device *dev, bool 
ignore_stop_errors)
if (!sdkp)  /* E.g.: runtime suspend following sd_remove() */
return 0;
 
-   if (sdkp->WCE && sdkp->media_present) {
+   if ((sdkp->WCE || sdkp->device->always_sync) && sdkp->media_present) {
sd_printk(KERN_NOTICE, sdkp, "Synchronizing SCSI cache\n");
ret = sd_sync_cache(sdkp);
if (ret) {
diff --git a/drivers/usb/storage/scsiglue.c b/drivers/usb/storage/scsiglue.c
index 33eb923..43e76ae 100644
--- a/drivers/usb/storage/scsiglue.c
+++ b/drivers/usb/storage/scsiglue.c
@@ -296,6 +296,10 @@ static int slave_configure(struct scsi_device *sdev)
if (us->fflags & US_FL_BROKEN_FUA)
sdev->broken_fua = 1;
 
+   /* Some even totally fail to indicate a cache */
+   if (us->fflags & US_FL_ALWAYS_SYNC)
+   sdev->always_sync = 1;
+
} else {
 
/*
diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c
index ef2d8cd..19255f1 100644
--- a/drivers/usb/storage/usb.c
+++ b/drivers/usb/storage/usb.c
@@ -498,7 +498,8 @@ void usb_stor_adjust_quirks(struct usb_device *udev, 
unsigned long *fflags)
US_FL_NO_READ_DISC_INFO | US_FL_NO_READ_CAPACITY_16 |
US_FL_INITIAL_READ10 | US_FL_WRITE_CACHE |
US_FL_NO_ATA_1X | US_FL_NO_REPORT_OPCODES |
-   US_FL_MAX_SECTORS_240 | US_FL_NO_REPORT_LUNS);
+   US_FL_MAX_SECTORS_240 | US_FL_NO_REPORT_LUNS |
+   US_FL_ALWAYS_SYNC);
 
p = quirks;
while (*p) {
@@ -581,6 +582,9 @@ void usb_stor_adjust_quirks(struct usb_device *udev, 
unsigned long *fflags)
case 'w':
f |= US_FL_NO_WP_DETECT;
break;
+   case 'y':
+   f |= US_FL_ALWAYS_SYNC;
+   break;
/* Ignore unrecognized flag characters */
}
}
diff --git a/include/linux/usb_usual.h b/include/linux/usb_usual.h
index 245f57d..0aae1b2 100644
--- a/include/linux/usb_usual.h
+++ b/include/linux/usb_usual.h
@@ -81,6 +81,8 @@
/* Sets max_sectors to 240 */   \
US_FLAG(NO_REPORT_LUNS, 0x1000) \
/* Cannot handle REPORT_LUNS */ \
+   US_FLAG(ALWAYS_SYNC, 0x2000)\
+   /* lies about caching, so always sync */\
 
 #define US_FLAG(name, value)   US_FL_##name = value ,
 enum { US_DO_ALL_FLAGS };
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index a6c346d..392d166 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -179,6 +179,7 @@ struct scsi_device {
unsigned try_rc_10_first:1; /* Try READ_CAPACACITY_10 first */
unsigned is_visible:1;  /* is the device visible in sysfs */
unsigned wce_default_on:1;  /* Cache is ON by default */
+   unsigned always_sync:1; /* synchronize cache in every case*/
unsigned no_dif:1;  /* T10 PI (DIF) should be disabled */
unsigned broken_fua:1;  /* Don't set FUA bit */
unsigned lun_in_cdb:1;  /* Store LUN bits in CDB[1] */
-- 
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] scsi: remove current_cmnd field from struct scsi_device

2016-07-11 Thread Tomas Henzl
On 11.7.2016 06:34, Christoph Hellwig wrote:
> The field is only used by the 53c700 driver, so move it into the
> driver-private device data instead of having it in the common structure.
>
> Signed-off-by: Christoph Hellwig 

Reviewed-by: Tomas Henzl 

Tomas

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


2% DARLEHEN

2016-07-11 Thread David Rogers
Aufmerksamkeit;

Hiermit teilen wir Ihnen mit, dass als privates Unternehmen, Kredite 
Unternehmen mit Sitz in Großbritannien (Um Financial Ltd). Wir geben Kredite in 
Höhe von $ 10.000 bis $ 300.000.000 bei 2% Zinssatz für alle Interessierten. 
Land ist kein Hindernis, so fühlen sich frei, mit uns an diese E-Mail zu 
kontaktieren: customer.serv...@umafh.co.uk
--
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: [Xen-devel] [PATCH] xen_pvscsi: reclaim the ring request when mapping data failed

2016-07-11 Thread Juergen Gross
On 11/07/16 11:50, David Vrabel wrote:
> On 11/07/16 10:33, Juergen Gross wrote:
>> On 11/07/16 04:51, Bin Wu wrote:
>>> During scsi command queueing, if mapping data fails, we need to
>>> reclaim the failed request. Otherwise, the garbage request will
>>> be pushed into the ring for the backend to work.
>>
>> Well spotted. There is another instance of this problem in
>> scsifront_action_handler(). Would you mind correcting this one, too?
> 
> Would it make more sense to advance req_prod_pvt only if the request has
> been successfully created?

Yeah, probably as the first action in scsifront_do_request().


Juergen

> 
> David
> 
>>> Signed-off-by: Bin Wu 
>>> ---
>>>  drivers/scsi/xen-scsifront.c | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/scsi/xen-scsifront.c b/drivers/scsi/xen-scsifront.c
>>> index 9dc8687..655163d 100644
>>> --- a/drivers/scsi/xen-scsifront.c
>>> +++ b/drivers/scsi/xen-scsifront.c
>>> @@ -565,6 +565,7 @@ static int scsifront_queuecommand(struct Scsi_Host
>>> *shost,
>>>  err = map_data_for_request(info, sc, ring_req, shadow);
>>>  if (err < 0) {
>>>  pr_debug("%s: err %d\n", __func__, err);
>>> +info->ring.req_prod_pvt--;
>>>  scsifront_put_rqid(info, rqid);
>>>  scsifront_return(info);
>>>  spin_unlock_irqrestore(shost->host_lock, flags);
>>
>>
>> ___
>> Xen-devel mailing list
>> xen-de...@lists.xen.org
>> https://lists.xen.org/xen-devel
>>
> 
> 

--
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: [Xen-devel] [PATCH] xen_pvscsi: reclaim the ring request when mapping data failed

2016-07-11 Thread David Vrabel
On 11/07/16 10:33, Juergen Gross wrote:
> On 11/07/16 04:51, Bin Wu wrote:
>> During scsi command queueing, if mapping data fails, we need to
>> reclaim the failed request. Otherwise, the garbage request will
>> be pushed into the ring for the backend to work.
> 
> Well spotted. There is another instance of this problem in
> scsifront_action_handler(). Would you mind correcting this one, too?

Would it make more sense to advance req_prod_pvt only if the request has
been successfully created?

David

>> Signed-off-by: Bin Wu 
>> ---
>>  drivers/scsi/xen-scsifront.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/scsi/xen-scsifront.c b/drivers/scsi/xen-scsifront.c
>> index 9dc8687..655163d 100644
>> --- a/drivers/scsi/xen-scsifront.c
>> +++ b/drivers/scsi/xen-scsifront.c
>> @@ -565,6 +565,7 @@ static int scsifront_queuecommand(struct Scsi_Host
>> *shost,
>>  err = map_data_for_request(info, sc, ring_req, shadow);
>>  if (err < 0) {
>>  pr_debug("%s: err %d\n", __func__, err);
>> +info->ring.req_prod_pvt--;
>>  scsifront_put_rqid(info, rqid);
>>  scsifront_return(info);
>>  spin_unlock_irqrestore(shost->host_lock, flags);
> 
> 
> ___
> Xen-devel mailing list
> xen-de...@lists.xen.org
> https://lists.xen.org/xen-devel
> 

--
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] xen_pvscsi: reclaim the ring request when mapping data failed

2016-07-11 Thread Juergen Gross
On 11/07/16 04:51, Bin Wu wrote:
> During scsi command queueing, if mapping data fails, we need to
> reclaim the failed request. Otherwise, the garbage request will
> be pushed into the ring for the backend to work.

Well spotted. There is another instance of this problem in
scsifront_action_handler(). Would you mind correcting this one, too?


Juergen

> 
> Signed-off-by: Bin Wu 
> ---
>  drivers/scsi/xen-scsifront.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/scsi/xen-scsifront.c b/drivers/scsi/xen-scsifront.c
> index 9dc8687..655163d 100644
> --- a/drivers/scsi/xen-scsifront.c
> +++ b/drivers/scsi/xen-scsifront.c
> @@ -565,6 +565,7 @@ static int scsifront_queuecommand(struct Scsi_Host
> *shost,
>  err = map_data_for_request(info, sc, ring_req, shadow);
>  if (err < 0) {
>  pr_debug("%s: err %d\n", __func__, err);
> +info->ring.req_prod_pvt--;
>  scsifront_put_rqid(info, rqid);
>  scsifront_return(info);
>  spin_unlock_irqrestore(shost->host_lock, flags);

--
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] qla2xxx: Fix NULL pointer deref in QLA interrupt

2016-07-11 Thread Bruno Prémont
On Mon, 11 Jul 2016 09:30:30 +0200 Thorsten Leemhuis wrote:
> Bruno Prémont wrote on 11.07.2016 09:17:
> > On Fri, 8 Jul 2016 09:27:18 +0200 Thorsten Leemhuis wrote:  
> >> Bruno Prémont wrote on 30.06.2016 17:00:  
> >> > In qla24xx_process_response_queue() rsp->msix->cpuid may trigger NULL
> >> > pointer dereference when rsp->msix is NULL:
> >> > […]
> >> > The affected code was introduced by commit 
> >> > cdb898c52d1dfad4b4800b83a58b3fe5d352edde
> >> > (qla2xxx: Add irq affinity notification).
> >> > 
> >> > Only dereference rsp->msix when it has been set so the machine can boot
> >> > fine. Possibly rsp->msix is unset because:
> >> > [3.479679] qla2xxx [:00:00.0]-0005: : QLogic Fibre Channel HBA 
> >> > Driver: 8.07.00.33-k.
> >> > [3.481839] qla2xxx [:13:00.0]-001d: : Found an ISP2432 irq 17 
> >> > iobase 0xc9038000.
> >> > [3.484081] qla2xxx [:13:00.0]-0035:0: MSI-X; Unsupported ISP2432 
> >> > (0x2, 0x3).
> >> > [3.485804] qla2xxx [:13:00.0]-0037:0: Falling back-to MSI mode 
> >> > -258.
> >> > [3.890145] scsi host0: qla2xxx
> >> > [3.891956] qla2xxx [:13:00.0]-00fb:0: QLogic QLE2460 - 
> >> > PCI-Express Single Channel 4Gb Fibre Channel HBA.
> >> > [3.894207] qla2xxx [:13:00.0]-00fc:0: ISP2432: PCIe (2.5GT/s x4) 
> >> > @ :13:00.0 hdma+ host#=0 fw=7.03.00 (9496).
> >> > [5.714774] qla2xxx [:13:00.0]-500a:0: LOOP UP detected (4 Gbps). 
> >> >
> >> 
> >> Bruno: Does that mean you actually tested that patch and it fixed the
> >> problem for you? It looks like it, but there is some confusion about it;
> >> that's one of the reasons why this patch didn't get any further yet
> >> afaics, so a quick clarification might help to finally get this fixed
> >> properly in mainline and stable.  
> > Yes, it does fix the Oops for me.  
> 
> Thx for the feedback. The patch hit mainline late last week (it's
> included in rc7) and should hopefully make it to the stable trees in a
> week or two.

I got the queued notification from James last week and kept an eye
at the state on patchwork before that.

> > I did not analyze the reason why rsp->msix is NULL (no idea if
> > it remains NULL forever on my hardware) - I just extracted messages
> > from qla driver shown during boot which seem to indicate a possible
> > reason why msix is NULL.
> > Further analysis should be done by someone with better knowledge of qla
> > driver than mine though I would be happy to perform tests.  
> 
> I have no idea about the details, but in case you missed it, this
> discussion might have some more relevant details:
> http://thread.gmane.org/gmane.linux.kernel/2247804/focus=2250727

I didn't see that thread, though it does have some insight.
Thanks for the reference!

Bruno

> Cheers, Thorsten
--
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


2% lånetilbud

2016-07-11 Thread David Rogers
oppmerksomhet;

Dette er for å informere deg gjorde som et privat selskap, Utlån selskap basert 
i Storbritannia (til økonomisk Ltd). Vi gir ut lån til melodi av $ 10 000 til $ 
300 millioner ved 2% rente til alle som er interessert. Country er ikke en 
barriere, så ta gjerne kontakt med oss på denne e-posten: 
customer.serv...@umafh.co.uk
--
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] qla2xxx: Fix NULL pointer deref in QLA interrupt

2016-07-11 Thread Thorsten Leemhuis
Bruno Prémont wrote on 11.07.2016 09:17:
> On Fri, 8 Jul 2016 09:27:18 +0200 Thorsten Leemhuis wrote:
>> Bruno Prémont wrote on 30.06.2016 17:00:
>> > In qla24xx_process_response_queue() rsp->msix->cpuid may trigger NULL
>> > pointer dereference when rsp->msix is NULL:
>> > […]
>> > The affected code was introduced by commit 
>> > cdb898c52d1dfad4b4800b83a58b3fe5d352edde
>> > (qla2xxx: Add irq affinity notification).
>> > 
>> > Only dereference rsp->msix when it has been set so the machine can boot
>> > fine. Possibly rsp->msix is unset because:
>> > [3.479679] qla2xxx [:00:00.0]-0005: : QLogic Fibre Channel HBA 
>> > Driver: 8.07.00.33-k.
>> > [3.481839] qla2xxx [:13:00.0]-001d: : Found an ISP2432 irq 17 
>> > iobase 0xc9038000.
>> > [3.484081] qla2xxx [:13:00.0]-0035:0: MSI-X; Unsupported ISP2432 
>> > (0x2, 0x3).
>> > [3.485804] qla2xxx [:13:00.0]-0037:0: Falling back-to MSI mode 
>> > -258.
>> > [3.890145] scsi host0: qla2xxx
>> > [3.891956] qla2xxx [:13:00.0]-00fb:0: QLogic QLE2460 - PCI-Express 
>> > Single Channel 4Gb Fibre Channel HBA.
>> > [3.894207] qla2xxx [:13:00.0]-00fc:0: ISP2432: PCIe (2.5GT/s x4) @ 
>> > :13:00.0 hdma+ host#=0 fw=7.03.00 (9496).
>> > [5.714774] qla2xxx [:13:00.0]-500a:0: LOOP UP detected (4 Gbps).  
>> 
>> Bruno: Does that mean you actually tested that patch and it fixed the
>> problem for you? It looks like it, but there is some confusion about it;
>> that's one of the reasons why this patch didn't get any further yet
>> afaics, so a quick clarification might help to finally get this fixed
>> properly in mainline and stable.
> Yes, it does fix the Oops for me.

Thx for the feedback. The patch hit mainline late last week (it's
included in rc7) and should hopefully make it to the stable trees in a
week or two.

> I did not analyze the reason why rsp->msix is NULL (no idea if
> it remains NULL forever on my hardware) - I just extracted messages
> from qla driver shown during boot which seem to indicate a possible
> reason why msix is NULL.
> Further analysis should be done by someone with better knowledge of qla
> driver than mine though I would be happy to perform tests.

I have no idea about the details, but in case you missed it, this
discussion might have some more relevant details:
http://thread.gmane.org/gmane.linux.kernel/2247804/focus=2250727

Cheers, Thorsten
--
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] qla2xxx: Fix NULL pointer deref in QLA interrupt

2016-07-11 Thread Bruno Prémont
On Fri, 8 Jul 2016 09:27:18 +0200 Thorsten Leemhuis wrote:
> Bruno Prémont wrote on 30.06.2016 17:00:
> > In qla24xx_process_response_queue() rsp->msix->cpuid may trigger NULL
> > pointer dereference when rsp->msix is NULL:
> > […]
> > The affected code was introduced by commit 
> > cdb898c52d1dfad4b4800b83a58b3fe5d352edde
> > (qla2xxx: Add irq affinity notification).
> > 
> > Only dereference rsp->msix when it has been set so the machine can boot
> > fine. Possibly rsp->msix is unset because:
> > [3.479679] qla2xxx [:00:00.0]-0005: : QLogic Fibre Channel HBA 
> > Driver: 8.07.00.33-k.
> > [3.481839] qla2xxx [:13:00.0]-001d: : Found an ISP2432 irq 17 
> > iobase 0xc9038000.
> > [3.484081] qla2xxx [:13:00.0]-0035:0: MSI-X; Unsupported ISP2432 
> > (0x2, 0x3).
> > [3.485804] qla2xxx [:13:00.0]-0037:0: Falling back-to MSI mode -258.
> > [3.890145] scsi host0: qla2xxx
> > [3.891956] qla2xxx [:13:00.0]-00fb:0: QLogic QLE2460 - PCI-Express 
> > Single Channel 4Gb Fibre Channel HBA.
> > [3.894207] qla2xxx [:13:00.0]-00fc:0: ISP2432: PCIe (2.5GT/s x4) @ 
> > :13:00.0 hdma+ host#=0 fw=7.03.00 (9496).
> > [5.714774] qla2xxx [:13:00.0]-500a:0: LOOP UP detected (4 Gbps).  
> 
> Bruno: Does that mean you actually tested that patch and it fixed the
> problem for you? It looks like it, but there is some confusion about it;
> that's one of the reasons why this patch didn't get any further yet
> afaics, so a quick clarification might help to finally get this fixed
> properly in mainline and stable.

Yes, it does fix the Oops for me.

I did not analyze the reason why rsp->msix is NULL (no idea if
it remains NULL forever on my hardware) - I just extracted messages
from qla driver shown during boot which seem to indicate a possible
reason why msix is NULL.
Further analysis should be done by someone with better knowledge of qla
driver than mine though I would be happy to perform tests.

Bruno


> Himanshu: While at it: Can you confirm this patch should get merged to
> mainline? Seems Quinn is on PTO and his out-of-office reply mentioned
> you as one point of contact.
> 
> Cheers, your regression tracker for Linux 4.7
>  Thorsten
> 
> > CC: 
> > Signed-off-by: Bruno Prémont 
> > ---
> > diff --git a/drivers/scsi/qla2xxx/qla_isr.c
> > b/drivers/scsi/qla2xxx/qla_isr.c index 5649c20..a92a62d 100644
> > --- a/drivers/scsi/qla2xxx/qla_isr.c
> > +++ b/drivers/scsi/qla2xxx/qla_isr.c
> > @@ -2548,7 +2548,7 @@ void qla24xx_process_response_queue(struct
> > scsi_qla_host *vha, if (!vha->flags.online)
> > return;
> >  
> > -   if (rsp->msix->cpuid != smp_processor_id()) {
> > +   if (rsp->msix && rsp->msix->cpuid != smp_processor_id()) {
> > /* if kernel does not notify qla of IRQ's CPU change,
> >  * then set it here.
> >  */
> > 
> > http://news.gmane.org/find-root.php?message_id=20160630170032.6dbaf496%40pluto.restena.lu
> >  
> > http://mid.gmane.org/20160630170032.6dbaf496%40pluto.restena.lu
> >   
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC] libata-scsi: introducing SANITIZE translation

2016-07-11 Thread Tom Yan
I don't suppose there would be any problem doing it in userspace /
with ATA PASS-THROUGH anyway. I just couldn't agree that it would be
the reason not to implement the translation (which covers the core
part of the feature set) in the kernel. But certainly I wouldn't keep
aruging on this.

I don't think we would really want to patch sg_sanitize the way you
proposed, otherwise we would have made the SCSI disk driver doing ATA
IDENTIFY DEVICE and issue TRIM commands directly, instead of relying
on a SATL. ATA PASS-THROUGH should never be "triggered" like that
anyway.

Well, it is alright to have an "sg_sat_sanitize" (which makes use of
ATA PASS-THROUGH like other sg_sat_*) though. I am just not sure if
sg3_utils should go on having more sg_sat_*...

On 9 July 2016 at 08:49, James Bottomley
 wrote:
> On Fri, 2016-07-08 at 19:38 +, Tom Yan wrote:
>> On 8 July 2016 at 17:29, James Bottomley
>>  wrote:
>> > Or we could simply patch sg_sanitze to issue the ATA_16 pass
>> > through when it sees a sata device ...
>> >
>>
>> Ugh that sounds ugly to me. Anyway that's off-topic.
>
> Not really.  The point is that you've proposed something as an addition
> to the kernel that can also be done in userspace.  Checking if it can
> work easily there is like a barrier to entry.  If it works, then fine,
> we're done.  If it throws up problems then we reconsider the kernel
> route.
>
> 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