2024-02-20 12:03:59

by Zhang Yi

[permalink] [raw]
Subject: [PATCH -next] iomap: add pos and dirty_len into trace_iomap_writepage_map

From: Zhang Yi <[email protected]>

Since commit fd07e0aa23c4 ("iomap: map multiple blocks at a time"), we
could map multi-blocks once a time, and the dirty_len indicates the
expected map length, map_len won't large than it. The pos and dirty_len
means the dirty range that should be mapped to write, add them into
trace_iomap_writepage_map() could be more useful for debug.

Signed-off-by: Zhang Yi <[email protected]>
Reviewed-by: Christoph Hellwig <[email protected]>
---
fs/iomap/buffered-io.c | 2 +-
fs/iomap/trace.h | 43 +++++++++++++++++++++++++++++++++++++++++-
2 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
index 2ad0e287c704..ae4e2026e59e 100644
--- a/fs/iomap/buffered-io.c
+++ b/fs/iomap/buffered-io.c
@@ -1776,7 +1776,7 @@ static int iomap_writepage_map_blocks(struct iomap_writepage_ctx *wpc,
error = wpc->ops->map_blocks(wpc, inode, pos, dirty_len);
if (error)
break;
- trace_iomap_writepage_map(inode, &wpc->iomap);
+ trace_iomap_writepage_map(inode, pos, dirty_len, &wpc->iomap);

map_len = min_t(u64, dirty_len,
wpc->iomap.offset + wpc->iomap.length - pos);
diff --git a/fs/iomap/trace.h b/fs/iomap/trace.h
index c16fd55f5595..3ef694f9489f 100644
--- a/fs/iomap/trace.h
+++ b/fs/iomap/trace.h
@@ -154,7 +154,48 @@ DEFINE_EVENT(iomap_class, name, \
TP_ARGS(inode, iomap))
DEFINE_IOMAP_EVENT(iomap_iter_dstmap);
DEFINE_IOMAP_EVENT(iomap_iter_srcmap);
-DEFINE_IOMAP_EVENT(iomap_writepage_map);
+
+TRACE_EVENT(iomap_writepage_map,
+ TP_PROTO(struct inode *inode, u64 pos, unsigned int dirty_len,
+ struct iomap *iomap),
+ TP_ARGS(inode, pos, dirty_len, iomap),
+ TP_STRUCT__entry(
+ __field(dev_t, dev)
+ __field(u64, ino)
+ __field(u64, pos)
+ __field(u64, dirty_len)
+ __field(u64, addr)
+ __field(loff_t, offset)
+ __field(u64, length)
+ __field(u16, type)
+ __field(u16, flags)
+ __field(dev_t, bdev)
+ ),
+ TP_fast_assign(
+ __entry->dev = inode->i_sb->s_dev;
+ __entry->ino = inode->i_ino;
+ __entry->pos = pos;
+ __entry->dirty_len = dirty_len;
+ __entry->addr = iomap->addr;
+ __entry->offset = iomap->offset;
+ __entry->length = iomap->length;
+ __entry->type = iomap->type;
+ __entry->flags = iomap->flags;
+ __entry->bdev = iomap->bdev ? iomap->bdev->bd_dev : 0;
+ ),
+ TP_printk("dev %d:%d ino 0x%llx bdev %d:%d pos 0x%llx dirty len 0x%llx "
+ "addr 0x%llx offset 0x%llx length 0x%llx type %s flags %s",
+ MAJOR(__entry->dev), MINOR(__entry->dev),
+ __entry->ino,
+ MAJOR(__entry->bdev), MINOR(__entry->bdev),
+ __entry->pos,
+ __entry->dirty_len,
+ __entry->addr,
+ __entry->offset,
+ __entry->length,
+ __print_symbolic(__entry->type, IOMAP_TYPE_STRINGS),
+ __print_flags(__entry->flags, "|", IOMAP_F_FLAGS_STRINGS))
+);

TRACE_EVENT(iomap_iter,
TP_PROTO(struct iomap_iter *iter, const void *ops,
--
2.39.2



2024-02-21 07:36:22

by Christian Brauner

[permalink] [raw]
Subject: Re: [PATCH -next] iomap: add pos and dirty_len into trace_iomap_writepage_map

On Tue, 20 Feb 2024 19:57:59 +0800, Zhang Yi wrote:
> Since commit fd07e0aa23c4 ("iomap: map multiple blocks at a time"), we
> could map multi-blocks once a time, and the dirty_len indicates the
> expected map length, map_len won't large than it. The pos and dirty_len
> means the dirty range that should be mapped to write, add them into
> trace_iomap_writepage_map() could be more useful for debug.
>
>
> [...]

Applied to the vfs.iomap branch of the vfs/vfs.git tree.
Patches in the vfs.iomap branch should appear in linux-next soon.

Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.

It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.

Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.

tree: https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs.iomap

[1/1] iomap: add pos and dirty_len into trace_iomap_writepage_map
https://git.kernel.org/vfs/vfs/c/1284865eff6a

2024-02-21 16:40:42

by Darrick J. Wong

[permalink] [raw]
Subject: Re: [PATCH -next] iomap: add pos and dirty_len into trace_iomap_writepage_map

On Tue, Feb 20, 2024 at 07:57:59PM +0800, Zhang Yi wrote:
> From: Zhang Yi <[email protected]>
>
> Since commit fd07e0aa23c4 ("iomap: map multiple blocks at a time"), we
> could map multi-blocks once a time, and the dirty_len indicates the
> expected map length, map_len won't large than it. The pos and dirty_len
> means the dirty range that should be mapped to write, add them into
> trace_iomap_writepage_map() could be more useful for debug.
>
> Signed-off-by: Zhang Yi <[email protected]>
> Reviewed-by: Christoph Hellwig <[email protected]>

LGTM too
Reviewed-by: Darrick J. Wong <[email protected]>

--D

> ---
> fs/iomap/buffered-io.c | 2 +-
> fs/iomap/trace.h | 43 +++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 43 insertions(+), 2 deletions(-)
>
> diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
> index 2ad0e287c704..ae4e2026e59e 100644
> --- a/fs/iomap/buffered-io.c
> +++ b/fs/iomap/buffered-io.c
> @@ -1776,7 +1776,7 @@ static int iomap_writepage_map_blocks(struct iomap_writepage_ctx *wpc,
> error = wpc->ops->map_blocks(wpc, inode, pos, dirty_len);
> if (error)
> break;
> - trace_iomap_writepage_map(inode, &wpc->iomap);
> + trace_iomap_writepage_map(inode, pos, dirty_len, &wpc->iomap);
>
> map_len = min_t(u64, dirty_len,
> wpc->iomap.offset + wpc->iomap.length - pos);
> diff --git a/fs/iomap/trace.h b/fs/iomap/trace.h
> index c16fd55f5595..3ef694f9489f 100644
> --- a/fs/iomap/trace.h
> +++ b/fs/iomap/trace.h
> @@ -154,7 +154,48 @@ DEFINE_EVENT(iomap_class, name, \
> TP_ARGS(inode, iomap))
> DEFINE_IOMAP_EVENT(iomap_iter_dstmap);
> DEFINE_IOMAP_EVENT(iomap_iter_srcmap);
> -DEFINE_IOMAP_EVENT(iomap_writepage_map);
> +
> +TRACE_EVENT(iomap_writepage_map,
> + TP_PROTO(struct inode *inode, u64 pos, unsigned int dirty_len,
> + struct iomap *iomap),
> + TP_ARGS(inode, pos, dirty_len, iomap),
> + TP_STRUCT__entry(
> + __field(dev_t, dev)
> + __field(u64, ino)
> + __field(u64, pos)
> + __field(u64, dirty_len)
> + __field(u64, addr)
> + __field(loff_t, offset)
> + __field(u64, length)
> + __field(u16, type)
> + __field(u16, flags)
> + __field(dev_t, bdev)
> + ),
> + TP_fast_assign(
> + __entry->dev = inode->i_sb->s_dev;
> + __entry->ino = inode->i_ino;
> + __entry->pos = pos;
> + __entry->dirty_len = dirty_len;
> + __entry->addr = iomap->addr;
> + __entry->offset = iomap->offset;
> + __entry->length = iomap->length;
> + __entry->type = iomap->type;
> + __entry->flags = iomap->flags;
> + __entry->bdev = iomap->bdev ? iomap->bdev->bd_dev : 0;
> + ),
> + TP_printk("dev %d:%d ino 0x%llx bdev %d:%d pos 0x%llx dirty len 0x%llx "
> + "addr 0x%llx offset 0x%llx length 0x%llx type %s flags %s",
> + MAJOR(__entry->dev), MINOR(__entry->dev),
> + __entry->ino,
> + MAJOR(__entry->bdev), MINOR(__entry->bdev),
> + __entry->pos,
> + __entry->dirty_len,
> + __entry->addr,
> + __entry->offset,
> + __entry->length,
> + __print_symbolic(__entry->type, IOMAP_TYPE_STRINGS),
> + __print_flags(__entry->flags, "|", IOMAP_F_FLAGS_STRINGS))
> +);
>
> TRACE_EVENT(iomap_iter,
> TP_PROTO(struct iomap_iter *iter, const void *ops,
> --
> 2.39.2
>