If device reports a small max_xfer_blocks and a zero opt_xfer_blocks, we
end up using BLK_DEF_MAX_SECTORS, which is wrong and r/w of that size
may get error.
Fixes: ca369d51b3e ("block/sd: Fix device-imposed transfer length limits")
Signed-off-by: Fam Zheng <[email protected]>
---
v2: Fix granularity mismatch. [Martin]
---
drivers/scsi/sd.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index fcfeddc..a5c7e67 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -2957,6 +2957,7 @@ static int sd_revalidate_disk(struct gendisk *disk)
rw_max = logical_to_sectors(sdp, sdkp->opt_xfer_blocks);
} else
rw_max = BLK_DEF_MAX_SECTORS;
+ rw_max = min_not_zero(rw_max, logical_to_sectors(sdp, dev_max));
/* Combine with controller limits */
q->limits.max_sectors = min(rw_max, queue_max_hw_sectors(q));
--
2.9.3
Hi Fam,
[auto build test WARNING on scsi/for-next]
[also build test WARNING on v4.11-rc4 next-20170327]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Fam-Zheng/sd-Consider-max_xfer_blocks-if-opt_xfer_blocks-is-unusable/20170328-141853
base: https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next
config: x86_64-randconfig-x016-201713 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All warnings (new ones prefixed by >>):
In file included from include/linux/list.h:8:0,
from include/linux/module.h:9,
from drivers//scsi/sd.c:35:
drivers//scsi/sd.c: In function 'sd_revalidate_disk':
include/linux/kernel.h:755:16: warning: comparison of distinct pointer types lacks a cast
(void) (&min1 == &min2); \
^
include/linux/kernel.h:758:2: note: in expansion of macro '__min'
__min(typeof(x), typeof(y), \
^~~~~
>> include/linux/kernel.h:783:39: note: in expansion of macro 'min'
__x == 0 ? __y : ((__y == 0) ? __x : min(__x, __y)); })
^~~
>> drivers//scsi/sd.c:2960:11: note: in expansion of macro 'min_not_zero'
rw_max = min_not_zero(rw_max, logical_to_sectors(sdp, dev_max));
^~~~~~~~~~~~
--
In file included from include/linux/list.h:8:0,
from include/linux/module.h:9,
from drivers/scsi/sd.c:35:
drivers/scsi/sd.c: In function 'sd_revalidate_disk':
include/linux/kernel.h:755:16: warning: comparison of distinct pointer types lacks a cast
(void) (&min1 == &min2); \
^
include/linux/kernel.h:758:2: note: in expansion of macro '__min'
__min(typeof(x), typeof(y), \
^~~~~
>> include/linux/kernel.h:783:39: note: in expansion of macro 'min'
__x == 0 ? __y : ((__y == 0) ? __x : min(__x, __y)); })
^~~
drivers/scsi/sd.c:2960:11: note: in expansion of macro 'min_not_zero'
rw_max = min_not_zero(rw_max, logical_to_sectors(sdp, dev_max));
^~~~~~~~~~~~
vim +/min_not_zero +2960 drivers//scsi/sd.c
2944 dev_max = min_not_zero(dev_max, sdkp->max_xfer_blocks);
2945 q->limits.max_dev_sectors = logical_to_sectors(sdp, dev_max);
2946
2947 /*
2948 * Use the device's preferred I/O size for reads and writes
2949 * unless the reported value is unreasonably small, large, or
2950 * garbage.
2951 */
2952 if (sdkp->opt_xfer_blocks &&
2953 sdkp->opt_xfer_blocks <= dev_max &&
2954 sdkp->opt_xfer_blocks <= SD_DEF_XFER_BLOCKS &&
2955 logical_to_bytes(sdp, sdkp->opt_xfer_blocks) >= PAGE_SIZE) {
2956 q->limits.io_opt = logical_to_bytes(sdp, sdkp->opt_xfer_blocks);
2957 rw_max = logical_to_sectors(sdp, sdkp->opt_xfer_blocks);
2958 } else
2959 rw_max = BLK_DEF_MAX_SECTORS;
> 2960 rw_max = min_not_zero(rw_max, logical_to_sectors(sdp, dev_max));
2961
2962 /* Combine with controller limits */
2963 q->limits.max_sectors = min(rw_max, queue_max_hw_sectors(q));
2964
2965 set_capacity(disk, logical_to_sectors(sdp, sdkp->capacity));
2966 sd_config_write_same(sdkp);
2967 kfree(buffer);
2968
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Fam Zheng <[email protected]> writes:
Fam,
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index fcfeddc..a5c7e67 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -2957,6 +2957,7 @@ static int sd_revalidate_disk(struct gendisk *disk)
> rw_max = logical_to_sectors(sdp, sdkp->opt_xfer_blocks);
> } else
> rw_max = BLK_DEF_MAX_SECTORS;
> + rw_max = min_not_zero(rw_max, logical_to_sectors(sdp, dev_max));
>
> /* Combine with controller limits */
> q->limits.max_sectors = min(rw_max, queue_max_hw_sectors(q));
Instead of updating rw_max twice, how about:
} else
rw_max = min_not_zero(logical_to_sectors(sdp, dev_max),
BLK_DEF_MAX_SECTORS);
?
--
Martin K. Petersen Oracle Linux Engineering
On Wed, 03/29 22:37, Martin K. Petersen wrote:
> Fam Zheng <[email protected]> writes:
>
> Fam,
>
> > diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> > index fcfeddc..a5c7e67 100644
> > --- a/drivers/scsi/sd.c
> > +++ b/drivers/scsi/sd.c
> > @@ -2957,6 +2957,7 @@ static int sd_revalidate_disk(struct gendisk *disk)
> > rw_max = logical_to_sectors(sdp, sdkp->opt_xfer_blocks);
> > } else
> > rw_max = BLK_DEF_MAX_SECTORS;
> > + rw_max = min_not_zero(rw_max, logical_to_sectors(sdp, dev_max));
> >
> > /* Combine with controller limits */
> > q->limits.max_sectors = min(rw_max, queue_max_hw_sectors(q));
>
> Instead of updating rw_max twice, how about:
>
> } else
> rw_max = min_not_zero(logical_to_sectors(sdp, dev_max),
> BLK_DEF_MAX_SECTORS);
Yes, it is better. Is it okay to make the change when you apply?
Fam
Fam Zheng <[email protected]> writes:
>> rw_max = min_not_zero(logical_to_sectors(sdp, dev_max),
>> BLK_DEF_MAX_SECTORS);
>
> Yes, it is better. Is it okay to make the change when you apply?
Sure. Applied to 4.11/scsi-fixes.
--
Martin K. Petersen Oracle Linux Engineering
On Thu, 03/30 11:30, Martin K. Petersen wrote:
> Fam Zheng <[email protected]> writes:
>
> >> rw_max = min_not_zero(logical_to_sectors(sdp, dev_max),
> >> BLK_DEF_MAX_SECTORS);
> >
> > Yes, it is better. Is it okay to make the change when you apply?
>
> Sure. Applied to 4.11/scsi-fixes.
Cool. Thanks!
Fam