| Message ID | b6fe6b879f17fa68eee6cbd876f459f6e5e33495.1526491581.git.osandov@fb.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 2018-05-16 at 10:56 -0700, Omar Sandoval wrote: > From: Omar Sandoval <osandov@fb.com> > > If swapon() fails after incrementing nr_rotate_swap, we don't > decrement > it and thus effectively leak it. Make sure we decrement it if we > incremented it. My first inclination when reading this patch was "surely there must be a better way to structure the error code", but given that swap on rotating media increments this count, while swap on SSD does not, and the rotating media and SSD code paths are 90% the same, my first inclination was wrong. Thanks for catching and fixing that bug. > Fixes: 81a0298bdfab ("mm, swap: don't use VMA based swap readahead if > HDD is used as swap") > Signed-off-by: Omar Sandoval <osandov@fb.com> Reviewed-by: Rik van Riel <riel@surriel.com>
Omar Sandoval <osandov@osandov.com> writes: > From: Omar Sandoval <osandov@fb.com> > > If swapon() fails after incrementing nr_rotate_swap, we don't decrement > it and thus effectively leak it. Make sure we decrement it if we > incremented it. > > Fixes: 81a0298bdfab ("mm, swap: don't use VMA based swap readahead if HDD is used as swap") > Signed-off-by: Omar Sandoval <osandov@fb.com> Good catch! Thanks! Reviewed-by: "Huang, Ying" <ying.huang@intel.com> Best Regards, Huang, Ying > --- > Based on v4.17-rc5. > > mm/swapfile.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/mm/swapfile.c b/mm/swapfile.c > index cc2cf04d9018..78a015fcec3b 100644 > --- a/mm/swapfile.c > +++ b/mm/swapfile.c > @@ -3112,6 +3112,7 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags) > unsigned long *frontswap_map = NULL; > struct page *page = NULL; > struct inode *inode = NULL; > + bool inced_nr_rotate_swap = false; > > if (swap_flags & ~SWAP_FLAGS_VALID) > return -EINVAL; > @@ -3215,8 +3216,10 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags) > cluster = per_cpu_ptr(p->percpu_cluster, cpu); > cluster_set_null(&cluster->index); > } > - } else > + } else { > atomic_inc(&nr_rotate_swap); > + inced_nr_rotate_swap = true; > + } > > error = swap_cgroup_swapon(p->type, maxpages); > if (error) > @@ -3307,6 +3310,8 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags) > vfree(swap_map); > kvfree(cluster_info); > kvfree(frontswap_map); > + if (inced_nr_rotate_swap) > + atomic_dec(&nr_rotate_swap); > if (swap_file) { > if (inode && S_ISREG(inode->i_mode)) { > inode_unlock(inode);
diff --git a/mm/swapfile.c b/mm/swapfile.c index cc2cf04d9018..78a015fcec3b 100644 --- a/mm/swapfile.c +++ b/mm/swapfile.c @@ -3112,6 +3112,7 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags) unsigned long *frontswap_map = NULL; struct page *page = NULL; struct inode *inode = NULL; + bool inced_nr_rotate_swap = false; if (swap_flags & ~SWAP_FLAGS_VALID) return -EINVAL; @@ -3215,8 +3216,10 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags) cluster = per_cpu_ptr(p->percpu_cluster, cpu); cluster_set_null(&cluster->index); } - } else + } else { atomic_inc(&nr_rotate_swap); + inced_nr_rotate_swap = true; + } error = swap_cgroup_swapon(p->type, maxpages); if (error) @@ -3307,6 +3310,8 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags) vfree(swap_map); kvfree(cluster_info); kvfree(frontswap_map); + if (inced_nr_rotate_swap) + atomic_dec(&nr_rotate_swap); if (swap_file) { if (inode && S_ISREG(inode->i_mode)) { inode_unlock(inode);