Re: [PATCH] mm/shmem: downgrade final i_blocks check in shmem_evict_inode() to pr_warn()

From: Jiacheng Yu

Date: Tue Jul 28 2026 - 23:43:17 EST


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@xxxxxxxxxx>
>> ---
>> 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);
>