2013/3/3, Jaegeuk Kim <jaegeuk.kim@xxxxxxxxxxx>: > 2013-03-02 (토), 12:41 +0900, Namjae Jeon: >> From: Namjae Jeon <namjae.jeon@xxxxxxxxxxx> >> >> Actual dirty of pages will occur in f2fs_delete_entry so move the >> f2fs_balance_fs just before deletion. >> >> Signed-off-by: Namjae Jeon <namjae.jeon@xxxxxxxxxxx> >> Signed-off-by: Amit Sahrawat <a.sahrawat@xxxxxxxxxxx> >> --- >> fs/f2fs/namei.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c >> index 1a49b88..eaa86f5 100644 >> --- a/fs/f2fs/namei.c >> +++ b/fs/f2fs/namei.c >> @@ -223,8 +223,6 @@ static int f2fs_unlink(struct inode *dir, struct >> dentry *dentry) >> struct page *page; >> int err = -ENOENT; >> >> - f2fs_balance_fs(sbi); >> - >> de = f2fs_find_entry(dir, &dentry->d_name, &page); >> if (!de) >> goto fail; >> @@ -236,6 +234,8 @@ static int f2fs_unlink(struct inode *dir, struct >> dentry *dentry) >> goto fail; >> } >> >> + f2fs_balance_fs(sbi); >> + > > I think we don't need to do this because of no issues on performance and > reliability. > In addition, it would be better to call f2fs_balance_fs without any > dentry page. Regarding moving “f2fs_balance_fs” in unlink part– we considered one scenario. Suppose – when the disk is full and it really needed to trigger the Garbage collection. But in this we considered one scenario. Let’s say the ‘name’ being passed is for invalid file i.e., the file does not exist. So, primarily in this case – I think it should return immediately. In such cases it actually results in wrong timing results for the non-existence files. For this observation we thought that f2fs_balance_fs be instead called at a proper place i.e., after there is no lookup-failure. Let me know your opinion. Thanks. > >> f2fs_delete_entry(de, page, inode); >> >> /* In order to evict this inode, we set it dirty */ > > -- > Jaegeuk Kim > Samsung > -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
