On Mon, Mar 06, 2023 at 01:01:25PM +0100, Hannes Reinecke wrote: > Add a module option 'rd_blksize' to allow the user to change > the sector size of the RAM disks. > > Signed-off-by: Hannes Reinecke <hare@xxxxxxx> > --- > drivers/block/brd.c | 30 +++++++++++++++++++----------- > 1 file changed, 19 insertions(+), 11 deletions(-) > > diff --git a/drivers/block/brd.c b/drivers/block/brd.c > index fc41ea641dfb..11bac3c3f1b6 100644 > --- a/drivers/block/brd.c > +++ b/drivers/block/brd.c > @@ -46,6 +46,8 @@ struct brd_device { > spinlock_t brd_lock; > struct radix_tree_root brd_folios; > u64 brd_nr_folios; > + unsigned int brd_sector_shift; > + unsigned int brd_sector_size; > }; Why not just do this first and initialize this to the defaults set without the module parameter. Then you don't need to declare over and over unsigned int rd_sectors, etc in tons of routines and just pass the brd which most routines get. Then most of this and the prior patch can be squeezed into one. The functional changes would be the addition of the module parameter. > @@ -410,7 +418,7 @@ static int brd_alloc(int i) > * otherwise fdisk will align on 1M. Regardless this call > * is harmless) > */ > - blk_queue_physical_block_size(disk->queue, PAGE_SIZE); > + blk_queue_physical_block_size(disk->queue, rd_blksize); And this was added for DAX support, but DAX support was long removed from brd see commit 7a862fbbdec6 ("brd: remove dax support"). This nugget was added by Boaz via commit c8fa31730fc72a8 ("brd: Request from fdisk 4k alignment"), but if we don't support DAX, can't we just kill that line? Current doc for blk_queue_physical_block_size() says: * This should be set to the lowest possible sector size that the * hardware can operate on without reverting to read-modify-write * operations. But since we're working directly with RAM, do we care? The comment above that line referring to direct_access should be killed at the very least. While you're at it, can you then also update Documentation/filesystems/dax.rst to remove the brd as an example driver with DAX support. That leaves us with only a few block drivers with DAX: - dcssblk: s390 dcss block device driver - pmem: NVDIMM persistent memory driver - some dm targets - fuse virtio_fs Wonder which is the right example these days for DAX, now that persistant memory has End of Life'd with Optane dead, curious also of the value of the above and DAX in general other than support for whatever made it out. Should we EOL DAX too? Luis