| Message ID | 20191021085140.14030-1-zhuguangqing83@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | PM/wakeup: Add print_wakeup_sour_stats(m, &deleted_ws) | expand |
On Monday, October 21, 2019 10:51:40 AM CET zhuguangqing83@gmail.com wrote: > From: zhuguangqing <zhuguangqing@xiaomi.com> > > After commit 00ee22c28915 (PM / wakeup: Use seq_open() > to show wakeup stats), print_wakeup_source_stats(m, &deleted_ws) > is deleted in function wakeup_sources_stats_seq_show(). > > Because deleted_ws is one of wakeup sources, so it should > also be showed. This patch add it to the end of all other > wakeup sources. > > Signed-off-by: zhuguangqing <zhuguangqing@xiaomi.com> > --- > drivers/base/power/wakeup.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c > index 5817b51d2b15..29e9434ccaaa 100644 > --- a/drivers/base/power/wakeup.c > +++ b/drivers/base/power/wakeup.c > @@ -1071,6 +1071,9 @@ static void *wakeup_sources_stats_seq_next(struct seq_file *m, > break; > } > > + if (&ws->entry == &wakeup_sources) > + print_wakeup_source_stats(m, &deleted_ws); > + That would be when NULL is about to be returned, right? Why not to check for !next_ws instead, then? Moreover, why to call print_wakeup_source_stats() directly instead of returning &deleted_ws? Also it looks like you need a similar change in wakeup_sources_stats_seq_start(), in case n is greater than the number of list entries, don't you? > return next_ws; > } > >
Rafael J. Wysocki <rjw@rjwysocki.net> 于2019年11月8日周五 下午7:34写道： > > On Monday, October 21, 2019 10:51:40 AM CET zhuguangqing83@gmail.com wrote: > > From: zhuguangqing <zhuguangqing@xiaomi.com> > > > > After commit 00ee22c28915 (PM / wakeup: Use seq_open() > > to show wakeup stats), print_wakeup_source_stats(m, &deleted_ws) > > is deleted in function wakeup_sources_stats_seq_show(). > > > > Because deleted_ws is one of wakeup sources, so it should > > also be showed. This patch add it to the end of all other > > wakeup sources. > > > > Signed-off-by: zhuguangqing <zhuguangqing@xiaomi.com> > > --- > > drivers/base/power/wakeup.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c > > index 5817b51d2b15..29e9434ccaaa 100644 > > --- a/drivers/base/power/wakeup.c > > +++ b/drivers/base/power/wakeup.c > > @@ -1071,6 +1071,9 @@ static void *wakeup_sources_stats_seq_next(struct seq_file *m, > > break; > > } > > > > + if (&ws->entry == &wakeup_sources) > > + print_wakeup_source_stats(m, &deleted_ws); > > + > > That would be when NULL is about to be returned, right? > > Why not to check for !next_ws instead, then? > Yes, that would be when NULL is about to be returned, so check for !next_ws instead is ok, and it's more suitable. > Moreover, why to call print_wakeup_source_stats() directly instead of returning > &deleted_ws? > > Also it looks like you need a similar change in wakeup_sources_stats_seq_start(), > in case n is greater than the number of list entries, don't you? > There are three reasons that I think calling print_wakeup_source_stats() directly is better. 1, Although deleted_ws is a wakeup_source, it is not in LIST_HEAD(wakeup_sources), it's a special wakeup_source. The intial design (before commit 00ee22c28915) is also using two seperated print_wakeup_source_stats() for LIST_HEAD(wakeup_sources) and deleted_ws. 2, If returning &deleted_ws, then wakeup_sources_stats_seq_show() and wakeup_sources_stats_seq_next() will run one more time. 3, If ++(*pos); in wakeup_sources_stats_seq_next() runs one more time, then n in wakeup_sources_stats_seq_start() may be greater than the number of list entries, and it needs some more change. Thanks for your comments. > > return next_ws; > > } > > > > > > > >
diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c index 5817b51d2b15..29e9434ccaaa 100644 --- a/drivers/base/power/wakeup.c +++ b/drivers/base/power/wakeup.c @@ -1071,6 +1071,9 @@ static void *wakeup_sources_stats_seq_next(struct seq_file *m, break; } + if (&ws->entry == &wakeup_sources) + print_wakeup_source_stats(m, &deleted_ws); + return next_ws; }