Nhat Pham <nphamcs@xxxxxxxxx> writes: > On Tue, Sep 24, 2024 at 6:53 PM Baolin Wang > <baolin.wang@xxxxxxxxxxxxxxxxx> wrote: >> >> >> One benefit I can mention is that removing 'SWAP_MAP_SHMEM' can help to >> batch free shmem swap entries in __swap_entries_free(), similar to the >> commit bea67dcc5eea ("mm: attempt to batch free swap entries for >> zap_pte_range()") did, which can improve the performance of shmem mTHP >> munmap() function based on my testing. > > Yeah, the problem with having an extraneous state is you have to > constantly check for it in code, and/or keep it in mind when you > develop things. I've been constantly having to check for this state > when I develop code around this area, and it gets old fast. > > If we can use it to optimize something, I can understand keeping it. > But it just seems like dead code to me :) > > My preference is to do this as simply as possible - add another case > (usage == 1, nr > 1, and we need to add swap continuations) in the > check in __swap_duplicate()'s first loop, and just WARN right there. > > That case CANNOT happen UNLESS we introduce a bug, or have a new use > case. When we actually have a use case, we can always introduce > handling/fallback logic for that case. > > Barry, Yosry, Baolin, Ying, how do you feel about this? Sounds good to me to just WARN now. We can always improve when it's necessary. -- Best Regards, Huang, Ying