Re: [PATCH 3/5] brd: make sector size configurable

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux RAID]     [Linux SCSI]     [Linux ATA RAID]     [IDE]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Device Mapper]

  Powered by Linux