All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] staging: erofs: removing an extra call to iloc() in fill_inode()
@ 2019-08-13 20:38 Pratik Shinde
  2019-08-14  1:59 ` Gao Xiang
  0 siblings, 1 reply; 7+ messages in thread
From: Pratik Shinde @ 2019-08-13 20:38 UTC (permalink / raw)


in fill_inode() we call iloc() twice.Avoiding the extra call by
storing the result.

Signed-off-by: Pratik Shinde <pratikshinde320 at gmail.com>
---
 drivers/staging/erofs/inode.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/erofs/inode.c b/drivers/staging/erofs/inode.c
index 4c3d8bf..d82ba6c 100644
--- a/drivers/staging/erofs/inode.c
+++ b/drivers/staging/erofs/inode.c
@@ -167,11 +167,12 @@ static int fill_inode(struct inode *inode, int isdir)
 	int err;
 	erofs_blk_t blkaddr;
 	unsigned int ofs;
+	erofs_off_t inode_loc;
 
 	trace_erofs_fill_inode(inode, isdir);
-
-	blkaddr = erofs_blknr(iloc(sbi, vi->nid));
-	ofs = erofs_blkoff(iloc(sbi, vi->nid));
+	inode_loc = iloc(sbi, vi->nid);
+	blkaddr = erofs_blknr(inode_loc);
+	ofs = erofs_blkoff(inode_loc);
 
 	debugln("%s, reading inode nid %llu at %u of blkaddr %u",
 		__func__, vi->nid, ofs, blkaddr);
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH] staging: erofs: removing an extra call to iloc() in fill_inode()
  2019-08-14  1:59 ` Gao Xiang
@ 2019-08-14  1:56   ` Chao Yu
  2019-08-14  2:24     ` Gao Xiang
  0 siblings, 1 reply; 7+ messages in thread
From: Chao Yu @ 2019-08-14  1:56 UTC (permalink / raw)


On 2019/8/14 9:59, Gao Xiang wrote:
> Hi Pratik,
> 
> On Wed, Aug 14, 2019@02:08:40AM +0530, Pratik Shinde wrote:
>> in fill_inode() we call iloc() twice.Avoiding the extra call by
>> storing the result.
>>
>> Signed-off-by: Pratik Shinde <pratikshinde320 at gmail.com>
> 
> I have no objection of this patch, but I'd like to
> hear Chao/Greg's idea about this...

It looks more clean. :)

Nitpick, maybe change 'inode_loc' to shorter 'iloc' will be better.

Reviewed-by: Chao Yu <yuchao0 at huawei.com>

Thanks,

> 
> Thanks,
> Gao Xiang
> 
>> ---
>>  drivers/staging/erofs/inode.c | 7 ++++---
>>  1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/staging/erofs/inode.c b/drivers/staging/erofs/inode.c
>> index 4c3d8bf..d82ba6c 100644
>> --- a/drivers/staging/erofs/inode.c
>> +++ b/drivers/staging/erofs/inode.c
>> @@ -167,11 +167,12 @@ static int fill_inode(struct inode *inode, int isdir)
>>  	int err;
>>  	erofs_blk_t blkaddr;
>>  	unsigned int ofs;
>> +	erofs_off_t inode_loc;
>>  
>>  	trace_erofs_fill_inode(inode, isdir);
>> -
>> -	blkaddr = erofs_blknr(iloc(sbi, vi->nid));
>> -	ofs = erofs_blkoff(iloc(sbi, vi->nid));
>> +	inode_loc = iloc(sbi, vi->nid);
>> +	blkaddr = erofs_blknr(inode_loc);
>> +	ofs = erofs_blkoff(inode_loc);
>>  
>>  	debugln("%s, reading inode nid %llu at %u of blkaddr %u",
>>  		__func__, vi->nid, ofs, blkaddr);
>> -- 
>> 2.9.3
>>
> .
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH] staging: erofs: removing an extra call to iloc() in fill_inode()
  2019-08-13 20:38 [PATCH] staging: erofs: removing an extra call to iloc() in fill_inode() Pratik Shinde
@ 2019-08-14  1:59 ` Gao Xiang
  2019-08-14  1:56   ` Chao Yu
  0 siblings, 1 reply; 7+ messages in thread
From: Gao Xiang @ 2019-08-14  1:59 UTC (permalink / raw)


Hi Pratik,

On Wed, Aug 14, 2019@02:08:40AM +0530, Pratik Shinde wrote:
> in fill_inode() we call iloc() twice.Avoiding the extra call by
> storing the result.
> 
> Signed-off-by: Pratik Shinde <pratikshinde320 at gmail.com>

I have no objection of this patch, but I'd like to
hear Chao/Greg's idea about this...

Thanks,
Gao Xiang

> ---
>  drivers/staging/erofs/inode.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/erofs/inode.c b/drivers/staging/erofs/inode.c
> index 4c3d8bf..d82ba6c 100644
> --- a/drivers/staging/erofs/inode.c
> +++ b/drivers/staging/erofs/inode.c
> @@ -167,11 +167,12 @@ static int fill_inode(struct inode *inode, int isdir)
>  	int err;
>  	erofs_blk_t blkaddr;
>  	unsigned int ofs;
> +	erofs_off_t inode_loc;
>  
>  	trace_erofs_fill_inode(inode, isdir);
> -
> -	blkaddr = erofs_blknr(iloc(sbi, vi->nid));
> -	ofs = erofs_blkoff(iloc(sbi, vi->nid));
> +	inode_loc = iloc(sbi, vi->nid);
> +	blkaddr = erofs_blknr(inode_loc);
> +	ofs = erofs_blkoff(inode_loc);
>  
>  	debugln("%s, reading inode nid %llu at %u of blkaddr %u",
>  		__func__, vi->nid, ofs, blkaddr);
> -- 
> 2.9.3
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH] staging: erofs: removing an extra call to iloc() in fill_inode()
  2019-08-14  1:56   ` Chao Yu
@ 2019-08-14  2:24     ` Gao Xiang
  2019-08-14  3:52       ` Pratik Shinde
  0 siblings, 1 reply; 7+ messages in thread
From: Gao Xiang @ 2019-08-14  2:24 UTC (permalink / raw)


On Wed, Aug 14, 2019@09:56:09AM +0800, Chao Yu wrote:
> On 2019/8/14 9:59, Gao Xiang wrote:
> > Hi Pratik,
> > 
> > On Wed, Aug 14, 2019@02:08:40AM +0530, Pratik Shinde wrote:
> >> in fill_inode() we call iloc() twice.Avoiding the extra call by
> >> storing the result.
> >>
> >> Signed-off-by: Pratik Shinde <pratikshinde320 at gmail.com>
> > 
> > I have no objection of this patch, but I'd like to
> > hear Chao/Greg's idea about this...
> 
> It looks more clean. :)
> 
> Nitpick, maybe change 'inode_loc' to shorter 'iloc' will be better.

iloc is the name of static inline helper function in internal.h
used for shorter lines...

Thanks,
Gao Xiang

> 
> Reviewed-by: Chao Yu <yuchao0 at huawei.com>
> 
> Thanks,
> 
> > 
> > Thanks,
> > Gao Xiang
> > 
> >> ---
> >>  drivers/staging/erofs/inode.c | 7 ++++---
> >>  1 file changed, 4 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/staging/erofs/inode.c b/drivers/staging/erofs/inode.c
> >> index 4c3d8bf..d82ba6c 100644
> >> --- a/drivers/staging/erofs/inode.c
> >> +++ b/drivers/staging/erofs/inode.c
> >> @@ -167,11 +167,12 @@ static int fill_inode(struct inode *inode, int isdir)
> >>  	int err;
> >>  	erofs_blk_t blkaddr;
> >>  	unsigned int ofs;
> >> +	erofs_off_t inode_loc;
> >>  
> >>  	trace_erofs_fill_inode(inode, isdir);
> >> -
> >> -	blkaddr = erofs_blknr(iloc(sbi, vi->nid));
> >> -	ofs = erofs_blkoff(iloc(sbi, vi->nid));
> >> +	inode_loc = iloc(sbi, vi->nid);
> >> +	blkaddr = erofs_blknr(inode_loc);
> >> +	ofs = erofs_blkoff(inode_loc);
> >>  
> >>  	debugln("%s, reading inode nid %llu at %u of blkaddr %u",
> >>  		__func__, vi->nid, ofs, blkaddr);
> >> -- 
> >> 2.9.3
> >>
> > .
> > 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH] staging: erofs: removing an extra call to iloc() in fill_inode()
  2019-08-14  2:24     ` Gao Xiang
