public inbox for linux-btrfs@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] btrfs-progs: mkfs: use device_discard_blocks() to replace prepare_discard_device()
@ 2026-01-08  4:42 Qu Wenruo
  2026-01-08  4:51 ` Roman Mamedov
  0 siblings, 1 reply; 5+ messages in thread
From: Qu Wenruo @ 2026-01-08  4:42 UTC (permalink / raw)
  To: linux-btrfs

The helper device_discard_blocks() is more generic, meanwhile
prepare_discard_device() is only utilized once to output a message.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 common/device-utils.c | 35 ++++++++++-------------------------
 1 file changed, 10 insertions(+), 25 deletions(-)

diff --git a/common/device-utils.c b/common/device-utils.c
index 9dfc50211955..e60196ec8e8b 100644
--- a/common/device-utils.c
+++ b/common/device-utils.c
@@ -80,29 +80,6 @@ int device_discard_blocks(int fd, u64 start, u64 len)
 	return 0;
 }
 
-static void prepare_discard_device(const char *filename, int fd, u64 byte_count, unsigned opflags)
-{
-	u64 cur = 0;
-
-	while (cur < byte_count) {
-		/* 1G granularity */
-		u64 chunk_size = (cur == 0) ? SZ_1M : min_t(u64, byte_count - cur, SZ_1G);
-		int ret;
-
-		ret = discard_range(fd, cur, chunk_size);
-		if (ret)
-			return;
-		/*
-		 * The first range discarded successfully, meaning the device supports
-		 * discard.
-		 */
-		if (opflags & PREP_DEVICE_VERBOSE && cur == 0)
-			printf("Performing full device TRIM %s (%s) ...\n",
-			       filename, pretty_size(byte_count));
-		cur += chunk_size;
-	}
-}
-
 /*
  * Write zeros to the given range [start, start + len)
  */
@@ -293,8 +270,16 @@ int btrfs_prepare_device(int fd, const char *file, u64 *byte_count_ret,
 		goto err;
 	}
 
-	if (!(opflags & PREP_DEVICE_ZONED) && (opflags & PREP_DEVICE_DISCARD))
-		prepare_discard_device(file, fd, byte_count, opflags);
+	if (!(opflags & PREP_DEVICE_ZONED) && (opflags & PREP_DEVICE_DISCARD)) {
+		ret = device_discard_blocks(fd, 0, byte_count);
+		if (ret < 0) {
+			errno = -ret;
+			warning("failed to discard device '%s': %m", file);
+		} else {
+			printf("Performing full device TRIM %s (%s) ...\n",
+			       file, pretty_size(byte_count));
+		}
+	}
 
 	ret = btrfs_wipe_existing_sb(fd, zinfo);
 	if (ret < 0) {
-- 
2.52.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] btrfs-progs: mkfs: use device_discard_blocks() to replace prepare_discard_device()
  2026-01-08  4:42 [PATCH] btrfs-progs: mkfs: use device_discard_blocks() to replace prepare_discard_device() Qu Wenruo
@ 2026-01-08  4:51 ` Roman Mamedov
  2026-01-08  4:54   ` Qu Wenruo
  0 siblings, 1 reply; 5+ messages in thread
From: Roman Mamedov @ 2026-01-08  4:51 UTC (permalink / raw)
  To: Qu Wenruo; +Cc: linux-btrfs

On Thu,  8 Jan 2026 15:12:17 +1030
Qu Wenruo <wqu@suse.com> wrote:

> -static void prepare_discard_device(const char *filename, int fd, u64 byte_count, unsigned opflags)
> -{
> -	u64 cur = 0;
> -
> -	while (cur < byte_count) {
> -		/* 1G granularity */
> -		u64 chunk_size = (cur == 0) ? SZ_1M : min_t(u64, byte_count - cur, SZ_1G);
> -		int ret;
> -
> -		ret = discard_range(fd, cur, chunk_size);
> -		if (ret)
> -			return;
> -		/*
> -		 * The first range discarded successfully, meaning the device supports
> -		 * discard.
> -		 */
> -		if (opflags & PREP_DEVICE_VERBOSE && cur == 0)
> -			printf("Performing full device TRIM %s (%s) ...\n",
> -			       filename, pretty_size(byte_count));
> -		cur += chunk_size;
> -	}
> -}
> -
>  /*
>   * Write zeros to the given range [start, start + len)
>   */
> @@ -293,8 +270,16 @@ int btrfs_prepare_device(int fd, const char *file, u64 *byte_count_ret,
>  		goto err;
>  	}
>  
> -	if (!(opflags & PREP_DEVICE_ZONED) && (opflags & PREP_DEVICE_DISCARD))
> -		prepare_discard_device(file, fd, byte_count, opflags);
> +	if (!(opflags & PREP_DEVICE_ZONED) && (opflags & PREP_DEVICE_DISCARD)) {
> +		ret = device_discard_blocks(fd, 0, byte_count);
> +		if (ret < 0) {
> +			errno = -ret;
> +			warning("failed to discard device '%s': %m", file);
> +		} else {
> +			printf("Performing full device TRIM %s (%s) ...\n",
> +			       file, pretty_size(byte_count));
> +		}
> +	}
>  
>  	ret = btrfs_wipe_existing_sb(fd, zinfo);
>  	if (ret < 0) {

Before: the message is printed after the first successful discard of a 1G
range, so with any real-world device at the very beginning of operation.

After: the message is printed only after the full device is discarded???
And it still implies the operation has just begun and is in progress.

It could take a significant time and it was good to print it in the beginning
to let the user know what is going on.

Or I am missing something here?

-- 
With respect,
Roman

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] btrfs-progs: mkfs: use device_discard_blocks() to replace prepare_discard_device()
  2026-01-08  4:51 ` Roman Mamedov
