| Message ID | 20230522070905.16773-4-ying.huang@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | swap: cleanup get/put_swap_device() usage | expand |
On 22.05.23 09:09, Huang Ying wrote: > __swp_swapcount() just encloses the calling to swap_swapcount() with > get/put_swap_device(). It is called in __read_swap_cache_async() > only, which encloses the calling with get/put_swap_device() already. > So, __read_swap_cache_async() can call swap_swapcount() directly. The previous patch contained the hunk - if (!__swp_swapcount(entry) && swap_slot_cache_enabled) - return NULL; + if (!swap_swapcount(si, entry) && swap_slot_cache_enabled) + goto fail; So something is a bit off here. Either that hunk should go here, or this patch description has to be adjusted. But I guess patch #2 doesn't compile on its own because this patch here adds swap_swapcount() to include/linux/swap.h ? > > Signed-off-by: "Huang, Ying" <ying.huang@intel.com> > Cc: David Hildenbrand <david@redhat.com> > Cc: Hugh Dickins <hughd@google.com> > Cc: Johannes Weiner <hannes@cmpxchg.org> > Cc: Matthew Wilcox <willy@infradead.org> > Cc: Michal Hocko <mhocko@suse.com> > Cc: Minchan Kim <minchan@kernel.org> > Cc: Tim Chen <tim.c.chen@linux.intel.com> > Cc: Yang Shi <shy828301@gmail.com> > Cc: Yu Zhao <yuzhao@google.com> > --- > include/linux/swap.h | 4 ++-- > mm/swapfile.c | 20 +------------------- > 2 files changed, 3 insertions(+), 21 deletions(-) > > diff --git a/include/linux/swap.h b/include/linux/swap.h > index 3c69cb653cb9..f6bd51aa05ea 100644 > --- a/include/linux/swap.h > +++ b/include/linux/swap.h > @@ -512,7 +512,7 @@ int find_first_swap(dev_t *device); > extern unsigned int count_swap_pages(int, int); > extern sector_t swapdev_block(int, pgoff_t); > extern int __swap_count(swp_entry_t entry); > -extern int __swp_swapcount(swp_entry_t entry); > +extern int swap_swapcount(struct swap_info_struct *si, swp_entry_t entry); > extern int swp_swapcount(swp_entry_t entry); > extern struct swap_info_struct *page_swap_info(struct page *); > extern struct swap_info_struct *swp_swap_info(swp_entry_t entry); > @@ -590,7 +590,7 @@ static inline int __swap_count(swp_entry_t entry) > return 0; > } > > -static inline int __swp_swapcount(swp_entry_t entry) > +static inline int swap_swapcount(struct swap_info_struct *si, swp_entry_t entry) > { > return 0; > } > diff --git a/mm/swapfile.c b/mm/swapfile.c > index 8419cba9c192..e9cce775fb25 100644 > --- a/mm/swapfile.c > +++ b/mm/swapfile.c > @@ -1443,7 +1443,7 @@ int __swap_count(swp_entry_t entry) > * This does not give an exact answer when swap count is continued, > * but does include the high COUNT_CONTINUED flag to allow for that. > */ > -static int swap_swapcount(struct swap_info_struct *si, swp_entry_t entry) > +int swap_swapcount(struct swap_info_struct *si, swp_entry_t entry) > { > pgoff_t offset = swp_offset(entry); > struct swap_cluster_info *ci; > @@ -1455,24 +1455,6 @@ static int swap_swapcount(struct swap_info_struct *si, swp_entry_t entry) > return count; > } > > -/* > - * How many references to @entry are currently swapped out? > - * This does not give an exact answer when swap count is continued, > - * but does include the high COUNT_CONTINUED flag to allow for that. > - */ > -int __swp_swapcount(swp_entry_t entry) > -{ > - int count = 0; > - struct swap_info_struct *si; > - > - si = get_swap_device(entry); > - if (si) { > - count = swap_swapcount(si, entry); > - put_swap_device(si); > - } > - return count; > -} > - > /* > * How many references to @entry are currently swapped out? > * This considers COUNT_CONTINUED so it returns exact answer.
David Hildenbrand <david@redhat.com> writes: > On 22.05.23 09:09, Huang Ying wrote: >> __swp_swapcount() just encloses the calling to swap_swapcount() with >> get/put_swap_device(). It is called in __read_swap_cache_async() >> only, which encloses the calling with get/put_swap_device() already. >> So, __read_swap_cache_async() can call swap_swapcount() directly. > > The previous patch contained the hunk > > - if (!__swp_swapcount(entry) && swap_slot_cache_enabled) > - return NULL; > + if (!swap_swapcount(si, entry) && swap_slot_cache_enabled) > + goto fail; > > So something is a bit off here. Either that hunk should go here, or > this patch description has to be adjusted. > > > But I guess patch #2 doesn't compile on its own because this patch > here adds swap_swapcount() to include/linux/swap.h ? Good catch! Will change this in the next version. Best Regards, Huang, Ying >> Signed-off-by: "Huang, Ying" <ying.huang@intel.com> >> Cc: David Hildenbrand <david@redhat.com> >> Cc: Hugh Dickins <hughd@google.com> >> Cc: Johannes Weiner <hannes@cmpxchg.org> >> Cc: Matthew Wilcox <willy@infradead.org> >> Cc: Michal Hocko <mhocko@suse.com> >> Cc: Minchan Kim <minchan@kernel.org> >> Cc: Tim Chen <tim.c.chen@linux.intel.com> >> Cc: Yang Shi <shy828301@gmail.com> >> Cc: Yu Zhao <yuzhao@google.com> >> --- >> include/linux/swap.h | 4 ++-- >> mm/swapfile.c | 20 +------------------- >> 2 files changed, 3 insertions(+), 21 deletions(-) >> diff --git a/include/linux/swap.h b/include/linux/swap.h >> index 3c69cb653cb9..f6bd51aa05ea 100644 >> --- a/include/linux/swap.h >> +++ b/include/linux/swap.h >> @@ -512,7 +512,7 @@ int find_first_swap(dev_t *device); >> extern unsigned int count_swap_pages(int, int); >> extern sector_t swapdev_block(int, pgoff_t); >> extern int __swap_count(swp_entry_t entry); >> -extern int __swp_swapcount(swp_entry_t entry); >> +extern int swap_swapcount(struct swap_info_struct *si, swp_entry_t entry); >> extern int swp_swapcount(swp_entry_t entry); >> extern struct swap_info_struct *page_swap_info(struct page *); >> extern struct swap_info_struct *swp_swap_info(swp_entry_t entry); >> @@ -590,7 +590,7 @@ static inline int __swap_count(swp_entry_t entry) >> return 0; >> } >> -static inline int __swp_swapcount(swp_entry_t entry) >> +static inline int swap_swapcount(struct swap_info_struct *si, swp_entry_t entry) >> { >> return 0; >> } >> diff --git a/mm/swapfile.c b/mm/swapfile.c >> index 8419cba9c192..e9cce775fb25 100644 >> --- a/mm/swapfile.c >> +++ b/mm/swapfile.c >> @@ -1443,7 +1443,7 @@ int __swap_count(swp_entry_t entry) >> * This does not give an exact answer when swap count is continued, >> * but does include the high COUNT_CONTINUED flag to allow for that. >> */ >> -static int swap_swapcount(struct swap_info_struct *si, swp_entry_t entry) >> +int swap_swapcount(struct swap_info_struct *si, swp_entry_t entry) >> { >> pgoff_t offset = swp_offset(entry); >> struct swap_cluster_info *ci; >> @@ -1455,24 +1455,6 @@ static int swap_swapcount(struct swap_info_struct *si, swp_entry_t entry) >> return count; >> } >> -/* >> - * How many references to @entry are currently swapped out? >> - * This does not give an exact answer when swap count is continued, >> - * but does include the high COUNT_CONTINUED flag to allow for that. >> - */ >> -int __swp_swapcount(swp_entry_t entry) >> -{ >> - int count = 0; >> - struct swap_info_struct *si; >> - >> - si = get_swap_device(entry); >> - if (si) { >> - count = swap_swapcount(si, entry); >> - put_swap_device(si); >> - } >> - return count; >> -} >> - >> /* >> * How many references to @entry are currently swapped out? >> * This considers COUNT_CONTINUED so it returns exact answer.
diff --git a/include/linux/swap.h b/include/linux/swap.h index 3c69cb653cb9..f6bd51aa05ea 100644 --- a/include/linux/swap.h +++ b/include/linux/swap.h @@ -512,7 +512,7 @@ int find_first_swap(dev_t *device); extern unsigned int count_swap_pages(int, int); extern sector_t swapdev_block(int, pgoff_t); extern int __swap_count(swp_entry_t entry); -extern int __swp_swapcount(swp_entry_t entry); +extern int swap_swapcount(struct swap_info_struct *si, swp_entry_t entry); extern int swp_swapcount(swp_entry_t entry); extern struct swap_info_struct *page_swap_info(struct page *); extern struct swap_info_struct *swp_swap_info(swp_entry_t entry); @@ -590,7 +590,7 @@ static inline int __swap_count(swp_entry_t entry) return 0; } -static inline int __swp_swapcount(swp_entry_t entry) +static inline int swap_swapcount(struct swap_info_struct *si, swp_entry_t entry) { return 0; } diff --git a/mm/swapfile.c b/mm/swapfile.c index 8419cba9c192..e9cce775fb25 100644 --- a/mm/swapfile.c +++ b/mm/swapfile.c @@ -1443,7 +1443,7 @@ int __swap_count(swp_entry_t entry) * This does not give an exact answer when swap count is continued, * but does include the high COUNT_CONTINUED flag to allow for that. */ -static int swap_swapcount(struct swap_info_struct *si, swp_entry_t entry) +int swap_swapcount(struct swap_info_struct *si, swp_entry_t entry) { pgoff_t offset = swp_offset(entry); struct swap_cluster_info *ci; @@ -1455,24 +1455,6 @@ static int swap_swapcount(struct swap_info_struct *si, swp_entry_t entry) return count; } -/* - * How many references to @entry are currently swapped out? - * This does not give an exact answer when swap count is continued, - * but does include the high COUNT_CONTINUED flag to allow for that. - */ -int __swp_swapcount(swp_entry_t entry) -{ - int count = 0; - struct swap_info_struct *si; - - si = get_swap_device(entry); - if (si) { - count = swap_swapcount(si, entry); - put_swap_device(si); - } - return count; -} - /* * How many references to @entry are currently swapped out? * This considers COUNT_CONTINUED so it returns exact answer.
__swp_swapcount() just encloses the calling to swap_swapcount() with get/put_swap_device(). It is called in __read_swap_cache_async() only, which encloses the calling with get/put_swap_device() already. So, __read_swap_cache_async() can call swap_swapcount() directly. Signed-off-by: "Huang, Ying" <ying.huang@intel.com> Cc: David Hildenbrand <david@redhat.com> Cc: Hugh Dickins <hughd@google.com> Cc: Johannes Weiner <hannes@cmpxchg.org> Cc: Matthew Wilcox <willy@infradead.org> Cc: Michal Hocko <mhocko@suse.com> Cc: Minchan Kim <minchan@kernel.org> Cc: Tim Chen <tim.c.chen@linux.intel.com> Cc: Yang Shi <shy828301@gmail.com> Cc: Yu Zhao <yuzhao@google.com> --- include/linux/swap.h | 4 ++-- mm/swapfile.c | 20 +------------------- 2 files changed, 3 insertions(+), 21 deletions(-)