diff mbox series

[f2fs-dev,4/8] f2fs: factor the read/write tracing logic into a helper

Message ID 20230119063625.466485-5-hch@lst.de (mailing list archive)
State New
Headers show
Series [f2fs-dev,1/8] f2fs: remove __add_sum_entry | expand

Commit Message

Christoph Hellwig Jan. 19, 2023, 6:36 a.m. UTC
Factor the logic to log a path for reads and writs into a helper
shared between the read_iter and write_iter methods.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/f2fs/file.c | 60 +++++++++++++++++++++-----------------------------
 1 file changed, 25 insertions(+), 35 deletions(-)

Comments

Chao Yu Jan. 29, 2023, 10:36 a.m. UTC | #1
On 2023/1/19 14:36, Christoph Hellwig wrote:
> Factor the logic to log a path for reads and writs into a helper
> shared between the read_iter and write_iter methods.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>   fs/f2fs/file.c | 60 +++++++++++++++++++++-----------------------------
>   1 file changed, 25 insertions(+), 35 deletions(-)
> 
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index f5c1b78149540c..305be6ac024196 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -4340,6 +4340,27 @@ static ssize_t f2fs_dio_read_iter(struct kiocb *iocb, struct iov_iter *to)
>   	return ret;
>   }
>   
> +static void f2fs_trace_rw_file_path(struct kiocb *iocb, size_t count, int rw)
> +{
> +	struct inode *inode = file_inode(iocb->ki_filp);
> +	char *buf, *path;
> +
> +	buf = f2fs_kmalloc(F2FS_I_SB(inode), PATH_MAX, GFP_KERNEL);
> +	if (!buf)
> +		return;
> +	path = dentry_path_raw(file_dentry(iocb->ki_filp), buf, PATH_MAX);
> +	if (IS_ERR(path))
> +		goto free_buf;
> +	if (rw == WRITE)
> +		trace_f2fs_datawrite_start(inode, iocb->ki_pos, count,
> +				current->pid, path, current->comm);
> +	else
> +		trace_f2fs_dataread_start(inode, iocb->ki_pos, count,
> +				current->pid, path, current->comm);
> +free_buf:
> +	kfree(buf);
> +}
> +
>   static ssize_t f2fs_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
>   {
>   	struct inode *inode = file_inode(iocb->ki_filp);
> @@ -4349,24 +4370,9 @@ static ssize_t f2fs_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
>   	if (!f2fs_is_compress_backend_ready(inode))
>   		return -EOPNOTSUPP;
>   
> -	if (trace_f2fs_dataread_start_enabled()) {
> -		char *p = f2fs_kmalloc(F2FS_I_SB(inode), PATH_MAX, GFP_KERNEL);
> -		char *path;
> -
> -		if (!p)
> -			goto skip_read_trace;
> +	if (trace_f2fs_dataread_start_enabled())
> +		f2fs_trace_rw_file_path(iocb, iov_iter_count(to), READ);
>   
> -		path = dentry_path_raw(file_dentry(iocb->ki_filp), p, PATH_MAX);
> -		if (IS_ERR(path)) {
> -			kfree(p);
> -			goto skip_read_trace;
> -		}
> -
> -		trace_f2fs_dataread_start(inode, pos, iov_iter_count(to),
> -					current->pid, path, current->comm);
> -		kfree(p);
> -	}
> -skip_read_trace:
>   	if (f2fs_should_use_dio(inode, iocb, to)) {
>   		ret = f2fs_dio_read_iter(iocb, to);
>   	} else {
> @@ -4672,24 +4678,8 @@ static ssize_t f2fs_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
>   	if (preallocated < 0) {
>   		ret = preallocated;
>   	} else {
> -		if (trace_f2fs_datawrite_start_enabled()) {
> -			char *p = f2fs_kmalloc(F2FS_I_SB(inode),
> -						PATH_MAX, GFP_KERNEL);
> -			char *path;
> -
> -			if (!p)
> -				goto skip_write_trace;
> -			path = dentry_path_raw(file_dentry(iocb->ki_filp),
> -								p, PATH_MAX);
> -			if (IS_ERR(path)) {
> -				kfree(p);
> -				goto skip_write_trace;
> -			}
> -			trace_f2fs_datawrite_start(inode, orig_pos, orig_count,
> -					current->pid, path, current->comm);
> -			kfree(p);
> -		}
> -skip_write_trace:
> +		f2fs_trace_rw_file_path(iocb, orig_count, WRITE);

if (trace_f2fs_datawrite_start_enabled())
	f2fs_trace_rw_file_path(..);

Thanks,

> +
>   		/* Do the actual write. */
>   		ret = dio ?
>   			f2fs_dio_write_iter(iocb, from, &may_need_sync) :
Jaegeuk Kim Jan. 30, 2023, 11 p.m. UTC | #2
On 01/29, Chao Yu wrote:
> On 2023/1/19 14:36, Christoph Hellwig wrote:
> > Factor the logic to log a path for reads and writs into a helper
> > shared between the read_iter and write_iter methods.
> > 
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > ---
> >   fs/f2fs/file.c | 60 +++++++++++++++++++++-----------------------------
> >   1 file changed, 25 insertions(+), 35 deletions(-)
> > 
> > diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> > index f5c1b78149540c..305be6ac024196 100644
> > --- a/fs/f2fs/file.c
> > +++ b/fs/f2fs/file.c
> > @@ -4340,6 +4340,27 @@ static ssize_t f2fs_dio_read_iter(struct kiocb *iocb, struct iov_iter *to)
> >   	return ret;
> >   }
> > +static void f2fs_trace_rw_file_path(struct kiocb *iocb, size_t count, int rw)
> > +{
> > +	struct inode *inode = file_inode(iocb->ki_filp);
> > +	char *buf, *path;
> > +
> > +	buf = f2fs_kmalloc(F2FS_I_SB(inode), PATH_MAX, GFP_KERNEL);
> > +	if (!buf)
> > +		return;
> > +	path = dentry_path_raw(file_dentry(iocb->ki_filp), buf, PATH_MAX);
> > +	if (IS_ERR(path))
> > +		goto free_buf;
> > +	if (rw == WRITE)
> > +		trace_f2fs_datawrite_start(inode, iocb->ki_pos, count,
> > +				current->pid, path, current->comm);
> > +	else
> > +		trace_f2fs_dataread_start(inode, iocb->ki_pos, count,
> > +				current->pid, path, current->comm);
> > +free_buf:
> > +	kfree(buf);
> > +}
> > +
> >   static ssize_t f2fs_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
> >   {
> >   	struct inode *inode = file_inode(iocb->ki_filp);
> > @@ -4349,24 +4370,9 @@ static ssize_t f2fs_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
> >   	if (!f2fs_is_compress_backend_ready(inode))
> >   		return -EOPNOTSUPP;
> > -	if (trace_f2fs_dataread_start_enabled()) {
> > -		char *p = f2fs_kmalloc(F2FS_I_SB(inode), PATH_MAX, GFP_KERNEL);
> > -		char *path;
> > -
> > -		if (!p)
> > -			goto skip_read_trace;
> > +	if (trace_f2fs_dataread_start_enabled())
> > +		f2fs_trace_rw_file_path(iocb, iov_iter_count(to), READ);
> > -		path = dentry_path_raw(file_dentry(iocb->ki_filp), p, PATH_MAX);
> > -		if (IS_ERR(path)) {
> > -			kfree(p);
> > -			goto skip_read_trace;
> > -		}
> > -
> > -		trace_f2fs_dataread_start(inode, pos, iov_iter_count(to),
> > -					current->pid, path, current->comm);
> > -		kfree(p);
> > -	}
> > -skip_read_trace:
> >   	if (f2fs_should_use_dio(inode, iocb, to)) {
> >   		ret = f2fs_dio_read_iter(iocb, to);
> >   	} else {
> > @@ -4672,24 +4678,8 @@ static ssize_t f2fs_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
> >   	if (preallocated < 0) {
> >   		ret = preallocated;
> >   	} else {
> > -		if (trace_f2fs_datawrite_start_enabled()) {
> > -			char *p = f2fs_kmalloc(F2FS_I_SB(inode),
> > -						PATH_MAX, GFP_KERNEL);
> > -			char *path;
> > -
> > -			if (!p)
> > -				goto skip_write_trace;
> > -			path = dentry_path_raw(file_dentry(iocb->ki_filp),
> > -								p, PATH_MAX);
> > -			if (IS_ERR(path)) {
> > -				kfree(p);
> > -				goto skip_write_trace;
> > -			}
> > -			trace_f2fs_datawrite_start(inode, orig_pos, orig_count,
> > -					current->pid, path, current->comm);
> > -			kfree(p);
> > -		}
> > -skip_write_trace:
> > +		f2fs_trace_rw_file_path(iocb, orig_count, WRITE);
> 
> if (trace_f2fs_datawrite_start_enabled())
> 	f2fs_trace_rw_file_path(..);

I queued the patch with this change in dev-test first.

Thanks,

> 
> Thanks,
> 
> > +
> >   		/* Do the actual write. */
> >   		ret = dio ?
> >   			f2fs_dio_write_iter(iocb, from, &may_need_sync) :
diff mbox series

Patch

diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index f5c1b78149540c..305be6ac024196 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -4340,6 +4340,27 @@  static ssize_t f2fs_dio_read_iter(struct kiocb *iocb, struct iov_iter *to)
 	return ret;
 }
 
+static void f2fs_trace_rw_file_path(struct kiocb *iocb, size_t count, int rw)
+{
+	struct inode *inode = file_inode(iocb->ki_filp);
+	char *buf, *path;
+
+	buf = f2fs_kmalloc(F2FS_I_SB(inode), PATH_MAX, GFP_KERNEL);
+	if (!buf)
+		return;
+	path = dentry_path_raw(file_dentry(iocb->ki_filp), buf, PATH_MAX);
+	if (IS_ERR(path))
+		goto free_buf;
+	if (rw == WRITE)
+		trace_f2fs_datawrite_start(inode, iocb->ki_pos, count,
+				current->pid, path, current->comm);
+	else
+		trace_f2fs_dataread_start(inode, iocb->ki_pos, count,
+				current->pid, path, current->comm);
+free_buf:
+	kfree(buf);
+}
+
 static ssize_t f2fs_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
 {
 	struct inode *inode = file_inode(iocb->ki_filp);
@@ -4349,24 +4370,9 @@  static ssize_t f2fs_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
 	if (!f2fs_is_compress_backend_ready(inode))
 		return -EOPNOTSUPP;
 
-	if (trace_f2fs_dataread_start_enabled()) {
-		char *p = f2fs_kmalloc(F2FS_I_SB(inode), PATH_MAX, GFP_KERNEL);
-		char *path;
-
-		if (!p)
-			goto skip_read_trace;
+	if (trace_f2fs_dataread_start_enabled())
+		f2fs_trace_rw_file_path(iocb, iov_iter_count(to), READ);
 
-		path = dentry_path_raw(file_dentry(iocb->ki_filp), p, PATH_MAX);
-		if (IS_ERR(path)) {
-			kfree(p);
-			goto skip_read_trace;
-		}
-
-		trace_f2fs_dataread_start(inode, pos, iov_iter_count(to),
-					current->pid, path, current->comm);
-		kfree(p);
-	}
-skip_read_trace:
 	if (f2fs_should_use_dio(inode, iocb, to)) {
 		ret = f2fs_dio_read_iter(iocb, to);
 	} else {
@@ -4672,24 +4678,8 @@  static ssize_t f2fs_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
 	if (preallocated < 0) {
 		ret = preallocated;
 	} else {
-		if (trace_f2fs_datawrite_start_enabled()) {
-			char *p = f2fs_kmalloc(F2FS_I_SB(inode),
-						PATH_MAX, GFP_KERNEL);
-			char *path;
-
-			if (!p)
-				goto skip_write_trace;
-			path = dentry_path_raw(file_dentry(iocb->ki_filp),
-								p, PATH_MAX);
-			if (IS_ERR(path)) {
-				kfree(p);
-				goto skip_write_trace;
-			}
-			trace_f2fs_datawrite_start(inode, orig_pos, orig_count,
-					current->pid, path, current->comm);
-			kfree(p);
-		}
-skip_write_trace:
+		f2fs_trace_rw_file_path(iocb, orig_count, WRITE);
+
 		/* Do the actual write. */
 		ret = dio ?
 			f2fs_dio_write_iter(iocb, from, &may_need_sync) :