lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Wed, 27 May 2020 21:37:24 -0400 From: Daniel Jordan <daniel.m.jordan@...cle.com> To: Huang Ying <ying.huang@...el.com> Cc: Andrew Morton <akpm@...ux-foundation.org>, linux-mm@...ck.org, linux-kernel@...r.kernel.org, Daniel Jordan <daniel.m.jordan@...cle.com>, Michal Hocko <mhocko@...e.com>, Minchan Kim <minchan@...nel.org>, Tim Chen <tim.c.chen@...ux.intel.com>, Hugh Dickins <hughd@...gle.com> Subject: Re: [PATCH -V3] swap: Reduce lock contention on swap cache from swap slots allocation On Mon, May 25, 2020 at 08:26:48AM +0800, Huang Ying wrote: > diff --git a/mm/swapfile.c b/mm/swapfile.c > index 423c234aca15..0abd93d2a4fc 100644 > --- a/mm/swapfile.c > +++ b/mm/swapfile.c > @@ -615,7 +615,8 @@ static bool scan_swap_map_try_ssd_cluster(struct swap_info_struct *si, > * discarding, do discard now and reclaim them > */ > swap_do_scheduled_discard(si); > - *scan_base = *offset = si->cluster_next; > + *scan_base = this_cpu_read(*si->cluster_next_cpu); > + *offset = *scan_base; > goto new_cluster; Why is this done? As far as I can tell, the values always get overwritten at the end of the function with tmp and tmp isn't derived from them. Seems ebc2a1a69111 moved some logic that used to make sense but doesn't have any effect now. > } else > return false; > @@ -721,6 +722,34 @@ static void swap_range_free(struct swap_info_struct *si, unsigned long offset, > } > } > > +static void set_cluster_next(struct swap_info_struct *si, unsigned long next) > +{ > + unsigned long prev; > + > + if (!(si->flags & SWP_SOLIDSTATE)) { > + si->cluster_next = next; > + return; > + } > + > + prev = this_cpu_read(*si->cluster_next_cpu); > + /* > + * Cross the swap address space size aligned trunk, choose > + * another trunk randomly to avoid lock contention on swap > + * address space if possible. > + */ > + if ((prev >> SWAP_ADDRESS_SPACE_SHIFT) != > + (next >> SWAP_ADDRESS_SPACE_SHIFT)) { > + /* No free swap slots available */ > + if (si->highest_bit <= si->lowest_bit) > + return; > + next = si->lowest_bit + > + prandom_u32_max(si->highest_bit - si->lowest_bit + 1); > + next = ALIGN(next, SWAP_ADDRESS_SPACE_PAGES); > + next = max_t(unsigned int, next, si->lowest_bit); next is always greater than lowest_bit because it's aligned up. I think the intent of the max_t line is to handle when next is aligned outside the valid range, so it'd have to be ALIGN_DOWN instead? These aside, patch looks good to me.
Powered by blists - more mailing lists