| Message ID | 20240611233906.2873639-1-jtp.park@samsung.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [f2fs-dev] f2fs: add scope based f2fs_putname() cleanup | expand |
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? 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)
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)
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)