[PATCH v4 4/6] mm/huge_memory: convert VM_BUG* to VM_WARN* in __folio_split.

Zi Yan posted 6 patches 2 months, 2 weeks ago
There is a newer version of this series
[PATCH v4 4/6] mm/huge_memory: convert VM_BUG* to VM_WARN* in __folio_split.
Posted by Zi Yan 2 months, 2 weeks ago
These VM_BUG* can be handled gracefully without crashing kernel.

Signed-off-by: Zi Yan <ziy@nvidia.com>
---
 mm/huge_memory.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index d36f7bdaeb38..d6ff5e8c89d7 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -3601,8 +3601,14 @@ static int __folio_split(struct folio *folio, unsigned int new_order,
 	pgoff_t end;
 	bool is_hzp;
 
-	VM_BUG_ON_FOLIO(!folio_test_locked(folio), folio);
-	VM_BUG_ON_FOLIO(!folio_test_large(folio), folio);
+	if (!folio_test_locked(folio)) {
+		VM_WARN_ON_ONCE_FOLIO(1, folio);
+		return -EINVAL;
+	}
+	if (!folio_test_large(folio)) {
+		VM_WARN_ON_ONCE_FOLIO(1, folio);
+		return -EINVAL;
+	}
 
 	if (folio != page_folio(split_at) || folio != page_folio(lock_at))
 		return -EINVAL;
@@ -3766,7 +3772,11 @@ static int __folio_split(struct folio *folio, unsigned int new_order,
 		}
 
 		if (folio_test_swapcache(folio)) {
-			VM_BUG_ON(mapping);
+			if (mapping) {
+				VM_WARN_ON_ONCE_FOLIO(mapping, folio);
+				ret = -EINVAL;
+				goto fail;
+			}
 
 			swap_cache = swap_address_space(folio->swap);
 			xa_lock(&swap_cache->i_pages);
-- 
2.47.2
Re: [PATCH v4 4/6] mm/huge_memory: convert VM_BUG* to VM_WARN* in __folio_split.
Posted by Lorenzo Stoakes 2 months, 2 weeks ago
On Thu, Jul 17, 2025 at 10:29:58PM -0400, Zi Yan wrote:
> These VM_BUG* can be handled gracefully without crashing kernel.
>
> Signed-off-by: Zi Yan <ziy@nvidia.com>

This LGTM, but obviously this is predicated on David being happy re: his reply
but from my side:

Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>

> ---
>  mm/huge_memory.c | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index d36f7bdaeb38..d6ff5e8c89d7 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -3601,8 +3601,14 @@ static int __folio_split(struct folio *folio, unsigned int new_order,
>  	pgoff_t end;
>  	bool is_hzp;
>
> -	VM_BUG_ON_FOLIO(!folio_test_locked(folio), folio);
> -	VM_BUG_ON_FOLIO(!folio_test_large(folio), folio);
> +	if (!folio_test_locked(folio)) {
> +		VM_WARN_ON_ONCE_FOLIO(1, folio);
> +		return -EINVAL;
> +	}
> +	if (!folio_test_large(folio)) {
> +		VM_WARN_ON_ONCE_FOLIO(1, folio);
> +		return -EINVAL;
> +	}
>
>  	if (folio != page_folio(split_at) || folio != page_folio(lock_at))
>  		return -EINVAL;
> @@ -3766,7 +3772,11 @@ static int __folio_split(struct folio *folio, unsigned int new_order,
>  		}
>
>  		if (folio_test_swapcache(folio)) {
> -			VM_BUG_ON(mapping);
> +			if (mapping) {
> +				VM_WARN_ON_ONCE_FOLIO(mapping, folio);
> +				ret = -EINVAL;
> +				goto fail;
> +			}
>
>  			swap_cache = swap_address_space(folio->swap);
>  			xa_lock(&swap_cache->i_pages);
> --
> 2.47.2
>
Re: [PATCH v4 4/6] mm/huge_memory: convert VM_BUG* to VM_WARN* in __folio_split.
Posted by David Hildenbrand 2 months, 2 weeks ago
On 18.07.25 04:29, Zi Yan wrote:
> These VM_BUG* can be handled gracefully without crashing kernel.
> 
> Signed-off-by: Zi Yan <ziy@nvidia.com>
> ---
>   mm/huge_memory.c | 16 +++++++++++++---
>   1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index d36f7bdaeb38..d6ff5e8c89d7 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -3601,8 +3601,14 @@ static int __folio_split(struct folio *folio, unsigned int new_order,
>   	pgoff_t end;
>   	bool is_hzp;
>   
> -	VM_BUG_ON_FOLIO(!folio_test_locked(folio), folio);
> -	VM_BUG_ON_FOLIO(!folio_test_large(folio), folio);
> +	if (!folio_test_locked(folio)) {
> +		VM_WARN_ON_ONCE_FOLIO(1, folio);
> +		return -EINVAL;
> +	}
> +	if (!folio_test_large(folio)) {
> +		VM_WARN_ON_ONCE_FOLIO(1, folio);
> +		return -EINVAL;
> +	}

For cases that we handle gracefully you usually want to use

if (WARN_ON_ONCE(..))

because then you get actually notified when that unexpected thing happens.

I am not really sure if recovery is warranted here: smells like a 
straight VM_WARN_ON_ONCE_FOLIO() is sufficient, and catching this early 
during development that something is extremely off.

-- 
Cheers,

David / dhildenb
Re: [PATCH v4 4/6] mm/huge_memory: convert VM_BUG* to VM_WARN* in __folio_split.
Posted by Zi Yan 2 months, 2 weeks ago
On 18 Jul 2025, at 3:22, David Hildenbrand wrote:

> On 18.07.25 04:29, Zi Yan wrote:
>> These VM_BUG* can be handled gracefully without crashing kernel.
>>
>> Signed-off-by: Zi Yan <ziy@nvidia.com>
>> ---
>>   mm/huge_memory.c | 16 +++++++++++++---
>>   1 file changed, 13 insertions(+), 3 deletions(-)
>>
>> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
>> index d36f7bdaeb38..d6ff5e8c89d7 100644
>> --- a/mm/huge_memory.c
>> +++ b/mm/huge_memory.c
>> @@ -3601,8 +3601,14 @@ static int __folio_split(struct folio *folio, unsigned int new_order,
>>   	pgoff_t end;
>>   	bool is_hzp;
>>  -	VM_BUG_ON_FOLIO(!folio_test_locked(folio), folio);
>> -	VM_BUG_ON_FOLIO(!folio_test_large(folio), folio);
>> +	if (!folio_test_locked(folio)) {
>> +		VM_WARN_ON_ONCE_FOLIO(1, folio);
>> +		return -EINVAL;
>> +	}
>> +	if (!folio_test_large(folio)) {
>> +		VM_WARN_ON_ONCE_FOLIO(1, folio);
>> +		return -EINVAL;
>> +	}
>
> For cases that we handle gracefully you usually want to use
>
> if (WARN_ON_ONCE(..))

Got it.

>
> because then you get actually notified when that unexpected thing happens.
>
> I am not really sure if recovery is warranted here: smells like a straight VM_WARN_ON_ONCE_FOLIO() is sufficient, and catching this early during development that something is extremely off.

OK. I will update it to just VM_WARN_ON_ONCE_FOLIO().

Thanks.

Best Regards,
Yan, Zi