Huang, Ying <ying.huang@xxxxxxxxx> 于2023年11月22日周三 08:38写道: > > Kairui Song <ryncsn@xxxxxxxxx> writes: > > > From: Kairui Song <kasong@xxxxxxxxxxx> > > > > 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.