@ 2026-01-08  4:54   ` Qu Wenruo
  2026-01-08  5:10     ` Qu Wenruo
  0 siblings, 1 reply; 5+ messages in thread
From: Qu Wenruo @ 2026-01-08  4:54 UTC (permalink / raw)
  To: Roman Mamedov; +Cc: linux-btrfs



在 2026/1/8 15:21, Roman Mamedov 写道:
> On Thu,  8 Jan 2026 15:12:17 +1030
> Qu Wenruo <wqu@suse.com> wrote:
> 
>> -static void prepare_discard_device(const char *filename, int fd, u64 byte_count, unsigned opflags)
>> -{
>> -	u64 cur = 0;
>> -
>> -	while (cur < byte_count) {
>> -		/* 1G granularity */
>> -		u64 chunk_size = (cur == 0) ? SZ_1M : min_t(u64, byte_count - cur, SZ_1G);
>> -		int ret;
>> -
>> -		ret = discard_range(fd, cur, chunk_size);
>> -		if (ret)
>> -			return;
>> -		/*
>> -		 * The first range discarded successfully, meaning the device supports
>> -		 * discard.
>> -		 */
>> -		if (opflags & PREP_DEVICE_VERBOSE && cur == 0)
>> -			printf("Performing full device TRIM %s (%s) ...\n",
>> -			       filename, pretty_size(byte_count));
>> -		cur += chunk_size;
>> -	}
>> -}
>> -
>>   /*
>>    * Write zeros to the given range [start, start + len)
>>    */
>> @@ -293,8 +270,16 @@ int btrfs_prepare_device(int fd, const char *file, u64 *byte_count_ret,
>>   		goto err;
>>   	}
>>   
>> -	if (!(opflags & PREP_DEVICE_ZONED) && (opflags & PREP_DEVICE_DISCARD))
>> -		prepare_discard_device(file, fd, byte_count, opflags);
>> +	if (!(opflags & PREP_DEVICE_ZONED) && (opflags & PREP_DEVICE_DISCARD)) {
>> +		ret = device_discard_blocks(fd, 0, byte_count);
>> +		if (ret < 0) {
>> +			errno = -ret;
>> +			warning("failed to discard device '%s': %m", file);
>> +		} else {
>> +			printf("Performing full device TRIM %s (%s) ...\n",
>> +			       file, pretty_size(byte_count));
>> +		}
>> +	}
>>   
>>   	ret = btrfs_wipe_existing_sb(fd, zinfo);
>>   	if (ret < 0) {
> 
> Before: the message is printed after the first successful discard of a 1G
> range, so with any real-world device at the very beginning of operation.
> 
> After: the message is printed only after the full device is discarded???
> And it still implies the operation has just begun and is in progress.

I'll change the message timing to before the call.

> 
> It could take a significant time and it was good to print it in the beginning
> to let the user know what is going on.
> 
> Or I am missing something here?
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] btrfs-progs: mkfs: use device_discard_blocks() to replace prepare_discard_device()
  2026-01-08  4:54   ` Qu Wenruo
@ 2026-01-08  5:10     ` Qu Wenruo
  2026-01-08  5:38       ` Roman Mamedov
  0 siblings, 1 reply; 5+ messages in thread
From: Qu Wenruo @ 2026-01-08  5:10 UTC (permalink / raw)
  To: Roman Mamedov; +Cc: linux-btrfs



在 2026/1/8 15:24, Qu Wenruo 写道:
> 
> 
> 在 2026/1/8 15:21, Roman Mamedov 写道:
>> On Thu,  8 Jan 2026 15:12:17 +1030
>> Qu Wenruo <wqu@suse.com> wrote:
>>
>>> -static void prepare_discard_device(const char *filename, int fd, u64 
>>> byte_count, unsigned opflags)
>>> -{
>>> -    u64 cur = 0;
>>> -
>>> -    while (cur < byte_count) {
>>> -        /* 1G granularity */
>>> -        u64 chunk_size = (cur == 0) ? SZ_1M : min_t(u64, byte_count 
>>> - cur, SZ_1G);
>>> -        int ret;
>>> -
>>> -        ret = discard_range(fd, cur, chunk_size);
>>> -        if (ret)
>>> -            return;
>>> -        /*
>>> -         * The first range discarded successfully, meaning the 
>>> device supports
>>> -         * discard.
>>> -         */
>>> -        if (opflags & PREP_DEVICE_VERBOSE && cur == 0)
>>> -            printf("Performing full device TRIM %s (%s) ...\n",
>>> -                   filename, pretty_size(byte_count));
>>> -        cur += chunk_size;
>>> -    }
>>> -}
>>> -
>>>   /*
>>>    * Write zeros to the given range [start, start + len)
>>>    */
>>> @@ -293,8 +270,16 @@ int btrfs_prepare_device(int fd, const char 
>>> *file, u64 *byte_count_ret,
>>>           goto err;
>>>       }
>>> -    if (!(opflags & PREP_DEVICE_ZONED) && (opflags & 
>>> PREP_DEVICE_DISCARD))
>>> -        prepare_discard_device(file, fd, byte_count, opflags);
>>> +    if (!(opflags & PREP_DEVICE_ZONED) && (opflags & 
>>> PREP_DEVICE_DISCARD)) {
>>> +        ret = device_discard_blocks(fd, 0, byte_count);
>>> +        if (ret < 0) {
>>> +            errno = -ret;
>>> +            warning("failed to discard device '%s': %m", file);
>>> +        } else {
>>> +            printf("Performing full device TRIM %s (%s) ...\n",
>>> +                   file, pretty_size(byte_count));
>>> +        }
>>> +    }
>>>       ret = btrfs_wipe_existing_sb(fd, zinfo);
>>>       if (ret < 0) {
>>
>> Before: the message is printed after the first successful discard of a 1G
>> range, so with any real-world device at the very beginning of operation.
>>
>> After: the message is printed only after the full device is discarded???
>> And it still implies the operation has just begun and is in progress.
> 
> I'll change the message timing to before the call.

Well, this is a special handling for full device trim, where we don't do 
any special probing on if the device support TRIM.
But based on the first discard try to determine and output needed message.

Either we need a reliable way to do the probe (which is not that simple, 
previously we had some try to use 0 length discard, but it's not 
reliable), thus we changed to use the current way.

So it's not that easy to change without changing the timing of the 
message or the behavior.

Please ignore this patch.

Thanks,
Qu

> 
>>
>> It could take a significant time and it was good to print it in the 
>> beginning
>> to let the user know what is going on.
>>
>> Or I am missing something here?
>>
> 
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] btrfs-progs: mkfs: use device_discard_blocks() to replace prepare_discard_device()
  2026-01-08  5:10     ` Qu Wenruo
