date:20180119

Re: [f2fs-dev] [PATCH v2] f2fs: correct removexattr behavior for null valued extended attribute

2018-01-19 Thread Chao Yu

On 2018/1/20 12:24, Jaegeuk Kim wrote:
> On 01/17, Chao Yu wrote:
>> Hi Jaegeuk,
>>
>> Forgot to merge this patch? ;)
> 
> Weird. I didn't get this patch before.

I'm sure it has been sent to f2fs mailing list, may be it's been junked?

Anyway, I just resent it for Daeho Jeong, please check it.

> Is this a full patch?
> 
>>
>> On 2018/1/10 10:24, Daeho Jeong wrote:
>>> __vfs_removexattr() transfers "NULL" value to the setxattr handler of
>>> the f2fs filesystem in order to remove the extended attribute. But,
>>> __f2fs_setxattr() just ignores the removal request when the value of
>>> the extended attribute is already NULL. We have to remove the extended
>>> attribute itself even if the value of that is already NULL.
>>>
>>> We can reporduce this bug with the below:
>>>
>>> 1. touch file
>>> 2. setfattr -n "user.foo" file
>>> 3. setfattr -x "user.foo" file
>>> 4. getfattr -d file
 user.foo
>>>
>>> Signed-off-by: Daeho Jeong 
>>> Signed-off-by: Youngjin Gil 
>>> Tested-by: Hobin Woo 
>>> Tested-by: Chao Yu 
>>> Reviewed-by: Chao Yu 
>>> ---
>>>  fs/f2fs/xattr.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/fs/f2fs/xattr.c b/fs/f2fs/xattr.c
>>> index ec8961e..2776618 100644
>>> --- a/fs/f2fs/xattr.c
>>> +++ b/fs/f2fs/xattr.c
>>> @@ -598,7 +598,7 @@ static int __f2fs_setxattr(struct inode *inode, int 
>>> index,
>>> goto exit;
>>> }
>>>  
>>> -   if (f2fs_xattr_value_same(here, value, size))
>>> +   if (value && f2fs_xattr_value_same(here, value, size))
>>> goto exit;
>>> } else if ((flags & XATTR_REPLACE)) {
>>> error = -ENODATA;
>>>
> 
> --
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, Slashdot.org! http://sdm.link/slashdot
> ___
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> 

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

[f2fs-dev] [PATCH] f2fs: correct removexattr behavior for null valued extended attribute

2018-01-19 Thread Chao Yu

From: Daeho Jeong 

__vfs_removexattr() transfers "NULL" value to the setxattr handler of
the f2fs filesystem in order to remove the extended attribute. But,
__f2fs_setxattr() just ignores the removal request when the value of
the extended attribute is already NULL. We have to remove the extended
attribute itself even if the value of that is already NULL.

We can reporduce this bug with the below:

1. touch file
2. setfattr -n "user.foo" file
3. setfattr -x "user.foo" file
4. getfattr -d file
> user.foo

Signed-off-by: Daeho Jeong 
Signed-off-by: Youngjin Gil 
Tested-by: Hobin Woo 
Tested-by: Chao Yu 
Reviewed-by: Chao Yu 
Signed-off-by: Chao Yu 
---
 fs/f2fs/xattr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/f2fs/xattr.c b/fs/f2fs/xattr.c
index 600162f4ddbf..ae2dfa709f5d 100644
--- a/fs/f2fs/xattr.c
+++ b/fs/f2fs/xattr.c
@@ -600,7 +600,7 @@ static int __f2fs_setxattr(struct inode *inode, int index,
goto exit;
}
 
-   if (f2fs_xattr_value_same(here, value, size))
+   if (value && f2fs_xattr_value_same(here, value, size))
goto exit;
} else if ((flags & XATTR_REPLACE)) {
error = -ENODATA;
-- 
2.14.1.145.gb3622a4ee


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH RFC] f2fs: add PRE2 to mark segments free to one checkpoint but obsolete to the other

