| Message ID | 1551425934-28068-1-git-send-email-laoar.shao@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mm: vmscan: show zone type in kswapd tracepoints | expand |
On Fri 01-03-19 15:38:54, Yafang Shao wrote: > If we want to know the zone type, we have to check whether > CONFIG_ZONE_DMA, CONFIG_ZONE_DMA32 and CONFIG_HIGHMEM are set or not, > that's not so convenient. > > We'd better show the zone type directly. I do agree that zone number is quite PITA to process in general but do we really need this information in the first place? Why do we even care? Zones are an MM internal implementation details and the more we export to the userspace the more we are going to argue about breaking userspace when touching them. So I would rather not export that information unless it is terribly useful. > Signed-off-by: Yafang Shao <laoar.shao@gmail.com> > --- > include/trace/events/vmscan.h | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/include/trace/events/vmscan.h b/include/trace/events/vmscan.h > index a1cb913..4c8880b 100644 > --- a/include/trace/events/vmscan.h > +++ b/include/trace/events/vmscan.h > @@ -73,7 +73,10 @@ > __entry->order = order; > ), > > - TP_printk("nid=%d zid=%d order=%d", __entry->nid, __entry->zid, __entry->order) > + TP_printk("nid=%d zid=%-8s order=%d", > + __entry->nid, > + __print_symbolic(__entry->zid, ZONE_TYPE), > + __entry->order) > ); > > TRACE_EVENT(mm_vmscan_wakeup_kswapd, > @@ -96,9 +99,9 @@ > __entry->gfp_flags = gfp_flags; > ), > > - TP_printk("nid=%d zid=%d order=%d gfp_flags=%s", > + TP_printk("nid=%d zid=%-8s order=%d gfp_flags=%s", > __entry->nid, > - __entry->zid, > + __print_symbolic(__entry->zid, ZONE_TYPE), > __entry->order, > show_gfp_flags(__entry->gfp_flags)) > ); > -- > 1.8.3.1 >
On Mon, Mar 11, 2019 at 4:47 PM Michal Hocko <mhocko@kernel.org> wrote: > > On Fri 01-03-19 15:38:54, Yafang Shao wrote: > > If we want to know the zone type, we have to check whether > > CONFIG_ZONE_DMA, CONFIG_ZONE_DMA32 and CONFIG_HIGHMEM are set or not, > > that's not so convenient. > > > > We'd better show the zone type directly. > > I do agree that zone number is quite PITA to process in general but do > we really need this information in the first place? Why do we even care? > Sometimes we want to know this event occurs in which zone, then we can get the information of this zone, for example via /proc/zoneinfo. It could give us more information for debugging. > Zones are an MM internal implementation details and the more we export > to the userspace the more we are going to argue about breaking userspace > when touching them. So I would rather not export that information unless > it is terribly useful. > I 'm not sure whether zone type is terribly useful or not, but the 'zid' is useless at all. I don't agree that Zones are MM internal. We can get the zone type in many ways, for example /proc/zoneinfo. If we show this event occurs in which zone, we'd better show the zone type, or we should drop this 'zid'. > > Signed-off-by: Yafang Shao <laoar.shao@gmail.com> > > --- > > include/trace/events/vmscan.h | 9 ++++++--- > > 1 file changed, 6 insertions(+), 3 deletions(-) > > > > diff --git a/include/trace/events/vmscan.h b/include/trace/events/vmscan.h > > index a1cb913..4c8880b 100644 > > --- a/include/trace/events/vmscan.h > > +++ b/include/trace/events/vmscan.h > > @@ -73,7 +73,10 @@ > > __entry->order = order; > > ), > > > > - TP_printk("nid=%d zid=%d order=%d", __entry->nid, __entry->zid, __entry->order) > > + TP_printk("nid=%d zid=%-8s order=%d", > > + __entry->nid, > > + __print_symbolic(__entry->zid, ZONE_TYPE), > > + __entry->order) > > ); > > > > TRACE_EVENT(mm_vmscan_wakeup_kswapd, > > @@ -96,9 +99,9 @@ > > __entry->gfp_flags = gfp_flags; > > ), > > > > - TP_printk("nid=%d zid=%d order=%d gfp_flags=%s", > > + TP_printk("nid=%d zid=%-8s order=%d gfp_flags=%s", > > __entry->nid, > > - __entry->zid, > > + __print_symbolic(__entry->zid, ZONE_TYPE), > > __entry->order, > > show_gfp_flags(__entry->gfp_flags)) > > ); > > -- > > 1.8.3.1 > > > > -- > Michal Hocko > SUSE Labs
On Tue 12-03-19 19:04:43, Yafang Shao wrote: > On Mon, Mar 11, 2019 at 4:47 PM Michal Hocko <mhocko@kernel.org> wrote: > > > > On Fri 01-03-19 15:38:54, Yafang Shao wrote: > > > If we want to know the zone type, we have to check whether > > > CONFIG_ZONE_DMA, CONFIG_ZONE_DMA32 and CONFIG_HIGHMEM are set or not, > > > that's not so convenient. > > > > > > We'd better show the zone type directly. > > > > I do agree that zone number is quite PITA to process in general but do > > we really need this information in the first place? Why do we even care? > > > > Sometimes we want to know this event occurs in which zone, then we can > get the information of this zone, > for example via /proc/zoneinfo. > It could give us more information for debugging. Could you be more specific please? > > Zones are an MM internal implementation details and the more we export > > to the userspace the more we are going to argue about breaking userspace > > when touching them. So I would rather not export that information unless > > it is terribly useful. > > > > I 'm not sure whether zone type is terribly useful or not, but the > 'zid' is useless at all. > > I don't agree that Zones are MM internal. > We can get the zone type in many ways, for example /proc/zoneinfo. > > If we show this event occurs in which zone, we'd better show the zone type, > or we should drop this 'zid'. Yes, I am suggesting the later. If somebody really needs it then I would like to see a _specific_ usecase. Then we can add the proper name.
On Tue, Mar 12, 2019 at 9:38 PM Michal Hocko <mhocko@kernel.org> wrote: > > On Tue 12-03-19 19:04:43, Yafang Shao wrote: > > On Mon, Mar 11, 2019 at 4:47 PM Michal Hocko <mhocko@kernel.org> wrote: > > > > > > On Fri 01-03-19 15:38:54, Yafang Shao wrote: > > > > If we want to know the zone type, we have to check whether > > > > CONFIG_ZONE_DMA, CONFIG_ZONE_DMA32 and CONFIG_HIGHMEM are set or not, > > > > that's not so convenient. > > > > > > > > We'd better show the zone type directly. > > > > > > I do agree that zone number is quite PITA to process in general but do > > > we really need this information in the first place? Why do we even care? > > > > > > > Sometimes we want to know this event occurs in which zone, then we can > > get the information of this zone, > > for example via /proc/zoneinfo. > > It could give us more information for debugging. > > Could you be more specific please? > Honestly speaking, this one hasn't help us fix the real issue yet. > > > Zones are an MM internal implementation details and the more we export > > > to the userspace the more we are going to argue about breaking userspace > > > when touching them. So I would rather not export that information unless > > > it is terribly useful. > > > > > > > I 'm not sure whether zone type is terribly useful or not, but the > > 'zid' is useless at all. > > > > I don't agree that Zones are MM internal. > > We can get the zone type in many ways, for example /proc/zoneinfo. > > > > If we show this event occurs in which zone, we'd better show the zone type, > > or we should drop this 'zid'. > > Yes, I am suggesting the later. If somebody really needs it then I would > like to see a _specific_ usecase. Then we can add the proper name. This 'zid' always seems like a noise currently. I will send a patch to drop this one. Thanks Yafang
diff --git a/include/trace/events/vmscan.h b/include/trace/events/vmscan.h index a1cb913..4c8880b 100644 --- a/include/trace/events/vmscan.h +++ b/include/trace/events/vmscan.h @@ -73,7 +73,10 @@ __entry->order = order; ), - TP_printk("nid=%d zid=%d order=%d", __entry->nid, __entry->zid, __entry->order) + TP_printk("nid=%d zid=%-8s order=%d", + __entry->nid, + __print_symbolic(__entry->zid, ZONE_TYPE), + __entry->order) ); TRACE_EVENT(mm_vmscan_wakeup_kswapd, @@ -96,9 +99,9 @@ __entry->gfp_flags = gfp_flags; ), - TP_printk("nid=%d zid=%d order=%d gfp_flags=%s", + TP_printk("nid=%d zid=%-8s order=%d gfp_flags=%s", __entry->nid, - __entry->zid, + __print_symbolic(__entry->zid, ZONE_TYPE), __entry->order, show_gfp_flags(__entry->gfp_flags)) );
If we want to know the zone type, we have to check whether CONFIG_ZONE_DMA, CONFIG_ZONE_DMA32 and CONFIG_HIGHMEM are set or not, that's not so convenient. We'd better show the zone type directly. Signed-off-by: Yafang Shao <laoar.shao@gmail.com> --- include/trace/events/vmscan.h | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)