* [f2fs-dev] [PATCH] f2fs: fix the assign logic of iocb
@ 2022-10-19 16:17 ` Mukesh Ojha
0 siblings, 0 replies; 12+ messages in thread
From: Mukesh Ojha @ 2022-10-19 16:17 UTC (permalink / raw)
To: jaegeuk, chao, mhiramat; +Cc: quic_mojha, linux-kernel, linux-f2fs-devel
commit 18ae8d12991b ("f2fs: show more DIO information in tracepoint")
introduces iocb field in 'f2fs_direct_IO_enter' trace event
And it only assigns the pointer and later it accesses its field
in trace print log.
Fix it by correcting data type and memcpy the content of iocb.
Fixes: 18ae8d12991b ("f2fs: show more DIO information in tracepoint")
Signed-off-by: Mukesh Ojha <quic_mojha@quicinc.com>
---
include/trace/events/f2fs.h | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/include/trace/events/f2fs.h b/include/trace/events/f2fs.h
index c6b3724..7727ec9 100644
--- a/include/trace/events/f2fs.h
+++ b/include/trace/events/f2fs.h
@@ -940,7 +940,7 @@ TRACE_EVENT(f2fs_direct_IO_enter,
TP_STRUCT__entry(
__field(dev_t, dev)
__field(ino_t, ino)
- __field(struct kiocb *, iocb)
+ __field_struct(struct kiocb, iocb)
__field(unsigned long, len)
__field(int, rw)
),
@@ -948,17 +948,17 @@ TRACE_EVENT(f2fs_direct_IO_enter,
TP_fast_assign(
__entry->dev = inode->i_sb->s_dev;
__entry->ino = inode->i_ino;
- __entry->iocb = iocb;
+ memcpy(&__entry->iocb, iocb, sizeof(*iocb));
__entry->len = len;
__entry->rw = rw;
),
TP_printk("dev = (%d,%d), ino = %lu pos = %lld len = %lu ki_flags = %x ki_ioprio = %x rw = %d",
show_dev_ino(__entry),
- __entry->iocb->ki_pos,
+ __entry->iocb.ki_pos,
__entry->len,
- __entry->iocb->ki_flags,
- __entry->iocb->ki_ioprio,
+ __entry->iocb.ki_flags,
+ __entry->iocb.ki_ioprio,
__entry->rw)
);
--
2.7.4
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH] f2fs: fix the assign logic of iocb
2022-10-19 16:17 ` [f2fs-dev] " Mukesh Ojha
@ 2022-10-20 9:01 ` Chao Yu
-1 siblings, 0 replies; 12+ messages in thread
From: Chao Yu @ 2022-10-20 9:01 UTC (permalink / raw)
To: Mukesh Ojha, jaegeuk, mhiramat; +Cc: linux-f2fs-devel, linux-kernel
On 2022/10/20 0:17, Mukesh Ojha wrote:
> commit 18ae8d12991b ("f2fs: show more DIO information in tracepoint")
> introduces iocb field in 'f2fs_direct_IO_enter' trace event
> And it only assigns the pointer and later it accesses its field
> in trace print log.
>
> Fix it by correcting data type and memcpy the content of iocb.
So the implementation below is wrong, right? since it just assign __entry->name
with dentry->d_name.name rather than copyiny entirely, so that, during printing
of tracepoint, __entry->name may be invalid.
TRACE_EVENT(f2fs_unlink_enter,
TP_PROTO(struct inode *dir, struct dentry *dentry),
TP_ARGS(dir, dentry),
TP_STRUCT__entry(
__field(dev_t, dev)
__field(ino_t, ino)
__field(loff_t, size)
__field(blkcnt_t, blocks)
__field(const char *, name)
),
TP_fast_assign(
__entry->dev = dir->i_sb->s_dev;
__entry->ino = dir->i_ino;
__entry->size = dir->i_size;
__entry->blocks = dir->i_blocks;
__entry->name = dentry->d_name.name;
),
>
> Fixes: 18ae8d12991b ("f2fs: show more DIO information in tracepoint")
> Signed-off-by: Mukesh Ojha <quic_mojha@quicinc.com>
> ---
> include/trace/events/f2fs.h | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/include/trace/events/f2fs.h b/include/trace/events/f2fs.h
> index c6b3724..7727ec9 100644
> --- a/include/trace/events/f2fs.h
> +++ b/include/trace/events/f2fs.h
> @@ -940,7 +940,7 @@ TRACE_EVENT(f2fs_direct_IO_enter,
> TP_STRUCT__entry(
> __field(dev_t, dev)
> __field(ino_t, ino)
> - __field(struct kiocb *, iocb)
> + __field_struct(struct kiocb, iocb)
> __field(unsigned long, len)
> __field(int, rw)
> ),
> @@ -948,17 +948,17 @@ TRACE_EVENT(f2fs_direct_IO_enter,
> TP_fast_assign(
> __entry->dev = inode->i_sb->s_dev;
> __entry->ino = inode->i_ino;
> - __entry->iocb = iocb;
> + memcpy(&__entry->iocb, iocb, sizeof(*iocb));
> __entry->len = len;
> __entry->rw = rw;
> ),
>
> TP_printk("dev = (%d,%d), ino = %lu pos = %lld len = %lu ki_flags = %x ki_ioprio = %x rw = %d",
> show_dev_ino(__entry),
> - __entry->iocb->ki_pos,
> + __entry->iocb.ki_pos,
> __entry->len,
> - __entry->iocb->ki_flags,
> - __entry->iocb->ki_ioprio,
> + __entry->iocb.ki_flags,
> + __entry->iocb.ki_ioprio,
> __entry->rw)
> );
>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [f2fs-dev] [PATCH] f2fs: fix the assign logic of iocb
@ 2022-10-20 9:01 ` Chao Yu
0 siblings, 0 replies; 12+ messages in thread
From: Chao Yu @ 2022-10-20 9:01 UTC (permalink / raw)
To: Mukesh Ojha, jaegeuk, mhiramat; +Cc: linux-kernel, linux-f2fs-devel
On 2022/10/20 0:17, Mukesh Ojha wrote:
> commit 18ae8d12991b ("f2fs: show more DIO information in tracepoint")
> introduces iocb field in 'f2fs_direct_IO_enter' trace event
> And it only assigns the pointer and later it accesses its field
> in trace print log.
>
> Fix it by correcting data type and memcpy the content of iocb.
So the implementation below is wrong, right? since it just assign __entry->name
with dentry->d_name.name rather than copyiny entirely, so that, during printing
of tracepoint, __entry->name may be invalid.
TRACE_EVENT(f2fs_unlink_enter,
TP_PROTO(struct inode *dir, struct dentry *dentry),
TP_ARGS(dir, dentry),
TP_STRUCT__entry(
__field(dev_t, dev)
__field(ino_t, ino)
__field(loff_t, size)
__field(blkcnt_t, blocks)
__field(const char *, name)
),
TP_fast_assign(
__entry->dev = dir->i_sb->s_dev;
__entry->ino = dir->i_ino;
__entry->size = dir->i_size;
__entry->blocks = dir->i_blocks;
__entry->name = dentry->d_name.name;
),
>
> Fixes: 18ae8d12991b ("f2fs: show more DIO information in tracepoint")
> Signed-off-by: Mukesh Ojha <quic_mojha@quicinc.com>
> ---
> include/trace/events/f2fs.h | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/include/trace/events/f2fs.h b/include/trace/events/f2fs.h
> index c6b3724..7727ec9 100644
> --- a/include/trace/events/f2fs.h
> +++ b/include/trace/events/f2fs.h
> @@ -940,7 +940,7 @@ TRACE_EVENT(f2fs_direct_IO_enter,
> TP_STRUCT__entry(
> __field(dev_t, dev)
> __field(ino_t, ino)
> - __field(struct kiocb *, iocb)
> + __field_struct(struct kiocb, iocb)
> __field(unsigned long, len)
> __field(int, rw)
> ),
> @@ -948,17 +948,17 @@ TRACE_EVENT(f2fs_direct_IO_enter,
> TP_fast_assign(
> __entry->dev = inode->i_sb->s_dev;
> __entry->ino = inode->i_ino;
> - __entry->iocb = iocb;
> + memcpy(&__entry->iocb, iocb, sizeof(*iocb));
> __entry->len = len;
> __entry->rw = rw;
> ),
>
> TP_printk("dev = (%d,%d), ino = %lu pos = %lld len = %lu ki_flags = %x ki_ioprio = %x rw = %d",
> show_dev_ino(__entry),
> - __entry->iocb->ki_pos,
> + __entry->iocb.ki_pos,
> __entry->len,
> - __entry->iocb->ki_flags,
> - __entry->iocb->ki_ioprio,
> + __entry->iocb.ki_flags,
> + __entry->iocb.ki_ioprio,
> __entry->rw)
> );
>
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] f2fs: fix the assign logic of iocb
2022-10-20 9:01 ` [f2fs-dev] " Chao Yu
@ 2022-10-20 9:27 ` Mukesh Ojha
-1 siblings, 0 replies; 12+ messages in thread
From: Mukesh Ojha @ 2022-10-20 9:27 UTC (permalink / raw)
To: Chao Yu, jaegeuk, mhiramat; +Cc: linux-f2fs-devel, linux-kernel
Hi,
On 10/20/2022 2:31 PM, Chao Yu wrote:
> On 2022/10/20 0:17, Mukesh Ojha wrote:
>> commit 18ae8d12991b ("f2fs: show more DIO information in tracepoint")
>> introduces iocb field in 'f2fs_direct_IO_enter' trace event
>> And it only assigns the pointer and later it accesses its field
>> in trace print log.
>>
>> Fix it by correcting data type and memcpy the content of iocb.
>
> So the implementation below is wrong, right? since it just assign
> __entry->name
> with dentry->d_name.name rather than copyiny entirely, so that, during
> printing
I think, yes.
About the patch, we were getting error as below on doing
echo 51200 > /d/tracing/buffer_size_kb
echo 1 > /d/tracing/events/f2fs/f2fs_direct_IO_enter/enable
echo 1 > /d/tracing/tracing_on
cat /d/tracing/trace_pipe > ftrace.log
Run something which exercise this path.
Unable to handle kernel paging request at virtual address ffffffc04cef3d30
Mem abort info:
ESR = 0x96000007
EC = 0x25: DABT (current EL), IL = 32 bits
pc : trace_raw_output_f2fs_direct_IO_enter+0x54/0xa4
lr : trace_raw_output_f2fs_direct_IO_enter+0x2c/0xa4
sp : ffffffc0443cbbd0
x29: ffffffc0443cbbf0 x28: ffffff8935b120d0 x27: ffffff8935b12108
x26: ffffff8935b120f0 x25: ffffff8935b12100 x24: ffffff8935b110c0
x23: ffffff8935b10000 x22: ffffff88859a936c x21: ffffff88859a936c
x20: ffffff8935b110c0 x19: ffffff8935b10000 x18: ffffffc03b195060
x17: ffffff8935b11e76 x16: 00000000000000cc x15: ffffffef855c4f2c
x14: 0000000000000001 x13: 000000000000004e x12: ffff0000ffffff00
x11: ffffffef86c350d0 x10: 00000000000010c0 x9 : 000000000fe0002c
x8 : ffffffc04cef3d28 x7 : 7f7f7f7f7f7f7f7f x6 : 0000000002000000
x5 : ffffff8935b11e9a x4 : 0000000000006250 x3 : ffff0a00ffffff04
x2 : 0000000000000002 x1 : ffffffef86a0a31f x0 : ffffff8935b10000
Call trace:
trace_raw_output_f2fs_direct_IO_enter+0x54/0xa4
print_trace_fmt+0x9c/0x138
print_trace_line+0x154/0x254
tracing_read_pipe+0x21c/0x380
vfs_read+0x108/0x3ac
ksys_read+0x7c/0xec
__arm64_sys_read+0x20/0x30
invoke_syscall+0x60/0x150
el0_svc_common.llvm.1237943816091755067+0xb8/0xf8
do_el0_svc+0x28/0xa0
-Mukesh
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [f2fs-dev] [PATCH] f2fs: fix the assign logic of iocb
@ 2022-10-20 9:27 ` Mukesh Ojha
0 siblings, 0 replies; 12+ messages in thread
From: Mukesh Ojha @ 2022-10-20 9:27 UTC (permalink / raw)
To: Chao Yu, jaegeuk, mhiramat; +Cc: linux-kernel, linux-f2fs-devel
Hi,
On 10/20/2022 2:31 PM, Chao Yu wrote:
> On 2022/10/20 0:17, Mukesh Ojha wrote:
>> commit 18ae8d12991b ("f2fs: show more DIO information in tracepoint")
>> introduces iocb field in 'f2fs_direct_IO_enter' trace event
>> And it only assigns the pointer and later it accesses its field
>> in trace print log.
>>
>> Fix it by correcting data type and memcpy the content of iocb.
>
> So the implementation below is wrong, right? since it just assign
> __entry->name
> with dentry->d_name.name rather than copyiny entirely, so that, during
> printing
I think, yes.
About the patch, we were getting error as below on doing
echo 51200 > /d/tracing/buffer_size_kb
echo 1 > /d/tracing/events/f2fs/f2fs_direct_IO_enter/enable
echo 1 > /d/tracing/tracing_on
cat /d/tracing/trace_pipe > ftrace.log
Run something which exercise this path.
Unable to handle kernel paging request at virtual address ffffffc04cef3d30
Mem abort info:
ESR = 0x96000007
EC = 0x25: DABT (current EL), IL = 32 bits
pc : trace_raw_output_f2fs_direct_IO_enter+0x54/0xa4
lr : trace_raw_output_f2fs_direct_IO_enter+0x2c/0xa4
sp : ffffffc0443cbbd0
x29: ffffffc0443cbbf0 x28: ffffff8935b120d0 x27: ffffff8935b12108
x26: ffffff8935b120f0 x25: ffffff8935b12100 x24: ffffff8935b110c0
x23: ffffff8935b10000 x22: ffffff88859a936c x21: ffffff88859a936c
x20: ffffff8935b110c0 x19: ffffff8935b10000 x18: ffffffc03b195060
x17: ffffff8935b11e76 x16: 00000000000000cc x15: ffffffef855c4f2c
x14: 0000000000000001 x13: 000000000000004e x12: ffff0000ffffff00
x11: ffffffef86c350d0 x10: 00000000000010c0 x9 : 000000000fe0002c
x8 : ffffffc04cef3d28 x7 : 7f7f7f7f7f7f7f7f x6 : 0000000002000000
x5 : ffffff8935b11e9a x4 : 0000000000006250 x3 : ffff0a00ffffff04
x2 : 0000000000000002 x1 : ffffffef86a0a31f x0 : ffffff8935b10000
Call trace:
trace_raw_output_f2fs_direct_IO_enter+0x54/0xa4
print_trace_fmt+0x9c/0x138
print_trace_line+0x154/0x254
tracing_read_pipe+0x21c/0x380
vfs_read+0x108/0x3ac
ksys_read+0x7c/0xec
__arm64_sys_read+0x20/0x30
invoke_syscall+0x60/0x150
el0_svc_common.llvm.1237943816091755067+0xb8/0xf8
do_el0_svc+0x28/0xa0
-Mukesh
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] f2fs: fix the assign logic of iocb
2022-10-20 9:27 ` [f2fs-dev] " Mukesh Ojha
@ 2022-10-20 10:26 ` Chao Yu
-1 siblings, 0 replies; 12+ messages in thread
From: Chao Yu @ 2022-10-20 10:26 UTC (permalink / raw)
To: Mukesh Ojha, jaegeuk, mhiramat; +Cc: linux-f2fs-devel, linux-kernel
On 2022/10/20 17:27, Mukesh Ojha wrote:
> Hi,
>
> On 10/20/2022 2:31 PM, Chao Yu wrote:
>> On 2022/10/20 0:17, Mukesh Ojha wrote:
>>> commit 18ae8d12991b ("f2fs: show more DIO information in tracepoint")
>>> introduces iocb field in 'f2fs_direct_IO_enter' trace event
>>> And it only assigns the pointer and later it accesses its field
>>> in trace print log.
>>>
>>> Fix it by correcting data type and memcpy the content of iocb.
>>
>> So the implementation below is wrong, right? since it just assign __entry->name
>> with dentry->d_name.name rather than copyiny entirely, so that, during printing
>
> I think, yes.
>
> About the patch, we were getting error as below on doing
Thanks for the explanation. :)
What do you think of adding below info into commit message? and fixing all
similar issues of include/trace/events/f2fs.h in one patch?
Thanks,
>
> echo 51200 > /d/tracing/buffer_size_kb
> echo 1 > /d/tracing/events/f2fs/f2fs_direct_IO_enter/enable
> echo 1 > /d/tracing/tracing_on
> cat /d/tracing/trace_pipe > ftrace.log
>
> Run something which exercise this path.
>
> Unable to handle kernel paging request at virtual address ffffffc04cef3d30
> Mem abort info:
> ESR = 0x96000007
> EC = 0x25: DABT (current EL), IL = 32 bits
>
> pc : trace_raw_output_f2fs_direct_IO_enter+0x54/0xa4
> lr : trace_raw_output_f2fs_direct_IO_enter+0x2c/0xa4
> sp : ffffffc0443cbbd0
> x29: ffffffc0443cbbf0 x28: ffffff8935b120d0 x27: ffffff8935b12108
> x26: ffffff8935b120f0 x25: ffffff8935b12100 x24: ffffff8935b110c0
> x23: ffffff8935b10000 x22: ffffff88859a936c x21: ffffff88859a936c
> x20: ffffff8935b110c0 x19: ffffff8935b10000 x18: ffffffc03b195060
> x17: ffffff8935b11e76 x16: 00000000000000cc x15: ffffffef855c4f2c
> x14: 0000000000000001 x13: 000000000000004e x12: ffff0000ffffff00
> x11: ffffffef86c350d0 x10: 00000000000010c0 x9 : 000000000fe0002c
> x8 : ffffffc04cef3d28 x7 : 7f7f7f7f7f7f7f7f x6 : 0000000002000000
> x5 : ffffff8935b11e9a x4 : 0000000000006250 x3 : ffff0a00ffffff04
> x2 : 0000000000000002 x1 : ffffffef86a0a31f x0 : ffffff8935b10000
> Call trace:
> trace_raw_output_f2fs_direct_IO_enter+0x54/0xa4
> print_trace_fmt+0x9c/0x138
> print_trace_line+0x154/0x254
> tracing_read_pipe+0x21c/0x380
> vfs_read+0x108/0x3ac
> ksys_read+0x7c/0xec
> __arm64_sys_read+0x20/0x30
> invoke_syscall+0x60/0x150
> el0_svc_common.llvm.1237943816091755067+0xb8/0xf8
> do_el0_svc+0x28/0xa0
>
> -Mukesh
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [f2fs-dev] [PATCH] f2fs: fix the assign logic of iocb
@ 2022-10-20 10:26 ` Chao Yu
0 siblings, 0 replies; 12+ messages in thread
From: Chao Yu @ 2022-10-20 10:26 UTC (permalink / raw)
To: Mukesh Ojha, jaegeuk, mhiramat; +Cc: linux-kernel, linux-f2fs-devel
On 2022/10/20 17:27, Mukesh Ojha wrote:
> Hi,
>
> On 10/20/2022 2:31 PM, Chao Yu wrote:
>> On 2022/10/20 0:17, Mukesh Ojha wrote:
>>> commit 18ae8d12991b ("f2fs: show more DIO information in tracepoint")
>>> introduces iocb field in 'f2fs_direct_IO_enter' trace event
>>> And it only assigns the pointer and later it accesses its field
>>> in trace print log.
>>>
>>> Fix it by correcting data type and memcpy the content of iocb.
>>
>> So the implementation below is wrong, right? since it just assign __entry->name
>> with dentry->d_name.name rather than copyiny entirely, so that, during printing
>
> I think, yes.
>
> About the patch, we were getting error as below on doing
Thanks for the explanation. :)
What do you think of adding below info into commit message? and fixing all
similar issues of include/trace/events/f2fs.h in one patch?
Thanks,
>
> echo 51200 > /d/tracing/buffer_size_kb
> echo 1 > /d/tracing/events/f2fs/f2fs_direct_IO_enter/enable
> echo 1 > /d/tracing/tracing_on
> cat /d/tracing/trace_pipe > ftrace.log
>
> Run something which exercise this path.
>
> Unable to handle kernel paging request at virtual address ffffffc04cef3d30
> Mem abort info:
> ESR = 0x96000007
> EC = 0x25: DABT (current EL), IL = 32 bits
>
> pc : trace_raw_output_f2fs_direct_IO_enter+0x54/0xa4
> lr : trace_raw_output_f2fs_direct_IO_enter+0x2c/0xa4
> sp : ffffffc0443cbbd0
> x29: ffffffc0443cbbf0 x28: ffffff8935b120d0 x27: ffffff8935b12108
> x26: ffffff8935b120f0 x25: ffffff8935b12100 x24: ffffff8935b110c0
> x23: ffffff8935b10000 x22: ffffff88859a936c x21: ffffff88859a936c
> x20: ffffff8935b110c0 x19: ffffff8935b10000 x18: ffffffc03b195060
> x17: ffffff8935b11e76 x16: 00000000000000cc x15: ffffffef855c4f2c
> x14: 0000000000000001 x13: 000000000000004e x12: ffff0000ffffff00
> x11: ffffffef86c350d0 x10: 00000000000010c0 x9 : 000000000fe0002c
> x8 : ffffffc04cef3d28 x7 : 7f7f7f7f7f7f7f7f x6 : 0000000002000000
> x5 : ffffff8935b11e9a x4 : 0000000000006250 x3 : ffff0a00ffffff04
> x2 : 0000000000000002 x1 : ffffffef86a0a31f x0 : ffffff8935b10000
> Call trace:
> trace_raw_output_f2fs_direct_IO_enter+0x54/0xa4
> print_trace_fmt+0x9c/0x138
> print_trace_line+0x154/0x254
> tracing_read_pipe+0x21c/0x380
> vfs_read+0x108/0x3ac
> ksys_read+0x7c/0xec
> __arm64_sys_read+0x20/0x30
> invoke_syscall+0x60/0x150
> el0_svc_common.llvm.1237943816091755067+0xb8/0xf8
> do_el0_svc+0x28/0xa0
>
> -Mukesh
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] f2fs: fix the assign logic of iocb
2022-10-19 16:17 ` [f2fs-dev] " Mukesh Ojha
@ 2022-10-21 5:00 ` Pavan Kondeti
-1 siblings, 0 replies; 12+ messages in thread
From: Pavan Kondeti @ 2022-10-21 5:00 UTC (permalink / raw)
To: Mukesh Ojha; +Cc: jaegeuk, chao, mhiramat, linux-f2fs-devel, linux-kernel
Hi Mukesh,
On Wed, Oct 19, 2022 at 09:47:57PM +0530, Mukesh Ojha wrote:
> commit 18ae8d12991b ("f2fs: show more DIO information in tracepoint")
> introduces iocb field in 'f2fs_direct_IO_enter' trace event
> And it only assigns the pointer and later it accesses its field
> in trace print log.
>
> Fix it by correcting data type and memcpy the content of iocb.
>
> Fixes: 18ae8d12991b ("f2fs: show more DIO information in tracepoint")
> Signed-off-by: Mukesh Ojha <quic_mojha@quicinc.com>
> ---
> include/trace/events/f2fs.h | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/include/trace/events/f2fs.h b/include/trace/events/f2fs.h
> index c6b3724..7727ec9 100644
> --- a/include/trace/events/f2fs.h
> +++ b/include/trace/events/f2fs.h
> @@ -940,7 +940,7 @@ TRACE_EVENT(f2fs_direct_IO_enter,
> TP_STRUCT__entry(
> __field(dev_t, dev)
> __field(ino_t, ino)
> - __field(struct kiocb *, iocb)
> + __field_struct(struct kiocb, iocb)
> __field(unsigned long, len)
> __field(int, rw)
> ),
> @@ -948,17 +948,17 @@ TRACE_EVENT(f2fs_direct_IO_enter,
> TP_fast_assign(
> __entry->dev = inode->i_sb->s_dev;
> __entry->ino = inode->i_ino;
> - __entry->iocb = iocb;
> + memcpy(&__entry->iocb, iocb, sizeof(*iocb));
> __entry->len = len;
> __entry->rw = rw;
> ),
>
Why copy the whole structure (48 bytes)? cache the three members you
need.
Thanks,
Pavan
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [f2fs-dev] [PATCH] f2fs: fix the assign logic of iocb
@ 2022-10-21 5:00 ` Pavan Kondeti
0 siblings, 0 replies; 12+ messages in thread
From: Pavan Kondeti @ 2022-10-21 5:00 UTC (permalink / raw)
To: Mukesh Ojha; +Cc: jaegeuk, mhiramat, linux-kernel, linux-f2fs-devel
Hi Mukesh,
On Wed, Oct 19, 2022 at 09:47:57PM +0530, Mukesh Ojha wrote:
> commit 18ae8d12991b ("f2fs: show more DIO information in tracepoint")
> introduces iocb field in 'f2fs_direct_IO_enter' trace event
> And it only assigns the pointer and later it accesses its field
> in trace print log.
>
> Fix it by correcting data type and memcpy the content of iocb.
>
> Fixes: 18ae8d12991b ("f2fs: show more DIO information in tracepoint")
> Signed-off-by: Mukesh Ojha <quic_mojha@quicinc.com>
> ---
> include/trace/events/f2fs.h | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/include/trace/events/f2fs.h b/include/trace/events/f2fs.h
> index c6b3724..7727ec9 100644
> --- a/include/trace/events/f2fs.h
> +++ b/include/trace/events/f2fs.h
> @@ -940,7 +940,7 @@ TRACE_EVENT(f2fs_direct_IO_enter,
> TP_STRUCT__entry(
> __field(dev_t, dev)
> __field(ino_t, ino)
> - __field(struct kiocb *, iocb)
> + __field_struct(struct kiocb, iocb)
> __field(unsigned long, len)
> __field(int, rw)
> ),
> @@ -948,17 +948,17 @@ TRACE_EVENT(f2fs_direct_IO_enter,
> TP_fast_assign(
> __entry->dev = inode->i_sb->s_dev;
> __entry->ino = inode->i_ino;
> - __entry->iocb = iocb;
> + memcpy(&__entry->iocb, iocb, sizeof(*iocb));
> __entry->len = len;
> __entry->rw = rw;
> ),
>
Why copy the whole structure (48 bytes)? cache the three members you
need.
Thanks,
Pavan
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] f2fs: fix the assign logic of iocb
2022-10-21 5:00 ` [f2fs-dev] " Pavan Kondeti
@ 2022-10-24 5:07 ` Masami Hiramatsu
-1 siblings, 0 replies; 12+ messages in thread
From: Masami Hiramatsu @ 2022-10-24 5:07 UTC (permalink / raw)
To: Pavan Kondeti
Cc: Mukesh Ojha, jaegeuk, chao, mhiramat, linux-f2fs-devel, linux-kernel
On Fri, 21 Oct 2022 10:30:46 +0530
Pavan Kondeti <quic_pkondeti@quicinc.com> wrote:
> Hi Mukesh,
>
> On Wed, Oct 19, 2022 at 09:47:57PM +0530, Mukesh Ojha wrote:
> > commit 18ae8d12991b ("f2fs: show more DIO information in tracepoint")
> > introduces iocb field in 'f2fs_direct_IO_enter' trace event
> > And it only assigns the pointer and later it accesses its field
> > in trace print log.
> >
> > Fix it by correcting data type and memcpy the content of iocb.
> >
> > Fixes: 18ae8d12991b ("f2fs: show more DIO information in tracepoint")
> > Signed-off-by: Mukesh Ojha <quic_mojha@quicinc.com>
> > ---
> > include/trace/events/f2fs.h | 10 +++++-----
> > 1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/include/trace/events/f2fs.h b/include/trace/events/f2fs.h
> > index c6b3724..7727ec9 100644
> > --- a/include/trace/events/f2fs.h
> > +++ b/include/trace/events/f2fs.h
> > @@ -940,7 +940,7 @@ TRACE_EVENT(f2fs_direct_IO_enter,
> > TP_STRUCT__entry(
> > __field(dev_t, dev)
> > __field(ino_t, ino)
> > - __field(struct kiocb *, iocb)
> > + __field_struct(struct kiocb, iocb)
> > __field(unsigned long, len)
> > __field(int, rw)
> > ),
> > @@ -948,17 +948,17 @@ TRACE_EVENT(f2fs_direct_IO_enter,
> > TP_fast_assign(
> > __entry->dev = inode->i_sb->s_dev;
> > __entry->ino = inode->i_ino;
> > - __entry->iocb = iocb;
> > + memcpy(&__entry->iocb, iocb, sizeof(*iocb));
> > __entry->len = len;
> > __entry->rw = rw;
> > ),
> >
>
> Why copy the whole structure (48 bytes)? cache the three members you
> need.
+1. If this only prints ki_pos, ki_flags and ki_ioprio, I recommend you
to save those 3 fields to the entry. It should not expose in-kernel
data structure because it can be changed.
Thank you,
>
> Thanks,
> Pavan
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [f2fs-dev] [PATCH] f2fs: fix the assign logic of iocb
@ 2022-10-24 5:07 ` Masami Hiramatsu
0 siblings, 0 replies; 12+ messages in thread
From: Masami Hiramatsu @ 2022-10-24 5:07 UTC (permalink / raw)
To: Pavan Kondeti
Cc: linux-kernel, linux-f2fs-devel, Mukesh Ojha, jaegeuk, mhiramat
On Fri, 21 Oct 2022 10:30:46 +0530
Pavan Kondeti <quic_pkondeti@quicinc.com> wrote:
> Hi Mukesh,
>
> On Wed, Oct 19, 2022 at 09:47:57PM +0530, Mukesh Ojha wrote:
> > commit 18ae8d12991b ("f2fs: show more DIO information in tracepoint")
> > introduces iocb field in 'f2fs_direct_IO_enter' trace event
> > And it only assigns the pointer and later it accesses its field
> > in trace print log.
> >
> > Fix it by correcting data type and memcpy the content of iocb.
> >
> > Fixes: 18ae8d12991b ("f2fs: show more DIO information in tracepoint")
> > Signed-off-by: Mukesh Ojha <quic_mojha@quicinc.com>
> > ---
> > include/trace/events/f2fs.h | 10 +++++-----
> > 1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/include/trace/events/f2fs.h b/include/trace/events/f2fs.h
> > index c6b3724..7727ec9 100644
> > --- a/include/trace/events/f2fs.h
> > +++ b/include/trace/events/f2fs.h
> > @@ -940,7 +940,7 @@ TRACE_EVENT(f2fs_direct_IO_enter,
> > TP_STRUCT__entry(
> > __field(dev_t, dev)
> > __field(ino_t, ino)
> > - __field(struct kiocb *, iocb)
> > + __field_struct(struct kiocb, iocb)
> > __field(unsigned long, len)
> > __field(int, rw)
> > ),
> > @@ -948,17 +948,17 @@ TRACE_EVENT(f2fs_direct_IO_enter,
> > TP_fast_assign(
> > __entry->dev = inode->i_sb->s_dev;
> > __entry->ino = inode->i_ino;
> > - __entry->iocb = iocb;
> > + memcpy(&__entry->iocb, iocb, sizeof(*iocb));
> > __entry->len = len;
> > __entry->rw = rw;
> > ),
> >
>
> Why copy the whole structure (48 bytes)? cache the three members you
> need.
+1. If this only prints ki_pos, ki_flags and ki_ioprio, I recommend you
to save those 3 fields to the entry. It should not expose in-kernel
data structure because it can be changed.
Thank you,
>
> Thanks,
> Pavan
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 12+ messages in thread