2018-01-19 Thread Chao Yu

Hi Weichao,

On 2018/1/20 7:29, Weichao Guo wrote:
> Currently, we set prefree segments as free ones after writing
> a checkpoint, then believe the segments could be used safely.
> However, if a sudden power-off coming and the newer checkpoint
> corrupted due to hardware issues at the same time, we will try
> to use the old checkpoint and may face an inconsistent file
> system status.

IIUC, you mean:

1. write nodes into segment x;
2. write checkpoint A;
3. remove nodes in segment x;
4. write checkpoint B;
5. issue discard or write datas into segment x;
6. sudden power-cut

But after reboot, we found checkpoint B is corrupted due to hardware, and
then start to use checkpoint A, but nodes in segment x recorded as valid
data in checkpoint A has been overcovered in step 5), so we will encounter
inconsistent meta data, right?

Thanks,

> 
> How about add an PRE2 status for prefree segments, and make
> sure the segments could be used safely to both checkpoints?
> Or any better solutions? Or this is not a problem?
> 
> Look forward to your comments!
> 
> Signed-off-by: Weichao Guo 
> ---
>  fs/f2fs/gc.c  | 11 +--
>  fs/f2fs/segment.c | 21 ++---
>  fs/f2fs/segment.h |  6 ++
>  3 files changed, 33 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> index 33e7969..153e3ea 100644
> --- a/fs/f2fs/gc.c
> +++ b/fs/f2fs/gc.c
> @@ -1030,7 +1030,12 @@ int f2fs_gc(struct f2fs_sb_info *sbi, bool sync,
>* threshold, we can make them free by checkpoint. Then, we
>* secure free segments which doesn't need fggc any more.
>*/
> - if (prefree_segments(sbi)) {
> + if (prefree_segments(sbi) || prefree2_segments(sbi)) {
> + ret = write_checkpoint(sbi, );
> + if (ret)
> + goto stop;
> + }
> + if (has_not_enough_free_secs(sbi, 0, 0) && 
> prefree2_segments(sbi)) {
>   ret = write_checkpoint(sbi, );
>   if (ret)
>   goto stop;
> @@ -1063,8 +1068,10 @@ int f2fs_gc(struct f2fs_sb_info *sbi, bool sync,
>   goto gc_more;
>   }
>  
> - if (gc_type == FG_GC)
> + if (gc_type == FG_GC) {
> + ret = write_checkpoint(sbi, );
>   ret = write_checkpoint(sbi, );
> + }
>   }
>  stop:
>   SIT_I(sbi)->last_victim[ALLOC_NEXT] = 0;
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 2e8e054d..9dec445 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -1606,7 +1606,7 @@ static void set_prefree_as_free_segments(struct 
> f2fs_sb_info *sbi)
>   unsigned int segno;
>  
>   mutex_lock(_i->seglist_lock);
> - for_each_set_bit(segno, dirty_i->dirty_segmap[PRE], MAIN_SEGS(sbi))
> + for_each_set_bit(segno, dirty_i->dirty_segmap[PRE2], MAIN_SEGS(sbi))
>   __set_test_and_free(sbi, segno);
>   mutex_unlock(_i->seglist_lock);
>  }
> @@ -1617,13 +1617,17 @@ void clear_prefree_segments(struct f2fs_sb_info *sbi, 
> struct cp_control *cpc)
>   struct list_head *head = >entry_list;
>   struct discard_entry *entry, *this;
>   struct dirty_seglist_info *dirty_i = DIRTY_I(sbi);
> - unsigned long *prefree_map = dirty_i->dirty_segmap[PRE];
> + unsigned long *prefree_map;
>   unsigned int start = 0, end = -1;
>   unsigned int secno, start_segno;
>   bool force = (cpc->reason & CP_DISCARD);
> + int phase = 0;
> + enum dirty_type dirty_type = PRE2;
>  
>   mutex_lock(_i->seglist_lock);
>  
> +next_step:
> + prefree_map = dirty_i->dirty_segmap[dirty_type];
>   while (1) {
>   int i;
>   start = find_next_bit(prefree_map, MAIN_SEGS(sbi), end + 1);
> @@ -1635,7 +1639,7 @@ void clear_prefree_segments(struct f2fs_sb_info *sbi, 
> struct cp_control *cpc)
>   for (i = start; i < end; i++)
>   clear_bit(i, prefree_map);
>  
> - dirty_i->nr_dirty[PRE] -= end - start;
> + dirty_i->nr_dirty[dirty_type] -= end - start;
>  
>   if (!test_opt(sbi, DISCARD))
>   continue;
> @@ -1663,6 +1667,16 @@ void clear_prefree_segments(struct f2fs_sb_info *sbi, 
> struct cp_control *cpc)
>   else
>   end = start - 1;
>   }
> + if (phase == 0) {
> + /* status change: PRE -> PRE2 */
> + for_each_set_bit(segno, dirty_i->dirty_segmap[PRE], 
> MAIN_SEGS(sbi))
> + if (!test_and_set_bit(segno, prefree_map))
> + dirty_i->nr_dirty[PRE2]++;
> +
> + phase = 1;
> + dirty_type = PRE;
> + goto next_step;
> + }
>   mutex_unlock(_i->seglist_lock);
>  
>   /* send small discards */
> @@ -2245,6 +2259,7 @@ static void

[f2fs-dev] [PATCH 1/2] f2fs: allow to recover node blocks given updated checkpoint

2018-01-19 Thread Jaegeuk Kim

If fsck.f2fs changes crc, we have no way to recover some inode blocks by roll-
forward recovery. Let's relax the condition to recover them.

Signed-off-by: Jaegeuk Kim 
---
 fs/f2fs/node.h | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/fs/f2fs/node.h b/fs/f2fs/node.h
index 0ee3e5ff49a3..15280eeb24ea 100644
--- a/fs/f2fs/node.h
+++ b/fs/f2fs/node.h
@@ -305,10 +305,11 @@ static inline bool is_recoverable_dnode(struct page *page)
struct f2fs_checkpoint *ckpt = F2FS_CKPT(F2FS_P_SB(page));
__u64 cp_ver = cur_cp_version(ckpt);
 
-   if (__is_set_ckpt_flags(ckpt, CP_CRC_RECOVERY_FLAG))
+   if (__is_set_ckpt_flags(ckpt, CP_CRC_RECOVERY_FLAG)) {
cp_ver |= (cur_cp_crc(ckpt) << 32);
-
-   return cp_ver == cpver_of_node(page);
+   return cp_ver == cpver_of_node(page);
+   }
+   return (cp_ver << 32) == (cpver_of_node(page) << 32);
 }
 
 /*
-- 
2.15.0.531.g2ccb3012c9-goog


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

[f2fs-dev] [PATCH 2/2] f2fs: recover some i_inline flags

2018-01-19 Thread Jaegeuk Kim

This fixes lost i_inline flags during roll-forward.

Signed-off-by: Jaegeuk Kim 
---
 fs/f2fs/recovery.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c
index cbeef73bc4dd..2354f1e05e19 100644
--- a/fs/f2fs/recovery.c
+++ b/fs/f2fs/recovery.c
@@ -211,6 +211,15 @@ static void recover_inode(struct inode *inode, struct page 
*page)
 
F2FS_I(inode)->i_advise = raw->i_advise;
 
+   if (raw->i_inline & F2FS_PIN_FILE)
+   set_inode_flag(inode, FI_PIN_FILE);
+   if (raw->i_inline & F2FS_DATA_EXIST)
+   set_inode_flag(inode, FI_DATA_EXIST);
+   else
+   clear_inode_flag(inode, FI_DATA_EXIST);
+   if (!(raw->i_inline & F2FS_INLINE_DOTS))
+   clear_inode_flag(inode, FI_INLINE_DOTS);
+
if (file_enc_name(inode))
name = "";
else
-- 
2.15.0.531.g2ccb3012c9-goog


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH v2] f2fs: correct removexattr behavior for null valued extended attribute

2018-01-19 Thread Jaegeuk Kim

On 01/17, Chao Yu wrote:
> Hi Jaegeuk,
> 
> Forgot to merge this patch? ;)

Weird. I didn't get this patch before.
Is this a full patch?

> 
> On 2018/1/10 10:24, Daeho Jeong wrote:
> > __vfs_removexattr() transfers "NULL" value to the setxattr handler of
> > the f2fs filesystem in order to remove the extended attribute. But,
> > __f2fs_setxattr() just ignores the removal request when the value of
> > the extended attribute is already NULL. We have to remove the extended
> > attribute itself even if the value of that is already NULL.
> > 
> > We can reporduce this bug with the below:
> > 
> > 1. touch file
> > 2. setfattr -n "user.foo" file
> > 3. setfattr -x "user.foo" file
> > 4. getfattr -d file
> >> user.foo
> > 
> > Signed-off-by: Daeho Jeong 
> > Signed-off-by: Youngjin Gil 
> > Tested-by: Hobin Woo 
> > Tested-by: Chao Yu 
> > Reviewed-by: Chao Yu 
> > ---
> >  fs/f2fs/xattr.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/fs/f2fs/xattr.c b/fs/f2fs/xattr.c
> > index ec8961e..2776618 100644
> > --- a/fs/f2fs/xattr.c
> > +++ b/fs/f2fs/xattr.c
> > @@ -598,7 +598,7 @@ static int __f2fs_setxattr(struct inode *inode, int 
> > index,
> > goto exit;
> > }
> >  
> > -   if (f2fs_xattr_value_same(here, value, size))
> > +   if (value && f2fs_xattr_value_same(here, value, size))
> > goto exit;
> > } else if ((flags & XATTR_REPLACE)) {
> > error = -ENODATA;
> > 

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH RFC] f2fs: add PRE2 to mark segments free to one checkpoint but obsolete to the other

2018-01-19 Thread Gao Xiang via Linux-f2fs-devel


Hi Weichao,


On 2018/1/19 23:47, guoweichao wrote:

A critical case is using the free segments as data segments which
are previously node segments to the old checkpoint. With fault injecting
of the newer CP pack, fsck can find errors when checking the sanity of nid.
Sorry to interrupt because I'm just curious about this scenario and the 
detail.


As far as I know, if the whole blocks in a segment become invalid,
the segment will become PREFREE, and then if a checkpoint is followed, 
we can reuse this segment or

discard the whole segment safely after this checkpoint was done
(I think It makes sure that this segment is certainly FREE and not 
reused in this checkpoint).


If the segment in the old checkpoint is a node segment, and node blocks 
in the segment are all invalid until the new checkpoint.
It seems no danger to reuse the FREE node segment as a data segment in 
the next checkpoint?


or something related to SPOR? In my mind f2fs-tools ignores POR node 
chain...


Thanks,

On 2018/1/20 7:29, Weichao Guo wrote:

Currently, we set prefree segments as free ones after writing
a checkpoint, then believe the segments could be used safely.
However, if a sudden power-off coming and the newer checkpoint
corrupted due to hardware issues at the same time, we will try
to use the old checkpoint and may face an inconsistent file
system status.

How about add an PRE2 status for prefree segments, and make
sure the segments could be used safely to both checkpoints?
Or any better solutions? Or this is not a problem?

Look forward to your comments!

Signed-off-by: Weichao Guo 
---
  fs/f2fs/gc.c  | 11 +--
  fs/f2fs/segment.c | 21 ++---
  fs/f2fs/segment.h |  6 ++
  3 files changed, 33 insertions(+), 5 deletions(-)

diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
index 33e7969..153e3ea 100644
--- a/fs/f2fs/gc.c
+++ b/fs/f2fs/gc.c
@@ -1030,7 +1030,12 @@ int f2fs_gc(struct f2fs_sb_info *sbi, bool sync,
 * threshold, we can make them free by checkpoint. Then, we
 * secure free segments which doesn't need fggc any more.
 */
