Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1599334 > unrolled thread

[PATCH v2] f2fs: restrict write IO alignment condition

Started byChao Yu <yuchao0@huawei.com>
First post2017-03-13 14:10 +0100
Last post2017-03-14 12:40 +0100
Articles 4 — 3 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH v2] f2fs: restrict write IO alignment condition Chao Yu <yuchao0@huawei.com> - 2017-03-13 14:10 +0100
    Re: [f2fs-dev] [PATCH v2] f2fs: restrict write IO alignment condition Kinglong Mee <kinglongmee@gmail.com> - 2017-03-13 14:50 +0100
    Re: [PATCH v2] f2fs: restrict write IO alignment condition Jaegeuk Kim <jaegeuk@kernel.org> - 2017-03-13 21:10 +0100
      Re: [PATCH v2] f2fs: restrict write IO alignment condition Chao Yu <yuchao0@huawei.com> - 2017-03-14 12:40 +0100

#1599334 — [PATCH v2] f2fs: restrict write IO alignment condition

FromChao Yu <yuchao0@huawei.com>
Date2017-03-13 14:10 +0100
Subject[PATCH v2] f2fs: restrict write IO alignment condition
Message-ID<tkxTz-XV-17@gated-at.bofh.it>
We should only align start offset of bio with defined IO_SIZE for below
conditions:
a. mode=lfs mount option
b. write IOs
c. Out-place-update
d. non-meta pages

Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
 fs/f2fs/data.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 1375fef11146..53fe79c70406 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -190,6 +190,9 @@ static inline void __submit_bio(struct f2fs_sb_info *sbi,
 			current->plug && (type == DATA || type == NODE))
 			blk_finish_plug(current->plug);
 
+		if (!test_opt(sbi, LFS))
+			goto submit_io;
+
 		if (type != DATA && type != NODE)
 			goto submit_io;
 
