> > In swap_range_free, we want to make sure that the write to > > si->inuse_pages in swap_range_free() happens *after* the cleanups > > (specifically zswap_invalidate() in this case). > > In swap_off, we want to make sure that the cleanups following > > try_to_unuse() (e.g. zswap_swapoff) happen *after* reading > > si->inuse_pages == 0 in try_to_unuse(). > > > > So I think we want smp_wmb() in swap_range_free() and smp_mb() in > > try_to_unuse(). Does the below look correct to you? > > > > diff --git a/mm/swapfile.c b/mm/swapfile.c > > index 2fedb148b9404..a2fa2f65a8ddd 100644 > > --- a/mm/swapfile.c > > +++ b/mm/swapfile.c > > @@ -750,6 +750,12 @@ static void swap_range_free(struct > > swap_info_struct *si, unsigned long offset, > > offset++; > > } > > clear_shadow_from_swap_cache(si->type, begin, end); > > + > > + /* > > + * Make sure that try_to_unuse() observes si->inuse_pages reaching 0 > > + * only after the above cleanups are done. > > + */ > > + smp_wmb(); > > atomic_long_add(nr_entries, &nr_swap_pages); > > WRITE_ONCE(si->inuse_pages, si->inuse_pages - nr_entries); > > } > > @@ -2130,6 +2136,11 @@ static int try_to_unuse(unsigned int type) > > return -EINTR; > > } > > > > + /* > > + * Make sure that further cleanups after try_to_unuse() returns happen > > + * after swap_range_free() reduces si->inuse_pages to 0. > > + */ > > + smp_mb(); > > return 0; > > } > > We need to take care of "si->inuse_pages" checking at the beginning of > try_to_unuse() too. Otherwise, it looks good to me. Hmm, why isn't one barrier at the end of the function enough? I think all we need is that before we return from try_to_unuse(), all the cleanups in swap_range_free() are taken care of, which the barrier at the end should be doing. We just want instructions after try_to_unuse() to not get re-ordered before si->inuse_pages is read as 0, right? > > > Alternatively, we may just hold the spinlock in try_to_unuse() when we > > check si->inuse_pages at the end. This will also ensure that any calls > > to swap_range_free() have completed. Let me know what you prefer. > > Personally, I prefer memory barriers here. Ack.
