Messages in this thread | | | From | Kairui Song <> | Date | Thu, 23 Nov 2023 19:13:27 +0800 | Subject | Re: [PATCH 16/24] mm/swap: reduce scope of get_swap_device in swapin path |
| |
Huang, Ying <ying.huang@intel.com> 于2023年11月22日周三 08:38写道: > > Kairui Song <ryncsn@gmail.com> writes: > > > From: Kairui Song <kasong@tencent.com> > > > > Move get_swap_device into swapin_readahead, simplify the code > > and prepare for follow up commits. > > No. Please don't do this. Please check the get/put_swap_device() usage > rule in the comments of get_swap_device(). > > " > * When we get a swap entry, if there aren't some other ways to > * prevent swapoff, such as the folio in swap cache is locked, page > * table lock is held, etc., the swap entry may become invalid because > * of swapoff. Then, we need to enclose all swap related functions > * with get_swap_device() and put_swap_device(), unless the swap > * functions call get/put_swap_device() by themselves. > " > > This is to simplify the reasoning about swapoff and swap entry. > > Why does it bother you?
Hi Ying,
This is trying to reduce LOC, avoid a trivial si read, and make error checking logic easier to refactor in later commits.
And besides there is one trivial change I forgot to include in this commit, get_swap_device can be put after swap_cache_get_folio in swapin_readahead, since swap_cache_get_folio doesn't need to hold the swap device, so in cache hit case this get/put_swap_device() can be saved.
The comment also mentioned:
"Then, we need to enclose all swap related functions with get_swap_device() and put_swap_device(), unless the swap functions call get/put_swap_device() by themselves"
So I think it should be OK to do this.
| |