2020-02-18 10:24:06

by Chao Yu

Subject: [PATCH 1/3] f2fs: avoid __GFP_NOFAIL in f2fs_bio_alloc

__f2fs_bio_alloc() won't fail due to memory pool backend, remove unneeded
__GFP_NOFAIL flag in __f2fs_bio_alloc().

Signed-off-by: Chao Yu <[email protected]>
---
fs/f2fs/data.c | 12 ++++--------
fs/f2fs/f2fs.h | 2 +-
2 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index baf12318ec64..3a4ece26928c 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -54,17 +54,13 @@ static inline struct bio *__f2fs_bio_alloc(gfp_t gfp_mask,
return bio_alloc_bioset(gfp_mask, nr_iovecs, &f2fs_bioset);
}

-struct bio *f2fs_bio_alloc(struct f2fs_sb_info *sbi, int npages, bool no_fail)
+struct bio *f2fs_bio_alloc(struct f2fs_sb_info *sbi, int npages, bool noio)
{
- struct bio *bio;
-
- if (no_fail) {
+ if (noio) {
/* No failure on bio allocation */
- bio = __f2fs_bio_alloc(GFP_NOIO, npages);
- if (!bio)
- bio = __f2fs_bio_alloc(GFP_NOIO | __GFP_NOFAIL, npages);
- return bio;
+ return __f2fs_bio_alloc(GFP_NOIO, npages);
}
+
if (time_to_inject(sbi, FAULT_ALLOC_BIO)) {
f2fs_show_injection_info(sbi, FAULT_ALLOC_BIO);
return NULL;
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 5316ac3eacdf..65f569949d42 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -3343,7 +3343,7 @@ void f2fs_destroy_checkpoint_caches(void);
*/
int __init f2fs_init_bioset(void);
void f2fs_destroy_bioset(void);
-struct bio *f2fs_bio_alloc(struct f2fs_sb_info *sbi, int npages, bool no_fail);
+struct bio *f2fs_bio_alloc(struct f2fs_sb_info *sbi, int npages, bool noio);
int f2fs_init_bio_entry_cache(void);
void f2fs_destroy_bio_entry_cache(void);
void f2fs_submit_bio(struct f2fs_sb_info *sbi,
--
2.18.0.rc1

2020-02-18 10:24:40

by Chao Yu

Subject: [PATCH 3/3] f2fs: avoid unneeded barrier in do_checkpoint()

We don't need to wait all dirty page submitting IO twice,
remove unneeded wait step.

Signed-off-by: Chao Yu <[email protected]>
---
fs/f2fs/checkpoint.c | 2 --
1 file changed, 2 deletions(-)

diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
index 751815cb4c2b..9c88fb3d255a 100644
--- a/fs/f2fs/checkpoint.c
+++ b/fs/f2fs/checkpoint.c
@@ -1384,8 +1384,6 @@ static int do_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc)

/* Flush all the NAT/SIT pages */
f2fs_sync_meta_pages(sbi, META, LONG_MAX, FS_CP_META_IO);
- /* Wait for all dirty meta pages to be submitted for IO */
- f2fs_wait_on_all_pages(sbi, F2FS_DIRTY_META);

/*
* modify checkpoint
--
2.18.0.rc1

2020-02-19 02:50:10

by Jaegeuk Kim

Subject: Re: [PATCH 1/3] f2fs: avoid __GFP_NOFAIL in f2fs_bio_alloc

On 02/18, Chao Yu wrote:
> __f2fs_bio_alloc() won't fail due to memory pool backend, remove unneeded
> __GFP_NOFAIL flag in __f2fs_bio_alloc().

It it safe for old kernels as well when thinking backports?

>
> Signed-off-by: Chao Yu <[email protected]>
> ---
> fs/f2fs/data.c | 12 ++++--------
> fs/f2fs/f2fs.h | 2 +-
> 2 files changed, 5 insertions(+), 9 deletions(-)
>
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index baf12318ec64..3a4ece26928c 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -54,17 +54,13 @@ static inline struct bio *__f2fs_bio_alloc(gfp_t gfp_mask,
> return bio_alloc_bioset(gfp_mask, nr_iovecs, &f2fs_bioset);
> }
>
> -struct bio *f2fs_bio_alloc(struct f2fs_sb_info *sbi, int npages, bool no_fail)
> +struct bio *f2fs_bio_alloc(struct f2fs_sb_info *sbi, int npages, bool noio)
> {
> - struct bio *bio;
> -
> - if (no_fail) {
> + if (noio) {
> /* No failure on bio allocation */
> - bio = __f2fs_bio_alloc(GFP_NOIO, npages);
> - if (!bio)
> - bio = __f2fs_bio_alloc(GFP_NOIO | __GFP_NOFAIL, npages);
> - return bio;
> + return __f2fs_bio_alloc(GFP_NOIO, npages);
> }
> +
> if (time_to_inject(sbi, FAULT_ALLOC_BIO)) {
> f2fs_show_injection_info(sbi, FAULT_ALLOC_BIO);
> return NULL;
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 5316ac3eacdf..65f569949d42 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -3343,7 +3343,7 @@ void f2fs_destroy_checkpoint_caches(void);
> */
> int __init f2fs_init_bioset(void);
> void f2fs_destroy_bioset(void);
> -struct bio *f2fs_bio_alloc(struct f2fs_sb_info *sbi, int npages, bool no_fail);
> +struct bio *f2fs_bio_alloc(struct f2fs_sb_info *sbi, int npages, bool noio);
> int f2fs_init_bio_entry_cache(void);
> void f2fs_destroy_bio_entry_cache(void);
> void f2fs_submit_bio(struct f2fs_sb_info *sbi,
> --
> 2.18.0.rc1

2020-02-19 02:52:29

by Jaegeuk Kim

Subject: Re: [PATCH 3/3] f2fs: avoid unneeded barrier in do_checkpoint()

On 02/18, Chao Yu wrote:
> We don't need to wait all dirty page submitting IO twice,
> remove unneeded wait step.

What happens if checkpoint and other meta writs are reordered?

>
> Signed-off-by: Chao Yu <[email protected]>
> ---
> fs/f2fs/checkpoint.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
> index 751815cb4c2b..9c88fb3d255a 100644
> --- a/fs/f2fs/checkpoint.c
> +++ b/fs/f2fs/checkpoint.c
> @@ -1384,8 +1384,6 @@ static int do_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc)
>
> /* Flush all the NAT/SIT pages */
> f2fs_sync_meta_pages(sbi, META, LONG_MAX, FS_CP_META_IO);
> - /* Wait for all dirty meta pages to be submitted for IO */
> - f2fs_wait_on_all_pages(sbi, F2FS_DIRTY_META);
>
> /*
> * modify checkpoint
> --
> 2.18.0.rc1

2020-02-19 03:13:34

by Chao Yu

Subject: Re: [PATCH 1/3] f2fs: avoid __GFP_NOFAIL in f2fs_bio_alloc

On 2020/2/19 10:49, Jaegeuk Kim wrote:
> On 02/18, Chao Yu wrote:
>> __f2fs_bio_alloc() won't fail due to memory pool backend, remove unneeded
>> __GFP_NOFAIL flag in __f2fs_bio_alloc().
>
> It it safe for old kernels as well when thinking backports?

^1da177e4c3f4 (Linus Torvalds 2005-04-16 15:20:36 -0700 134) struct bio *bio = mempool_alloc(bs->bio_pool, gfp_mask);

It looks if we have a backend mempool, we will never fail to allocate bio
for very long time, we don't need to backport to 2.6.x kernel, right?

Thanks,

>
>>
>> Signed-off-by: Chao Yu <[email protected]>
>> ---
>> fs/f2fs/data.c | 12 ++++--------
>> fs/f2fs/f2fs.h | 2 +-
>> 2 files changed, 5 insertions(+), 9 deletions(-)
>>
>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>> index baf12318ec64..3a4ece26928c 100644
>> --- a/fs/f2fs/data.c
>> +++ b/fs/f2fs/data.c
>> @@ -54,17 +54,13 @@ static inline struct bio *__f2fs_bio_alloc(gfp_t gfp_mask,
>> return bio_alloc_bioset(gfp_mask, nr_iovecs, &f2fs_bioset);
>> }
>>
>> -struct bio *f2fs_bio_alloc(struct f2fs_sb_info *sbi, int npages, bool no_fail)
>> +struct bio *f2fs_bio_alloc(struct f2fs_sb_info *sbi, int npages, bool noio)
>> {
>> - struct bio *bio;
>> -
>> - if (no_fail) {
>> + if (noio) {
>> /* No failure on bio allocation */
>> - bio = __f2fs_bio_alloc(GFP_NOIO, npages);
>> - if (!bio)
>> - bio = __f2fs_bio_alloc(GFP_NOIO | __GFP_NOFAIL, npages);
>> - return bio;
>> + return __f2fs_bio_alloc(GFP_NOIO, npages);
>> }
>> +
>> if (time_to_inject(sbi, FAULT_ALLOC_BIO)) {
>> f2fs_show_injection_info(sbi, FAULT_ALLOC_BIO);
>> return NULL;
>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>> index 5316ac3eacdf..65f569949d42 100644
>> --- a/fs/f2fs/f2fs.h
>> +++ b/fs/f2fs/f2fs.h
>> @@ -3343,7 +3343,7 @@ void f2fs_destroy_checkpoint_caches(void);
>> */
>> int __init f2fs_init_bioset(void);
>> void f2fs_destroy_bioset(void);
>> -struct bio *f2fs_bio_alloc(struct f2fs_sb_info *sbi, int npages, bool no_fail);
>> +struct bio *f2fs_bio_alloc(struct f2fs_sb_info *sbi, int npages, bool noio);
>> int f2fs_init_bio_entry_cache(void);
>> void f2fs_destroy_bio_entry_cache(void);
>> void f2fs_submit_bio(struct f2fs_sb_info *sbi,
>> --
>> 2.18.0.rc1
> .
>

2020-02-19 03:19:45

by Chao Yu

Subject: Re: [PATCH 3/3] f2fs: avoid unneeded barrier in do_checkpoint()

On 2020/2/19 10:51, Jaegeuk Kim wrote:
> On 02/18, Chao Yu wrote:
>> We don't need to wait all dirty page submitting IO twice,
>> remove unneeded wait step.
>
> What happens if checkpoint and other meta writs are reordered?

checkpoint can be done as following:

1. All meta except last cp-park of checkpoint area.
2. last cp-park of checkpoint area

So we only need to keep barrier in between step 1 and 2, we don't need
to care about the write order of meta in step 1, right?

Thanks,

>
>>
>> Signed-off-by: Chao Yu <[email protected]>
>> ---
>> fs/f2fs/checkpoint.c | 2 --
>> 1 file changed, 2 deletions(-)
>>
>> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
>> index 751815cb4c2b..9c88fb3d255a 100644
>> --- a/fs/f2fs/checkpoint.c
>> +++ b/fs/f2fs/checkpoint.c
>> @@ -1384,8 +1384,6 @@ static int do_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc)
>>
>> /* Flush all the NAT/SIT pages */
>> f2fs_sync_meta_pages(sbi, META, LONG_MAX, FS_CP_META_IO);
>> - /* Wait for all dirty meta pages to be submitted for IO */
>> - f2fs_wait_on_all_pages(sbi, F2FS_DIRTY_META);
>>
>> /*
>> * modify checkpoint
>> --
>> 2.18.0.rc1
> .
>

2020-02-24 22:00:52

by Jaegeuk Kim

Subject: Re: [PATCH 3/3] f2fs: avoid unneeded barrier in do_checkpoint()

On 02/19, Chao Yu wrote:
> On 2020/2/19 10:51, Jaegeuk Kim wrote:
> > On 02/18, Chao Yu wrote:
> >> We don't need to wait all dirty page submitting IO twice,
> >> remove unneeded wait step.
> >
> > What happens if checkpoint and other meta writs are reordered?
>
> checkpoint can be done as following:
>
> 1. All meta except last cp-park of checkpoint area.
> 2. last cp-park of checkpoint area
>
> So we only need to keep barrier in between step 1 and 2, we don't need
> to care about the write order of meta in step 1, right?

Ah, let me integrate this patch into Sahitya's patch.

f2fs: fix the panic in do_checkpoint()

>
> Thanks,
>
> >
> >>
> >> Signed-off-by: Chao Yu <[email protected]>
> >> ---
> >> fs/f2fs/checkpoint.c | 2 --
> >> 1 file changed, 2 deletions(-)
> >>
> >> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
> >> index 751815cb4c2b..9c88fb3d255a 100644
> >> --- a/fs/f2fs/checkpoint.c
> >> +++ b/fs/f2fs/checkpoint.c
> >> @@ -1384,8 +1384,6 @@ static int do_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc)
> >>
> >> /* Flush all the NAT/SIT pages */
> >> f2fs_sync_meta_pages(sbi, META, LONG_MAX, FS_CP_META_IO);
> >> - /* Wait for all dirty meta pages to be submitted for IO */
> >> - f2fs_wait_on_all_pages(sbi, F2FS_DIRTY_META);
> >>
> >> /*
> >> * modify checkpoint
> >> --
> >> 2.18.0.rc1
> > .
> >

2020-02-25 06:11:12

by Chao Yu

Subject: Re: [PATCH 3/3] f2fs: avoid unneeded barrier in do_checkpoint()

On 2020/2/25 6:00, Jaegeuk Kim wrote:
> On 02/19, Chao Yu wrote:
>> On 2020/2/19 10:51, Jaegeuk Kim wrote:
>>> On 02/18, Chao Yu wrote:
>>>> We don't need to wait all dirty page submitting IO twice,
>>>> remove unneeded wait step.
>>>
>>> What happens if checkpoint and other meta writs are reordered?
>>
>> checkpoint can be done as following:
>>
>> 1. All meta except last cp-park of checkpoint area.
>> 2. last cp-park of checkpoint area
>>
>> So we only need to keep barrier in between step 1 and 2, we don't need
>> to care about the write order of meta in step 1, right?
>
> Ah, let me integrate this patch into Sahitya's patch.>
> f2fs: fix the panic in do_checkpoint()

No problem.

Thanks,

>
>>
>> Thanks,
>>
>>>
>>>>
>>>> Signed-off-by: Chao Yu <[email protected]>
>>>> ---
>>>> fs/f2fs/checkpoint.c | 2 --
>>>> 1 file changed, 2 deletions(-)
>>>>
>>>> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
>>>> index 751815cb4c2b..9c88fb3d255a 100644
>>>> --- a/fs/f2fs/checkpoint.c
>>>> +++ b/fs/f2fs/checkpoint.c
>>>> @@ -1384,8 +1384,6 @@ static int do_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc)
>>>>
>>>> /* Flush all the NAT/SIT pages */
>>>> f2fs_sync_meta_pages(sbi, META, LONG_MAX, FS_CP_META_IO);
>>>> - /* Wait for all dirty meta pages to be submitted for IO */
>>>> - f2fs_wait_on_all_pages(sbi, F2FS_DIRTY_META);
>>>>
>>>> /*
>>>> * modify checkpoint
>>>> --
>>>> 2.18.0.rc1
>>> .
>>>
> .
>