[PATCH 2/3] dm bufio: remove redundant __GFP_NOWARN

Qianfeng Rong posted 3 patches 1 month, 3 weeks ago
[PATCH 2/3] dm bufio: remove redundant __GFP_NOWARN
Posted by Qianfeng Rong 1 month, 3 weeks ago
GFP_NOWAIT already includes __GFP_NOWARN, so let's remove the redundant
__GFP_NOWARN.  Also update comments to clarify the flag semantics.

Signed-off-by: Qianfeng Rong <rongqianfeng@vivo.com>
---
 drivers/md/dm-bufio.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/md/dm-bufio.c b/drivers/md/dm-bufio.c
index ff7595caf440..4b20854e92f5 100644
--- a/drivers/md/dm-bufio.c
+++ b/drivers/md/dm-bufio.c
@@ -1337,7 +1337,7 @@ static void use_bio(struct dm_buffer *b, enum req_op op, sector_t sector,
 	char *ptr;
 	unsigned int len;
 
-	bio = bio_kmalloc(1, GFP_NOWAIT | __GFP_NORETRY | __GFP_NOWARN);
+	bio = bio_kmalloc(1, GFP_NOWAIT | __GFP_NORETRY);
 	if (!bio) {
 		use_dmio(b, op, sector, n_sectors, offset, ioprio);
 		return;
@@ -1601,18 +1601,18 @@ static struct dm_buffer *__alloc_buffer_wait_no_callback(struct dm_bufio_client
 	 * dm-bufio is resistant to allocation failures (it just keeps
 	 * one buffer reserved in cases all the allocations fail).
 	 * So set flags to not try too hard:
-	 *	GFP_NOWAIT: don't wait; if we need to sleep we'll release our
-	 *		    mutex and wait ourselves.
+	 *	GFP_NOWAIT: don't wait and don't print a warning in case of
+	 *		    failure; if we need to sleep we'll release our mutex
+	 *		    and wait ourselves.
 	 *	__GFP_NORETRY: don't retry and rather return failure
 	 *	__GFP_NOMEMALLOC: don't use emergency reserves
-	 *	__GFP_NOWARN: don't print a warning in case of failure
 	 *
 	 * For debugging, if we set the cache size to 1, no new buffers will
 	 * be allocated.
 	 */
 	while (1) {
 		if (dm_bufio_cache_size_latch != 1) {
-			b = alloc_buffer(c, GFP_NOWAIT | __GFP_NORETRY | __GFP_NOMEMALLOC | __GFP_NOWARN);
+			b = alloc_buffer(c, GFP_NOWAIT | __GFP_NORETRY | __GFP_NOMEMALLOC);
 			if (b)
 				return b;
 		}
-- 
2.34.1
Re: [PATCH 2/3] dm bufio: remove redundant __GFP_NOWARN
Posted by Mikulas Patocka 1 month, 3 weeks ago
Hi

I think that GFP_NOWAIT already includes __GFP_NORETRY too. So, should we 
drop __GFP_NORETRY as well?

Mikulas


On Mon, 11 Aug 2025, Qianfeng Rong wrote:

> GFP_NOWAIT already includes __GFP_NOWARN, so let's remove the redundant
> __GFP_NOWARN.  Also update comments to clarify the flag semantics.
> 
> Signed-off-by: Qianfeng Rong <rongqianfeng@vivo.com>
> ---
>  drivers/md/dm-bufio.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/md/dm-bufio.c b/drivers/md/dm-bufio.c
> index ff7595caf440..4b20854e92f5 100644
> --- a/drivers/md/dm-bufio.c
> +++ b/drivers/md/dm-bufio.c
> @@ -1337,7 +1337,7 @@ static void use_bio(struct dm_buffer *b, enum req_op op, sector_t sector,
>  	char *ptr;
>  	unsigned int len;
>  
> -	bio = bio_kmalloc(1, GFP_NOWAIT | __GFP_NORETRY | __GFP_NOWARN);
> +	bio = bio_kmalloc(1, GFP_NOWAIT | __GFP_NORETRY);
>  	if (!bio) {
>  		use_dmio(b, op, sector, n_sectors, offset, ioprio);
>  		return;
> @@ -1601,18 +1601,18 @@ static struct dm_buffer *__alloc_buffer_wait_no_callback(struct dm_bufio_client
>  	 * dm-bufio is resistant to allocation failures (it just keeps
>  	 * one buffer reserved in cases all the allocations fail).
>  	 * So set flags to not try too hard:
> -	 *	GFP_NOWAIT: don't wait; if we need to sleep we'll release our
> -	 *		    mutex and wait ourselves.
> +	 *	GFP_NOWAIT: don't wait and don't print a warning in case of
> +	 *		    failure; if we need to sleep we'll release our mutex
> +	 *		    and wait ourselves.
>  	 *	__GFP_NORETRY: don't retry and rather return failure
>  	 *	__GFP_NOMEMALLOC: don't use emergency reserves
> -	 *	__GFP_NOWARN: don't print a warning in case of failure
>  	 *
>  	 * For debugging, if we set the cache size to 1, no new buffers will
>  	 * be allocated.
>  	 */
>  	while (1) {
>  		if (dm_bufio_cache_size_latch != 1) {
> -			b = alloc_buffer(c, GFP_NOWAIT | __GFP_NORETRY | __GFP_NOMEMALLOC | __GFP_NOWARN);
> +			b = alloc_buffer(c, GFP_NOWAIT | __GFP_NORETRY | __GFP_NOMEMALLOC);
>  			if (b)
>  				return b;
>  		}
> -- 
> 2.34.1
>
Re: [PATCH 2/3] dm bufio: remove redundant __GFP_NOWARN
Posted by Qianfeng Rong 1 month, 3 weeks ago
在 2025/8/11 20:44, Mikulas Patocka 写道:
> Hi
>
> I think that GFP_NOWAIT already includes __GFP_NORETRY too. So, should we
> drop __GFP_NORETRY as well?
GFP_NOWAIT does not include __GFP_NORETRY:
#define GFP_NOWAIT (__GFP_KSWAPD_RECLAIM | __GFP_NOWARN)

GFP_NOWAIT tells the memory manager to only wake up kswapd to perform
memory reclamation, not to perform direct memory reclaim.  Even if the
request fails, no error message is printed.

Best regards,
Qianfeng
>
> Mikulas
>
Re: [PATCH 2/3] dm bufio: remove redundant __GFP_NOWARN
Posted by Mikulas Patocka 1 month, 3 weeks ago

On Mon, 11 Aug 2025, Qianfeng Rong wrote:

> 
> 在 2025/8/11 20:44, Mikulas Patocka 写道:
> > Hi
> > 
> > I think that GFP_NOWAIT already includes __GFP_NORETRY too. So, should we
> > drop __GFP_NORETRY as well?
>
> GFP_NOWAIT does not include __GFP_NORETRY:
> #define GFP_NOWAIT (__GFP_KSWAPD_RECLAIM | __GFP_NOWARN)
> 
> GFP_NOWAIT tells the memory manager to only wake up kswapd to perform
> memory reclamation, not to perform direct memory reclaim.  Even if the
> request fails, no error message is printed.
> 
> Best regards,
> Qianfeng

Yes, but if GFP_NOWAIT allocation can't sleep, it can't retry - thus 
GFP_NOWAIT should imply __GFP_NORETRY.

Mikulas
Re: [PATCH 2/3] dm bufio: remove redundant __GFP_NOWARN
Posted by Qianfeng Rong 1 month, 3 weeks ago
在 2025/8/11 23:10, Mikulas Patocka 写道:
>
> On Mon, 11 Aug 2025, Qianfeng Rong wrote:
>
>> 在 2025/8/11 20:44, Mikulas Patocka 写道:
>>> Hi
>>>
>>> I think that GFP_NOWAIT already includes __GFP_NORETRY too. So, should we
>>> drop __GFP_NORETRY as well?
>> GFP_NOWAIT does not include __GFP_NORETRY:
>> #define GFP_NOWAIT (__GFP_KSWAPD_RECLAIM | __GFP_NOWARN)
>>
>> GFP_NOWAIT tells the memory manager to only wake up kswapd to perform
>> memory reclamation, not to perform direct memory reclaim.  Even if the
>> request fails, no error message is printed.
>>
>> Best regards,
>> Qianfeng
> Yes, but if GFP_NOWAIT allocation can't sleep, it can't retry - thus
> GFP_NOWAIT should imply __GFP_NORETRY.
>
> Mikulas
I strongly agree with your perspective, but I'm uncertain whether
GFP_NOWAIT should include __GFP_NORETRY, or if we should add a
note stating GFP_NOWAIT/GFP_ATOMIC should not be combined with
__GFP_NORETRY since it becomes redundant in atomic contexts.  I'll
raise this issue to see if others have insights.

Best regards,
Qianfeng