@ 2026-01-08  5:38       ` Roman Mamedov
  0 siblings, 0 replies; 5+ messages in thread
From: Roman Mamedov @ 2026-01-08  5:38 UTC (permalink / raw)
  To: Qu Wenruo; +Cc: linux-btrfs

On Thu, 8 Jan 2026 15:40:07 +1030
Qu Wenruo <wqu@suse.com> wrote:

> Well, this is a special handling for full device trim, where we don't do 
> any special probing on if the device support TRIM.
> But based on the first discard try to determine and output needed message.
> 
> Either we need a reliable way to do the probe (which is not that simple, 
> previously we had some try to use 0 length discard, but it's not 
> reliable), thus we changed to use the current way.
> 
> So it's not that easy to change without changing the timing of the 
> message or the behavior.

One idea would be to first call device_discard_blocks with "0, 1M" arguments
(or less than 1M, maybe even 4K), and if that succeeds then print the message
and call it again with the full device size.

But I am not certain if this will turn out simpler or more straightforward
than the current code.

-- 
With respect,
Roman

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-01-08  5:38 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-08  4:42 [PATCH] btrfs-progs: mkfs: use device_discard_blocks() to replace prepare_discard_device() Qu Wenruo
2026-01-08  4:51 ` Roman Mamedov
2026-01-08  4:54   ` Qu Wenruo
2026-01-08  5:10     ` Qu Wenruo
2026-01-08  5:38       ` Roman Mamedov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox