Skip to content

Commit d795a90

Browse files
naotatorvalds
authored andcommitted
mm/swapfile.c: move inode_lock out of claim_swapfile
claim_swapfile() currently keeps the inode locked when it is successful, or the file is already swapfile (with -EBUSY). And, on the other error cases, it does not lock the inode. This inconsistency of the lock state and return value is quite confusing and actually causing a bad unlock balance as below in the "bad_swap" section of __do_sys_swapon(). This commit fixes this issue by moving the inode_lock() and IS_SWAPFILE check out of claim_swapfile(). The inode is unlocked in "bad_swap_unlock_inode" section, so that the inode is ensured to be unlocked at "bad_swap". Thus, error handling codes after the locking now jumps to "bad_swap_unlock_inode" instead of "bad_swap". ===================================== WARNING: bad unlock balance detected! 5.5.0-rc7+ #176 Not tainted ------------------------------------- swapon/4294 is trying to release lock (&sb->s_type->i_mutex_key) at: __do_sys_swapon+0x94b/0x3550 but there are no more locks to release! other info that might help us debug this: no locks held by swapon/4294. stack backtrace: CPU: 5 PID: 4294 Comm: swapon Not tainted 5.5.0-rc7-BTRFS-ZNS+ #176 Hardware name: ASUS All Series/H87-PRO, BIOS 2102 07/29/2014 Call Trace: dump_stack+0xa1/0xea print_unlock_imbalance_bug.cold+0x114/0x123 lock_release+0x562/0xed0 up_write+0x2d/0x490 __do_sys_swapon+0x94b/0x3550 __x64_sys_swapon+0x54/0x80 do_syscall_64+0xa4/0x4b0 entry_SYSCALL_64_after_hwframe+0x49/0xbe RIP: 0033:0x7f15da0a0dc7 Fixes: 1638045 ("mm: set S_SWAPFILE on blockdev swap devices") Signed-off-by: Naohiro Aota <[email protected]> Signed-off-by: Andrew Morton <[email protected]> Tested-by: Qais Youef <[email protected]> Reviewed-by: Andrew Morton <[email protected]> Reviewed-by: Darrick J. Wong <[email protected]> Cc: Christoph Hellwig <[email protected]> Cc: <[email protected]> Link: http://lkml.kernel.org/r/[email protected] Signed-off-by: Linus Torvalds <[email protected]>
1 parent 83fd69c commit d795a90

File tree

1 file changed

+20
-21
lines changed

1 file changed

+20
-21
lines changed

mm/swapfile.c

Lines changed: 20 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2899,10 +2899,6 @@ static int claim_swapfile(struct swap_info_struct *p, struct inode *inode)
28992899
p->bdev = inode->i_sb->s_bdev;
29002900
}
29012901

2902-
inode_lock(inode);
2903-
if (IS_SWAPFILE(inode))
2904-
return -EBUSY;
2905-
29062902
return 0;
29072903
}
29082904

@@ -3157,36 +3153,41 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
31573153
mapping = swap_file->f_mapping;
31583154
inode = mapping->host;
31593155

3160-
/* will take i_rwsem; */
31613156
error = claim_swapfile(p, inode);
31623157
if (unlikely(error))
31633158
goto bad_swap;
31643159

3160+
inode_lock(inode);
3161+
if (IS_SWAPFILE(inode)) {
3162+
error = -EBUSY;
3163+
goto bad_swap_unlock_inode;
3164+
}
3165+
31653166
/*
31663167
* Read the swap header.
31673168
*/
31683169
if (!mapping->a_ops->readpage) {
31693170
error = -EINVAL;
3170-
goto bad_swap;
3171+
goto bad_swap_unlock_inode;
31713172
}
31723173
page = read_mapping_page(mapping, 0, swap_file);
31733174
if (IS_ERR(page)) {
31743175
error = PTR_ERR(page);
3175-
goto bad_swap;
3176+
goto bad_swap_unlock_inode;
31763177
}
31773178
swap_header = kmap(page);
31783179

