* [PATCH] f2fs: quota: fix potential deadlock
@ 2022-01-04 13:05 ` Chao Yu
0 siblings, 0 replies; 16+ messages in thread
From: Chao Yu @ 2022-01-04 13:05 UTC (permalink / raw)
To: stable; +Cc: jaegeuk, chao, linux-f2fs-devel, Yi Zhuang
commit a5c0042200b28fff3bde6fa128ddeaef97990f8d upstream.
As Yi Zhuang reported in bugzilla:
https://bugzilla.kernel.org/show_bug.cgi?id=214299
There is potential deadlock during quota data flush as below:
Thread A: Thread B:
f2fs_dquot_acquire
down_read(&sbi->quota_sem)
f2fs_write_checkpoint
block_operations
f2fs_look_all
down_write(&sbi->cp_rwsem)
f2fs_quota_write
f2fs_write_begin
__do_map_lock
f2fs_lock_op
down_read(&sbi->cp_rwsem)
__need_flush_qutoa
down_write(&sbi->quota_sem)
This patch changes block_operations() to use trylock, if it fails,
it means there is potential quota data updater, in this condition,
let's flush quota data first and then trylock again to check dirty
status of quota data.
The side effect is: in heavy race condition (e.g. multi quota data
upaters vs quota data flusher), it may decrease the probability of
synchronizing quota data successfully in checkpoint() due to limited
retry time of quota flush.
Fixes: db6ec53b7e03 ("f2fs: add a rw_sem to cover quota flag changes")
Cc: stable@vger.kernel.org # v5.3+
Reported-by: Yi Zhuang <zhuangyi1@huawei.com>
Signed-off-by: Chao Yu <chao@kernel.org>
Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
fs/f2fs/checkpoint.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
index 83e9bc0f91ff..7b0282724231 100644
--- a/fs/f2fs/checkpoint.c
+++ b/fs/f2fs/checkpoint.c
@@ -1162,7 +1162,8 @@ static bool __need_flush_quota(struct f2fs_sb_info *sbi)
if (!is_journalled_quota(sbi))
return false;
- down_write(&sbi->quota_sem);
+ if (!down_write_trylock(&sbi->quota_sem))
+ return true;
if (is_sbi_flag_set(sbi, SBI_QUOTA_SKIP_FLUSH)) {
ret = false;
} else if (is_sbi_flag_set(sbi, SBI_QUOTA_NEED_REPAIR)) {
--
2.32.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* [f2fs-dev] [PATCH] f2fs: quota: fix potential deadlock
@ 2022-01-04 13:05 ` Chao Yu
0 siblings, 0 replies; 16+ messages in thread
From: Chao Yu @ 2022-01-04 13:05 UTC (permalink / raw)
To: stable; +Cc: jaegeuk, Yi Zhuang, linux-f2fs-devel
commit a5c0042200b28fff3bde6fa128ddeaef97990f8d upstream.
As Yi Zhuang reported in bugzilla:
https://bugzilla.kernel.org/show_bug.cgi?id=214299
There is potential deadlock during quota data flush as below:
Thread A: Thread B:
f2fs_dquot_acquire
down_read(&sbi->quota_sem)
f2fs_write_checkpoint
block_operations
f2fs_look_all
down_write(&sbi->cp_rwsem)
f2fs_quota_write
f2fs_write_begin
__do_map_lock
f2fs_lock_op
down_read(&sbi->cp_rwsem)
__need_flush_qutoa
down_write(&sbi->quota_sem)
This patch changes block_operations() to use trylock, if it fails,
it means there is potential quota data updater, in this condition,
let's flush quota data first and then trylock again to check dirty
status of quota data.
The side effect is: in heavy race condition (e.g. multi quota data
upaters vs quota data flusher), it may decrease the probability of
synchronizing quota data successfully in checkpoint() due to limited
retry time of quota flush.
Fixes: db6ec53b7e03 ("f2fs: add a rw_sem to cover quota flag changes")
Cc: stable@vger.kernel.org # v5.3+
Reported-by: Yi Zhuang <zhuangyi1@huawei.com>
Signed-off-by: Chao Yu <chao@kernel.org>
Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
fs/f2fs/checkpoint.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
index 83e9bc0f91ff..7b0282724231 100644
--- a/fs/f2fs/checkpoint.c
+++ b/fs/f2fs/checkpoint.c
@@ -1162,7 +1162,8 @@ static bool __need_flush_quota(struct f2fs_sb_info *sbi)
if (!is_journalled_quota(sbi))
return false;
- down_write(&sbi->quota_sem);
+ if (!down_write_trylock(&sbi->quota_sem))
+ return true;
if (is_sbi_flag_set(sbi, SBI_QUOTA_SKIP_FLUSH)) {
ret = false;
} else if (is_sbi_flag_set(sbi, SBI_QUOTA_NEED_REPAIR)) {
--
2.32.0
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH] f2fs: quota: fix potential deadlock
2022-01-04 13:05 ` [f2fs-dev] " Chao Yu
@ 2022-01-04 13:18 ` Greg KH
-1 siblings, 0 replies; 16+ messages in thread
From: Greg KH @ 2022-01-04 13:18 UTC (permalink / raw)
To: Chao Yu; +Cc: stable, jaegeuk, linux-f2fs-devel, Yi Zhuang
On Tue, Jan 04, 2022 at 09:05:13PM +0800, Chao Yu wrote:
> commit a5c0042200b28fff3bde6fa128ddeaef97990f8d upstream.
>
> As Yi Zhuang reported in bugzilla:
>
> https://bugzilla.kernel.org/show_bug.cgi?id=214299
>
> There is potential deadlock during quota data flush as below:
>
> Thread A: Thread B:
> f2fs_dquot_acquire
> down_read(&sbi->quota_sem)
> f2fs_write_checkpoint
> block_operations
> f2fs_look_all
> down_write(&sbi->cp_rwsem)
> f2fs_quota_write
> f2fs_write_begin
> __do_map_lock
> f2fs_lock_op
> down_read(&sbi->cp_rwsem)
> __need_flush_qutoa
> down_write(&sbi->quota_sem)
>
> This patch changes block_operations() to use trylock, if it fails,
> it means there is potential quota data updater, in this condition,
> let's flush quota data first and then trylock again to check dirty
> status of quota data.
>
> The side effect is: in heavy race condition (e.g. multi quota data
> upaters vs quota data flusher), it may decrease the probability of
> synchronizing quota data successfully in checkpoint() due to limited
> retry time of quota flush.
>
> Fixes: db6ec53b7e03 ("f2fs: add a rw_sem to cover quota flag changes")
> Cc: stable@vger.kernel.org # v5.3+
> Reported-by: Yi Zhuang <zhuangyi1@huawei.com>
> Signed-off-by: Chao Yu <chao@kernel.org>
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> ---
> fs/f2fs/checkpoint.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
> index 83e9bc0f91ff..7b0282724231 100644
> --- a/fs/f2fs/checkpoint.c
> +++ b/fs/f2fs/checkpoint.c
> @@ -1162,7 +1162,8 @@ static bool __need_flush_quota(struct f2fs_sb_info *sbi)
> if (!is_journalled_quota(sbi))
> return false;
>
> - down_write(&sbi->quota_sem);
> + if (!down_write_trylock(&sbi->quota_sem))
> + return true;
> if (is_sbi_flag_set(sbi, SBI_QUOTA_SKIP_FLUSH)) {
> ret = false;
> } else if (is_sbi_flag_set(sbi, SBI_QUOTA_NEED_REPAIR)) {
> --
> 2.32.0
>
What stable tree(s) is this for?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [f2fs-dev] [PATCH] f2fs: quota: fix potential deadlock
@ 2022-01-04 13:18 ` Greg KH
0 siblings, 0 replies; 16+ messages in thread
From: Greg KH @ 2022-01-04 13:18 UTC (permalink / raw)
To: Chao Yu; +Cc: jaegeuk, Yi Zhuang, stable, linux-f2fs-devel
On Tue, Jan 04, 2022 at 09:05:13PM +0800, Chao Yu wrote:
> commit a5c0042200b28fff3bde6fa128ddeaef97990f8d upstream.
>
> As Yi Zhuang reported in bugzilla:
>
> https://bugzilla.kernel.org/show_bug.cgi?id=214299
>
> There is potential deadlock during quota data flush as below:
>
> Thread A: Thread B:
> f2fs_dquot_acquire
> down_read(&sbi->quota_sem)
> f2fs_write_checkpoint
> block_operations
> f2fs_look_all
> down_write(&sbi->cp_rwsem)
> f2fs_quota_write
> f2fs_write_begin
> __do_map_lock
> f2fs_lock_op
> down_read(&sbi->cp_rwsem)
> __need_flush_qutoa
> down_write(&sbi->quota_sem)
>
> This patch changes block_operations() to use trylock, if it fails,
> it means there is potential quota data updater, in this condition,
> let's flush quota data first and then trylock again to check dirty
> status of quota data.
>
> The side effect is: in heavy race condition (e.g. multi quota data
> upaters vs quota data flusher), it may decrease the probability of
> synchronizing quota data successfully in checkpoint() due to limited
> retry time of quota flush.
>
> Fixes: db6ec53b7e03 ("f2fs: add a rw_sem to cover quota flag changes")
> Cc: stable@vger.kernel.org # v5.3+
> Reported-by: Yi Zhuang <zhuangyi1@huawei.com>
> Signed-off-by: Chao Yu <chao@kernel.org>
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> ---
> fs/f2fs/checkpoint.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
> index 83e9bc0f91ff..7b0282724231 100644
> --- a/fs/f2fs/checkpoint.c
> +++ b/fs/f2fs/checkpoint.c
> @@ -1162,7 +1162,8 @@ static bool __need_flush_quota(struct f2fs_sb_info *sbi)
> if (!is_journalled_quota(sbi))
> return false;
>
> - down_write(&sbi->quota_sem);
> + if (!down_write_trylock(&sbi->quota_sem))
> + return true;
> if (is_sbi_flag_set(sbi, SBI_QUOTA_SKIP_FLUSH)) {
> ret = false;
> } else if (is_sbi_flag_set(sbi, SBI_QUOTA_NEED_REPAIR)) {
> --
> 2.32.0
>
What stable tree(s) is this for?
thanks,
greg k-h
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH] f2fs: quota: fix potential deadlock
2022-01-04 13:18 ` [f2fs-dev] " Greg KH
@ 2022-01-04 15:05 ` Chao Yu
-1 siblings, 0 replies; 16+ messages in thread
From: Chao Yu @ 2022-01-04 15:05 UTC (permalink / raw)
To: Greg KH; +Cc: stable, jaegeuk, linux-f2fs-devel, Yi Zhuang
On 2022/1/4 21:18, Greg KH wrote:
> On Tue, Jan 04, 2022 at 09:05:13PM +0800, Chao Yu wrote:
>> commit a5c0042200b28fff3bde6fa128ddeaef97990f8d upstream.
>>
>> As Yi Zhuang reported in bugzilla:
>>
>> https://bugzilla.kernel.org/show_bug.cgi?id=214299
>>
>> There is potential deadlock during quota data flush as below:
>>
>> Thread A: Thread B:
>> f2fs_dquot_acquire
>> down_read(&sbi->quota_sem)
>> f2fs_write_checkpoint
>> block_operations
>> f2fs_look_all
>> down_write(&sbi->cp_rwsem)
>> f2fs_quota_write
>> f2fs_write_begin
>> __do_map_lock
>> f2fs_lock_op
>> down_read(&sbi->cp_rwsem)
>> __need_flush_qutoa
>> down_write(&sbi->quota_sem)
>>
>> This patch changes block_operations() to use trylock, if it fails,
>> it means there is potential quota data updater, in this condition,
>> let's flush quota data first and then trylock again to check dirty
>> status of quota data.
>>
>> The side effect is: in heavy race condition (e.g. multi quota data
>> upaters vs quota data flusher), it may decrease the probability of
>> synchronizing quota data successfully in checkpoint() due to limited
>> retry time of quota flush.
>>
>> Fixes: db6ec53b7e03 ("f2fs: add a rw_sem to cover quota flag changes")
>> Cc: stable@vger.kernel.org # v5.3+
>> Reported-by: Yi Zhuang <zhuangyi1@huawei.com>
>> Signed-off-by: Chao Yu <chao@kernel.org>
>> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
>> ---
>> fs/f2fs/checkpoint.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
>> index 83e9bc0f91ff..7b0282724231 100644
>> --- a/fs/f2fs/checkpoint.c
>> +++ b/fs/f2fs/checkpoint.c
>> @@ -1162,7 +1162,8 @@ static bool __need_flush_quota(struct f2fs_sb_info *sbi)
>> if (!is_journalled_quota(sbi))
>> return false;
>>
>> - down_write(&sbi->quota_sem);
>> + if (!down_write_trylock(&sbi->quota_sem))
>> + return true;
>> if (is_sbi_flag_set(sbi, SBI_QUOTA_SKIP_FLUSH)) {
>> ret = false;
>> } else if (is_sbi_flag_set(sbi, SBI_QUOTA_NEED_REPAIR)) {
>> --
>> 2.32.0
>>
>
> What stable tree(s) is this for?
Oh, please help to try applying to 5.4, 5.10, and 5.15 stable trees, thanks!
Let me know if I should send patches for different trees separately.
Thanks,
>
> thanks,
>
> greg k-h
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [f2fs-dev] [PATCH] f2fs: quota: fix potential deadlock
@ 2022-01-04 15:05 ` Chao Yu
0 siblings, 0 replies; 16+ messages in thread
From: Chao Yu @ 2022-01-04 15:05 UTC (permalink / raw)
To: Greg KH; +Cc: jaegeuk, Yi Zhuang, stable, linux-f2fs-devel
On 2022/1/4 21:18, Greg KH wrote:
> On Tue, Jan 04, 2022 at 09:05:13PM +0800, Chao Yu wrote:
>> commit a5c0042200b28fff3bde6fa128ddeaef97990f8d upstream.
>>
>> As Yi Zhuang reported in bugzilla:
>>
>> https://bugzilla.kernel.org/show_bug.cgi?id=214299
>>
>> There is potential deadlock during quota data flush as below:
>>
>> Thread A: Thread B:
>> f2fs_dquot_acquire
>> down_read(&sbi->quota_sem)
>> f2fs_write_checkpoint
>> block_operations
>> f2fs_look_all
>> down_write(&sbi->cp_rwsem)
>> f2fs_quota_write
>> f2fs_write_begin
>> __do_map_lock
>> f2fs_lock_op
>> down_read(&sbi->cp_rwsem)
>> __need_flush_qutoa
>> down_write(&sbi->quota_sem)
>>
>> This patch changes block_operations() to use trylock, if it fails,
>> it means there is potential quota data updater, in this condition,
>> let's flush quota data first and then trylock again to check dirty
>> status of quota data.
>>
>> The side effect is: in heavy race condition (e.g. multi quota data
>> upaters vs quota data flusher), it may decrease the probability of
>> synchronizing quota data successfully in checkpoint() due to limited
>> retry time of quota flush.
>>
>> Fixes: db6ec53b7e03 ("f2fs: add a rw_sem to cover quota flag changes")
>> Cc: stable@vger.kernel.org # v5.3+
>> Reported-by: Yi Zhuang <zhuangyi1@huawei.com>
>> Signed-off-by: Chao Yu <chao@kernel.org>
>> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
>> ---
>> fs/f2fs/checkpoint.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
>> index 83e9bc0f91ff..7b0282724231 100644
>> --- a/fs/f2fs/checkpoint.c
>> +++ b/fs/f2fs/checkpoint.c
>> @@ -1162,7 +1162,8 @@ static bool __need_flush_quota(struct f2fs_sb_info *sbi)
>> if (!is_journalled_quota(sbi))
>> return false;
>>
>> - down_write(&sbi->quota_sem);
>> + if (!down_write_trylock(&sbi->quota_sem))
>> + return true;
>> if (is_sbi_flag_set(sbi, SBI_QUOTA_SKIP_FLUSH)) {
>> ret = false;
>> } else if (is_sbi_flag_set(sbi, SBI_QUOTA_NEED_REPAIR)) {
>> --
>> 2.32.0
>>
>
> What stable tree(s) is this for?
Oh, please help to try applying to 5.4, 5.10, and 5.15 stable trees, thanks!
Let me know if I should send patches for different trees separately.
Thanks,
>
> thanks,
>
> greg k-h
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH] f2fs: quota: fix potential deadlock
2022-01-04 15:05 ` [f2fs-dev] " Chao Yu
@ 2022-01-04 15:17 ` Greg KH
-1 siblings, 0 replies; 16+ messages in thread
From: Greg KH @ 2022-01-04 15:17 UTC (permalink / raw)
To: Chao Yu; +Cc: stable, jaegeuk, linux-f2fs-devel, Yi Zhuang
On Tue, Jan 04, 2022 at 11:05:36PM +0800, Chao Yu wrote:
> On 2022/1/4 21:18, Greg KH wrote:
> > On Tue, Jan 04, 2022 at 09:05:13PM +0800, Chao Yu wrote:
> > > commit a5c0042200b28fff3bde6fa128ddeaef97990f8d upstream.
> > >
> > > As Yi Zhuang reported in bugzilla:
> > >
> > > https://bugzilla.kernel.org/show_bug.cgi?id=214299
> > >
> > > There is potential deadlock during quota data flush as below:
> > >
> > > Thread A: Thread B:
> > > f2fs_dquot_acquire
> > > down_read(&sbi->quota_sem)
> > > f2fs_write_checkpoint
> > > block_operations
> > > f2fs_look_all
> > > down_write(&sbi->cp_rwsem)
> > > f2fs_quota_write
> > > f2fs_write_begin
> > > __do_map_lock
> > > f2fs_lock_op
> > > down_read(&sbi->cp_rwsem)
> > > __need_flush_qutoa
> > > down_write(&sbi->quota_sem)
> > >
> > > This patch changes block_operations() to use trylock, if it fails,
> > > it means there is potential quota data updater, in this condition,
> > > let's flush quota data first and then trylock again to check dirty
> > > status of quota data.
> > >
> > > The side effect is: in heavy race condition (e.g. multi quota data
> > > upaters vs quota data flusher), it may decrease the probability of
> > > synchronizing quota data successfully in checkpoint() due to limited
> > > retry time of quota flush.
> > >
> > > Fixes: db6ec53b7e03 ("f2fs: add a rw_sem to cover quota flag changes")
> > > Cc: stable@vger.kernel.org # v5.3+
> > > Reported-by: Yi Zhuang <zhuangyi1@huawei.com>
> > > Signed-off-by: Chao Yu <chao@kernel.org>
> > > Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> > > ---
> > > fs/f2fs/checkpoint.c | 3 ++-
> > > 1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
> > > index 83e9bc0f91ff..7b0282724231 100644
> > > --- a/fs/f2fs/checkpoint.c
> > > +++ b/fs/f2fs/checkpoint.c
> > > @@ -1162,7 +1162,8 @@ static bool __need_flush_quota(struct f2fs_sb_info *sbi)
> > > if (!is_journalled_quota(sbi))
> > > return false;
> > > - down_write(&sbi->quota_sem);
> > > + if (!down_write_trylock(&sbi->quota_sem))
> > > + return true;
> > > if (is_sbi_flag_set(sbi, SBI_QUOTA_SKIP_FLUSH)) {
> > > ret = false;
> > > } else if (is_sbi_flag_set(sbi, SBI_QUOTA_NEED_REPAIR)) {
> > > --
> > > 2.32.0
> > >
> >
> > What stable tree(s) is this for?
>
> Oh, please help to try applying to 5.4, 5.10, and 5.15 stable trees, thanks!
This is already in the 5.15.6 kernel release, do you need it applied
there again? :)
> Let me know if I should send patches for different trees separately.
If the same commit here works for all of the above, it's fine. But for
some reason I don't think it will work in 5.15.y...
thanks,
greg k-h
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [f2fs-dev] [PATCH] f2fs: quota: fix potential deadlock
@ 2022-01-04 15:17 ` Greg KH
0 siblings, 0 replies; 16+ messages in thread
From: Greg KH @ 2022-01-04 15:17 UTC (permalink / raw)
To: Chao Yu; +Cc: jaegeuk, Yi Zhuang, stable, linux-f2fs-devel
On Tue, Jan 04, 2022 at 11:05:36PM +0800, Chao Yu wrote:
> On 2022/1/4 21:18, Greg KH wrote:
> > On Tue, Jan 04, 2022 at 09:05:13PM +0800, Chao Yu wrote:
> > > commit a5c0042200b28fff3bde6fa128ddeaef97990f8d upstream.
> > >
> > > As Yi Zhuang reported in bugzilla:
> > >
> > > https://bugzilla.kernel.org/show_bug.cgi?id=214299
> > >
> > > There is potential deadlock during quota data flush as below:
> > >
> > > Thread A: Thread B:
> > > f2fs_dquot_acquire
> > > down_read(&sbi->quota_sem)
> > > f2fs_write_checkpoint
> > > block_operations
> > > f2fs_look_all
> > > down_write(&sbi->cp_rwsem)
> > > f2fs_quota_write
> > > f2fs_write_begin
> > > __do_map_lock
> > > f2fs_lock_op
> > > down_read(&sbi->cp_rwsem)
> > > __need_flush_qutoa
> > > down_write(&sbi->quota_sem)
> > >
> > > This patch changes block_operations() to use trylock, if it fails,
> > > it means there is potential quota data updater, in this condition,
> > > let's flush quota data first and then trylock again to check dirty
> > > status of quota data.
> > >
> > > The side effect is: in heavy race condition (e.g. multi quota data
> > > upaters vs quota data flusher), it may decrease the probability of
> > > synchronizing quota data successfully in checkpoint() due to limited
> > > retry time of quota flush.
> > >
> > > Fixes: db6ec53b7e03 ("f2fs: add a rw_sem to cover quota flag changes")
> > > Cc: stable@vger.kernel.org # v5.3+
> > > Reported-by: Yi Zhuang <zhuangyi1@huawei.com>
> > > Signed-off-by: Chao Yu <chao@kernel.org>
> > > Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> > > ---
> > > fs/f2fs/checkpoint.c | 3 ++-
> > > 1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
> > > index 83e9bc0f91ff..7b0282724231 100644
> > > --- a/fs/f2fs/checkpoint.c
> > > +++ b/fs/f2fs/checkpoint.c
> > > @@ -1162,7 +1162,8 @@ static bool __need_flush_quota(struct f2fs_sb_info *sbi)
> > > if (!is_journalled_quota(sbi))
> > > return false;
> > > - down_write(&sbi->quota_sem);
> > > + if (!down_write_trylock(&sbi->quota_sem))
> > > + return true;
> > > if (is_sbi_flag_set(sbi, SBI_QUOTA_SKIP_FLUSH)) {
> > > ret = false;
> > > } else if (is_sbi_flag_set(sbi, SBI_QUOTA_NEED_REPAIR)) {
> > > --
> > > 2.32.0
> > >
> >
> > What stable tree(s) is this for?
>
> Oh, please help to try applying to 5.4, 5.10, and 5.15 stable trees, thanks!
This is already in the 5.15.6 kernel release, do you need it applied
there again? :)
> Let me know if I should send patches for different trees separately.
If the same commit here works for all of the above, it's fine. But for
some reason I don't think it will work in 5.15.y...
thanks,
greg k-h
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH] f2fs: quota: fix potential deadlock
2022-01-04 15:17 ` [f2fs-dev] " Greg KH
@ 2022-01-04 15:48 ` Chao Yu
-1 siblings, 0 replies; 16+ messages in thread
From: Chao Yu @ 2022-01-04 15:48 UTC (permalink / raw)
To: Greg KH; +Cc: stable, jaegeuk, linux-f2fs-devel, Yi Zhuang
On 2022/1/4 23:17, Greg KH wrote:
> On Tue, Jan 04, 2022 at 11:05:36PM +0800, Chao Yu wrote:
>> On 2022/1/4 21:18, Greg KH wrote:
>>> On Tue, Jan 04, 2022 at 09:05:13PM +0800, Chao Yu wrote:
>>>> commit a5c0042200b28fff3bde6fa128ddeaef97990f8d upstream.
>>>>
>>>> As Yi Zhuang reported in bugzilla:
>>>>
>>>> https://bugzilla.kernel.org/show_bug.cgi?id=214299
>>>>
>>>> There is potential deadlock during quota data flush as below:
>>>>
>>>> Thread A: Thread B:
>>>> f2fs_dquot_acquire
>>>> down_read(&sbi->quota_sem)
>>>> f2fs_write_checkpoint
>>>> block_operations
>>>> f2fs_look_all
>>>> down_write(&sbi->cp_rwsem)
>>>> f2fs_quota_write
>>>> f2fs_write_begin
>>>> __do_map_lock
>>>> f2fs_lock_op
>>>> down_read(&sbi->cp_rwsem)
>>>> __need_flush_qutoa
>>>> down_write(&sbi->quota_sem)
>>>>
>>>> This patch changes block_operations() to use trylock, if it fails,
>>>> it means there is potential quota data updater, in this condition,
>>>> let's flush quota data first and then trylock again to check dirty
>>>> status of quota data.
>>>>
>>>> The side effect is: in heavy race condition (e.g. multi quota data
>>>> upaters vs quota data flusher), it may decrease the probability of
>>>> synchronizing quota data successfully in checkpoint() due to limited
>>>> retry time of quota flush.
>>>>
>>>> Fixes: db6ec53b7e03 ("f2fs: add a rw_sem to cover quota flag changes")
>>>> Cc: stable@vger.kernel.org # v5.3+
>>>> Reported-by: Yi Zhuang <zhuangyi1@huawei.com>
>>>> Signed-off-by: Chao Yu <chao@kernel.org>
>>>> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
>>>> ---
>>>> fs/f2fs/checkpoint.c | 3 ++-
>>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
>>>> index 83e9bc0f91ff..7b0282724231 100644
>>>> --- a/fs/f2fs/checkpoint.c
>>>> +++ b/fs/f2fs/checkpoint.c
>>>> @@ -1162,7 +1162,8 @@ static bool __need_flush_quota(struct f2fs_sb_info *sbi)
>>>> if (!is_journalled_quota(sbi))
>>>> return false;
>>>> - down_write(&sbi->quota_sem);
>>>> + if (!down_write_trylock(&sbi->quota_sem))
>>>> + return true;
>>>> if (is_sbi_flag_set(sbi, SBI_QUOTA_SKIP_FLUSH)) {
>>>> ret = false;
>>>> } else if (is_sbi_flag_set(sbi, SBI_QUOTA_NEED_REPAIR)) {
>>>> --
>>>> 2.32.0
>>>>
>>>
>>> What stable tree(s) is this for?
>>
>> Oh, please help to try applying to 5.4, 5.10, and 5.15 stable trees, thanks!
>
> This is already in the 5.15.6 kernel release, do you need it applied
> there again? :)
Oops, no, so 5.4 and 5.10 is enough. ;)
We can skip 5.15 since this patch was merged in 5.15-rc1 at the first time.
>
>> Let me know if I should send patches for different trees separately.
>
> If the same commit here works for all of the above, it's fine. But for
> some reason I don't think it will work in 5.15.y...
Copied.
Thank you for the help!
Thanks,
>
> thanks,
>
> greg k-h
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [f2fs-dev] [PATCH] f2fs: quota: fix potential deadlock
@ 2022-01-04 15:48 ` Chao Yu
0 siblings, 0 replies; 16+ messages in thread
From: Chao Yu @ 2022-01-04 15:48 UTC (permalink / raw)
To: Greg KH; +Cc: jaegeuk, Yi Zhuang, stable, linux-f2fs-devel
On 2022/1/4 23:17, Greg KH wrote:
> On Tue, Jan 04, 2022 at 11:05:36PM +0800, Chao Yu wrote:
>> On 2022/1/4 21:18, Greg KH wrote:
>>> On Tue, Jan 04, 2022 at 09:05:13PM +0800, Chao Yu wrote:
>>>> commit a5c0042200b28fff3bde6fa128ddeaef97990f8d upstream.
>>>>
>>>> As Yi Zhuang reported in bugzilla:
>>>>
>>>> https://bugzilla.kernel.org/show_bug.cgi?id=214299
>>>>
>>>> There is potential deadlock during quota data flush as below:
>>>>
>>>> Thread A: Thread B:
>>>> f2fs_dquot_acquire
>>>> down_read(&sbi->quota_sem)
>>>> f2fs_write_checkpoint
>>>> block_operations
>>>> f2fs_look_all
>>>> down_write(&sbi->cp_rwsem)
>>>> f2fs_quota_write
>>>> f2fs_write_begin
>>>> __do_map_lock
>>>> f2fs_lock_op
>>>> down_read(&sbi->cp_rwsem)
>>>> __need_flush_qutoa
>>>> down_write(&sbi->quota_sem)
>>>>
>>>> This patch changes block_operations() to use trylock, if it fails,
>>>> it means there is potential quota data updater, in this condition,
>>>> let's flush quota data first and then trylock again to check dirty
>>>> status of quota data.
>>>>
>>>> The side effect is: in heavy race condition (e.g. multi quota data
>>>> upaters vs quota data flusher), it may decrease the probability of
>>>> synchronizing quota data successfully in checkpoint() due to limited
>>>> retry time of quota flush.
>>>>
>>>> Fixes: db6ec53b7e03 ("f2fs: add a rw_sem to cover quota flag changes")
>>>> Cc: stable@vger.kernel.org # v5.3+
>>>> Reported-by: Yi Zhuang <zhuangyi1@huawei.com>
>>>> Signed-off-by: Chao Yu <chao@kernel.org>
>>>> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
>>>> ---
>>>> fs/f2fs/checkpoint.c | 3 ++-
>>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
>>>> index 83e9bc0f91ff..7b0282724231 100644
>>>> --- a/fs/f2fs/checkpoint.c
>>>> +++ b/fs/f2fs/checkpoint.c
>>>> @@ -1162,7 +1162,8 @@ static bool __need_flush_quota(struct f2fs_sb_info *sbi)
>>>> if (!is_journalled_quota(sbi))
>>>> return false;
>>>> - down_write(&sbi->quota_sem);
>>>> + if (!down_write_trylock(&sbi->quota_sem))
>>>> + return true;
>>>> if (is_sbi_flag_set(sbi, SBI_QUOTA_SKIP_FLUSH)) {
>>>> ret = false;
>>>> } else if (is_sbi_flag_set(sbi, SBI_QUOTA_NEED_REPAIR)) {
>>>> --
>>>> 2.32.0
>>>>
>>>
>>> What stable tree(s) is this for?
>>
>> Oh, please help to try applying to 5.4, 5.10, and 5.15 stable trees, thanks!
>
> This is already in the 5.15.6 kernel release, do you need it applied
> there again? :)
Oops, no, so 5.4 and 5.10 is enough. ;)
We can skip 5.15 since this patch was merged in 5.15-rc1 at the first time.
>
>> Let me know if I should send patches for different trees separately.
>
> If the same commit here works for all of the above, it's fine. But for
> some reason I don't think it will work in 5.15.y...
Copied.
Thank you for the help!
Thanks,
>
> thanks,
>
> greg k-h
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH] f2fs: quota: fix potential deadlock
2022-01-04 15:48 ` [f2fs-dev] " Chao Yu
@ 2022-01-05 15:01 ` Greg KH
-1 siblings, 0 replies; 16+ messages in thread
From: Greg KH @ 2022-01-05 15:01 UTC (permalink / raw)
To: Chao Yu; +Cc: stable, jaegeuk, linux-f2fs-devel, Yi Zhuang
On Tue, Jan 04, 2022 at 11:48:25PM +0800, Chao Yu wrote:
> On 2022/1/4 23:17, Greg KH wrote:
> > On Tue, Jan 04, 2022 at 11:05:36PM +0800, Chao Yu wrote:
> > > On 2022/1/4 21:18, Greg KH wrote:
> > > > On Tue, Jan 04, 2022 at 09:05:13PM +0800, Chao Yu wrote:
> > > > > commit a5c0042200b28fff3bde6fa128ddeaef97990f8d upstream.
> > > > >
> > > > > As Yi Zhuang reported in bugzilla:
> > > > >
> > > > > https://bugzilla.kernel.org/show_bug.cgi?id=214299
> > > > >
> > > > > There is potential deadlock during quota data flush as below:
> > > > >
> > > > > Thread A: Thread B:
> > > > > f2fs_dquot_acquire
> > > > > down_read(&sbi->quota_sem)
> > > > > f2fs_write_checkpoint
> > > > > block_operations
> > > > > f2fs_look_all
> > > > > down_write(&sbi->cp_rwsem)
> > > > > f2fs_quota_write
> > > > > f2fs_write_begin
> > > > > __do_map_lock
> > > > > f2fs_lock_op
> > > > > down_read(&sbi->cp_rwsem)
> > > > > __need_flush_qutoa
> > > > > down_write(&sbi->quota_sem)
> > > > >
> > > > > This patch changes block_operations() to use trylock, if it fails,
> > > > > it means there is potential quota data updater, in this condition,
> > > > > let's flush quota data first and then trylock again to check dirty
> > > > > status of quota data.
> > > > >
> > > > > The side effect is: in heavy race condition (e.g. multi quota data
> > > > > upaters vs quota data flusher), it may decrease the probability of
> > > > > synchronizing quota data successfully in checkpoint() due to limited
> > > > > retry time of quota flush.
> > > > >
> > > > > Fixes: db6ec53b7e03 ("f2fs: add a rw_sem to cover quota flag changes")
> > > > > Cc: stable@vger.kernel.org # v5.3+
> > > > > Reported-by: Yi Zhuang <zhuangyi1@huawei.com>
> > > > > Signed-off-by: Chao Yu <chao@kernel.org>
> > > > > Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> > > > > ---
> > > > > fs/f2fs/checkpoint.c | 3 ++-
> > > > > 1 file changed, 2 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
> > > > > index 83e9bc0f91ff..7b0282724231 100644
> > > > > --- a/fs/f2fs/checkpoint.c
> > > > > +++ b/fs/f2fs/checkpoint.c
> > > > > @@ -1162,7 +1162,8 @@ static bool __need_flush_quota(struct f2fs_sb_info *sbi)
> > > > > if (!is_journalled_quota(sbi))
> > > > > return false;
> > > > > - down_write(&sbi->quota_sem);
> > > > > + if (!down_write_trylock(&sbi->quota_sem))
> > > > > + return true;
> > > > > if (is_sbi_flag_set(sbi, SBI_QUOTA_SKIP_FLUSH)) {
> > > > > ret = false;
> > > > > } else if (is_sbi_flag_set(sbi, SBI_QUOTA_NEED_REPAIR)) {
> > > > > --
> > > > > 2.32.0
> > > > >
> > > >
> > > > What stable tree(s) is this for?
> > >
> > > Oh, please help to try applying to 5.4, 5.10, and 5.15 stable trees, thanks!
> >
> > This is already in the 5.15.6 kernel release, do you need it applied
> > there again? :)
>
> Oops, no, so 5.4 and 5.10 is enough. ;)
> We can skip 5.15 since this patch was merged in 5.15-rc1 at the first time.
It was merged in 5.16-rc1, and then backported to 5.15.6. You might
want to check your git scripts.
Anyway, now queued up, thanks.
greg k-h
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [f2fs-dev] [PATCH] f2fs: quota: fix potential deadlock
@ 2022-01-05 15:01 ` Greg KH
0 siblings, 0 replies; 16+ messages in thread
From: Greg KH @ 2022-01-05 15:01 UTC (permalink / raw)
To: Chao Yu; +Cc: jaegeuk, Yi Zhuang, stable, linux-f2fs-devel
On Tue, Jan 04, 2022 at 11:48:25PM +0800, Chao Yu wrote:
> On 2022/1/4 23:17, Greg KH wrote:
> > On Tue, Jan 04, 2022 at 11:05:36PM +0800, Chao Yu wrote:
> > > On 2022/1/4 21:18, Greg KH wrote:
> > > > On Tue, Jan 04, 2022 at 09:05:13PM +0800, Chao Yu wrote:
> > > > > commit a5c0042200b28fff3bde6fa128ddeaef97990f8d upstream.
> > > > >
> > > > > As Yi Zhuang reported in bugzilla:
> > > > >
> > > > > https://bugzilla.kernel.org/show_bug.cgi?id=214299
> > > > >
> > > > > There is potential deadlock during quota data flush as below:
> > > > >
> > > > > Thread A: Thread B:
> > > > > f2fs_dquot_acquire
> > > > > down_read(&sbi->quota_sem)
> > > > > f2fs_write_checkpoint
> > > > > block_operations
> > > > > f2fs_look_all
> > > > > down_write(&sbi->cp_rwsem)
> > > > > f2fs_quota_write
> > > > > f2fs_write_begin
> > > > > __do_map_lock
> > > > > f2fs_lock_op
> > > > > down_read(&sbi->cp_rwsem)
> > > > > __need_flush_qutoa
> > > > > down_write(&sbi->quota_sem)
> > > > >
> > > > > This patch changes block_operations() to use trylock, if it fails,
> > > > > it means there is potential quota data updater, in this condition,
> > > > > let's flush quota data first and then trylock again to check dirty
> > > > > status of quota data.
> > > > >
> > > > > The side effect is: in heavy race condition (e.g. multi quota data
> > > > > upaters vs quota data flusher), it may decrease the probability of
> > > > > synchronizing quota data successfully in checkpoint() due to limited
> > > > > retry time of quota flush.
> > > > >
> > > > > Fixes: db6ec53b7e03 ("f2fs: add a rw_sem to cover quota flag changes")
> > > > > Cc: stable@vger.kernel.org # v5.3+
> > > > > Reported-by: Yi Zhuang <zhuangyi1@huawei.com>
> > > > > Signed-off-by: Chao Yu <chao@kernel.org>
> > > > > Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> > > > > ---
> > > > > fs/f2fs/checkpoint.c | 3 ++-
> > > > > 1 file changed, 2 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
> > > > > index 83e9bc0f91ff..7b0282724231 100644
> > > > > --- a/fs/f2fs/checkpoint.c
> > > > > +++ b/fs/f2fs/checkpoint.c
> > > > > @@ -1162,7 +1162,8 @@ static bool __need_flush_quota(struct f2fs_sb_info *sbi)
> > > > > if (!is_journalled_quota(sbi))
> > > > > return false;
> > > > > - down_write(&sbi->quota_sem);
> > > > > + if (!down_write_trylock(&sbi->quota_sem))
> > > > > + return true;
> > > > > if (is_sbi_flag_set(sbi, SBI_QUOTA_SKIP_FLUSH)) {
> > > > > ret = false;
> > > > > } else if (is_sbi_flag_set(sbi, SBI_QUOTA_NEED_REPAIR)) {
> > > > > --
> > > > > 2.32.0
> > > > >
> > > >
> > > > What stable tree(s) is this for?
> > >
> > > Oh, please help to try applying to 5.4, 5.10, and 5.15 stable trees, thanks!
> >
> > This is already in the 5.15.6 kernel release, do you need it applied
> > there again? :)
>
> Oops, no, so 5.4 and 5.10 is enough. ;)
> We can skip 5.15 since this patch was merged in 5.15-rc1 at the first time.
It was merged in 5.16-rc1, and then backported to 5.15.6. You might
want to check your git scripts.
Anyway, now queued up, thanks.
greg k-h
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH] f2fs: quota: fix potential deadlock
2022-01-05 15:01 ` [f2fs-dev] " Greg KH
@ 2022-01-05 15:25 ` Chao Yu
-1 siblings, 0 replies; 16+ messages in thread
From: Chao Yu @ 2022-01-05 15:25 UTC (permalink / raw)
To: Greg KH; +Cc: stable, jaegeuk, linux-f2fs-devel, Yi Zhuang
On 2022/1/5 23:01, Greg KH wrote:
> On Tue, Jan 04, 2022 at 11:48:25PM +0800, Chao Yu wrote:
>> On 2022/1/4 23:17, Greg KH wrote:
>>> On Tue, Jan 04, 2022 at 11:05:36PM +0800, Chao Yu wrote:
>>>> On 2022/1/4 21:18, Greg KH wrote:
>>>>> On Tue, Jan 04, 2022 at 09:05:13PM +0800, Chao Yu wrote:
>>>>>> commit a5c0042200b28fff3bde6fa128ddeaef97990f8d upstream.
>>>>>>
>>>>>> As Yi Zhuang reported in bugzilla:
>>>>>>
>>>>>> https://bugzilla.kernel.org/show_bug.cgi?id=214299
>>>>>>
>>>>>> There is potential deadlock during quota data flush as below:
>>>>>>
>>>>>> Thread A: Thread B:
>>>>>> f2fs_dquot_acquire
>>>>>> down_read(&sbi->quota_sem)
>>>>>> f2fs_write_checkpoint
>>>>>> block_operations
>>>>>> f2fs_look_all
>>>>>> down_write(&sbi->cp_rwsem)
>>>>>> f2fs_quota_write
>>>>>> f2fs_write_begin
>>>>>> __do_map_lock
>>>>>> f2fs_lock_op
>>>>>> down_read(&sbi->cp_rwsem)
>>>>>> __need_flush_qutoa
>>>>>> down_write(&sbi->quota_sem)
>>>>>>
>>>>>> This patch changes block_operations() to use trylock, if it fails,
>>>>>> it means there is potential quota data updater, in this condition,
>>>>>> let's flush quota data first and then trylock again to check dirty
>>>>>> status of quota data.
>>>>>>
>>>>>> The side effect is: in heavy race condition (e.g. multi quota data
>>>>>> upaters vs quota data flusher), it may decrease the probability of
>>>>>> synchronizing quota data successfully in checkpoint() due to limited
>>>>>> retry time of quota flush.
>>>>>>
>>>>>> Fixes: db6ec53b7e03 ("f2fs: add a rw_sem to cover quota flag changes")
>>>>>> Cc: stable@vger.kernel.org # v5.3+
>>>>>> Reported-by: Yi Zhuang <zhuangyi1@huawei.com>
>>>>>> Signed-off-by: Chao Yu <chao@kernel.org>
>>>>>> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
>>>>>> ---
>>>>>> fs/f2fs/checkpoint.c | 3 ++-
>>>>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
>>>>>> index 83e9bc0f91ff..7b0282724231 100644
>>>>>> --- a/fs/f2fs/checkpoint.c
>>>>>> +++ b/fs/f2fs/checkpoint.c
>>>>>> @@ -1162,7 +1162,8 @@ static bool __need_flush_quota(struct f2fs_sb_info *sbi)
>>>>>> if (!is_journalled_quota(sbi))
>>>>>> return false;
>>>>>> - down_write(&sbi->quota_sem);
>>>>>> + if (!down_write_trylock(&sbi->quota_sem))
>>>>>> + return true;
>>>>>> if (is_sbi_flag_set(sbi, SBI_QUOTA_SKIP_FLUSH)) {
>>>>>> ret = false;
>>>>>> } else if (is_sbi_flag_set(sbi, SBI_QUOTA_NEED_REPAIR)) {
>>>>>> --
>>>>>> 2.32.0
>>>>>>
>>>>>
>>>>> What stable tree(s) is this for?
>>>>
>>>> Oh, please help to try applying to 5.4, 5.10, and 5.15 stable trees, thanks!
>>>
>>> This is already in the 5.15.6 kernel release, do you need it applied
>>> there again? :)
>>
>> Oops, no, so 5.4 and 5.10 is enough. ;)
>> We can skip 5.15 since this patch was merged in 5.15-rc1 at the first time.
>
> It was merged in 5.16-rc1, and then backported to 5.15.6. You might
> want to check your git scripts.
Oh, yes, you're right, it looks we merged patches with same title in both 5.15-rc1
and 5.16-rc1, that won't happen usually... so I positioning the wrong merge time
only based on patch title...
https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs.git/commit/?h=dev&id=a5c0042200b28fff3bde6fa128ddeaef97990f8d
https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs.git/commit/?h=dev&id=9de71ede81e6d1a111fdd868b2d78d459fa77f80
>
> Anyway, now queued up, thanks.
Thank you!
Thanks,
>
> greg k-h
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [f2fs-dev] [PATCH] f2fs: quota: fix potential deadlock
@ 2022-01-05 15:25 ` Chao Yu
0 siblings, 0 replies; 16+ messages in thread
From: Chao Yu @ 2022-01-05 15:25 UTC (permalink / raw)
To: Greg KH; +Cc: jaegeuk, Yi Zhuang, stable, linux-f2fs-devel
On 2022/1/5 23:01, Greg KH wrote:
> On Tue, Jan 04, 2022 at 11:48:25PM +0800, Chao Yu wrote:
>> On 2022/1/4 23:17, Greg KH wrote:
>>> On Tue, Jan 04, 2022 at 11:05:36PM +0800, Chao Yu wrote:
>>>> On 2022/1/4 21:18, Greg KH wrote:
>>>>> On Tue, Jan 04, 2022 at 09:05:13PM +0800, Chao Yu wrote:
>>>>>> commit a5c0042200b28fff3bde6fa128ddeaef97990f8d upstream.
>>>>>>
>>>>>> As Yi Zhuang reported in bugzilla:
>>>>>>
>>>>>> https://bugzilla.kernel.org/show_bug.cgi?id=214299
>>>>>>
>>>>>> There is potential deadlock during quota data flush as below:
>>>>>>
>>>>>> Thread A: Thread B:
>>>>>> f2fs_dquot_acquire
>>>>>> down_read(&sbi->quota_sem)
>>>>>> f2fs_write_checkpoint
>>>>>> block_operations
>>>>>> f2fs_look_all
>>>>>> down_write(&sbi->cp_rwsem)
>>>>>> f2fs_quota_write
>>>>>> f2fs_write_begin
>>>>>> __do_map_lock
>>>>>> f2fs_lock_op
>>>>>> down_read(&sbi->cp_rwsem)
>>>>>> __need_flush_qutoa
>>>>>> down_write(&sbi->quota_sem)
>>>>>>
>>>>>> This patch changes block_operations() to use trylock, if it fails,
>>>>>> it means there is potential quota data updater, in this condition,
>>>>>> let's flush quota data first and then trylock again to check dirty
>>>>>> status of quota data.
>>>>>>
>>>>>> The side effect is: in heavy race condition (e.g. multi quota data
>>>>>> upaters vs quota data flusher), it may decrease the probability of
>>>>>> synchronizing quota data successfully in checkpoint() due to limited
>>>>>> retry time of quota flush.
>>>>>>
>>>>>> Fixes: db6ec53b7e03 ("f2fs: add a rw_sem to cover quota flag changes")
>>>>>> Cc: stable@vger.kernel.org # v5.3+
>>>>>> Reported-by: Yi Zhuang <zhuangyi1@huawei.com>
>>>>>> Signed-off-by: Chao Yu <chao@kernel.org>
>>>>>> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
>>>>>> ---
>>>>>> fs/f2fs/checkpoint.c | 3 ++-
>>>>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
>>>>>> index 83e9bc0f91ff..7b0282724231 100644
>>>>>> --- a/fs/f2fs/checkpoint.c
>>>>>> +++ b/fs/f2fs/checkpoint.c
>>>>>> @@ -1162,7 +1162,8 @@ static bool __need_flush_quota(struct f2fs_sb_info *sbi)
>>>>>> if (!is_journalled_quota(sbi))
>>>>>> return false;
>>>>>> - down_write(&sbi->quota_sem);
>>>>>> + if (!down_write_trylock(&sbi->quota_sem))
>>>>>> + return true;
>>>>>> if (is_sbi_flag_set(sbi, SBI_QUOTA_SKIP_FLUSH)) {
>>>>>> ret = false;
>>>>>> } else if (is_sbi_flag_set(sbi, SBI_QUOTA_NEED_REPAIR)) {
>>>>>> --
>>>>>> 2.32.0
>>>>>>
>>>>>
>>>>> What stable tree(s) is this for?
>>>>
>>>> Oh, please help to try applying to 5.4, 5.10, and 5.15 stable trees, thanks!
>>>
>>> This is already in the 5.15.6 kernel release, do you need it applied
>>> there again? :)
>>
>> Oops, no, so 5.4 and 5.10 is enough. ;)
>> We can skip 5.15 since this patch was merged in 5.15-rc1 at the first time.
>
> It was merged in 5.16-rc1, and then backported to 5.15.6. You might
> want to check your git scripts.
Oh, yes, you're right, it looks we merged patches with same title in both 5.15-rc1
and 5.16-rc1, that won't happen usually... so I positioning the wrong merge time
only based on patch title...
https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs.git/commit/?h=dev&id=a5c0042200b28fff3bde6fa128ddeaef97990f8d
https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs.git/commit/?h=dev&id=9de71ede81e6d1a111fdd868b2d78d459fa77f80
>
> Anyway, now queued up, thanks.
Thank you!
Thanks,
>
> greg k-h
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH] f2fs: quota: fix potential deadlock
@ 2021-09-03 2:38 Chao Yu
0 siblings, 0 replies; 16+ messages in thread
From: Chao Yu @ 2021-09-03 2:38 UTC (permalink / raw)
To: jaegeuk; +Cc: linux-f2fs-devel, linux-kernel, Chao Yu, Yi Zhuang
As Yi Zhuang reported in bugzilla:
https://bugzilla.kernel.org/show_bug.cgi?id=214299
There is potential deadlock during quota data flush as below:
Thread A: Thread B:
f2fs_dquot_acquire
down_read(&sbi->quota_sem)
f2fs_write_checkpoint
block_operations
f2fs_look_all
down_write(&sbi->cp_rwsem)
f2fs_quota_write
f2fs_write_begin
__do_map_lock
f2fs_lock_op
down_read(&sbi->cp_rwsem)
__need_flush_qutoa
down_write(&sbi->quota_sem)
This patch changes block_operations() to use trylock, if it fails,
it means there is potential quota data updater, in this condition,
let's flush quota data first and then trylock again to check dirty
status of quota data.
The side effect is: in heavy race condition (e.g. multi quota data
upaters vs quota data flusher), it may decrease the probability of
synchronizing quota data successfully in checkpoint() due to limited
retry time of quota flush.
Reported-by: Yi Zhuang <zhuangyi1@huawei.com>
Signed-off-by: Chao Yu <chao@kernel.org>
---
fs/f2fs/checkpoint.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
index 7d8803a4cbc2..6f6a7d812d60 100644
--- a/fs/f2fs/checkpoint.c
+++ b/fs/f2fs/checkpoint.c
@@ -1159,7 +1159,8 @@ static bool __need_flush_quota(struct f2fs_sb_info *sbi)
if (!is_journalled_quota(sbi))
return false;
- down_write(&sbi->quota_sem);
+ if (!down_write_trylock(&sbi->quota_sem))
+ return true;
if (is_sbi_flag_set(sbi, SBI_QUOTA_SKIP_FLUSH)) {
ret = false;
} else if (is_sbi_flag_set(sbi, SBI_QUOTA_NEED_REPAIR)) {
--
2.32.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH] f2fs: quota: fix potential deadlock
@ 2021-07-19 8:46 Chao Yu
0 siblings, 0 replies; 16+ messages in thread
From: Chao Yu @ 2021-07-19 8:46 UTC (permalink / raw)
To: jaegeuk; +Cc: linux-f2fs-devel, linux-kernel, Chao Yu, Chao Yu
xfstest generic/587 reports a deadlock issue as below:
======================================================
WARNING: possible circular locking dependency detected
5.14.0-rc1 #69 Not tainted
------------------------------------------------------
repquota/8606 is trying to acquire lock:
ffff888022ac9320 (&sb->s_type->i_mutex_key#18){+.+.}-{3:3}, at: f2fs_quota_sync+0x207/0x300 [f2fs]
but task is already holding lock:
ffff8880084bcde8 (&sbi->quota_sem){.+.+}-{3:3}, at: f2fs_quota_sync+0x59/0x300 [f2fs]
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #2 (&sbi->quota_sem){.+.+}-{3:3}:
__lock_acquire+0x648/0x10b0
lock_acquire+0x128/0x470
down_read+0x3b/0x2a0
f2fs_quota_sync+0x59/0x300 [f2fs]
f2fs_quota_on+0x48/0x100 [f2fs]
do_quotactl+0x5e3/0xb30
__x64_sys_quotactl+0x23a/0x4e0
do_syscall_64+0x3b/0x90
entry_SYSCALL_64_after_hwframe+0x44/0xae
-> #1 (&sbi->cp_rwsem){++++}-{3:3}:
__lock_acquire+0x648/0x10b0
lock_acquire+0x128/0x470
down_read+0x3b/0x2a0
f2fs_unlink+0x353/0x670 [f2fs]
vfs_unlink+0x1c7/0x380
do_unlinkat+0x413/0x4b0
__x64_sys_unlinkat+0x50/0xb0
do_syscall_64+0x3b/0x90
entry_SYSCALL_64_after_hwframe+0x44/0xae
-> #0 (&sb->s_type->i_mutex_key#18){+.+.}-{3:3}:
check_prev_add+0xdc/0xb30
validate_chain+0xa67/0xb20
__lock_acquire+0x648/0x10b0
lock_acquire+0x128/0x470
down_write+0x39/0xc0
f2fs_quota_sync+0x207/0x300 [f2fs]
do_quotactl+0xaff/0xb30
__x64_sys_quotactl+0x23a/0x4e0
do_syscall_64+0x3b/0x90
entry_SYSCALL_64_after_hwframe+0x44/0xae
other info that might help us debug this:
Chain exists of:
&sb->s_type->i_mutex_key#18 --> &sbi->cp_rwsem --> &sbi->quota_sem
Possible unsafe locking scenario:
CPU0 CPU1
---- ----
lock(&sbi->quota_sem);
lock(&sbi->cp_rwsem);
lock(&sbi->quota_sem);
lock(&sb->s_type->i_mutex_key#18);
*** DEADLOCK ***
3 locks held by repquota/8606:
#0: ffff88801efac0e0 (&type->s_umount_key#53){++++}-{3:3}, at: user_get_super+0xd9/0x190
#1: ffff8880084bc380 (&sbi->cp_rwsem){++++}-{3:3}, at: f2fs_quota_sync+0x3e/0x300 [f2fs]
#2: ffff8880084bcde8 (&sbi->quota_sem){.+.+}-{3:3}, at: f2fs_quota_sync+0x59/0x300 [f2fs]
stack backtrace:
CPU: 6 PID: 8606 Comm: repquota Not tainted 5.14.0-rc1 #69
Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006
Call Trace:
dump_stack_lvl+0xce/0x134
dump_stack+0x17/0x20
print_circular_bug.isra.0.cold+0x239/0x253
check_noncircular+0x1be/0x1f0
check_prev_add+0xdc/0xb30
validate_chain+0xa67/0xb20
__lock_acquire+0x648/0x10b0
lock_acquire+0x128/0x470
down_write+0x39/0xc0
f2fs_quota_sync+0x207/0x300 [f2fs]
do_quotactl+0xaff/0xb30
__x64_sys_quotactl+0x23a/0x4e0
do_syscall_64+0x3b/0x90
entry_SYSCALL_64_after_hwframe+0x44/0xae
RIP: 0033:0x7f883b0b4efe
The root cause is ABBA deadlock of inode lock and cp_rwsem,
reorder locks in f2fs_quota_sync() as below to fix this issue:
- lock inode
- lock cp_rwsem
- lock quota_sem
Fixes: db6ec53b7e03 ("f2fs: add a rw_sem to cover quota flag changes")
Signed-off-by: Chao Yu <chao@kernel.org>
---
fs/f2fs/super.c | 84 ++++++++++++++++++++++++++++---------------------
1 file changed, 48 insertions(+), 36 deletions(-)
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 8fecd3050ccd..72eb9d70969f 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -2518,6 +2518,33 @@ static int f2fs_enable_quotas(struct super_block *sb)
return 0;
}
+static int f2fs_quota_sync_file(struct f2fs_sb_info *sbi, int type)
+{
+ struct quota_info *dqopt = sb_dqopt(sbi->sb);
+ struct address_space *mapping = dqopt->files[type]->i_mapping;
+ int ret = 0;
+
+ ret = dquot_writeback_dquots(sbi->sb, type);
+ if (ret)
+ goto out;
+
+ ret = filemap_fdatawrite(mapping);
+ if (ret)
+ goto out;
+
+ /* if we are using journalled quota */
+ if (is_journalled_quota(sbi))
+ goto out;
+
+ ret = filemap_fdatawait(mapping);
+
+ truncate_inode_pages(&dqopt->files[type]->i_data, 0);
+out:
+ if (ret)
+ set_sbi_flag(sbi, SBI_QUOTA_NEED_REPAIR);
+ return ret;
+}
+
int f2fs_quota_sync(struct super_block *sb, int type)
{
struct f2fs_sb_info *sbi = F2FS_SB(sb);
@@ -2525,57 +2552,42 @@ int f2fs_quota_sync(struct super_block *sb, int type)
int cnt;
int ret;
- /*
- * do_quotactl
- * f2fs_quota_sync
- * down_read(quota_sem)
- * dquot_writeback_dquots()
- * f2fs_dquot_commit
- * block_operation
- * down_read(quota_sem)
- */
- f2fs_lock_op(sbi);
-
- down_read(&sbi->quota_sem);
- ret = dquot_writeback_dquots(sb, type);
- if (ret)
- goto out;
-
/*
* Now when everything is written we can discard the pagecache so
* that userspace sees the changes.
*/
for (cnt = 0; cnt < MAXQUOTAS; cnt++) {
- struct address_space *mapping;
if (type != -1 && cnt != type)
continue;
- if (!sb_has_quota_active(sb, cnt))
- continue;
- mapping = dqopt->files[cnt]->i_mapping;
+ if (!sb_has_quota_active(sb, type))
+ return 0;
- ret = filemap_fdatawrite(mapping);
- if (ret)
- goto out;
+ inode_lock(dqopt->files[cnt]);
- /* if we are using journalled quota */
- if (is_journalled_quota(sbi))
- continue;
+ /*
+ * do_quotactl
+ * f2fs_quota_sync
+ * down_read(quota_sem)
+ * dquot_writeback_dquots()
+ * f2fs_dquot_commit
+ * block_operation
+ * down_read(quota_sem)
+ */
+ f2fs_lock_op(sbi);
+ down_read(&sbi->quota_sem);
- ret = filemap_fdatawait(mapping);
- if (ret)
- set_sbi_flag(F2FS_SB(sb), SBI_QUOTA_NEED_REPAIR);
+ ret = f2fs_quota_sync_file(sbi, cnt);
+
+ up_read(&sbi->quota_sem);
+ f2fs_unlock_op(sbi);
- inode_lock(dqopt->files[cnt]);
- truncate_inode_pages(&dqopt->files[cnt]->i_data, 0);
inode_unlock(dqopt->files[cnt]);
+
+ if (ret)
+ break;
}
-out:
- if (ret)
- set_sbi_flag(F2FS_SB(sb), SBI_QUOTA_NEED_REPAIR);
- up_read(&sbi->quota_sem);
- f2fs_unlock_op(sbi);
return ret;
}
--
2.22.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
end of thread, other threads:[~2022-01-05 15:25 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-04 13:05 [PATCH] f2fs: quota: fix potential deadlock Chao Yu
2022-01-04 13:05 ` [f2fs-dev] " Chao Yu
2022-01-04 13:18 ` Greg KH
2022-01-04 13:18 ` [f2fs-dev] " Greg KH
2022-01-04 15:05 ` Chao Yu
2022-01-04 15:05 ` [f2fs-dev] " Chao Yu
2022-01-04 15:17 ` Greg KH
2022-01-04 15:17 ` [f2fs-dev] " Greg KH
2022-01-04 15:48 ` Chao Yu
2022-01-04 15:48 ` [f2fs-dev] " Chao Yu
2022-01-05 15:01 ` Greg KH
2022-01-05 15:01 ` [f2fs-dev] " Greg KH
2022-01-05 15:25 ` Chao Yu
2022-01-05 15:25 ` [f2fs-dev] " Chao Yu
-- strict thread matches above, loose matches on Subject: below --
2021-09-03 2:38 Chao Yu
2021-07-19 8:46 Chao Yu
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.