| Message ID | 20200923055248.1901-1-tom.ty89@gmail.com (mailing list archive) |
|---|---|
| State | Rejected |
| Headers | show |
| Series | [1/2] scsi: sg: use queue_logical_block_size() in max_sectors_bytes() | expand |
On 2020-09-22 22:52, Tom Yan wrote: > Logical block size was never / is no longer necessarily 512. > > Signed-off-by: Tom Yan <tom.ty89@gmail.com> > --- > drivers/scsi/sg.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c > index 20472aaaf630..8a2cca71017f 100644 > --- a/drivers/scsi/sg.c > +++ b/drivers/scsi/sg.c > @@ -848,10 +848,11 @@ static int srp_done(Sg_fd *sfp, Sg_request *srp) > static int max_sectors_bytes(struct request_queue *q) > { > unsigned int max_sectors = queue_max_sectors(q); > + max_sectors *= queue_logical_block_size(q); > > - max_sectors = min_t(unsigned int, max_sectors, INT_MAX >> 9); > + max_sectors = min_t(unsigned int, max_sectors, INT_MAX); > > - return max_sectors << 9; > + return max_sectors; > } I think the above patch is wrong and also that it breaks code that is correct. In the Linux kernel, "one sector" is by definition 512 bytes. See also the definition of the SECTOR_SIZE and SECTOR_SHIFT constants. Bart.
Oh right, that's what logical_to_sectors()'s for. Thanks! :-) On Fri, 25 Sep 2020 at 01:52, Bart Van Assche <bvanassche@acm.org> wrote: > > On 2020-09-22 22:52, Tom Yan wrote: > > Logical block size was never / is no longer necessarily 512. > > > > Signed-off-by: Tom Yan <tom.ty89@gmail.com> > > --- > > drivers/scsi/sg.c | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c > > index 20472aaaf630..8a2cca71017f 100644 > > --- a/drivers/scsi/sg.c > > +++ b/drivers/scsi/sg.c > > @@ -848,10 +848,11 @@ static int srp_done(Sg_fd *sfp, Sg_request *srp) > > static int max_sectors_bytes(struct request_queue *q) > > { > > unsigned int max_sectors = queue_max_sectors(q); > > + max_sectors *= queue_logical_block_size(q); > > > > - max_sectors = min_t(unsigned int, max_sectors, INT_MAX >> 9); > > + max_sectors = min_t(unsigned int, max_sectors, INT_MAX); > > > > - return max_sectors << 9; > > + return max_sectors; > > } > > I think the above patch is wrong and also that it breaks code that is > correct. In the Linux kernel, "one sector" is by definition 512 bytes. > See also the definition of the SECTOR_SIZE and SECTOR_SHIFT constants. > > Bart.
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c index 20472aaaf630..8a2cca71017f 100644 --- a/drivers/scsi/sg.c +++ b/drivers/scsi/sg.c @@ -848,10 +848,11 @@ static int srp_done(Sg_fd *sfp, Sg_request *srp) static int max_sectors_bytes(struct request_queue *q) { unsigned int max_sectors = queue_max_sectors(q); + max_sectors *= queue_logical_block_size(q); - max_sectors = min_t(unsigned int, max_sectors, INT_MAX >> 9); + max_sectors = min_t(unsigned int, max_sectors, INT_MAX); - return max_sectors << 9; + return max_sectors; } static void
Logical block size was never / is no longer necessarily 512. Signed-off-by: Tom Yan <tom.ty89@gmail.com> --- drivers/scsi/sg.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)