31793180
maxpages = read_swap_header(p, swap_header, inode);
31803181
if (unlikely(!maxpages)) {
31813182
error = -EINVAL;
3182-
goto bad_swap;
3183+
goto bad_swap_unlock_inode;
31833184
}
31843185

31853186
/* OK, set up the swap map and apply the bad block list */
31863187
swap_map = vzalloc(maxpages);
31873188
if (!swap_map) {
31883189
error = -ENOMEM;
3189-
goto bad_swap;
3190+
goto bad_swap_unlock_inode;
31903191
}
31913192

31923193
if (bdi_cap_stable_pages_required(inode_to_bdi(inode)))
@@ -3211,7 +3212,7 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
32113212
GFP_KERNEL);
32123213
if (!cluster_info) {
32133214
error = -ENOMEM;
3214-
goto bad_swap;
3215+
goto bad_swap_unlock_inode;
32153216
}
32163217

32173218
for (ci = 0; ci < nr_cluster; ci++)
@@ -3220,7 +3221,7 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
32203221
p->percpu_cluster = alloc_percpu(struct percpu_cluster);
32213222
if (!p->percpu_cluster) {
32223223
error = -ENOMEM;
3223-
goto bad_swap;
3224+
goto bad_swap_unlock_inode;
32243225
}
32253226
for_each_possible_cpu(cpu) {
32263227
struct percpu_cluster *cluster;
@@ -3234,13 +3235,13 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
32343235

32353236
error = swap_cgroup_swapon(p->type, maxpages);
32363237
if (error)
3237-
goto bad_swap;
3238+
goto bad_swap_unlock_inode;
32383239

32393240
nr_extents = setup_swap_map_and_extents(p, swap_header, swap_map,
32403241
cluster_info, maxpages, &span);
32413242
if (unlikely(nr_extents < 0)) {
32423243
error = nr_extents;
3243-
goto bad_swap;
3244+
goto bad_swap_unlock_inode;
32443245
}
32453246
/* frontswap enabled? set up bit-per-page map for frontswap */
32463247
if (IS_ENABLED(CONFIG_FRONTSWAP))
@@ -3280,7 +3281,7 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
32803281

32813282
error = init_swap_address_space(p->type, maxpages);
32823283
if (error)
3283-
goto bad_swap;
3284+
goto bad_swap_unlock_inode;
32843285

32853286
/*
32863287
* Flush any pending IO and dirty mappings before we start using this
@@ -3290,7 +3291,7 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
32903291
error = inode_drain_writes(inode);
32913292
if (error) {
32923293
inode->i_flags &= ~S_SWAPFILE;
3293-
goto bad_swap;
3294+
goto bad_swap_unlock_inode;
32943295
}
32953296

32963297
mutex_lock(&swapon_mutex);
@@ -3315,13 +3316,16 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
33153316

33163317
error = 0;
33173318
goto out;
3319+
bad_swap_unlock_inode:
3320+
inode_unlock(inode);
33183321
bad_swap:
33193322
free_percpu(p->percpu_cluster);
33203323
p->percpu_cluster = NULL;
33213324
if (inode && S_ISBLK(inode->i_mode) && p->bdev) {
33223325
set_blocksize(p->bdev, p->old_block_size);
33233326
blkdev_put(p->bdev, FMODE_READ | FMODE_WRITE | FMODE_EXCL);
33243327
}
3328+
inode = NULL;
33253329
destroy_swap_extents(p);
33263330
swap_cgroup_swapoff(p->type);
33273331
spin_lock(&swap_lock);
@@ -3333,13 +3337,8 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
33333337
kvfree(frontswap_map);
33343338
if (inced_nr_rotate_swap)
33353339
atomic_dec(&nr_rotate_swap);
3336-
if (swap_file) {
3337-
if (inode) {
3338-
inode_unlock(inode);
3339-
inode = NULL;
3340-
}
3340+
if (swap_file)
33413341
filp_close(swap_file, NULL);
3342-
}
33433342
out:
33443343
if (page && !IS_ERR(page)) {
33453344
kunmap(page);

0 commit comments

Comments
 (0)