f2fs: fix missing unlock(sbi->gc_mutex)

PATCH In review Jaegeuk Kim 3 participants 5 messages Dec 18, 2018 – Dec 19, 2018

Subsystems (2)

Stable branch F2FS file system

lore

Share & tools

lore

Jaegeuk Kim

author maintainer

2018-12-18 02:15 UTC #0 lore raw

PATCH 1/1 f2fs: fix missing unlock(sbi->gc_mutex)

This fixes missing unlock call.

Cc <[email protected]>

Signed-off-by Jaegeuk Kim <[email protected]>

---
 fs/f2fs/super.c | 3 +++
 1 file changed, 3 insertions(+)

fs/f2fs/super.c +3 -0

index b79677639108..2689a2cb56cc 100644
@@ -1462,6 +1462,9 @@ static int f2fs_disable_checkpoint(struct f2fs_sb_info *sbi)
 
 	while (!f2fs_time_over(sbi, DISABLE_TIME)) {
 		err = f2fs_gc(sbi, true, false, NULL_SEGNO);
+
+		/* f2fs_gc guarantees unlock gc_mutex */
+		mutex_lock(&sbi->gc_mutex);
 		if (err == -ENODATA)
 			break;
 		if (err && err != -EAGAIN) {

Signature · 1 line

2.19.0.605.g01d371f741-goog

Chao Yu

2018-12-18 10:33 UTC #1 lore raw

Quoted 13 lines · Jaegeuk Kim

This fixes missing unlock call.
Cc <[email protected]>
Signed-off-by Jaegeuk Kim <[email protected]>
---
 fs/f2fs/super.c | 3 +++
 1 file changed, 3 insertions(+)
fs/f2fs/super.c +0 -0
index b79677639108..2689a2cb56cc 100644

fs/f2fs/super.c static int f2fs_disable_checkpoint(struct f2fs_sb_info *sbi) · line 1466

Quoted 5 lines

fs/f2fs/super.c +2 -0

@@ -1462,6 +1462,9 @@ static int f2fs_disable_checkpoint(struct f2fs_sb_info *sbi)
 
 	while (!f2fs_time_over(sbi, DISABLE_TIME)) {
 		err = f2fs_gc(sbi, true, false, NULL_SEGNO);
+
+		/* f2fs_gc guarantees unlock gc_mutex */

How about:

	while () {
		mutex_lock(&sbi->gc_mutex);
		err = f2fs_gc();
		if (err) {
			handle error cases..
		}
	}

Thanks,

Quoted 5 lines

fs/f2fs/super.c +1 -0

+		mutex_lock(&sbi->gc_mutex);
 		if (err == -ENODATA)
 			break;
 		if (err && err != -EAGAIN) {

Sasha Levin

maintainer

2018-12-18 20:40 UTC #2 lore raw

In reply to Jaegeuk Kim

Hi,

[This is an automated email]

This commit has been processed because it contains a -stable tag. The stable tag indicates that it's relevant for the following trees: all

The bot has tested the following trees: v4.19.10, v4.14.89, v4.9.146, v4.4.168, v3.18.130,

v4.19.10: Failed to apply! Possible dependencies:
4354994f097d ("f2fs: checkpoint disabling")
5ce805869cbe ("f2fs: submit bio after shutdown")
a7d10cf3e4e3 ("f2fs: add new idle interval timing for discard and gc paths") abde73c71829 ("f2fs: fix unnecessary periodic wakeup of discard thread when dev is busy") f847c699cff3 ("f2fs: allow out-place-update for direct IO in LFS mode")

v4.14.89: Failed to apply! Possible dependencies:
079396270b0f ("f2fs: add mount option for segment allocation policy") 0cdd31953967 ("f2fs: support passing down write hints given by users to block layer") 1751e8a6cb93 ("Rename superblock flags (MS_xyz -> SB_xyz)")
4354994f097d ("f2fs: checkpoint disabling")
49566f6f06b3 ("afs: Note the cell in the superblock info also") 4f0a03d34dd4 ("f2fs: apply write hints to select the type of segments for buffered write") 578c647879f7 ("f2fs: implement cgroup writeback support")
5e4def203816 ("Pass mode to wait_on_atomic_t() action funcs and provide default actions") 6afc662e68b5 ("f2fs: support flexible inline xattr size")
71ad682c1c29 ("f2fs: convert inline data for direct I/O & FI_NO_PREALLOC") 8b3a0ca0fde1 ("f2fs: Add the 'whint_mode' mount option to f2fs documentation") 93cf93f17c8e ("f2fs: introduce mount option for fsync mode")
a5f433f74105 ("f2fs: introduce read_inline_xattr")
d5097be55c21 ("f2fs: apply write hints to select the type of segment for direct write") f044c8847bb6 ("afs: Lay the groundwork for supporting network namespaces") ff62af200b94 ("f2fs: introduce a new mount option test_dummy_encryption")

v4.9.146: Failed to apply! Possible dependencies:
079396270b0f ("f2fs: add mount option for segment allocation policy") 0abd675e97e6 ("f2fs: support plain user/group quota")
3325bea5b26a ("fscrypt: rename get_crypt_info() to fscrypt_get_crypt_info()") 4354994f097d ("f2fs: checkpoint disabling")
43b113fea2d4 ("ubifs: Massage ubifs_listxattr() for encryption context") 46f47e48008b ("fscrypt: split supp and notsupp declarations into their own headers") 4b2414d04e99 ("f2fs: support journalled quota")
5c57132eaf52 ("f2fs: support project quota")
8b3a0ca0fde1 ("f2fs: Add the 'whint_mode' mount option to f2fs documentation") 9270b2f4cd9d ("ubifs: Preload crypto context in ->lookup()") 93cf93f17c8e ("f2fs: introduce mount option for fsync mode") ac7e47a9ed1b ("ubifs: Enforce crypto policy in ->link and ->rename") cc41a536524f ("ubifs: Use a random number for cookies")
d475a507457b ("ubifs: Add skeleton for fscrypto")
d63d61c16972 ("ubifs: Implement UBIFS_FLG_DOUBLE_HASH")
db717d8e26c2 ("fscrypto: move ioctl processing more fully into common code") e021986ee411 ("ubifs: Implement UBIFS_FLG_ENCRYPTION")
ec91538dccd4 ("f2fs: get io size bit from mount option")
ec9160dacdb0 ("ubifs: Use fscrypt ioctl() helpers")
f4f61d2cc6d8 ("ubifs: Implement encrypted filenames")
f6337d842668 ("ubifs: Export ubifs_check_dir_empty()")
ff62af200b94 ("f2fs: introduce a new mount option test_dummy_encryption")

v4.4.168: Failed to apply! Possible dependencies:
079396270b0f ("f2fs: add mount option for segment allocation policy") 0abd675e97e6 ("f2fs: support plain user/group quota") 0f18b462b2e5 ("f2fs: flush inode metadata when checkpoint is doing") 2710fd7e00b4 ("f2fs: introduce dirty list node in inode info") 33fbd5100de6 ("f2fs: stat dirty regular/symlink inodes") 343f40f0a70e ("f2fs: introduce new option for controlling data flush") 36abef4e796d ("f2fs: introduce mode=lfs mount option") 4354994f097d ("f2fs: checkpoint disabling")
4b2414d04e99 ("f2fs: support journalled quota")
5c57132eaf52 ("f2fs: support project quota")
69e9e4274450 ("f2fs: set flush_merge by default")
6ad7609a183a ("f2fs: introduce __remove_dirty_inode") 6d94c74ab85f ("f2fs: add lazytime mount option")
73faec4d9935 ("f2fs: add mount option to select fault injection ratio") 8b3a0ca0fde1 ("f2fs: Add the 'whint_mode' mount option to f2fs documentation") 93cf93f17c8e ("f2fs: introduce mount option for fsync mode") c227f912732f ("f2fs: record dirty status of regular/symlink inode") d323d005ac4a ("f2fs: support file defragment")
eb7e813cc791 ("f2fs: fix to remove directory inode from dirty list") ec91538dccd4 ("f2fs: get io size bit from mount option") ff62af200b94 ("f2fs: introduce a new mount option test_dummy_encryption")

v3.18.130: Failed to apply! Possible dependencies:
079396270b0f ("f2fs: add mount option for segment allocation policy") 0abd675e97e6 ("f2fs: support plain user/group quota")
13fd8f89f662 ("f2fs: fix to call f2fs_unlock_op")
1ce86bf6f882 ("f2fs: fix race conditon on truncation with inline_data") 1e84371ffeef ("f2fs: change atomic and volatile write policies") 201a05be9628 ("f2fs: add key function to handle inline dir") 3289c061c5aa ("f2fs: add stat info for inline_dentry inodes") 343f40f0a70e ("f2fs: introduce new option for controlling data flush") 34ba94bac938 ("f2fs: do not make dirty any inmemory pages") 34d67debe02b ("f2fs: add infra struct and helper for inline dir") 36abef4e796d ("f2fs: introduce mode=lfs mount option")
4354994f097d ("f2fs: checkpoint disabling")
4b2414d04e99 ("f2fs: support journalled quota")
5ab18570b85c ("f2fs: should not truncate any inline_dentry") 5c57132eaf52 ("f2fs: support project quota")
5efd3c6f1be9 ("f2fs: add a new mount option for inline dir") 6aefd93b0137 ("f2fs: introduce background_gc=sync mount option") 75342797988a ("f2fs: enable inline data by default")
8967215954a5 ("f2fs: add a mount option for rb-tree extent cache") 8b3a0ca0fde1 ("f2fs: Add the 'whint_mode' mount option to f2fs documentation") 93cf93f17c8e ("f2fs: introduce mount option for fsync mode") 9ba69cf98773 ("f2fs: avoid to allocate when inline_data was written") b3d208f96d6b ("f2fs: revisit inline_data to avoid data races and potential bugs") c08a690b4691 ("f2fs: should truncate any allocated block for inline_data write") c1c1b58359d4 ("f2fs: add new ioctl F2FS_IOC_GARBAGE_COLLECT") cbcb2872e37b ("f2fs: invalidate inmemory page")
d5053a34a9cc ("f2fs: introduce -o fastboot for reducing booting time only") d530d4d8e237 ("f2fs: support synchronous gc in ioctl")
ec91538dccd4 ("f2fs: get io size bit from mount option") f1e33a041e39 ("f2fs: use kmap_atomic instead of kmap")
ff62af200b94 ("f2fs: introduce a new mount option test_dummy_encryption")

How should we proceed with this patch?

--
Thanks,
Sasha

Jaegeuk Kim

author maintainer

2018-12-18 23:22 UTC #3 lore raw

In reply to Jaegeuk Kim

This fixes missing unlock call.

Cc <[email protected]>

Signed-off-by Jaegeuk Kim <[email protected]>

---

v1 -> v2:
- clean up

 fs/f2fs/super.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

fs/f2fs/super.c +2 -5

index b79677639108..cec7ef27b389 100644
@@ -1457,19 +1457,16 @@ static int f2fs_disable_checkpoint(struct f2fs_sb_info *sbi)
 
 	sbi->sb->s_flags |= SB_ACTIVE;
 
-	mutex_lock(&sbi->gc_mutex);
 	f2fs_update_time(sbi, DISABLE_TIME);
 
 	while (!f2fs_time_over(sbi, DISABLE_TIME)) {
+		mutex_lock(&sbi->gc_mutex);
 		err = f2fs_gc(sbi, true, false, NULL_SEGNO);
 		if (err == -ENODATA)
 			break;
-		if (err && err != -EAGAIN) {
-			mutex_unlock(&sbi->gc_mutex);
+		if (err && err != -EAGAIN)
 			return err;
-		}
 	}
-	mutex_unlock(&sbi->gc_mutex);
 
 	err = sync_filesystem(sbi->sb);
 	if (err)

Signature · 1 line

2.19.0.605.g01d371f741-goog

Chao Yu

Reviewed-by

2018-12-19 03:44 UTC #4 lore raw

Quoted 4 lines · Jaegeuk Kim

This fixes missing unlock call.
Cc <[email protected]>
Signed-off-by Jaegeuk Kim <[email protected]>

Looks good to me now. :)

Reviewed-by Chao Yu <[email protected]>

Thanks,