[f2fs-dev] [PATCH -next] f2fs: remove unnecessary read for F2FS_FITS_IN_INODE

[f2fs-dev] [PATCH -next] f2fs: remove unnecessary read for F2FS_FITS_IN_INODE

[Jia Yang via Linux-f2fs-devel]

F2FS_FITS_IN_INODE only cares the type of f2fs inode, so there
is no need to read node page of f2fs inode.

Signed-off-by: Jia Yang <jiayang5@huawei.com>
---
 fs/f2fs/file.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index cfdc41f87f5d..4b93fbec2ec0 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -2999,7 +2999,7 @@ static int f2fs_ioc_setproject(struct inode *inode, __u32 projid)
 {
 	struct f2fs_inode_info *fi = F2FS_I(inode);
 	struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
-	struct page *ipage;
+	struct f2fs_inode *ri;
 	kprojid_t kprojid;
 	int err;
 
@@ -3023,17 +3023,11 @@ static int f2fs_ioc_setproject(struct inode *inode, __u32 projid)
 	if (IS_NOQUOTA(inode))
 		return err;
 
-	ipage = f2fs_get_node_page(sbi, inode->i_ino);
-	if (IS_ERR(ipage))
-		return PTR_ERR(ipage);
 
-	if (!F2FS_FITS_IN_INODE(F2FS_INODE(ipage), fi->i_extra_isize,
-								i_projid)) {
+	if (!F2FS_FITS_IN_INODE(ri, fi->i_extra_isize, i_projid)) {
 		err = -EOVERFLOW;
-		f2fs_put_page(ipage, 1);
 		return err;
 	}
-	f2fs_put_page(ipage, 1);
 
 	err = f2fs_dquot_initialize(inode);
 	if (err)
-- 
2.22.0



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH -next] f2fs: remove unnecessary read for F2FS_FITS_IN_INODE

[Chao Yu]

On 2022/3/7 16:12, Jia Yang via Linux-f2fs-devel wrote:
> F2FS_FITS_IN_INODE only cares the type of f2fs inode, so there
> is no need to read node page of f2fs inode.
> 
> Signed-off-by: Jia Yang <jiayang5@huawei.com>
> ---
>   fs/f2fs/file.c | 10 ++--------
>   1 file changed, 2 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index cfdc41f87f5d..4b93fbec2ec0 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -2999,7 +2999,7 @@ static int f2fs_ioc_setproject(struct inode *inode, __u32 projid)
>   {
>   	struct f2fs_inode_info *fi = F2FS_I(inode);
>   	struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
> -	struct page *ipage;
> +	struct f2fs_inode *ri;

struct f2fs_inode *ri = NULL;

(offsetof(typeof(*(f2fs_inode)), field) +
sizeof((f2fs_inode)->field)

A little bit worry about using a NULL pointer here, due to not sure the result of
offsetof() and sizeof() will always be calculated at compiling time for all kind of
compilers, can we guarantee that?

>   	kprojid_t kprojid;
>   	int err;
>   
> @@ -3023,17 +3023,11 @@ static int f2fs_ioc_setproject(struct inode *inode, __u32 projid)
>   	if (IS_NOQUOTA(inode))
>   		return err;
>   
> -	ipage = f2fs_get_node_page(sbi, inode->i_ino);
> -	if (IS_ERR(ipage))
> -		return PTR_ERR(ipage);
>   
> -	if (!F2FS_FITS_IN_INODE(F2FS_INODE(ipage), fi->i_extra_isize,
> -								i_projid)) {
> +	if (!F2FS_FITS_IN_INODE(ri, fi->i_extra_isize, i_projid)) {
>   		err = -EOVERFLOW;
> -		f2fs_put_page(ipage, 1);
>   		return err;

return -EOVERFLOW;

Thanks,

>   	}
> -	f2fs_put_page(ipage, 1);
>   
>   	err = f2fs_dquot_initialize(inode);
>   	if (err)


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH -next] f2fs: remove unnecessary read for F2FS_FITS_IN_INODE

[Jia Yang via Linux-f2fs-devel]

On 2022/3/7 18:40, Chao Yu wrote:
> On 2022/3/7 16:12, Jia Yang via Linux-f2fs-devel wrote:
>> F2FS_FITS_IN_INODE only cares the type of f2fs inode, so there
>> is no need to read node page of f2fs inode.
>>
>> Signed-off-by: Jia Yang <jiayang5@huawei.com>
>> ---
>>   fs/f2fs/file.c | 10 ++--------
>>   1 file changed, 2 insertions(+), 8 deletions(-)
>>
>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
>> index cfdc41f87f5d..4b93fbec2ec0 100644
>> --- a/fs/f2fs/file.c
>> +++ b/fs/f2fs/file.c
>> @@ -2999,7 +2999,7 @@ static int f2fs_ioc_setproject(struct inode *inode, __u32 projid)
>>   {
>>       struct f2fs_inode_info *fi = F2FS_I(inode);
>>       struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
>> -    struct page *ipage;
>> +    struct f2fs_inode *ri;
> 
> struct f2fs_inode *ri = NULL;
> 
> (offsetof(typeof(*(f2fs_inode)), field) +
> sizeof((f2fs_inode)->field)
> 
> A little bit worry about using a NULL pointer here, due to not sure the result of
> offsetof() and sizeof() will always be calculated at compiling time for all kind of
> compilers, can we guarantee that?

We can't guarantee that, but I see that f2fs_getattr also runs in this way. Do you think that allocating memory for
f2fs inode is available?

Thanks.

> 
>>       kprojid_t kprojid;
>>       int err;
>>   @@ -3023,17 +3023,11 @@ static int f2fs_ioc_setproject(struct inode *inode, __u32 projid)
>>       if (IS_NOQUOTA(inode))
>>           return err;
>>   -    ipage = f2fs_get_node_page(sbi, inode->i_ino);
>> -    if (IS_ERR(ipage))
>> -        return PTR_ERR(ipage);
>>   -    if (!F2FS_FITS_IN_INODE(F2FS_INODE(ipage), fi->i_extra_isize,
>> -                                i_projid)) {
>> +    if (!F2FS_FITS_IN_INODE(ri, fi->i_extra_isize, i_projid)) {
>>           err = -EOVERFLOW;
>> -        f2fs_put_page(ipage, 1);
>>           return err;
> 
> return -EOVERFLOW;
> 
> Thanks,
> 
>>       }
>> -    f2fs_put_page(ipage, 1);
>>         err = f2fs_dquot_initialize(inode);
>>       if (err)
> .


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH -next] f2fs: remove unnecessary read for F2FS_FITS_IN_INODE

[Chao Yu]

On 2022/3/8 16:19, Jia Yang via Linux-f2fs-devel wrote:
> 
> 
> On 2022/3/7 18:40, Chao Yu wrote:
>> On 2022/3/7 16:12, Jia Yang via Linux-f2fs-devel wrote:
>>> F2FS_FITS_IN_INODE only cares the type of f2fs inode, so there
>>> is no need to read node page of f2fs inode.
>>>
>>> Signed-off-by: Jia Yang <jiayang5@huawei.com>
>>> ---
>>>    fs/f2fs/file.c | 10 ++--------
>>>    1 file changed, 2 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
>>> index cfdc41f87f5d..4b93fbec2ec0 100644
>>> --- a/fs/f2fs/file.c
>>> +++ b/fs/f2fs/file.c
>>> @@ -2999,7 +2999,7 @@ static int f2fs_ioc_setproject(struct inode *inode, __u32 projid)
>>>    {
>>>        struct f2fs_inode_info *fi = F2FS_I(inode);
>>>        struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
>>> -    struct page *ipage;
>>> +    struct f2fs_inode *ri;
>>
>> struct f2fs_inode *ri = NULL;
>>
>> (offsetof(typeof(*(f2fs_inode)), field) +
>> sizeof((f2fs_inode)->field)
>>
>> A little bit worry about using a NULL pointer here, due to not sure the result of
>> offsetof() and sizeof() will always be calculated at compiling time for all kind of
>> compilers, can we guarantee that?
> 
> We can't guarantee that, but I see that f2fs_getattr also runs in this way. Do you think that allocating memory for
> f2fs inode is available?

Well, it looks ext4_getattr() also did this way...

How about initializing ri w/ NULL at least?

Thanks,

> 
> Thanks.
> 
>>
>>>        kprojid_t kprojid;
>>>        int err;
>>>    @@ -3023,17 +3023,11 @@ static int f2fs_ioc_setproject(struct inode *inode, __u32 projid)
>>>        if (IS_NOQUOTA(inode))
>>>            return err;
>>>    -    ipage = f2fs_get_node_page(sbi, inode->i_ino);
>>> -    if (IS_ERR(ipage))
>>> -        return PTR_ERR(ipage);
>>>    -    if (!F2FS_FITS_IN_INODE(F2FS_INODE(ipage), fi->i_extra_isize,
>>> -                                i_projid)) {
>>> +    if (!F2FS_FITS_IN_INODE(ri, fi->i_extra_isize, i_projid)) {
>>>            err = -EOVERFLOW;
>>> -        f2fs_put_page(ipage, 1);
>>>            return err;
>>
>> return -EOVERFLOW;
>>
>> Thanks,
>>
>>>        }
>>> -    f2fs_put_page(ipage, 1);
>>>          err = f2fs_dquot_initialize(inode);
>>>        if (err)
>> .
> 
> 
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH -next] f2fs: remove unnecessary read for F2FS_FITS_IN_INODE

[Jia Yang via Linux-f2fs-devel]

On 2022/3/9 12:12, Chao Yu wrote:
> On 2022/3/8 16:19, Jia Yang via Linux-f2fs-devel wrote:
>>
>>
>> On 2022/3/7 18:40, Chao Yu wrote:
>>> On 2022/3/7 16:12, Jia Yang via Linux-f2fs-devel wrote:
>>>> F2FS_FITS_IN_INODE only cares the type of f2fs inode, so there
>>>> is no need to read node page of f2fs inode.
>>>>
>>>> Signed-off-by: Jia Yang <jiayang5@huawei.com>
>>>> ---
>>>>    fs/f2fs/file.c | 10 ++--------
>>>>    1 file changed, 2 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
>>>> index cfdc41f87f5d..4b93fbec2ec0 100644
>>>> --- a/fs/f2fs/file.c
>>>> +++ b/fs/f2fs/file.c
>>>> @@ -2999,7 +2999,7 @@ static int f2fs_ioc_setproject(struct inode *inode, __u32 projid)
>>>>    {
>>>>        struct f2fs_inode_info *fi = F2FS_I(inode);
>>>>        struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
>>>> -    struct page *ipage;
>>>> +    struct f2fs_inode *ri;
>>>
>>> struct f2fs_inode *ri = NULL;
>>>
>>> (offsetof(typeof(*(f2fs_inode)), field) +
>>> sizeof((f2fs_inode)->field)
>>>
>>> A little bit worry about using a NULL pointer here, due to not sure the result of
>>> offsetof() and sizeof() will always be calculated at compiling time for all kind of
>>> compilers, can we guarantee that?
>>
>> We can't guarantee that, but I see that f2fs_getattr also runs in this way. Do you think that allocating memory for
>> f2fs inode is available?
> 
> Well, it looks ext4_getattr() also did this way...
> 
> How about initializing ri w/ NULL at least?
> 

I agree with that and I will fix it in V2 version.

Thanks,

> Thanks,
> 
>>
>> Thanks.
>>
>>>
>>>>        kprojid_t kprojid;
>>>>        int err;
>>>>    @@ -3023,17 +3023,11 @@ static int f2fs_ioc_setproject(struct inode *inode, __u32 projid)
>>>>        if (IS_NOQUOTA(inode))
>>>>            return err;
>>>>    -    ipage = f2fs_get_node_page(sbi, inode->i_ino);
>>>> -    if (IS_ERR(ipage))
>>>> -        return PTR_ERR(ipage);
>>>>    -    if (!F2FS_FITS_IN_INODE(F2FS_INODE(ipage), fi->i_extra_isize,
>>>> -                                i_projid)) {
>>>> +    if (!F2FS_FITS_IN_INODE(ri, fi->i_extra_isize, i_projid)) {
>>>>            err = -EOVERFLOW;
>>>> -        f2fs_put_page(ipage, 1);
>>>>            return err;
>>>
>>> return -EOVERFLOW;
>>>
>>> Thanks,
>>>
>>>>        }
>>>> -    f2fs_put_page(ipage, 1);
>>>>          err = f2fs_dquot_initialize(inode);
>>>>        if (err)
>>> .
>>
>>
>> _______________________________________________
>> Linux-f2fs-devel mailing list
>> Linux-f2fs-devel@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> .


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel