[PATCH] f2fs: fix missing unlock(sbi->gc_mutex)

[PATCH] f2fs: fix missing unlock(sbi->gc_mutex)

[Jaegeuk Kim]

This fixes missing unlock call.

Cc: <stable@vger.kernel.org>
Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/super.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index b79677639108..2689a2cb56cc 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -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) {
-- 
2.19.0.605.g01d371f741-goog

Re: [f2fs-dev] [PATCH] f2fs: fix missing unlock(sbi->gc_mutex)

[Chao Yu]

On 2018/12/18 10:15, Jaegeuk Kim wrote:
> This fixes missing unlock call.
> 
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> ---
>  fs/f2fs/super.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index b79677639108..2689a2cb56cc 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -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,

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

Re: [f2fs-dev] [PATCH] f2fs: fix missing unlock(sbi->gc_mutex)

[Chao Yu]

On 2018/12/18 10:15, Jaegeuk Kim wrote:
> This fixes missing unlock call.
> 
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> ---
>  fs/f2fs/super.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index b79677639108..2689a2cb56cc 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -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,

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

Re: [PATCH] f2fs: fix missing unlock(sbi->gc_mutex)

[Sasha Levin]

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

Re: [PATCH v2] f2fs: fix missing unlock(sbi->gc_mutex)

[Jaegeuk Kim]

This fixes missing unlock call.

Cc: <stable@vger.kernel.org>
Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---

v1 -> v2:
 - clean up

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

diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index b79677639108..cec7ef27b389 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -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)
-- 
2.19.0.605.g01d371f741-goog

Re: [f2fs-dev] [PATCH v2] f2fs: fix missing unlock(sbi->gc_mutex)

[Chao Yu]

On 2018/12/19 7:22, Jaegeuk Kim wrote:
> This fixes missing unlock call.
> 
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>

Looks good to me now. :)

Reviewed-by: Chao Yu <yuchao0@huawei.com>

Thanks,

Re: [f2fs-dev] [PATCH v2] f2fs: fix missing unlock(sbi->gc_mutex)

[Chao Yu]

On 2018/12/19 7:22, Jaegeuk Kim wrote:
> This fixes missing unlock call.
> 
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>

Looks good to me now. :)

Reviewed-by: Chao Yu <yuchao0@huawei.com>

Thanks,