* [PATCH] f2fs: avoid unnecessary function call @ 2017-06-30 23:02 Tiezhu Yang 2017-07-01 7:39 ` Jaegeuk Kim 0 siblings, 1 reply; 4+ messages in thread From: Tiezhu Yang @ 2017-06-30 23:02 UTC (permalink / raw) To: jaegeuk; +Cc: linux-f2fs-devel It should call the function fscrypt_fname_free_buffer() in the exception handling only after the function fscrypt_fname_alloc_buffer() returns 0, otherwise it is unnecessary. Signed-off-by: Tiezhu Yang <kernelpatch@126.com> --- fs/f2fs/namei.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c index c31b40e..3225a82 100644 --- a/fs/f2fs/namei.c +++ b/fs/f2fs/namei.c @@ -1036,12 +1036,12 @@ static const char *f2fs_encrypted_get_link(struct dentry *dentry, res = fscrypt_fname_disk_to_usr(inode, 0, 0, &cstr, &pstr); if (res) - goto errout; + goto out; /* this is broken symlink case */ if (unlikely(pstr.name[0] == 0)) { res = -ENOENT; - goto errout; + goto out; } paddr = pstr.name; @@ -1052,8 +1052,9 @@ static const char *f2fs_encrypted_get_link(struct dentry *dentry, put_page(cpage); set_delayed_call(done, kfree_link, paddr); return paddr; -errout: +out: fscrypt_fname_free_buffer(&pstr); +errout: put_page(cpage); return ERR_PTR(res); } -- 1.8.3.1 ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] f2fs: avoid unnecessary function call 2017-06-30 23:02 [PATCH] f2fs: avoid unnecessary function call Tiezhu Yang @ 2017-07-01 7:39 ` Jaegeuk Kim 2017-07-02 16:04 ` Tiezhu Yang 0 siblings, 1 reply; 4+ messages in thread From: Jaegeuk Kim @ 2017-07-01 7:39 UTC (permalink / raw) To: Tiezhu Yang; +Cc: linux-f2fs-devel On 07/01, Tiezhu Yang wrote: > It should call the function fscrypt_fname_free_buffer() in the exception > handling only after the function fscrypt_fname_alloc_buffer() returns 0, > otherwise it is unnecessary. Hi, fscrypt_fname_free_buffer returns if crypto_str is null. So, this flow has no problem, which would be a quite conventional flow, no? Thanks, > Signed-off-by: Tiezhu Yang <kernelpatch@126.com> > --- > fs/f2fs/namei.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c > index c31b40e..3225a82 100644 > --- a/fs/f2fs/namei.c > +++ b/fs/f2fs/namei.c > @@ -1036,12 +1036,12 @@ static const char *f2fs_encrypted_get_link(struct dentry *dentry, > > res = fscrypt_fname_disk_to_usr(inode, 0, 0, &cstr, &pstr); > if (res) > - goto errout; > + goto out; > > /* this is broken symlink case */ > if (unlikely(pstr.name[0] == 0)) { > res = -ENOENT; > - goto errout; > + goto out; > } > > paddr = pstr.name; > @@ -1052,8 +1052,9 @@ static const char *f2fs_encrypted_get_link(struct dentry *dentry, > put_page(cpage); > set_delayed_call(done, kfree_link, paddr); > return paddr; > -errout: > +out: > fscrypt_fname_free_buffer(&pstr); > +errout: > put_page(cpage); > return ERR_PTR(res); > } > -- > 1.8.3.1 ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] f2fs: avoid unnecessary function call 2017-07-01 7:39 ` Jaegeuk Kim @ 2017-07-02 16:04 ` Tiezhu Yang 2017-07-05 2:25 ` Chao Yu 0 siblings, 1 reply; 4+ messages in thread From: Tiezhu Yang @ 2017-07-02 16:04 UTC (permalink / raw) To: Jaegeuk Kim; +Cc: linux-f2fs-devel Hi, Jaegeuk At 2017-07-01 15:39:32, "Jaegeuk Kim" <jaegeuk@kernel.org> wrote: >On 07/01, Tiezhu Yang wrote: >> It should call the function fscrypt_fname_free_buffer() in the exception >> handling only after the function fscrypt_fname_alloc_buffer() returns 0, >> otherwise it is unnecessary. > >Hi, fscrypt_fname_free_buffer returns if crypto_str is null. So, this flow >has no problem, which would be a quite conventional flow, no? Understood, just two points of view, both are OK. In my opinion, I would prefer not to call the function fscrypt_fname_free_buffer since I am absolutely sure that its argument is NULL and it will return directly without doing anything, this patch is to avoid unnecessary function call. Thanks, > >Thanks, > >> Signed-off-by: Tiezhu Yang <kernelpatch@126.com> >> --- >> fs/f2fs/namei.c | 7 ++++--- >> 1 file changed, 4 insertions(+), 3 deletions(-) >> >> diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c >> index c31b40e..3225a82 100644 >> --- a/fs/f2fs/namei.c >> +++ b/fs/f2fs/namei.c >> @@ -1036,12 +1036,12 @@ static const char *f2fs_encrypted_get_link(struct dentry *dentry, >> >> res = fscrypt_fname_disk_to_usr(inode, 0, 0, &cstr, &pstr); >> if (res) >> - goto errout; >> + goto out; >> >> /* this is broken symlink case */ >> if (unlikely(pstr.name[0] == 0)) { >> res = -ENOENT; >> - goto errout; >> + goto out; >> } >> >> paddr = pstr.name; >> @@ -1052,8 +1052,9 @@ static const char *f2fs_encrypted_get_link(struct dentry *dentry, >> put_page(cpage); >> set_delayed_call(done, kfree_link, paddr); >> return paddr; >> -errout: >> +out: >> fscrypt_fname_free_buffer(&pstr); >> +errout: >> put_page(cpage); >> return ERR_PTR(res); >> } >> -- >> 1.8.3.1 ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] f2fs: avoid unnecessary function call 2017-07-02 16:04 ` Tiezhu Yang @ 2017-07-05 2:25 ` Chao Yu 0 siblings, 0 replies; 4+ messages in thread From: Chao Yu @ 2017-07-05 2:25 UTC (permalink / raw) To: Tiezhu Yang, Jaegeuk Kim; +Cc: linux-f2fs-devel Hi all, I'm not against this change, since this follows most error handling rule of f2fs codes that caller will know whether it need to do error handling instead of callee. Would it be better to change as below: --- fs/f2fs/namei.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c index 66d6dd280184..175167a51970 100644 --- a/fs/f2fs/namei.c +++ b/fs/f2fs/namei.c @@ -1086,26 +1086,26 @@ static const char *f2fs_encrypted_get_link(struct dentry *dentry, /* this is broken symlink case */ if (unlikely(cstr.len == 0)) { res = -ENOENT; - goto errout; + goto out_put; } if ((cstr.len + sizeof(struct fscrypt_symlink_data) - 1) > max_size) { /* Symlink data on the disk is corrupted */ res = -EIO; - goto errout; + goto out_put; } res = fscrypt_fname_alloc_buffer(inode, cstr.len, &pstr); if (res) - goto errout; + goto out_put; res = fscrypt_fname_disk_to_usr(inode, 0, 0, &cstr, &pstr); if (res) - goto errout; + goto out_free; /* this is broken symlink case */ if (unlikely(pstr.name[0] == 0)) { res = -ENOENT; - goto errout; + goto out_free; } paddr = pstr.name; @@ -1116,8 +1116,9 @@ static const char *f2fs_encrypted_get_link(struct dentry *dentry, put_page(cpage); set_delayed_call(done, kfree_link, paddr); return paddr; -errout: +out_free: fscrypt_fname_free_buffer(&pstr); +out_put: put_page(cpage); return ERR_PTR(res); } -- 2.13.0.90.g1eb437020 On 2017/7/3 0:04, Tiezhu Yang wrote: > Hi, Jaegeuk > > At 2017-07-01 15:39:32, "Jaegeuk Kim" <jaegeuk@kernel.org> wrote: >> On 07/01, Tiezhu Yang wrote: >>> It should call the function fscrypt_fname_free_buffer() in the exception >>> handling only after the function fscrypt_fname_alloc_buffer() returns 0, >>> otherwise it is unnecessary. >> >> Hi, fscrypt_fname_free_buffer returns if crypto_str is null. So, this flow >> has no problem, which would be a quite conventional flow, no? > > Understood, just two points of view, both are OK. > In my opinion, I would prefer not to call the function fscrypt_fname_free_buffer since I am > absolutely sure that its argument is NULL and it will return directly without doing anything, > this patch is to avoid unnecessary function call. > > Thanks, > >> >> Thanks, >> >>> Signed-off-by: Tiezhu Yang <kernelpatch@126.com> >>> --- >>> fs/f2fs/namei.c | 7 ++++--- >>> 1 file changed, 4 insertions(+), 3 deletions(-) >>> >>> diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c >>> index c31b40e..3225a82 100644 >>> --- a/fs/f2fs/namei.c >>> +++ b/fs/f2fs/namei.c >>> @@ -1036,12 +1036,12 @@ static const char *f2fs_encrypted_get_link(struct dentry *dentry, >>> >>> res = fscrypt_fname_disk_to_usr(inode, 0, 0, &cstr, &pstr); >>> if (res) >>> - goto errout; >>> + goto out; >>> >>> /* this is broken symlink case */ >>> if (unlikely(pstr.name[0] == 0)) { >>> res = -ENOENT; >>> - goto errout; >>> + goto out; >>> } >>> >>> paddr = pstr.name; >>> @@ -1052,8 +1052,9 @@ static const char *f2fs_encrypted_get_link(struct dentry *dentry, >>> put_page(cpage); >>> set_delayed_call(done, kfree_link, paddr); >>> return paddr; >>> -errout: >>> +out: >>> fscrypt_fname_free_buffer(&pstr); >>> +errout: >>> put_page(cpage); >>> return ERR_PTR(res); >>> } >>> -- >>> 1.8.3.1 > ------------------------------------------------------------------------------ > Check out the vibrant tech community on one of the world's most > engaging tech sites, Slashdot.org! http://sdm.link/slashdot > _______________________________________________ > Linux-f2fs-devel mailing list > Linux-f2fs-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel > ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot ^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-07-05 2:26 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-06-30 23:02 [PATCH] f2fs: avoid unnecessary function call Tiezhu Yang 2017-07-01 7:39 ` Jaegeuk Kim 2017-07-02 16:04 ` Tiezhu Yang 2017-07-05 2:25 ` Chao Yu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).