mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] mm/shmem: downgrade final i_blocks check in shmem_evict_inode() to pr_warn()
@ 2026-07-28  9:10 Jiacheng Yu
  2026-07-28 19:49 ` Andrew Morton
  2026-07-29  3:14 ` [PATCH] mm/shmem: downgrade final i_blocks check in shmem_evict_inode() to pr_warn() Baolin Wang
  0 siblings, 2 replies; 5+ messages in thread
From: Jiacheng Yu @ 2026-07-28  9:10 UTC (permalink / raw)
  To: hughd
  Cc: baolin.wang, akpm, linux-mm, linux-kernel, liuyongqiang13, Jiacheng Yu

shmem_evict_inode() ends with WARN_ON(inode->i_blocks) as a final
consistency check of shmem's block accounting.  When it fires, the
inode-local counters die with the inode; what may linger is a small
residue in accounting kept outside the inode, such as per-mount or
per-user charges.  No data is lost, and no corruption follows.

On kernels running with panic_on_warn=1, this accounting inconsistency
escalates to a full machine panic, which is disproportionate to the impact.

Downgrade the WARN_ON() to a pr_warn() that reports the inode together
with its accounting counters (i_blocks, alloced, swapped, nrpages),
keeping the inconsistency visible in the logs.

The accounting bugs this check has caught over the years -- the
swapout race described in commit 0f3c42f522dc ("tmpfs: change
final i_blocks BUG to WARNING") and the error recovery race fixed
in commit 267a4c76bbdb ("tmpfs: fix shmem_evict_inode() warnings
on i_blocks") -- are real and should still be fixed; this change
only removes the disproportionate escalation.

Fixes: 0f3c42f522dc ("tmpfs: change final i_blocks BUG to WARNING")
Signed-off-by: Jiacheng Yu <yujiacheng3@huawei.com>
---
One way to hit this race: soft_offline_in_use_page()'s fast path drops
a clean, unmapped shmem folio via mapping_evict_folio(), where the
xas_store() and the nrpages decrement are not atomic against a
concurrent shmem_evict_inode(); the final shmem_recalc_inode() can
then read the pre-decrement nrpages, compute freed = 0, and leave one 
page charged.  Same class as the races in 0f3c42f522dc and            
267a4c76bbdb, this time in the under-count direction; reproduced on   
7.2-rc4 with madvise(MADV_SOFT_OFFLINE) racing MAP_FIXED replacement  
of a shared-anonymous VMA.

 mm/shmem.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/mm/shmem.c b/mm/shmem.c
index b51f83c970bb..223484cdc8f2 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -1428,7 +1428,11 @@ static void shmem_evict_inode(struct inode *inode)
 	simple_xattrs_free(&sbinfo->xa_cache, &info->xattrs, sbinfo->max_inodes ? &freed : NULL);
 
 	shmem_free_inode(inode->i_sb, freed);
-	WARN_ON(inode->i_blocks);
+	if (inode->i_blocks)
+		pr_warn("%s: ino=%llu i_blocks=%llu alloced=%lu swapped=%lu nrpages=%lu\n",
+			__func__, (unsigned long long)inode->i_ino,
+			(unsigned long long)inode->i_blocks,
+			info->alloced, info->swapped, inode->i_mapping->nrpages);
 	clear_inode(inode);
 #ifdef CONFIG_TMPFS_QUOTA
 	dquot_free_inode(inode);
-- 
2.34.1


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

* Re: [PATCH] mm/shmem: downgrade final i_blocks check in shmem_evict_inode() to pr_warn()
  2026-07-28  9:10 [PATCH] mm/shmem: downgrade final i_blocks check in shmem_evict_inode() to pr_warn() Jiacheng Yu
@ 2026-07-28 19:49 ` Andrew Morton
  2026-07-29 12:12   ` [PATCH] mm/shmem: drop redundant casts in shmem_evict_inode() pr_warn Jiacheng Yu
  2026-07-29  3:14 ` [PATCH] mm/shmem: downgrade final i_blocks check in shmem_evict_inode() to pr_warn() Baolin Wang
  1 sibling, 1 reply; 5+ messages in thread
From: Andrew Morton @ 2026-07-28 19:49 UTC (permalink / raw)
  To: Jiacheng Yu
  Cc: hughd, baolin.wang, linux-mm, linux-kernel, liuyongqiang13,
	Christian Brauner

On Tue, 28 Jul 2026 09:10:14 +0000 Jiacheng Yu <yujiacheng3@huawei.com> wrote:

> shmem_evict_inode() ends with WARN_ON(inode->i_blocks) as a final
> consistency check of shmem's block accounting.  When it fires, the
> inode-local counters die with the inode; what may linger is a small
> residue in accounting kept outside the inode, such as per-mount or
> per-user charges.  No data is lost, and no corruption follows.
> 
> On kernels running with panic_on_warn=1, this accounting inconsistency
> escalates to a full machine panic, which is disproportionate to the impact.
> 
> Downgrade the WARN_ON() to a pr_warn() that reports the inode together
> with its accounting counters (i_blocks, alloced, swapped, nrpages),
> keeping the inconsistency visible in the logs.

Fair enough.

Does anyone actually use panic_on_warn=1?

> The accounting bugs this check has caught over the years -- the
> swapout race described in commit 0f3c42f522dc ("tmpfs: change
> final i_blocks BUG to WARNING") and the error recovery race fixed
> in commit 267a4c76bbdb ("tmpfs: fix shmem_evict_inode() warnings
> on i_blocks") -- are real and should still be fixed; this change
> only removes the disproportionate escalation.

Boy we have a lot of bugs :(

> Fixes: 0f3c42f522dc ("tmpfs: change final i_blocks BUG to WARNING")
> Signed-off-by: Jiacheng Yu <yujiacheng3@huawei.com>
> ---
> One way to hit this race: soft_offline_in_use_page()'s fast path drops
> a clean, unmapped shmem folio via mapping_evict_folio(), where the
> xas_store() and the nrpages decrement are not atomic against a
> concurrent shmem_evict_inode(); the final shmem_recalc_inode() can
> then read the pre-decrement nrpages, compute freed = 0, and leave one 
> page charged.  Same class as the races in 0f3c42f522dc and            
> 267a4c76bbdb, this time in the under-count direction; reproduced on   
> 7.2-rc4 with madvise(MADV_SOFT_OFFLINE) racing MAP_FIXED replacement  
> of a shared-anonymous VMA.

This paragraph is nice - I'll move it into the permanent changelog.

> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -1428,7 +1428,11 @@ static void shmem_evict_inode(struct inode *inode)
>  	simple_xattrs_free(&sbinfo->xa_cache, &info->xattrs, sbinfo->max_inodes ? &freed : NULL);
>  
>  	shmem_free_inode(inode->i_sb, freed);
> -	WARN_ON(inode->i_blocks);
> +	if (inode->i_blocks)
> +		pr_warn("%s: ino=%llu i_blocks=%llu alloced=%lu swapped=%lu nrpages=%lu\n",
> +			__func__, (unsigned long long)inode->i_ino,
> +			(unsigned long long)inode->i_blocks,
> +			info->alloced, info->swapped, inode->i_mapping->nrpages);

I don't think the i_ino cast is needed?

blkcnt_t was possibly 32-bit a long time ago but afaict it's now
unconditionally u64.

>  	clear_inode(inode);
>  #ifdef CONFIG_TMPFS_QUOTA
>  	dquot_free_inode(inode);
> -- 
> 2.34.1

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

* Re: [PATCH] mm/shmem: downgrade final i_blocks check in shmem_evict_inode() to pr_warn()
  2026-07-28  9:10 [PATCH] mm/shmem: downgrade final i_blocks check in shmem_evict_inode() to pr_warn() Jiacheng Yu
  2026-07-28 19:49 ` Andrew Morton
@ 2026-07-29  3:14 ` Baolin Wang
  2026-07-29  3:37   ` Jiacheng Yu
  1 sibling, 1 reply; 5+ messages in thread
From: Baolin Wang @ 2026-07-29  3:14 UTC (permalink / raw)
  To: Jiacheng Yu, hughd; +Cc: akpm, linux-mm, linux-kernel, liuyongqiang13



On 7/28/26 5:10 PM, Jiacheng Yu wrote:
> shmem_evict_inode() ends with WARN_ON(inode->i_blocks) as a final
> consistency check of shmem's block accounting.  When it fires, the
> inode-local counters die with the inode; what may linger is a small
> residue in accounting kept outside the inode, such as per-mount or
> per-user charges.  No data is lost, and no corruption follows.
> 
> On kernels running with panic_on_warn=1, this accounting inconsistency
> escalates to a full machine panic, which is disproportionate to the impact.
> 
> Downgrade the WARN_ON() to a pr_warn() that reports the inode together
> with its accounting counters (i_blocks, alloced, swapped, nrpages),
> keeping the inconsistency visible in the logs.
> 
> The accounting bugs this check has caught over the years -- the
> swapout race described in commit 0f3c42f522dc ("tmpfs: change
> final i_blocks BUG to WARNING") and the error recovery race fixed
> in commit 267a4c76bbdb ("tmpfs: fix shmem_evict_inode() warnings
> on i_blocks") -- are real and should still be fixed; this change
> only removes the disproportionate escalation.
> 
> Fixes: 0f3c42f522dc ("tmpfs: change final i_blocks BUG to WARNING")
> Signed-off-by: Jiacheng Yu <yujiacheng3@huawei.com>
> ---
> One way to hit this race: soft_offline_in_use_page()'s fast path drops
> a clean, unmapped shmem folio via mapping_evict_folio(), where the
> xas_store() and the nrpages decrement are not atomic against a
> concurrent shmem_evict_inode(); the final shmem_recalc_inode() can
> then read the pre-decrement nrpages, compute freed = 0, and leave one
> page charged.  Same class as the races in 0f3c42f522dc and
> 267a4c76bbdb, this time in the under-count direction; reproduced on
> 7.2-rc4 with madvise(MADV_SOFT_OFFLINE) racing MAP_FIXED replacement
> of a shared-anonymous VMA.

Shouldn't we fix the race first instead of adding more warning information?

>   mm/shmem.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/shmem.c b/mm/shmem.c
> index b51f83c970bb..223484cdc8f2 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -1428,7 +1428,11 @@ static void shmem_evict_inode(struct inode *inode)
>   	simple_xattrs_free(&sbinfo->xa_cache, &info->xattrs, sbinfo->max_inodes ? &freed : NULL);
>   
>   	shmem_free_inode(inode->i_sb, freed);
> -	WARN_ON(inode->i_blocks);
> +	if (inode->i_blocks)
> +		pr_warn("%s: ino=%llu i_blocks=%llu alloced=%lu swapped=%lu nrpages=%lu\n",
> +			__func__, (unsigned long long)inode->i_ino,
> +			(unsigned long long)inode->i_blocks,
> +			info->alloced, info->swapped, inode->i_mapping->nrpages);
>   	clear_inode(inode);
>   #ifdef CONFIG_TMPFS_QUOTA
>   	dquot_free_inode(inode);


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

* Re: [PATCH] mm/shmem: downgrade final i_blocks check in shmem_evict_inode() to pr_warn()
  2026-07-29  3:14 ` [PATCH] mm/shmem: downgrade final i_blocks check in shmem_evict_inode() to pr_warn() Baolin Wang
@ 2026-07-29  3:37   ` Jiacheng Yu
  0 siblings, 0 replies; 5+ messages in thread
From: Jiacheng Yu @ 2026-07-29  3:37 UTC (permalink / raw)
  To: Baolin Wang, hughd; +Cc: akpm, linux-mm, linux-kernel, liuyongqiang13

On 29/07/2026 11:14, Baolin Wang wrote:
> 
> 
> On 7/28/26 5:10 PM, Jiacheng Yu wrote:
>> shmem_evict_inode() ends with WARN_ON(inode->i_blocks) as a final
>> consistency check of shmem's block accounting.  When it fires, the
>> inode-local counters die with the inode; what may linger is a small
>> residue in accounting kept outside the inode, such as per-mount or
>> per-user charges.  No data is lost, and no corruption follows.
>>
>> On kernels running with panic_on_warn=1, this accounting inconsistency
>> escalates to a full machine panic, which is disproportionate to the impact.
>>
>> Downgrade the WARN_ON() to a pr_warn() that reports the inode together
>> with its accounting counters (i_blocks, alloced, swapped, nrpages),
>> keeping the inconsistency visible in the logs.
>>
>> The accounting bugs this check has caught over the years -- the
>> swapout race described in commit 0f3c42f522dc ("tmpfs: change
>> final i_blocks BUG to WARNING") and the error recovery race fixed
>> in commit 267a4c76bbdb ("tmpfs: fix shmem_evict_inode() warnings
>> on i_blocks") -- are real and should still be fixed; this change
>> only removes the disproportionate escalation.
>>
>> Fixes: 0f3c42f522dc ("tmpfs: change final i_blocks BUG to WARNING")
>> Signed-off-by: Jiacheng Yu <yujiacheng3@huawei.com>
>> ---
>> One way to hit this race: soft_offline_in_use_page()'s fast path drops
>> a clean, unmapped shmem folio via mapping_evict_folio(), where the
>> xas_store() and the nrpages decrement are not atomic against a
>> concurrent shmem_evict_inode(); the final shmem_recalc_inode() can
>> then read the pre-decrement nrpages, compute freed = 0, and leave one
>> page charged.  Same class as the races in 0f3c42f522dc and
>> 267a4c76bbdb, this time in the under-count direction; reproduced on
>> 7.2-rc4 with madvise(MADV_SOFT_OFFLINE) racing MAP_FIXED replacement
>> of a shared-anonymous VMA.
> 
> Shouldn't we fix the race first instead of adding more warning information?

Yes, the race should be fixed, ant that needs more time and discussion.
Back in 0f3c42f522dc Hugh already outlined the two candidate          
directions -- coherent locking between nrpages and the shmem counters,
or eliminating shmem_recalc_inode() altogether.

I plan to follow up with a fix attempt along those directions;

> 
>>   mm/shmem.c | 6 +++++-
>>   1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/mm/shmem.c b/mm/shmem.c
>> index b51f83c970bb..223484cdc8f2 100644
>> --- a/mm/shmem.c
>> +++ b/mm/shmem.c
>> @@ -1428,7 +1428,11 @@ static void shmem_evict_inode(struct inode *inode)
>>       simple_xattrs_free(&sbinfo->xa_cache, &info->xattrs, sbinfo->max_inodes ? &freed : NULL);
>>         shmem_free_inode(inode->i_sb, freed);
>> -    WARN_ON(inode->i_blocks);
>> +    if (inode->i_blocks)
>> +        pr_warn("%s: ino=%llu i_blocks=%llu alloced=%lu swapped=%lu nrpages=%lu\n",
>> +            __func__, (unsigned long long)inode->i_ino,
>> +            (unsigned long long)inode->i_blocks,
>> +            info->alloced, info->swapped, inode->i_mapping->nrpages);
>>       clear_inode(inode);
>>   #ifdef CONFIG_TMPFS_QUOTA
>>       dquot_free_inode(inode);
> 


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

* [PATCH] mm/shmem: drop redundant casts in shmem_evict_inode() pr_warn
  2026-07-28 19:49 ` Andrew Morton
@ 2026-07-29 12:12   ` Jiacheng Yu
  0 siblings, 0 replies; 5+ messages in thread
From: Jiacheng Yu @ 2026-07-29 12:12 UTC (permalink / raw)
  To: akpm
  Cc: hughd, baolin.wang, linux-mm, linux-kernel, liuyongqiang13,
	brauner, Jiacheng Yu

blkcnt_t and i_ino are u64, %llu matches both directly, so drop the casts.

Signed-off-by: Jiacheng Yu <yujiacheng3@huawei.com>
---

On Tue, 28 Jul 2026 Andrew Morton wrote:

> I don't think the i_ino cast is needed?
>
> blkcnt_t was possibly 32-bit a long time ago but
> afaict it's now unconditionally u64.

yes -- i_ino is u64 since 0b2600f81cef ("treewide: change inode->i_ino
from unsigned long to u64"), and blkcnt_t is unconditionally u64,
so both casts can go.

 mm/shmem.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/mm/shmem.c b/mm/shmem.c
index 223484cdc8f2..96cf73c906df 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -1430,8 +1430,7 @@ static void shmem_evict_inode(struct inode *inode)
 	shmem_free_inode(inode->i_sb, freed);
 	if (inode->i_blocks)
 		pr_warn("%s: ino=%llu i_blocks=%llu alloced=%lu swapped=%lu nrpages=%lu\n",
-			__func__, (unsigned long long)inode->i_ino,
-			(unsigned long long)inode->i_blocks,
+			__func__, inode->i_ino, inode->i_blocks,
 			info->alloced, info->swapped, inode->i_mapping->nrpages);
 	clear_inode(inode);
 #ifdef CONFIG_TMPFS_QUOTA
-- 
2.34.1


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

end of thread, other threads:[~2026-07-29 11:05 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-07-28  9:10 [PATCH] mm/shmem: downgrade final i_blocks check in shmem_evict_inode() to pr_warn() Jiacheng Yu
2026-07-28 19:49 ` Andrew Morton
2026-07-29 12:12   ` [PATCH] mm/shmem: drop redundant casts in shmem_evict_inode() pr_warn Jiacheng Yu
2026-07-29  3:14 ` [PATCH] mm/shmem: downgrade final i_blocks check in shmem_evict_inode() to pr_warn() Baolin Wang
2026-07-29  3:37   ` Jiacheng Yu

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

all inboxes | Powered by JetHome®