Re: [PATCH 6.18.y] mm: shmem: fix potential livelock issue for shmem direct swapin

[Baolin Wang <baolin.wang@linux.alibaba.com>]

On 7/7/26 10:34 PM, Barry Song wrote:
> On Tue, Jul 7, 2026 at 9:53 AM Baolin Wang
> <baolin.wang@linux.alibaba.com> wrote:
>>
>>
>>
>> On 7/6/26 9:04 PM, Barry Song wrote:
>>> On Mon, Jul 6, 2026 at 8:08 PM Baolin Wang
>>> <baolin.wang@linux.alibaba.com> wrote:
>>>>
>>>>
>>>>
>>>> On 7/6/26 1:59 PM, Kairui Song wrote:
>>>>> On Mon, Jul 6, 2026 at 11:25 AM Baolin Wang
>>>>> <baolin.wang@linux.alibaba.com> wrote:
>>>>>>
>>>>>> When skipping swapcache for synchronous IO swap devices, swapcache_prepare()
>>>>>> is used to prevent parallel swapin from proceeding with the swap cache flag.
>>>>>> However, on PREEMPT kernels this can lead to a livelock, as reported by Chao[1]:
>>>>>>
>>>>>> Thread A starts direct swapin of a shmem folio and calls swapcache_prepare()
>>>>>> to set SWAP_HAS_CACHE. It may then be preempted inside workingset_refault().
>>>>>> Meanwhile, a higher priority thread B also attempts direct swapin of the same
>>>>>> shmem swap entry. Since swapcache_prepare() already marks the entry, thread B
>>>>>> repeatedly gets -EEXIST and busy-loops waiting for thread A to finish. But as
>>>>>> thread B runs at higher priority, thread A cannot preempt it, resulting in
>>>>>> starvation and a livelock.
>>>>>>
>>>>>> Fix it by yielding the CPU with schedule_timeout_uninterruptible(1) when
>>>>>> swapcache_prepare() fails, following the same approach used in commits
>>>>>> 029c4628b2eb ("mm: swap: get rid of livelock in swapin readahead") and
>>>>>> 13ddaf26be32 ("mm/swap: fix race when skipping swapcache").
>>>>>>
>>>>>> Note that mainline does not have this potential issue, which has already been
>>>>>> resolved by Kairui's swap refactoring work[2].
>>>>>>
>>>>>> [1] https://lore.kernel.org/all/700a2cbf90a2484f979aac858f08f5d4@xiaomi.com/
>>>>>> [2] https://lore.kernel.org/all/20260517-swap-table-p4-v5-0-88ae43e064c7@tencent.com/
>>>>>> Fixes: 1dd44c0af4fa ("mm: shmem: skip swapcache for swapin of synchronous swap device")
>>>>>> Reported-by: Ma Chao <machao26@xiaomi.com>
>>>>>> Closes: https://lore.kernel.org/all/700a2cbf90a2484f979aac858f08f5d4@xiaomi.com/
>>>>>> Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com>
>>>>>> ---
>>>>>> Hi Chao, could you try this patch to check if it fixes your issue? Thanks.
>>>>>> ---
>>>>>>     mm/shmem.c | 2 ++
>>>>>>     1 file changed, 2 insertions(+)
>>>>>>
>>>>>> diff --git a/mm/shmem.c b/mm/shmem.c
>>>>>> index 94c5b0d78ac3..d4cb57b3b0ef 100644
>>>>>> --- a/mm/shmem.c
>>>>>> +++ b/mm/shmem.c
>>>>>> @@ -2066,6 +2066,8 @@ static struct folio *shmem_swap_alloc_folio(struct inode *inode,
>>>>>>            if (swapcache_prepare(entry, nr_pages)) {
>>>>>>                    folio_put(new);
>>>>>>                    new = ERR_PTR(-EEXIST);
>>>>>> +               /* Relax a bit to prevent rapid repeated page faults */
>>>>>> +               schedule_timeout_uninterruptible(1);
>>>>>>                    /* Try smaller folio to avoid cache conflict */
>>>>>>                    goto fallback;
>>>>>>            }
>>>>>> --
>>>>>> 2.47.3
>>>>>>
>>>>>
>>>>> Thanks! That's much more simpler than I expected. Do we need a wakeup
>>>>> queue like the one in commit 01626a1823024? Perhaps the reporter can
>>>>> help confirm and test? I personally prefer to keep it simple if shmem
>>>>> users aren't as sensitive as anon users.
>>>>
>>>> I agree. I'd like to keep the bugfix as simple as possible, if the
>>>> reporter's scenario isn't latency-sensitive.
>>>
>>> On Android, we don't see much shmem; it's much less common
>>> than anon. So the chance of this concurrency happening should
>>> be lower than for anon. However, shmem can be shared by
>>> multiple processes, so could this still happen if process A is
>>> blocked by process B?
>>
>> Could you be more specific about how that happens? I think we should fix
>> this starvation/livelock issue if you think it could still happen.
> 
> Hi Baolin,
> 
> I think your change has fixed the livelock issue, but an unconditional
> one-tick sleep could still be problematic, as commit 01626a1823 tried to
> address in do_swap_page():
> 
> "mm: avoid unconditional one-tick sleep when swapcache_prepare fails"
> 
> If possible, I would suggest that your fix also include the change from
> commit 01626a1823 to avoid the issue caused by
> schedule_timeout_uninterruptible(1): an unconditional one-tick sleep
> could cause UI stuttering. At least, this would make the code more
> defensive.

Sounds reasonable to me. Will do in v2. Thanks.