Re: [PATCH 3/3] brd: implement write zeroes

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

 



On 7/19/2023 1:21 PM, Mikulas Patocka wrote:
> This patch implements REQ_OP_WRITE_ZEROES on brd. Write zeroes will free
> the pages just like discard, but the difference is that it writes zeroes
> to the preceding and following page if the range is not aligned on page
> boundary.
> 
> Signed-off-by: Mikulas Patocka <mpatocka@xxxxxxxxxx>
> 
> ---
>   drivers/block/brd.c |   22 ++++++++++++++++++----
>   1 file changed, 18 insertions(+), 4 deletions(-)
> 
> Index: linux-2.6/drivers/block/brd.c
> ===================================================================
> --- linux-2.6.orig/drivers/block/brd.c
> +++ linux-2.6/drivers/block/brd.c
> @@ -272,7 +272,8 @@ out:
>   
>   void brd_do_discard(struct brd_device *brd, struct bio *bio)
>   {
> -	sector_t sector, len, front_pad;
> +	bool zero_padding = bio_op(bio) == REQ_OP_WRITE_ZEROES;
> +	sector_t sector, len, front_pad, end_pad;
>   
>   	if (unlikely(!discard)) {
>   		bio->bi_status = BLK_STS_NOTSUPP;
> @@ -282,11 +283,22 @@ void brd_do_discard(struct brd_device *b
>   	sector = bio->bi_iter.bi_sector;
>   	len = bio_sectors(bio);
>   	front_pad = -sector & (PAGE_SECTORS - 1);
> +
> +	if (zero_padding && unlikely(front_pad != 0))
> +		copy_to_brd(brd, page_address(ZERO_PAGE(0)),
> +			    sector, min(len, front_pad) << SECTOR_SHIFT);
> +

Is it possible to create different interface for discard and write
zeroes ? I think it is a lot of logic adding on one function if everyone
else is okay please discard my comment ..

>   	sector += front_pad;
>   	if (unlikely(len <= front_pad))
>   		return;
>   	len -= front_pad;
> -	len = round_down(len, PAGE_SECTORS);
> +
> +	end_pad = len & (PAGE_SECTORS - 1);
> +	if (zero_padding && unlikely(end_pad != 0))
> +		copy_to_brd(brd, page_address(ZERO_PAGE(0)),
> +			    sector + len - end_pad, end_pad << SECTOR_SHIFT);
> +	len -= end_pad;
> +
>   	while (len) {
>   		brd_free_page(brd, sector);
>   		sector += PAGE_SECTORS;
> @@ -302,7 +314,8 @@ static void brd_submit_bio(struct bio *b
>   	struct bio_vec bvec;
>   	struct bvec_iter iter;
>   
> -	if (bio_op(bio) == REQ_OP_DISCARD) {
> +	if (bio_op(bio) == REQ_OP_DISCARD ||
> +	    bio_op(bio) == REQ_OP_WRITE_ZEROES) {
>   		brd_do_discard(brd, bio);
>   		goto endio;
>   	}

can we please use switch ? unless there is a strong need for if
which I failed to understand ...

> @@ -355,7 +368,7 @@ MODULE_PARM_DESC(max_part, "Num Minors t
>   
>   static bool discard = false;
>   module_param(discard, bool, 0444);
> -MODULE_PARM_DESC(discard, "Support discard");
> +MODULE_PARM_DESC(discard, "Support discard and write zeroes");
>  

write-zeroes and discards are both different req_op and they should have
a separate module parameter ...


>   MODULE_LICENSE("GPL");
>   MODULE_ALIAS_BLOCKDEV_MAJOR(RAMDISK_MAJOR);
> @@ -425,6 +438,7 @@ static int brd_alloc(int i)
>   	if (discard) {
>   		disk->queue->limits.discard_granularity = PAGE_SIZE;

>   		blk_queue_max_discard_sectors(disk->queue, round_down(UINT_MAX, PAGE_SECTORS));
> +		blk_queue_max_write_zeroes_sectors(disk->queue, round_down(UINT_MAX, PAGE_SECTORS));

above should be moved to new module parameter write_zeroes, unless there
is a reason to use one module parameter for both req_op...

>   	}
>   
>   	/* Tell the block layer that this is not a rotational device */
> 

-ck






[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