2023-05-26 07:36:45

by Luis Chamberlain

[permalink] [raw]
Subject: [PATCH v2 0/5] block: simplify with PAGE_SECTORS_SHIFT

A bit of block drivers have their own incantations with
PAGE_SHIFT - SECTOR_SHIFT. Just simplfy and use PAGE_SECTORS_SHIFT
all over.

Based on linux-next next-20230525.

Changes since v1:

o keep iomap math visibly simple
o Add tags for Reviews/acks
o rebase onto next-20230525

Luis Chamberlain (5):
block: annotate bdev_disk_changed() deprecation with a symbol
namespace
drbd: use PAGE_SECTORS_SHIFT and PAGE_SECTORS
iomap: simplify iomap_init() with PAGE_SECTORS
dm bufio: simplify by using PAGE_SECTORS_SHIFT
zram: use generic PAGE_SECTORS and PAGE_SECTORS_SHIFT

block/partitions/core.c | 6 +-----
drivers/block/drbd/drbd_bitmap.c | 4 ++--
drivers/block/loop.c | 2 ++
drivers/block/zram/zram_drv.c | 12 ++++++------
drivers/block/zram/zram_drv.h | 2 --
drivers/md/dm-bufio.c | 4 ++--
drivers/s390/block/dasd_genhd.c | 2 ++
fs/iomap/buffered-io.c | 2 +-
8 files changed, 16 insertions(+), 18 deletions(-)

--
2.39.2



2023-05-26 07:44:29

by Luis Chamberlain

[permalink] [raw]
Subject: [PATCH v2 5/5] zram: use generic PAGE_SECTORS and PAGE_SECTORS_SHIFT

Instead of re-defining the already existing constants use the provided ones:

So replace:

o SECTORS_PER_PAGE_SHIFT with PAGE_SECTORS_SHIFT
o SECTORS_PER_PAGE with PAGE_SECTORS

This produces no functional changes.

