On Mon, 09 May 2022, Miaohe Lin wrote:
> Add helper swap_offset_available() to remove some duplicated codes.
> Minor readability improvement.
I don't think that putting the spin_lock() inside the inline helper is
good for readability.
If the function was called
swap_offset_available_and_locked()
it might be ok. Otherwise I would rather the spin_lock() was called
when the function returned true.
Thanks,
NeilBrown
>
> Signed-off-by: Miaohe Lin <linmiaohe@xxxxxxxxxx>
> ---
> mm/swapfile.c | 33 +++++++++++++++++----------------
> 1 file changed, 17 insertions(+), 16 deletions(-)
>
> diff --git a/mm/swapfile.c b/mm/swapfile.c
> index c90298a0561a..d5d3e2d03d28 100644
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c
> @@ -776,6 +776,21 @@ static void set_cluster_next(struct swap_info_struct *si, unsigned long next)
> this_cpu_write(*si->cluster_next_cpu, next);
> }
>
> +static inline bool swap_offset_available(struct swap_info_struct *si, unsigned long offset)
> +{
> + if (data_race(!si->swap_map[offset])) {
> + spin_lock(&si->lock);
> + return true;
> + }
> +
> + if (vm_swap_full() && READ_ONCE(si->swap_map[offset]) == SWAP_HAS_CACHE) {
> + spin_lock(&si->lock);
> + return true;
> + }
> +
> + return false;
> +}
> +
> static int scan_swap_map_slots(struct swap_info_struct *si,
> unsigned char usage, int nr,
> swp_entry_t slots[])
> @@ -953,15 +968,8 @@ static int scan_swap_map_slots(struct swap_info_struct *si,
> scan:
> spin_unlock(&si->lock);
> while (++offset <= READ_ONCE(si->highest_bit)) {
> - if (data_race(!si->swap_map[offset])) {
> - spin_lock(&si->lock);
> + if (swap_offset_available(si, offset))
> goto checks;
> - }
> - if (vm_swap_full() &&
> - READ_ONCE(si->swap_map[offset]) == SWAP_HAS_CACHE) {
> - spin_lock(&si->lock);
> - goto checks;
> - }
> if (unlikely(--latency_ration < 0)) {
> cond_resched();
> latency_ration = LATENCY_LIMIT;
> @@ -970,15 +978,8 @@ static int scan_swap_map_slots(struct swap_info_struct *si,
> }
> offset = si->lowest_bit;
> while (offset < scan_base) {
> - if (data_race(!si->swap_map[offset])) {
> - spin_lock(&si->lock);
> + if (swap_offset_available(si, offset))
> goto checks;
> - }
> - if (vm_swap_full() &&
> - READ_ONCE(si->swap_map[offset]) == SWAP_HAS_CACHE) {
> - spin_lock(&si->lock);
> - goto checks;
> - }
> if (unlikely(--latency_ration < 0)) {
> cond_resched();
> latency_ration = LATENCY_LIMIT;
> --
> 2.23.0
>
>
