| Message ID | 20190617012832.4311-1-chaitanya.kulkarni@wdc.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | block: use right accessor to read nr_setcs | expand |
On 6/16/19 6:28 PM, Chaitanya Kulkarni wrote: > In the blk-zoned, bcache, f2fs and blktrace implementation > block device->hd_part->number of sectors field is accessed directly > without any appropriate locking or accessor function. There is > an existing accessor function present in the in include/linux/genhd.h > which should be used to read the bdev->hd_part->nr_sects. In the subject, "nr_setcs" should probably be changed into "nr_sects"? Bart.
On 6/16/19 6:28 PM, Chaitanya Kulkarni wrote: > Changes from V1:- > > 1. Drop the target_pscsi patch. (Bart) > 2. Remove rcu locking which is not needed. (Bart) > > Chaitanya Kulkarni (7): > block: add a helper function to read nr_setcs > blk-zoned: update blkdev_nr_zones() with helper > blk-zoned: update blkdev_report_zone() with helper > blk-zoned: update blkdev_reset_zones() with helper > bcache: update cached_dev_init() with helper > f2fs: use helper in init_blkz_info() > blktrace: use helper in blk_trace_setup_lba() > > block/blk-zoned.c | 12 ++++++------ > drivers/md/bcache/super.c | 2 +- > fs/f2fs/super.c | 2 +- > include/linux/blkdev.h | 10 ++++++++++ > kernel/trace/blktrace.c | 2 +- > 5 files changed, 19 insertions(+), 9 deletions(-) My feedback about the pscsi_get_blocks() was misleading: what I meant is that it is not necessary to introduce RCU locking in that function. I think that using bdev_nr_sects() or part_nr_sects_read() to read nr_sects in that function is useful. Is there any reason that the following Xen macro has not been converted? #define vbd_sz(_v) ((_v)->bdev->bd_part ? \ (_v)->bdev->bd_part->nr_sects : \ get_capacity((_v)->bdev->bd_disk)) Thanks, Bart.
On 06/17/2019 08:45 AM, Bart Van Assche wrote: > On 6/16/19 6:28 PM, Chaitanya Kulkarni wrote: >> In the blk-zoned, bcache, f2fs and blktrace implementation >> block device->hd_part->number of sectors field is accessed directly >> without any appropriate locking or accessor function. There is >> an existing accessor function present in the in include/linux/genhd.h >> which should be used to read the bdev->hd_part->nr_sects. > > In the subject, "nr_setcs" should probably be changed into "nr_sects"? > Yes, will fix it in next version. > Bart. >
On 06/17/2019 08:53 AM, Bart Van Assche wrote: > On 6/16/19 6:28 PM, Chaitanya Kulkarni wrote: >> Changes from V1:- >> >> 1. Drop the target_pscsi patch. (Bart) >> 2. Remove rcu locking which is not needed. (Bart) >> >> Chaitanya Kulkarni (7): >> block: add a helper function to read nr_setcs >> blk-zoned: update blkdev_nr_zones() with helper >> blk-zoned: update blkdev_report_zone() with helper >> blk-zoned: update blkdev_reset_zones() with helper >> bcache: update cached_dev_init() with helper >> f2fs: use helper in init_blkz_info() >> blktrace: use helper in blk_trace_setup_lba() >> >> block/blk-zoned.c | 12 ++++++------ >> drivers/md/bcache/super.c | 2 +- >> fs/f2fs/super.c | 2 +- >> include/linux/blkdev.h | 10 ++++++++++ >> kernel/trace/blktrace.c | 2 +- >> 5 files changed, 19 insertions(+), 9 deletions(-) > > My feedback about the pscsi_get_blocks() was misleading: what I meant is > that it is not necessary to introduce RCU locking in that function. I > think that using bdev_nr_sects() or part_nr_sects_read() to read > nr_sects in that function is useful. > My bad. I'll add pscsi patch. > Is there any reason that the following Xen macro has not been converted? > > #define vbd_sz(_v) ((_v)->bdev->bd_part ? \ > (_v)->bdev->bd_part->nr_sects : \ > get_capacity((_v)->bdev->bd_disk)) > I'll convert this too and appropriate mailing list. Thanks Bart again for detailed feedback. > Thanks, > > Bart. >
Hi, In the blk-zoned, bcache, f2fs and blktrace implementation block device->hd_part->number of sectors field is accessed directly without any appropriate locking or accessor function. There is an existing accessor function present in the in include/linux/genhd.h which should be used to read the bdev->hd_part->nr_sects. From ${KERN_DIR}/include/linux/genhd.h:- <snip> 714 /* 715 * Any access of part->nr_sects which is not protected by partition 716 * bd_mutex or gendisk bdev bd_mutex, should be done using this 717 * accessor function. 718 * 719 * Code written along the lines of i_size_read() and i_size_write(). 720 * CONFIG_PREEMPT case optimizes the case of UP kernel with preemption 721 * on. 722 */ 723 static inline sector_t part_nr_sects_read(struct hd_struct *part) 724 { <snip> This patch series introduces a helper function on the top of the part_nr_sects_read() and removes the all direct accesses to the bdev->hd_part->nr_sects for blk-zoned.c. This series is based on :- 1. Repo :- git://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git. 2. Branch :- for-next. Regards, Chaitanya Changes from V1:- 1. Drop the target_pscsi patch. (Bart) 2. Remove rcu locking which is not needed. (Bart) Chaitanya Kulkarni (7): block: add a helper function to read nr_setcs blk-zoned: update blkdev_nr_zones() with helper blk-zoned: update blkdev_report_zone() with helper blk-zoned: update blkdev_reset_zones() with helper bcache: update cached_dev_init() with helper f2fs: use helper in init_blkz_info() blktrace: use helper in blk_trace_setup_lba() block/blk-zoned.c | 12 ++++++------ drivers/md/bcache/super.c | 2 +- fs/f2fs/super.c | 2 +- include/linux/blkdev.h | 10 ++++++++++ kernel/trace/blktrace.c | 2 +- 5 files changed, 19 insertions(+), 9 deletions(-)