@ 2019-08-14  3:52       ` Pratik Shinde
  2019-08-14  6:36         ` Chao Yu
  2019-08-14  8:02         ` Gao Xiang
  0 siblings, 2 replies; 7+ messages in thread
From: Pratik Shinde @ 2019-08-14  3:52 UTC (permalink / raw)


Yes.since we already have a function with same name (and we are using it in
same context).
'inode_loc' was the most meaningful name I could come up with :)

--Pratik.

On Wed, Aug 14, 2019@7:37 AM Gao Xiang <gaoxiang25@huawei.com> wrote:

> On Wed, Aug 14, 2019@09:56:09AM +0800, Chao Yu wrote:
> > On 2019/8/14 9:59, Gao Xiang wrote:
> > > Hi Pratik,
> > >
> > > On Wed, Aug 14, 2019@02:08:40AM +0530, Pratik Shinde wrote:
> > >> in fill_inode() we call iloc() twice.Avoiding the extra call by
> > >> storing the result.
> > >>
> > >> Signed-off-by: Pratik Shinde <pratikshinde320 at gmail.com>
> > >
> > > I have no objection of this patch, but I'd like to
> > > hear Chao/Greg's idea about this...
> >
> > It looks more clean. :)
> >
> > Nitpick, maybe change 'inode_loc' to shorter 'iloc' will be better.
>
> iloc is the name of static inline helper function in internal.h
> used for shorter lines...
>
> Thanks,
> Gao Xiang
>
> >
> > Reviewed-by: Chao Yu <yuchao0 at huawei.com>
> >
> > Thanks,
> >
> > >
> > > Thanks,
> > > Gao Xiang
> > >
> > >> ---
> > >>  drivers/staging/erofs/inode.c | 7 ++++---
> > >>  1 file changed, 4 insertions(+), 3 deletions(-)
> > >>
> > >> diff --git a/drivers/staging/erofs/inode.c
> b/drivers/staging/erofs/inode.c
> > >> index 4c3d8bf..d82ba6c 100644
> > >> --- a/drivers/staging/erofs/inode.c
> > >> +++ b/drivers/staging/erofs/inode.c
> > >> @@ -167,11 +167,12 @@ static int fill_inode(struct inode *inode, int
> isdir)
> > >>    int err;
> > >>    erofs_blk_t blkaddr;
> > >>    unsigned int ofs;
> > >> +  erofs_off_t inode_loc;
> > >>
> > >>    trace_erofs_fill_inode(inode, isdir);
> > >> -
> > >> -  blkaddr = erofs_blknr(iloc(sbi, vi->nid));
> > >> -  ofs = erofs_blkoff(iloc(sbi, vi->nid));
> > >> +  inode_loc = iloc(sbi, vi->nid);
> > >> +  blkaddr = erofs_blknr(inode_loc);
> > >> +  ofs = erofs_blkoff(inode_loc);
> > >>
> > >>    debugln("%s, reading inode nid %llu at %u of blkaddr %u",
> > >>            __func__, vi->nid, ofs, blkaddr);
> > >> --
> > >> 2.9.3
> > >>
> > > .
> > >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ozlabs.org/pipermail/linux-erofs/attachments/20190814/7db46d5c/attachment.htm>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH] staging: erofs: removing an extra call to iloc() in fill_inode()
  2019-08-14  3:52       ` Pratik Shinde
@ 2019-08-14  6:36         ` Chao Yu
  2019-08-14  8:02         ` Gao Xiang
  1 sibling, 0 replies; 7+ messages in thread
From: Chao Yu @ 2019-08-14  6:36 UTC (permalink / raw)


On 2019/8/14 11:52, Pratik Shinde wrote:
> Yes.since we already have a function with same name (and we are using it in same
> context).
> 'inode_loc' was the most meaningful name I could come up with :)

[snip]

On Wed, Aug 14, 2019@7:37 AM Gao Xiang <gaoxiang25@huawei.com> wrote:
> iloc is the name of static inline helper function in internal.h
> used for shorter lines...

