On Mon, May 22, 2023 at 12:09 AM Huang Ying <ying.huang@xxxxxxxxx> wrote: > > The general rule to use a swap entry is as follows. > > When we get a swap entry, if there isn't some other way to prevent > swapoff, such as page lock for swap cache, page table lock, 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. > > Add the rule as comments of get_swap_device(). > > Signed-off-by: "Huang, Ying" <ying.huang@xxxxxxxxx> > Cc: David Hildenbrand <david@xxxxxxxxxx> > Cc: Hugh Dickins <hughd@xxxxxxxxxx> > Cc: Johannes Weiner <hannes@xxxxxxxxxxx> > Cc: Matthew Wilcox <willy@xxxxxxxxxxxxx> > Cc: Michal Hocko <mhocko@xxxxxxxx> > Cc: Minchan Kim <minchan@xxxxxxxxxx> > Cc: Tim Chen <tim.c.chen@xxxxxxxxxxxxxxx> > Cc: Yang Shi <shy828301@xxxxxxxxx> > Cc: Yu Zhao <yuzhao@xxxxxxxxxx> > --- > mm/swapfile.c | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/mm/swapfile.c b/mm/swapfile.c > index 4dbaea64635d..0c1cb935b2eb 100644 > --- a/mm/swapfile.c > +++ b/mm/swapfile.c > @@ -1219,6 +1219,13 @@ static unsigned char __swap_entry_free_locked(struct swap_info_struct *p, > } > > /* > + * When we get a swap entry, if there isn't some other way to prevent > + * swapoff, such as page lock for swap cache, page table lock, 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. > + * > * Check whether swap entry is valid in the swap device. If so, > * return pointer to swap_info_struct, and keep the swap entry valid > * via preventing the swap device from being swapoff, until > @@ -1227,9 +1234,8 @@ static unsigned char __swap_entry_free_locked(struct swap_info_struct *p, > * Notice that swapoff or swapoff+swapon can still happen before the > * percpu_ref_tryget_live() in get_swap_device() or after the > * percpu_ref_put() in put_swap_device() if there isn't any other way > - * to prevent swapoff, such as page lock, page table lock, etc. The > - * caller must be prepared for that. For example, the following > - * situation is possible. > + * to prevent swapoff. The caller must be prepared for that. For > + * example, the following situation is possible. > * > * CPU1 CPU2 > * do_swap_page() > -- > 2.39.2 > > Thanks for clarifying the code! With David's comments: Reviewed-by: Yosry Ahmed <yosryahmed@xxxxxxxxxx>
