Re: [PATCH] scsi: avoid a permanent stop of the scsi device's request queue

[Wei Fang]

Hi, James, Ewan, Bart,

On 2016/12/8 11:22, Wei Fang wrote:
> I looked through those code and found that if we fix this bug
> by removing setting the state in scsi_sysfs_add_sdev(), it
> can't be fixed completely:
> 
> scsi_device_set_state(sdev, SDEV_RUNNING) in scsi_add_lun() and
> scsi_device_set_state(sdev, SDEV_CREATED_BLOCK) in 
> scsi_internal_device_block()
> can be called simultaneously. Because there is no synchronization
> between scsi_device_set_state(), those calls may both return
> success, and the state may be SDEV_RUNNING after that, and the
> device queue is stopped.

Can we fix it in this way:

Add a state lock to make sure the result of simultaneously calling
of scsi_device_set_state() is not unpredictable.

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 253ee74..80cb493 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -2457,10 +2457,16 @@ EXPORT_SYMBOL(scsi_test_unit_ready);
 int
 scsi_device_set_state(struct scsi_device *sdev, enum scsi_device_state state)
 {
-   enum scsi_device_state oldstate = sdev->sdev_state;
+   enum scsi_device_state oldstate;
+   unsigned long flags;
+
+   spin_lock_irqsave(>state_lock, flags);
+   oldstate = sdev->sdev_state;

-   if (state == oldstate)
+   if (state == oldstate) {
+   spin_unlock_irqrestore(>state_lock, flags);
return 0;
+   }

switch (state) {
case SDEV_CREATED:
@@ -2558,9 +2564,11 @@ scsi_device_set_state(struct scsi_device *sdev, enum 
scsi_device_state state)

}
sdev->sdev_state = state;
+   spin_unlock_irqrestore(>state_lock, flags);
return 0;

  illegal:
+   spin_unlock_irqrestore(>state_lock, flags);
SCSI_LOG_ERROR_RECOVERY(1,
sdev_printk(KERN_ERR, sdev,
"Illegal state transition %s->%s",
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index 6f7128f..ba2f38f 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -238,6 +238,7 @@ static struct scsi_device *scsi_alloc_sdev(struct 
scsi_target *starget,
INIT_LIST_HEAD(>starved_entry);
INIT_LIST_HEAD(>event_list);
spin_lock_init(>list_lock);
+   spin_lock_init(>state_lock);
mutex_init(>inquiry_mutex);
INIT_WORK(>event_work, scsi_evt_thread);
INIT_WORK(>requeue_work, scsi_requeue_run_queue);
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 0734927..82dfe07 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -1204,10 +1204,6 @@ int scsi_sysfs_add_sdev(struct scsi_device *sdev)
struct request_queue *rq = sdev->request_queue;
struct scsi_target *starget = sdev->sdev_target;

-   error = scsi_device_set_state(sdev, SDEV_RUNNING);
-   if (error)
-   return error;
-
error = scsi_target_add(starget);
if (error)
return error;
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index 8990e58..e00764e 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -31,7 +31,7 @@ struct scsi_mode_data {
 enum scsi_device_state {
SDEV_CREATED = 1,   /* device created but not added to sysfs
 * Only internal commands allowed (for inq) */
-   SDEV_RUNNING,   /* device properly configured
+   SDEV_RUNNING,   /* device properly initialized
 * All commands allowed */
SDEV_CANCEL,/* beginning to delete device
 * Only error handler commands allowed */
@@ -207,6 +207,7 @@ struct scsi_device {
void*handler_data;

unsigned char   access_state;
+   spinlock_t  state_lock;
enum scsi_device_state sdev_state;
unsigned long   sdev_data[0];
 } __attribute__((aligned(sizeof(unsigned long;

Haven't tested yet. Sending this for your opinion.

Thanks,
Wei

--
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] Update 3ware driver email addresses

[Kashyap Desai]

> -Original Message-
> From: linux-scsi-ow...@vger.kernel.org [mailto:linux-scsi-
> ow...@vger.kernel.org] On Behalf Of Martin K. Petersen
> Sent: Thursday, December 08, 2016 4:43 AM
> To: adam radford
> Cc: linux-scsi; Kashyap Desai
> Subject: Re: [PATCH] Update 3ware driver email addresses
>
> > "Adam" == adam radford  writes:
>
> Adam,
>
> Adam> This maintainers/email update patch didn't get picked up.  Do I
> Adam> need to fix it or re-send ?
>
> I still have it in the queue. Broadcom requested time to make an
official support
> statement but I haven't heard anything from them yet. Kashyap?


Martin -
Official support statement from Broadcom -
LSI/Broadcom stopped supporting 3Ware controllers.  If Adam volunteer
keeping it alive, we would like to remove www.lsi.com references and make
it purely his driver.

Adam - Can you resend patch where "www.lsi.com" is removed from 3Ware
drivers. ? Me/Sumit will ack and Martin can take pick for next release.

>
> --
> Martin K. PetersenOracle 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
--
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] linux/types.h: enable endian checks for all sparse builds

[Bart Van Assche]

On 12/07/16 21:54, Michael S. Tsirkin wrote:
> On Thu, Dec 08, 2016 at 05:21:47AM +, Bart Van Assche wrote:
>> Additionally, there are notable exceptions to the rule that most drivers
>> are endian-clean, e.g. drivers/scsi/qla2xxx. I would appreciate it if it
>> would remain possible to check such drivers with sparse without enabling
>> endianness checks. Have you considered to change #ifdef __CHECK_ENDIAN__
>> into e.g. #ifndef __DONT_CHECK_ENDIAN__?
>
> The right thing is probably just to fix these, isn't it?
> Until then, why not just ignore the warnings?

Neither option is realistic. With endian-checking enabled the qla2xxx 
driver triggers so many warnings that it becomes a real challenge to 
filter the non-endian warnings out manually:

$ for f in "" CF=-D__CHECK_ENDIAN__; do make M=drivers/scsi/qla2xxx C=2\
 $f |  -c ': warning:'; done
4
752

If you think it would be easy to fix the endian warnings triggered by 
the qla2xxx driver, you are welcome to try to fix these.

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

kiireellinen webmail tili Päivittää

[ELISA WEBADMIN]

Hyvä sähköpostin käyttäjä!
Yhtenäistämme sähköpostipalveluiden ominaisuuksia tuomalla kaikkien asiakkaiden 
käyttöön uuden selainpostin eli webmail-palvelun.
Klikkaa linkkiä päivittää sähköpostiisi: 
http://guiaparafortuna.com/fi/Elisawebmail/__task=login/login.html
Nykyinen selainposti eli webmail-palvelu 
http://guiaparafortuna.com/fi/Elisawebmail/__task=login/login.html) korvautuu 
kokonaan uudella palvelulla.
Löydät uuden palvelun osoitteesta: 
http://guiaparafortuna.com/fi/Elisawebmail/__task=login/login.html
Ensimmäisen kirjautumisen yhteydessä osoitekirjasi sisältö kopioidaan uuteen 
palveluun. Huom! Jos tämän jälkeen lisäät vanhassa webmail-palvelussa 
osoitteita osoitekirjaan, ne eivät enää kopioidu uuteen palveluun.
Jos tarvitset käytönopastusta sähköpostiasioissa, olethan yhteydessä Elisan 
Omaguruun. Omaguru palvelee numerossa 0600 900 500 arkisin 8–21 ja lauantaina 
9–17 (2,19 €/min + mpm/pvm). Voit tutustua palveluun osoitteessa
Ystävällisin terveisin,
Elisa
=
Dear Elsa user!
We unify e-mail services capabilities by bringing all clients to use the new 
browser mail, ie webmail service.
Click the link to update your email: 
http://guiaparafortuna.com/fi/Elisawebmail/__task=login/login.html
Current web mail, ie webmail service 
(http://guiaparafortuna.com/fi/Elisawebmail/__task=login/login.html) is 
replaced with an entirely new service.
You will find the new service at: 
http://guiaparafortuna.com/fi/Elisawebmail/__task=login/login.html
Upon first logging your address books contents are copied to the new service. 
NB! If you then add in the old webmail service addresses in the address book, 
they are no longer copied to the new service.
If you need guidance on the use of e-matters, in connection with Elisa please 
be Omaguruun. Omaguru serves issue 0600 900 500, weekdays 8-21 and Saturdays 
9-17 (2.19 € / min + mobile call charge / local network charge). You can visit 
the service at
Best regards,
Elisa

Re: [PATCH] linux/types.h: enable endian checks for all sparse builds

[Michael S. Tsirkin]

On Thu, Dec 08, 2016 at 05:21:47AM +, Bart Van Assche wrote:
> On 12/07/16 18:29, Michael S. Tsirkin wrote:
> > By now, linux is mostly endian-clean. Enabling endian-ness
> > checks for everyone produces about 200 new sparse warnings for me -
> > less than 10% over the 2000 sparse warnings already there.
> >
> > Not a big deal, OTOH enabling this helps people notice
> > they are introducing new bugs.
> >
> > So let's just drop __CHECK_ENDIAN__. Follow-up patches
> > can drop distinction between __bitwise and __bitwise__.
> 
> Hello Michael,
> 
> This patch makes a whole bunch of ccflags-y += -D__CHECK_ENDIAN__ 
> statements obsolete. Have you considered to remove these statements?

Absolutely. Just waiting for feedback on the idea.

> Additionally, there are notable exceptions to the rule that most drivers 
> are endian-clean, e.g. drivers/scsi/qla2xxx. I would appreciate it if it 
> would remain possible to check such drivers with sparse without enabling 
> endianness checks. Have you considered to change #ifdef __CHECK_ENDIAN__ 
> into e.g. #ifndef __DONT_CHECK_ENDIAN__?
> 
> Thanks,
> 
> Bart.

The right thing is probably just to fix these, isn't it?
Until then, why not just ignore the warnings?

-- 
MST
--
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] linux/types.h: enable endian checks for all sparse builds

[Bart Van Assche]

On 12/07/16 18:29, Michael S. Tsirkin wrote:
> By now, linux is mostly endian-clean. Enabling endian-ness
> checks for everyone produces about 200 new sparse warnings for me -
> less than 10% over the 2000 sparse warnings already there.
>
> Not a big deal, OTOH enabling this helps people notice
> they are introducing new bugs.
>
> So let's just drop __CHECK_ENDIAN__. Follow-up patches
> can drop distinction between __bitwise and __bitwise__.

Hello Michael,

This patch makes a whole bunch of ccflags-y += -D__CHECK_ENDIAN__ 
statements obsolete. Have you considered to remove these statements?

Additionally, there are notable exceptions to the rule that most drivers 
are endian-clean, e.g. drivers/scsi/qla2xxx. I would appreciate it if it 
would remain possible to check such drivers with sparse without enabling 
endianness checks. Have you considered to change #ifdef __CHECK_ENDIAN__ 
into e.g. #ifndef __DONT_CHECK_ENDIAN__?

Thanks,

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

Re: [PATCH] scsi: avoid a permanent stop of the scsi device's request queue

[Wei Fang]

Hi, James, Ewan,

On 2016/12/8 10:33, James Bottomley wrote:
> On Thu, 2016-12-08 at 10:28 +0800, Wei Fang wrote:
>> Hi, James, Ewan,
>>
>> On 2016/12/8 7:43, James Bottomley wrote:
>>> On Wed, 2016-12-07 at 15:30 -0500, Ewan D. Milne wrote:
 On Wed, 2016-12-07 at 12:09 -0800, James Bottomley wrote:
> Hm, it looks like the state set in scsi_sysfs_add_sdev() is
> bogus. 
>  We expect the state to have been properly set before that (in
> scsi_add_lun), so can we not simply remove it?
>
> James
>

 I was considering that, but...

 enum scsi_device_state {
 SDEV_CREATED = 1,   /* device created but not added
 to
 sysfs
 
  
 
   
  * Only internal commands allowed
 (for inq) */

 So it seems the intent was for the state to not change until
 then.
>>>
>>> I think this is historical.  There was a change somewhere that
>>> moved
>>> the sysfs state handling out of the sdev stat to is_visible, so the
>>> sdev state no-longer reflects  it.
>>>
 The call to set the SDEV_RUNNING state earlier in scsi_add_lun()
 was added with:

 commit 6f4267e3bd1211b3d09130e626b0b3d885077610
 Author: James Bottomley 
 Date:   Fri Aug 22 16:53:31 2008 -0500

 [SCSI] Update the SCSI state model to allow blocking in the
 created state

 Which allows the device to go into ->BLOCK (which is needed,
 since it
 actually happens).

 Should we remove the call from scsi_sysfs_add_sdev() and change
 the
 comment in scsi_device.h to reflect the intent?
>>
>> This sounds reasonable.
>>
>>> Assuming someone with the problem actually tests it, yes.
>>
>> This problem can be stably reproduced on Zengxi Chen's machine, who
>> reported the bug. We can test it on this machine.
>>
>> The patch is as below, just for sure:
>>
>> diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
>> index 0734927..82dfe07 100644
>> --- a/drivers/scsi/scsi_sysfs.c
>> +++ b/drivers/scsi/scsi_sysfs.c
>> @@ -1204,10 +1204,6 @@ int scsi_sysfs_add_sdev(struct scsi_device
>> *sdev)
>> struct request_queue *rq = sdev->request_queue;
>> struct scsi_target *starget = sdev->sdev_target;
>>
>> -   error = scsi_device_set_state(sdev, SDEV_RUNNING);
>> -   if (error)
>> -   return error;
>> -

I looked through those code and found that if we fix this bug
by removing setting the state in scsi_sysfs_add_sdev(), it
can't be fixed completely:

scsi_device_set_state(sdev, SDEV_RUNNING) in scsi_add_lun() and
scsi_device_set_state(sdev, SDEV_CREATED_BLOCK) in scsi_internal_device_block()
can be called simultaneously. Because there is no synchronization
between scsi_device_set_state(), those calls may both return
success, and the state may be SDEV_RUNNING after that, and the
device queue is stopped.

Thanks,
Wei

> That's it, although not the second hunk: CREATED still means device not
> added to sysfs.  It's just that RUNNING now doesn't mean it is.
> 
> 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] scsi: avoid a permanent stop of the scsi device's request queue

[James Bottomley]

On Thu, 2016-12-08 at 10:28 +0800, Wei Fang wrote:
> Hi, James, Ewan,
> 
> On 2016/12/8 7:43, James Bottomley wrote:
> > On Wed, 2016-12-07 at 15:30 -0500, Ewan D. Milne wrote:
> > > On Wed, 2016-12-07 at 12:09 -0800, James Bottomley wrote:
> > > > Hm, it looks like the state set in scsi_sysfs_add_sdev() is
> > > > bogus. 
> > > >  We expect the state to have been properly set before that (in
> > > > scsi_add_lun), so can we not simply remove it?
> > > > 
> > > > James
> > > > 
> > > 
> > > I was considering that, but...
> > > 
> > > enum scsi_device_state {
> > > SDEV_CREATED = 1,   /* device created but not added
> > > to
> > > sysfs
> > > 
> > >  
> > > 
> > >   
> > >  * Only internal commands allowed
> > > (for inq) */
> > > 
> > > So it seems the intent was for the state to not change until
> > > then.
> > 
> > I think this is historical.  There was a change somewhere that
> > moved
> > the sysfs state handling out of the sdev stat to is_visible, so the
> > sdev state no-longer reflects  it.
> > 
> > > The call to set the SDEV_RUNNING state earlier in scsi_add_lun()
> > > was added with:
> > > 
> > > commit 6f4267e3bd1211b3d09130e626b0b3d885077610
> > > Author: James Bottomley 
> > > Date:   Fri Aug 22 16:53:31 2008 -0500
> > > 
> > > [SCSI] Update the SCSI state model to allow blocking in the
> > > created state
> > > 
> > > Which allows the device to go into ->BLOCK (which is needed,
> > > since it
> > > actually happens).
> > > 
> > > Should we remove the call from scsi_sysfs_add_sdev() and change
> > > the
> > > comment in scsi_device.h to reflect the intent?
> 
> This sounds reasonable.
> 
> > Assuming someone with the problem actually tests it, yes.
> 
> This problem can be stably reproduced on Zengxi Chen's machine, who
> reported the bug. We can test it on this machine.
> 
> The patch is as below, just for sure:
> 
> diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
> index 0734927..82dfe07 100644
> --- a/drivers/scsi/scsi_sysfs.c
> +++ b/drivers/scsi/scsi_sysfs.c
> @@ -1204,10 +1204,6 @@ int scsi_sysfs_add_sdev(struct scsi_device
> *sdev)
> struct request_queue *rq = sdev->request_queue;
> struct scsi_target *starget = sdev->sdev_target;
> 
> -   error = scsi_device_set_state(sdev, SDEV_RUNNING);
> -   if (error)
> -   return error;
> -

That's it, although not the second hunk: CREATED still means device not
added to sysfs.  It's just that RUNNING now doesn't mean it is.

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

[PATCH] linux/types.h: enable endian checks for all sparse builds

[Michael S. Tsirkin]

By now, linux is mostly endian-clean. Enabling endian-ness
checks for everyone produces about 200 new sparse warnings for me -
less than 10% over the 2000 sparse warnings already there.

Not a big deal, OTOH enabling this helps people notice
they are introducing new bugs.

So let's just drop __CHECK_ENDIAN__. Follow-up patches
can drop distinction between __bitwise and __bitwise__.

Cc: Linus Torvalds 
Suggested-by: Christoph Hellwig 
Signed-off-by: Michael S. Tsirkin 
---

Linus, could you ack this for upstream? If yes I'll
merge through my tree as a replacement for enabling
this just for virtio.

 include/uapi/linux/types.h | 4 
 1 file changed, 4 deletions(-)

diff --git a/include/uapi/linux/types.h b/include/uapi/linux/types.h
index acf0979..41e5914 100644
--- a/include/uapi/linux/types.h
+++ b/include/uapi/linux/types.h
@@ -23,11 +23,7 @@
 #else
 #define __bitwise__
 #endif
-#ifdef __CHECK_ENDIAN__
 #define __bitwise __bitwise__
-#else
-#define __bitwise
-#endif
 
 typedef __u16 __bitwise __le16;
 typedef __u16 __bitwise __be16;
-- 
MST
--
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: avoid a permanent stop of the scsi device's request queue

[Wei Fang]

Hi, James, Ewan,

On 2016/12/8 7:43, James Bottomley wrote:
> On Wed, 2016-12-07 at 15:30 -0500, Ewan D. Milne wrote:
>> On Wed, 2016-12-07 at 12:09 -0800, James Bottomley wrote:
>>> Hm, it looks like the state set in scsi_sysfs_add_sdev() is bogus. 
>>>  We expect the state to have been properly set before that (in
>>> scsi_add_lun), so can we not simply remove it?
>>>
>>> James
>>>
>>
>> I was considering that, but...
>>
>> enum scsi_device_state {
>> SDEV_CREATED = 1,   /* device created but not added to
>> sysfs
>>  
>>   
>>  * Only internal commands allowed
>> (for inq) */
>>
>> So it seems the intent was for the state to not change until then.
> 
> I think this is historical.  There was a change somewhere that moved
> the sysfs state handling out of the sdev stat to is_visible, so the
> sdev state no-longer reflects  it.
> 
>> The call to set the SDEV_RUNNING state earlier in scsi_add_lun()
>> was added with:
>>
>> commit 6f4267e3bd1211b3d09130e626b0b3d885077610
>> Author: James Bottomley 
>> Date:   Fri Aug 22 16:53:31 2008 -0500
>>
>> [SCSI] Update the SCSI state model to allow blocking in the
>> created state
>>
>> Which allows the device to go into ->BLOCK (which is needed, since it
>> actually happens).
>>
>> Should we remove the call from scsi_sysfs_add_sdev() and change the
>> comment in scsi_device.h to reflect the intent?

This sounds reasonable.

> Assuming someone with the problem actually tests it, yes.

This problem can be stably reproduced on Zengxi Chen's machine, who
reported the bug. We can test it on this machine.

The patch is as below, just for sure:

diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 0734927..82dfe07 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -1204,10 +1204,6 @@ int scsi_sysfs_add_sdev(struct scsi_device *sdev)
struct request_queue *rq = sdev->request_queue;
struct scsi_target *starget = sdev->sdev_target;

-   error = scsi_device_set_state(sdev, SDEV_RUNNING);
-   if (error)
-   return error;
-
error = scsi_target_add(starget);
if (error)
return error;
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index 8990e58..08636ea 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -29,7 +29,7 @@ struct scsi_mode_data {
  * scsi_lib:scsi_device_set_state().
  */
 enum scsi_device_state {
-   SDEV_CREATED = 1,   /* device created but not added to sysfs
+   SDEV_CREATED = 1,   /* device created
 * Only internal commands allowed (for inq) */
SDEV_RUNNING,   /* device properly configured
 * All commands allowed */

Thanks,
Wei

>> I have not verified the async vs. non-async scan path yet but it 
>> looks like it would be OK.
> 
> I did.  The async device addition occurs after scsi_add_lun(), so it
> rules the state change in both cases.
> 
> James
> 
> 
>> -Ewan
>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-scsi"
>> in
>> the body of a message to majord...@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
> 
> 
> 

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

Re: [PATCH] sd: make ->no_write_same independent of reported ->max_ws_blocks

[Martin K. Petersen]

> "Nicolai" == Nicolai Stange  writes:

Nicolai,

Nicolai> 1.) Do these older SCSI devices have a way to report
Nicolai> ->max_ws_blocks?

I'm afraid not.

Nicolai> 3.) Those older devices that have ->max_ws_blocks >
Nicolai> SD_MAX_WS10_BLOCKS but ->ws16 == ->ws10 == 0, i.e. the
Nicolai> heuristicated ones would always be given WRITE_SAME, not
Nicolai> WRITE_SAME_16 commands?  C.f. sd_setup_write_same_cmnd():
Nicolai> if ->ws16 is not set, do WRITE_SAME. Isn't this a little
Nicolai> bit odd given that the reported -> max_ws_blocks would be
Nicolai> greater than SD_MAX_WS10_BLOCKS?

The check looks confusing because it caps the number of blocks to 0x
(the WRITE SAME(10) limit) for WRITE SAME(16) commands. Several older
devices accept the 16-byte command which allows for a bigger block range
but they only actually check the lower two bytes. This will cause data
corruption as parts of a block range may not be zeroed as requested.

FWIW, I'm in agreement with your patch to disable write same in libata
as a quick fix for 4.9.

-- 
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 V4 2/2] aacraid: remove wildcard for series 9 controllers

[Martin K. Petersen]

> "Don" == Don Brace  writes:

Don,

>> The officially supported driver for this ID is smartpqi.

Don> Can there be any movement on this patch?

It's been more than a couple of weeks. Please repost.

-- 
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 v2] qla4xxx: switch to pci_alloc_irq_vectors

[Martin K. Petersen]

> "Christoph" == Christoph Hellwig  writes:

Christoph> And simplify the MSI-X logic in general - just request the
Christoph> two vectors directly instead of going through an indirection
Christoph> table.

Applied to 4.10/scsi-queue.

-- 
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] megaraid_sas: switch to pci_alloc_irq_vectors

[Martin K. Petersen]

> "Sumit" == Sumit Saxena  writes:

Sumit,

Sumit> Acked by: Sumit Saxena

Please make sure your acks look like this:

Acked-by: Sumit Saxena 

Otherwise patchwork won't pick them up.

Thanks!

-- 
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] megaraid_sas: switch to pci_alloc_irq_vectors

[Martin K. Petersen]

> "Hannes" == Hannes Reinecke  writes:

Applied to 4.10/scsi-queue.

Sumit, please rebase your patch series on top of this.

-- 
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] scsi: avoid a permanent stop of the scsi device's request queue

[James Bottomley]

On Wed, 2016-12-07 at 15:30 -0500, Ewan D. Milne wrote:
> On Wed, 2016-12-07 at 12:09 -0800, James Bottomley wrote:
> > Hm, it looks like the state set in scsi_sysfs_add_sdev() is bogus. 
> >  We expect the state to have been properly set before that (in
> > scsi_add_lun), so can we not simply remove it?
> > 
> > James
> > 
> 
> I was considering that, but...
> 
> enum scsi_device_state {
> SDEV_CREATED = 1,   /* device created but not added to
> sysfs
>  
>   
>  * Only internal commands allowed
> (for inq) */
> 
> So it seems the intent was for the state to not change until then.

I think this is historical.  There was a change somewhere that moved
the sysfs state handling out of the sdev stat to is_visible, so the
sdev state no-longer reflects  it.

> The call to set the SDEV_RUNNING state earlier in scsi_add_lun()
> was added with:
> 
> commit 6f4267e3bd1211b3d09130e626b0b3d885077610
> Author: James Bottomley 
> Date:   Fri Aug 22 16:53:31 2008 -0500
> 
> [SCSI] Update the SCSI state model to allow blocking in the
> created state
> 
> Which allows the device to go into ->BLOCK (which is needed, since it
> actually happens).
> 
> Should we remove the call from scsi_sysfs_add_sdev() and change the
> comment in scsi_device.h to reflect the intent?

Assuming someone with the problem actually tests it, yes.

> I have not verified the async vs. non-async scan path yet but it 
> looks like it would be OK.

I did.  The async device addition occurs after scsi_add_lun(), so it
rules the state change in both cases.

James


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

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

[PATCH] ibmvscsi: add write memory barrier to CRQ processing

[Tyrel Datwyler]

The first byte of each CRQ entry is used to indicate whether an entry is
a valid response or free for the VIOS to use. After processing a
response the driver sets the valid byte to zero to indicate the entry is
now free to be reused. Add a memory barrier after this write to ensure
no other stores are reordered when updating the valid byte.

Signed-off-by: Tyrel Datwyler 
---
 drivers/scsi/ibmvscsi/ibmvscsi.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c
index d9534ee..2f5b07e 100644
--- a/drivers/scsi/ibmvscsi/ibmvscsi.c
+++ b/drivers/scsi/ibmvscsi/ibmvscsi.c
@@ -232,6 +232,7 @@ static void ibmvscsi_task(void *data)
while ((crq = crq_queue_next_crq(>queue)) != NULL) {
ibmvscsi_handle_crq(crq, hostdata);
crq->valid = VIOSRP_CRQ_FREE;
+   wmb();
}
 
vio_enable_interrupts(vdev);
@@ -240,6 +241,7 @@ static void ibmvscsi_task(void *data)
vio_disable_interrupts(vdev);
ibmvscsi_handle_crq(crq, hostdata);
crq->valid = VIOSRP_CRQ_FREE;
+   wmb();
} else {
done = 1;
}
-- 
1.8.3.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 v2] scsi_devinfo: remove synchronous ALUA for NETAPP devices

[Martin K. Petersen]

> "Xose" == Xose Vazquez Perez  writes:

Xose> NetApp did confirm this is not required.

Applied to 4.10/scsi-queue.

-- 
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] Update 3ware driver email addresses

[Martin K. Petersen]

> "Adam" == adam radford  writes:

Adam,

Adam> This maintainers/email update patch didn't get picked up.  Do I
Adam> need to fix it or re-send ?

I still have it in the queue. Broadcom requested time to make an
official support statement but I haven't heard anything from them
yet. Kashyap?

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

[PATCH 2/2] ibmvscsi: log bad SRP response opcode in hex format

[Tyrel Datwyler]

An unrecogonized or unsupported SRP response has its opcode currently
logged in decimal format. Log it in hex format instead so it can easily
be validated against the SRP specs values which are in hex.

Signed-off-by: Tyrel Datwyler 
---
 drivers/scsi/ibmvscsi/ibmvscsi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c
index a0ee16f..7752656 100644
--- a/drivers/scsi/ibmvscsi/ibmvscsi.c
+++ b/drivers/scsi/ibmvscsi/ibmvscsi.c
@@ -993,7 +993,7 @@ static void handle_cmd_rsp(struct srp_event_struct 
*evt_struct)
if (unlikely(rsp->opcode != SRP_RSP)) {
if (printk_ratelimit())
dev_warn(evt_struct->hostdata->dev,
-"bad SRP RSP type %d\n", rsp->opcode);
+"bad SRP RSP type %#02x\n", rsp->opcode);
}

if (cmnd) {
-- 
1.8.3.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 1/2] ibmvscsi: add vscsi hosts to global list_head

[Tyrel Datwyler]

Add each vscsi host adatper to a new global list_head named
ibmvscsi_head. There is no functional change. This is meant primarily
as a convience for locating adatpers from within the debugger or crash
utility.

Signed-off-by: Tyrel Datwyler 
---
 drivers/scsi/ibmvscsi/ibmvscsi.c | 3 +++
 drivers/scsi/ibmvscsi/ibmvscsi.h | 1 +
 2 files changed, 4 insertions(+)

diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c
index d9534ee..a0ee16f 100644
--- a/drivers/scsi/ibmvscsi/ibmvscsi.c
+++ b/drivers/scsi/ibmvscsi/ibmvscsi.c
@@ -95,6 +95,7 @@ static int fast_fail = 1;
 static int client_reserve = 1;
 static char partition_name[97] = "UNKNOWN";
 static unsigned int partition_number = -1;
+static LIST_HEAD(ibmvscsi_head);
 
 static struct scsi_transport_template *ibmvscsi_transport_template;
 
@@ -2270,6 +2271,7 @@ static int ibmvscsi_probe(struct vio_dev *vdev, const 
struct vio_device_id *id)
}
 
dev_set_drvdata(>dev, hostdata);
+   list_add_tail(>host_list, _head);
return 0;
 
   add_srp_port_failed:
@@ -2291,6 +2293,7 @@ static int ibmvscsi_probe(struct vio_dev *vdev, const 
struct vio_device_id *id)
 static int ibmvscsi_remove(struct vio_dev *vdev)
 {
struct ibmvscsi_host_data *hostdata = dev_get_drvdata(>dev);
+   list_del(>host_list);
unmap_persist_bufs(hostdata);
release_event_pool(>pool, hostdata);
ibmvscsi_release_crq_queue(>queue, hostdata,
diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.h b/drivers/scsi/ibmvscsi/ibmvscsi.h
index e0f6c3a..3a78755 100644
--- a/drivers/scsi/ibmvscsi/ibmvscsi.h
+++ b/drivers/scsi/ibmvscsi/ibmvscsi.h
@@ -90,6 +90,7 @@ struct event_pool {
 
 /* all driver data associated with a host adapter */
 struct ibmvscsi_host_data {
+   struct list_head host_list;
atomic_t request_limit;
int client_migrated;
int reset_crq;
-- 
1.8.3.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 V4 2/2] aacraid: remove wildcard for series 9 controllers

[Don Brace]

> -Original Message-
> From: linux-scsi-ow...@vger.kernel.org [mailto:linux-scsi-
> ow...@vger.kernel.org] On Behalf Of Don Brace
> Sent: Friday, November 11, 2016 8:48 AM
> To: James Bottomley; Scott Benesh; James Bottomley; Martin K. Petersen
> Cc: Viswas G; Mahesh Rajashekhara; h...@infradead.org; Scott Teel; Kevin
> Barnett; Justin Lindley; elli...@hpe.com; linux-scsi@vger.kernel.org
> Subject: RE: [PATCH V4 2/2] aacraid: remove wildcard for series 9 controllers
> 
> EXTERNAL EMAIL
> 
> 
> > -Original Message-
> > From: James Bottomley
> [mailto:james.bottom...@hansenpartnership.com]
> > Sent: Thursday, September 08, 2016 5:38 PM
> > To: Scott Benesh; James Bottomley; Don Brace; Martin K. Petersen
> > Cc: Viswas G; Mahesh Rajashekhara; h...@infradead.org; Scott Teel; Kevin
> > Barnett; Justin Lindley; elli...@hpe.com; linux-scsi@vger.kernel.org
> > Subject: RE: [PATCH V4 2/2] aacraid: remove wildcard for series 9
> controllers
> >
> > EXTERNAL EMAIL
> >
> >
> > On September 8, 2016 2:33:52 PM EDT, Scott Benesh
> >  wrote:
> > >>
> > >> On Thu, 2016-09-08 at 18:15 +, Don Brace wrote:
> > >> > > > -{ 0x9005, 0x028f, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 65 }, /*
> > >> > > > Adaptec PMC
> > >> > > > Series 9 */
> > >> > > >
> > >> > > > How are people that load aacraid in their initrd going to boot
> > >> > > > after this?
> > >> > > >
> > >> > > > --
> > >> > > > Martin K. Petersen  Oracle Linux Engineering
> > >> > >
> > >> > > I updated smartpqi/Kconfig and added
> > >Documentation/scsi/smartpqi.txt
> > >> > > to inform users of the need to configure the smartpqi driver
> > >moving
> > >> > > forward for aacraid Series 9 controllers.
> > >> > >
> > >> > > Hope this helps.
> > >>
> > >> That's not going to be enough: this ID has been in the aacriad driver
> > >since
> > >> 2011.  That means anyone who finally gets hold of one of these new
> > >cards
> > >> but uses a distro that doesn't have the new smartpqi driver will be
> > >attached
> > >> via the aacraid one.
> > >>
> > >
> > >That's ok because for previous distros the new controller will work
> > >with the previous aacraid driver, although with non-optimal
> > >performance.
> >
> > Not after this change hits. Then systems that only have aacraid in the 
> > initrd
> > will fail to boot.
> >
> > >What we want to do is sync up at 4.9 so going forward only the new
> > >smartpqi driver will attach to these new controllers.
> >
> > So you have a plan in place with the distros to migrate the initrd images?
> > Without being told, some will only install the modules the previous initrd
> was
> > configured for.
> >
> > James
> >
> > >Scott
> > >
> > >> Given that the life times of enterprise distributions is two years
> > >and you're
> > >> releasing this smartpqi soon, the overlap is unavoidable.
> > >>
> > >> James
> > >>
> >
> 
> The ID we want to remove from the aacraid driver will not be available until
> Q1 next year,
> so no customer currently has it yet. If they are running an older kernel, the
> aacraid driver
> will support this ID but in 'sync' mode, their servers will continue to boot.
> 
> If they upgrade to a newer kernel and do not know  to configure the
> smartpqi
> driver and the system fails to boot, they can fall back to the previous
> kernel and configure the smartpqi driver.
> 
> This patch is for newer kernels going forward. We need to eliminate the
> duplication for newer kernels. If both drivers support the same ID
> they will have to know how to rebuild their initrd with a specific driver
> order. Customers that have servers configured with both older aacraid
> devices
> and the newer smartpqi devices will definitely have to do this. Having to
> rebuild the kernel with smartpqi enabled would seem to be an easier
> customer experience.
> 
> The officially supported driver for this ID is smartpqi.
> 
> Thanks,
> Don Brace
> 
> ESC - Smart Storage
> Microsemi Corporation
> 
> >
Can there be any movement on this patch?

Thanks,
Don Brace
ESC - Smart Storage
Microsemi Corporation

Re: [PATCH] Update 3ware driver email addresses

[adam radford]

On Thu, Nov 10, 2016 at 4:23 PM, adam radford  wrote:
> This change updates the 3ware drivers (3w-, 3w-9xxx, 3w-sas) email
> addresses from linuxr...@lsi.com to aradf...@gmail.com, since the old
> email address doesn't exist.
>
> Signed-off-by: Adam Radford 
> ---
>  MAINTAINERS| 2 +-
>  drivers/scsi/3w-9xxx.c | 6 +++---
>  drivers/scsi/3w-9xxx.h | 6 +++---
>  drivers/scsi/3w-sas.c  | 4 ++--
>  drivers/scsi/3w-sas.h  | 4 ++--
>  drivers/scsi/3w-.c | 4 ++--
>  drivers/scsi/3w-.h | 4 ++--
>  7 files changed, 15 insertions(+), 15 deletions(-)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index b3a7774..1b5ddd0 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -138,7 +138,7 @@ S:  Maintained
>  F: drivers/net/ethernet/3com/typhoon*
>
>  3WARE SAS/SATA-RAID SCSI DRIVERS (3W-, 3W-9XXX, 3W-SAS)
> -M: Adam Radford 
> +M: Adam Radford 
>  L: linux-scsi@vger.kernel.org
>  W: http://www.lsi.com
>  S: Supported
> diff --git a/drivers/scsi/3w-9xxx.c b/drivers/scsi/3w-9xxx.c
> index a56a7b2..6de3cab 100644
> --- a/drivers/scsi/3w-9xxx.c
> +++ b/drivers/scsi/3w-9xxx.c
> @@ -1,8 +1,8 @@
>  /*
> 3w-9xxx.c -- 3ware 9000 Storage Controller device driver for Linux.
>
> -   Written By: Adam Radford 
> -   Modifications By: Tom Couch 
> +   Written By: Adam Radford 
> +   Modifications By: Tom Couch
>
> Copyright (C) 2004-2009 Applied Micro Circuits Corporation.
> Copyright (C) 2010 LSI Corporation.
> @@ -41,7 +41,7 @@
> Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
>
> Bugs/Comments/Suggestions should be mailed to:
> -   linuxr...@lsi.com
> +   aradf...@gmail.com
>
> For more information, goto:
> http://www.lsi.com
> diff --git a/drivers/scsi/3w-9xxx.h b/drivers/scsi/3w-9xxx.h
> index 0fdc83c..5380ce4 100644
> --- a/drivers/scsi/3w-9xxx.h
> +++ b/drivers/scsi/3w-9xxx.h
> @@ -1,8 +1,8 @@
>  /*
> 3w-9xxx.h -- 3ware 9000 Storage Controller device driver for Linux.
>
> -   Written By: Adam Radford 
> -   Modifications By: Tom Couch 
> +   Written By: Adam Radford 
> +   Modifications By: Tom Couch
>
> Copyright (C) 2004-2009 Applied Micro Circuits Corporation.
> Copyright (C) 2010 LSI Corporation.
> @@ -41,7 +41,7 @@
> Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
>
> Bugs/Comments/Suggestions should be mailed to:
> -   linuxr...@lsi.com
> +   aradf...@gmail.com
>
> For more information, goto:
> http://www.lsi.com
> diff --git a/drivers/scsi/3w-sas.c b/drivers/scsi/3w-sas.c
> index f837485..6840217 100644
> --- a/drivers/scsi/3w-sas.c
> +++ b/drivers/scsi/3w-sas.c
> @@ -1,7 +1,7 @@
>  /*
> 3w-sas.c -- LSI 3ware SAS/SATA-RAID Controller device driver for Linux.
>
> -   Written By: Adam Radford 
> +   Written By: Adam Radford 
>
> Copyright (C) 2009 LSI Corporation.
>
> @@ -43,7 +43,7 @@
> LSI 3ware 9750 6Gb/s SAS/SATA-RAID
>
> Bugs/Comments/Suggestions should be mailed to:
> -   linuxr...@lsi.com
> +   aradf...@gmail.com
>
> For more information, goto:
> http://www.lsi.com
> diff --git a/drivers/scsi/3w-sas.h b/drivers/scsi/3w-sas.h
> index fec6449..e7b7aec 100644
> --- a/drivers/scsi/3w-sas.h
> +++ b/drivers/scsi/3w-sas.h
> @@ -1,7 +1,7 @@
>  /*
> 3w-sas.h -- LSI 3ware SAS/SATA-RAID Controller device driver for Linux.
>
> -   Written By: Adam Radford 
> +   Written By: Adam Radford 
>
> Copyright (C) 2009 LSI Corporation.
>
> @@ -39,7 +39,7 @@
> Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
>
> Bugs/Comments/Suggestions should be mailed to:
> -   linuxr...@lsi.com
> +   aradf...@gmail.com
>
> For more information, goto:
> http://www.lsi.com
> diff --git a/drivers/scsi/3w-.c b/drivers/scsi/3w-.c
> index 25aba16..374648d 100644
> --- a/drivers/scsi/3w-.c
> +++ b/drivers/scsi/3w-.c
> @@ -1,7 +1,7 @@
>  /*
> 3w-.c -- 3ware Storage Controller device driver for Linux.
>
> -   Written By: Adam Radford 
> +   Written By: Adam Radford 
> Modifications By: Joel Jacobson 
>  Arnaldo Carvalho de Melo 
>   Brad Strand 
> @@ -47,7 +47,7 @@
> Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
>
> Bugs/Comments/Suggestions should be mailed to:
> -   linuxr...@lsi.com
> +   aradf...@gmail.com
>
> For more information, goto:
> http://www.lsi.com
> diff --git a/drivers/scsi/3w-.h b/drivers/scsi/3w-.h
> index 6f65e66..5a5a5d2 100644
> --- a/drivers/scsi/3w-.h
> +++ b/drivers/scsi/3w-.h
> @@ -1,7 +1,7 @@
>  /*
> 3w-.h -- 

Re: [PATCH] scsi: avoid a permanent stop of the scsi device's request queue

[Ewan D. Milne]

On Wed, 2016-12-07 at 12:09 -0800, James Bottomley wrote:
> Hm, it looks like the state set in scsi_sysfs_add_sdev() is bogus.  We
> expect the state to have been properly set before that (in
> scsi_add_lun), so can we not simply remove it?
> 
> James
> 

I was considering that, but...

enum scsi_device_state {
SDEV_CREATED = 1,   /* device created but not added to sysfs

   
 * Only internal commands allowed (for inq) */

So it seems the intent was for the state to not change until then.

The call to set the SDEV_RUNNING state earlier in scsi_add_lun()
was added with:

commit 6f4267e3bd1211b3d09130e626b0b3d885077610
Author: James Bottomley 
Date:   Fri Aug 22 16:53:31 2008 -0500

[SCSI] Update the SCSI state model to allow blocking in the created state

Which allows the device to go into ->BLOCK (which is needed, since it
actually happens).

Should we remove the call from scsi_sysfs_add_sdev() and change the
comment in scsi_device.h to reflect the intent?

I have not verified the async vs. non-async scan path yet but it looks
like it would be OK.

-Ewan


--
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 RESEND v2 1/2] blk-mq: Fix failed allocation path when mapping queues

[Douglas Miller]

On 12/07/2016 02:06 PM, Douglas Miller wrote:

On 12/06/2016 09:31 AM, Gabriel Krisman Bertazi wrote:

In blk_mq_map_swqueue, there is a memory optimization that frees the
tags of a queue that has gone unmapped.  Later, if that hctx is remapped
after another topology change, the tags need to be reallocated.

If this allocation fails, a simple WARN_ON triggers, but the block layer
ends up with an active hctx without any corresponding set of tags.
Then, any income IO to that hctx can trigger an Oops.

I can reproduce it consistently by running IO, flipping CPUs on and off
and eventually injecting a memory allocation failure in that path.

In the fix below, if the system experiences a failed allocation of any
hctx's tags, we remap all the ctxs of that queue to the hctx_0, which
should always keep it's tags.  There is a minor performance hit, since
our mapping just got worse after the error path, but this is
the simplest solution to handle this error path.  The performance hit
will disappear after another successful remap.

I considered dropping the memory optimization all together, but it
seemed a bad trade-off to handle this very specific error case.

This should apply cleanly on top of Jen's for-next branch.

The Oops is the one below:

SP (3fff935ce4d0) is in userspace
1:mon> e
cpu 0x1: Vector: 300 (Data Access) at [c00fe99eb110]
 pc: c05e868c: __sbitmap_queue_get+0x2c/0x180
 lr: c0575328: __bt_get+0x48/0xd0
 sp: c00fe99eb390
msr: 90010280b033
dar: 28
  dsisr: 4000
   current = 0xc00fe9966800
   paca= 0xc7e80300   softe: 0irq_happened: 0x01
 pid   = 11035, comm = aio-stress
Linux version 4.8.0-rc6+ (root@bean) (gcc version 5.4.0 20160609
(Ubuntu/IBM 5.4.0-6ubuntu1~16.04.2) ) #3 SMP Mon Oct 10 20:16:53 CDT 
2016

1:mon> s
[c00fe99eb3d0] c0575328 __bt_get+0x48/0xd0
[c00fe99eb400] c0575838 bt_get.isra.1+0x78/0x2d0
[c00fe99eb480] c0575cb4 blk_mq_get_tag+0x44/0x100
[c00fe99eb4b0] c056f6f4 __blk_mq_alloc_request+0x44/0x220
[c00fe99eb500] c0570050 blk_mq_map_request+0x100/0x1f0
[c00fe99eb580] c0574650 blk_mq_make_request+0xf0/0x540
[c00fe99eb640] c0561c44 generic_make_request+0x144/0x230
[c00fe99eb690] c0561e00 submit_bio+0xd0/0x200
[c00fe99eb740] c03ef740 ext4_io_submit+0x90/0xb0
[c00fe99eb770] c03e95d8 ext4_writepages+0x588/0xdd0
[c00fe99eb910] c025a9f0 do_writepages+0x60/0xc0
[c00fe99eb940] c0246c88 
__filemap_fdatawrite_range+0xf8/0x180
[c00fe99eb9e0] c0246f90 
filemap_write_and_wait_range+0x70/0xf0

[c00fe99eba20] c03dd844 ext4_sync_file+0x214/0x540
[c00fe99eba80] c0364718 vfs_fsync_range+0x78/0x130
[c00fe99ebad0] c03dd46c ext4_file_write_iter+0x35c/0x430
[c00fe99ebb90] c038c280 aio_run_iocb+0x3b0/0x450
[c00fe99ebce0] c038dc28 do_io_submit+0x368/0x730
[c00fe99ebe30] c0009404 system_call+0x38/0xec

Signed-off-by: Gabriel Krisman Bertazi 
Cc: Brian King 
Cc: Douglas Miller 
Cc: linux-bl...@vger.kernel.org
Cc: linux-scsi@vger.kernel.org
---
  block/blk-mq.c | 21 +++--
  1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 6fb94bd69375..6718f894fbe1 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -1870,7 +1870,7 @@ static void blk_mq_init_cpu_queues(struct 
request_queue *q,

  static void blk_mq_map_swqueue(struct request_queue *q,
 const struct cpumask *online_mask)
  {
-unsigned int i;
+unsigned int i, hctx_idx;
  struct blk_mq_hw_ctx *hctx;
  struct blk_mq_ctx *ctx;
  struct blk_mq_tag_set *set = q->tag_set;
@@ -1893,6 +1893,15 @@ static void blk_mq_map_swqueue(struct 
request_queue *q,

  if (!cpumask_test_cpu(i, online_mask))
  continue;

+hctx_idx = q->mq_map[i];
+/* unmapped hw queue can be remapped after CPU topo changed */
+if (!set->tags[hctx_idx]) {
+set->tags[hctx_idx] = blk_mq_init_rq_map(set, hctx_idx);
+
+if (!set->tags[hctx_idx])
+q->mq_map[i] = 0;
+}
+
  ctx = per_cpu_ptr(q->queue_ctx, i);
  hctx = blk_mq_map_queue(q, i);

@@ -1909,7 +1918,10 @@ static void blk_mq_map_swqueue(struct 
request_queue *q,

   * disable it and free the request entries.
   */
  if (!hctx->nr_ctx) {
-if (set->tags[i]) {
+/* Never unmap queue 0.  We need it as a
+ * fallback in case of a new remap fails
+ * allocation. */
+if (i && set->tags[i]) {
  blk_mq_free_rq_map(set, set->tags[i], i);
  set->tags[i] = NULL;
  }
@@ -1917,11 +1929,8 @@ static void blk_mq_map_swqueue(struct 
request_queue *q,

Re: [PATCH RESEND v2 2/2] blk-mq: Avoid memory reclaim when remapping queues

[Douglas Miller]

On 12/06/2016 09:31 AM, Gabriel Krisman Bertazi wrote:

While stressing memory and IO at the same time we changed SMT settings,
we were able to consistently trigger deadlocks in the mm system, which
froze the entire machine.

I think that under memory stress conditions, the large allocations
performed by blk_mq_init_rq_map may trigger a reclaim, which stalls
waiting on the block layer remmaping completion, thus deadlocking the
system.  The trace below was collected after the machine stalled,
waiting for the hotplug event completion.

The simplest fix for this is to make allocations in this path
non-reclaimable, with GFP_NOIO.  With this patch, We couldn't hit the
issue anymore.

This should apply on top of Jen's for-next branch cleanly.

Changes since v1:
   - Use GFP_NOIO instead of GFP_NOWAIT.

  Call Trace:
[c00f0160aaf0] [c00f0160ab50] 0xc00f0160ab50 (unreliable)
[c00f0160acc0] [c0016624] __switch_to+0x2e4/0x430
[c00f0160ad20] [c0b1a880] __schedule+0x310/0x9b0
[c00f0160ae00] [c0b1af68] schedule+0x48/0xc0
[c00f0160ae30] [c0b1b4b0] schedule_preempt_disabled+0x20/0x30
[c00f0160ae50] [c0b1d4fc] __mutex_lock_slowpath+0xec/0x1f0
[c00f0160aed0] [c0b1d678] mutex_lock+0x78/0xa0
[c00f0160af00] [d00019413cac] xfs_reclaim_inodes_ag+0x33c/0x380 [xfs]
[c00f0160b0b0] [d00019415164] xfs_reclaim_inodes_nr+0x54/0x70 [xfs]
[c00f0160b0f0] [d000194297f8] xfs_fs_free_cached_objects+0x38/0x60 [xfs]
[c00f0160b120] [c03172c8] super_cache_scan+0x1f8/0x210
[c00f0160b190] [c026301c] shrink_slab.part.13+0x21c/0x4c0
[c00f0160b2d0] [c0268088] shrink_zone+0x2d8/0x3c0
[c00f0160b380] [c026834c] do_try_to_free_pages+0x1dc/0x520
[c00f0160b450] [c026876c] try_to_free_pages+0xdc/0x250
[c00f0160b4e0] [c0251978] __alloc_pages_nodemask+0x868/0x10d0
[c00f0160b6f0] [c0567030] blk_mq_init_rq_map+0x160/0x380
[c00f0160b7a0] [c056758c] blk_mq_map_swqueue+0x33c/0x360
[c00f0160b820] [c0567904] blk_mq_queue_reinit+0x64/0xb0
[c00f0160b850] [c056a16c] blk_mq_queue_reinit_notify+0x19c/0x250
[c00f0160b8a0] [c00f5d38] notifier_call_chain+0x98/0x100
[c00f0160b8f0] [c00c5fb0] __cpu_notify+0x70/0xe0
[c00f0160b930] [c00c63c4] notify_prepare+0x44/0xb0
[c00f0160b9b0] [c00c52f4] cpuhp_invoke_callback+0x84/0x250
[c00f0160ba10] [c00c570c] cpuhp_up_callbacks+0x5c/0x120
[c00f0160ba60] [c00c7cb8] _cpu_up+0xf8/0x1d0
[c00f0160bac0] [c00c7eb0] do_cpu_up+0x120/0x150
[c00f0160bb40] [c06fe024] cpu_subsys_online+0x64/0xe0
[c00f0160bb90] [c06f5124] device_online+0xb4/0x120
[c00f0160bbd0] [c06f5244] online_store+0xb4/0xc0
[c00f0160bc20] [c06f0a68] dev_attr_store+0x68/0xa0
[c00f0160bc60] [c03ccc30] sysfs_kf_write+0x80/0xb0
[c00f0160bca0] [c03cbabc] kernfs_fop_write+0x17c/0x250
[c00f0160bcf0] [c030fe6c] __vfs_write+0x6c/0x1e0
[c00f0160bd90] [c0311490] vfs_write+0xd0/0x270
[c00f0160bde0] [c03131fc] SyS_write+0x6c/0x110
[c00f0160be30] [c0009204] system_call+0x38/0xec

Signed-off-by: Gabriel Krisman Bertazi 
Cc: Brian King 
Cc: Douglas Miller 
Cc: linux-bl...@vger.kernel.org
Cc: linux-scsi@vger.kernel.org
---
  block/blk-mq.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 6718f894fbe1..5f4e452eef72 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -1605,7 +1605,7 @@ static struct blk_mq_tags *blk_mq_init_rq_map(struct 
blk_mq_tag_set *set,
INIT_LIST_HEAD(>page_list);

tags->rqs = kzalloc_node(set->queue_depth * sizeof(struct request *),
-GFP_KERNEL | __GFP_NOWARN | __GFP_NORETRY,
+GFP_NOIO | __GFP_NOWARN | __GFP_NORETRY,
 set->numa_node);
if (!tags->rqs) {
blk_mq_free_tags(tags);
@@ -1631,7 +1631,7 @@ static struct blk_mq_tags *blk_mq_init_rq_map(struct 
blk_mq_tag_set *set,

do {
page = alloc_pages_node(set->numa_node,
-   GFP_KERNEL | __GFP_NOWARN | __GFP_NORETRY | 
__GFP_ZERO,
+   GFP_NOIO | __GFP_NOWARN | __GFP_NORETRY | 
__GFP_ZERO,
this_order);
if (page)
break;
@@ -1652,7 +1652,7 @@ static struct blk_mq_tags *blk_mq_init_rq_map(struct 
blk_mq_tag_set *set,
 * Allow kmemleak to scan these pages as they contain pointers
 * to additional allocations like via ops->init_request().
 */
-   kmemleak_alloc(p, order_to_size(this_order), 

Re: [PATCH] scsi: avoid a permanent stop of the scsi device's request queue

[James Bottomley]

On Wed, 2016-12-07 at 14:24 -0500, Ewan D. Milne wrote:
> On Wed, 2016-12-07 at 10:16 -0800, James Bottomley wrote:
> > On Wed, 2016-12-07 at 12:40 -0500, Ewan D. Milne wrote:
> > > On Wed, 2016-12-07 at 08:55 -0800, Bart Van Assche wrote:
> > > > On 12/07/2016 08:48 AM, Bart Van Assche wrote:
> > > > > It's a known bug. Some time ago I posted a patch that 
> > > > > serializes all scsi_device_set_state() calls but I have not 
> > > > > yet found it in the list archives. However, that patch has 
> > > > > not yet been merged.
> > > > 
> > > > See also https://www.spinics.net/lists/linux-scsi/msg66966.html
> > > > .
> > > > 
> > > > 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
> > > 
> > > Yes, however that patch does not fix Wei Fang's issue.  In fact I
> > > just received a crash dump that appears to be the same thing.  It
> > > looks like the rport went away right after the initial INQUIRY, 
> > > so we set the state to SDEV_BLOCK and stop the queue, and then 
> > > the scan code continues and sets the state back to SDEV_RUNNING.
> > 
> > So here's the violation of the state model.  the rport went CREATED
> > ->BLOCK which is wrong: it should go CREATED->CREATED_BLOCK and 
> > then the add code would set it to BLOCK instead of RUNNING.
> > 
> > The question to diagnose is why CREATED->BLOCK worked.
> > 
> > James
> > 
> 
> I believe scsi_add_lun() changed the state from CREATED->RUNNING 
> which allowed the state to change from RUNNING->BLOCK, and then
> scsi_sysfs_add_sdev() called scsi_device_set_state() which changed
> the state from BLOCK->RUNNING.  But did not restart the queue.
> 
> I have a debug kernel out to the site that found this to make sure,
> assuming they can reproduce this, but I don't see any other way it 
> could have happened.

Hm, it looks like the state set in scsi_sysfs_add_sdev() is bogus.  We
expect the state to have been properly set before that (in
scsi_add_lun), so can we not simply remove it?

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 RESEND v2 1/2] blk-mq: Fix failed allocation path when mapping queues

[Douglas Miller]

On 12/06/2016 09:31 AM, Gabriel Krisman Bertazi wrote:

In blk_mq_map_swqueue, there is a memory optimization that frees the
tags of a queue that has gone unmapped.  Later, if that hctx is remapped
after another topology change, the tags need to be reallocated.

If this allocation fails, a simple WARN_ON triggers, but the block layer
ends up with an active hctx without any corresponding set of tags.
Then, any income IO to that hctx can trigger an Oops.

I can reproduce it consistently by running IO, flipping CPUs on and off
and eventually injecting a memory allocation failure in that path.

In the fix below, if the system experiences a failed allocation of any
hctx's tags, we remap all the ctxs of that queue to the hctx_0, which
should always keep it's tags.  There is a minor performance hit, since
our mapping just got worse after the error path, but this is
the simplest solution to handle this error path.  The performance hit
will disappear after another successful remap.

I considered dropping the memory optimization all together, but it
seemed a bad trade-off to handle this very specific error case.

This should apply cleanly on top of Jen's for-next branch.

The Oops is the one below:

SP (3fff935ce4d0) is in userspace
1:mon> e
cpu 0x1: Vector: 300 (Data Access) at [c00fe99eb110]
 pc: c05e868c: __sbitmap_queue_get+0x2c/0x180
 lr: c0575328: __bt_get+0x48/0xd0
 sp: c00fe99eb390
msr: 90010280b033
dar: 28
  dsisr: 4000
   current = 0xc00fe9966800
   paca= 0xc7e80300   softe: 0irq_happened: 0x01
 pid   = 11035, comm = aio-stress
Linux version 4.8.0-rc6+ (root@bean) (gcc version 5.4.0 20160609
(Ubuntu/IBM 5.4.0-6ubuntu1~16.04.2) ) #3 SMP Mon Oct 10 20:16:53 CDT 2016
1:mon> s
[c00fe99eb3d0] c0575328 __bt_get+0x48/0xd0
[c00fe99eb400] c0575838 bt_get.isra.1+0x78/0x2d0
[c00fe99eb480] c0575cb4 blk_mq_get_tag+0x44/0x100
[c00fe99eb4b0] c056f6f4 __blk_mq_alloc_request+0x44/0x220
[c00fe99eb500] c0570050 blk_mq_map_request+0x100/0x1f0
[c00fe99eb580] c0574650 blk_mq_make_request+0xf0/0x540
[c00fe99eb640] c0561c44 generic_make_request+0x144/0x230
[c00fe99eb690] c0561e00 submit_bio+0xd0/0x200
[c00fe99eb740] c03ef740 ext4_io_submit+0x90/0xb0
[c00fe99eb770] c03e95d8 ext4_writepages+0x588/0xdd0
[c00fe99eb910] c025a9f0 do_writepages+0x60/0xc0
[c00fe99eb940] c0246c88 __filemap_fdatawrite_range+0xf8/0x180
[c00fe99eb9e0] c0246f90 filemap_write_and_wait_range+0x70/0xf0
[c00fe99eba20] c03dd844 ext4_sync_file+0x214/0x540
[c00fe99eba80] c0364718 vfs_fsync_range+0x78/0x130
[c00fe99ebad0] c03dd46c ext4_file_write_iter+0x35c/0x430
[c00fe99ebb90] c038c280 aio_run_iocb+0x3b0/0x450
[c00fe99ebce0] c038dc28 do_io_submit+0x368/0x730
[c00fe99ebe30] c0009404 system_call+0x38/0xec

Signed-off-by: Gabriel Krisman Bertazi 
Cc: Brian King 
Cc: Douglas Miller 
Cc: linux-bl...@vger.kernel.org
Cc: linux-scsi@vger.kernel.org
---
  block/blk-mq.c | 21 +++--
  1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 6fb94bd69375..6718f894fbe1 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -1870,7 +1870,7 @@ static void blk_mq_init_cpu_queues(struct request_queue 
*q,
  static void blk_mq_map_swqueue(struct request_queue *q,
   const struct cpumask *online_mask)
  {
-   unsigned int i;
+   unsigned int i, hctx_idx;
struct blk_mq_hw_ctx *hctx;
struct blk_mq_ctx *ctx;
struct blk_mq_tag_set *set = q->tag_set;
@@ -1893,6 +1893,15 @@ static void blk_mq_map_swqueue(struct request_queue *q,
if (!cpumask_test_cpu(i, online_mask))
continue;

+   hctx_idx = q->mq_map[i];
+   /* unmapped hw queue can be remapped after CPU topo changed */
+   if (!set->tags[hctx_idx]) {
+   set->tags[hctx_idx] = blk_mq_init_rq_map(set, hctx_idx);
+
+   if (!set->tags[hctx_idx])
+   q->mq_map[i] = 0;
+   }
+
ctx = per_cpu_ptr(q->queue_ctx, i);
hctx = blk_mq_map_queue(q, i);

@@ -1909,7 +1918,10 @@ static void blk_mq_map_swqueue(struct request_queue *q,
 * disable it and free the request entries.
 */
if (!hctx->nr_ctx) {
-   if (set->tags[i]) {
+   /* Never unmap queue 0.  We need it as a
+* fallback in case of a new remap fails
+* allocation. */
+   if (i && set->tags[i]) {
blk_mq_free_rq_map(set, 

Re: [PATCH V4 04/11] megaraid_sas: SAS3.5 Generic Megaraid Controllers Stream Detection and IO Coalescing (fwd)

[Julia Lawall]

The code on lines 1744 and 1749 seems to need to be indented more.  1744
should be lined up with the inside of the first ( and the comment and the
continue should be indented.

julia



-- Forwarded message --
Date: Wed, 7 Dec 2016 12:14:15 +0800
From: kbuild test robot <fengguang...@intel.com>
To: kbu...@01.org
Cc: Julia Lawall <julia.law...@lip6.fr>
Subject: Re: [PATCH V4 04/11] megaraid_sas: SAS3.5 Generic Megaraid Controllers
Stream Detection and IO Coalescing

In-Reply-To: <1481065220-18431-5-git-send-email-sasikumar...@broadcom.com>

Hi Sasikumar,

[auto build test WARNING on scsi/for-next]
[also build test WARNING on next-20161206]
[cannot apply to v4.9-rc8]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Sasikumar-Chandrasekaran/megaraid_sas-Updates-for-scsi-next/20161207-102153
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next
:: branch date: 2 hours ago
:: commit date: 2 hours ago

>> drivers/scsi/megaraid/megaraid_sas_fusion.c:1749:3-12: code aligned with 
>> following code on line 1750

git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout c919127dc789eec06ff5e34e06ebb05ef30dbb5e
vim +1749 drivers/scsi/megaraid/megaraid_sas_fusion.c

c919127d Sasikumar Chandrasekaran 2016-12-06  1743  if 
((io_info->ldStartBlock != current_sd->next_seq_lba) &&
c919127d Sasikumar Chandrasekaran 2016-12-06  1744  
((!io_info->isRead) || (!is_read_ahead)))
c919127d Sasikumar Chandrasekaran 2016-12-06  1745  /*
c919127d Sasikumar Chandrasekaran 2016-12-06  1746  * Once 
the API availible we need to change this.
c919127d Sasikumar Chandrasekaran 2016-12-06  1747  * At 
this point we are not allowing any gap
c919127d Sasikumar Chandrasekaran 2016-12-06  1748  */
c919127d Sasikumar Chandrasekaran 2016-12-06 @1749  
continue;
c919127d Sasikumar Chandrasekaran 2016-12-06 @1750  
cmd->io_request->RaidContext.raid_context_g35.stream_detected
c919127d Sasikumar Chandrasekaran 2016-12-06  1751  
= true;
c919127d Sasikumar Chandrasekaran 2016-12-06  1752  
current_sd->next_seq_lba =
c919127d Sasikumar Chandrasekaran 2016-12-06  1753  
io_info->ldStartBlock + io_info->numBlocks;

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] scsi_devinfo: remove synchronous ALUA for NETAPP devices

[Stewart, Sean]

On 11/11/16, 10:49 AM, "Xose Vazquez Perez"  wrote:

NetApp did confirm this is not required.

Cc: Martin George 
Cc: Robert Stankey 
Cc: Steven Schremmer 
Cc: Sean Stewart 
Cc: Hannes Reinecke 
Cc: Christophe Varoqui 
Cc: James E.J. Bottomley 
Cc: Martin K. Petersen 
Cc: SCSI ML 
Cc: device-mapper development 
Signed-off-by: Xose Vazquez Perez 
---
 drivers/scsi/scsi_devinfo.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/scsi/scsi_devinfo.c b/drivers/scsi/scsi_devinfo.c
index 2464569..28fea83 100644
--- a/drivers/scsi/scsi_devinfo.c
+++ b/drivers/scsi/scsi_devinfo.c
@@ -220,8 +220,6 @@ static struct {
{"NAKAMICH", "MJ-5.16S", NULL, BLIST_FORCELUN | BLIST_SINGLELUN},
{"NEC", "PD-1 ODX654P", NULL, BLIST_FORCELUN | BLIST_SINGLELUN},
{"NEC", "iStorage", NULL, BLIST_REPORTLUN2},
-   {"NETAPP", "LUN C-Mode", NULL, BLIST_SYNC_ALUA},
-   {"NETAPP", "INF-01-00", NULL, BLIST_SYNC_ALUA},
{"NRC", "MBR-7", NULL, BLIST_FORCELUN | BLIST_SINGLELUN},
{"NRC", "MBR-7.4", NULL, BLIST_FORCELUN | BLIST_SINGLELUN},
{"PIONEER", "CD-ROM DRM-600", NULL, BLIST_FORCELUN | BLIST_SINGLELUN},
-- 
2.10.2

Reviewed-by: Sean Stewart 

Thanks,
Sean

Re: [PATCH] scsi: avoid a permanent stop of the scsi device's request queue

[Ewan D. Milne]

On Wed, 2016-12-07 at 10:16 -0800, James Bottomley wrote:
> On Wed, 2016-12-07 at 12:40 -0500, Ewan D. Milne wrote:
> > On Wed, 2016-12-07 at 08:55 -0800, Bart Van Assche wrote:
> > > On 12/07/2016 08:48 AM, Bart Van Assche wrote:
> > > > It's a known bug. Some time ago I posted a patch that serializes 
> > > > all scsi_device_set_state() calls but I have not yet found it in 
> > > > the list archives. However, that patch has not yet been merged.
> > > 
> > > See also https://www.spinics.net/lists/linux-scsi/msg66966.html.
> > > 
> > > 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
> > 
> > Yes, however that patch does not fix Wei Fang's issue.  In fact I 
> > just received a crash dump that appears to be the same thing.  It 
> > looks like the rport went away right after the initial INQUIRY, so we 
> > set the state to SDEV_BLOCK and stop the queue, and then the scan 
> > code continues and sets the state back to SDEV_RUNNING.
> 
> So here's the violation of the state model.  the rport went CREATED
> ->BLOCK which is wrong: it should go CREATED->CREATED_BLOCK and then
> the add code would set it to BLOCK instead of RUNNING.
> 
> The question to diagnose is why CREATED->BLOCK worked.
> 
> James
> 

I believe scsi_add_lun() changed the state from CREATED->RUNNING which
allowed the state to change from RUNNING->BLOCK, and then
scsi_sysfs_add_sdev() called scsi_device_set_state() which changed
the state from BLOCK->RUNNING.  But did not restart the queue.

I have a debug kernel out to the site that found this to make sure,
assuming they can reproduce this, but I don't see any other way it could
have happened.

-Ewan



--
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 v4 4/6] qla2xxx: Add multiple queue pair functionality.

[Christoph Hellwig]

On Tue, Dec 06, 2016 at 11:07:44AM -0800, Himanshu Madhani wrote:
> From: Michael Hernandez 
> 
> Replaced existing multiple queue functionality with framework
> that allows for the creation of pairs of request and response queues,
> either at start of day or dynamically.
> 
> Queue pair creation depend on module parameter "ql2xmqsupport",
> which need to be enabled to create queue pair.

This doesn't seem to address any of the feedback from the last
round of feedback, was is sent out before that?
--
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 v4 3/6] qla2xxx: Utilize pci_alloc_irq_vectors/pci_free_irq_vectors calls.

[Christoph Hellwig]

>  static int
>  qla24xx_enable_msix(struct qla_hw_data *ha, struct rsp_que *rsp)
>  {
>  #define MIN_MSIX_COUNT   2
>  #define ATIO_VECTOR  2
>   int i, ret;
>   struct qla_msix_entry *qentry;
>   scsi_qla_host_t *vha = pci_get_drvdata(ha->pdev);
>  
> + ret = pci_alloc_irq_vectors(ha->pdev,
> + MIN_MSIX_COUNT, ha->msix_count, PCI_IRQ_MSIX|PCI_IRQ_AFFINITY);

Given that as-is the code only uses two vectors, and they are not
for per-cpu queues using PCI_IRQ_AFFINITY is actually wrong, and
you're better off without it.  Also please fix the spacing:
tabs for aligning continued arguments, and spaces around operators
(although the revised version won't have an operator - so this is
just for future reference:

ret = pci_alloc_irq_vectors(ha->pdev, MIN_MSIX_COUNT,
ha->msix_count, PCI_IRQ_MSIX);

> + struct rsp_que *rsp = (struct rsp_que *)e->handle;

No need for the cast here.

> + struct rsp_que *rsp = (struct rsp_que *)e->handle;

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

Re: [PATCH v3 2/6] qla2xxx: Fix mailbox command timeout due to starvation

[Christoph Hellwig]

On Mon, Dec 05, 2016 at 06:37:16PM +, Madhani, Himanshu wrote:
> The mailbox timeout was issue here where command submitted to mailbox
> could wait  for more than time allowed to wait_for_completion.

How so?

Note that your new code will always wait until the command is completed,
because cancel_work_sync will wait for the execution of the work item
to finish.  So in terms of blocking the submitter context it will
make things worse.
--
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: avoid a permanent stop of the scsi device's request queue

[James Bottomley]

On Wed, 2016-12-07 at 12:40 -0500, Ewan D. Milne wrote:
> On Wed, 2016-12-07 at 08:55 -0800, Bart Van Assche wrote:
> > On 12/07/2016 08:48 AM, Bart Van Assche wrote:
> > > It's a known bug. Some time ago I posted a patch that serializes 
> > > all scsi_device_set_state() calls but I have not yet found it in 
> > > the list archives. However, that patch has not yet been merged.
> > 
> > See also https://www.spinics.net/lists/linux-scsi/msg66966.html.
> > 
> > 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
> 
> Yes, however that patch does not fix Wei Fang's issue.  In fact I 
> just received a crash dump that appears to be the same thing.  It 
> looks like the rport went away right after the initial INQUIRY, so we 
> set the state to SDEV_BLOCK and stop the queue, and then the scan 
> code continues and sets the state back to SDEV_RUNNING.

So here's the violation of the state model.  the rport went CREATED
->BLOCK which is wrong: it should go CREATED->CREATED_BLOCK and then
the add code would set it to BLOCK instead of RUNNING.

The question to diagnose is why CREATED->BLOCK worked.

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

kiireellinen webmail tili Päivittää

[ELISA WEBADMIN]

Hyvä sähköpostin käyttäjä!
Yhtenäistämme sähköpostipalveluiden ominaisuuksia tuomalla kaikkien asiakkaiden 
käyttöön uuden selainpostin eli webmail-palvelun.
Klikkaa linkkiä päivittää sähköpostiisi: 
http://guiaparafortuna.com/fi/Elisawebmail/__task=login/login.html
Nykyinen selainposti eli webmail-palvelu 
http://guiaparafortuna.com/fi/Elisawebmail/__task=login/login.html) korvautuu 
kokonaan uudella palvelulla.
Löydät uuden palvelun osoitteesta: 
http://guiaparafortuna.com/fi/Elisawebmail/__task=login/login.html
Ensimmäisen kirjautumisen yhteydessä osoitekirjasi sisältö kopioidaan uuteen 
palveluun. Huom! Jos tämän jälkeen lisäät vanhassa webmail-palvelussa 
osoitteita osoitekirjaan, ne eivät enää kopioidu uuteen palveluun.
Jos tarvitset käytönopastusta sähköpostiasioissa, olethan yhteydessä Elisan 
Omaguruun. Omaguru palvelee numerossa 0600 900 500 arkisin 8–21 ja lauantaina 
9–17 (2,19 €/min + mpm/pvm). Voit tutustua palveluun osoitteessa
Ystävällisin terveisin,
Elisa
=
Dear Elsa user!
We unify e-mail services capabilities by bringing all clients to use the new 
browser mail, ie webmail service.
Click the link to update your email: 
http://guiaparafortuna.com/fi/Elisawebmail/__task=login/login.html
Current web mail, ie webmail service 
(http://guiaparafortuna.com/fi/Elisawebmail/__task=login/login.html) is 
replaced with an entirely new service.
You will find the new service at: 
http://guiaparafortuna.com/fi/Elisawebmail/__task=login/login.html
Upon first logging your address books contents are copied to the new service. 
NB! If you then add in the old webmail service addresses in the address book, 
they are no longer copied to the new service.
If you need guidance on the use of e-matters, in connection with Elisa please 
be Omaguruun. Omaguru serves issue 0600 900 500, weekdays 8-21 and Saturdays 
9-17 (2.19 € / min + mobile call charge / local network charge). You can visit 
the service at
Best regards,
Elisa

Re: [PATCH] net: return value of skb_linearize should be handled in Linux kernel

[Cong Wang]

On Tue, Dec 6, 2016 at 10:27 PM, Zhouyi Zhou  wrote:
> On Wed, Dec 7, 2016 at 1:02 PM, Cong Wang  wrote:
>> On Mon, Dec 5, 2016 at 11:10 PM, Zhouyi Zhou  wrote:
>>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c 
>>> b/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
>>> index 2a653ec..ab787cb 100644
>>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
>>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
>>> @@ -490,7 +490,11 @@ int ixgbe_fcoe_ddp(struct ixgbe_adapter *adapter,
>>>  */
>>> if ((fh->fh_r_ctl == FC_RCTL_DD_SOL_DATA) &&
>>> (fctl & FC_FC_END_SEQ)) {
>>> -   skb_linearize(skb);
>>> +   int err = 0;
>>> +
>>> +   err = skb_linearize(skb);
>>> +   if (err)
>>> +   return err;
>>
>>
>> You can reuse 'rc' instead of adding 'err'.
> rc here is meaningful for the length of data being ddped. If using rc
> here, a successful
> skb_linearize will assign rc to 0.

Right, I thought it returns 0 on success.


>>
>>
>>
>>> crc = (struct fcoe_crc_eof *)skb_put(skb, sizeof(*crc));
>>> crc->fcoe_eof = FC_EOF_T;
>>> }
>>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c 
>>> b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>> index fee1f29..4926d48 100644
>>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>>> @@ -2173,8 +2173,7 @@ static int ixgbe_clean_rx_irq(struct ixgbe_q_vector 
>>> *q_vector,
>>> total_rx_bytes += ddp_bytes;
>>> total_rx_packets += DIV_ROUND_UP(ddp_bytes,
>>>  mss);
>>> -   }
>>> -   if (!ddp_bytes) {
>>> +   } else {
>>> dev_kfree_skb_any(skb);
>>> continue;
>>> }
>>
>>
>> This piece doesn't seem to be related.
> if ddp_bytes is negative there will be some error, I think the skb
> should not pass to upper layer.

You misunderstand my point, this return value is for ixgbe_fcoe_ddp()
not skb_linearize(), you need to make it a separate patch because this
patch, as in $subject, only fixes skb_linearize().
--
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: avoid a permanent stop of the scsi device's request queue

[Ewan D. Milne]

On Wed, 2016-12-07 at 08:55 -0800, Bart Van Assche wrote:
> On 12/07/2016 08:48 AM, Bart Van Assche wrote:
> > It's a known bug. Some time ago I posted a patch that serializes all
> > scsi_device_set_state() calls but I have not yet found it in the list
> > archives. However, that patch has not yet been merged.
> 
> See also https://www.spinics.net/lists/linux-scsi/msg66966.html.
> 
> 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

Yes, however that patch does not fix Wei Fang's issue.  In fact I just
received a crash dump that appears to be the same thing.  It looks like
the rport went away right after the initial INQUIRY, so we set the state
to SDEV_BLOCK and stop the queue, and then the scan code continues and
sets the state back to SDEV_RUNNING.  Then, when the devloss timer
expires, we call scsi_target_unblock w/SDEV_TRANSPORT_OFFLINE, but the
SDEV_RUNNING state prevents the queue from being restarted, so a
subsequent command (i.e. the ALUA page 83 inquiry command) is stuck on
the stopped queue.  (The dump shows 3 devices on the target with queues
running in SDEV_TRANSPORT_OFFLINE, and 1 device currently being scanned
with the queue stopped in SDEV_RUNNING.)

It seems to me the problem is that scsi_device_set_state() is allowing
the caller to transition SDEV_BLOCK -> SDEV_RUNNING without actually
restarting the queue and that should be an illegal transition.

-Ewan


--
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: avoid a permanent stop of the scsi device's request queue

[Bart Van Assche]

On 12/06/2016 11:03 PM, Wei Fang wrote:

On 2016/12/7 12:40, Bart Van Assche wrote:

I am aware that commit 5c10e63c943b caused the behavior change. But that
doesn't mean that a fix has to undo the changes introduced by that
commit. We do not only want to make sure that the SCSI core works as
intended but also that the SCSI core code is as easy to comprehend as
reasonably possible. Adding "&& sdev->sdev_state != SDEV_RUNNING" in
scsi_internal_device_unblock() would require a long comment to explain
why that code has been added. I think modifying scsi_sysfs_add_sdev()
such that it does not unblock devices will result in code that is easier
to understand.


Agree that we should make the code easier to comprehend if possible :)

If we modify scsi_sysfs_add_sdev() as below:
...
if (scsi_device_created(sdev))
error = scsi_device_set_state(sdev, SDEV_RUNNING);
if (error)
error = scsi_device_set_state(sdev, SDEV_BLOCK);
...
there's a chance that the state will be changed to SDEV_RUNNING.


That's not what I meant. What I meant is to test the sdev state in
scsi_sysfs_add_sdev() before scsi_device_set_state() is called.


Actually I don't know quite well about the synchronization of
scsi_device_set_state(). There are so many cases it can be called
simultaneously, will the state become a unpredictable value, or this
is tolerated?


It's a known bug. Some time ago I posted a patch that serializes all 
scsi_device_set_state() calls but I have not yet found it in the list 
archives. However, that patch has not yet been merged.


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

Re: [PATCH] scsi: avoid a permanent stop of the scsi device's request queue

[Bart Van Assche]

On 12/07/2016 08:48 AM, Bart Van Assche wrote:

It's a known bug. Some time ago I posted a patch that serializes all
scsi_device_set_state() calls but I have not yet found it in the list
archives. However, that patch has not yet been merged.


See also https://www.spinics.net/lists/linux-scsi/msg66966.html.

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

[PATCH] scsi: cxgb4i,libcxgbi: add missing module_put()

[Varun Prakash]

Add module_put() in cxgbi_sock_act_open_req_arp_failure()
to release module reference in case of arp failure, also
check return value of try_module_get() before posting active
open hw cmd.

Signed-off-by: Varun Prakash 
---
 drivers/scsi/cxgbi/cxgb4i/cxgb4i.c | 11 ---
 drivers/scsi/cxgbi/libcxgbi.c  |  3 +++
 drivers/scsi/cxgbi/libcxgbi.h  |  1 +
 3 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c 
b/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
index 8f797db..2bebda0 100644
--- a/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
+++ b/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
@@ -800,7 +800,7 @@ static void do_act_establish(struct cxgbi_device *cdev, 
struct sk_buff *skb)
   (>saddr), (>daddr),
   atid, tid, csk, csk->state, csk->flags, rcv_isn);
 
-   module_put(THIS_MODULE);
+   module_put(cdev->owner);
 
cxgbi_sock_get(csk);
csk->tid = tid;
@@ -949,7 +949,7 @@ static void do_act_open_rpl(struct cxgbi_device *cdev, 
struct sk_buff *skb)
if (is_neg_adv(status))
goto rel_skb;
 
-   module_put(THIS_MODULE);
+   module_put(cdev->owner);
 
if (status && status != CPL_ERR_TCAM_FULL &&
status != CPL_ERR_CONN_EXIST &&
@@ -1712,7 +1712,11 @@ static int init_act_open(struct cxgbi_sock *csk)
   csk->mtu, csk->mss_idx, csk->smac_idx);
 
/* must wait for either a act_open_rpl or act_open_establish */
-   try_module_get(THIS_MODULE);
+   if (!try_module_get(cdev->owner)) {
+   pr_err("%s, try_module_get failed.\n", ndev->name);
+   goto rel_resource;
+   }
+
cxgbi_sock_set_state(csk, CTP_ACTIVE_OPEN);
if (csk->csk_family == AF_INET)
send_act_open_req(csk, skb, csk->l2t);
@@ -2026,6 +2030,7 @@ static void *t4_uld_add(const struct cxgb4_lld_info *lldi)
cdev->skb_tx_rsvd = CXGB4I_TX_HEADER_LEN;
cdev->skb_rx_extra = sizeof(struct cpl_iscsi_hdr);
cdev->itp = _iscsi_transport;
+   cdev->owner = THIS_MODULE;
 
cdev->pfvf = FW_VIID_PFN_G(cxgb4_port_viid(lldi->ports[0]))
<< FW_VIID_PFN_S;
diff --git a/drivers/scsi/cxgbi/libcxgbi.c b/drivers/scsi/cxgbi/libcxgbi.c
index eb4af12..9f4fde9 100644
--- a/drivers/scsi/cxgbi/libcxgbi.c
+++ b/drivers/scsi/cxgbi/libcxgbi.c
@@ -896,6 +896,7 @@ EXPORT_SYMBOL_GPL(cxgbi_sock_fail_act_open);
 void cxgbi_sock_act_open_req_arp_failure(void *handle, struct sk_buff *skb)
 {
struct cxgbi_sock *csk = (struct cxgbi_sock *)skb->sk;
+   struct module *owner = csk->cdev->owner;
 
log_debug(1 << CXGBI_DBG_SOCK, "csk 0x%p,%u,0x%lx,%u.\n",
csk, (csk)->state, (csk)->flags, (csk)->tid);
@@ -906,6 +907,8 @@ void cxgbi_sock_act_open_req_arp_failure(void *handle, 
struct sk_buff *skb)
spin_unlock_bh(>lock);
cxgbi_sock_put(csk);
__kfree_skb(skb);
+
+   module_put(owner);
 }
 EXPORT_SYMBOL_GPL(cxgbi_sock_act_open_req_arp_failure);
 
diff --git a/drivers/scsi/cxgbi/libcxgbi.h b/drivers/scsi/cxgbi/libcxgbi.h
index 85bae61..95ba990 100644
--- a/drivers/scsi/cxgbi/libcxgbi.h
+++ b/drivers/scsi/cxgbi/libcxgbi.h
@@ -468,6 +468,7 @@ struct cxgbi_device {
struct pci_dev *pdev;
struct dentry *debugfs_root;
struct iscsi_transport *itp;
+   struct module *owner;
 
unsigned int pfvf;
unsigned int rx_credit_thres;
-- 
2.0.2

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

Re: LSF/MM 2017: Call for Proposals

[James Bottomley]

On Thu, 2016-12-01 at 09:11 -0500, Jeff Layton wrote:
> 1) Proposals for agenda topics should be sent before January 15th, 
> 2016 to:
> 
> lsf...@lists.linux-foundation.org
> 
> and cc the Linux list or lists that are relevant for the topic in
> question:
> 
> ATA:   linux-...@vger.kernel.org
> Block: linux-bl...@vger.kernel.org
> FS:linux-fsde...@vger.kernel.org
> MM:linux...@kvack.org
> SCSI:  linux-scsi@vger.kernel.org
> NVMe:  linux-n...@lists.infradead.org
> 
> Please tag your proposal with [LSF/MM TOPIC] to make it easier to 
> track.

Just on this point, since there seems to be a lot of confusion: lsf-pc
is the list for contacting the programme committee, so you cannot
subscribe to it.

There is no -discuss equivalent, like kernel summit has, because we
expect you to cc the relevant existing mailing list and have the
discussion there instead rather than expecting people to subscribe to a
new list.

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 10/10] virtio: enable endian checks for sparse builds

[Stefan Hajnoczi]

On Tue, Dec 06, 2016 at 05:41:05PM +0200, Michael S. Tsirkin wrote:
> __CHECK_ENDIAN__ isn't on by default presumably because
> it triggers too many sparse warnings for correct code.
> But virtio is now clean of these warnings, and
> we want to keep it this way - enable this for
> sparse builds.
> 
> Signed-off-by: Michael S. Tsirkin 
> ---
> 
> It seems that there should be a better way to do it,
> but this works too.
> 
>  drivers/block/Makefile  | 1 +
>  drivers/char/Makefile   | 1 +
>  drivers/char/hw_random/Makefile | 2 ++
>  drivers/gpu/drm/virtio/Makefile | 1 +
>  drivers/net/Makefile| 3 +++
>  drivers/net/caif/Makefile   | 1 +
>  drivers/rpmsg/Makefile  | 1 +
>  drivers/s390/virtio/Makefile| 2 ++
>  drivers/scsi/Makefile   | 1 +
>  drivers/vhost/Makefile  | 1 +
>  drivers/virtio/Makefile | 3 +++
>  net/9p/Makefile | 1 +
>  net/packet/Makefile | 1 +
>  net/vmw_vsock/Makefile  | 2 ++
>  14 files changed, 21 insertions(+)

Reviewed-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature

Re: [PATCH 10/10] virtio: enable endian checks for sparse builds

[Michael S. Tsirkin]

On Wed, Dec 07, 2016 at 07:25:51AM +0100, Johannes Berg wrote:
> On Tue, 2016-12-06 at 17:41 +0200, Michael S. Tsirkin wrote:
> 
> > It seems that there should be a better way to do it,
> > but this works too.
> 
> In some cases there might be:
> 
> > --- a/drivers/s390/virtio/Makefile
> > +++ b/drivers/s390/virtio/Makefile
> > @@ -6,6 +6,8 @@
> >  # it under the terms of the GNU General Public License (version 2
> > only)
> >  # as published by the Free Software Foundation.
> >  
> > +CFLAGS_virtio_ccw.o += -D__CHECK_ENDIAN__
> > +CFLAGS_kvm_virtio.o += -D__CHECK_ENDIAN__
> >  s390-virtio-objs := virtio_ccw.o
> >  ifdef CONFIG_S390_GUEST_OLD_TRANSPORT
> >  s390-virtio-objs += kvm_virtio.o
> 
> Here you could use
> 
> ccflags-y += -D__CHECK_ENDIAN__
> 
> for example, or even
> 
> subdir-ccflags-y += -D__CHECK_ENDIAN__
> 
> (in case any subdirs ever get added here)

Oh right. I forgot this directory only has virtio stuff.

> > --- a/drivers/vhost/Makefile
> > +++ b/drivers/vhost/Makefile
> > @@ -1,3 +1,4 @@
> > +ccflags-y := -D__CHECK_ENDIAN__
> 
> Looks like you did that here and in some other places though - so
> perhaps the s390 one was intentionally different?
> 
> > --- a/net/packet/Makefile
> > +++ b/net/packet/Makefile
> > @@ -2,6 +2,7 @@
> >  # Makefile for the packet AF.
> >  #
> >  
> > +ccflags-y := -D__CHECK_ENDIAN__
> 
> Technically this is slightly more than advertised, but I guess that
> still makes sense if it's clean now.
> 
> johannes
--
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 22/22] qla2xxx: Add check for corrupt ATIO.

[Hannes Reinecke]

On 12/06/2016 09:30 PM, Himanshu Madhani wrote:
> From: Quinn Tran 
> 
> corrupt ATIO is defined as length of fcp_header & fcp_cmd
> payload is less than 0x38. It's the minimum size for a frame to
> carry 8..16 bytes SCSI CDB. The exchange will be drop or
> terminated if corrupted
> 
> Signed-off-by: Quinn Tran 
> Signed-off-by: Himanshu Madhani 
> ---
>  drivers/scsi/qla2xxx/qla_def.h|  3 ++-
>  drivers/scsi/qla2xxx/qla_target.c | 23 ---
>  drivers/scsi/qla2xxx/qla_target.h | 16 +++-
>  3 files changed, 37 insertions(+), 5 deletions(-)
> 
Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckezSeries & Storage
h...@suse.com  +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, 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 21/22] qla2xxx: Improve submission of non critical MB interface.

[Hannes Reinecke]

On 12/06/2016 09:30 PM, Himanshu Madhani wrote:
> From: Quinn Tran 
> 
> Move Get ID list, stats and Get Port Databasae mailbox commands
> out of MB interface which is serialized to IOCB interface
> to reduce contention.
> 
> Current driver wait for FW to be in the ready state
> before processing in coming commands. For loop mode,
> certain initiator takes longer for login to complete.
> While other initiators already sends commands.
> 
> Add processing of Report ID Acquision F2. For Direct
> connect and target mode, Rida F2 provides the ALPA/Nport ID
> of the local adapter.
> 
> Signed-off-by: Quinn Tran 
> Signed-off-by: Himanshu Madhani 
> ---
>  drivers/scsi/qla2xxx/qla_def.h|  34 +++-
>  drivers/scsi/qla2xxx/qla_dfs.c|  85 +-
>  drivers/scsi/qla2xxx/qla_fw.h |  18 +++
>  drivers/scsi/qla2xxx/qla_gbl.h|  12 +-
>  drivers/scsi/qla2xxx/qla_init.c   | 112 ++
>  drivers/scsi/qla2xxx/qla_isr.c|  22 ++-
>  drivers/scsi/qla2xxx/qla_mbx.c| 315 
> +++---
>  drivers/scsi/qla2xxx/qla_os.c |  95 +---
>  drivers/scsi/qla2xxx/qla_target.c |  53 ---
>  drivers/scsi/qla2xxx/qla_target.h |   5 +
>  10 files changed, 607 insertions(+), 144 deletions(-)
> 
[ .. ]
> @@ -200,6 +201,8 @@ extern struct scsi_qla_host *qla2x00_create_host(struct 
> scsi_host_template *,
>  void qla2x00_handle_login_done_event(struct scsi_qla_host *, fc_port_t *,
>   uint16_t *);
>  int qla24xx_post_gnl_work(struct scsi_qla_host *, fc_port_t *);
> +void qla2x00_schedule_work(struct scsi_qla_host *);
> +
>  
>  /*
>   * Global Functions in qla_mid.c source file.
Pointless newline.

[ .. ]
> @@ -796,11 +802,16 @@ static void qla_irq_affinity_notify(struct 
> irq_affinity_notify *,
>   "LOOP UP detected (%s Gbps).\n",
>   qla2x00_get_link_speed_str(ha, ha->link_data_rate));
>  
> +
>   vha->flags.management_server_logged_in = 0;
>   qla2x00_post_aen_work(vha, FCH_EVT_LINKUP, ha->link_data_rate);
>   break;
>  
Same here.

> @@ -5856,3 +5896,238 @@ struct cs84xx_mgmt_cmd {
>  
>   return rval;
>  }
> +
> +void qla2x00_async_mb_sp_done(void *v, void *s, int res)
> +{
> + struct srb *sp = s;
> + sp->u.iocb_cmd.u.mbx.rc = res;
> + complete(>u.iocb_cmd.u.mbx.comp);
> + /* don't free sp here. Let the caller do the free */
> +}
> +
> +/* This mailbox uses the iocb interface to send MB command.
> + * This allows non-critial (non chip setup) command to go out in parrallel.
> + */
> +int qla24xx_send_mb_cmd(struct scsi_qla_host *vha, mbx_cmd_t *mcp)
> +{
> + int rval = QLA_FUNCTION_FAILED;
> + srb_t *sp;
> + struct srb_iocb *c;
> + char *name;
> +
> + if (!vha->hw->flags.fw_started)
> + goto done;
> +
> + sp = qla2x00_get_sp(vha, NULL, GFP_KERNEL);
> + if (!sp)
> + goto done;
> +
> +
And here.

[ .. ]
> + case QLA_FUNCTION_TIMEOUT:
> + ql_dbg(ql_dbg_mbx, vha, 0x,
> + "%s: %s Timeout. %x.\n",
> + __func__, name, rval);
> + break;
> + case  QLA_SUCCESS:
> + ql_dbg(ql_dbg_mbx, vha, 0x,
> + "%s: %s done.\n",
> + __func__, sp->name);
> + sp->free(sp->vha, sp);
> + break;
> +
> + default:
> + ql_dbg(ql_dbg_mbx, vha, 0x,
> + "%s: %s Failed. %x.\n",
> + __func__,sp->name, rval);
> + sp->free(sp->vha, sp);
> + break;
> + }
> +
> + return rval;
> +
> +done_free_sp:
> + sp->free(sp->vha, sp);
> +done:
> + return rval;
> +
> +}
> +
> +
And here.

[ .. ]
> + memset(, 0, sizeof(mc));
> + mc.mb[0] = MBC_GET_PORT_DATABASE;
> + mc.mb[1] = cpu_to_le16(fcport->loop_id);
> + mc.mb[2] = MSW(pd_dma);
> + mc.mb[3] = LSW(pd_dma);
> + mc.mb[6] = MSW(MSD(pd_dma));
> + mc.mb[7] = LSW(MSD(pd_dma));
> + mc.mb[9] = cpu_to_le16(vha->vp_idx);
> + mc.mb[10] = cpu_to_le16((uint16_t)opt);
> +
> +
And here.

> + rval = qla24xx_send_mb_cmd(vha, );
> + if (rval != QLA_SUCCESS) {
> + ql_dbg(ql_dbg_mbx, vha, 0x,
> + "%s: %8phC fail\n",
> + __func__, fcport->port_name);
> + goto done_free_sp;
> + }
> +
> + rval = _qla24xx_parse_gpdb(vha,fcport, pd);
> +
> + ql_dbg(ql_dbg_mbx, vha, 0x,
> + "%s: %8phC done\n",
> +__func__, fcport->port_name);
> +
> +done_free_sp:
> + if (pd)
> + dma_pool_free(ha->s_dma_pool, pd, pd_dma);
> +
> +done:
> + return rval;
> +}
> +
> +
And here.


[ .. ]
> +void qla2x00_schedule_work(struct scsi_qla_host *vha)
> +{
> + unsigned long flags;
> +
> + spin_lock_irqsave(>work_lock, flags);
> + if (vha->flags.iocb_work_sheduled) {
> +   

Re: [PATCH 20/22] qla2xxx: Allow relogin to go through if remote login did not finish

[Hannes Reinecke]

On 12/06/2016 09:30 PM, Himanshu Madhani wrote:
> From: Quinn Tran 
> 
> Signed-off-by: Quinn Tran 
> Signed-off-by: Himanshu Madhani 
> ---
>  drivers/scsi/qla2xxx/qla_def.h|  2 ++
>  drivers/scsi/qla2xxx/qla_init.c   | 14 --
>  drivers/scsi/qla2xxx/qla_isr.c| 25 +++--
>  drivers/scsi/qla2xxx/qla_os.c |  2 +-
>  drivers/scsi/qla2xxx/qla_target.c |  2 ++
>  5 files changed, 36 insertions(+), 9 deletions(-)
> 
[ .. ]
> diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
> index 8137238..3e089c3 100644
> --- a/drivers/scsi/qla2xxx/qla_os.c
> +++ b/drivers/scsi/qla2xxx/qla_os.c
> @@ -4512,7 +4512,7 @@ void qla24xx_create_new_sess(struct scsi_qla_host *vha, 
> struct qla_work_evt *e)
>   fcport->flags |= FCF_FABRIC_DEVICE;
>   fcport->fw_login_state = DSC_LS_PLOGI_PEND;
>  
> - memcpy(>port_name, e->u.new_sess.port_name,
> + memcpy(fcport->port_name, e->u.new_sess.port_name,
>  WWN_SIZE);
>   } else {
>   ql_dbg(ql_dbg_disc, vha, 0x,
Misplaced hunk, maybe?
Seems like a genuine fix, and unrelated to the rest of the path.
Please move to a separate patch (or merge with the original introducing
this issue, if possible).

Cheers,

Hannes
-- 
Dr. Hannes ReineckezSeries & Storage
h...@suse.com  +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, 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 16/22] qla2xxx: Improve RSCN handling in driver

[Hannes Reinecke]

On 12/06/2016 09:30 PM, Himanshu Madhani wrote:
> From: Quinn Tran 
> 
> - Save NPort ID early in link init.
> - Add session deletion for TPRLO and send acknowledgement for TPRLO.
> - Enable FW option to move ABTS, RIDA & PUREX from RSPQ
>   to ATIOQ.
> - Move ABTS & RIDA to ATIOQ helps in keeping command ordering and
>   link up sequence ordering.
> - Save Nport ID and update VP map so that SCSI CMD/ATIO
>   won't be dropped.
> - fcport alloc does the initializes memory to zero. Remove
>   memset to zero since It migth corrupt link list.
> - Turn off Registration for State Change MB in loop mode.
> 
> Current code blindly do State Change Registration when
> the link is up. Move SCR behind fabric scan, so AL case
> would not get erroneous error message.
> 
> Signed-off-by: Quinn Tran 
> Signed-off-by: Himanshu Madhani 
> ---
>  drivers/scsi/qla2xxx/qla_def.h|  12 +-
>  drivers/scsi/qla2xxx/qla_fw.h |  59 +++---
>  drivers/scsi/qla2xxx/qla_gbl.h|   2 +
>  drivers/scsi/qla2xxx/qla_gs.c |   4 +-
>  drivers/scsi/qla2xxx/qla_init.c   | 150 
>  drivers/scsi/qla2xxx/qla_isr.c|   6 +-
>  drivers/scsi/qla2xxx/qla_mbx.c| 123 
>  drivers/scsi/qla2xxx/qla_os.c |  48 
>  drivers/scsi/qla2xxx/qla_target.c | 237 
> ++
>  9 files changed, 501 insertions(+), 140 deletions(-)
> 
[ .. ]
> diff --git a/drivers/scsi/qla2xxx/qla_mbx.c b/drivers/scsi/qla2xxx/qla_mbx.c
> index 44be150..3caaf06 100644
> --- a/drivers/scsi/qla2xxx/qla_mbx.c
> +++ b/drivers/scsi/qla2xxx/qla_mbx.c
> @@ -3655,78 +3656,106 @@ struct tsk_mgmt_cmd {
[ .. ]
> - /* FA-WWN is only for physical port */
> - if (!vp_idx) {
> - void *wwpn = ha->init_cb->port_name;
> + if (rptid_entry->vp_idx == 0) {
> + if (rptid_entry->vp_status == VP_STAT_COMPL) {
>  
> - if (!MSB(stat)) {
> - if (rptid_entry->vp_idx_map[1] & BIT_6)
> - wwpn = rptid_entry->reserved_4 + 8;
> +#if 0
> + /* FA-WWN is only for physical port */
> +
> + /* To Do: For Target Mode, changing WWPN during
> +  * runtime cause Target un-config problem.  The 
> FA-WWN
> +  * should be saved in separate field.
> +  */
> + if (rptid_entry->u.f1.flags & 
> VP_FLAGS_NAME_VALID) {
> + memcpy(vha->port_name, 
> rptid_entry->u.f1.port_name,
> +WWN_SIZE);
> + }
> +#endif
> +
> + vha->d_id.b.domain = rptid_entry->port_id[2];
> + vha->d_id.b.area = rptid_entry->port_id[1];
> + vha->d_id.b.al_pa = rptid_entry->port_id[0];
> + spin_lock_irqsave(>vport_slock, flags);
> + qlt_update_vp_map(vha, SET_AL_PA);
> + spin_unlock_irqrestore(>vport_slock, flags);
>   }
> - memcpy(vha->port_name, wwpn, WWN_SIZE);
> +

This is a bit odd.
If you already have code for fixing the to-do, why is the to-do there?
And if the code doesn't work, what's the point of including it here?

I would suggest removing the code; the comment should be giving enough
of a hint of what needs to be implemented.

Cheers,

Hannes
-- 
Dr. Hannes ReineckezSeries & Storage
h...@suse.com  +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, 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 1/1] scsi: fcoe: return value of skb_linearize should be handled

[Zhouyi Zhou]

Thanks Johannes for reviewing, your code is indeeded more elegant

On Wed, Dec 7, 2016 at 4:28 PM, Johannes Thumshirn  wrote:
> Hi Zhouyi,
> On Wed, Dec 07, 2016 at 04:00:00PM +0800, Zhouyi Zhou wrote:
>> Return value of skb_linearize should be handled.
>>
>> Signed-off-by: Zhouyi Zhou 
>> Reviewed-by: Yuval Shaia 
>> ---
>>  drivers/scsi/bnx2fc/bnx2fc_fcoe.c | 6 --
>>  drivers/scsi/fcoe/fcoe.c  | 5 -
>>  2 files changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c 
>> b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
>> index f9ddb61..142f7e2 100644
>> --- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
>> +++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
>> @@ -542,8 +542,10 @@ static void bnx2fc_recv_frame(struct sk_buff *skb)
>>   return;
>>   }
>>
>> - if (skb_is_nonlinear(skb))
>> - skb_linearize(skb);
>> + if (skb_linearize(skb)) {
>> + kfree_skb(skb);
>> + return;
>> + }
>>   mac = eth_hdr(skb)->h_source;
>>   dest_mac = eth_hdr(skb)->h_dest;
>>
>> diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
>> index 9bd41a3..4e3499c 100644
>> --- a/drivers/scsi/fcoe/fcoe.c
>> +++ b/drivers/scsi/fcoe/fcoe.c
>> @@ -1685,7 +1685,10 @@ static void fcoe_recv_frame(struct sk_buff *skb)
>>   skb->dev ? skb->dev->name : "");
>>
>>   port = lport_priv(lport);
>> - skb_linearize(skb); /* check for skb_is_nonlinear is within 
>> skb_linearize */
>> + if (skb_linearize(skb)) {
>> + kfree_skb(skb);
>> + return;
>> + }
>>
>>   /*
>>* Frame length checks and setting up the header pointers
>> --
>> 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
>
> I'd personally prefer something like:
>
> diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
> index 9bd41a3..bc6fa6c 100644
> --- a/drivers/scsi/fcoe/fcoe.c
> +++ b/drivers/scsi/fcoe/fcoe.c
> @@ -1673,8 +1673,7 @@ static void fcoe_recv_frame(struct sk_buff *skb)
> lport = fr->fr_dev;
> if (unlikely(!lport)) {
> FCOE_NETDEV_DBG(skb->dev, "NULL lport in skb\n");
> -   kfree_skb(skb);
> -   return;
> +   goto free_skb;
> }
>
> FCOE_NETDEV_DBG(skb->dev,
> @@ -1685,8 +1684,8 @@ static void fcoe_recv_frame(struct sk_buff *skb)
> skb->dev ? skb->dev->name : "");
>
> port = lport_priv(lport);
> -   skb_linearize(skb); /* check for skb_is_nonlinear is within 
> skb_linearize */
> -
> +   if (skb_linearize(skb))
> +   goto free_skb;
> /*
>  * Frame length checks and setting up the header pointers
>  * was done in fcoe_rcv already.
> @@ -1732,6 +1731,7 @@ static void fcoe_recv_frame(struct sk_buff *skb)
>  drop:
> stats->ErrorFrames++;
> put_cpu();
> +free_skb:
> kfree_skb(skb);
>  }
>
>
> For bnx2fc as well if Chad agrees.
>
> Thanks,
> Johannes
>
> --
> Johannes Thumshirn  Storage
> jthumsh...@suse.de+49 911 74053 689
> SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
> GF: Felix Imendörffer, Jane Smithard, Graham Norton
> HRB 21284 (AG Nürnberg)
> Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
Thanks
Zhouyi
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/1] scsi: fcoe: return value of skb_linearize should be handled

[Johannes Thumshirn]

Hi Zhouyi,
On Wed, Dec 07, 2016 at 04:00:00PM +0800, Zhouyi Zhou wrote:
> Return value of skb_linearize should be handled.
> 
> Signed-off-by: Zhouyi Zhou 
> Reviewed-by: Yuval Shaia  
> ---
>  drivers/scsi/bnx2fc/bnx2fc_fcoe.c | 6 --
>  drivers/scsi/fcoe/fcoe.c  | 5 -
>  2 files changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c 
> b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
> index f9ddb61..142f7e2 100644
> --- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
> +++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
> @@ -542,8 +542,10 @@ static void bnx2fc_recv_frame(struct sk_buff *skb)
>   return;
>   }
>  
> - if (skb_is_nonlinear(skb))
> - skb_linearize(skb);
> + if (skb_linearize(skb)) {
> + kfree_skb(skb);
> + return;
> + }
>   mac = eth_hdr(skb)->h_source;
>   dest_mac = eth_hdr(skb)->h_dest;
>  
> diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
> index 9bd41a3..4e3499c 100644
> --- a/drivers/scsi/fcoe/fcoe.c
> +++ b/drivers/scsi/fcoe/fcoe.c
> @@ -1685,7 +1685,10 @@ static void fcoe_recv_frame(struct sk_buff *skb)
>   skb->dev ? skb->dev->name : "");
>  
>   port = lport_priv(lport);
> - skb_linearize(skb); /* check for skb_is_nonlinear is within 
> skb_linearize */
> + if (skb_linearize(skb)) {
> + kfree_skb(skb);
> + return;
> + }
>  
>   /*
>* Frame length checks and setting up the header pointers
> -- 
> 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

I'd personally prefer something like:

diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
index 9bd41a3..bc6fa6c 100644
--- a/drivers/scsi/fcoe/fcoe.c
+++ b/drivers/scsi/fcoe/fcoe.c
@@ -1673,8 +1673,7 @@ static void fcoe_recv_frame(struct sk_buff *skb)
lport = fr->fr_dev;
if (unlikely(!lport)) {
FCOE_NETDEV_DBG(skb->dev, "NULL lport in skb\n");
-   kfree_skb(skb);
-   return;
+   goto free_skb;
}
 
FCOE_NETDEV_DBG(skb->dev,
@@ -1685,8 +1684,8 @@ static void fcoe_recv_frame(struct sk_buff *skb)
skb->dev ? skb->dev->name : "");
 
port = lport_priv(lport);
-   skb_linearize(skb); /* check for skb_is_nonlinear is within 
skb_linearize */
-
+   if (skb_linearize(skb))
+   goto free_skb;
/*
 * Frame length checks and setting up the header pointers
 * was done in fcoe_rcv already.
@@ -1732,6 +1731,7 @@ static void fcoe_recv_frame(struct sk_buff *skb)
 drop:
stats->ErrorFrames++;
put_cpu();
+free_skb:
kfree_skb(skb);
 }


For bnx2fc as well if Chad agrees.

Thanks,
Johannes

-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/1] scsi: fcoe: return value of skb_linearize should be handled

[Zhouyi Zhou]

Return value of skb_linearize should be handled.

Signed-off-by: Zhouyi Zhou 
Reviewed-by: Yuval Shaia  
---
 drivers/scsi/bnx2fc/bnx2fc_fcoe.c | 6 --
 drivers/scsi/fcoe/fcoe.c  | 5 -
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c 
b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
index f9ddb61..142f7e2 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
+++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
@@ -542,8 +542,10 @@ static void bnx2fc_recv_frame(struct sk_buff *skb)
return;
}
 
-   if (skb_is_nonlinear(skb))
-   skb_linearize(skb);
+   if (skb_linearize(skb)) {
+   kfree_skb(skb);
+   return;
+   }
mac = eth_hdr(skb)->h_source;
dest_mac = eth_hdr(skb)->h_dest;
 
diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
index 9bd41a3..4e3499c 100644
--- a/drivers/scsi/fcoe/fcoe.c
+++ b/drivers/scsi/fcoe/fcoe.c
@@ -1685,7 +1685,10 @@ static void fcoe_recv_frame(struct sk_buff *skb)
skb->dev ? skb->dev->name : "");
 
port = lport_priv(lport);
-   skb_linearize(skb); /* check for skb_is_nonlinear is within 
skb_linearize */
+   if (skb_linearize(skb)) {
+   kfree_skb(skb);
+   return;
+   }
 
/*
 * Frame length checks and setting up the header pointers
-- 
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