linux-kernel - Re: [PATCH 3/5] f2fs: move f2fs_balance

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 4 Mar 2013 15:10:38 +0900
From:	Namjae Jeon <linkinjeon@...il.com>
To:	jaegeuk.kim@...sung.com
Cc:	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-f2fs-devel@...ts.sourceforge.net,
	Namjae Jeon <namjae.jeon@...sung.com>,
	Amit Sahrawat <a.sahrawat@...sung.com>
Subject: Re: [PATCH 3/5] f2fs: move f2fs_balance_fs to correct place in unlink

2013/3/3, Jaegeuk Kim <jaegeuk.kim@...sung.com>:
> 2013-03-02 (토), 12:41 +0900, Namjae Jeon:
>> From: Namjae Jeon <namjae.jeon@...sung.com>
>>
>> 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@...sung.com>
>> Signed-off-by: Amit Sahrawat <a.sahrawat@...sung.com>
>> ---
>>  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-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/