| Message ID | 56d0b08c.e99c420a.dbca1.0415@mx.google.com (mailing list archive) |
|---|---|
| State | Rejected, archived |
| Headers | show |
On Sat, 2016-02-27 at 04:07 +0800, tom.ty89@gmail.com wrote: > From: Tom Yan <tom.ty89@gmail.com> > > Signed-off-by: Tom Yan <tom.ty89@gmail.com> > > diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c > index 1179ec1..9eeee51 100644 > --- a/drivers/scsi/sd.c > +++ b/drivers/scsi/sd.c > @@ -2786,7 +2786,7 @@ static void sd_read_write_same(struct scsi_disk > *sdkp, unsigned char *buffer) > * CODES is unsupported and the device has an ATA > * Information VPD page (SAT). > */ > - if (!scsi_get_vpd_page(sdev, 0x89, buffer, > vpd_buf_len)) > + if (scsi_get_vpd_page(sdev, 0x89, buffer, > vpd_buf_len)) > sdev->no_write_same = 1; > } If you're inverting the condition, you'd need to invert the comment as well. scsi_get_vpd_page returns 0 on success so !scsi_get_vpd_page is true if it got the page (which is what the comment says). James -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Oh I made a mistake on this one then. Since I send it with another patch, should I resend that alone? On 27 February 2016 at 04:16, James Bottomley <James.Bottomley@hansenpartnership.com> wrote: > On Sat, 2016-02-27 at 04:07 +0800, tom.ty89@gmail.com wrote: >> From: Tom Yan <tom.ty89@gmail.com> >> >> Signed-off-by: Tom Yan <tom.ty89@gmail.com> >> >> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c >> index 1179ec1..9eeee51 100644 >> --- a/drivers/scsi/sd.c >> +++ b/drivers/scsi/sd.c >> @@ -2786,7 +2786,7 @@ static void sd_read_write_same(struct scsi_disk >> *sdkp, unsigned char *buffer) >> * CODES is unsupported and the device has an ATA >> * Information VPD page (SAT). >> */ >> - if (!scsi_get_vpd_page(sdev, 0x89, buffer, >> vpd_buf_len)) >> + if (scsi_get_vpd_page(sdev, 0x89, buffer, >> vpd_buf_len)) >> sdev->no_write_same = 1; >> } > > > If you're inverting the condition, you'd need to invert the comment as > well. scsi_get_vpd_page returns 0 on success so !scsi_get_vpd_page is > true if it got the page (which is what the comment says). > > James > -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, 2016-02-27 at 04:32 +0800, Tom Yan wrote: > Oh I made a mistake on this one then. > > Since I send it with another patch, should I resend that alone? Yes, that's fine. but you need to explain as part of the changelog why this condition needs inverting because your Subject just says "as per the comment" which makes no sense if you're changing the comment. James -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
No I mean I no longer thinks that this condition needs to be inverted. I just THOUGHT that !scsi_get_vpd_page is true if it DIDN'T get the page. On 27 February 2016 at 04:57, James Bottomley <James.Bottomley@hansenpartnership.com> wrote: > On Sat, 2016-02-27 at 04:32 +0800, Tom Yan wrote: >> Oh I made a mistake on this one then. >> >> Since I send it with another patch, should I resend that alone? > > Yes, that's fine. but you need to explain as part of the changelog why > this condition needs inverting because your Subject just says "as per > the comment" which makes no sense if you're changing the comment. > > James > > -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index 1179ec1..9eeee51 100644 --- a/drivers/scsi/sd.c +++ b/drivers/scsi/sd.c @@ -2786,7 +2786,7 @@ static void sd_read_write_same(struct scsi_disk *sdkp, unsigned char *buffer) * CODES is unsupported and the device has an ATA * Information VPD page (SAT). */ - if (!scsi_get_vpd_page(sdev, 0x89, buffer, vpd_buf_len)) + if (scsi_get_vpd_page(sdev, 0x89, buffer, vpd_buf_len)) sdev->no_write_same = 1; }