| Message ID | 20170328044126.10006-1-famz@redhat.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
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 <famz@redhat.com> 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); ?
On Wed, 03/29 22:37, Martin K. Petersen wrote: > Fam Zheng <famz@redhat.com> 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 <famz@redhat.com> 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.
On Thu, 03/30 11:30, Martin K. Petersen wrote: > Fam Zheng <famz@redhat.com> 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
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));
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 <famz@redhat.com> --- v2: Fix granularity mismatch. [Martin] --- drivers/scsi/sd.c | 1 + 1 file changed, 1 insertion(+)