All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2 v2] f2fs: fix a missing discard prefree segments
@ 2016-12-20  3:11 Yunlei He
  2016-12-20  3:11 ` [PATCH 2/2 v2] f2fs: add a case of no need to read a page in write begin Yunlei He
  2016-12-21  2:02 ` [PATCH 1/2 v2] f2fs: fix a missing discard prefree segments Chao Yu
  0 siblings, 2 replies; 10+ messages in thread
From: Yunlei He @ 2016-12-20  3:11 UTC (permalink / raw)
  To: linux-f2fs-devel, jaegeuk, yuchao0; +Cc: heyunlei

If userspace issue a fstrim with a range not involve prefree segments,
it will reuse these segments without discard. This patch fix it.

Signed-off-by: Yunlei He <heyunlei@huawei.com>
---
 fs/f2fs/segment.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 0738f48..a40a34b 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -916,7 +916,11 @@ void clear_prefree_segments(struct f2fs_sb_info *sbi, struct cp_control *cpc)
 
 		dirty_i->nr_dirty[PRE] -= end - start;
 
-		if (force || !test_opt(sbi, DISCARD))
+		if (!test_opt(sbi, DISCARD))
+			continue;
+
+		if (force && start * sbi->blocks_per_seg >= cpc->trim_start &&
+						end * sbi->blocks_per_seg <= cpc->trim_end)
 			continue;
 
 		if (!test_opt(sbi, LFS) || sbi->segs_per_sec == 1) {
-- 
2.10.1


------------------------------------------------------------------------------
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/intel

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

* [PATCH 2/2 v2] f2fs: add a case of no need to read a page in write begin
  2016-12-20  3:11 [PATCH 1/2 v2] f2fs: fix a missing discard prefree segments Yunlei He
@ 2016-12-20  3:11 ` Yunlei He
  2016-12-20 18:03   ` Jaegeuk Kim
  2016-12-21  2:02 ` [PATCH 1/2 v2] f2fs: fix a missing discard prefree segments Chao Yu
  1 sibling, 1 reply; 10+ messages in thread
From: Yunlei He @ 2016-12-20  3:11 UTC (permalink / raw)
  To: linux-f2fs-devel, jaegeuk, yuchao0; +Cc: heyunlei

If the range we write cover the whole valid data in the last page,
we do not need to read it.

Signed-off-by: Yunlei He <heyunlei@huawei.com>
---
 fs/f2fs/data.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 9ac2625..303873f 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1715,6 +1715,9 @@ static int f2fs_write_begin(struct file *file, struct address_space *mapping,
 	if (len == PAGE_SIZE || PageUptodate(page))
 		return 0;
 
+	if (!(pos & (PAGE_SIZE - 1)) && (pos + len) >= i_size_read(inode))
+		return 0;
+
 	if (blkaddr == NEW_ADDR) {
 		zero_user_segment(page, 0, PAGE_SIZE);
 		SetPageUptodate(page);
@@ -1768,7 +1771,7 @@ static int f2fs_write_end(struct file *file,
 	 * let generic_perform_write() try to copy data again through copied=0.
 	 */
 	if (!PageUptodate(page)) {
-		if (unlikely(copied != PAGE_SIZE))
+		if (unlikely(copied != len))
 			copied = 0;
 		else
 			SetPageUptodate(page);
-- 
2.10.1


------------------------------------------------------------------------------
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/intel

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

* Re: [PATCH 2/2 v2] f2fs: add a case of no need to read a page in write begin
  2016-12-20  3:11 ` [PATCH 2/2 v2] f2fs: add a case of no need to read a page in write begin Yunlei He
@ 2016-12-20 18:03   ` Jaegeuk Kim
  2016-12-21  1:28     ` Chao Yu
  0 siblings, 1 reply; 10+ messages in thread
From: Jaegeuk Kim @ 2016-12-20 18:03 UTC (permalink / raw)
  To: Yunlei He; +Cc: heyunlei, linux-f2fs-devel

On 12/20, Yunlei He wrote:
> If the range we write cover the whole valid data in the last page,
> we do not need to read it.
> 
> Signed-off-by: Yunlei He <heyunlei@huawei.com>
> ---
>  fs/f2fs/data.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 9ac2625..303873f 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -1715,6 +1715,9 @@ static int f2fs_write_begin(struct file *file, struct address_space *mapping,
>  	if (len == PAGE_SIZE || PageUptodate(page))
>  		return 0;
>  
> +	if (!(pos & (PAGE_SIZE - 1)) && (pos + len) >= i_size_read(inode))

I added here:
		zero_user_segment(page, 0, PAGE_SIZE);

Otherwise, xfstests/f2fs/001 gives a failure.

Thanks,

> +		return 0;
> +
>  	if (blkaddr == NEW_ADDR) {
>  		zero_user_segment(page, 0, PAGE_SIZE);
>  		SetPageUptodate(page);
> @@ -1768,7 +1771,7 @@ static int f2fs_write_end(struct file *file,
>  	 * let generic_perform_write() try to copy data again through copied=0.
>  	 */
>  	if (!PageUptodate(page)) {
> -		if (unlikely(copied != PAGE_SIZE))
> +		if (unlikely(copied != len))
>  			copied = 0;
>  		else
>  			SetPageUptodate(page);
> -- 
> 2.10.1
> 
> 
> ------------------------------------------------------------------------------
> Developer Access Program for Intel Xeon Phi Processors
> Access to Intel Xeon Phi processor-based developer platforms.
> With one year of Intel Parallel Studio XE.
> Training and support from Colfax.
> Order your platform today.http://sdm.link/intel
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

------------------------------------------------------------------------------
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/intel

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

* Re: [PATCH 2/2 v2] f2fs: add a case of no need to read a page in write begin
  2016-12-20 18:03   ` Jaegeuk Kim
@ 2016-12-21  1:28     ` Chao Yu
  2016-12-21 17:58       ` Jaegeuk Kim
  0 siblings, 1 reply; 10+ messages in thread
From: Chao Yu @ 2016-12-21  1:28 UTC (permalink / raw)
  To: Jaegeuk Kim, Yunlei He; +Cc: heyunlei, linux-f2fs-devel

On 2016/12/21 2:03, Jaegeuk Kim wrote:
> On 12/20, Yunlei He wrote:
>> If the range we write cover the whole valid data in the last page,
>> we do not need to read it.
>>
>> Signed-off-by: Yunlei He <heyunlei@huawei.com>
>> ---
>>  fs/f2fs/data.c | 5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>> index 9ac2625..303873f 100644
>> --- a/fs/f2fs/data.c
>> +++ b/fs/f2fs/data.c
>> @@ -1715,6 +1715,9 @@ static int f2fs_write_begin(struct file *file, struct address_space *mapping,
>>  	if (len == PAGE_SIZE || PageUptodate(page))
>>  		return 0;
>>  
>> +	if (!(pos & (PAGE_SIZE - 1)) && (pos + len) >= i_size_read(inode))
> 
> I added here:
> 		zero_user_segment(page, 0, PAGE_SIZE);

Only need to zeroout in range of [len, page_size] in the page.

Thanks,

> 
> Otherwise, xfstests/f2fs/001 gives a failure.
> 
> Thanks,
> 
>> +		return 0;
>> +
>>  	if (blkaddr == NEW_ADDR) {
>>  		zero_user_segment(page, 0, PAGE_SIZE);
>>  		SetPageUptodate(page);
>> @@ -1768,7 +1771,7 @@ static int f2fs_write_end(struct file *file,
>>  	 * let generic_perform_write() try to copy data again through copied=0.
>>  	 */
>>  	if (!PageUptodate(page)) {
>> -		if (unlikely(copied != PAGE_SIZE))
>> +		if (unlikely(copied != len))
>>  			copied = 0;
>>  		else
>>  			SetPageUptodate(page);
>> -- 
>> 2.10.1
>>
>>
>> ------------------------------------------------------------------------------
>> Developer Access Program for Intel Xeon Phi Processors
>> Access to Intel Xeon Phi processor-based developer platforms.
>> With one year of Intel Parallel Studio XE.
>> Training and support from Colfax.
>> Order your platform today.http://sdm.link/intel
>> _______________________________________________
>> Linux-f2fs-devel mailing list
>> Linux-f2fs-devel@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> 
> .
> 


------------------------------------------------------------------------------
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/intel

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

* Re: [PATCH 1/2 v2] f2fs: fix a missing discard prefree segments
  2016-12-20  3:11 [PATCH 1/2 v2] f2fs: fix a missing discard prefree segments Yunlei He
  2016-12-20  3:11 ` [PATCH 2/2 v2] f2fs: add a case of no need to read a page in write begin Yunlei He
@ 2016-12-21  2:02 ` Chao Yu
  2016-12-21  2:27   ` Chao Yu
  1 sibling, 1 reply; 10+ messages in thread
From: Chao Yu @ 2016-12-21  2:02 UTC (permalink / raw)
  To: Yunlei He, linux-f2fs-devel, jaegeuk; +Cc: heyunlei

Hi Jaegeuk, Yunlei,

On 2016/12/20 11:11, Yunlei He wrote:
> If userspace issue a fstrim with a range not involve prefree segments,
> it will reuse these segments without discard. This patch fix it.

>From v1 patch, I guess originally Yunlei wants to skip clearing prefree segments
which is not be included in fstrim range, now v2 patch doesn't follow the
original intention.

So I guess below modification is enough:

-		if (force || !test_opt(sbi, DISCARD))
+		if (!test_opt(sbi, DISCARD))

Only we should consider is whether we would send redundant discards though
fstrim when small discard is on.

Thoughts?

> 
> Signed-off-by: Yunlei He <heyunlei@huawei.com>
> ---
>  fs/f2fs/segment.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 0738f48..a40a34b 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -916,7 +916,11 @@ void clear_prefree_segments(struct f2fs_sb_info *sbi, struct cp_control *cpc)
>  
>  		dirty_i->nr_dirty[PRE] -= end - start;
>  
> -		if (force || !test_opt(sbi, DISCARD))
> +		if (!test_opt(sbi, DISCARD))
> +			continue;
> +
> +		if (force && start * sbi->blocks_per_seg >= cpc->trim_start &&
> +						end * sbi->blocks_per_seg <= cpc->trim_end)
>  			continue;
>  
>  		if (!test_opt(sbi, LFS) || sbi->segs_per_sec == 1) {
> 


------------------------------------------------------------------------------
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/intel

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

* Re: [PATCH 1/2 v2] f2fs: fix a missing discard prefree segments
  2016-12-21  2:02 ` [PATCH 1/2 v2] f2fs: fix a missing discard prefree segments Chao Yu
@ 2016-12-21  2:27   ` Chao Yu
  2016-12-21  3:35     ` heyunlei
  0 siblings, 1 reply; 10+ messages in thread
From: Chao Yu @ 2016-12-21  2:27 UTC (permalink / raw)
  To: Yunlei He, linux-f2fs-devel, jaegeuk; +Cc: heyunlei

On 2016/12/21 10:02, Chao Yu wrote:
> Hi Jaegeuk, Yunlei,
> 
> On 2016/12/20 11:11, Yunlei He wrote:
>> If userspace issue a fstrim with a range not involve prefree segments,
>> it will reuse these segments without discard. This patch fix it.
> 
>>From v1 patch, I guess originally Yunlei wants to skip clearing prefree segments
> which is not be included in fstrim range, now v2 patch doesn't follow the
> original intention.
> 
> So I guess below modification is enough:
> 
> -		if (force || !test_opt(sbi, DISCARD))
> +		if (!test_opt(sbi, DISCARD))
> 
> Only we should consider is whether we would send redundant discards though
> fstrim when small discard is on.

Oh, below condition has handling redundant case, sorry for my wrong understanding.

if (force && start * sbi->blocks_per_seg >= cpc->trim_start &&
	end * sbi->blocks_per_seg <= cpc->trim_end)
		continue;

But should fix end boundary judegment as below:
(end - 1) * sbi->blocks_per_seg <= cpc->trim_end

Thanks,

> 
> Thoughts?
> 
>>
>> Signed-off-by: Yunlei He <heyunlei@huawei.com>
>> ---
>>  fs/f2fs/segment.c | 6 +++++-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
>> index 0738f48..a40a34b 100644
>> --- a/fs/f2fs/segment.c
>> +++ b/fs/f2fs/segment.c
>> @@ -916,7 +916,11 @@ void clear_prefree_segments(struct f2fs_sb_info *sbi, struct cp_control *cpc)
>>  
>>  		dirty_i->nr_dirty[PRE] -= end - start;
>>  
>> -		if (force || !test_opt(sbi, DISCARD))
>> +		if (!test_opt(sbi, DISCARD))
>> +			continue;
>> +
>> +		if (force && start * sbi->blocks_per_seg >= cpc->trim_start &&
>> +						end * sbi->blocks_per_seg <= cpc->trim_end)
>>  			continue;
>>  
>>  		if (!test_opt(sbi, LFS) || sbi->segs_per_sec == 1) {
>>


------------------------------------------------------------------------------
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/intel

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

* Re: [PATCH 1/2 v2] f2fs: fix a missing discard prefree segments
  2016-12-21  2:27   ` Chao Yu
@ 2016-12-21  3:35     ` heyunlei
  2016-12-21  3:55       ` Chao Yu
  0 siblings, 1 reply; 10+ messages in thread
From: heyunlei @ 2016-12-21  3:35 UTC (permalink / raw)
  To: Chao Yu, linux-f2fs-devel, jaegeuk; +Cc: heyunlei

Hi Chao,

On 2016/12/21 10:27, Chao Yu wrote:
> On 2016/12/21 10:02, Chao Yu wrote:
>> Hi Jaegeuk, Yunlei,
>>
>> On 2016/12/20 11:11, Yunlei He wrote:
>>> If userspace issue a fstrim with a range not involve prefree segments,
>>> it will reuse these segments without discard. This patch fix it.
>>
>> >From v1 patch, I guess originally Yunlei wants to skip clearing prefree segments
>> which is not be included in fstrim range, now v2 patch doesn't follow the
>> original intention.
>>
>> So I guess below modification is enough:
>>
>> -		if (force || !test_opt(sbi, DISCARD))
>> +		if (!test_opt(sbi, DISCARD))
>>
>> Only we should consider is whether we would send redundant discards though
>> fstrim when small discard is on.
>
> Oh, below condition has handling redundant case, sorry for my wrong understanding.
>
> if (force && start * sbi->blocks_per_seg >= cpc->trim_start &&
> 	end * sbi->blocks_per_seg <= cpc->trim_end)
> 		continue;
>
> But should fix end boundary judegment as below:
> (end - 1) * sbi->blocks_per_seg <= cpc->trim_end

For example:  if start=0, end=2, it means that the first and second segment is prefree,
but the third segment is not.

start block address of first segment = 0 * 512 = start * 512
end block address of secnod segment = (3-1) * 512 = end * 512

So, boundary judgement has no problem?

Thanks.

>
> Thanks,
>
>>
>> Thoughts?
>>
>>>
>>> Signed-off-by: Yunlei He <heyunlei@huawei.com>
>>> ---
>>>  fs/f2fs/segment.c | 6 +++++-
>>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
>>> index 0738f48..a40a34b 100644
>>> --- a/fs/f2fs/segment.c
>>> +++ b/fs/f2fs/segment.c
>>> @@ -916,7 +916,11 @@ void clear_prefree_segments(struct f2fs_sb_info *sbi, struct cp_control *cpc)
>>>
>>>  		dirty_i->nr_dirty[PRE] -= end - start;
>>>
>>> -		if (force || !test_opt(sbi, DISCARD))
>>> +		if (!test_opt(sbi, DISCARD))
>>> +			continue;
>>> +
>>> +		if (force && start * sbi->blocks_per_seg >= cpc->trim_start &&
>>> +						end * sbi->blocks_per_seg <= cpc->trim_end)
>>>  			continue;
>>>
>>>  		if (!test_opt(sbi, LFS) || sbi->segs_per_sec == 1) {
>>>
>
>
> .
>


------------------------------------------------------------------------------
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/intel

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

* Re: [PATCH 1/2 v2] f2fs: fix a missing discard prefree segments
  2016-12-21  3:35     ` heyunlei
@ 2016-12-21  3:55       ` Chao Yu
  2016-12-21  9:00         ` heyunlei
  0 siblings, 1 reply; 10+ messages in thread
From: Chao Yu @ 2016-12-21  3:55 UTC (permalink / raw)
  To: heyunlei, linux-f2fs-devel, jaegeuk; +Cc: heyunlei

On 2016/12/21 11:35, heyunlei wrote:
> Hi Chao,
> 
> On 2016/12/21 10:27, Chao Yu wrote:
>> On 2016/12/21 10:02, Chao Yu wrote:
>>> Hi Jaegeuk, Yunlei,
>>>
>>> On 2016/12/20 11:11, Yunlei He wrote:
>>>> If userspace issue a fstrim with a range not involve prefree segments,
>>>> it will reuse these segments without discard. This patch fix it.
>>>
>>> >From v1 patch, I guess originally Yunlei wants to skip clearing prefree segments
>>> which is not be included in fstrim range, now v2 patch doesn't follow the
>>> original intention.
>>>
>>> So I guess below modification is enough:
>>>
>>> -		if (force || !test_opt(sbi, DISCARD))
>>> +		if (!test_opt(sbi, DISCARD))
>>>
>>> Only we should consider is whether we would send redundant discards though
>>> fstrim when small discard is on.
>>
>> Oh, below condition has handling redundant case, sorry for my wrong understanding.
>>
>> if (force && start * sbi->blocks_per_seg >= cpc->trim_start &&
>> 	end * sbi->blocks_per_seg <= cpc->trim_end)
>> 		continue;
>>
>> But should fix end boundary judegment as below:
>> (end - 1) * sbi->blocks_per_seg <= cpc->trim_end
> 
> For example:  if start=0, end=2, it means that the first and second segment is prefree,
> but the third segment is not.
> 
> start block address of first segment = 0 * 512 = start * 512
> end block address of secnod segment = (3-1) * 512 = end * 512

IIRC, if we want to trim [0, 1] segment during fstrim, and #0, #1 segment is
prefree, so in fstrim:
start = 0
end = 2
trim_start = 0
trim_end = 1

So we should use (end - 1) to match trim_end, is that right?

Thanks,

> 
> So, boundary judgement has no problem?
> 
> Thanks.
> 
>>
>> Thanks,
>>
>>>
>>> Thoughts?
>>>
>>>>
>>>> Signed-off-by: Yunlei He <heyunlei@huawei.com>
>>>> ---
>>>>  fs/f2fs/segment.c | 6 +++++-
>>>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
>>>> index 0738f48..a40a34b 100644
>>>> --- a/fs/f2fs/segment.c
>>>> +++ b/fs/f2fs/segment.c
>>>> @@ -916,7 +916,11 @@ void clear_prefree_segments(struct f2fs_sb_info *sbi, struct cp_control *cpc)
>>>>
>>>>  		dirty_i->nr_dirty[PRE] -= end - start;
>>>>
>>>> -		if (force || !test_opt(sbi, DISCARD))
>>>> +		if (!test_opt(sbi, DISCARD))
>>>> +			continue;
>>>> +
>>>> +		if (force && start * sbi->blocks_per_seg >= cpc->trim_start &&
>>>> +						end * sbi->blocks_per_seg <= cpc->trim_end)
>>>>  			continue;
>>>>
>>>>  		if (!test_opt(sbi, LFS) || sbi->segs_per_sec == 1) {
>>>>
>>
>>
>> .
>>
> 
> 
> .
> 


------------------------------------------------------------------------------
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/intel

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

* Re: [PATCH 1/2 v2] f2fs: fix a missing discard prefree segments
  2016-12-21  3:55       ` Chao Yu
@ 2016-12-21  9:00         ` heyunlei
  0 siblings, 0 replies; 10+ messages in thread
From: heyunlei @ 2016-12-21  9:00 UTC (permalink / raw)
  To: Chao Yu, linux-f2fs-devel, jaegeuk; +Cc: heyunlei

Hi Chao,

On 2016/12/21 11:55, Chao Yu wrote:
> On 2016/12/21 11:35, heyunlei wrote:
>> Hi Chao,
>>
>> On 2016/12/21 10:27, Chao Yu wrote:
>>> On 2016/12/21 10:02, Chao Yu wrote:
>>>> Hi Jaegeuk, Yunlei,
>>>>
>>>> On 2016/12/20 11:11, Yunlei He wrote:
>>>>> If userspace issue a fstrim with a range not involve prefree segments,
>>>>> it will reuse these segments without discard. This patch fix it.
>>>>
>>>> >From v1 patch, I guess originally Yunlei wants to skip clearing prefree segments
>>>> which is not be included in fstrim range, now v2 patch doesn't follow the
>>>> original intention.
>>>>
>>>> So I guess below modification is enough:
>>>>
>>>> -		if (force || !test_opt(sbi, DISCARD))
>>>> +		if (!test_opt(sbi, DISCARD))
>>>>
>>>> Only we should consider is whether we would send redundant discards though
>>>> fstrim when small discard is on.
>>>
>>> Oh, below condition has handling redundant case, sorry for my wrong understanding.
>>>
>>> if (force && start * sbi->blocks_per_seg >= cpc->trim_start &&
>>> 	end * sbi->blocks_per_seg <= cpc->trim_end)
>>> 		continue;
>>>
>>> But should fix end boundary judegment as below:
>>> (end - 1) * sbi->blocks_per_seg <= cpc->trim_end
>>
>> For example:  if start=0, end=2, it means that the first and second segment is prefree,
>> but the third segment is not.
>>
>> start block address of first segment = 0 * 512 = start * 512
>> end block address of secnod segment = (3-1) * 512 = end * 512
>
> IIRC, if we want to trim [0, 1] segment during fstrim, and #0, #1 segment is
> prefree, so in fstrim:
> start = 0
> end = 2
> trim_start = 0
> trim_end = 1
>
> So we should use (end - 1) to match trim_end, is that right?

Yeah, you are right! I'll send a new version.

Thanks,
>
> Thanks,
>
>>
>> So, boundary judgement has no problem?
>>
>> Thanks.
>>
>>>
>>> Thanks,
>>>
>>>>
>>>> Thoughts?
>>>>
>>>>>
>>>>> Signed-off-by: Yunlei He <heyunlei@huawei.com>
>>>>> ---
>>>>>  fs/f2fs/segment.c | 6 +++++-
>>>>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
>>>>> index 0738f48..a40a34b 100644
>>>>> --- a/fs/f2fs/segment.c
>>>>> +++ b/fs/f2fs/segment.c
>>>>> @@ -916,7 +916,11 @@ void clear_prefree_segments(struct f2fs_sb_info *sbi, struct cp_control *cpc)
>>>>>
>>>>>  		dirty_i->nr_dirty[PRE] -= end - start;
>>>>>
>>>>> -		if (force || !test_opt(sbi, DISCARD))
>>>>> +		if (!test_opt(sbi, DISCARD))
>>>>> +			continue;
>>>>> +
>>>>> +		if (force && start * sbi->blocks_per_seg >= cpc->trim_start &&
>>>>> +						end * sbi->blocks_per_seg <= cpc->trim_end)
>>>>>  			continue;
>>>>>
>>>>>  		if (!test_opt(sbi, LFS) || sbi->segs_per_sec == 1) {
>>>>>
>>>
>>>
>>> .
>>>
>>
>>
>> .
>>
>
>
> .
>


------------------------------------------------------------------------------
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/intel

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

* Re: [PATCH 2/2 v2] f2fs: add a case of no need to read a page in write begin
  2016-12-21  1:28     ` Chao Yu
@ 2016-12-21 17:58       ` Jaegeuk Kim
  0 siblings, 0 replies; 10+ messages in thread
From: Jaegeuk Kim @ 2016-12-21 17:58 UTC (permalink / raw)
  To: Chao Yu; +Cc: heyunlei, linux-f2fs-devel

On 12/21, Chao Yu wrote:
> On 2016/12/21 2:03, Jaegeuk Kim wrote:
> > On 12/20, Yunlei He wrote:
> >> If the range we write cover the whole valid data in the last page,
> >> we do not need to read it.
> >>
> >> Signed-off-by: Yunlei He <heyunlei@huawei.com>
> >> ---
> >>  fs/f2fs/data.c | 5 ++++-
> >>  1 file changed, 4 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> >> index 9ac2625..303873f 100644
> >> --- a/fs/f2fs/data.c
> >> +++ b/fs/f2fs/data.c
> >> @@ -1715,6 +1715,9 @@ static int f2fs_write_begin(struct file *file, struct address_space *mapping,
> >>  	if (len == PAGE_SIZE || PageUptodate(page))
> >>  		return 0;
> >>  
> >> +	if (!(pos & (PAGE_SIZE - 1)) && (pos + len) >= i_size_read(inode))
> > 
> > I added here:
> > 		zero_user_segment(page, 0, PAGE_SIZE);
> 
> Only need to zeroout in range of [len, page_size] in the page.

Yup, fixed.

Thanks,

> 
> Thanks,
> 
> > 
> > Otherwise, xfstests/f2fs/001 gives a failure.
> > 
> > Thanks,
> > 
> >> +		return 0;
> >> +
> >>  	if (blkaddr == NEW_ADDR) {
> >>  		zero_user_segment(page, 0, PAGE_SIZE);
> >>  		SetPageUptodate(page);
> >> @@ -1768,7 +1771,7 @@ static int f2fs_write_end(struct file *file,
> >>  	 * let generic_perform_write() try to copy data again through copied=0.
> >>  	 */
> >>  	if (!PageUptodate(page)) {
> >> -		if (unlikely(copied != PAGE_SIZE))
> >> +		if (unlikely(copied != len))
> >>  			copied = 0;
> >>  		else
> >>  			SetPageUptodate(page);
> >> -- 
> >> 2.10.1
> >>
> >>
> >> ------------------------------------------------------------------------------
> >> Developer Access Program for Intel Xeon Phi Processors
> >> Access to Intel Xeon Phi processor-based developer platforms.
> >> With one year of Intel Parallel Studio XE.
> >> Training and support from Colfax.
> >> Order your platform today.http://sdm.link/intel
> >> _______________________________________________
> >> Linux-f2fs-devel mailing list
> >> Linux-f2fs-devel@lists.sourceforge.net
> >> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> > 
> > .
> > 

------------------------------------------------------------------------------
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/intel

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

end of thread, other threads:[~2016-12-21 17:58 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-20  3:11 [PATCH 1/2 v2] f2fs: fix a missing discard prefree segments Yunlei He
2016-12-20  3:11 ` [PATCH 2/2 v2] f2fs: add a case of no need to read a page in write begin Yunlei He
2016-12-20 18:03   ` Jaegeuk Kim
2016-12-21  1:28     ` Chao Yu
2016-12-21 17:58       ` Jaegeuk Kim
2016-12-21  2:02 ` [PATCH 1/2 v2] f2fs: fix a missing discard prefree segments Chao Yu
2016-12-21  2:27   ` Chao Yu
2016-12-21  3:35     ` heyunlei
2016-12-21  3:55       ` Chao Yu
2016-12-21  9:00         ` heyunlei

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.