@@ -395,11 +398,12 @@ int f2fs_submit_page_mbio(struct f2fs_io_info *fio)
 		__submit_merged_bio(io);
 alloc_new:
 	if (io->bio == NULL) {
-		if ((fio->type == DATA || fio->type == NODE) &&
+		if (test_opt(sbi, LFS) &&
+			!is_read && (fio->type == DATA || fio->type == NODE) &&
 				fio->new_blkaddr & F2FS_IO_SIZE_MASK(sbi)) {
 			err = -EAGAIN;
-			if (!is_read)
-				dec_page_count(sbi, WB_DATA_TYPE(bio_page));
+			f2fs_bug_on(sbi, fio->new_blkaddr == fio->old_blkaddr);
+			dec_page_count(sbi, WB_DATA_TYPE(bio_page));
 			goto out_fail;
 		}
 		io->bio = __bio_alloc(sbi, fio->new_blkaddr,
-- 
2.8.2.295.g3f1c1d0

[toc] | [next] | [standalone]


#1599369 — Re: [f2fs-dev] [PATCH v2] f2fs: restrict write IO alignment condition

FromKinglong Mee <kinglongmee@gmail.com>
Date2017-03-13 14:50 +0100
SubjectRe: [f2fs-dev] [PATCH v2] f2fs: restrict write IO alignment condition
Message-ID<tkywh-1es-5@gated-at.bofh.it>
In reply to#1599334
On 3/13/2017 21:02, Chao Yu wrote:
> We should only align start offset of bio with defined IO_SIZE for below
> conditions:
> a. mode=lfs mount option
> b. write IOs
> c. Out-place-update
> d. non-meta pages
> 
> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> ---
>  fs/f2fs/data.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 1375fef11146..53fe79c70406 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -190,6 +190,9 @@ static inline void __submit_bio(struct f2fs_sb_info *sbi,
>  			current->plug && (type == DATA || type == NODE))
>  			blk_finish_plug(current->plug);
>  
> +		if (!test_opt(sbi, LFS))
> +			goto submit_io;
> +
>  		if (type != DATA && type != NODE)
>  			goto submit_io;

Is the position of checking LFS here right?

I have a plan of change the code order as, 
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 2ab5ca0..698dbf3 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -186,13 +186,12 @@ static inline void __submit_bio(struct f2fs_sb_info *sbi,
        if (!is_read_io(bio_op(bio))) {
                unsigned int start;

-               if (f2fs_sb_mounted_blkzoned(sbi->sb) &&
-                       current->plug && (type == DATA || type == NODE))
-                       blk_finish_plug(current->plug);
-
                if (type != DATA && type != NODE)
                        goto submit_io;

+               if (f2fs_sb_mounted_blkzoned(sbi->sb) && current->plug)
+                       blk_finish_plug(current->plug);
+
                start = bio->bi_iter.bi_size >> F2FS_BLKSIZE_BITS;
                start %= F2FS_IO_SIZE(sbi);

thanks,
Kinglong Mee

>  
> @@ -395,11 +398,12 @@ int f2fs_submit_page_mbio(struct f2fs_io_info *fio)
>  		__submit_merged_bio(io);
>  alloc_new:
>  	if (io->bio == NULL) {
> -		if ((fio->type == DATA || fio->type == NODE) &&
> +		if (test_opt(sbi, LFS) &&
> +			!is_read && (fio->type == DATA || fio->type == NODE) &&
>  				fio->new_blkaddr & F2FS_IO_SIZE_MASK(sbi)) {
>  			err = -EAGAIN;
> -			if (!is_read)
> -				dec_page_count(sbi, WB_DATA_TYPE(bio_page));
> +			f2fs_bug_on(sbi, fio->new_blkaddr == fio->old_blkaddr);
> +			dec_page_count(sbi, WB_DATA_TYPE(bio_page));
>  			goto out_fail;
>  		}
>  		io->bio = __bio_alloc(sbi, fio->new_blkaddr,
> 

[toc] | [prev] | [next] | [standalone]


#1599763

FromJaegeuk Kim <jaegeuk@kernel.org>
Date2017-03-13 21:10 +0100
Message-ID<tkEs3-5H9-51@gated-at.bofh.it>
In reply to#1599334
On 03/13, Chao Yu wrote:
> We should only align start offset of bio with defined IO_SIZE for below
> conditions:
> a. mode=lfs mount option
> b. write IOs
> c. Out-place-update
> d. non-meta pages
> 
> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> ---
>  fs/f2fs/data.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 1375fef11146..53fe79c70406 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -190,6 +190,9 @@ static inline void __submit_bio(struct f2fs_sb_info *sbi,
>  			current->plug && (type == DATA || type == NODE))
>  			blk_finish_plug(current->plug);
>  
> +		if (!test_opt(sbi, LFS))
> +			goto submit_io;

The parse_options in fill_super checks this.

> +
>  		if (type != DATA && type != NODE)
>  			goto submit_io;
>  
> @@ -395,11 +398,12 @@ int f2fs_submit_page_mbio(struct f2fs_io_info *fio)
>  		__submit_merged_bio(io);
>  alloc_new:
>  	if (io->bio == NULL) {
> -		if ((fio->type == DATA || fio->type == NODE) &&
> +		if (test_opt(sbi, LFS) &&
> +			!is_read && (fio->type == DATA || fio->type == NODE) &&

The missing case is !is_read, here?
But, actually, DATA and NODE don't issue any read commands at all.

Thanks,

>  				fio->new_blkaddr & F2FS_IO_SIZE_MASK(sbi)) {
>  			err = -EAGAIN;
> -			if (!is_read)
> -				dec_page_count(sbi, WB_DATA_TYPE(bio_page));
> +			f2fs_bug_on(sbi, fio->new_blkaddr == fio->old_blkaddr);
> +			dec_page_count(sbi, WB_DATA_TYPE(bio_page));
>  			goto out_fail;
>  		}
>  		io->bio = __bio_alloc(sbi, fio->new_blkaddr,
> -- 
> 2.8.2.295.g3f1c1d0

[toc] | [prev] | [next] | [standalone]


#1600247

FromChao Yu <yuchao0@huawei.com>
Date2017-03-14 12:40 +0100
Message-ID<tkSY2-7U6-23@gated-at.bofh.it>
In reply to#1599763
On 2017/3/14 4:05, Jaegeuk Kim wrote:
> On 03/13, Chao Yu wrote:
>> We should only align start offset of bio with defined IO_SIZE for below
>> conditions:
>> a. mode=lfs mount option
>> b. write IOs
>> c. Out-place-update
>> d. non-meta pages
>>
>> Signed-off-by: Chao Yu <yuchao0@huawei.com>
>> ---
>>  fs/f2fs/data.c | 10 +++++++---
>>  1 file changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>> index 1375fef11146..53fe79c70406 100644
>> --- a/fs/f2fs/data.c
>> +++ b/fs/f2fs/data.c
>> @@ -190,6 +190,9 @@ static inline void __submit_bio(struct f2fs_sb_info *sbi,
>>  			current->plug && (type == DATA || type == NODE))
>>  			blk_finish_plug(current->plug);
>>  
>> +		if (!test_opt(sbi, LFS))
>> +			goto submit_io;
> 
> The parse_options in fill_super checks this.

You're right, I missed that one...

> 
>> +
>>  		if (type != DATA && type != NODE)
>>  			goto submit_io;
>>  
>> @@ -395,11 +398,12 @@ int f2fs_submit_page_mbio(struct f2fs_io_info *fio)
>>  		__submit_merged_bio(io);
>>  alloc_new:
>>  	if (io->bio == NULL) {
>> -		if ((fio->type == DATA || fio->type == NODE) &&
>> +		if (test_opt(sbi, LFS) &&
>> +			!is_read && (fio->type == DATA || fio->type == NODE) &&
> 
> The missing case is !is_read, here?
> But, actually, DATA and NODE don't issue any read commands at all.

Correct, let me add BUG_ON here. :)

Thanks,

> 
> Thanks,
> 
>>  				fio->new_blkaddr & F2FS_IO_SIZE_MASK(sbi)) {
>>  			err = -EAGAIN;
>> -			if (!is_read)
>> -				dec_page_count(sbi, WB_DATA_TYPE(bio_page));
>> +			f2fs_bug_on(sbi, fio->new_blkaddr == fio->old_blkaddr);
>> +			dec_page_count(sbi, WB_DATA_TYPE(bio_page));
>>  			goto out_fail;
>>  		}
>>  		io->bio = __bio_alloc(sbi, fio->new_blkaddr,
>> -- 
>> 2.8.2.295.g3f1c1d0
> 
> .
> 

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web