Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1540992 > unrolled thread
| Started by | Dashi DS1 Cao <caods1@lenovo.com> |
|---|---|
| First post | 2016-12-13 12:00 +0100 |
| Last post | 2016-12-13 17:50 +0100 |
| Articles | 7 — 5 participants |
Back to article view | Back to linux.kernel
version 3.18.44 to 3.18.45 introduced a bug in "drivers/scsi/megaraid/megaraid_sas_base.c" Dashi DS1 Cao <caods1@lenovo.com> - 2016-12-13 12:00 +0100
Re: version 3.18.44 to 3.18.45 introduced a bug in "drivers/scsi/megaraid/megaraid_sas_base.c" Randy Dunlap <rdunlap@infradead.org> - 2016-12-13 17:10 +0100
Re: version 3.18.44 to 3.18.45 introduced a bug in "drivers/scsi/megaraid/megaraid_sas_base.c" Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-12-13 17:40 +0100
Re: version 3.18.44 to 3.18.45 introduced a bug in "drivers/scsi/megaraid/megaraid_sas_base.c" Randy Dunlap <rdunlap@infradead.org> - 2016-12-13 17:40 +0100
Re: version 3.18.44 to 3.18.45 introduced a bug in "drivers/scsi/megaraid/megaraid_sas_base.c" Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-12-13 17:50 +0100
Re: version 3.18.44 to 3.18.45 introduced a bug in "drivers/scsi/megaraid/megaraid_sas_base.c" alexander.levin@verizon.com - 2016-12-13 22:00 +0100
Re: version 3.18.44 to 3.18.45 introduced a bug in "drivers/scsi/megaraid/megaraid_sas_base.c" James Bottomley <James.Bottomley@HansenPartnership.com> - 2016-12-13 17:50 +0100
| From | Dashi DS1 Cao <caods1@lenovo.com> |
|---|---|
| Date | 2016-12-13 12:00 +0100 |
| Subject | version 3.18.44 to 3.18.45 introduced a bug in "drivers/scsi/megaraid/megaraid_sas_base.c" |
| Message-ID | <sNSYq-aO-45@gated-at.bofh.it> |
--- 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
[toc] | [next] | [standalone]
| From | Randy Dunlap <rdunlap@infradead.org> |
|---|---|
| Date | 2016-12-13 17:10 +0100 |
| Message-ID | <sNXOp-3i2-17@gated-at.bofh.it> |
| In reply to | #1540992 |
[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
[toc] | [prev] | [next] | [standalone]
| From | Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
|---|---|
| Date | 2016-12-13 17:40 +0100 |
| Message-ID | <sNYhr-3rV-17@gated-at.bofh.it> |
| In reply to | #1541217 |
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
[toc] | [prev] | [next] | [standalone]
| From | Randy Dunlap <rdunlap@infradead.org> |
|---|---|
| Date | 2016-12-13 17:40 +0100 |
| Message-ID | <sNYhr-3rV-35@gated-at.bofh.it> |
| In reply to | #1541235 |
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
[toc] | [prev] | [next] | [standalone]
| From | Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
|---|---|
| Date | 2016-12-13 17:50 +0100 |
| Message-ID | <sNYr7-3vi-17@gated-at.bofh.it> |
| In reply to | #1541239 |
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
[toc] | [prev] | [next] | [standalone]
| From | alexander.levin@verizon.com |
|---|---|
| Date | 2016-12-13 22:00 +0100 |
| Message-ID | <sO2l4-5Po-11@gated-at.bofh.it> |
| In reply to | #1541249 |
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
[toc] | [prev] | [next] | [standalone]
| From | James Bottomley <James.Bottomley@HansenPartnership.com> |
|---|---|
| Date | 2016-12-13 17:50 +0100 |
| Message-ID | <sNYr7-3vi-11@gated-at.bofh.it> |
| In reply to | #1541239 |
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
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web