[PATCH] block: fix the initial value of wp_offset for npo2 zone size

[PATCH] block: fix the initial value of wp_offset for npo2 zone size

[LongPing Wei]

The zone size of Zoned UFS may be not power of 2.
It should be better to get wp_offset by bio_offset_from_zone_start
 like disk_zone_wplug_abort_unaligned.

Fixes: dd291d77cc90 ("block: Introduce zone write plugging")
Signed-off-by: LongPing Wei <weilongping@oppo.com>
---
https://lore.kernel.org/linux-block/20220923173618.6899-2-p.raghav@samsung.com/

bdev_nr_zones/disk_zone_no could support npo2 zone size by this commit.
---
 block/blk-zoned.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/blk-zoned.c b/block/blk-zoned.c
index af19296fa50d..41b302dfd437 100644
--- a/block/blk-zoned.c
+++ b/block/blk-zoned.c
@@ -537,7 +537,7 @@ static struct blk_zone_wplug *disk_get_and_lock_zone_wplug(struct gendisk *disk,
 	spin_lock_init(&zwplug->lock);
 	zwplug->flags = 0;
 	zwplug->zone_no = zno;
-	zwplug->wp_offset = sector & (disk->queue->limits.chunk_sectors - 1);
+	zwplug->wp_offset = bio_offset_from_zone_start(sector);
 	bio_list_init(&zwplug->bio_list);
 	INIT_WORK(&zwplug->bio_work, blk_zone_wplug_bio_work);
 	zwplug->disk = disk;
-- 
2.34.1

Re: [PATCH] block: fix the initial value of wp_offset for npo2 zone size

[Christoph Hellwig]

On Tue, Nov 05, 2024 at 06:11:22PM +0800, LongPing Wei wrote:
> The zone size of Zoned UFS may be not power of 2.

.. which is not supported by Linux to start with.

Re: [PATCH] block: fix the initial value of wp_offset for npo2 zone size

[kernel test robot]

Hi LongPing,

kernel test robot noticed the following build errors:

[auto build test ERROR on axboe-block/for-next]
[also build test ERROR on linus/master v6.12-rc6 next-20241105]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/LongPing-Wei/block-fix-the-initial-value-of-wp_offset-for-npo2-zone-size/20241105-181423
base:   https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next
patch link:    https://lore.kernel.org/r/20241105101120.1207567-1-weilongping%40oppo.com
patch subject: [PATCH] block: fix the initial value of wp_offset for npo2 zone size
config: x86_64-buildonly-randconfig-002-20241106 (https://download.01.org/0day-ci/archive/20241106/202411060445.EVXgwLvF-lkp@intel.com/config)
compiler: clang version 19.1.3 (https://github.com/llvm/llvm-project ab51eccf88f5321e7c60591c5546b254b6afab99)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241106/202411060445.EVXgwLvF-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202411060445.EVXgwLvF-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from block/blk-zoned.c:15:
   In file included from include/linux/blkdev.h:9:
   In file included from include/linux/blk_types.h:10:
   In file included from include/linux/bvec.h:10:
   In file included from include/linux/highmem.h:8:
   In file included from include/linux/cacheflush.h:5:
   In file included from arch/x86/include/asm/cacheflush.h:5:
   In file included from include/linux/mm.h:2213:
   include/linux/vmstat.h:504:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     504 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     505 |                            item];
         |                            ~~~~
   include/linux/vmstat.h:511:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     511 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     512 |                            NR_VM_NUMA_EVENT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~~
   include/linux/vmstat.h:518:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
     518 |         return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
         |                               ~~~~~~~~~~~ ^ ~~~
   include/linux/vmstat.h:524:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     524 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     525 |                            NR_VM_NUMA_EVENT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~~
>> block/blk-zoned.c:540:49: error: incompatible integer to pointer conversion passing 'sector_t' (aka 'unsigned long long') to parameter of type 'struct bio *' [-Wint-conversion]
     540 |         zwplug->wp_offset = bio_offset_from_zone_start(sector);
         |                                                        ^~~~~~
   include/linux/blkdev.h:1371:63: note: passing argument to parameter 'bio' here
    1371 | static inline sector_t bio_offset_from_zone_start(struct bio *bio)
         |                                                               ^
   4 warnings and 1 error generated.


vim +540 block/blk-zoned.c

   494	
   495	/*
   496	 * Get a reference on the write plug for the zone containing @sector.
   497	 * If the plug does not exist, it is allocated and hashed.
   498	 * Return a pointer to the zone write plug with the plug spinlock held.
   499	 */
   500	static struct blk_zone_wplug *disk_get_and_lock_zone_wplug(struct gendisk *disk,
   501						sector_t sector, gfp_t gfp_mask,
   502						unsigned long *flags)
   503	{
   504		unsigned int zno = disk_zone_no(disk, sector);
   505		struct blk_zone_wplug *zwplug;
   506	
   507	again:
   508		zwplug = disk_get_zone_wplug(disk, sector);
   509		if (zwplug) {
   510			/*
   511			 * Check that a BIO completion or a zone reset or finish
   512			 * operation has not already removed the zone write plug from
   513			 * the hash table and dropped its reference count. In such case,
   514			 * we need to get a new plug so start over from the beginning.
   515			 */
   516			spin_lock_irqsave(&zwplug->lock, *flags);
   517			if (zwplug->flags & BLK_ZONE_WPLUG_UNHASHED) {
   518				spin_unlock_irqrestore(&zwplug->lock, *flags);
   519				disk_put_zone_wplug(zwplug);
   520				goto again;
   521			}
   522			return zwplug;
   523		}
   524	
   525		/*
   526		 * Allocate and initialize a zone write plug with an extra reference
   527		 * so that it is not freed when the zone write plug becomes idle without
   528		 * the zone being full.
   529		 */
   530		zwplug = mempool_alloc(disk->zone_wplugs_pool, gfp_mask);
   531		if (!zwplug)
   532			return NULL;
   533	
   534		INIT_HLIST_NODE(&zwplug->node);
   535		INIT_LIST_HEAD(&zwplug->link);
   536		atomic_set(&zwplug->ref, 2);
   537		spin_lock_init(&zwplug->lock);
   538		zwplug->flags = 0;
   539		zwplug->zone_no = zno;
 > 540		zwplug->wp_offset = bio_offset_from_zone_start(sector);
   541		bio_list_init(&zwplug->bio_list);
   542		INIT_WORK(&zwplug->bio_work, blk_zone_wplug_bio_work);
   543		zwplug->disk = disk;
   544	
   545		spin_lock_irqsave(&zwplug->lock, *flags);
   546	
   547		/*
   548		 * Insert the new zone write plug in the hash table. This can fail only
   549		 * if another context already inserted a plug. Retry from the beginning
   550		 * in such case.
   551		 */
   552		if (!disk_insert_zone_wplug(disk, zwplug)) {
   553			spin_unlock_irqrestore(&zwplug->lock, *flags);
   554			mempool_free(zwplug, disk->zone_wplugs_pool);
   555			goto again;
   556		}
   557	
   558		return zwplug;
   559	}
   560	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH] block: fix the initial value of wp_offset for npo2 zone size

[kernel test robot]

Hi LongPing,

kernel test robot noticed the following build warnings:

[auto build test WARNING on axboe-block/for-next]
[also build test WARNING on linus/master v6.12-rc6 next-20241105]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/LongPing-Wei/block-fix-the-initial-value-of-wp_offset-for-npo2-zone-size/20241105-181423
base:   https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next
patch link:    https://lore.kernel.org/r/20241105101120.1207567-1-weilongping%40oppo.com
patch subject: [PATCH] block: fix the initial value of wp_offset for npo2 zone size
config: x86_64-rhel-8.3-func (https://download.01.org/0day-ci/archive/20241106/202411060401.MveGdWg2-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241106/202411060401.MveGdWg2-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202411060401.MveGdWg2-lkp@intel.com/

All warnings (new ones prefixed by >>):

   block/blk-zoned.c: In function 'disk_get_and_lock_zone_wplug':
>> block/blk-zoned.c:540:56: warning: passing argument 1 of 'bio_offset_from_zone_start' makes pointer from integer without a cast [-Wint-conversion]
     540 |         zwplug->wp_offset = bio_offset_from_zone_start(sector);
         |                                                        ^~~~~~
         |                                                        |
         |                                                        sector_t {aka long long unsigned int}
   In file included from block/blk-zoned.c:15:
   include/linux/blkdev.h:1371:63: note: expected 'struct bio *' but argument is of type 'sector_t' {aka 'long long unsigned int'}
    1371 | static inline sector_t bio_offset_from_zone_start(struct bio *bio)
         |                                                   ~~~~~~~~~~~~^~~


vim +/bio_offset_from_zone_start +540 block/blk-zoned.c

   494	
   495	/*
   496	 * Get a reference on the write plug for the zone containing @sector.
   497	 * If the plug does not exist, it is allocated and hashed.
   498	 * Return a pointer to the zone write plug with the plug spinlock held.
   499	 */
   500	static struct blk_zone_wplug *disk_get_and_lock_zone_wplug(struct gendisk *disk,
   501						sector_t sector, gfp_t gfp_mask,
   502						unsigned long *flags)
   503	{
   504		unsigned int zno = disk_zone_no(disk, sector);
   505		struct blk_zone_wplug *zwplug;
   506	
   507	again:
   508		zwplug = disk_get_zone_wplug(disk, sector);
   509		if (zwplug) {
   510			/*
   511			 * Check that a BIO completion or a zone reset or finish
   512			 * operation has not already removed the zone write plug from
   513			 * the hash table and dropped its reference count. In such case,
   514			 * we need to get a new plug so start over from the beginning.
   515			 */
   516			spin_lock_irqsave(&zwplug->lock, *flags);
   517			if (zwplug->flags & BLK_ZONE_WPLUG_UNHASHED) {
   518				spin_unlock_irqrestore(&zwplug->lock, *flags);
   519				disk_put_zone_wplug(zwplug);
   520				goto again;
   521			}
   522			return zwplug;
   523		}
   524	
   525		/*
   526		 * Allocate and initialize a zone write plug with an extra reference
   527		 * so that it is not freed when the zone write plug becomes idle without
   528		 * the zone being full.
   529		 */
   530		zwplug = mempool_alloc(disk->zone_wplugs_pool, gfp_mask);
   531		if (!zwplug)
   532			return NULL;
   533	
   534		INIT_HLIST_NODE(&zwplug->node);
   535		INIT_LIST_HEAD(&zwplug->link);
   536		atomic_set(&zwplug->ref, 2);
   537		spin_lock_init(&zwplug->lock);
   538		zwplug->flags = 0;
   539		zwplug->zone_no = zno;
 > 540		zwplug->wp_offset = bio_offset_from_zone_start(sector);
   541		bio_list_init(&zwplug->bio_list);
   542		INIT_WORK(&zwplug->bio_work, blk_zone_wplug_bio_work);
   543		zwplug->disk = disk;
   544	
   545		spin_lock_irqsave(&zwplug->lock, *flags);
   546	
   547		/*
   548		 * Insert the new zone write plug in the hash table. This can fail only
   549		 * if another context already inserted a plug. Retry from the beginning
   550		 * in such case.
   551		 */
   552		if (!disk_insert_zone_wplug(disk, zwplug)) {
   553			spin_unlock_irqrestore(&zwplug->lock, *flags);
   554			mempool_free(zwplug, disk->zone_wplugs_pool);
   555			goto again;
   556		}
   557	
   558		return zwplug;
   559	}
   560	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH] block: fix the initial value of wp_offset for npo2 zone size

[kernel test robot]

Hi LongPing,

kernel test robot noticed the following build errors:

[auto build test ERROR on axboe-block/for-next]
[also build test ERROR on linus/master v6.12-rc6 next-20241105]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/LongPing-Wei/block-fix-the-initial-value-of-wp_offset-for-npo2-zone-size/20241105-181423
base:   https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next
patch link:    https://lore.kernel.org/r/20241105101120.1207567-1-weilongping%40oppo.com
patch subject: [PATCH] block: fix the initial value of wp_offset for npo2 zone size
config: arm-allmodconfig (https://download.01.org/0day-ci/archive/20241106/202411060615.u1WE9nlu-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 14.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241106/202411060615.u1WE9nlu-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202411060615.u1WE9nlu-lkp@intel.com/

All errors (new ones prefixed by >>):

   block/blk-zoned.c: In function 'disk_get_and_lock_zone_wplug':
>> block/blk-zoned.c:540:56: error: passing argument 1 of 'bio_offset_from_zone_start' makes pointer from integer without a cast [-Wint-conversion]
     540 |         zwplug->wp_offset = bio_offset_from_zone_start(sector);
         |                                                        ^~~~~~
         |                                                        |
         |                                                        sector_t {aka long long unsigned int}
   In file included from block/blk-zoned.c:15:
   include/linux/blkdev.h:1371:63: note: expected 'struct bio *' but argument is of type 'sector_t' {aka 'long long unsigned int'}
    1371 | static inline sector_t bio_offset_from_zone_start(struct bio *bio)
         |                                                   ~~~~~~~~~~~~^~~


vim +/bio_offset_from_zone_start +540 block/blk-zoned.c

   494	
   495	/*
   496	 * Get a reference on the write plug for the zone containing @sector.
   497	 * If the plug does not exist, it is allocated and hashed.
   498	 * Return a pointer to the zone write plug with the plug spinlock held.
   499	 */
   500	static struct blk_zone_wplug *disk_get_and_lock_zone_wplug(struct gendisk *disk,
   501						sector_t sector, gfp_t gfp_mask,
   502						unsigned long *flags)
   503	{
   504		unsigned int zno = disk_zone_no(disk, sector);
   505		struct blk_zone_wplug *zwplug;
   506	
   507	again:
   508		zwplug = disk_get_zone_wplug(disk, sector);
   509		if (zwplug) {
   510			/*
   511			 * Check that a BIO completion or a zone reset or finish
   512			 * operation has not already removed the zone write plug from
   513			 * the hash table and dropped its reference count. In such case,
   514			 * we need to get a new plug so start over from the beginning.
   515			 */
   516			spin_lock_irqsave(&zwplug->lock, *flags);
   517			if (zwplug->flags & BLK_ZONE_WPLUG_UNHASHED) {
   518				spin_unlock_irqrestore(&zwplug->lock, *flags);
   519				disk_put_zone_wplug(zwplug);
   520				goto again;
   521			}
   522			return zwplug;
   523		}
   524	
   525		/*
   526		 * Allocate and initialize a zone write plug with an extra reference
   527		 * so that it is not freed when the zone write plug becomes idle without
   528		 * the zone being full.
   529		 */
   530		zwplug = mempool_alloc(disk->zone_wplugs_pool, gfp_mask);
   531		if (!zwplug)
   532			return NULL;
   533	
   534		INIT_HLIST_NODE(&zwplug->node);
   535		INIT_LIST_HEAD(&zwplug->link);
   536		atomic_set(&zwplug->ref, 2);
   537		spin_lock_init(&zwplug->lock);
   538		zwplug->flags = 0;
   539		zwplug->zone_no = zno;
 > 540		zwplug->wp_offset = bio_offset_from_zone_start(sector);
   541		bio_list_init(&zwplug->bio_list);
   542		INIT_WORK(&zwplug->bio_work, blk_zone_wplug_bio_work);
   543		zwplug->disk = disk;
   544	
   545		spin_lock_irqsave(&zwplug->lock, *flags);
   546	
   547		/*
   548		 * Insert the new zone write plug in the hash table. This can fail only
   549		 * if another context already inserted a plug. Retry from the beginning
   550		 * in such case.
   551		 */
   552		if (!disk_insert_zone_wplug(disk, zwplug)) {
   553			spin_unlock_irqrestore(&zwplug->lock, *flags);
   554			mempool_free(zwplug, disk->zone_wplugs_pool);
   555			goto again;
   556		}
   557	
   558		return zwplug;
   559	}
   560	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki