public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] f2fs: restrict write IO alignment condition
@ 2017-03-13 13:02 Chao Yu
  2017-03-13 13:45 ` [f2fs-dev] " Kinglong Mee
  2017-03-13 20:05 ` Jaegeuk Kim
  0 siblings, 2 replies; 4+ messages in thread
From: Chao Yu @ 2017-03-13 13:02 UTC (permalink / raw)
  To: jaegeuk; +Cc: linux-f2fs-devel, linux-kernel, chao, Chao Yu

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

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [f2fs-dev] [PATCH v2] f2fs: restrict write IO alignment condition
  2017-03-13 13:02 [PATCH v2] f2fs: restrict write IO alignment condition Chao Yu
@ 2017-03-13 13:45 ` Kinglong Mee
  2017-03-13 20:05 ` Jaegeuk Kim
  1 sibling, 0 replies; 4+ messages in thread
From: Kinglong Mee @ 2017-03-13 13:45 UTC (permalink / raw)
  To: Chao Yu, jaegeuk; +Cc: chao, linux-kernel, linux-f2fs-devel, Kinglong Mee

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,
> 

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] f2fs: restrict write IO alignment condition
  2017-03-13 13:02 [PATCH v2] f2fs: restrict write IO alignment condition Chao Yu
  2017-03-13 13:45 ` [f2fs-dev] " Kinglong Mee
@ 2017-03-13 20:05 ` Jaegeuk Kim
  2017-03-14 11:32   ` Chao Yu
  1 sibling, 1 reply; 4+ messages in thread
From: Jaegeuk Kim @ 2017-03-13 20:05 UTC (permalink / raw)
  To: Chao Yu; +Cc: linux-f2fs-devel, linux-kernel, chao

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] f2fs: restrict write IO alignment condition
  2017-03-13 20:05 ` Jaegeuk Kim
@ 2017-03-14 11:32   ` Chao Yu
  0 siblings, 0 replies; 4+ messages in thread
From: Chao Yu @ 2017-03-14 11:32 UTC (permalink / raw)
  To: Jaegeuk Kim; +Cc: linux-f2fs-devel, linux-kernel, chao

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
> 
> .
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2017-03-14 11:32 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-03-13 13:02 [PATCH v2] f2fs: restrict write IO alignment condition Chao Yu
2017-03-13 13:45 ` [f2fs-dev] " Kinglong Mee
2017-03-13 20:05 ` Jaegeuk Kim
2017-03-14 11:32   ` Chao Yu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox