linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* version 3.18.44 to 3.18.45 introduced a bug in "drivers/scsi/megaraid/megaraid_sas_base.c"
@ 2016-12-13 10:56 Dashi DS1 Cao
  2016-12-13 16:08 ` Randy Dunlap
  0 siblings, 1 reply; 7+ messages in thread
From: Dashi DS1 Cao @ 2016-12-13 10:56 UTC (permalink / raw)
  To: linux-kernel

--- a/drivers/scsi/megaraid/megaraid_sas_base.c
+++ b/drivers/scsi/megaraid/megaraid_sas_base.c
@@ -1614,16 +1614,13 @@ megasas_queue_command(struct Scsi_Host *shost, struct scsi_cmnd *scmd)
                goto out_done;
        }

-       switch (scmd->cmnd[0]) {
-       case SYNCHRONIZE_CACHE:
-               /*
-                * FW takes care of flush cache on its own
-                * No need to send it down
-                */
+       /*
+        * FW takes care of flush cache on its own for Virtual Disk.
+        * No need to send it down for VD. For JBOD send SYNCHRONIZE_CACHE to FW.
+        */
+       if ((scmd->cmnd[0] == SYNCHRONIZE_CACHE) && MEGASAS_IS_LOGICAL(scmd)) {
                scmd->result = DID_OK << 16;
                goto out_done;
-       default:
-               break;
        }

        if (instance->instancet->build_and_issue_cmd(instance, scmd)) {

MEGASAS_IS_LOGICAL is defined to be a macro with '?' operator, which has a lower precedence than '&&'.
The macro should have been defined as:
--- a/drivers/scsi/megaraid/megaraid_sas.h
+++ b/drivers/scsi/megaraid/megaraid_sas.h
@@ -1823,7 +1823,7 @@ struct megasas_instance_template {
 };

 #define MEGASAS_IS_LOGICAL(scp)                                                \
-       (scp->device->channel < MEGASAS_MAX_PD_CHANNELS) ? 0 : 1
+       ((scp->device->channel < MEGASAS_MAX_PD_CHANNELS) ? 0 : 1)

 #define MEGASAS_DEV_INDEX(inst, scp)                                   \
        ((scp->device->channel % 2) * MEGASAS_MAX_DEV_PER_CHANNEL) +    \

Dashi Cao

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: version 3.18.44 to 3.18.45 introduced a bug in "drivers/scsi/megaraid/megaraid_sas_base.c"
  2016-12-13 10:56 version 3.18.44 to 3.18.45 introduced a bug in "drivers/scsi/megaraid/megaraid_sas_base.c" Dashi DS1 Cao
@ 2016-12-13 16:08 ` Randy Dunlap
  2016-12-13 16:30   ` Greg Kroah-Hartman
  0 siblings, 1 reply; 7+ messages in thread
From: Randy Dunlap @ 2016-12-13 16:08 UTC (permalink / raw)
  To: Dashi DS1 Cao, linux-kernel, linux-scsi, Greg Kroah-Hartman, stable

[adding other lists + gregkh]


On 12/13/16 02:56, Dashi DS1 Cao wrote:
> --- a/drivers/scsi/megaraid/megaraid_sas_base.c
> +++ b/drivers/scsi/megaraid/megaraid_sas_base.c
> @@ -1614,16 +1614,13 @@ megasas_queue_command(struct Scsi_Host *shost, struct scsi_cmnd *scmd)
>                 goto out_done;
>         }
> 
> -       switch (scmd->cmnd[0]) {
> -       case SYNCHRONIZE_CACHE:
> -               /*
> -                * FW takes care of flush cache on its own
> -                * No need to send it down
> -                */
> +       /*
> +        * FW takes care of flush cache on its own for Virtual Disk.
> +        * No need to send it down for VD. For JBOD send SYNCHRONIZE_CACHE to FW.
> +        */
> +       if ((scmd->cmnd[0] == SYNCHRONIZE_CACHE) && MEGASAS_IS_LOGICAL(scmd)) {
>                 scmd->result = DID_OK << 16;
>                 goto out_done;
> -       default:
> -               break;
>         }
> 
>         if (instance->instancet->build_and_issue_cmd(instance, scmd)) {
> 
> MEGASAS_IS_LOGICAL is defined to be a macro with '?' operator, which has a lower precedence than '&&'.
> The macro should have been defined as:
> --- a/drivers/scsi/megaraid/megaraid_sas.h
> +++ b/drivers/scsi/megaraid/megaraid_sas.h
> @@ -1823,7 +1823,7 @@ struct megasas_instance_template {
>  };
> 
>  #define MEGASAS_IS_LOGICAL(scp)                                                \
> -       (scp->device->channel < MEGASAS_MAX_PD_CHANNELS) ? 0 : 1
> +       ((scp->device->channel < MEGASAS_MAX_PD_CHANNELS) ? 0 : 1)
> 
>  #define MEGASAS_DEV_INDEX(inst, scp)                                   \
>         ((scp->device->channel % 2) * MEGASAS_MAX_DEV_PER_CHANNEL) +    \
> 
> Dashi Cao
> 


-- 
~Randy

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: version 3.18.44 to 3.18.45 introduced a bug in "drivers/scsi/megaraid/megaraid_sas_base.c"
  2016-12-13 16:08 ` Randy Dunlap
