Huang, Ying <ying.huang@xxxxxxxxx> 于2024年1月8日周一 16:28写道: > > Kairui Song <ryncsn@xxxxxxxxx> writes: > > > From: Kairui Song <kasong@xxxxxxxxxxx> > > > > Since all callers of swapin_entry need to check the swap cache first, we > > can merge this common routine into swapin_entry, so it can be shared and > > optimized later. > > > > Also introduce a enum to better represent possible swap cache usage, and > > add some comments about it, make the usage of swap cache easier to > > understand. > > I don't find any benefit to do this. The code line number isn't > reduced. The concept of swap cache isn't hided either. Hi Ying Thanks for the comments. Maybe I should squash this with the following commit? The following commit want to do cache lookup in swapin_entry to avoid a redundant shadow lookup, it can help to improve the performance by about 4% for swapin path. So it need to return a enum to represent cache status. Further more, note the comments added here: +/* + * Caller of swapin_entry may need to know the cache lookup result: + * + * SWAP_CACHE_HIT: cache hit, cached folio is retured. + * SWAP_CACHE_MISS: cache miss, folio is allocated, read from swap device + * and adde to swap cache, but still may return a cached + * folio if raced (check __read_swap_cache_async). + * SWAP_CACHE_BYPASS: cache miss, folio is new allocated and read + * from swap device bypassing the cache. + */ SWAP_CACHE_MISS might be inaccurate, this is not an issue introduced by this commit, but better exposed. May worth a fix later. So far I can see two benefits fixing it: - More accurate maj/min page fault count. - Note the PageHWPoison check in do_swap_page, it ignored the race case, if a page getting poisoned is raced with swapcache then it may not work as expected. These are all minor issue indeed, some other optimization might also be doable, but at least a comment might be helpful. > -- > Best Regards, > Huang, Ying >