From: Jan Kara <jack@suse.cz>
To: Tejun Heo <tj@kernel.org>
Cc: gregkh@linuxfoundation.org, kernel-team@fb.com,
linux-kernel@vger.kernel.org, cgroups@vger.kernel.org,
lizefan@huawei.com, hannes@cmpxchg.org, namhyung@kernel.org,
ast@kernel.org, daniel@iogearbox.net, Jan Kara <jack@suse.cz>,
Jens Axboe <axboe@kernel.dk>
Subject: Re: [PATCH 02/10] writeback: use ino_t for inodes in tracepoints
Date: Tue, 5 Nov 2019 10:11:11 +0100 [thread overview]
Message-ID: <20191105091111.GJ22379@quack2.suse.cz> (raw)
In-Reply-To: <20191104235944.3470866-3-tj@kernel.org>
On Mon 04-11-19 15:59:36, Tejun Heo wrote:
> Writeback TPs currently use mix of 32 and 64bits for inos. This isn't
> currently broken because only cgroup inos are using 32bits and they're
> limited to 32bits. cgroup inos will make use of 64bits. Let's
> uniformly use ino_t.
>
> While at it, switch the default cgroup ino value used when cgroup is
> disabled to 1 instead of -1U as root cgroup always uses ino 1.
>
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Cc: Jan Kara <jack@suse.cz>
> Cc: Jens Axboe <axboe@kernel.dk>
> Cc: Namhyung Kim <namhyung@kernel.org>
Thanks! The patch looks good to me. You can add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> Hello,
>
> This is to prepare for kernfs 64bit ino support. It'd be best to
> route this with the rest of kernfs patchset.
>
> Thanks.
>
> include/trace/events/writeback.h | 88 ++++++++++++++++----------------
> 1 file changed, 44 insertions(+), 44 deletions(-)
>
> diff --git a/include/trace/events/writeback.h b/include/trace/events/writeback.h
> index c2ce6480b4b1..95e50677476b 100644
> --- a/include/trace/events/writeback.h
> +++ b/include/trace/events/writeback.h
> @@ -61,7 +61,7 @@ DECLARE_EVENT_CLASS(writeback_page_template,
>
> TP_STRUCT__entry (
> __array(char, name, 32)
> - __field(unsigned long, ino)
> + __field(ino_t, ino)
> __field(pgoff_t, index)
> ),
>
> @@ -102,7 +102,7 @@ DECLARE_EVENT_CLASS(writeback_dirty_inode_template,
>
> TP_STRUCT__entry (
> __array(char, name, 32)
> - __field(unsigned long, ino)
> + __field(ino_t, ino)
> __field(unsigned long, state)
> __field(unsigned long, flags)
> ),
> @@ -150,28 +150,28 @@ DEFINE_EVENT(writeback_dirty_inode_template, writeback_dirty_inode,
> #ifdef CREATE_TRACE_POINTS
> #ifdef CONFIG_CGROUP_WRITEBACK
>
> -static inline unsigned int __trace_wb_assign_cgroup(struct bdi_writeback *wb)
> +static inline ino_t __trace_wb_assign_cgroup(struct bdi_writeback *wb)
> {
> return wb->memcg_css->cgroup->kn->id.ino;
> }
>
> -static inline unsigned int __trace_wbc_assign_cgroup(struct writeback_control *wbc)
> +static inline ino_t __trace_wbc_assign_cgroup(struct writeback_control *wbc)
> {
> if (wbc->wb)
> return __trace_wb_assign_cgroup(wbc->wb);
> else
> - return -1U;
> + return 1;
> }
> #else /* CONFIG_CGROUP_WRITEBACK */
>
> -static inline unsigned int __trace_wb_assign_cgroup(struct bdi_writeback *wb)
> +static inline ino_t __trace_wb_assign_cgroup(struct bdi_writeback *wb)
> {
> - return -1U;
> + return 1;
> }
>
> -static inline unsigned int __trace_wbc_assign_cgroup(struct writeback_control *wbc)
> +static inline ino_t __trace_wbc_assign_cgroup(struct writeback_control *wbc)
> {
> - return -1U;
> + return 1;
> }
>
> #endif /* CONFIG_CGROUP_WRITEBACK */
> @@ -187,8 +187,8 @@ TRACE_EVENT(inode_foreign_history,
>
> TP_STRUCT__entry(
> __array(char, name, 32)
> - __field(unsigned long, ino)
> - __field(unsigned int, cgroup_ino)
> + __field(ino_t, ino)
> + __field(ino_t, cgroup_ino)
> __field(unsigned int, history)
> ),
>
> @@ -199,7 +199,7 @@ TRACE_EVENT(inode_foreign_history,
> __entry->history = history;
> ),
>
> - TP_printk("bdi %s: ino=%lu cgroup_ino=%u history=0x%x",
> + TP_printk("bdi %s: ino=%lu cgroup_ino=%lu history=0x%x",
> __entry->name,
> __entry->ino,
> __entry->cgroup_ino,
> @@ -216,9 +216,9 @@ TRACE_EVENT(inode_switch_wbs,
>
> TP_STRUCT__entry(
> __array(char, name, 32)
> - __field(unsigned long, ino)
> - __field(unsigned int, old_cgroup_ino)
> - __field(unsigned int, new_cgroup_ino)
> + __field(ino_t, ino)
> + __field(ino_t, old_cgroup_ino)
> + __field(ino_t, new_cgroup_ino)
> ),
>
> TP_fast_assign(
> @@ -228,7 +228,7 @@ TRACE_EVENT(inode_switch_wbs,
> __entry->new_cgroup_ino = __trace_wb_assign_cgroup(new_wb);
> ),
>
> - TP_printk("bdi %s: ino=%lu old_cgroup_ino=%u new_cgroup_ino=%u",
> + TP_printk("bdi %s: ino=%lu old_cgroup_ino=%lu new_cgroup_ino=%lu",
> __entry->name,
> __entry->ino,
> __entry->old_cgroup_ino,
> @@ -245,10 +245,10 @@ TRACE_EVENT(track_foreign_dirty,
> TP_STRUCT__entry(
> __array(char, name, 32)
> __field(u64, bdi_id)
> - __field(unsigned long, ino)
> + __field(ino_t, ino)
> __field(unsigned int, memcg_id)
> - __field(unsigned int, cgroup_ino)
> - __field(unsigned int, page_cgroup_ino)
> + __field(ino_t, cgroup_ino)
> + __field(ino_t, page_cgroup_ino)
> ),
>
> TP_fast_assign(
> @@ -263,7 +263,7 @@ TRACE_EVENT(track_foreign_dirty,
> __entry->page_cgroup_ino = page->mem_cgroup->css.cgroup->kn->id.ino;
> ),
>
> - TP_printk("bdi %s[%llu]: ino=%lu memcg_id=%u cgroup_ino=%u page_cgroup_ino=%u",
> + TP_printk("bdi %s[%llu]: ino=%lu memcg_id=%u cgroup_ino=%lu page_cgroup_ino=%lu",
> __entry->name,
> __entry->bdi_id,
> __entry->ino,
> @@ -282,7 +282,7 @@ TRACE_EVENT(flush_foreign,
>
> TP_STRUCT__entry(
> __array(char, name, 32)
> - __field(unsigned int, cgroup_ino)
> + __field(ino_t, cgroup_ino)
> __field(unsigned int, frn_bdi_id)
> __field(unsigned int, frn_memcg_id)
> ),
> @@ -294,7 +294,7 @@ TRACE_EVENT(flush_foreign,
> __entry->frn_memcg_id = frn_memcg_id;
> ),
>
> - TP_printk("bdi %s: cgroup_ino=%u frn_bdi_id=%u frn_memcg_id=%u",
> + TP_printk("bdi %s: cgroup_ino=%lu frn_bdi_id=%u frn_memcg_id=%u",
> __entry->name,
> __entry->cgroup_ino,
> __entry->frn_bdi_id,
> @@ -311,9 +311,9 @@ DECLARE_EVENT_CLASS(writeback_write_inode_template,
>
> TP_STRUCT__entry (
> __array(char, name, 32)
> - __field(unsigned long, ino)
> + __field(ino_t, ino)
> __field(int, sync_mode)
> - __field(unsigned int, cgroup_ino)
> + __field(ino_t, cgroup_ino)
> ),
>
> TP_fast_assign(
> @@ -324,7 +324,7 @@ DECLARE_EVENT_CLASS(writeback_write_inode_template,
> __entry->cgroup_ino = __trace_wbc_assign_cgroup(wbc);
> ),
>
> - TP_printk("bdi %s: ino=%lu sync_mode=%d cgroup_ino=%u",
> + TP_printk("bdi %s: ino=%lu sync_mode=%d cgroup_ino=%lu",
> __entry->name,
> __entry->ino,
> __entry->sync_mode,
> @@ -358,7 +358,7 @@ DECLARE_EVENT_CLASS(writeback_work_class,
> __field(int, range_cyclic)
> __field(int, for_background)
> __field(int, reason)
> - __field(unsigned int, cgroup_ino)
> + __field(ino_t, cgroup_ino)
> ),
> TP_fast_assign(
> strscpy_pad(__entry->name,
> @@ -374,7 +374,7 @@ DECLARE_EVENT_CLASS(writeback_work_class,
> __entry->cgroup_ino = __trace_wb_assign_cgroup(wb);
> ),
> TP_printk("bdi %s: sb_dev %d:%d nr_pages=%ld sync_mode=%d "
> - "kupdate=%d range_cyclic=%d background=%d reason=%s cgroup_ino=%u",
> + "kupdate=%d range_cyclic=%d background=%d reason=%s cgroup_ino=%lu",
> __entry->name,
> MAJOR(__entry->sb_dev), MINOR(__entry->sb_dev),
> __entry->nr_pages,
> @@ -413,13 +413,13 @@ DECLARE_EVENT_CLASS(writeback_class,
> TP_ARGS(wb),
> TP_STRUCT__entry(
> __array(char, name, 32)
> - __field(unsigned int, cgroup_ino)
> + __field(ino_t, cgroup_ino)
> ),
> TP_fast_assign(
> strscpy_pad(__entry->name, dev_name(wb->bdi->dev), 32);
> __entry->cgroup_ino = __trace_wb_assign_cgroup(wb);
> ),
> - TP_printk("bdi %s: cgroup_ino=%u",
> + TP_printk("bdi %s: cgroup_ino=%lu",
> __entry->name,
> __entry->cgroup_ino
> )
> @@ -459,7 +459,7 @@ DECLARE_EVENT_CLASS(wbc_class,
> __field(int, range_cyclic)
> __field(long, range_start)
> __field(long, range_end)
> - __field(unsigned int, cgroup_ino)
> + __field(ino_t, cgroup_ino)
> ),
>
> TP_fast_assign(
> @@ -478,7 +478,7 @@ DECLARE_EVENT_CLASS(wbc_class,
>
> TP_printk("bdi %s: towrt=%ld skip=%ld mode=%d kupd=%d "
> "bgrd=%d reclm=%d cyclic=%d "
> - "start=0x%lx end=0x%lx cgroup_ino=%u",
> + "start=0x%lx end=0x%lx cgroup_ino=%lu",
> __entry->name,
> __entry->nr_to_write,
> __entry->pages_skipped,
> @@ -510,7 +510,7 @@ TRACE_EVENT(writeback_queue_io,
> __field(long, age)
> __field(int, moved)
> __field(int, reason)
> - __field(unsigned int, cgroup_ino)
> + __field(ino_t, cgroup_ino)
> ),
> TP_fast_assign(
> unsigned long *older_than_this = work->older_than_this;
> @@ -522,7 +522,7 @@ TRACE_EVENT(writeback_queue_io,
> __entry->reason = work->reason;
> __entry->cgroup_ino = __trace_wb_assign_cgroup(wb);
> ),
> - TP_printk("bdi %s: older=%lu age=%ld enqueue=%d reason=%s cgroup_ino=%u",
> + TP_printk("bdi %s: older=%lu age=%ld enqueue=%d reason=%s cgroup_ino=%lu",
> __entry->name,
> __entry->older, /* older_than_this in jiffies */
> __entry->age, /* older_than_this in relative milliseconds */
> @@ -596,7 +596,7 @@ TRACE_EVENT(bdi_dirty_ratelimit,
> __field(unsigned long, dirty_ratelimit)
> __field(unsigned long, task_ratelimit)
> __field(unsigned long, balanced_dirty_ratelimit)
> - __field(unsigned int, cgroup_ino)
> + __field(ino_t, cgroup_ino)
> ),
>
> TP_fast_assign(
> @@ -614,7 +614,7 @@ TRACE_EVENT(bdi_dirty_ratelimit,
> TP_printk("bdi %s: "
> "write_bw=%lu awrite_bw=%lu dirty_rate=%lu "
> "dirty_ratelimit=%lu task_ratelimit=%lu "
> - "balanced_dirty_ratelimit=%lu cgroup_ino=%u",
> + "balanced_dirty_ratelimit=%lu cgroup_ino=%lu",
> __entry->bdi,
> __entry->write_bw, /* write bandwidth */
> __entry->avg_write_bw, /* avg write bandwidth */
> @@ -660,7 +660,7 @@ TRACE_EVENT(balance_dirty_pages,
> __field( long, pause)
> __field(unsigned long, period)
> __field( long, think)
> - __field(unsigned int, cgroup_ino)
> + __field(ino_t, cgroup_ino)
> ),
>
> TP_fast_assign(
> @@ -692,7 +692,7 @@ TRACE_EVENT(balance_dirty_pages,
> "bdi_setpoint=%lu bdi_dirty=%lu "
> "dirty_ratelimit=%lu task_ratelimit=%lu "
> "dirtied=%u dirtied_pause=%u "
> - "paused=%lu pause=%ld period=%lu think=%ld cgroup_ino=%u",
> + "paused=%lu pause=%ld period=%lu think=%ld cgroup_ino=%lu",
> __entry->bdi,
> __entry->limit,
> __entry->setpoint,
> @@ -718,10 +718,10 @@ TRACE_EVENT(writeback_sb_inodes_requeue,
>
> TP_STRUCT__entry(
> __array(char, name, 32)
> - __field(unsigned long, ino)
> + __field(ino_t, ino)
> __field(unsigned long, state)
> __field(unsigned long, dirtied_when)
> - __field(unsigned int, cgroup_ino)
> + __field(ino_t, cgroup_ino)
> ),
>
> TP_fast_assign(
> @@ -733,7 +733,7 @@ TRACE_EVENT(writeback_sb_inodes_requeue,
> __entry->cgroup_ino = __trace_wb_assign_cgroup(inode_to_wb(inode));
> ),
>
> - TP_printk("bdi %s: ino=%lu state=%s dirtied_when=%lu age=%lu cgroup_ino=%u",
> + TP_printk("bdi %s: ino=%lu state=%s dirtied_when=%lu age=%lu cgroup_ino=%lu",
> __entry->name,
> __entry->ino,
> show_inode_state(__entry->state),
> @@ -789,13 +789,13 @@ DECLARE_EVENT_CLASS(writeback_single_inode_template,
>
> TP_STRUCT__entry(
> __array(char, name, 32)
> - __field(unsigned long, ino)
> + __field(ino_t, ino)
> __field(unsigned long, state)
> __field(unsigned long, dirtied_when)
> __field(unsigned long, writeback_index)
> __field(long, nr_to_write)
> __field(unsigned long, wrote)
> - __field(unsigned int, cgroup_ino)
> + __field(ino_t, cgroup_ino)
> ),
>
> TP_fast_assign(
> @@ -811,7 +811,7 @@ DECLARE_EVENT_CLASS(writeback_single_inode_template,
> ),
>
> TP_printk("bdi %s: ino=%lu state=%s dirtied_when=%lu age=%lu "
> - "index=%lu to_write=%ld wrote=%lu cgroup_ino=%u",
> + "index=%lu to_write=%ld wrote=%lu cgroup_ino=%lu",
> __entry->name,
> __entry->ino,
> show_inode_state(__entry->state),
> @@ -845,7 +845,7 @@ DECLARE_EVENT_CLASS(writeback_inode_template,
>
> TP_STRUCT__entry(
> __field( dev_t, dev )
> - __field(unsigned long, ino )
> + __field( ino_t, ino )
> __field(unsigned long, state )
> __field( __u16, mode )
> __field(unsigned long, dirtied_when )
> --
> 2.17.1
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
next prev parent reply other threads:[~2019-11-05 9:11 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-11-04 23:59 [PATCHSET cgroup/for-5.5] kernfs,cgroup: support 64bit inos and unify cgroup IDs Tejun Heo
2019-11-04 23:59 ` [PATCH 01/10] kernfs: fix ino wrap-around detection Tejun Heo
2019-11-04 23:59 ` [PATCH 02/10] writeback: use ino_t for inodes in tracepoints Tejun Heo
2019-11-05 9:11 ` Jan Kara [this message]
2019-11-04 23:59 ` [PATCH 03/10] netprio: use css ID instead of cgroup ID Tejun Heo
2019-11-05 13:27 ` Neil Horman
2019-11-04 23:59 ` [PATCH 04/10] kernfs: use dumber locking for kernfs_find_and_get_node_by_ino() Tejun Heo
2019-11-04 23:59 ` [PATCH 05/10] kernfs: kernfs_find_and_get_node_by_ino() should only look up activated nodes Tejun Heo
2019-11-04 23:59 ` [PATCH 06/10] kernfs: convert kernfs_node->id from union kernfs_node_id to u64 Tejun Heo
2019-11-04 23:59 ` [PATCH 07/10] kernfs: combine ino/id lookup functions into kernfs_find_and_get_node_by_id() Tejun Heo
2019-11-04 23:59 ` [PATCH 08/10] kernfs: implement custom exportfs ops and fid type Tejun Heo
2019-11-04 23:59 ` [PATCH 09/10] kernfs: use 64bit inos if ino_t is 64bit Tejun Heo
2019-11-04 23:59 ` [PATCH 10/10] cgroup: use cgrp->kn->id as the cgroup ID Tejun Heo
2019-11-06 13:50 ` [PATCHSET cgroup/for-5.5] kernfs,cgroup: support 64bit inos and unify cgroup IDs Namhyung Kim
2019-11-07 15:31 ` Tejun Heo
2019-11-12 15:51 ` Tejun Heo
2019-11-12 16:08 ` Greg KH
2019-11-12 16:09 ` Greg KH
2019-11-12 16:19 ` Tejun Heo
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=20191105091111.GJ22379@quack2.suse.cz \
--to=jack@suse.cz \
--cc=ast@kernel.org \
--cc=axboe@kernel.dk \
--cc=cgroups@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=gregkh@linuxfoundation.org \
--cc=hannes@cmpxchg.org \
--cc=kernel-team@fb.com \
--cc=linux-kernel@vger.kernel.org \
--cc=lizefan@huawei.com \
--cc=namhyung@kernel.org \
--cc=tj@kernel.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.