Re: [PATCH v2 2/2] mm: shmem: make unused huge shrinker memcg aware

[Qi Zheng <qi.zheng@linux.dev>]

Hi Baolin,

On 7/29/26 11:29 AM, Baolin Wang wrote:
>>>> @@ -2532,28 +2727,6 @@ static int shmem_get_folio_gfp(struct inode 
>>>> *inode, pgoff_t index,
>>>>   alloced:
>>>>       alloced = true;
>>>> -    if (folio_test_large(folio) &&
>>>> -        DIV_ROUND_UP(i_size_read(inode), PAGE_SIZE) <
>>>> -                    folio_next_index(folio)) {
>>>> -        struct shmem_sb_info *sbinfo = SHMEM_SB(inode->i_sb);
>>>> -        struct shmem_inode_info *info = SHMEM_I(inode);
>>>> -        /*
>>>> -         * Part of the large folio is beyond i_size: subject
>>>> -         * to shrink under memory pressure.
>>>> -         */
>>>> -        spin_lock(&sbinfo->shrinklist_lock);
>>>> -        /*
>>>> -         * _careful to defend against unlocked access to
>>>> -         * ->shrink_list in shmem_unused_huge_shrink()
>>>> -         */
>>>> -        if (list_empty_careful(&info->shrinklist)) {
>>>> -            list_add_tail(&info->shrinklist,
>>>> -                      &sbinfo->shrinklist);
>>>> -            sbinfo->shrinklist_len++;
>>>> -        }
>>>> -        spin_unlock(&sbinfo->shrinklist_lock);
>>>> -    }
>>>
>>> Why move this additional logic down?
>>
>> Because under the following condition:
>>
>>          if (sgp <= SGP_CACHE &&
>>          ((loff_t)index << PAGE_SHIFT) >= i_size_read(inode)) {
>>          error = -EINVAL;
>>          goto unlock;
>>      }
>>
>> we will jump to unlock and remove the folio from page cache.
>>
>> With the new memcg-aware design, shmem_unused_huge_add() takes a memcg
>> reference. If the folio is then removed by the error path, the inode
>> sits on the shrinker list holding a stale memcg reference and pointing
>> at an i_size that no longer matches the folio.
>>
>> So we should ensure the inode is only queued when the folio is fully set
>> up and about to be returned successfully.
> 
> Right. But I think this deserves a separate preparation patch with above 
> explanation, moving the original shrinklist addition logic to after all 
> checks are completed.

Agree, will split this into a separate preparation patch.

Thanks,
Qi