diff mbox series

loop: Use bdev limit helpers for configuring discard

Message ID 20241030111900.3981223-1-john.g.garry@oracle.com (mailing list archive)
State New
Headers show
Series loop: Use bdev limit helpers for configuring discard | expand

Commit Message

John Garry Oct. 30, 2024, 11:19 a.m. UTC
Instead of directly looking at the request_queue limits, use the bdev
limits helpers, which is preferable.

Signed-off-by: John Garry <john.g.garry@oracle.com>

Comments

Jens Axboe Oct. 30, 2024, 1:24 p.m. UTC | #1
On Wed, 30 Oct 2024 11:19:00 +0000, John Garry wrote:
> Instead of directly looking at the request_queue limits, use the bdev
> limits helpers, which is preferable.
> 
> 

Applied, thanks!

[1/1] loop: Use bdev limit helpers for configuring discard
      commit: 8d3fd059dd289e6c322e5741ad56794bcce699a2

Best regards,
Christoph Hellwig Oct. 30, 2024, 2:06 p.m. UTC | #2
On Wed, Oct 30, 2024 at 11:19:00AM +0000, John Garry wrote:
> +		granularity = bdev_discard_granularity(bdev) ?:
> +			bdev_physical_block_size(bdev);

The discard granularity is always set to at least the physical block
size, so this can be simplified to:

		granularity = bdev_discard_granularity(bdev);
John Garry Oct. 30, 2024, 2:13 p.m. UTC | #3
On 30/10/2024 14:06, Christoph Hellwig wrote:
> On Wed, Oct 30, 2024 at 11:19:00AM +0000, John Garry wrote:
>> +		granularity = bdev_discard_granularity(bdev) ?:
>> +			bdev_physical_block_size(bdev);
> The discard granularity is always set to at least the physical block
> size, so this can be simplified to:
> 
> 		granularity = bdev_discard_granularity(bdev);

ok, I see that set in blk_validate_limits()

Jens has already queued this and it is now buried beneath other patches, 
so I suppose that this change can be made in the follow on patch.

Cheers
John Garry Nov. 1, 2024, 9:08 a.m. UTC | #4
On 30/10/2024 14:13, John Garry wrote:
>> On Wed, Oct 30, 2024 at 11:19:00AM +0000, John Garry wrote:
>>> +        granularity = bdev_discard_granularity(bdev) ?:
>>> +            bdev_physical_block_size(bdev);
>> The discard granularity is always set to at least the physical block
>> size, so this can be simplified to:
>>
>>         granularity = bdev_discard_granularity(bdev);
> 
> ok, I see that set in blk_validate_limits()

BTW, can the check for granularity ever fail in 
queue_limit_discard_alignment()

static unsigned int queue_limit_discard_alignment(...)
{
	...

	granularity = lim->discard_granularity >> SECTOR_SHIFT;
	if (!granularity)
		return 0;
Christoph Hellwig Nov. 4, 2024, 7:47 a.m. UTC | #5
On Fri, Nov 01, 2024 at 09:08:51AM +0000, John Garry wrote:
>> ok, I see that set in blk_validate_limits()
>
> BTW, can the check for granularity ever fail in 
> queue_limit_discard_alignment()
>
> static unsigned int queue_limit_discard_alignment(...)
> {
> 	...
>
> 	granularity = lim->discard_granularity >> SECTOR_SHIFT;
> 	if (!granularity)
> 		return 0;

lim->discard_granularity is always set to SECTOR_SIZE or large,
so granularity can't be 0 here.
diff mbox series

Patch

diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 78a7bb28defe..7719858c49bb 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -786,11 +786,11 @@  static void loop_config_discard(struct loop_device *lo,
 	 * file-backed loop devices: discarded regions read back as zero.
 	 */
 	if (S_ISBLK(inode->i_mode)) {
-		struct request_queue *backingq = bdev_get_queue(I_BDEV(inode));
+		struct block_device *bdev = I_BDEV(inode);
 
-		max_discard_sectors = backingq->limits.max_write_zeroes_sectors;
-		granularity = bdev_discard_granularity(I_BDEV(inode)) ?:
-			queue_physical_block_size(backingq);
+		max_discard_sectors = bdev_write_zeroes_sectors(bdev);
+		granularity = bdev_discard_granularity(bdev) ?:
+			bdev_physical_block_size(bdev);
 
 	/*
 	 * We use punch hole to reclaim the free space used by the