@ 2016-12-13 16:30   ` Greg Kroah-Hartman
  2016-12-13 16:33     ` Randy Dunlap
  0 siblings, 1 reply; 7+ messages in thread
From: Greg Kroah-Hartman @ 2016-12-13 16:30 UTC (permalink / raw)
  To: Randy Dunlap; +Cc: Dashi DS1 Cao, linux-kernel, linux-scsi, stable

On Tue, Dec 13, 2016 at 08:08:27AM -0800, Randy Dunlap wrote:
> [adding other lists + gregkh]
> 
> 
> On 12/13/16 02:56, Dashi DS1 Cao wrote:
> > --- a/drivers/scsi/megaraid/megaraid_sas_base.c
> > +++ b/drivers/scsi/megaraid/megaraid_sas_base.c
> > @@ -1614,16 +1614,13 @@ megasas_queue_command(struct Scsi_Host *shost, struct scsi_cmnd *scmd)
> >                 goto out_done;
> >         }
> > 
> > -       switch (scmd->cmnd[0]) {
> > -       case SYNCHRONIZE_CACHE:
> > -               /*
> > -                * FW takes care of flush cache on its own
> > -                * No need to send it down
> > -                */
> > +       /*
> > +        * FW takes care of flush cache on its own for Virtual Disk.
> > +        * No need to send it down for VD. For JBOD send SYNCHRONIZE_CACHE to FW.
> > +        */
> > +       if ((scmd->cmnd[0] == SYNCHRONIZE_CACHE) && MEGASAS_IS_LOGICAL(scmd)) {
> >                 scmd->result = DID_OK << 16;
> >                 goto out_done;
> > -       default:
> > -               break;
> >         }
> > 
> >         if (instance->instancet->build_and_issue_cmd(instance, scmd)) {
> > 
> > MEGASAS_IS_LOGICAL is defined to be a macro with '?' operator, which has a lower precedence than '&&'.
> > The macro should have been defined as:
> > --- a/drivers/scsi/megaraid/megaraid_sas.h
> > +++ b/drivers/scsi/megaraid/megaraid_sas.h
> > @@ -1823,7 +1823,7 @@ struct megasas_instance_template {
> >  };
> > 
> >  #define MEGASAS_IS_LOGICAL(scp)                                                \
> > -       (scp->device->channel < MEGASAS_MAX_PD_CHANNELS) ? 0 : 1
> > +       ((scp->device->channel < MEGASAS_MAX_PD_CHANNELS) ? 0 : 1)
> > 
> >  #define MEGASAS_DEV_INDEX(inst, scp)                                   \
> >         ((scp->device->channel % 2) * MEGASAS_MAX_DEV_PER_CHANNEL) +    \
> > 
> > Dashi Cao
> > 

I don't maintain 3.18-stable :)

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: version 3.18.44 to 3.18.45 introduced a bug in "drivers/scsi/megaraid/megaraid_sas_base.c"
  2016-12-13 16:30   ` Greg Kroah-Hartman
@ 2016-12-13 16:33     ` Randy Dunlap
  2016-12-13 16:43       ` James Bottomley
  0 siblings, 1 reply; 7+ messages in thread
From: Randy Dunlap @ 2016-12-13 16:33 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Dashi DS1 Cao, linux-kernel, linux-scsi, stable, Sasha Levin

On 12/13/16 08:30, Greg Kroah-Hartman wrote:
> On Tue, Dec 13, 2016 at 08:08:27AM -0800, Randy Dunlap wrote:
>> [adding other lists + gregkh]
>>
>>
>> On 12/13/16 02:56, Dashi DS1 Cao wrote:
>>> --- a/drivers/scsi/megaraid/megaraid_sas_base.c
>>> +++ b/drivers/scsi/megaraid/megaraid_sas_base.c
>>> @@ -1614,16 +1614,13 @@ megasas_queue_command(struct Scsi_Host *shost, struct scsi_cmnd *scmd)
>>>                 goto out_done;
>>>         }
>>>
>>> -       switch (scmd->cmnd[0]) {
>>> -       case SYNCHRONIZE_CACHE:
>>> -               /*
>>> -                * FW takes care of flush cache on its own
>>> -                * No need to send it down
>>> -                */
>>> +       /*
>>> +        * FW takes care of flush cache on its own for Virtual Disk.
>>> +        * No need to send it down for VD. For JBOD send SYNCHRONIZE_CACHE to FW.
>>> +        */
>>> +       if ((scmd->cmnd[0] == SYNCHRONIZE_CACHE) && MEGASAS_IS_LOGICAL(scmd)) {
>>>                 scmd->result = DID_OK << 16;
>>>                 goto out_done;
>>> -       default:
>>> -               break;
>>>         }
>>>
>>>         if (instance->instancet->build_and_issue_cmd(instance, scmd)) {
>>>
>>> MEGASAS_IS_LOGICAL is defined to be a macro with '?' operator, which has a lower precedence than '&&'.
>>> The macro should have been defined as:
>>> --- a/drivers/scsi/megaraid/megaraid_sas.h
>>> +++ b/drivers/scsi/megaraid/megaraid_sas.h
>>> @@ -1823,7 +1823,7 @@ struct megasas_instance_template {
>>>  };
>>>
>>>  #define MEGASAS_IS_LOGICAL(scp)                                                \
>>> -       (scp->device->channel < MEGASAS_MAX_PD_CHANNELS) ? 0 : 1
>>> +       ((scp->device->channel < MEGASAS_MAX_PD_CHANNELS) ? 0 : 1)
>>>
>>>  #define MEGASAS_DEV_INDEX(inst, scp)                                   \
>>>         ((scp->device->channel % 2) * MEGASAS_MAX_DEV_PER_CHANNEL) +    \
>>>
>>> Dashi Cao
>>>
> 
> I don't maintain 3.18-stable :)
> 
> thanks,
> 
> greg k-h
> 

Thanks. My bad.

adding Sasha.


-- 
~Randy

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: version 3.18.44 to 3.18.45 introduced a bug in "drivers/scsi/megaraid/megaraid_sas_base.c"
  2016-12-13 16:33     ` Randy Dunlap
@ 2016-12-13 16:43       ` James Bottomley
  2016-12-13 16:47         ` Greg Kroah-Hartman
  0 siblings, 1 reply; 7+ messages in thread
From: James Bottomley @ 2016-12-13 16:43 UTC (permalink / raw)
  To: Randy Dunlap, Greg Kroah-Hartman
  Cc: Dashi DS1 Cao, linux-kernel, linux-scsi, stable, Sasha Levin

On Tue, 2016-12-13 at 08:33 -0800, Randy Dunlap wrote:
> On 12/13/16 08:30, Greg Kroah-Hartman wrote:
> > I don't maintain 3.18-stable :)
> > 
> > thanks,
> > 
> > greg k-h
> > 
> 
> Thanks. My bad.
> 
> adding Sasha.

This was all covered here:

https://www.spinics.net/lists/stable/msg150608.html

How did it get missed, and what should the process be?

James

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: version 3.18.44 to 3.18.45 introduced a bug in "drivers/scsi/megaraid/megaraid_sas_base.c"
  2016-12-13 16:43       ` James Bottomley
@ 2016-12-13 16:47         ` Greg Kroah-Hartman
  2016-12-13 20:51           ` alexander.levin
  0 siblings, 1 reply; 7+ messages in thread
From: Greg Kroah-Hartman @ 2016-12-13 16:47 UTC (permalink / raw)
  To: James Bottomley
  Cc: Randy Dunlap, Dashi DS1 Cao, linux-kernel, linux-scsi, stable,
	Sasha Levin

On Tue, Dec 13, 2016 at 08:43:41AM -0800, James Bottomley wrote:
> On Tue, 2016-12-13 at 08:33 -0800, Randy Dunlap wrote:
> > On 12/13/16 08:30, Greg Kroah-Hartman wrote:
> > > I don't maintain 3.18-stable :)
> > > 
> > > thanks,
> > > 
> > > greg k-h
> > > 
> > 
> > Thanks. My bad.
> > 
> > adding Sasha.
> 
> This was all covered here:
> 
> https://www.spinics.net/lists/stable/msg150608.html
> 
> How did it get missed, and what should the process be?

I don't know how that got missed, did the "fixup" patch never make it
into 3.18-stable?  I know Sasha has been slow on that kernel, and it's
about to go end-of-life, so perhaps he's just releasing them on a slower
cycle at the moment...

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: version 3.18.44 to 3.18.45 introduced a bug in "drivers/scsi/megaraid/megaraid_sas_base.c"
  2016-12-13 16:47         ` Greg Kroah-Hartman
@ 2016-12-13 20:51           ` alexander.levin
  0 siblings, 0 replies; 7+ messages in thread
From: alexander.levin @ 2016-12-13 20:51 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: James Bottomley, Randy Dunlap, Dashi DS1 Cao, linux-kernel,
	linux-scsi, stable

On Tue, Dec 13, 2016 at 08:47:33AM -0800, Greg Kroah-Hartman wrote:
> On Tue, Dec 13, 2016 at 08:43:41AM -0800, James Bottomley wrote:
> > On Tue, 2016-12-13 at 08:33 -0800, Randy Dunlap wrote:
> > > On 12/13/16 08:30, Greg Kroah-Hartman wrote:
> > > > I don't maintain 3.18-stable :)
> > > > 
> > > > thanks,
> > > > 
> > > > greg k-h
> > > > 
> > > 
> > > Thanks. My bad.
> > > 
> > > adding Sasha.
> > 
> > This was all covered here:
> > 
> > https://www.spinics.net/lists/stable/msg150608.html
> > 
> > How did it get missed, and what should the process be?
> 
> I don't know how that got missed, did the "fixup" patch never make it
> into 3.18-stable?  I know Sasha has been slow on that kernel, and it's
> about to go end-of-life, so perhaps he's just releasing them on a slower
> cycle at the moment...

I think that what happened there is that I built the branch before the fixup
was upstreamed, so I never noticed that.

It's queued up right now, but I don't think I'll make another release except
a final one in January when it EOLs.

If this fix is urgent for anyone then something is wrong as this kernel dies
in two weeks...

-- 

Thanks,
Sasha

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2016-12-13 20:53 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-13 10:56 version 3.18.44 to 3.18.45 introduced a bug in "drivers/scsi/megaraid/megaraid_sas_base.c" Dashi DS1 Cao
2016-12-13 16:08 ` Randy Dunlap
2016-12-13 16:30   ` Greg Kroah-Hartman
2016-12-13 16:33     ` Randy Dunlap
2016-12-13 16:43       ` James Bottomley
2016-12-13 16:47         ` Greg Kroah-Hartman
2016-12-13 20:51           ` alexander.levin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).