Reviewed-by: Sergey Senozhatsky <[email protected]>
Signed-off-by: Luis Chamberlain <[email protected]>
---
drivers/block/zram/zram_drv.c | 12 ++++++------
drivers/block/zram/zram_drv.h | 2 --
2 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index f6d90f1ba5cf..5fdeb78ace9a 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -1834,8 +1834,8 @@ static ssize_t recompress_store(struct device *dev,
static void zram_bio_discard(struct zram *zram, struct bio *bio)
{
size_t n = bio->bi_iter.bi_size;
- u32 index = bio->bi_iter.bi_sector >> SECTORS_PER_PAGE_SHIFT;
- u32 offset = (bio->bi_iter.bi_sector & (SECTORS_PER_PAGE - 1)) <<
+ u32 index = bio->bi_iter.bi_sector >> PAGE_SECTORS_SHIFT;
+ u32 offset = (bio->bi_iter.bi_sector & (PAGE_SECTORS - 1)) <<
SECTOR_SHIFT;

/*
@@ -1876,8 +1876,8 @@ static void zram_bio_read(struct zram *zram, struct bio *bio)

start_time = bio_start_io_acct(bio);
bio_for_each_segment(bv, bio, iter) {
- u32 index = iter.bi_sector >> SECTORS_PER_PAGE_SHIFT;
- u32 offset = (iter.bi_sector & (SECTORS_PER_PAGE - 1)) <<
+ u32 index = iter.bi_sector >> PAGE_SECTORS_SHIFT;
+ u32 offset = (iter.bi_sector & (PAGE_SECTORS - 1)) <<
SECTOR_SHIFT;

if (zram_bvec_read(zram, &bv, index, offset, bio) < 0) {
@@ -1903,8 +1903,8 @@ static void zram_bio_write(struct zram *zram, struct bio *bio)

start_time = bio_start_io_acct(bio);
bio_for_each_segment(bv, bio, iter) {
- u32 index = iter.bi_sector >> SECTORS_PER_PAGE_SHIFT;
- u32 offset = (iter.bi_sector & (SECTORS_PER_PAGE - 1)) <<
+ u32 index = iter.bi_sector >> PAGE_SECTORS_SHIFT;
+ u32 offset = (iter.bi_sector & (PAGE_SECTORS - 1)) <<
SECTOR_SHIFT;

if (zram_bvec_write(zram, &bv, index, offset, bio) < 0) {
diff --git a/drivers/block/zram/zram_drv.h b/drivers/block/zram/zram_drv.h
index ca7a15bd4845..9f2543af5c76 100644
--- a/drivers/block/zram/zram_drv.h
+++ b/drivers/block/zram/zram_drv.h
@@ -21,8 +21,6 @@

#include "zcomp.h"

-#define SECTORS_PER_PAGE_SHIFT (PAGE_SHIFT - SECTOR_SHIFT)
-#define SECTORS_PER_PAGE (1 << SECTORS_PER_PAGE_SHIFT)
#define ZRAM_LOGICAL_BLOCK_SHIFT 12
#define ZRAM_LOGICAL_BLOCK_SIZE (1 << ZRAM_LOGICAL_BLOCK_SHIFT)
#define ZRAM_SECTOR_PER_LOGICAL_BLOCK \
--
2.39.2


2023-05-26 07:46:14

by Luis Chamberlain

[permalink] [raw]
Subject: [PATCH v2 1/5] block: annotate bdev_disk_changed() deprecation with a symbol namespace

This ensures no other users pop up by mistake easily and provides
us a with an easy vehicle to do the same with other routines should
we need it later.

Signed-off-by: Luis Chamberlain <[email protected]>
---
block/partitions/core.c | 6 +-----
drivers/block/loop.c | 2 ++
drivers/s390/block/dasd_genhd.c | 2 ++
3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/block/partitions/core.c b/block/partitions/core.c
index 49e0496ff23c..6f18444be4fe 100644
--- a/block/partitions/core.c
+++ b/block/partitions/core.c
@@ -690,11 +690,7 @@ int bdev_disk_changed(struct gendisk *disk, bool invalidate)

return ret;
}
-/*
- * Only exported for loop and dasd for historic reasons. Don't use in new
- * code!
- */
-EXPORT_SYMBOL_GPL(bdev_disk_changed);
+EXPORT_SYMBOL_NS_GPL(bdev_disk_changed, BLOCK_DEPRECATED);

void *read_part_sector(struct parsed_partitions *state, sector_t n, Sector *p)
{
diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index bc31bb7072a2..50aa5b4c0c56 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -37,6 +37,8 @@
#include <linux/spinlock.h>
#include <uapi/linux/loop.h>

+MODULE_IMPORT_NS(BLOCK_DEPRECATED);
+
/* Possible states of device */
enum {
Lo_unbound,
diff --git a/drivers/s390/block/dasd_genhd.c b/drivers/s390/block/dasd_genhd.c
index 998a961e1704..5ea244aec534 100644
--- a/drivers/s390/block/dasd_genhd.c
+++ b/drivers/s390/block/dasd_genhd.c
@@ -25,6 +25,8 @@

#include "dasd_int.h"

+MODULE_IMPORT_NS(BLOCK_DEPRECATED);
+
static unsigned int queue_depth = 32;
static unsigned int nr_hw_queues = 4;

--
2.39.2


2023-05-26 08:21:05

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH v2 1/5] block: annotate bdev_disk_changed() deprecation with a symbol namespace

On Fri, May 26, 2023 at 12:33:32AM -0700, Luis Chamberlain wrote:
> This ensures no other users pop up by mistake easily and provides
> us a with an easy vehicle to do the same with other routines should
> we need it later.

I don't see how this is related to the rest of the seris. I also don't
think it's a good idea. The APIs isn't deprecated per se. It just
should not be called by drivers. The right thing would be an interface
like

EXPORT_SYMBOL_GPL_FOR(bdev_disk_changed, loop.ko, CONFIG_BLK_DEV_LOOP);
EXPORT_SYMBOL_GPL_FOR(bdev_disk_changed, dasd_mod.ko, CONFIG_DASD);

with the modulo code enforcing that no one but the module this is
explicitly exorted for can use the symbol.

2023-05-26 08:35:24

by Luis Chamberlain

[permalink] [raw]
Subject: Re: [PATCH v2 1/5] block: annotate bdev_disk_changed() deprecation with a symbol namespace

On Fri, May 26, 2023 at 01:13:14AM -0700, Christoph Hellwig wrote:
> On Fri, May 26, 2023 at 12:33:32AM -0700, Luis Chamberlain wrote:
> > This ensures no other users pop up by mistake easily and provides
> > us a with an easy vehicle to do the same with other routines should
> > we need it later.
>
> I don't see how this is related to the rest of the seris.

Jessh, sorry it is too late here and that was a typo that the commit
went into the series. I'll go sleep now. This I just had queued
as a reminder for the new annotation for deprecated symbols to be
used in some situations.

Please ignore this patch.

Luis

2023-05-26 09:32:49

by Johannes Thumshirn

[permalink] [raw]
Subject: Re: [PATCH v2 5/5] zram: use generic PAGE_SECTORS and PAGE_SECTORS_SHIFT

On 26.05.23 09:34, Luis Chamberlain wrote:
> + u32 index = bio->bi_iter.bi_sector >> PAGE_SECTORS_SHIFT;
> + u32 offset = (bio->bi_iter.bi_sector & (PAGE_SECTORS - 1)) <<
> SECTOR_SHIFT;

(PAGE_SECTORS - 1) is SECTOR_MASK, please use this.

Thanks,
Johannes