linux-f2fs-devel.lists.sourceforge.net archive mirror
 help / color / mirror / Atom feed
From: Jaegeuk Kim <jaegeuk@kernel.org>
To: Chao Yu <chao@kernel.org>
Cc: jtp.park@samsung.com, Jeongtae Park <jeongtae.park@gmail.com>,
	linux-f2fs-devel@lists.sourceforge.net
Subject: Re: [f2fs-dev] [PATCH] f2fs: add scope based f2fs_putname() cleanup
Date: Mon, 24 Jun 2024 17:47:14 +0000	[thread overview]
Message-ID: <ZnmxIuq7sqopF-cJ@google.com> (raw)
In-Reply-To: <4a79eea2-e360-493d-8c98-d93aacbd4d00@kernel.org>

On 06/18, Chao Yu wrote:
> On 2024/6/12 7:39, jtp.park@samsung.com wrote:
> > From: Jeongtae Park <jtp.park@samsung.com>
> > 
> > This patch adds a new scope based f2fs_putname() cleanup to reduce
> > the chances of forgetting a f2fs_putname(). And doing so removes
> 
> Actually, f2fs_trace_rw_file_path() won't change frequently, so the risk
> of forgetting f2fs_putname() here is very low.
> 
> > a goto statement for error handling.
> 
> The code logic is fine to me, but not sure whether we should apply this.
> 
> Jaegeuk, any comments?

IMO, we don't need this since it's a very trivial path.

> 
> Thanks,
> 
> > 
> > Signed-off-by: Jeongtae Park <jtp.park@samsung.com>
> > ---
> >   fs/f2fs/f2fs.h | 2 ++
> >   fs/f2fs/file.c | 8 +++-----
> >   2 files changed, 5 insertions(+), 5 deletions(-)
> > 
> > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > index 1974b6aff397..284024c12ee5 100644
> > --- a/fs/f2fs/f2fs.h
> > +++ b/fs/f2fs/f2fs.h
> > @@ -3402,6 +3402,8 @@ static inline void f2fs_putname(char *buf)
> >   	__putname(buf);
> >   }
> > +DEFINE_FREE(f2fs_putname, void *, if (_T) f2fs_putname(_T))
> > +
> >   static inline void *f2fs_kzalloc(struct f2fs_sb_info *sbi,
> >   					size_t size, gfp_t flags)
> >   {
> > diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> > index 5c0b281a70f3..c783d017ed28 100644
> > --- a/fs/f2fs/file.c
> > +++ b/fs/f2fs/file.c
> > @@ -4511,22 +4511,20 @@ static void f2fs_trace_rw_file_path(struct file *file, loff_t pos, size_t count,
> >   				    int rw)
> >   {
> >   	struct inode *inode = file_inode(file);
> > -	char *buf, *path;
> > +	char *buf __free(f2fs_putname) = f2fs_getname(F2FS_I_SB(inode));
> > +	char *path;
> > -	buf = f2fs_getname(F2FS_I_SB(inode));
> >   	if (!buf)
> >   		return;
> >   	path = dentry_path_raw(file_dentry(file), buf, PATH_MAX);
> >   	if (IS_ERR(path))
> > -		goto free_buf;
> > +		return;
> >   	if (rw == WRITE)
> >   		trace_f2fs_datawrite_start(inode, pos, count,
> >   				current->pid, path, current->comm);
> >   	else
> >   		trace_f2fs_dataread_start(inode, pos, count,
> >   				current->pid, path, current->comm);
> > -free_buf:
> > -	f2fs_putname(buf);
> >   }
> >   static ssize_t f2fs_file_read_iter(struct kiocb *iocb, struct iov_iter *to)


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

      reply	other threads:[~2024-06-24 17:47 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CGME20240611234050epcas2p40bd72d91c6a7b67e411a5ebfcddbcb97@epcas2p4.samsung.com>
2024-06-11 23:39 ` [f2fs-dev] [PATCH] f2fs: add scope based f2fs_putname() cleanup jtp.park
2024-06-18  1:32   ` Chao Yu
2024-06-24 17:47     ` Jaegeuk Kim [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZnmxIuq7sqopF-cJ@google.com \
    --to=jaegeuk@kernel.org \
    --cc=chao@kernel.org \
    --cc=jeongtae.park@gmail.com \
    --cc=jtp.park@samsung.com \
    --cc=linux-f2fs-devel@lists.sourceforge.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).