* [PATCH 1/3] f2fs: clean up destroy_discard_cmd_control
@ 2017-03-27 10:14 Chao Yu
2017-03-27 10:14 ` [PATCH 2/3] f2fs: shrink blk plug region Chao Yu
2017-03-27 10:14 ` [PATCH 3/3] f2fs: prevent waiter encountering incorrect discard states Chao Yu
0 siblings, 2 replies; 10+ messages in thread
From: Chao Yu @ 2017-03-27 10:14 UTC (permalink / raw)
To: jaegeuk; +Cc: linux-f2fs-devel, linux-kernel, chao, Chao Yu
Remove unneeded parameter and simply change flow in
destroy_discard_cmd_control.
Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
fs/f2fs/segment.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index ff53423e35fb..c1a03b0857f9 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -1181,20 +1181,22 @@ static int create_discard_cmd_control(struct f2fs_sb_info *sbi)
return err;
}
-static void destroy_discard_cmd_control(struct f2fs_sb_info *sbi, bool free)
+static void destroy_discard_cmd_control(struct f2fs_sb_info *sbi)
{
struct discard_cmd_control *dcc = SM_I(sbi)->dcc_info;
- if (dcc && dcc->f2fs_issue_discard) {
+ if (!dcc)
+ return;
+
+ if (dcc->f2fs_issue_discard) {
struct task_struct *discard_thread = dcc->f2fs_issue_discard;
dcc->f2fs_issue_discard = NULL;
kthread_stop(discard_thread);
}
- if (free) {
- kfree(dcc);
- SM_I(sbi)->dcc_info = NULL;
- }
+
+ kfree(dcc);
+ SM_I(sbi)->dcc_info = NULL;
}
static bool __mark_sit_entry_dirty(struct f2fs_sb_info *sbi, unsigned int segno)
@@ -3086,7 +3088,7 @@ void destroy_segment_manager(struct f2fs_sb_info *sbi)
if (!sm_info)
return;
destroy_flush_cmd_control(sbi, true);
- destroy_discard_cmd_control(sbi, true);
+ destroy_discard_cmd_control(sbi);
destroy_dirty_segmap(sbi);
destroy_curseg(sbi);
destroy_free_segmap(sbi);
--
2.8.2.295.g3f1c1d0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 2/3] f2fs: shrink blk plug region
2017-03-27 10:14 [PATCH 1/3] f2fs: clean up destroy_discard_cmd_control Chao Yu
@ 2017-03-27 10:14 ` Chao Yu
2017-04-12 8:33 ` Chao Yu
2017-03-27 10:14 ` [PATCH 3/3] f2fs: prevent waiter encountering incorrect discard states Chao Yu
1 sibling, 1 reply; 10+ messages in thread
From: Chao Yu @ 2017-03-27 10:14 UTC (permalink / raw)
To: jaegeuk; +Cc: linux-f2fs-devel, linux-kernel, chao, Chao Yu
Don't use blk plug covering area where there won't be any IOs being issued.
Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
fs/f2fs/segment.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index c1a03b0857f9..57a81f9c8c14 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -848,9 +848,8 @@ static int issue_discard_thread(void *data)
if (kthread_should_stop())
return 0;
- blk_start_plug(&plug);
-
mutex_lock(&dcc->cmd_lock);
+ blk_start_plug(&plug);
list_for_each_entry_safe(dc, tmp, pend_list, list) {
f2fs_bug_on(sbi, dc->state != D_PREP);
@@ -860,6 +859,7 @@ static int issue_discard_thread(void *data)
if (iter++ > DISCARD_ISSUE_RATE)
break;
}
+ blk_finish_plug(&plug);
list_for_each_entry_safe(dc, tmp, wait_list, list) {
if (dc->state == D_DONE)
@@ -867,8 +867,6 @@ static int issue_discard_thread(void *data)
}
mutex_unlock(&dcc->cmd_lock);
- blk_finish_plug(&plug);
-
iter = 0;
congestion_wait(BLK_RW_SYNC, HZ/50);
--
2.8.2.295.g3f1c1d0
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 2/3] f2fs: shrink blk plug region
2017-03-27 10:14 ` [PATCH 2/3] f2fs: shrink blk plug region Chao Yu
@ 2017-04-12 8:33 ` Chao Yu
2017-04-12 19:57 ` Jaegeuk Kim
0 siblings, 1 reply; 10+ messages in thread
From: Chao Yu @ 2017-04-12 8:33 UTC (permalink / raw)
To: jaegeuk; +Cc: linux-f2fs-devel, linux-kernel, chao
Hi Jaegeuk,
Just reminding, miss merging this patch? :)
Thanks,
On 2017/3/27 18:14, Chao Yu wrote:
> Don't use blk plug covering area where there won't be any IOs being issued.
>
> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> ---
> fs/f2fs/segment.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index c1a03b0857f9..57a81f9c8c14 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -848,9 +848,8 @@ static int issue_discard_thread(void *data)
> if (kthread_should_stop())
> return 0;
>
> - blk_start_plug(&plug);
> -
> mutex_lock(&dcc->cmd_lock);
> + blk_start_plug(&plug);
> list_for_each_entry_safe(dc, tmp, pend_list, list) {
> f2fs_bug_on(sbi, dc->state != D_PREP);
>
> @@ -860,6 +859,7 @@ static int issue_discard_thread(void *data)
> if (iter++ > DISCARD_ISSUE_RATE)
> break;
> }
> + blk_finish_plug(&plug);
>
> list_for_each_entry_safe(dc, tmp, wait_list, list) {
> if (dc->state == D_DONE)
> @@ -867,8 +867,6 @@ static int issue_discard_thread(void *data)
> }
> mutex_unlock(&dcc->cmd_lock);
>
> - blk_finish_plug(&plug);
> -
> iter = 0;
> congestion_wait(BLK_RW_SYNC, HZ/50);
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 2/3] f2fs: shrink blk plug region
2017-04-12 8:33 ` Chao Yu
@ 2017-04-12 19:57 ` Jaegeuk Kim
0 siblings, 0 replies; 10+ messages in thread
From: Jaegeuk Kim @ 2017-04-12 19:57 UTC (permalink / raw)
To: Chao Yu; +Cc: linux-f2fs-devel, linux-kernel, chao
On 04/12, Chao Yu wrote:
> Hi Jaegeuk,
>
> Just reminding, miss merging this patch? :)
Yup, merged. ;)
>
> Thanks,
>
> On 2017/3/27 18:14, Chao Yu wrote:
> > Don't use blk plug covering area where there won't be any IOs being issued.
> >
> > Signed-off-by: Chao Yu <yuchao0@huawei.com>
> > ---
> > fs/f2fs/segment.c | 6 ++----
> > 1 file changed, 2 insertions(+), 4 deletions(-)
> >
> > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> > index c1a03b0857f9..57a81f9c8c14 100644
> > --- a/fs/f2fs/segment.c
> > +++ b/fs/f2fs/segment.c
> > @@ -848,9 +848,8 @@ static int issue_discard_thread(void *data)
> > if (kthread_should_stop())
> > return 0;
> >
> > - blk_start_plug(&plug);
> > -
> > mutex_lock(&dcc->cmd_lock);
> > + blk_start_plug(&plug);
> > list_for_each_entry_safe(dc, tmp, pend_list, list) {
> > f2fs_bug_on(sbi, dc->state != D_PREP);
> >
> > @@ -860,6 +859,7 @@ static int issue_discard_thread(void *data)
> > if (iter++ > DISCARD_ISSUE_RATE)
> > break;
> > }
> > + blk_finish_plug(&plug);
> >
> > list_for_each_entry_safe(dc, tmp, wait_list, list) {
> > if (dc->state == D_DONE)
> > @@ -867,8 +867,6 @@ static int issue_discard_thread(void *data)
> > }
> > mutex_unlock(&dcc->cmd_lock);
> >
> > - blk_finish_plug(&plug);
> > -
> > iter = 0;
> > congestion_wait(BLK_RW_SYNC, HZ/50);
> >
> >
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 3/3] f2fs: prevent waiter encountering incorrect discard states
2017-03-27 10:14 [PATCH 1/3] f2fs: clean up destroy_discard_cmd_control Chao Yu
2017-03-27 10:14 ` [PATCH 2/3] f2fs: shrink blk plug region Chao Yu
@ 2017-03-27 10:14 ` Chao Yu
2017-03-27 23:56 ` Jaegeuk Kim
1 sibling, 1 reply; 10+ messages in thread
From: Chao Yu @ 2017-03-27 10:14 UTC (permalink / raw)
To: jaegeuk; +Cc: linux-f2fs-devel, linux-kernel, chao, Chao Yu
In f2fs_submit_discard_endio, we will wake up waiter before setting
discard command states, so waiter may use incorrect states. Change
the order between complete() and states setting to fix this issue.
Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
fs/f2fs/segment.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 57a81f9c8c14..9f9542c9fe47 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -717,9 +717,9 @@ static void f2fs_submit_discard_endio(struct bio *bio)
{
struct discard_cmd *dc = (struct discard_cmd *)bio->bi_private;
- complete(&dc->wait);
dc->error = bio->bi_error;
dc->state = D_DONE;
+ complete(&dc->wait);
bio_put(bio);
}
--
2.8.2.295.g3f1c1d0
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 3/3] f2fs: prevent waiter encountering incorrect discard states
2017-03-27 10:14 ` [PATCH 3/3] f2fs: prevent waiter encountering incorrect discard states Chao Yu
@ 2017-03-27 23:56 ` Jaegeuk Kim
2017-03-28 1:17 ` Chao Yu
0 siblings, 1 reply; 10+ messages in thread
From: Jaegeuk Kim @ 2017-03-27 23:56 UTC (permalink / raw)
To: Chao Yu; +Cc: linux-f2fs-devel, linux-kernel, chao
On 03/27, Chao Yu wrote:
> In f2fs_submit_discard_endio, we will wake up waiter before setting
> discard command states, so waiter may use incorrect states. Change
> the order between complete() and states setting to fix this issue.
>
> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> ---
> fs/f2fs/segment.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 57a81f9c8c14..9f9542c9fe47 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -717,9 +717,9 @@ static void f2fs_submit_discard_endio(struct bio *bio)
> {
> struct discard_cmd *dc = (struct discard_cmd *)bio->bi_private;
>
> - complete(&dc->wait);
> dc->error = bio->bi_error;
> dc->state = D_DONE;
> + complete(&dc->wait);
If we set D_DONE first, the object can be released by __remove_discard_cmd()?
Thanks,
> bio_put(bio);
> }
>
> --
> 2.8.2.295.g3f1c1d0
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 3/3] f2fs: prevent waiter encountering incorrect discard states
2017-03-27 23:56 ` Jaegeuk Kim
@ 2017-03-28 1:17 ` Chao Yu
2017-04-01 6:54 ` Chao Yu
0 siblings, 1 reply; 10+ messages in thread
From: Chao Yu @ 2017-03-28 1:17 UTC (permalink / raw)
To: Jaegeuk Kim; +Cc: linux-f2fs-devel, linux-kernel, chao
On 2017/3/28 7:56, Jaegeuk Kim wrote:
> On 03/27, Chao Yu wrote:
>> In f2fs_submit_discard_endio, we will wake up waiter before setting
>> discard command states, so waiter may use incorrect states. Change
>> the order between complete() and states setting to fix this issue.
>>
>> Signed-off-by: Chao Yu <yuchao0@huawei.com>
>> ---
>> fs/f2fs/segment.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
>> index 57a81f9c8c14..9f9542c9fe47 100644
>> --- a/fs/f2fs/segment.c
>> +++ b/fs/f2fs/segment.c
>> @@ -717,9 +717,9 @@ static void f2fs_submit_discard_endio(struct bio *bio)
>> {
>> struct discard_cmd *dc = (struct discard_cmd *)bio->bi_private;
>>
>> - complete(&dc->wait);
>> dc->error = bio->bi_error;
>> dc->state = D_DONE;
>> + complete(&dc->wait);
>
> If we set D_DONE first, the object can be released by __remove_discard_cmd()?
Yes, I think so.
Thanks,
>
> Thanks,
>
>> bio_put(bio);
>> }
>>
>> --
>> 2.8.2.295.g3f1c1d0
>
> .
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 3/3] f2fs: prevent waiter encountering incorrect discard states
2017-03-28 1:17 ` Chao Yu
@ 2017-04-01 6:54 ` Chao Yu
2017-04-03 17:40 ` Jaegeuk Kim
0 siblings, 1 reply; 10+ messages in thread
From: Chao Yu @ 2017-04-01 6:54 UTC (permalink / raw)
To: Jaegeuk Kim; +Cc: linux-f2fs-devel, linux-kernel, chao
Ping,
Any problem here?
Thanks,
On 2017/3/28 9:17, Chao Yu wrote:
> On 2017/3/28 7:56, Jaegeuk Kim wrote:
>> On 03/27, Chao Yu wrote:
>>> In f2fs_submit_discard_endio, we will wake up waiter before setting
>>> discard command states, so waiter may use incorrect states. Change
>>> the order between complete() and states setting to fix this issue.
>>>
>>> Signed-off-by: Chao Yu <yuchao0@huawei.com>
>>> ---
>>> fs/f2fs/segment.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
>>> index 57a81f9c8c14..9f9542c9fe47 100644
>>> --- a/fs/f2fs/segment.c
>>> +++ b/fs/f2fs/segment.c
>>> @@ -717,9 +717,9 @@ static void f2fs_submit_discard_endio(struct bio *bio)
>>> {
>>> struct discard_cmd *dc = (struct discard_cmd *)bio->bi_private;
>>>
>>> - complete(&dc->wait);
>>> dc->error = bio->bi_error;
>>> dc->state = D_DONE;
>>> + complete(&dc->wait);
>>
>> If we set D_DONE first, the object can be released by __remove_discard_cmd()?
>
> Yes, I think so.
>
> Thanks,
>
>>
>> Thanks,
>>
>>> bio_put(bio);
>>> }
>>>
>>> --
>>> 2.8.2.295.g3f1c1d0
>>
>> .
>>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 3/3] f2fs: prevent waiter encountering incorrect discard states
2017-04-01 6:54 ` Chao Yu
@ 2017-04-03 17:40 ` Jaegeuk Kim
2017-04-05 10:25 ` Chao Yu
0 siblings, 1 reply; 10+ messages in thread
From: Jaegeuk Kim @ 2017-04-03 17:40 UTC (permalink / raw)
To: Chao Yu; +Cc: linux-f2fs-devel, linux-kernel, chao
On 04/01, Chao Yu wrote:
> Ping,
>
> Any problem here?
>
> Thanks,
>
> On 2017/3/28 9:17, Chao Yu wrote:
> > On 2017/3/28 7:56, Jaegeuk Kim wrote:
> >> On 03/27, Chao Yu wrote:
> >>> In f2fs_submit_discard_endio, we will wake up waiter before setting
> >>> discard command states, so waiter may use incorrect states. Change
> >>> the order between complete() and states setting to fix this issue.
> >>>
> >>> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> >>> ---
> >>> fs/f2fs/segment.c | 2 +-
> >>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> >>> index 57a81f9c8c14..9f9542c9fe47 100644
> >>> --- a/fs/f2fs/segment.c
> >>> +++ b/fs/f2fs/segment.c
> >>> @@ -717,9 +717,9 @@ static void f2fs_submit_discard_endio(struct bio *bio)
> >>> {
> >>> struct discard_cmd *dc = (struct discard_cmd *)bio->bi_private;
> >>>
> >>> - complete(&dc->wait);
> >>> dc->error = bio->bi_error;
> >>> dc->state = D_DONE;
> >>> + complete(&dc->wait);
> >>
> >> If we set D_DONE first, the object can be released by __remove_discard_cmd()?
What I mean was about use-after-free.
Thanks,
> >
> > Yes, I think so.
> >
> > Thanks,
> >
> >>
> >> Thanks,
> >>
> >>> bio_put(bio);
> >>> }
> >>>
> >>> --
> >>> 2.8.2.295.g3f1c1d0
> >>
> >> .
> >>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 3/3] f2fs: prevent waiter encountering incorrect discard states
2017-04-03 17:40 ` Jaegeuk Kim
@ 2017-04-05 10:25 ` Chao Yu
0 siblings, 0 replies; 10+ messages in thread
From: Chao Yu @ 2017-04-05 10:25 UTC (permalink / raw)
To: Jaegeuk Kim; +Cc: linux-f2fs-devel, linux-kernel, chao
On 2017/4/4 1:40, Jaegeuk Kim wrote:
> On 04/01, Chao Yu wrote:
>> Ping,
>>
>> Any problem here?
>>
>> Thanks,
>>
>> On 2017/3/28 9:17, Chao Yu wrote:
>>> On 2017/3/28 7:56, Jaegeuk Kim wrote:
>>>> On 03/27, Chao Yu wrote:
>>>>> In f2fs_submit_discard_endio, we will wake up waiter before setting
>>>>> discard command states, so waiter may use incorrect states. Change
>>>>> the order between complete() and states setting to fix this issue.
>>>>>
>>>>> Signed-off-by: Chao Yu <yuchao0@huawei.com>
>>>>> ---
>>>>> fs/f2fs/segment.c | 2 +-
>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
>>>>> index 57a81f9c8c14..9f9542c9fe47 100644
>>>>> --- a/fs/f2fs/segment.c
>>>>> +++ b/fs/f2fs/segment.c
>>>>> @@ -717,9 +717,9 @@ static void f2fs_submit_discard_endio(struct bio *bio)
>>>>> {
>>>>> struct discard_cmd *dc = (struct discard_cmd *)bio->bi_private;
>>>>>
>>>>> - complete(&dc->wait);
>>>>> dc->error = bio->bi_error;
>>>>> dc->state = D_DONE;
>>>>> + complete(&dc->wait);
>>>>
>>>> If we set D_DONE first, the object can be released by __remove_discard_cmd()?
>
> What I mean was about use-after-free.
I updated the patch, could you help to review it?
Thanks,
>
> Thanks,
>
>>>
>>> Yes, I think so.
>>>
>>> Thanks,
>>>
>>>>
>>>> Thanks,
>>>>
>>>>> bio_put(bio);
>>>>> }
>>>>>
>>>>> --
>>>>> 2.8.2.295.g3f1c1d0
>>>>
>>>> .
>>>>
>
> .
>
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2017-04-12 19:57 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-27 10:14 [PATCH 1/3] f2fs: clean up destroy_discard_cmd_control Chao Yu
2017-03-27 10:14 ` [PATCH 2/3] f2fs: shrink blk plug region Chao Yu
2017-04-12 8:33 ` Chao Yu
2017-04-12 19:57 ` Jaegeuk Kim
2017-03-27 10:14 ` [PATCH 3/3] f2fs: prevent waiter encountering incorrect discard states Chao Yu
2017-03-27 23:56 ` Jaegeuk Kim
2017-03-28 1:17 ` Chao Yu
2017-04-01 6:54 ` Chao Yu
2017-04-03 17:40 ` Jaegeuk Kim
2017-04-05 10:25 ` Chao Yu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).