Re: [f2fs-dev] [PATCH] f2fs: update i_size after DIO completion
On 2018/9/21 5:51, Jaegeuk Kim wrote:
> This is related to
> ee70daaba82d ("xfs: update i_size after unwritten conversion in dio
> completion")
>
> If we update i_size during dio_write, dio_read can read out stale data, which
> breaks xfstests/465.
>
> Signed-off-by: Jaegeuk Kim
Reviewed-by: Chao Yu
Thanks,
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH v3] f2fs: submit cached bio to avoid endless PageWriteback
On 2018/9/26 11:32, Jaegeuk Kim wrote:
> On 09/26, Chao Yu wrote:
>> On 2018/9/26 9:42, Jaegeuk Kim wrote:
>>> On 09/26, Chao Yu wrote:
On 2018/9/26 8:20, Jaegeuk Kim wrote:
> On 09/21, Chao Yu wrote:
>> On 2018/9/18 10:14, Chao Yu wrote:
>>> On 2018/9/18 10:02, Jaegeuk Kim wrote:
On 09/18, Chao Yu wrote:
> On 2018/9/18 9:37, Jaegeuk Kim wrote:
>> On 09/18, Chao Yu wrote:
>>> On 2018/9/18 9:04, Jaegeuk Kim wrote:
On 09/13, Chao Yu wrote:
> From: Chao Yu
>
> When migrating encrypted block from background GC thread, we only
> add
> them into f2fs inner bio cache, but forget to submit the cached
> bio, it
> may cause potential deadlock when we are waiting page
> writebacked, fix
> it.
>
> Signed-off-by: Chao Yu
> ---
> v3:
> clean up codes suggested by Jaegeuk.
> fs/f2fs/f2fs.h | 2 +-
> fs/f2fs/gc.c | 71
> +++---
> fs/f2fs/node.c | 13 ++---
> 3 files changed, 61 insertions(+), 25 deletions(-)
>
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index b676b82312e0..917b2ca76aac 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -2869,7 +2869,7 @@ struct page *f2fs_new_node_page(struct
> dnode_of_data *dn, unsigned int ofs);
> void f2fs_ra_node_page(struct f2fs_sb_info *sbi, nid_t nid);
> struct page *f2fs_get_node_page(struct f2fs_sb_info *sbi,
> pgoff_t nid);
> struct page *f2fs_get_node_page_ra(struct page *parent, int
> start);
> -void f2fs_move_node_page(struct page *node_page, int gc_type);
> +int f2fs_move_node_page(struct page *node_page, int gc_type);
> int f2fs_fsync_node_pages(struct f2fs_sb_info *sbi, struct inode
> *inode,
> struct writeback_control *wbc, bool atomic,
> unsigned int *seq_id);
> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> index a4c1a419611d..f57622cfe058 100644
> --- a/fs/f2fs/gc.c
> +++ b/fs/f2fs/gc.c
> @@ -461,7 +461,7 @@ static int check_valid_map(struct
> f2fs_sb_info *sbi,
> * On validity, copy that node with cold status, otherwise
> (invalid node)
> * ignore that.
> */
> -static void gc_node_segment(struct f2fs_sb_info *sbi,
> +static int gc_node_segment(struct f2fs_sb_info *sbi,
> struct f2fs_summary *sum, unsigned int segno, int
> gc_type)
> {
> struct f2fs_summary *entry;
> @@ -469,6 +469,7 @@ static void gc_node_segment(struct
> f2fs_sb_info *sbi,
> int off;
> int phase = 0;
> bool fggc = (gc_type == FG_GC);
> + int submitted = 0;
>
> start_addr = START_BLOCK(sbi, segno);
>
> @@ -482,10 +483,11 @@ static void gc_node_segment(struct
> f2fs_sb_info *sbi,
> nid_t nid = le32_to_cpu(entry->nid);
> struct page *node_page;
> struct node_info ni;
> + int err;
>
> /* stop BG_GC if there is not enough free sections. */
> if (gc_type == BG_GC && has_not_enough_free_secs(sbi,
> 0, 0))
> - return;
> + return submitted;
>
> if (check_valid_map(sbi, segno, off) == 0)
> continue;
> @@ -522,7 +524,9 @@ static void gc_node_segment(struct
> f2fs_sb_info *sbi,
> continue;
> }
>
> - f2fs_move_node_page(node_page, gc_type);
> + err = f2fs_move_node_page(node_page, gc_type);
> + if (!err && gc_type == FG_GC)
> + submitted++;
> stat_inc_node_blk_count(sbi, 1, gc_type);
> }
>
> @@ -531,6 +535,7 @@ static void gc_node_segment(struct
> f2fs_sb_info *sbi,
>
> if (fggc)
> atomic_dec(>wb_sync_req[NODE]);
> + return submitted;
> }
>
> /*
> @@ -666,7 +671,7 @@ static int ra_data_block(struct inode *inode,
> pgoff_t index)
> * Move data block via META_MAPPING while keeping locked data
Re: [f2fs-dev] [PATCH 4/5] f2fs-tools: introduce sb checksum
Hi, Jaegeuk
On 2018/9/26 9:57, Jaegeuk Kim wrote:
> On 09/21, Junling Zheng wrote:
>> On 2018/9/21 5:38, Jaegeuk Kim wrote:
>>> On 09/20, Junling Zheng wrote:
Hi, Jaegeuk
On 2018/9/20 7:35, Jaegeuk Kim wrote:
> On 09/19, Junling Zheng wrote:
>> This patch introduced crc for superblock.
>>
>> Signed-off-by: Junling Zheng
>> ---
>> fsck/mount.c | 33 +
>> fsck/resize.c | 12 ++--
>> include/f2fs_fs.h | 6 +-
>> mkfs/f2fs_format.c | 8
>> 4 files changed, 52 insertions(+), 7 deletions(-)
>>
>> diff --git a/fsck/mount.c b/fsck/mount.c
>> index 74ff7c6..9019921 100644
>> --- a/fsck/mount.c
>> +++ b/fsck/mount.c
>> @@ -340,6 +340,7 @@ void print_raw_sb_info(struct f2fs_super_block *sb)
>> DISP_u32(sb, node_ino);
>> DISP_u32(sb, meta_ino);
>> DISP_u32(sb, cp_payload);
>> +DISP_u32(sb, crc);
>> DISP("%-.256s", sb, version);
>> printf("\n");
>> }
>> @@ -467,6 +468,9 @@ void print_sb_state(struct f2fs_super_block *sb)
>> if (f & cpu_to_le32(F2FS_FEATURE_LOST_FOUND)) {
>> MSG(0, "%s", " lost_found");
>> }
>> +if (f & cpu_to_le32(F2FS_FEATURE_SB_CHKSUM)) {
>> +MSG(0, "%s", " sb_checksum");
>> +}
>> MSG(0, "\n");
>> MSG(0, "Info: superblock encrypt level = %d, salt = ",
>> sb->encryption_level);
>> @@ -479,10 +483,20 @@ void update_superblock(struct f2fs_super_block
>> *sb, int sb_mask)
>> {
>> int index, ret;
>> u_int8_t *buf;
>> +u32 old_crc, new_crc;
>>
>> buf = calloc(BLOCK_SZ, 1);
>> ASSERT(buf);
>>
>> +if (get_sb(feature) & F2FS_FEATURE_SB_CHKSUM) {
>> +old_crc = get_sb(crc);
>> +new_crc = f2fs_cal_crc32(F2FS_SUPER_MAGIC, sb,
>> +SB_CHKSUM_OFFSET);
>> +set_sb(crc, new_crc);
>> +MSG(1, "Info: SB CRC is updated (0x%x -> 0x%x)\n",
>> +old_crc,
>> new_crc);
>> +}
>> +
>> memcpy(buf + F2FS_SUPER_OFFSET, sb, sizeof(*sb));
>> for (index = 0; index < SB_ADDR_MAX; index++) {
>> if ((1 << index) & sb_mask) {
>> @@ -575,10 +589,29 @@ static inline int
>> sanity_check_area_boundary(struct f2fs_super_block *sb,
>> return 0;
>> }
>>
>> +static int verify_sb_chksum(struct f2fs_super_block *sb)
>> +{
>> +if (SB_CHKSUM_OFFSET != get_sb(checksum_offset)) {
>> +MSG(0, "\tInvalid SB CRC offset: %u\n",
>> +get_sb(checksum_offset));
>> +return -1;
>> +}
>> +if (f2fs_crc_valid(get_sb(crc), sb,
>> +get_sb(checksum_offset))) {
>> +MSG(0, "\tInvalid SB CRC: 0x%x\n", get_sb(crc));
>> +return -1;
>> +}
>> +return 0;
>> +}
>> +
>> int sanity_check_raw_super(struct f2fs_super_block *sb, enum SB_ADDR
>> sb_addr)
>> {
>> unsigned int blocksize;
>>
>> +if ((get_sb(feature) & F2FS_FEATURE_SB_CHKSUM) &&
>> +verify_sb_chksum(sb))
>> +return -1;
>> +
>> if (F2FS_SUPER_MAGIC != get_sb(magic))
>> return -1;
>>
>> diff --git a/fsck/resize.c b/fsck/resize.c
>> index 5161a1f..3462165 100644
>> --- a/fsck/resize.c
>> +++ b/fsck/resize.c
>> @@ -603,9 +603,6 @@ static int f2fs_resize_grow(struct f2fs_sb_info *sbi)
>> }
>> }
>>
>> -print_raw_sb_info(sb);
>> -print_raw_sb_info(new_sb);
>
> It'd be worth to keep this to show the previous states.
>
Here, I just want to move the printing of sb and new_sb to the place
behind update_superblock(), where the crc in sb will be updated.
>>>
>>> We'd better to see the changes, no?
>>>
>>
>> Yeah, we print sb and new_sb here just for seeing the changes of superblock.
>> However, the crc in new_sb will not be updated until calling
>> update_superblock(),
>> so if we keep the current printing location, we can't get the changes.
>
> I mean...
> print_raw_sb_info(sb);
> print_raw_sb_info(new_sb);
I don't think it's necessary to print new_sb here.
Before update_superblock() is called, the crc in new_sb hasn't been updated yet,
and is the same with that in sb.
Here, as we introduce sb checksum, all we have to do is delay printing
Re: [f2fs-dev] [PATCH v3] f2fs: submit cached bio to avoid endless PageWriteback
On 09/26, Chao Yu wrote:
> On 2018/9/26 9:42, Jaegeuk Kim wrote:
> > On 09/26, Chao Yu wrote:
> >> On 2018/9/26 8:20, Jaegeuk Kim wrote:
> >>> On 09/21, Chao Yu wrote:
> On 2018/9/18 10:14, Chao Yu wrote:
> > On 2018/9/18 10:02, Jaegeuk Kim wrote:
> >> On 09/18, Chao Yu wrote:
> >>> On 2018/9/18 9:37, Jaegeuk Kim wrote:
> On 09/18, Chao Yu wrote:
> > On 2018/9/18 9:04, Jaegeuk Kim wrote:
> >> On 09/13, Chao Yu wrote:
> >>> From: Chao Yu
> >>>
> >>> When migrating encrypted block from background GC thread, we only
> >>> add
> >>> them into f2fs inner bio cache, but forget to submit the cached
> >>> bio, it
> >>> may cause potential deadlock when we are waiting page
> >>> writebacked, fix
> >>> it.
> >>>
> >>> Signed-off-by: Chao Yu
> >>> ---
> >>> v3:
> >>> clean up codes suggested by Jaegeuk.
> >>> fs/f2fs/f2fs.h | 2 +-
> >>> fs/f2fs/gc.c | 71
> >>> +++---
> >>> fs/f2fs/node.c | 13 ++---
> >>> 3 files changed, 61 insertions(+), 25 deletions(-)
> >>>
> >>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> >>> index b676b82312e0..917b2ca76aac 100644
> >>> --- a/fs/f2fs/f2fs.h
> >>> +++ b/fs/f2fs/f2fs.h
> >>> @@ -2869,7 +2869,7 @@ struct page *f2fs_new_node_page(struct
> >>> dnode_of_data *dn, unsigned int ofs);
> >>> void f2fs_ra_node_page(struct f2fs_sb_info *sbi, nid_t nid);
> >>> struct page *f2fs_get_node_page(struct f2fs_sb_info *sbi,
> >>> pgoff_t nid);
> >>> struct page *f2fs_get_node_page_ra(struct page *parent, int
> >>> start);
> >>> -void f2fs_move_node_page(struct page *node_page, int gc_type);
> >>> +int f2fs_move_node_page(struct page *node_page, int gc_type);
> >>> int f2fs_fsync_node_pages(struct f2fs_sb_info *sbi, struct inode
> >>> *inode,
> >>> struct writeback_control *wbc, bool atomic,
> >>> unsigned int *seq_id);
> >>> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> >>> index a4c1a419611d..f57622cfe058 100644
> >>> --- a/fs/f2fs/gc.c
> >>> +++ b/fs/f2fs/gc.c
> >>> @@ -461,7 +461,7 @@ static int check_valid_map(struct
> >>> f2fs_sb_info *sbi,
> >>> * On validity, copy that node with cold status, otherwise
> >>> (invalid node)
> >>> * ignore that.
> >>> */
> >>> -static void gc_node_segment(struct f2fs_sb_info *sbi,
> >>> +static int gc_node_segment(struct f2fs_sb_info *sbi,
> >>> struct f2fs_summary *sum, unsigned int segno, int
> >>> gc_type)
> >>> {
> >>> struct f2fs_summary *entry;
> >>> @@ -469,6 +469,7 @@ static void gc_node_segment(struct
> >>> f2fs_sb_info *sbi,
> >>> int off;
> >>> int phase = 0;
> >>> bool fggc = (gc_type == FG_GC);
> >>> + int submitted = 0;
> >>>
> >>> start_addr = START_BLOCK(sbi, segno);
> >>>
> >>> @@ -482,10 +483,11 @@ static void gc_node_segment(struct
> >>> f2fs_sb_info *sbi,
> >>> nid_t nid = le32_to_cpu(entry->nid);
> >>> struct page *node_page;
> >>> struct node_info ni;
> >>> + int err;
> >>>
> >>> /* stop BG_GC if there is not enough free sections. */
> >>> if (gc_type == BG_GC && has_not_enough_free_secs(sbi,
> >>> 0, 0))
> >>> - return;
> >>> + return submitted;
> >>>
> >>> if (check_valid_map(sbi, segno, off) == 0)
> >>> continue;
> >>> @@ -522,7 +524,9 @@ static void gc_node_segment(struct
> >>> f2fs_sb_info *sbi,
> >>> continue;
> >>> }
> >>>
> >>> - f2fs_move_node_page(node_page, gc_type);
> >>> + err = f2fs_move_node_page(node_page, gc_type);
> >>> + if (!err && gc_type == FG_GC)
> >>> + submitted++;
> >>> stat_inc_node_blk_count(sbi, 1, gc_type);
> >>> }
> >>>
> >>> @@ -531,6 +535,7 @@ static void gc_node_segment(struct
> >>> f2fs_sb_info *sbi,
> >>>
> >>> if (fggc)
> >>> atomic_dec(>wb_sync_req[NODE]);
> >>> + return submitted;
> >>> }
> >>>
> >>> /*
> >>> @@ -666,7 +671,7 @@ static int ra_data_block(struct inode *inode,
> >>> pgoff_t index)
> >>> * Move data block via META_MAPPING while keeping locked data
> >>> page.
> >>> * This
Re: [f2fs-dev] [PATCH] f2fs: fix quota info to adjust recovered data
On 2018/9/26 10:09, Jaegeuk Kim wrote:
> On 09/26, Chao Yu wrote:
>> On 2018/9/26 9:44, Jaegeuk Kim wrote:
>>> On 09/26, Chao Yu wrote:
On 2018/9/26 8:29, Jaegeuk Kim wrote:
> On 09/21, Chao Yu wrote:
>> On 2018/9/21 5:42, Jaegeuk Kim wrote:
>>> On 09/20, Chao Yu wrote:
On 2018/9/20 6:38, Jaegeuk Kim wrote:
> On 09/19, Chao Yu wrote:
>> On 2018/9/19 0:45, Jaegeuk Kim wrote:
>>> On 09/18, Chao Yu wrote:
On 2018/9/18 10:05, Jaegeuk Kim wrote:
> On 09/18, Chao Yu wrote:
>> On 2018/9/18 9:19, Jaegeuk Kim wrote:
>>> On 09/13, Chao Yu wrote:
On 2018/9/13 3:54, Jaegeuk Kim wrote:
> On 09/12, Chao Yu wrote:
>> On 2018/9/12 9:40, Chao Yu wrote:
>>> On 2018/9/12 9:25, Jaegeuk Kim wrote:
On 09/12, Chao Yu wrote:
> On 2018/9/12 8:27, Jaegeuk Kim wrote:
>> On 09/11, Jaegeuk Kim wrote:
>>> On 09/12, Chao Yu wrote:
On 2018/9/12 4:15, Jaegeuk Kim wrote:
> fsck.f2fs is able to recover the quota structure,
> since roll-forward recovery
> can recover it based on previous user information.
I didn't get it, both fsck and kernel recover quota
file based all inodes'
uid/gid/prjid, if {x}id didn't change, wouldn't those
two recovery result be the
same?
>>>
>>> I thought that, but had to add this, since I was
>>> encountering quota errors right
>>> after getting some files recovered. And, I thought it'd
>>> make it more safe to do
>>> fsck after roll-forward recovery.
>>>
>>> Anyway, let me test again without this patch for a
>>> while.
>>
>> Hmm, I just got a fsck failure right after some files
>> recovered.
>
> To make sure, do you test with "f2fs: guarantee
> journalled quota data by
> checkpoint"? if not, I think there is no guarantee that
> f2fs can recover
> quote info into correct quote file, because, in last
> checkpoint, quota file
> may was corrupted/inconsistent. Right?
>>>
>>> Oh, I forget to mention that, I add a patch to fsck to let
>>> it noticing
>>> CP_QUOTA_NEED_FSCK_FLAG flag, and by default, fsck will fix
>>> corrupted quote
>>> file if the flag is set, but w/o this flag, quota file is
>>> still corrupted
>>> detected by fsck, I guess there is bug in v8.
>>
>> In v8, there are two cases we didn't guarantee quota file's
>> consistence:
>> 1. flush time in block_operation exceed a threshold.
>> 2. dquot subsystem error occurs.
>>
>> For above case, fsck should repair the quota file by default.
>
> Okay, I got another failure and it seems
> CP_QUOTA_NEED_FSCK_FLAG was not set
> during the recovery. So, we have something missing in the
> recovery in terms
> of quota updates.
Yeah, I checked the code, just found one suspected place:
find_fsync_dnodes()
- f2fs_recover_inode_page
- inc_valid_node_count
- dquot_reserve_block dquot info is not initialized now
- add_fsync_inode
- dquot_initialize
I think we should reserve block for inode block after
dquot_initialize(), can
you confirm this?
>>>
>>> Let me test this.
>>>
>>> >From b90260bc577fe87570b1ef7b134554a8295b1f6c Mon Sep 17
>>> >00:00:00 2001
>>> From: Jaegeuk Kim
>>> Date: Mon, 17 Sep 2018 18:14:41 -0700
>>> Subject: [PATCH] f2fs: count inode block for recovered files
>>>
>>> If a new file is recovered, we missed to reserve its inode
>>> block.
>>
>> I remember, in order to keep line with other filesystem, unlike
>> on-disk, we
>> have to keep backward compatibilty, in memory we don't account
>> block number
Re: [f2fs-dev] [PATCH] f2fs: fix quota info to adjust recovered data
On 09/26, Chao Yu wrote:
> On 2018/9/26 9:44, Jaegeuk Kim wrote:
> > On 09/26, Chao Yu wrote:
> >> On 2018/9/26 8:29, Jaegeuk Kim wrote:
> >>> On 09/21, Chao Yu wrote:
> On 2018/9/21 5:42, Jaegeuk Kim wrote:
> > On 09/20, Chao Yu wrote:
> >> On 2018/9/20 6:38, Jaegeuk Kim wrote:
> >>> On 09/19, Chao Yu wrote:
> On 2018/9/19 0:45, Jaegeuk Kim wrote:
> > On 09/18, Chao Yu wrote:
> >> On 2018/9/18 10:05, Jaegeuk Kim wrote:
> >>> On 09/18, Chao Yu wrote:
> On 2018/9/18 9:19, Jaegeuk Kim wrote:
> > On 09/13, Chao Yu wrote:
> >> On 2018/9/13 3:54, Jaegeuk Kim wrote:
> >>> On 09/12, Chao Yu wrote:
> On 2018/9/12 9:40, Chao Yu wrote:
> > On 2018/9/12 9:25, Jaegeuk Kim wrote:
> >> On 09/12, Chao Yu wrote:
> >>> On 2018/9/12 8:27, Jaegeuk Kim wrote:
> On 09/11, Jaegeuk Kim wrote:
> > On 09/12, Chao Yu wrote:
> >> On 2018/9/12 4:15, Jaegeuk Kim wrote:
> >>> fsck.f2fs is able to recover the quota structure,
> >>> since roll-forward recovery
> >>> can recover it based on previous user information.
> >>
> >> I didn't get it, both fsck and kernel recover quota
> >> file based all inodes'
> >> uid/gid/prjid, if {x}id didn't change, wouldn't those
> >> two recovery result be the
> >> same?
> >
> > I thought that, but had to add this, since I was
> > encountering quota errors right
> > after getting some files recovered. And, I thought it'd
> > make it more safe to do
> > fsck after roll-forward recovery.
> >
> > Anyway, let me test again without this patch for a
> > while.
>
> Hmm, I just got a fsck failure right after some files
> recovered.
> >>>
> >>> To make sure, do you test with "f2fs: guarantee
> >>> journalled quota data by
> >>> checkpoint"? if not, I think there is no guarantee that
> >>> f2fs can recover
> >>> quote info into correct quote file, because, in last
> >>> checkpoint, quota file
> >>> may was corrupted/inconsistent. Right?
> >
> > Oh, I forget to mention that, I add a patch to fsck to let
> > it noticing
> > CP_QUOTA_NEED_FSCK_FLAG flag, and by default, fsck will fix
> > corrupted quote
> > file if the flag is set, but w/o this flag, quota file is
> > still corrupted
> > detected by fsck, I guess there is bug in v8.
>
> In v8, there are two cases we didn't guarantee quota file's
> consistence:
> 1. flush time in block_operation exceed a threshold.
> 2. dquot subsystem error occurs.
>
> For above case, fsck should repair the quota file by default.
> >>>
> >>> Okay, I got another failure and it seems
> >>> CP_QUOTA_NEED_FSCK_FLAG was not set
> >>> during the recovery. So, we have something missing in the
> >>> recovery in terms
> >>> of quota updates.
> >>
> >> Yeah, I checked the code, just found one suspected place:
> >>
> >> find_fsync_dnodes()
> >> - f2fs_recover_inode_page
> >> - inc_valid_node_count
> >>- dquot_reserve_block dquot info is not initialized now
> >> - add_fsync_inode
> >> - dquot_initialize
> >>
> >> I think we should reserve block for inode block after
> >> dquot_initialize(), can
> >> you confirm this?
> >
> > Let me test this.
> >
> > >From b90260bc577fe87570b1ef7b134554a8295b1f6c Mon Sep 17
> > >00:00:00 2001
> > From: Jaegeuk Kim
> > Date: Mon, 17 Sep 2018 18:14:41 -0700
> > Subject: [PATCH] f2fs: count inode block for recovered files
> >
> > If a new file is recovered, we missed to reserve its inode
> > block.
>
> I remember, in order to keep line with other filesystem, unlike
> on-disk, we
> have to keep backward compatibilty, in memory we don't account
> block number
> for f2fs' inode block, but only account
Re: [f2fs-dev] [PATCH] f2fs: fix quota info to adjust recovered data
On 2018/9/26 9:44, Jaegeuk Kim wrote:
> On 09/26, Chao Yu wrote:
>> On 2018/9/26 8:29, Jaegeuk Kim wrote:
>>> On 09/21, Chao Yu wrote:
On 2018/9/21 5:42, Jaegeuk Kim wrote:
> On 09/20, Chao Yu wrote:
>> On 2018/9/20 6:38, Jaegeuk Kim wrote:
>>> On 09/19, Chao Yu wrote:
On 2018/9/19 0:45, Jaegeuk Kim wrote:
> On 09/18, Chao Yu wrote:
>> On 2018/9/18 10:05, Jaegeuk Kim wrote:
>>> On 09/18, Chao Yu wrote:
On 2018/9/18 9:19, Jaegeuk Kim wrote:
> On 09/13, Chao Yu wrote:
>> On 2018/9/13 3:54, Jaegeuk Kim wrote:
>>> On 09/12, Chao Yu wrote:
On 2018/9/12 9:40, Chao Yu wrote:
> On 2018/9/12 9:25, Jaegeuk Kim wrote:
>> On 09/12, Chao Yu wrote:
>>> On 2018/9/12 8:27, Jaegeuk Kim wrote:
On 09/11, Jaegeuk Kim wrote:
> On 09/12, Chao Yu wrote:
>> On 2018/9/12 4:15, Jaegeuk Kim wrote:
>>> fsck.f2fs is able to recover the quota structure, since
>>> roll-forward recovery
>>> can recover it based on previous user information.
>>
>> I didn't get it, both fsck and kernel recover quota file
>> based all inodes'
>> uid/gid/prjid, if {x}id didn't change, wouldn't those
>> two recovery result be the
>> same?
>
> I thought that, but had to add this, since I was
> encountering quota errors right
> after getting some files recovered. And, I thought it'd
> make it more safe to do
> fsck after roll-forward recovery.
>
> Anyway, let me test again without this patch for a while.
Hmm, I just got a fsck failure right after some files
recovered.
>>>
>>> To make sure, do you test with "f2fs: guarantee journalled
>>> quota data by
>>> checkpoint"? if not, I think there is no guarantee that
>>> f2fs can recover
>>> quote info into correct quote file, because, in last
>>> checkpoint, quota file
>>> may was corrupted/inconsistent. Right?
>
> Oh, I forget to mention that, I add a patch to fsck to let it
> noticing
> CP_QUOTA_NEED_FSCK_FLAG flag, and by default, fsck will fix
> corrupted quote
> file if the flag is set, but w/o this flag, quota file is
> still corrupted
> detected by fsck, I guess there is bug in v8.
In v8, there are two cases we didn't guarantee quota file's
consistence:
1. flush time in block_operation exceed a threshold.
2. dquot subsystem error occurs.
For above case, fsck should repair the quota file by default.
>>>
>>> Okay, I got another failure and it seems
>>> CP_QUOTA_NEED_FSCK_FLAG was not set
>>> during the recovery. So, we have something missing in the
>>> recovery in terms
>>> of quota updates.
>>
>> Yeah, I checked the code, just found one suspected place:
>>
>> find_fsync_dnodes()
>> - f2fs_recover_inode_page
>> - inc_valid_node_count
>>- dquot_reserve_block dquot info is not initialized now
>> - add_fsync_inode
>> - dquot_initialize
>>
>> I think we should reserve block for inode block after
>> dquot_initialize(), can
>> you confirm this?
>
> Let me test this.
>
> >From b90260bc577fe87570b1ef7b134554a8295b1f6c Mon Sep 17
> >00:00:00 2001
> From: Jaegeuk Kim
> Date: Mon, 17 Sep 2018 18:14:41 -0700
> Subject: [PATCH] f2fs: count inode block for recovered files
>
> If a new file is recovered, we missed to reserve its inode block.
I remember, in order to keep line with other filesystem, unlike
on-disk, we
have to keep backward compatibilty, in memory we don't account
block number
for f2fs' inode block, but only account inode number for it, so
here like
we did in inc_valid_node_count(), we don't need to do this.
>>>
>>> Okay, I just hit the error again w/o your patch. Another one coming
>>> to my mind
>>> is that caused by
Re: [f2fs-dev] [PATCH v3] f2fs: submit cached bio to avoid endless PageWriteback
On 2018/9/26 9:42, Jaegeuk Kim wrote:
> On 09/26, Chao Yu wrote:
>> On 2018/9/26 8:20, Jaegeuk Kim wrote:
>>> On 09/21, Chao Yu wrote:
On 2018/9/18 10:14, Chao Yu wrote:
> On 2018/9/18 10:02, Jaegeuk Kim wrote:
>> On 09/18, Chao Yu wrote:
>>> On 2018/9/18 9:37, Jaegeuk Kim wrote:
On 09/18, Chao Yu wrote:
> On 2018/9/18 9:04, Jaegeuk Kim wrote:
>> On 09/13, Chao Yu wrote:
>>> From: Chao Yu
>>>
>>> When migrating encrypted block from background GC thread, we only
>>> add
>>> them into f2fs inner bio cache, but forget to submit the cached
>>> bio, it
>>> may cause potential deadlock when we are waiting page writebacked,
>>> fix
>>> it.
>>>
>>> Signed-off-by: Chao Yu
>>> ---
>>> v3:
>>> clean up codes suggested by Jaegeuk.
>>> fs/f2fs/f2fs.h | 2 +-
>>> fs/f2fs/gc.c | 71
>>> +++---
>>> fs/f2fs/node.c | 13 ++---
>>> 3 files changed, 61 insertions(+), 25 deletions(-)
>>>
>>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>>> index b676b82312e0..917b2ca76aac 100644
>>> --- a/fs/f2fs/f2fs.h
>>> +++ b/fs/f2fs/f2fs.h
>>> @@ -2869,7 +2869,7 @@ struct page *f2fs_new_node_page(struct
>>> dnode_of_data *dn, unsigned int ofs);
>>> void f2fs_ra_node_page(struct f2fs_sb_info *sbi, nid_t nid);
>>> struct page *f2fs_get_node_page(struct f2fs_sb_info *sbi, pgoff_t
>>> nid);
>>> struct page *f2fs_get_node_page_ra(struct page *parent, int start);
>>> -void f2fs_move_node_page(struct page *node_page, int gc_type);
>>> +int f2fs_move_node_page(struct page *node_page, int gc_type);
>>> int f2fs_fsync_node_pages(struct f2fs_sb_info *sbi, struct inode
>>> *inode,
>>> struct writeback_control *wbc, bool atomic,
>>> unsigned int *seq_id);
>>> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
>>> index a4c1a419611d..f57622cfe058 100644
>>> --- a/fs/f2fs/gc.c
>>> +++ b/fs/f2fs/gc.c
>>> @@ -461,7 +461,7 @@ static int check_valid_map(struct f2fs_sb_info
>>> *sbi,
>>> * On validity, copy that node with cold status, otherwise
>>> (invalid node)
>>> * ignore that.
>>> */
>>> -static void gc_node_segment(struct f2fs_sb_info *sbi,
>>> +static int gc_node_segment(struct f2fs_sb_info *sbi,
>>> struct f2fs_summary *sum, unsigned int segno, int
>>> gc_type)
>>> {
>>> struct f2fs_summary *entry;
>>> @@ -469,6 +469,7 @@ static void gc_node_segment(struct f2fs_sb_info
>>> *sbi,
>>> int off;
>>> int phase = 0;
>>> bool fggc = (gc_type == FG_GC);
>>> + int submitted = 0;
>>>
>>> start_addr = START_BLOCK(sbi, segno);
>>>
>>> @@ -482,10 +483,11 @@ static void gc_node_segment(struct
>>> f2fs_sb_info *sbi,
>>> nid_t nid = le32_to_cpu(entry->nid);
>>> struct page *node_page;
>>> struct node_info ni;
>>> + int err;
>>>
>>> /* stop BG_GC if there is not enough free sections. */
>>> if (gc_type == BG_GC && has_not_enough_free_secs(sbi,
>>> 0, 0))
>>> - return;
>>> + return submitted;
>>>
>>> if (check_valid_map(sbi, segno, off) == 0)
>>> continue;
>>> @@ -522,7 +524,9 @@ static void gc_node_segment(struct f2fs_sb_info
>>> *sbi,
>>> continue;
>>> }
>>>
>>> - f2fs_move_node_page(node_page, gc_type);
>>> + err = f2fs_move_node_page(node_page, gc_type);
>>> + if (!err && gc_type == FG_GC)
>>> + submitted++;
>>> stat_inc_node_blk_count(sbi, 1, gc_type);
>>> }
>>>
>>> @@ -531,6 +535,7 @@ static void gc_node_segment(struct f2fs_sb_info
>>> *sbi,
>>>
>>> if (fggc)
>>> atomic_dec(>wb_sync_req[NODE]);
>>> + return submitted;
>>> }
>>>
>>> /*
>>> @@ -666,7 +671,7 @@ static int ra_data_block(struct inode *inode,
>>> pgoff_t index)
>>> * Move data block via META_MAPPING while keeping locked data page.
>>> * This can be used to move blocks, aka LBAs, directly on disk.
>>> */
>>> -static void move_data_block(struct inode *inode, block_t bidx,
>>> +static int move_data_block(struct inode *inode, block_t
Re: [f2fs-dev] [PATCH 4/5] f2fs-tools: introduce sb checksum
On 09/21, Junling Zheng wrote:
> On 2018/9/21 5:38, Jaegeuk Kim wrote:
> > On 09/20, Junling Zheng wrote:
> >> Hi, Jaegeuk
> >>
> >> On 2018/9/20 7:35, Jaegeuk Kim wrote:
> >>> On 09/19, Junling Zheng wrote:
> This patch introduced crc for superblock.
>
> Signed-off-by: Junling Zheng
> ---
> fsck/mount.c | 33 +
> fsck/resize.c | 12 ++--
> include/f2fs_fs.h | 6 +-
> mkfs/f2fs_format.c | 8
> 4 files changed, 52 insertions(+), 7 deletions(-)
>
> diff --git a/fsck/mount.c b/fsck/mount.c
> index 74ff7c6..9019921 100644
> --- a/fsck/mount.c
> +++ b/fsck/mount.c
> @@ -340,6 +340,7 @@ void print_raw_sb_info(struct f2fs_super_block *sb)
> DISP_u32(sb, node_ino);
> DISP_u32(sb, meta_ino);
> DISP_u32(sb, cp_payload);
> +DISP_u32(sb, crc);
> DISP("%-.256s", sb, version);
> printf("\n");
> }
> @@ -467,6 +468,9 @@ void print_sb_state(struct f2fs_super_block *sb)
> if (f & cpu_to_le32(F2FS_FEATURE_LOST_FOUND)) {
> MSG(0, "%s", " lost_found");
> }
> +if (f & cpu_to_le32(F2FS_FEATURE_SB_CHKSUM)) {
> +MSG(0, "%s", " sb_checksum");
> +}
> MSG(0, "\n");
> MSG(0, "Info: superblock encrypt level = %d, salt = ",
> sb->encryption_level);
> @@ -479,10 +483,20 @@ void update_superblock(struct f2fs_super_block
> *sb, int sb_mask)
> {
> int index, ret;
> u_int8_t *buf;
> +u32 old_crc, new_crc;
>
> buf = calloc(BLOCK_SZ, 1);
> ASSERT(buf);
>
> +if (get_sb(feature) & F2FS_FEATURE_SB_CHKSUM) {
> +old_crc = get_sb(crc);
> +new_crc = f2fs_cal_crc32(F2FS_SUPER_MAGIC, sb,
> +SB_CHKSUM_OFFSET);
> +set_sb(crc, new_crc);
> +MSG(1, "Info: SB CRC is updated (0x%x -> 0x%x)\n",
> +old_crc,
> new_crc);
> +}
> +
> memcpy(buf + F2FS_SUPER_OFFSET, sb, sizeof(*sb));
> for (index = 0; index < SB_ADDR_MAX; index++) {
> if ((1 << index) & sb_mask) {
> @@ -575,10 +589,29 @@ static inline int
> sanity_check_area_boundary(struct f2fs_super_block *sb,
> return 0;
> }
>
> +static int verify_sb_chksum(struct f2fs_super_block *sb)
> +{
> +if (SB_CHKSUM_OFFSET != get_sb(checksum_offset)) {
> +MSG(0, "\tInvalid SB CRC offset: %u\n",
> +get_sb(checksum_offset));
> +return -1;
> +}
> +if (f2fs_crc_valid(get_sb(crc), sb,
> +get_sb(checksum_offset))) {
> +MSG(0, "\tInvalid SB CRC: 0x%x\n", get_sb(crc));
> +return -1;
> +}
> +return 0;
> +}
> +
> int sanity_check_raw_super(struct f2fs_super_block *sb, enum SB_ADDR
> sb_addr)
> {
> unsigned int blocksize;
>
> +if ((get_sb(feature) & F2FS_FEATURE_SB_CHKSUM) &&
> +verify_sb_chksum(sb))
> +return -1;
> +
> if (F2FS_SUPER_MAGIC != get_sb(magic))
> return -1;
>
> diff --git a/fsck/resize.c b/fsck/resize.c
> index 5161a1f..3462165 100644
> --- a/fsck/resize.c
> +++ b/fsck/resize.c
> @@ -603,9 +603,6 @@ static int f2fs_resize_grow(struct f2fs_sb_info *sbi)
> }
> }
>
> -print_raw_sb_info(sb);
> -print_raw_sb_info(new_sb);
> >>>
> >>> It'd be worth to keep this to show the previous states.
> >>>
> >>
> >> Here, I just want to move the printing of sb and new_sb to the place
> >> behind update_superblock(), where the crc in sb will be updated.
> >
> > We'd better to see the changes, no?
> >
>
> Yeah, we print sb and new_sb here just for seeing the changes of superblock.
> However, the crc in new_sb will not be updated until calling
> update_superblock(),
> so if we keep the current printing location, we can't get the changes.
I mean...
print_raw_sb_info(sb);
print_raw_sb_info(new_sb);
update...
print_raw_sb_info(new_sb);
>
> Thanks,
>
> >>
> -
> old_main_blkaddr = get_sb(main_blkaddr);
> new_main_blkaddr = get_newsb(main_blkaddr);
> offset = new_main_blkaddr - old_main_blkaddr;
> @@ -629,6
[f2fs-dev] [PATCH 1/2] f2fs: clear PageError on the read path
When running fault injection test, I hit somewhat wrong behavior in f2fs_gc ->
gc_data_segment():
0. fault injection generated some PageError'ed pages
1. gc_data_segment
-> f2fs_get_read_data_page(REQ_RAHEAD)
2. move_data_page
-> f2fs_get_lock_data_page()
-> f2f_get_read_data_page()
-> f2fs_submit_page_read()
-> submit_bio(READ)
-> return EIO due to PageError
-> fail to move data
Signed-off-by: Jaegeuk Kim
---
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 41102c227eee..cb3ebcae0398 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -77,7 +77,8 @@ static void __read_end_io(struct bio *bio)
/* PG_error was set if any post_read step failed */
if (bio->bi_status || PageError(page)) {
ClearPageUptodate(page);
- SetPageError(page);
+ /* will re-read again later */
+ ClearPageError(page);
} else {
SetPageUptodate(page);
}
@@ -591,6 +592,7 @@ static int f2fs_submit_page_read(struct inode *inode,
struct page *page,
bio_put(bio);
return -EFAULT;
}
+ ClearPageError(page);
__submit_bio(F2FS_I_SB(inode), bio, DATA);
return 0;
}
@@ -1571,6 +1573,7 @@ static int f2fs_mpage_readpages(struct address_space
*mapping,
if (bio_add_page(bio, page, blocksize, 0) < blocksize)
goto submit_and_realloc;
+ ClearPageError(page);
last_block_in_bio = block_nr;
goto next_page;
set_error_page:
--
2.17.0.441.gb46fe60e1d-goog
___
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: return correct errno in f2fs_gc
This fixes overriding error number in f2fs_gc. Signed-off-by: Jaegeuk Kim --- fs/f2fs/gc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c index 77ffa8045a3b..c051dc4ddf1a 100644 --- a/fs/f2fs/gc.c +++ b/fs/f2fs/gc.c @@ -1241,7 +1241,7 @@ int f2fs_gc(struct f2fs_sb_info *sbi, bool sync, put_gc_inode(_list); - if (sync) + if (sync && !ret) ret = sec_freed ? 0 : -EAGAIN; return ret; } -- 2.17.0.441.gb46fe60e1d-goog ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH] f2fs: fix quota info to adjust recovered data
On 09/26, Chao Yu wrote:
> On 2018/9/26 8:29, Jaegeuk Kim wrote:
> > On 09/21, Chao Yu wrote:
> >> On 2018/9/21 5:42, Jaegeuk Kim wrote:
> >>> On 09/20, Chao Yu wrote:
> On 2018/9/20 6:38, Jaegeuk Kim wrote:
> > On 09/19, Chao Yu wrote:
> >> On 2018/9/19 0:45, Jaegeuk Kim wrote:
> >>> On 09/18, Chao Yu wrote:
> On 2018/9/18 10:05, Jaegeuk Kim wrote:
> > On 09/18, Chao Yu wrote:
> >> On 2018/9/18 9:19, Jaegeuk Kim wrote:
> >>> On 09/13, Chao Yu wrote:
> On 2018/9/13 3:54, Jaegeuk Kim wrote:
> > On 09/12, Chao Yu wrote:
> >> On 2018/9/12 9:40, Chao Yu wrote:
> >>> On 2018/9/12 9:25, Jaegeuk Kim wrote:
> On 09/12, Chao Yu wrote:
> > On 2018/9/12 8:27, Jaegeuk Kim wrote:
> >> On 09/11, Jaegeuk Kim wrote:
> >>> On 09/12, Chao Yu wrote:
> On 2018/9/12 4:15, Jaegeuk Kim wrote:
> > fsck.f2fs is able to recover the quota structure, since
> > roll-forward recovery
> > can recover it based on previous user information.
>
> I didn't get it, both fsck and kernel recover quota file
> based all inodes'
> uid/gid/prjid, if {x}id didn't change, wouldn't those
> two recovery result be the
> same?
> >>>
> >>> I thought that, but had to add this, since I was
> >>> encountering quota errors right
> >>> after getting some files recovered. And, I thought it'd
> >>> make it more safe to do
> >>> fsck after roll-forward recovery.
> >>>
> >>> Anyway, let me test again without this patch for a while.
> >>
> >> Hmm, I just got a fsck failure right after some files
> >> recovered.
> >
> > To make sure, do you test with "f2fs: guarantee journalled
> > quota data by
> > checkpoint"? if not, I think there is no guarantee that
> > f2fs can recover
> > quote info into correct quote file, because, in last
> > checkpoint, quota file
> > may was corrupted/inconsistent. Right?
> >>>
> >>> Oh, I forget to mention that, I add a patch to fsck to let it
> >>> noticing
> >>> CP_QUOTA_NEED_FSCK_FLAG flag, and by default, fsck will fix
> >>> corrupted quote
> >>> file if the flag is set, but w/o this flag, quota file is
> >>> still corrupted
> >>> detected by fsck, I guess there is bug in v8.
> >>
> >> In v8, there are two cases we didn't guarantee quota file's
> >> consistence:
> >> 1. flush time in block_operation exceed a threshold.
> >> 2. dquot subsystem error occurs.
> >>
> >> For above case, fsck should repair the quota file by default.
> >
> > Okay, I got another failure and it seems
> > CP_QUOTA_NEED_FSCK_FLAG was not set
> > during the recovery. So, we have something missing in the
> > recovery in terms
> > of quota updates.
>
> Yeah, I checked the code, just found one suspected place:
>
> find_fsync_dnodes()
> - f2fs_recover_inode_page
> - inc_valid_node_count
> - dquot_reserve_block dquot info is not initialized now
> - add_fsync_inode
> - dquot_initialize
>
> I think we should reserve block for inode block after
> dquot_initialize(), can
> you confirm this?
> >>>
> >>> Let me test this.
> >>>
> >>> >From b90260bc577fe87570b1ef7b134554a8295b1f6c Mon Sep 17
> >>> >00:00:00 2001
> >>> From: Jaegeuk Kim
> >>> Date: Mon, 17 Sep 2018 18:14:41 -0700
> >>> Subject: [PATCH] f2fs: count inode block for recovered files
> >>>
> >>> If a new file is recovered, we missed to reserve its inode block.
> >>
> >> I remember, in order to keep line with other filesystem, unlike
> >> on-disk, we
> >> have to keep backward compatibilty, in memory we don't account
> >> block number
> >> for f2fs' inode block, but only account inode number for it, so
> >> here like
> >> we did in inc_valid_node_count(), we don't need to do this.
> >
> > Okay, I just hit the error again w/o your patch. Another one coming
> > to my mind
> > is that caused by uid/gid change during recovery. Let me try
Re: [f2fs-dev] [PATCH v3] f2fs: submit cached bio to avoid endless PageWriteback
On 09/26, Chao Yu wrote:
> On 2018/9/26 8:20, Jaegeuk Kim wrote:
> > On 09/21, Chao Yu wrote:
> >> On 2018/9/18 10:14, Chao Yu wrote:
> >>> On 2018/9/18 10:02, Jaegeuk Kim wrote:
> On 09/18, Chao Yu wrote:
> > On 2018/9/18 9:37, Jaegeuk Kim wrote:
> >> On 09/18, Chao Yu wrote:
> >>> On 2018/9/18 9:04, Jaegeuk Kim wrote:
> On 09/13, Chao Yu wrote:
> > From: Chao Yu
> >
> > When migrating encrypted block from background GC thread, we only
> > add
> > them into f2fs inner bio cache, but forget to submit the cached
> > bio, it
> > may cause potential deadlock when we are waiting page writebacked,
> > fix
> > it.
> >
> > Signed-off-by: Chao Yu
> > ---
> > v3:
> > clean up codes suggested by Jaegeuk.
> > fs/f2fs/f2fs.h | 2 +-
> > fs/f2fs/gc.c | 71
> > +++---
> > fs/f2fs/node.c | 13 ++---
> > 3 files changed, 61 insertions(+), 25 deletions(-)
> >
> > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > index b676b82312e0..917b2ca76aac 100644
> > --- a/fs/f2fs/f2fs.h
> > +++ b/fs/f2fs/f2fs.h
> > @@ -2869,7 +2869,7 @@ struct page *f2fs_new_node_page(struct
> > dnode_of_data *dn, unsigned int ofs);
> > void f2fs_ra_node_page(struct f2fs_sb_info *sbi, nid_t nid);
> > struct page *f2fs_get_node_page(struct f2fs_sb_info *sbi, pgoff_t
> > nid);
> > struct page *f2fs_get_node_page_ra(struct page *parent, int start);
> > -void f2fs_move_node_page(struct page *node_page, int gc_type);
> > +int f2fs_move_node_page(struct page *node_page, int gc_type);
> > int f2fs_fsync_node_pages(struct f2fs_sb_info *sbi, struct inode
> > *inode,
> > struct writeback_control *wbc, bool atomic,
> > unsigned int *seq_id);
> > diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> > index a4c1a419611d..f57622cfe058 100644
> > --- a/fs/f2fs/gc.c
> > +++ b/fs/f2fs/gc.c
> > @@ -461,7 +461,7 @@ static int check_valid_map(struct f2fs_sb_info
> > *sbi,
> > * On validity, copy that node with cold status, otherwise
> > (invalid node)
> > * ignore that.
> > */
> > -static void gc_node_segment(struct f2fs_sb_info *sbi,
> > +static int gc_node_segment(struct f2fs_sb_info *sbi,
> > struct f2fs_summary *sum, unsigned int segno, int
> > gc_type)
> > {
> > struct f2fs_summary *entry;
> > @@ -469,6 +469,7 @@ static void gc_node_segment(struct f2fs_sb_info
> > *sbi,
> > int off;
> > int phase = 0;
> > bool fggc = (gc_type == FG_GC);
> > + int submitted = 0;
> >
> > start_addr = START_BLOCK(sbi, segno);
> >
> > @@ -482,10 +483,11 @@ static void gc_node_segment(struct
> > f2fs_sb_info *sbi,
> > nid_t nid = le32_to_cpu(entry->nid);
> > struct page *node_page;
> > struct node_info ni;
> > + int err;
> >
> > /* stop BG_GC if there is not enough free sections. */
> > if (gc_type == BG_GC && has_not_enough_free_secs(sbi,
> > 0, 0))
> > - return;
> > + return submitted;
> >
> > if (check_valid_map(sbi, segno, off) == 0)
> > continue;
> > @@ -522,7 +524,9 @@ static void gc_node_segment(struct f2fs_sb_info
> > *sbi,
> > continue;
> > }
> >
> > - f2fs_move_node_page(node_page, gc_type);
> > + err = f2fs_move_node_page(node_page, gc_type);
> > + if (!err && gc_type == FG_GC)
> > + submitted++;
> > stat_inc_node_blk_count(sbi, 1, gc_type);
> > }
> >
> > @@ -531,6 +535,7 @@ static void gc_node_segment(struct f2fs_sb_info
> > *sbi,
> >
> > if (fggc)
> > atomic_dec(>wb_sync_req[NODE]);
> > + return submitted;
> > }
> >
> > /*
> > @@ -666,7 +671,7 @@ static int ra_data_block(struct inode *inode,
> > pgoff_t index)
> > * Move data block via META_MAPPING while keeping locked data page.
> > * This can be used to move blocks, aka LBAs, directly on disk.
> > */
> > -static void move_data_block(struct inode *inode, block_t bidx,
> > +static int move_data_block(struct inode *inode, block_t bidx,
> >
Re: [f2fs-dev] [PATCH] f2fs: fix quota info to adjust recovered data
On 2018/9/26 8:29, Jaegeuk Kim wrote:
> On 09/21, Chao Yu wrote:
>> On 2018/9/21 5:42, Jaegeuk Kim wrote:
>>> On 09/20, Chao Yu wrote:
On 2018/9/20 6:38, Jaegeuk Kim wrote:
> On 09/19, Chao Yu wrote:
>> On 2018/9/19 0:45, Jaegeuk Kim wrote:
>>> On 09/18, Chao Yu wrote:
On 2018/9/18 10:05, Jaegeuk Kim wrote:
> On 09/18, Chao Yu wrote:
>> On 2018/9/18 9:19, Jaegeuk Kim wrote:
>>> On 09/13, Chao Yu wrote:
On 2018/9/13 3:54, Jaegeuk Kim wrote:
> On 09/12, Chao Yu wrote:
>> On 2018/9/12 9:40, Chao Yu wrote:
>>> On 2018/9/12 9:25, Jaegeuk Kim wrote:
On 09/12, Chao Yu wrote:
> On 2018/9/12 8:27, Jaegeuk Kim wrote:
>> On 09/11, Jaegeuk Kim wrote:
>>> On 09/12, Chao Yu wrote:
On 2018/9/12 4:15, Jaegeuk Kim wrote:
> fsck.f2fs is able to recover the quota structure, since
> roll-forward recovery
> can recover it based on previous user information.
I didn't get it, both fsck and kernel recover quota file
based all inodes'
uid/gid/prjid, if {x}id didn't change, wouldn't those two
recovery result be the
same?
>>>
>>> I thought that, but had to add this, since I was
>>> encountering quota errors right
>>> after getting some files recovered. And, I thought it'd
>>> make it more safe to do
>>> fsck after roll-forward recovery.
>>>
>>> Anyway, let me test again without this patch for a while.
>>
>> Hmm, I just got a fsck failure right after some files
>> recovered.
>
> To make sure, do you test with "f2fs: guarantee journalled
> quota data by
> checkpoint"? if not, I think there is no guarantee that f2fs
> can recover
> quote info into correct quote file, because, in last
> checkpoint, quota file
> may was corrupted/inconsistent. Right?
>>>
>>> Oh, I forget to mention that, I add a patch to fsck to let it
>>> noticing
>>> CP_QUOTA_NEED_FSCK_FLAG flag, and by default, fsck will fix
>>> corrupted quote
>>> file if the flag is set, but w/o this flag, quota file is still
>>> corrupted
>>> detected by fsck, I guess there is bug in v8.
>>
>> In v8, there are two cases we didn't guarantee quota file's
>> consistence:
>> 1. flush time in block_operation exceed a threshold.
>> 2. dquot subsystem error occurs.
>>
>> For above case, fsck should repair the quota file by default.
>
> Okay, I got another failure and it seems CP_QUOTA_NEED_FSCK_FLAG
> was not set
> during the recovery. So, we have something missing in the
> recovery in terms
> of quota updates.
Yeah, I checked the code, just found one suspected place:
find_fsync_dnodes()
- f2fs_recover_inode_page
- inc_valid_node_count
- dquot_reserve_block dquot info is not initialized now
- add_fsync_inode
- dquot_initialize
I think we should reserve block for inode block after
dquot_initialize(), can
you confirm this?
>>>
>>> Let me test this.
>>>
>>> >From b90260bc577fe87570b1ef7b134554a8295b1f6c Mon Sep 17 00:00:00
>>> >2001
>>> From: Jaegeuk Kim
>>> Date: Mon, 17 Sep 2018 18:14:41 -0700
>>> Subject: [PATCH] f2fs: count inode block for recovered files
>>>
>>> If a new file is recovered, we missed to reserve its inode block.
>>
>> I remember, in order to keep line with other filesystem, unlike
>> on-disk, we
>> have to keep backward compatibilty, in memory we don't account block
>> number
>> for f2fs' inode block, but only account inode number for it, so here
>> like
>> we did in inc_valid_node_count(), we don't need to do this.
>
> Okay, I just hit the error again w/o your patch. Another one coming
> to my mind
> is that caused by uid/gid change during recovery. Let me try out your
> patch.
I guess we should update dquot and inode's uid/gid atomically under
lock_op() in f2fs_setattr() to prevent corruption on sys quota file.
v9 can pass all xfstest
Re: [f2fs-dev] [PATCH v3] f2fs: submit cached bio to avoid endless PageWriteback
On 2018/9/26 8:20, Jaegeuk Kim wrote:
> On 09/21, Chao Yu wrote:
>> On 2018/9/18 10:14, Chao Yu wrote:
>>> On 2018/9/18 10:02, Jaegeuk Kim wrote:
On 09/18, Chao Yu wrote:
> On 2018/9/18 9:37, Jaegeuk Kim wrote:
>> On 09/18, Chao Yu wrote:
>>> On 2018/9/18 9:04, Jaegeuk Kim wrote:
On 09/13, Chao Yu wrote:
> From: Chao Yu
>
> When migrating encrypted block from background GC thread, we only add
> them into f2fs inner bio cache, but forget to submit the cached bio,
> it
> may cause potential deadlock when we are waiting page writebacked, fix
> it.
>
> Signed-off-by: Chao Yu
> ---
> v3:
> clean up codes suggested by Jaegeuk.
> fs/f2fs/f2fs.h | 2 +-
> fs/f2fs/gc.c | 71
> +++---
> fs/f2fs/node.c | 13 ++---
> 3 files changed, 61 insertions(+), 25 deletions(-)
>
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index b676b82312e0..917b2ca76aac 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -2869,7 +2869,7 @@ struct page *f2fs_new_node_page(struct
> dnode_of_data *dn, unsigned int ofs);
> void f2fs_ra_node_page(struct f2fs_sb_info *sbi, nid_t nid);
> struct page *f2fs_get_node_page(struct f2fs_sb_info *sbi, pgoff_t
> nid);
> struct page *f2fs_get_node_page_ra(struct page *parent, int start);
> -void f2fs_move_node_page(struct page *node_page, int gc_type);
> +int f2fs_move_node_page(struct page *node_page, int gc_type);
> int f2fs_fsync_node_pages(struct f2fs_sb_info *sbi, struct inode
> *inode,
> struct writeback_control *wbc, bool atomic,
> unsigned int *seq_id);
> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> index a4c1a419611d..f57622cfe058 100644
> --- a/fs/f2fs/gc.c
> +++ b/fs/f2fs/gc.c
> @@ -461,7 +461,7 @@ static int check_valid_map(struct f2fs_sb_info
> *sbi,
> * On validity, copy that node with cold status, otherwise (invalid
> node)
> * ignore that.
> */
> -static void gc_node_segment(struct f2fs_sb_info *sbi,
> +static int gc_node_segment(struct f2fs_sb_info *sbi,
> struct f2fs_summary *sum, unsigned int segno, int
> gc_type)
> {
> struct f2fs_summary *entry;
> @@ -469,6 +469,7 @@ static void gc_node_segment(struct f2fs_sb_info
> *sbi,
> int off;
> int phase = 0;
> bool fggc = (gc_type == FG_GC);
> + int submitted = 0;
>
> start_addr = START_BLOCK(sbi, segno);
>
> @@ -482,10 +483,11 @@ static void gc_node_segment(struct f2fs_sb_info
> *sbi,
> nid_t nid = le32_to_cpu(entry->nid);
> struct page *node_page;
> struct node_info ni;
> + int err;
>
> /* stop BG_GC if there is not enough free sections. */
> if (gc_type == BG_GC && has_not_enough_free_secs(sbi,
> 0, 0))
> - return;
> + return submitted;
>
> if (check_valid_map(sbi, segno, off) == 0)
> continue;
> @@ -522,7 +524,9 @@ static void gc_node_segment(struct f2fs_sb_info
> *sbi,
> continue;
> }
>
> - f2fs_move_node_page(node_page, gc_type);
> + err = f2fs_move_node_page(node_page, gc_type);
> + if (!err && gc_type == FG_GC)
> + submitted++;
> stat_inc_node_blk_count(sbi, 1, gc_type);
> }
>
> @@ -531,6 +535,7 @@ static void gc_node_segment(struct f2fs_sb_info
> *sbi,
>
> if (fggc)
> atomic_dec(>wb_sync_req[NODE]);
> + return submitted;
> }
>
> /*
> @@ -666,7 +671,7 @@ static int ra_data_block(struct inode *inode,
> pgoff_t index)
> * Move data block via META_MAPPING while keeping locked data page.
> * This can be used to move blocks, aka LBAs, directly on disk.
> */
> -static void move_data_block(struct inode *inode, block_t bidx,
> +static int move_data_block(struct inode *inode, block_t bidx,
> int gc_type, unsigned int segno, int
> off)
We don't need to submit IOs in this case.
>>>
>>> Actually, previously, we missed to submit IOs for encrypted block only
>>>
Re: [f2fs-dev] [PATCH 2/2 v2] f2fs: avoid f2fs_bug_on if f2fs_get_meta_page_nofail got EIO
On 2018/9/26 8:18, Jaegeuk Kim wrote:
> On 09/21, Chao Yu wrote:
>> On 2018/9/21 5:46, Jaegeuk Kim wrote:
>>> On 09/20, Chao Yu wrote:
On 2018/9/19 1:56, Jaegeuk Kim wrote:
> This patch avoids BUG_ON when f2fs_get_meta_page_nofail got EIO during
> xfstests/generic/475.
>
> Signed-off-by: Jaegeuk Kim
> ---
> Change log from v1:
> - propagate errno
> - drop sum_pages
>
> fs/f2fs/checkpoint.c | 10 +-
> fs/f2fs/f2fs.h | 2 +-
> fs/f2fs/gc.c | 9 +
> fs/f2fs/node.c | 32
> fs/f2fs/recovery.c | 2 ++
> fs/f2fs/segment.c| 3 +++
> 6 files changed, 44 insertions(+), 14 deletions(-)
>
> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
> index 01e0d8f5bbbe..1ddf332ce4b2 100644
> --- a/fs/f2fs/checkpoint.c
> +++ b/fs/f2fs/checkpoint.c
> @@ -119,11 +119,8 @@ struct page *f2fs_get_meta_page_nofail(struct
> f2fs_sb_info *sbi, pgoff_t index)
> if (PTR_ERR(page) == -EIO &&
> ++count <= DEFAULT_RETRY_IO_COUNT)
> goto retry;
> -
> f2fs_stop_checkpoint(sbi, false);
> - f2fs_bug_on(sbi, 1);
> }
> -
> return page;
> }
>
> @@ -1512,7 +1509,10 @@ int f2fs_write_checkpoint(struct f2fs_sb_info
> *sbi, struct cp_control *cpc)
> ckpt->checkpoint_ver = cpu_to_le64(++ckpt_ver);
>
> /* write cached NAT/SIT entries to NAT/SIT area */
> - f2fs_flush_nat_entries(sbi, cpc);
> + err = f2fs_flush_nat_entries(sbi, cpc);
> + if (err)
> + goto stop;
To make sure, if partial NAT pages become dirty, flush them back to device
outside checkpoint() is not harmful, right?
>>>
>>> I think so.
>>
>> Oh, one more case, now we use NAT #0, if partial NAT pages were
>> writebacked, then we start to use NAT #1, later, in another checkpoint(),
>> we update those NAT pages again, we will write them to NAT #0, which is
>> belong to last valid checkpoint(), it's may cause corrupted checkpoint in
>> scenario of SPO. So it's harmfull, right?
>
> We already stopped checkpoint anymore, so there'd be no way to proceed another
> checkpoint. Am I missing something?
You're right, we have called f2fs_stop_checkpoint(), so we are safe now. :)
Thanks,
>
>>
>> Thanks,
>>
>>>
> +
> f2fs_flush_sit_entries(sbi, cpc);
>
> /* unlock all the fs_lock[] in do_checkpoint() */
> @@ -1521,7 +1521,7 @@ int f2fs_write_checkpoint(struct f2fs_sb_info *sbi,
> struct cp_control *cpc)
> f2fs_release_discard_addrs(sbi);
> else
> f2fs_clear_prefree_segments(sbi, cpc);
> -
> +stop:
> unblock_operations(sbi);
> stat_inc_cp_count(sbi->stat_info);
>
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index a0c868127a7c..29021dbc8f2a 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -2895,7 +2895,7 @@ int f2fs_recover_xattr_data(struct inode *inode,
> struct page *page);
> int f2fs_recover_inode_page(struct f2fs_sb_info *sbi, struct page *page);
> int f2fs_restore_node_summary(struct f2fs_sb_info *sbi,
> unsigned int segno, struct f2fs_summary_block *sum);
> -void f2fs_flush_nat_entries(struct f2fs_sb_info *sbi, struct cp_control
> *cpc);
> +int f2fs_flush_nat_entries(struct f2fs_sb_info *sbi, struct cp_control
> *cpc);
> int f2fs_build_node_manager(struct f2fs_sb_info *sbi);
> void f2fs_destroy_node_manager(struct f2fs_sb_info *sbi);
> int __init f2fs_create_node_manager_caches(void);
> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> index 4bcc8a59fdef..c7d14282dbf4 100644
> --- a/fs/f2fs/gc.c
> +++ b/fs/f2fs/gc.c
> @@ -1070,6 +1070,15 @@ static int do_garbage_collect(struct f2fs_sb_info
> *sbi,
> /* reference all summary page */
> while (segno < end_segno) {
> sum_page = f2fs_get_sum_page(sbi, segno++);
> + if (IS_ERR(sum_page)) {
> + end_segno = segno - 1;
> + for (segno = start_segno; segno < end_segno; segno++) {
> + sum_page = find_get_page(META_MAPPING(sbi),
> + GET_SUM_BLOCK(sbi, segno));
> + f2fs_put_page(sum_page, 0);
find_get_page() will add one more reference on page, so we need to call
f2fs_put_page(sum_page, 0) twice.
>>>
>>> Done.
>>>
Thanks,
> + }
> + return PTR_ERR(sum_page);
> + }
> unlock_page(sum_page);
> }
>
> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
> index fa2381c0bc47..79b6fee354f7 100644
> --- a/fs/f2fs/node.c
> +++ b/fs/f2fs/node.c
> @@ -126,6 +126,8 @@ static struct page
Re: [f2fs-dev] [PATCH] f2fs: fix quota info to adjust recovered data
On 09/21, Chao Yu wrote:
> On 2018/9/21 5:42, Jaegeuk Kim wrote:
> > On 09/20, Chao Yu wrote:
> >> On 2018/9/20 6:38, Jaegeuk Kim wrote:
> >>> On 09/19, Chao Yu wrote:
> On 2018/9/19 0:45, Jaegeuk Kim wrote:
> > On 09/18, Chao Yu wrote:
> >> On 2018/9/18 10:05, Jaegeuk Kim wrote:
> >>> On 09/18, Chao Yu wrote:
> On 2018/9/18 9:19, Jaegeuk Kim wrote:
> > On 09/13, Chao Yu wrote:
> >> On 2018/9/13 3:54, Jaegeuk Kim wrote:
> >>> On 09/12, Chao Yu wrote:
> On 2018/9/12 9:40, Chao Yu wrote:
> > On 2018/9/12 9:25, Jaegeuk Kim wrote:
> >> On 09/12, Chao Yu wrote:
> >>> On 2018/9/12 8:27, Jaegeuk Kim wrote:
> On 09/11, Jaegeuk Kim wrote:
> > On 09/12, Chao Yu wrote:
> >> On 2018/9/12 4:15, Jaegeuk Kim wrote:
> >>> fsck.f2fs is able to recover the quota structure, since
> >>> roll-forward recovery
> >>> can recover it based on previous user information.
> >>
> >> I didn't get it, both fsck and kernel recover quota file
> >> based all inodes'
> >> uid/gid/prjid, if {x}id didn't change, wouldn't those two
> >> recovery result be the
> >> same?
> >
> > I thought that, but had to add this, since I was
> > encountering quota errors right
> > after getting some files recovered. And, I thought it'd
> > make it more safe to do
> > fsck after roll-forward recovery.
> >
> > Anyway, let me test again without this patch for a while.
>
> Hmm, I just got a fsck failure right after some files
> recovered.
> >>>
> >>> To make sure, do you test with "f2fs: guarantee journalled
> >>> quota data by
> >>> checkpoint"? if not, I think there is no guarantee that f2fs
> >>> can recover
> >>> quote info into correct quote file, because, in last
> >>> checkpoint, quota file
> >>> may was corrupted/inconsistent. Right?
> >
> > Oh, I forget to mention that, I add a patch to fsck to let it
> > noticing
> > CP_QUOTA_NEED_FSCK_FLAG flag, and by default, fsck will fix
> > corrupted quote
> > file if the flag is set, but w/o this flag, quota file is still
> > corrupted
> > detected by fsck, I guess there is bug in v8.
>
> In v8, there are two cases we didn't guarantee quota file's
> consistence:
> 1. flush time in block_operation exceed a threshold.
> 2. dquot subsystem error occurs.
>
> For above case, fsck should repair the quota file by default.
> >>>
> >>> Okay, I got another failure and it seems CP_QUOTA_NEED_FSCK_FLAG
> >>> was not set
> >>> during the recovery. So, we have something missing in the
> >>> recovery in terms
> >>> of quota updates.
> >>
> >> Yeah, I checked the code, just found one suspected place:
> >>
> >> find_fsync_dnodes()
> >> - f2fs_recover_inode_page
> >> - inc_valid_node_count
> >>- dquot_reserve_block dquot info is not initialized now
> >> - add_fsync_inode
> >> - dquot_initialize
> >>
> >> I think we should reserve block for inode block after
> >> dquot_initialize(), can
> >> you confirm this?
> >
> > Let me test this.
> >
> > >From b90260bc577fe87570b1ef7b134554a8295b1f6c Mon Sep 17 00:00:00
> > >2001
> > From: Jaegeuk Kim
> > Date: Mon, 17 Sep 2018 18:14:41 -0700
> > Subject: [PATCH] f2fs: count inode block for recovered files
> >
> > If a new file is recovered, we missed to reserve its inode block.
>
> I remember, in order to keep line with other filesystem, unlike
> on-disk, we
> have to keep backward compatibilty, in memory we don't account block
> number
> for f2fs' inode block, but only account inode number for it, so here
> like
> we did in inc_valid_node_count(), we don't need to do this.
> >>>
> >>> Okay, I just hit the error again w/o your patch. Another one coming
> >>> to my mind
> >>> is that caused by uid/gid change during recovery. Let me try out your
> >>> patch.
> >>
> >> I guess we should update dquot and inode's uid/gid atomically under
> >> lock_op() in f2fs_setattr() to prevent corruption on sys quota file.
> >>
> >> v9 can pass all xfstest cases and por_fsstress case w/ sys quota
Re: [f2fs-dev] [PATCH v3] f2fs: submit cached bio to avoid endless PageWriteback
On 09/21, Chao Yu wrote:
> On 2018/9/18 10:14, Chao Yu wrote:
> > On 2018/9/18 10:02, Jaegeuk Kim wrote:
> >> On 09/18, Chao Yu wrote:
> >>> On 2018/9/18 9:37, Jaegeuk Kim wrote:
> On 09/18, Chao Yu wrote:
> > On 2018/9/18 9:04, Jaegeuk Kim wrote:
> >> On 09/13, Chao Yu wrote:
> >>> From: Chao Yu
> >>>
> >>> When migrating encrypted block from background GC thread, we only add
> >>> them into f2fs inner bio cache, but forget to submit the cached bio,
> >>> it
> >>> may cause potential deadlock when we are waiting page writebacked, fix
> >>> it.
> >>>
> >>> Signed-off-by: Chao Yu
> >>> ---
> >>> v3:
> >>> clean up codes suggested by Jaegeuk.
> >>> fs/f2fs/f2fs.h | 2 +-
> >>> fs/f2fs/gc.c | 71
> >>> +++---
> >>> fs/f2fs/node.c | 13 ++---
> >>> 3 files changed, 61 insertions(+), 25 deletions(-)
> >>>
> >>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> >>> index b676b82312e0..917b2ca76aac 100644
> >>> --- a/fs/f2fs/f2fs.h
> >>> +++ b/fs/f2fs/f2fs.h
> >>> @@ -2869,7 +2869,7 @@ struct page *f2fs_new_node_page(struct
> >>> dnode_of_data *dn, unsigned int ofs);
> >>> void f2fs_ra_node_page(struct f2fs_sb_info *sbi, nid_t nid);
> >>> struct page *f2fs_get_node_page(struct f2fs_sb_info *sbi, pgoff_t
> >>> nid);
> >>> struct page *f2fs_get_node_page_ra(struct page *parent, int start);
> >>> -void f2fs_move_node_page(struct page *node_page, int gc_type);
> >>> +int f2fs_move_node_page(struct page *node_page, int gc_type);
> >>> int f2fs_fsync_node_pages(struct f2fs_sb_info *sbi, struct inode
> >>> *inode,
> >>> struct writeback_control *wbc, bool atomic,
> >>> unsigned int *seq_id);
> >>> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> >>> index a4c1a419611d..f57622cfe058 100644
> >>> --- a/fs/f2fs/gc.c
> >>> +++ b/fs/f2fs/gc.c
> >>> @@ -461,7 +461,7 @@ static int check_valid_map(struct f2fs_sb_info
> >>> *sbi,
> >>> * On validity, copy that node with cold status, otherwise (invalid
> >>> node)
> >>> * ignore that.
> >>> */
> >>> -static void gc_node_segment(struct f2fs_sb_info *sbi,
> >>> +static int gc_node_segment(struct f2fs_sb_info *sbi,
> >>> struct f2fs_summary *sum, unsigned int segno, int
> >>> gc_type)
> >>> {
> >>> struct f2fs_summary *entry;
> >>> @@ -469,6 +469,7 @@ static void gc_node_segment(struct f2fs_sb_info
> >>> *sbi,
> >>> int off;
> >>> int phase = 0;
> >>> bool fggc = (gc_type == FG_GC);
> >>> + int submitted = 0;
> >>>
> >>> start_addr = START_BLOCK(sbi, segno);
> >>>
> >>> @@ -482,10 +483,11 @@ static void gc_node_segment(struct f2fs_sb_info
> >>> *sbi,
> >>> nid_t nid = le32_to_cpu(entry->nid);
> >>> struct page *node_page;
> >>> struct node_info ni;
> >>> + int err;
> >>>
> >>> /* stop BG_GC if there is not enough free sections. */
> >>> if (gc_type == BG_GC && has_not_enough_free_secs(sbi,
> >>> 0, 0))
> >>> - return;
> >>> + return submitted;
> >>>
> >>> if (check_valid_map(sbi, segno, off) == 0)
> >>> continue;
> >>> @@ -522,7 +524,9 @@ static void gc_node_segment(struct f2fs_sb_info
> >>> *sbi,
> >>> continue;
> >>> }
> >>>
> >>> - f2fs_move_node_page(node_page, gc_type);
> >>> + err = f2fs_move_node_page(node_page, gc_type);
> >>> + if (!err && gc_type == FG_GC)
> >>> + submitted++;
> >>> stat_inc_node_blk_count(sbi, 1, gc_type);
> >>> }
> >>>
> >>> @@ -531,6 +535,7 @@ static void gc_node_segment(struct f2fs_sb_info
> >>> *sbi,
> >>>
> >>> if (fggc)
> >>> atomic_dec(>wb_sync_req[NODE]);
> >>> + return submitted;
> >>> }
> >>>
> >>> /*
> >>> @@ -666,7 +671,7 @@ static int ra_data_block(struct inode *inode,
> >>> pgoff_t index)
> >>> * Move data block via META_MAPPING while keeping locked data page.
> >>> * This can be used to move blocks, aka LBAs, directly on disk.
> >>> */
> >>> -static void move_data_block(struct inode *inode, block_t bidx,
> >>> +static int move_data_block(struct inode *inode, block_t bidx,
> >>> int gc_type, unsigned int segno, int
> >>> off)
> >>
> >> We don't need to submit IOs in this case.
> >
> > Actually, previously, we missed to submit IOs for encrypted block only
> > in
> > BGGC, so we fix to submit for
Re: [f2fs-dev] [PATCH 2/2 v2] f2fs: avoid f2fs_bug_on if f2fs_get_meta_page_nofail got EIO
On 09/21, Chao Yu wrote:
> On 2018/9/21 5:46, Jaegeuk Kim wrote:
> > On 09/20, Chao Yu wrote:
> >> On 2018/9/19 1:56, Jaegeuk Kim wrote:
> >>> This patch avoids BUG_ON when f2fs_get_meta_page_nofail got EIO during
> >>> xfstests/generic/475.
> >>>
> >>> Signed-off-by: Jaegeuk Kim
> >>> ---
> >>> Change log from v1:
> >>> - propagate errno
> >>> - drop sum_pages
> >>>
> >>> fs/f2fs/checkpoint.c | 10 +-
> >>> fs/f2fs/f2fs.h | 2 +-
> >>> fs/f2fs/gc.c | 9 +
> >>> fs/f2fs/node.c | 32
> >>> fs/f2fs/recovery.c | 2 ++
> >>> fs/f2fs/segment.c| 3 +++
> >>> 6 files changed, 44 insertions(+), 14 deletions(-)
> >>>
> >>> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
> >>> index 01e0d8f5bbbe..1ddf332ce4b2 100644
> >>> --- a/fs/f2fs/checkpoint.c
> >>> +++ b/fs/f2fs/checkpoint.c
> >>> @@ -119,11 +119,8 @@ struct page *f2fs_get_meta_page_nofail(struct
> >>> f2fs_sb_info *sbi, pgoff_t index)
> >>> if (PTR_ERR(page) == -EIO &&
> >>> ++count <= DEFAULT_RETRY_IO_COUNT)
> >>> goto retry;
> >>> -
> >>> f2fs_stop_checkpoint(sbi, false);
> >>> - f2fs_bug_on(sbi, 1);
> >>> }
> >>> -
> >>> return page;
> >>> }
> >>>
> >>> @@ -1512,7 +1509,10 @@ int f2fs_write_checkpoint(struct f2fs_sb_info
> >>> *sbi, struct cp_control *cpc)
> >>> ckpt->checkpoint_ver = cpu_to_le64(++ckpt_ver);
> >>>
> >>> /* write cached NAT/SIT entries to NAT/SIT area */
> >>> - f2fs_flush_nat_entries(sbi, cpc);
> >>> + err = f2fs_flush_nat_entries(sbi, cpc);
> >>> + if (err)
> >>> + goto stop;
> >>
> >> To make sure, if partial NAT pages become dirty, flush them back to device
> >> outside checkpoint() is not harmful, right?
> >
> > I think so.
>
> Oh, one more case, now we use NAT #0, if partial NAT pages were
> writebacked, then we start to use NAT #1, later, in another checkpoint(),
> we update those NAT pages again, we will write them to NAT #0, which is
> belong to last valid checkpoint(), it's may cause corrupted checkpoint in
> scenario of SPO. So it's harmfull, right?
We already stopped checkpoint anymore, so there'd be no way to proceed another
checkpoint. Am I missing something?
>
> Thanks,
>
> >
> >>
> >>> +
> >>> f2fs_flush_sit_entries(sbi, cpc);
> >>>
> >>> /* unlock all the fs_lock[] in do_checkpoint() */
> >>> @@ -1521,7 +1521,7 @@ int f2fs_write_checkpoint(struct f2fs_sb_info *sbi,
> >>> struct cp_control *cpc)
> >>> f2fs_release_discard_addrs(sbi);
> >>> else
> >>> f2fs_clear_prefree_segments(sbi, cpc);
> >>> -
> >>> +stop:
> >>> unblock_operations(sbi);
> >>> stat_inc_cp_count(sbi->stat_info);
> >>>
> >>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> >>> index a0c868127a7c..29021dbc8f2a 100644
> >>> --- a/fs/f2fs/f2fs.h
> >>> +++ b/fs/f2fs/f2fs.h
> >>> @@ -2895,7 +2895,7 @@ int f2fs_recover_xattr_data(struct inode *inode,
> >>> struct page *page);
> >>> int f2fs_recover_inode_page(struct f2fs_sb_info *sbi, struct page *page);
> >>> int f2fs_restore_node_summary(struct f2fs_sb_info *sbi,
> >>> unsigned int segno, struct f2fs_summary_block *sum);
> >>> -void f2fs_flush_nat_entries(struct f2fs_sb_info *sbi, struct cp_control
> >>> *cpc);
> >>> +int f2fs_flush_nat_entries(struct f2fs_sb_info *sbi, struct cp_control
> >>> *cpc);
> >>> int f2fs_build_node_manager(struct f2fs_sb_info *sbi);
> >>> void f2fs_destroy_node_manager(struct f2fs_sb_info *sbi);
> >>> int __init f2fs_create_node_manager_caches(void);
> >>> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> >>> index 4bcc8a59fdef..c7d14282dbf4 100644
> >>> --- a/fs/f2fs/gc.c
> >>> +++ b/fs/f2fs/gc.c
> >>> @@ -1070,6 +1070,15 @@ static int do_garbage_collect(struct f2fs_sb_info
> >>> *sbi,
> >>> /* reference all summary page */
> >>> while (segno < end_segno) {
> >>> sum_page = f2fs_get_sum_page(sbi, segno++);
> >>> + if (IS_ERR(sum_page)) {
> >>> + end_segno = segno - 1;
> >>> + for (segno = start_segno; segno < end_segno; segno++) {
> >>> + sum_page = find_get_page(META_MAPPING(sbi),
> >>> + GET_SUM_BLOCK(sbi, segno));
> >>> + f2fs_put_page(sum_page, 0);
> >>
> >> find_get_page() will add one more reference on page, so we need to call
> >> f2fs_put_page(sum_page, 0) twice.
> >
> > Done.
> >
> >>
> >> Thanks,
> >>
> >>> + }
> >>> + return PTR_ERR(sum_page);
> >>> + }
> >>> unlock_page(sum_page);
> >>> }
> >>>
> >>> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
> >>> index fa2381c0bc47..79b6fee354f7 100644
> >>> --- a/fs/f2fs/node.c
> >>> +++ b/fs/f2fs/node.c
> >>> @@ -126,6 +126,8 @@ static struct page *get_next_nat_page(struct
> >>> f2fs_sb_info *sbi, nid_t nid)
> >>>
> >>> /* get current nat block page with lock */
> >>> src_page =
Re: [f2fs-dev] [PATCH] generic: add a testcase to test uid/gid recovery
Hi Eryu,
There are several fields in inode rather than uid/gid didn't recover in
f2fs, I'm not sure we need to cover all of them with one generic testcase,
or with several testcases. Any suggestion?
On 2018/9/25 16:45, Chao Yu wrote:
> After fsync, filesystem should guarantee inode metadata including
> uid/gid being persisted, so even after sudden power-cut, durign
> mount, we should recover uid/gid fields correctly, in order to not
> loss those meta info.
>
> So adding this testcase to check whether generic filesystem can
> guarantee that.
>
> Signed-off-by: Chao Yu
> ---
> tests/generic/505 | 95 +++
> tests/generic/505.out | 2 +
> tests/generic/group | 1 +
> 3 files changed, 98 insertions(+)
> create mode 100755 tests/generic/505
> create mode 100644 tests/generic/505.out
>
> diff --git a/tests/generic/505 b/tests/generic/505
> new file mode 100755
> index ..103a1e9bbe47
> --- /dev/null
> +++ b/tests/generic/505
> @@ -0,0 +1,95 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2018 Huawei. All Rights Reserved.
> +#
> +# FS QA Test 505
> +#
> +# This testcase is trying to test recovery flow of generic filesystem, w/
> below
> +# steps, once uid or gid changes, after we fsync that file, we can expect
> that
> +# uid/gid can be recovered after sudden power-cuts.
> +# 1. touch testfile;
> +# 1.1 sync (optional)
> +# 2. chown 100 testfile;
> +# 3. chgrp 100 testfile;
> +# 4. xfs_io -f testfile -c "fsync";
> +# 5. godown;
> +# 6. umount;
> +# 7. mount;
> +# 8. check uid/gid
> +#
> +seq=`basename $0`
> +seqres=$RESULT_DIR/$seq
> +echo "QA output created by $seq"
> +
> +here=`pwd`
> +tmp=/tmp/$$
> +status=1 # failure is the default!
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +_cleanup()
> +{
> + cd /
> + rm -f $tmp.*
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +
> +# remove previous $seqres.full before test
> +rm -f $seqres.full
> +
> +# real QA test starts here
> +_supported_fs generic
> +_supported_os Linux
> +
> +_require_scratch
> +_require_scratch_shutdown
> +
> +_scratch_mkfs >/dev/null 2>&1
> +_require_metadata_journaling $SCRATCH_DEV
> +
> +testfile=$SCRATCH_MNT/testfile
> +stat_opt='-c "uid: %u, gid: %g"'
> +
> +_do_check()
> +{
> + _scratch_mount
> +
> + touch $testfile
> +
> + if [ "$1" == "sync" ]; then
> + sync
> + fi
> +
> + chown 100 $testfile
> + chgrp 100 $testfile
> +
> + before=`stat "$stat_opt" $testfile`
> +
> + $XFS_IO_PROG -f $testfile -c "fsync" | _filter_xfs_io
> +
> + _scratch_shutdown | tee -a $seqres.full
> + _scratch_unmount
> + _scratch_mount
> +
> + after=`stat "$stat_opt" $testfile`
> +
> + # check inode's uid/gid
> + if [ "$before" != "$after" ]; then
> + echo "Before: $before"
> + echo "After : $after"
> + fi
> + echo "Before: $before" >> $seqres.full
> + echo "After : $after" >> $seqres.full
> +
> + rm $testfile
> + _scratch_unmount
> +}
> +
> +_do_check
> +_do_check sync
> +
> +status=0
> +echo "Silence is golden"
> +exit
> diff --git a/tests/generic/505.out b/tests/generic/505.out
> new file mode 100644
> index ..80e3bd1d9abb
> --- /dev/null
> +++ b/tests/generic/505.out
> @@ -0,0 +1,2 @@
> +QA output created by 505
> +Silence is golden
> diff --git a/tests/generic/group b/tests/generic/group
> index 55155de8bc29..4da0e1888f57 100644
> --- a/tests/generic/group
> +++ b/tests/generic/group
> @@ -507,3 +507,4 @@
> 502 auto quick log
> 503 auto quick dax punch collapse zero
> 504 auto quick locks
> +505 shutdown auto quick metadata
>
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH] generic: add a testcase to test uid/gid recovery
After fsync, filesystem should guarantee inode metadata including
uid/gid being persisted, so even after sudden power-cut, durign
mount, we should recover uid/gid fields correctly, in order to not
loss those meta info.
So adding this testcase to check whether generic filesystem can
guarantee that.
Signed-off-by: Chao Yu
---
tests/generic/505 | 95 +++
tests/generic/505.out | 2 +
tests/generic/group | 1 +
3 files changed, 98 insertions(+)
create mode 100755 tests/generic/505
create mode 100644 tests/generic/505.out
diff --git a/tests/generic/505 b/tests/generic/505
new file mode 100755
index ..103a1e9bbe47
--- /dev/null
+++ b/tests/generic/505
@@ -0,0 +1,95 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2018 Huawei. All Rights Reserved.
+#
+# FS QA Test 505
+#
+# This testcase is trying to test recovery flow of generic filesystem, w/ below
+# steps, once uid or gid changes, after we fsync that file, we can expect that
+# uid/gid can be recovered after sudden power-cuts.
+# 1. touch testfile;
+# 1.1 sync (optional)
+# 2. chown 100 testfile;
+# 3. chgrp 100 testfile;
+# 4. xfs_io -f testfile -c "fsync";
+# 5. godown;
+# 6. umount;
+# 7. mount;
+# 8. check uid/gid
+#
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1 # failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+ cd /
+ rm -f $tmp.*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# remove previous $seqres.full before test
+rm -f $seqres.full
+
+# real QA test starts here
+_supported_fs generic
+_supported_os Linux
+
+_require_scratch
+_require_scratch_shutdown
+
+_scratch_mkfs >/dev/null 2>&1
+_require_metadata_journaling $SCRATCH_DEV
+
+testfile=$SCRATCH_MNT/testfile
+stat_opt='-c "uid: %u, gid: %g"'
+
+_do_check()
+{
+ _scratch_mount
+
+ touch $testfile
+
+ if [ "$1" == "sync" ]; then
+ sync
+ fi
+
+ chown 100 $testfile
+ chgrp 100 $testfile
+
+ before=`stat "$stat_opt" $testfile`
+
+ $XFS_IO_PROG -f $testfile -c "fsync" | _filter_xfs_io
+
+ _scratch_shutdown | tee -a $seqres.full
+ _scratch_unmount
+ _scratch_mount
+
+ after=`stat "$stat_opt" $testfile`
+
+ # check inode's uid/gid
+ if [ "$before" != "$after" ]; then
+ echo "Before: $before"
+ echo "After : $after"
+ fi
+ echo "Before: $before" >> $seqres.full
+ echo "After : $after" >> $seqres.full
+
+ rm $testfile
+ _scratch_unmount
+}
+
+_do_check
+_do_check sync
+
+status=0
+echo "Silence is golden"
+exit
diff --git a/tests/generic/505.out b/tests/generic/505.out
new file mode 100644
index ..80e3bd1d9abb
--- /dev/null
+++ b/tests/generic/505.out
@@ -0,0 +1,2 @@
+QA output created by 505
+Silence is golden
diff --git a/tests/generic/group b/tests/generic/group
index 55155de8bc29..4da0e1888f57 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -507,3 +507,4 @@
502 auto quick log
503 auto quick dax punch collapse zero
504 auto quick locks
+505 shutdown auto quick metadata
--
2.18.0.rc1
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH] f2fs: fix remount problem of option io_bits
On 2018/9/22 22:43, Chengguang Xu wrote: > Currently we show mount option "io_bits=%u" as "io_size=%uKB", > it will cause option parsing problem(unrecognized mount option) > in remount. > > Signed-off-by: Chengguang Xu Reviewed-by: Chao Yu Thanks, ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH] f2fs: remove default option setting in remount
On 2018/9/22 22:40, cgxu519 wrote:
> On 09/20/2018 02:29 PM, Chao Yu wrote:
>> On 2018/9/20 7:54, cgxu519 wrote:
>>> On 9/19/18 10:02 PM, Chao Yu wrote:
On 2018/9/18 21:47, cgxu519 wrote:
> On 09/18/2018 09:20 PM, Chao Yu wrote:
>> On 2018/9/18 14:23, Chengguang Xu wrote:
>>> Currently we set default value to options before parsing remount
>>> options,
>>> it will cause unexpected change of options which were specified in first
>>> mount.
>> Can you check below commit? It looks w/o it we may lose default option
>> after
>> remount.
>>
>> 498c5e9fcd10 ("f2fs: add default mount options to remount")
> Hi Chao,
Hi Chengguang,
> It looks like there was a bug in remount at that time, but I think the
> fix above
> is not correct.
>
> from the patch '498c5e9fcd10', I think it was caused by clearing
> sbi->mount_opt.opt before
>
> parsing. I think remount should not change the options which were
> specified by
> user in
>
> previous mount unless user specifies in remount.
Can you check description in manual of mount.
IIRC, old mount option will be record into /etc/mtab or /proc/mounts (for
adapting namespace feature), with command of "mount -o remount,rw /dir",
old
mount options can be loaded from above config file, and merge them with new
specified options.
Even we kill old mount options by call default_options() in ->remount,
user's
old mount option can still be set through parameters.
>>> Please let me show you different behaviors before/after this patch.
>>>
>>>
>>> [root@x201 cgxu]# uname -a
>>> Linux x201 4.19.0-rc2+ #51 SMP Wed Sep 19 22:41:20 CST 2018 x86_64
>>> x86_64 x86_64 GNU/Linux
>>>
>>>
>>> Before:
>>>
>>> [root@x201 cgxu]# mount -o fsync_mode=strict /dev/mapper/test-test1
>>> /mnt/test/test1
>>> [root@x201 cgxu]# mount | grep test1
>>> /dev/mapper/test-test1 on /mnt/test/test1 type f2fs
>>> (rw,relatime,lazytime,background_gc=on,discard,no_heap,user_xattr,inline_xattr,acl,inline_data,inline_dentry,flush_merge,extent_cache,mode=adaptive,active_logs=6,alloc_mode=reuse,fsync_mode=strict)
>>>
>>> [root@x201 cgxu]# mount -o remount,background_gc=sync
>>> /dev/mapper/test-test1 /mnt/test/test1
>>> [root@x201 cgxu]# mount | grep test1
>>> /dev/mapper/test-test1 on /mnt/test/test1 type f2fs
>>> (rw,relatime,background_gc=sync,discard,no_heap,user_xattr,inline_xattr,acl,inline_data,inline_dentry,flush_merge,extent_cache,mode=adaptive,active_logs=6,alloc_mode=default,fsync_mode=posix)
>>>
>>>
>>> I only changed background_gc in ->remount, but fsync_mode is implicitly
>>> set to default value 'posix'.
>> IMO, the resul is as expected, let's referenced description in manual of
>> mount:
>>
>>The remount functionality follows the standard way how the
>> mount command works with options from fstab. It means the mount command
>> doesn't read fstab (or mtab) only when a device and
>>dir are fully specified.
>>
>>mount -o remount,rw /dev/foo /dir
>>
>>After this call all old mount options are replaced and
>> arbitrary stuff from fstab is ignored, except the loop= option which is
>> internally generated and maintained by the mount command.
>>
>>mount -o remount,rw /dir
>>
>>After this call mount reads fstab (or mtab) and merges these
>> options with options from command line ( -o ).
>>
>>
>> So if you want to keep old mount option, you should use:
>>
>> mount -o remount,background_gc=sync /mnt/test/test1
>>
>> instead of
>>
>> mount -o remount,background_gc=sync /dev/mapper/test-test1 /mnt/test/test1
>
> I get your point about how mount command organizes options ,
> but it seems other filesystem do not initialize mount option in remount.
> What do you think for that? and if we keep initialization in f2fs,
> shouldn't we set default value to all mount options for remount?
I'm okay with this change to keep line with other filesystems.
+Cc Yunlei to check whether we are missing something.
Thanks,
>
> Thanks,
> Chengguang
>
>
>
>
>
>
>
>
> .
>
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH 1/6] f2fs: fix to recover inode's project id during POR
Testcase to reproduce this bug:
1. mkfs.f2fs -O extra_attr -O project_quota /dev/sdd
2. mount -t f2fs /dev/sdd /mnt/f2fs
3. touch /mnt/f2fs/file
4. sync
5. chattr -p 1 /mnt/f2fs/file
6. xfs_io -f /mnt/f2fs/file -c "fsync"
7. godown /mnt/f2fs
8. umount /mnt/f2fs
9. mount -t f2fs /dev/sdd /mnt/f2fs
10. lsattr -p /mnt/f2fs/file
0 -N- /mnt/f2fs/file
But actually, we expect the correct result is:
1 -N- /mnt/f2fs/file
The reason is we didn't recover inode.i_projid field during mount,
fix it.
Signed-off-by: Chao Yu
---
fs/f2fs/recovery.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c
index 70f05650191e..c7e292f21b64 100644
--- a/fs/f2fs/recovery.c
+++ b/fs/f2fs/recovery.c
@@ -244,6 +244,19 @@ static int recover_inode(struct inode *inode, struct page
*page)
i_uid_write(inode, le32_to_cpu(raw->i_uid));
i_gid_write(inode, le32_to_cpu(raw->i_gid));
+
+ if (raw->i_inline & F2FS_EXTRA_ATTR) {
+ if (f2fs_sb_has_project_quota(F2FS_I_SB(inode)->sb) &&
+ F2FS_FITS_IN_INODE(raw, le16_to_cpu(raw->i_extra_isize),
+ i_projid)) {
+ projid_t i_projid;
+
+ i_projid = (projid_t)le32_to_cpu(raw->i_projid);
+ F2FS_I(inode)->i_projid =
+ make_kprojid(_user_ns, i_projid);
+ }
+ }
+
f2fs_i_size_write(inode, le64_to_cpu(raw->i_size));
inode->i_atime.tv_sec = le64_to_cpu(raw->i_atime);
inode->i_ctime.tv_sec = le64_to_cpu(raw->i_ctime);
--
2.18.0.rc1
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH 5/6] f2fs: fix to keep project quota consistent
This patch does below changes to keep consistence of project quota data
in sudden power-cut case:
- update inode.i_projid and project quota atomically under lock_op() in
f2fs_ioc_setproject()
- recover inode.i_projid and project quota in recover_inode()
Signed-off-by: Chao Yu
---
fs/f2fs/f2fs.h | 1 +
fs/f2fs/file.c | 37 -
fs/f2fs/recovery.c | 12 ++--
3 files changed, 39 insertions(+), 11 deletions(-)
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index be16a49c8b62..171917c0961c 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -2793,6 +2793,7 @@ void f2fs_truncate_data_blocks_range(struct dnode_of_data
*dn, int count);
int f2fs_precache_extents(struct inode *inode);
long f2fs_ioctl(struct file *filp, unsigned int cmd, unsigned long arg);
long f2fs_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg);
+int f2fs_transfer_project_quota(struct inode *inode, kprojid_t kprojid);
int f2fs_pin_file_control(struct inode *inode, bool inc);
/*
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index a388866e71ee..6f02c8daff76 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -2604,13 +2604,29 @@ static int f2fs_ioc_get_features(struct file *filp,
unsigned long arg)
}
#ifdef CONFIG_QUOTA
+int f2fs_transfer_project_quota(struct inode *inode, kprojid_t kprojid)
+{
+ struct dquot *transfer_to[MAXQUOTAS] = {};
+ struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
+ struct super_block *sb = sbi->sb;
+ int err = 0;
+
+ transfer_to[PRJQUOTA] = dqget(sb, make_kqid_projid(kprojid));
+ if (!IS_ERR(transfer_to[PRJQUOTA])) {
+ err = __dquot_transfer(inode, transfer_to);
+ if (err)
+ set_sbi_flag(sbi, SBI_QUOTA_NEED_REPAIR);
+ dqput(transfer_to[PRJQUOTA]);
+ }
+ return err;
+}
+
static int f2fs_ioc_setproject(struct file *filp, __u32 projid)
{
struct inode *inode = file_inode(filp);
struct f2fs_inode_info *fi = F2FS_I(inode);
struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
struct super_block *sb = sbi->sb;
- struct dquot *transfer_to[MAXQUOTAS] = {};
struct page *ipage;
kprojid_t kprojid;
int err;
@@ -2651,21 +2667,24 @@ static int f2fs_ioc_setproject(struct file *filp, __u32
projid)
if (err)
return err;
- transfer_to[PRJQUOTA] = dqget(sb, make_kqid_projid(kprojid));
- if (!IS_ERR(transfer_to[PRJQUOTA])) {
- err = __dquot_transfer(inode, transfer_to);
- dqput(transfer_to[PRJQUOTA]);
- if (err)
- goto out_dirty;
- }
+ f2fs_lock_op(sbi);
+ err = f2fs_transfer_project_quota(inode, kprojid);
+ if (err)
+ goto out_unlock;
F2FS_I(inode)->i_projid = kprojid;
inode->i_ctime = current_time(inode);
-out_dirty:
f2fs_mark_inode_dirty_sync(inode, true);
+out_unlock:
+ f2fs_unlock_op(sbi);
return err;
}
#else
+int f2fs_transfer_project_quota(struct inode *inode, kprojid_t kprojid)
+{
+ return 0;
+}
+
static int f2fs_ioc_setproject(struct file *filp, __u32 projid)
{
if (projid != F2FS_DEF_PROJID)
diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c
index 71577f30a889..5e68e31989c8 100644
--- a/fs/f2fs/recovery.c
+++ b/fs/f2fs/recovery.c
@@ -250,10 +250,18 @@ static int recover_inode(struct inode *inode, struct page
*page)
F2FS_FITS_IN_INODE(raw, le16_to_cpu(raw->i_extra_isize),
i_projid)) {
projid_t i_projid;
+ kprojid_t kprojid;
i_projid = (projid_t)le32_to_cpu(raw->i_projid);
- F2FS_I(inode)->i_projid =
- make_kprojid(_user_ns, i_projid);
+ kprojid = make_kprojid(_user_ns, i_projid);
+
+ if (!projid_eq(kprojid, F2FS_I(inode)->i_projid)) {
+ err = f2fs_transfer_project_quota(inode,
+ kprojid);
+ if (err)
+ return err;
+ F2FS_I(inode)->i_projid = kprojid;
+ }
}
}
--
2.18.0.rc1
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH 3/6] f2fs: fix to recover inode's i_gc_failures during POR
inode.i_gc_failures is used to indicate that skip count of migrating on blocks of inode, we should guarantee it can be recovered in sudden power-off case. Signed-off-by: Chao Yu --- fs/f2fs/recovery.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c index 4a0f49654573..71577f30a889 100644 --- a/fs/f2fs/recovery.c +++ b/fs/f2fs/recovery.c @@ -267,6 +267,8 @@ static int recover_inode(struct inode *inode, struct page *page) F2FS_I(inode)->i_advise = raw->i_advise; F2FS_I(inode)->i_flags = le32_to_cpu(raw->i_flags); + F2FS_I(inode)->i_gc_failures[GC_FAILURE_PIN] = + le16_to_cpu(raw->i_gc_failures); recover_inline_flags(inode, raw); -- 2.18.0.rc1 ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH 2/6] f2fs: fix to recover inode's i_flags during POR
Testcase to reproduce this bug: 1. mkfs.f2fs /dev/sdd 2. mount -t f2fs /dev/sdd /mnt/f2fs 3. touch /mnt/f2fs/file 4. sync 5. chattr +A /mnt/f2fs/file 6. xfs_io -f /mnt/f2fs/file -c "fsync" 7. godown /mnt/f2fs 8. umount /mnt/f2fs 9. mount -t f2fs /dev/sdd /mnt/f2fs 10. lsattr /mnt/f2fs/file -N- /mnt/f2fs/file But actually, we expect the corrct result is: ---A-N- /mnt/f2fs/file The reason is we didn't recover inode.i_flags field during mount, fix it. Signed-off-by: Chao Yu --- fs/f2fs/recovery.c | 1 + 1 file changed, 1 insertion(+) diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c index c7e292f21b64..4a0f49654573 100644 --- a/fs/f2fs/recovery.c +++ b/fs/f2fs/recovery.c @@ -266,6 +266,7 @@ static int recover_inode(struct inode *inode, struct page *page) inode->i_mtime.tv_nsec = le32_to_cpu(raw->i_mtime_nsec); F2FS_I(inode)->i_advise = raw->i_advise; + F2FS_I(inode)->i_flags = le32_to_cpu(raw->i_flags); recover_inline_flags(inode, raw); -- 2.18.0.rc1 ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH 4/6] f2fs: fix to recover inode's crtime during POR
Testcase to reproduce this bug:
1. mkfs.f2fs -O extra_attr -O inode_crtime /dev/sdd
2. mount -t f2fs /dev/sdd /mnt/f2fs
3. touch /mnt/f2fs/file
4. xfs_io -f /mnt/f2fs/file -c "fsync"
5. godown /mnt/f2fs
6. umount /mnt/f2fs
7. mount -t f2fs /dev/sdd /mnt/f2fs
8. xfs_io -f /mnt/f2fs/file -c "statx -r"
stat.btime.tv_sec = 0
stat.btime.tv_nsec = 0
This patch fixes to recover inode creation time fields during
mount.
Signed-off-by: Chao Yu
---
fs/f2fs/node.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
index 5717fb6e7d7d..ea151e07790f 100644
--- a/fs/f2fs/node.c
+++ b/fs/f2fs/node.c
@@ -2565,6 +2565,13 @@ int f2fs_recover_inode_page(struct f2fs_sb_info *sbi,
struct page *page)
F2FS_FITS_IN_INODE(src, le16_to_cpu(src->i_extra_isize),
i_projid))
dst->i_projid = src->i_projid;
+
+ if (f2fs_sb_has_inode_crtime(sbi->sb) &&
+ F2FS_FITS_IN_INODE(src, le16_to_cpu(src->i_extra_isize),
+ i_crtime_nsec)) {
+ dst->i_crtime = src->i_crtime;
+ dst->i_crtime_nsec = src->i_crtime_nsec;
+ }
}
new_ni = old_ni;
--
2.18.0.rc1
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH 6/6] f2fs: mark inode dirty explicitly in recover_inode()
Mark inode dirty explicitly in the end of recover_inode() to make sure that all recoverable fields can be persisted later. Signed-off-by: Chao Yu --- fs/f2fs/recovery.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c index 5e68e31989c8..409be551ba03 100644 --- a/fs/f2fs/recovery.c +++ b/fs/f2fs/recovery.c @@ -280,6 +280,8 @@ static int recover_inode(struct inode *inode, struct page *page) recover_inline_flags(inode, raw); + f2fs_mark_inode_dirty_sync(inode, true); + if (file_enc_name(inode)) name = ""; else -- 2.18.0.rc1 ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