Correct, so let's keep as it is.

Thanks,

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH] staging: erofs: removing an extra call to iloc() in fill_inode()
  2019-08-14  3:52       ` Pratik Shinde
  2019-08-14  6:36         ` Chao Yu
@ 2019-08-14  8:02         ` Gao Xiang
  1 sibling, 0 replies; 7+ messages in thread
From: Gao Xiang @ 2019-08-14  8:02 UTC (permalink / raw)


On Wed, Aug 14, 2019@09:22:53AM +0530, Pratik Shinde wrote:
> Yes.since we already have a function with same name (and we are using it in
> same context).
> 'inode_loc' was the most meaningful name I could come up with :)
> 
> --Pratik.

And one more small suggestion... see the following,
https://lore.kernel.org/lkml/20190805044225.GA14087 at kroah.com/

Happy hacking! :)

Thanks,
Gao Xiang

> 
> On Wed, Aug 14, 2019@7:37 AM Gao Xiang <gaoxiang25@huawei.com> wrote:
> 
> > On Wed, Aug 14, 2019@09:56:09AM +0800, Chao Yu wrote:
> > > On 2019/8/14 9:59, Gao Xiang wrote:
> > > > Hi Pratik,
> > > >
> > > > On Wed, Aug 14, 2019@02:08:40AM +0530, Pratik Shinde wrote:
> > > >> in fill_inode() we call iloc() twice.Avoiding the extra call by
> > > >> storing the result.
> > > >>
> > > >> Signed-off-by: Pratik Shinde <pratikshinde320 at gmail.com>
> > > >
> > > > I have no objection of this patch, but I'd like to
> > > > hear Chao/Greg's idea about this...
> > >
> > > It looks more clean. :)
> > >
> > > Nitpick, maybe change 'inode_loc' to shorter 'iloc' will be better.
> >
> > iloc is the name of static inline helper function in internal.h
> > used for shorter lines...
> >
> > Thanks,
> > Gao Xiang
> >
> > >
> > > Reviewed-by: Chao Yu <yuchao0 at huawei.com>
> > >
> > > Thanks,
> > >
> > > >
> > > > Thanks,
> > > > Gao Xiang
> > > >
> > > >> ---
> > > >>  drivers/staging/erofs/inode.c | 7 ++++---
> > > >>  1 file changed, 4 insertions(+), 3 deletions(-)
> > > >>
> > > >> diff --git a/drivers/staging/erofs/inode.c
> > b/drivers/staging/erofs/inode.c
> > > >> index 4c3d8bf..d82ba6c 100644
> > > >> --- a/drivers/staging/erofs/inode.c
> > > >> +++ b/drivers/staging/erofs/inode.c
> > > >> @@ -167,11 +167,12 @@ static int fill_inode(struct inode *inode, int
> > isdir)
> > > >>    int err;
> > > >>    erofs_blk_t blkaddr;
> > > >>    unsigned int ofs;
> > > >> +  erofs_off_t inode_loc;
> > > >>
> > > >>    trace_erofs_fill_inode(inode, isdir);
> > > >> -
> > > >> -  blkaddr = erofs_blknr(iloc(sbi, vi->nid));
> > > >> -  ofs = erofs_blkoff(iloc(sbi, vi->nid));
> > > >> +  inode_loc = iloc(sbi, vi->nid);
> > > >> +  blkaddr = erofs_blknr(inode_loc);
> > > >> +  ofs = erofs_blkoff(inode_loc);
> > > >>
> > > >>    debugln("%s, reading inode nid %llu at %u of blkaddr %u",
> > > >>            __func__, vi->nid, ofs, blkaddr);
> > > >> --
> > > >> 2.9.3
> > > >>
> > > > .
> > > >
> >

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2019-08-14  8:02 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-08-13 20:38 [PATCH] staging: erofs: removing an extra call to iloc() in fill_inode() Pratik Shinde
2019-08-14  1:59 ` Gao Xiang
2019-08-14  1:56   ` Chao Yu
2019-08-14  2:24     ` Gao Xiang
2019-08-14  3:52       ` Pratik Shinde
2019-08-14  6:36         ` Chao Yu
2019-08-14  8:02         ` Gao Xiang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.