| Message ID | 20200906012716.1553-3-tom.ty89@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [RESEND,1/4] scsi: sg: fix BLKSECTGET ioctl | expand |
On Sun, Sep 06, 2020 at 09:27:15AM +0800, Tom Yan wrote: > Signed-off-by: Tom Yan <tom.ty89@gmail.com> > --- > drivers/scsi/sg.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) I know I don't take patches without any changelog text at all, but maybe the scsi maintainers are more leniant. You should write changelogs that explain _why_ you are doing what you are doing, you just say what you did, with no reasoning at all. Same for another patch in this series. thanks, greg k-h
The resend is done to add linux-api@vger.kernel.org to the CC list. I also removed some CC because the email addresses no longer exist (and accidentally made a typo to one that still does, hence the second resend). I don't know if that counts as a change. I didn't think 3/4 and 4/4 requires further explanation, as I thought they are self-explanatory enough (logical sector size has never been, or at least is no longer, necessarily 512). I can add that to the commit message. P.S. Even I myself isn't exactly sure what/which clamping should be used in max_sectors_bytes(). The reason I picked USHRT_MAX is that BLKSECTGET in sg should work identically to its equivalence in the block layer, regardless of whether that is technically/programmatically necessary. So I decided to use the same clamping in max_sector_bytes(). But it seems fine/correct to clamp (max_sectors * logical_sector_size) to INT_MAX instead (https://github.com/torvalds/linux/blob/v5.9-rc3/block/blk-mq.c#L3211). (On the other hand, in that case it could end up being inconsistent to what BLKSECTGET + BLKSSZGET reports). Perhaps I should add my uncertainty / the alternative to the commit message. Regards, Tom On Sun, 6 Sep 2020 at 14:26, Greg KH <gregkh@linuxfoundation.org> wrote: > > On Sun, Sep 06, 2020 at 09:27:15AM +0800, Tom Yan wrote: > > Signed-off-by: Tom Yan <tom.ty89@gmail.com> > > --- > > drivers/scsi/sg.c | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > I know I don't take patches without any changelog text at all, but maybe > the scsi maintainers are more leniant. > > You should write changelogs that explain _why_ you are doing what you > are doing, you just say what you did, with no reasoning at all. > > Same for another patch in this series. > > thanks, > > greg k-h
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c index 0e3f084141a3..deeab4855172 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); + unsigned int logical_block_size = queue_logical_block_size(q); - max_sectors = min_t(unsigned int, max_sectors, INT_MAX >> 9); + max_sectors = min_t(unsigned int, max_sectors, USHRT_MAX); - return max_sectors << 9; + return max_sectors * logical_block_size; } static void
Signed-off-by: Tom Yan <tom.ty89@gmail.com> --- drivers/scsi/sg.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)