-   if (prefree_segments(sbi)) {
+   if (prefree_segments(sbi) || prefree2_segments(sbi)) {
+   ret = write_checkpoint(sbi, );
+   if (ret)
+   goto stop;
+   }
+   if (has_not_enough_free_secs(sbi, 0, 0) && 
prefree2_segments(sbi)) {
ret = write_checkpoint(sbi, );
if (ret)
goto stop;
@@ -1063,8 +1068,10 @@ int f2fs_gc(struct f2fs_sb_info *sbi, bool sync,
goto gc_more;
}
  
-		if (gc_type == FG_GC)

+   if (gc_type == FG_GC) {
+   ret = write_checkpoint(sbi, );
ret = write_checkpoint(sbi, );
+   }
}
  stop:
SIT_I(sbi)->last_victim[ALLOC_NEXT] = 0;
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 2e8e054d..9dec445 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -1606,7 +1606,7 @@ static void set_prefree_as_free_segments(struct 
f2fs_sb_info *sbi)
unsigned int segno;
  
  	mutex_lock(_i->seglist_lock);

-   for_each_set_bit(segno, dirty_i->dirty_segmap[PRE], MAIN_SEGS(sbi))
+   for_each_set_bit(segno, dirty_i->dirty_segmap[PRE2], MAIN_SEGS(sbi))
__set_test_and_free(sbi, segno);
mutex_unlock(_i->seglist_lock);
  }
