* [PATCH v2] sd: Consider max_xfer_blocks if opt_xfer_blocks is unusable
@ 2017-03-28 4:41 Fam Zheng
2017-03-28 7:14 ` kbuild test robot
2017-03-30 2:37 ` Martin K. Petersen
0 siblings, 2 replies; 7+ messages in thread
From: Fam Zheng @ 2017-03-28 4:41 UTC (permalink / raw)
To: linux-kernel
Cc: Martin K. Petersen, famz, linux-scsi, Laurence Oberman,
James E.J. Bottomley
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(+)
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
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH v2] sd: Consider max_xfer_blocks if opt_xfer_blocks is unusable 2017-03-28 4:41 [PATCH v2] sd: Consider max_xfer_blocks if opt_xfer_blocks is unusable Fam Zheng @ 2017-03-28 7:14 ` kbuild test robot 2017-03-30 2:37 ` Martin K. Petersen 1 sibling, 0 replies; 7+ messages in thread From: kbuild test robot @ 2017-03-28 7:14 UTC (permalink / raw) To: Fam Zheng Cc: kbuild-all, linux-kernel, Martin K. Petersen, famz, linux-scsi, Laurence Oberman, James E.J. Bottomley [-- Attachment #1: Type: text/plain, Size: 3578 bytes --] 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 [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 28375 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] sd: Consider max_xfer_blocks if opt_xfer_blocks is unusable @ 2017-03-28 7:14 ` kbuild test robot 0 siblings, 0 replies; 7+ messages in thread From: kbuild test robot @ 2017-03-28 7:14 UTC (permalink / raw) Cc: kbuild-all, linux-kernel, Martin K. Petersen, famz, linux-scsi, Laurence Oberman, James E.J. Bottomley [-- Attachment #1: Type: text/plain, Size: 3578 bytes --] 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 [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 28375 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] sd: Consider max_xfer_blocks if opt_xfer_blocks is unusable 2017-03-28 4:41 [PATCH v2] sd: Consider max_xfer_blocks if opt_xfer_blocks is unusable Fam Zheng 2017-03-28 7:14 ` kbuild test robot @ 2017-03-30 2:37 ` Martin K. Petersen 2017-03-30 4:13 ` Fam Zheng 1 sibling, 1 reply; 7+ messages in thread From: Martin K. Petersen @ 2017-03-30 2:37 UTC (permalink / raw) To: Fam Zheng Cc: linux-kernel, Martin K. Petersen, linux-scsi, Laurence Oberman, James E.J. Bottomley 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); ? -- Martin K. Petersen Oracle Linux Engineering ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] sd: Consider max_xfer_blocks if opt_xfer_blocks is unusable 2017-03-30 2:37 ` Martin K. Petersen @ 2017-03-30 4:13 ` Fam Zheng 2017-03-30 15:30 ` Martin K. Petersen 0 siblings, 1 reply; 7+ messages in thread From: Fam Zheng @ 2017-03-30 4:13 UTC (permalink / raw) To: Martin K. Petersen Cc: linux-kernel, linux-scsi, Laurence Oberman, James E.J. Bottomley 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] sd: Consider max_xfer_blocks if opt_xfer_blocks is unusable 2017-03-30 4:13 ` Fam Zheng @ 2017-03-30 15:30 ` Martin K. Petersen 2017-03-31 4:05 ` Fam Zheng 0 siblings, 1 reply; 7+ messages in thread From: Martin K. Petersen @ 2017-03-30 15:30 UTC (permalink / raw) To: Fam Zheng Cc: Martin K. Petersen, linux-kernel, linux-scsi, Laurence Oberman, James E.J. Bottomley 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. -- Martin K. Petersen Oracle Linux Engineering ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] sd: Consider max_xfer_blocks if opt_xfer_blocks is unusable 2017-03-30 15:30 ` Martin K. Petersen @ 2017-03-31 4:05 ` Fam Zheng 0 siblings, 0 replies; 7+ messages in thread From: Fam Zheng @ 2017-03-31 4:05 UTC (permalink / raw) To: Martin K. Petersen Cc: linux-kernel, linux-scsi, Laurence Oberman, James E.J. Bottomley 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-03-31 4:05 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-03-28 4:41 [PATCH v2] sd: Consider max_xfer_blocks if opt_xfer_blocks is unusable Fam Zheng 2017-03-28 7:14 ` kbuild test robot 2017-03-28 7:14 ` kbuild test robot 2017-03-30 2:37 ` Martin K. Petersen 2017-03-30 4:13 ` Fam Zheng 2017-03-30 15:30 ` Martin K. Petersen 2017-03-31 4:05 ` Fam Zheng
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.