| Message ID | 20231228075545.362768-8-hch@lst.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/9] block: remove two comments in bio_split_discard | expand |
On (23/12/28 07:55), Christoph Hellwig wrote: > > The discard granularity now defaults to a single sector, so don't set > that value explicitly Hmm, but sector size != PAGE_SIZE [..] > @@ -2227,7 +2227,6 @@ static int zram_add(void) > ZRAM_LOGICAL_BLOCK_SIZE); > blk_queue_io_min(zram->disk->queue, PAGE_SIZE); > blk_queue_io_opt(zram->disk->queue, PAGE_SIZE); > - zram->disk->queue->limits.discard_granularity = PAGE_SIZE;
On 1/1/24 6:15 PM, Sergey Senozhatsky wrote: > On (23/12/28 07:55), Christoph Hellwig wrote: >> >> The discard granularity now defaults to a single sector, so don't set >> that value explicitly > > Hmm, but sector size != PAGE_SIZE > > [..] > >> @@ -2227,7 +2227,6 @@ static int zram_add(void) >> ZRAM_LOGICAL_BLOCK_SIZE); >> blk_queue_io_min(zram->disk->queue, PAGE_SIZE); >> blk_queue_io_opt(zram->disk->queue, PAGE_SIZE); >> - zram->disk->queue->limits.discard_granularity = PAGE_SIZE; Yep, that does indeed look buggy.
On 1/2/24 8:40 AM, Jens Axboe wrote: > On 1/1/24 6:15 PM, Sergey Senozhatsky wrote: >> On (23/12/28 07:55), Christoph Hellwig wrote: >>> >>> The discard granularity now defaults to a single sector, so don't set >>> that value explicitly >> >> Hmm, but sector size != PAGE_SIZE >> >> [..] >> >>> @@ -2227,7 +2227,6 @@ static int zram_add(void) >>> ZRAM_LOGICAL_BLOCK_SIZE); >>> blk_queue_io_min(zram->disk->queue, PAGE_SIZE); >>> blk_queue_io_opt(zram->disk->queue, PAGE_SIZE); >>> - zram->disk->queue->limits.discard_granularity = PAGE_SIZE; > > Yep, that does indeed look buggy. Ah no it's fine, it'll default to the sector size of the device, which is set before the discard limit/granularity. So should work fine as-is.
On 1/2/24 8:44 AM, Jens Axboe wrote: > On 1/2/24 8:40 AM, Jens Axboe wrote: >> On 1/1/24 6:15 PM, Sergey Senozhatsky wrote: >>> On (23/12/28 07:55), Christoph Hellwig wrote: >>>> >>>> The discard granularity now defaults to a single sector, so don't set >>>> that value explicitly >>> >>> Hmm, but sector size != PAGE_SIZE >>> >>> [..] >>> >>>> @@ -2227,7 +2227,6 @@ static int zram_add(void) >>>> ZRAM_LOGICAL_BLOCK_SIZE); >>>> blk_queue_io_min(zram->disk->queue, PAGE_SIZE); >>>> blk_queue_io_opt(zram->disk->queue, PAGE_SIZE); >>>> - zram->disk->queue->limits.discard_granularity = PAGE_SIZE; >> >> Yep, that does indeed look buggy. > > Ah no it's fine, it'll default to the sector size of the device, which > is set before the discard limit/granularity. So should work fine as-is. Replying to myself again... It does then default it to the logical block size, not the physical one. Which does look odd, seems like that should be the physical size? In any case, this does change behavior for zram. Christoph?
On (23/12/28 07:55), Christoph Hellwig wrote: > > The discard granularity now defaults to a single sector, so don't set > that value explicitly. > > Signed-off-by: Christoph Hellwig <hch@lst.de> FWIW, Reviewed-by: Sergey Senozhatsky <senozhatsky@chromium.org>
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c index d77d3664ca0805..e1dec0483a012b 100644 --- a/drivers/block/zram/zram_drv.c +++ b/drivers/block/zram/zram_drv.c @@ -2227,7 +2227,6 @@ static int zram_add(void) ZRAM_LOGICAL_BLOCK_SIZE); blk_queue_io_min(zram->disk->queue, PAGE_SIZE); blk_queue_io_opt(zram->disk->queue, PAGE_SIZE); - zram->disk->queue->limits.discard_granularity = PAGE_SIZE; blk_queue_max_discard_sectors(zram->disk->queue, UINT_MAX); /*
The discard granularity now defaults to a single sector, so don't set that value explicitly. Signed-off-by: Christoph Hellwig <hch@lst.de> --- drivers/block/zram/zram_drv.c | 1 - 1 file changed, 1 deletion(-)