@@ -1617,13 +1617,17 @@ void clear_prefree_segments(struct f2fs_sb_info *sbi, 
struct cp_control *cpc)
struct list_head *head = >entry_list;
struct discard_entry *entry, *this;
struct dirty_seglist_info *dirty_i = DIRTY_I(sbi);
-   unsigned long *prefree_map = dirty_i->dirty_segmap[PRE];
+   unsigned long *prefree_map;
unsigned int start = 0, end = -1;
unsigned int secno, start_segno;
bool force = (cpc->reason & CP_DISCARD);
+   int phase = 0;
+   enum dirty_type dirty_type = PRE2;
  
  	mutex_lock(_i->seglist_lock);
  
+next_step:

+   prefree_map = dirty_i->dirty_segmap[dirty_type];
while (1) {
int i;
start = find_next_bit(prefree_map, MAIN_SEGS(sbi), end + 1);
@@ -1635,7 +1639,7 @@ void clear_prefree_segments(struct f2fs_sb_info *sbi, 
struct cp_control *cpc)
for (i = start; i < end; i++)
clear_bit(i, prefree_map);
  
-		dirty_i->nr_dirty[PRE] -= end - start;

+   dirty_i->nr_dirty[dirty_type] -= end - start;
  
  		if (!test_opt(sbi, DISCARD))

continue;
@@ -1663,6 +1667,16 @@ void clear_prefree_segments(struct f2fs_sb_info *sbi, 
struct cp_control *cpc)
else
end = start - 1;
}
+   if (phase == 0) {
+   /* status change: PRE -> PRE2 */
+   for_each_set_bit(segno,

Re: [f2fs-dev] [PATCH RFC] f2fs: add PRE2 to mark segments free to one checkpoint but obsolete to the other

2018-01-19 Thread guoweichao


A critical case is using the free segments as data segments which
are previously node segments to the old checkpoint. With fault injecting
of the newer CP pack, fsck can find errors when checking the sanity of nid.

On 2018/1/20 7:29, Weichao Guo wrote:
> Currently, we set prefree segments as free ones after writing
> a checkpoint, then believe the segments could be used safely.
> However, if a sudden power-off coming and the newer checkpoint
> corrupted due to hardware issues at the same time, we will try
> to use the old checkpoint and may face an inconsistent file
> system status.
> 
> How about add an PRE2 status for prefree segments, and make
> sure the segments could be used safely to both checkpoints?
> Or any better solutions? Or this is not a problem?
> 
> Look forward to your comments!
> 
> Signed-off-by: Weichao Guo 
> ---
>  fs/f2fs/gc.c  | 11 +--
>  fs/f2fs/segment.c | 21 ++---
>  fs/f2fs/segment.h |  6 ++
>  3 files changed, 33 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> index 33e7969..153e3ea 100644
> --- a/fs/f2fs/gc.c
> +++ b/fs/f2fs/gc.c
> @@ -1030,7 +1030,12 @@ int f2fs_gc(struct f2fs_sb_info *sbi, bool sync,
>* threshold, we can make them free by checkpoint. Then, we
>* secure free segments which doesn't need fggc any more.
>*/
> - if (prefree_segments(sbi)) {
> + if (prefree_segments(sbi) || prefree2_segments(sbi)) {
> + ret = write_checkpoint(sbi, );
> + if (ret)
> + goto stop;
> + }
> + if (has_not_enough_free_secs(sbi, 0, 0) && 
> prefree2_segments(sbi)) {
>   ret = write_checkpoint(sbi, );
>   if (ret)
>   goto stop;
> @@ -1063,8 +1068,10 @@ int f2fs_gc(struct f2fs_sb_info *sbi, bool sync,
>   goto gc_more;
>   }
>  
> - if (gc_type == FG_GC)
> + if (gc_type == FG_GC) {
> + ret = write_checkpoint(sbi, );
>   ret = write_checkpoint(sbi, );
> + }
>   }
>  stop:
>   SIT_I(sbi)->last_victim[ALLOC_NEXT] = 0;
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 2e8e054d..9dec445 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -1606,7 +1606,7 @@ static void set_prefree_as_free_segments(struct 
> f2fs_sb_info *sbi)
>   unsigned int segno;
>  
>   mutex_lock(_i->seglist_lock);
> - for_each_set_bit(segno, dirty_i->dirty_segmap[PRE], MAIN_SEGS(sbi))
> + for_each_set_bit(segno, dirty_i->dirty_segmap[PRE2], MAIN_SEGS(sbi))
>   __set_test_and_free(sbi, segno);
>   mutex_unlock(_i->seglist_lock);
>  }
> @@ -1617,13 +1617,17 @@ void clear_prefree_segments(struct f2fs_sb_info *sbi, 
> struct cp_control *cpc)
>   struct list_head *head = >entry_list;
>   struct discard_entry *entry, *this;
>   struct dirty_seglist_info *dirty_i = DIRTY_I(sbi);
> - unsigned long *prefree_map = dirty_i->dirty_segmap[PRE];
> + unsigned long *prefree_map;
>   unsigned int start = 0, end = -1;
>   unsigned int secno, start_segno;
>   bool force = (cpc->reason & CP_DISCARD);
> + int phase = 0;
> + enum dirty_type dirty_type = PRE2;
>  
>   mutex_lock(_i->seglist_lock);
>  
> +next_step:
> + prefree_map = dirty_i->dirty_segmap[dirty_type];
>   while (1) {
>   int i;
>   start = find_next_bit(prefree_map, MAIN_SEGS(sbi), end + 1);
> @@ -1635,7 +1639,7 @@ void clear_prefree_segments(struct f2fs_sb_info *sbi, 
> struct cp_control *cpc)
>   for (i = start; i < end; i++)
>   clear_bit(i, prefree_map);
>  
> - dirty_i->nr_dirty[PRE] -= end - start;
> + dirty_i->nr_dirty[dirty_type] -= end - start;
>  
>   if (!test_opt(sbi, DISCARD))
>   continue;
> @@ -1663,6 +1667,16 @@ void clear_prefree_segments(struct f2fs_sb_info *sbi, 
> struct cp_control *cpc)
>   else
>   end = start - 1;
>   }
> + if (phase == 0) {
> + /* status change: PRE -> PRE2 */
> + for_each_set_bit(segno, dirty_i->dirty_segmap[PRE], 
> MAIN_SEGS(sbi))
> + if (!test_and_set_bit(segno, prefree_map))
> + dirty_i->nr_dirty[PRE2]++;
> +
> + phase = 1;
> + dirty_type = PRE;
> + goto next_step;
> + }
>   mutex_unlock(_i->seglist_lock);
>  
>   /* send small discards */
> @@ -2245,6 +2259,7 @@ static void change_curseg(struct f2fs_sb_info *sbi, int 
> type)
>  
>   mutex_lock(_i->seglist_lock);
>   __remove_dirty_segment(sbi, new_segno, PRE);
> + __remove_dirty_segment(sbi, new_segno, PRE2);
>   __remove_dirty_segment(sbi, new_segno, DIRTY);
>

[f2fs-dev] [PATCH RFC] f2fs: add PRE2 to mark segments free to one checkpoint but obsolete to the other

2018-01-19 Thread Weichao Guo

Currently, we set prefree segments as free ones after writing
a checkpoint, then believe the segments could be used safely.
However, if a sudden power-off coming and the newer checkpoint
corrupted due to hardware issues at the same time, we will try
to use the old checkpoint and may face an inconsistent file
system status.

How about add an PRE2 status for prefree segments, and make
sure the segments could be used safely to both checkpoints?
Or any better solutions? Or this is not a problem?

Look forward to your comments!

Signed-off-by: Weichao Guo 
---
 fs/f2fs/gc.c  | 11 +--
 fs/f2fs/segment.c | 21 ++---
 fs/f2fs/segment.h |  6 ++
 3 files changed, 33 insertions(+), 5 deletions(-)

diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
index 33e7969..153e3ea 100644
--- a/fs/f2fs/gc.c
+++ b/fs/f2fs/gc.c
@@ -1030,7 +1030,12 @@ int f2fs_gc(struct f2fs_sb_info *sbi, bool sync,
 * threshold, we can make them free by checkpoint. Then, we
 * secure free segments which doesn't need fggc any more.
 */
-   if (prefree_segments(sbi)) {
+   if (prefree_segments(sbi) || prefree2_segments(sbi)) {
+   ret = write_checkpoint(sbi, );
+   if (ret)
+   goto stop;
+   }
+   if (has_not_enough_free_secs(sbi, 0, 0) && 
prefree2_segments(sbi)) {
ret = write_checkpoint(sbi, );
if (ret)
goto stop;
@@ -1063,8 +1068,10 @@ int f2fs_gc(struct f2fs_sb_info *sbi, bool sync,
goto gc_more;
}
 
-   if (gc_type == FG_GC)
+   if (gc_type == FG_GC) {
+   ret = write_checkpoint(sbi, );
ret = write_checkpoint(sbi, );
+   }
}
 stop:
SIT_I(sbi)->last_victim[ALLOC_NEXT] = 0;
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 2e8e054d..9dec445 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -1606,7 +1606,7 @@ static void set_prefree_as_free_segments(struct 
f2fs_sb_info *sbi)
unsigned int segno;
 
mutex_lock(_i->seglist_lock);
-   for_each_set_bit(segno, dirty_i->dirty_segmap[PRE], MAIN_SEGS(sbi))
+   for_each_set_bit(segno, dirty_i->dirty_segmap[PRE2], MAIN_SEGS(sbi))
__set_test_and_free(sbi, segno);
mutex_unlock(_i->seglist_lock);
 }
@@ -1617,13 +1617,17 @@ void clear_prefree_segments(struct f2fs_sb_info *sbi, 
struct cp_control *cpc)
struct list_head *head = >entry_list;
struct discard_entry *entry, *this;
struct dirty_seglist_info *dirty_i = DIRTY_I(sbi);
-   unsigned long *prefree_map = dirty_i->dirty_segmap[PRE];
+   unsigned long *prefree_map;
unsigned int start = 0, end = -1;
unsigned int secno, start_segno;
bool force = (cpc->reason & CP_DISCARD);
+   int phase = 0;
+   enum dirty_type dirty_type = PRE2;
 
mutex_lock(_i->seglist_lock);
 
+next_step:
+   prefree_map = dirty_i->dirty_segmap[dirty_type];
while (1) {
int i;
start = find_next_bit(prefree_map, MAIN_SEGS(sbi), end + 1);
@@ -1635,7 +1639,7 @@ void clear_prefree_segments(struct f2fs_sb_info *sbi, 
struct cp_control *cpc)
for (i = start; i < end; i++)
clear_bit(i, prefree_map);
 
-   dirty_i->nr_dirty[PRE] -= end - start;
+   dirty_i->nr_dirty[dirty_type] -= end - start;
 
if (!test_opt(sbi, DISCARD))
continue;
@@ -1663,6 +1667,16 @@ void clear_prefree_segments(struct f2fs_sb_info *sbi, 
struct cp_control *cpc)
else
end = start - 1;
}
+   if (phase == 0) {
+   /* status change: PRE -> PRE2 */
+   for_each_set_bit(segno, dirty_i->dirty_segmap[PRE], 
MAIN_SEGS(sbi))
+   if (!test_and_set_bit(segno, prefree_map))
+   dirty_i->nr_dirty[PRE2]++;
+
+   phase = 1;
+   dirty_type = PRE;
+   goto next_step;
+   }
mutex_unlock(_i->seglist_lock);
 
/* send small discards */
@@ -2245,6 +2259,7 @@ static void change_curseg(struct f2fs_sb_info *sbi, int 
type)
 
mutex_lock(_i->seglist_lock);
__remove_dirty_segment(sbi, new_segno, PRE);
+   __remove_dirty_segment(sbi, new_segno, PRE2);
__remove_dirty_segment(sbi, new_segno, DIRTY);
mutex_unlock(_i->seglist_lock);
 
diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
index 71a2aaa..f702726 100644
--- a/fs/f2fs/segment.h
+++ b/fs/f2fs/segment.h
@@ -263,6 +263,7 @@ enum dirty_type {
DIRTY_COLD_NODE,/* dirty segments assigned as cold node logs */
DIRTY,  /* to count # of dirty segments */
PRE,

Re: [f2fs-dev] [PATCH v2] f2fs: correct removexattr behavior for null valued extended attribute

[f2fs-dev] [PATCH] f2fs: correct removexattr behavior for null valued extended attribute

Re: [f2fs-dev] [PATCH RFC] f2fs: add PRE2 to mark segments free to one checkpoint but obsolete to the other

[f2fs-dev] [PATCH 1/2] f2fs: allow to recover node blocks given updated checkpoint

[f2fs-dev] [PATCH 2/2] f2fs: recover some i_inline flags

Re: [f2fs-dev] [PATCH v2] f2fs: correct removexattr behavior for null valued extended attribute

Re: [f2fs-dev] [PATCH RFC] f2fs: add PRE2 to mark segments free to one checkpoint but obsolete to the other

Re: [f2fs-dev] [PATCH RFC] f2fs: add PRE2 to mark segments free to one checkpoint but obsolete to the other

[f2fs-dev] [PATCH RFC] f2fs: add PRE2 to mark segments free to one checkpoint but obsolete to the other

9 matches

Site Navigation

Mail list logo

Footer information