* [PATCH] mm/page-writeback: introduce tracepoint for wait_on_page_writeback
@ 2019-04-26 10:26 Yafang Shao
2019-04-26 18:25 ` Andrew Morton
0 siblings, 1 reply; 5+ messages in thread
From: Yafang Shao @ 2019-04-26 10:26 UTC (permalink / raw)
To: jack, mhocko, akpm; +Cc: linux-mm, shaoyafang, Yafang Shao
Recently there're some hungtasks on our server due to
wait_on_page_writeback, and we want to know the details of this
PG_writeback, i.e. this page is writing back to which device.
But it is not so convenient to get the details.
I think it would be better to introduce a tracepoint for diagnosing
the writeback details.
Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
include/linux/pagemap.h | 10 +---------
include/trace/events/writeback.h | 16 +++++++++++++++-
mm/page-writeback.c | 12 ++++++++++++
3 files changed, 28 insertions(+), 10 deletions(-)
diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
index f939e00..0f26c38 100644
--- a/include/linux/pagemap.h
+++ b/include/linux/pagemap.h
@@ -537,15 +537,7 @@ static inline int wait_on_page_locked_killable(struct page *page)
extern void put_and_wait_on_page_locked(struct page *page);
-/*
- * Wait for a page to complete writeback
- */
-static inline void wait_on_page_writeback(struct page *page)
-{
- if (PageWriteback(page))
- wait_on_page_bit(page, PG_writeback);
-}
-
+void wait_on_page_writeback(struct page *page);
extern void end_page_writeback(struct page *page);
void wait_for_stable_page(struct page *page);
diff --git a/include/trace/events/writeback.h b/include/trace/events/writeback.h
index 32db72c..aa7f3ae 100644
--- a/include/trace/events/writeback.h
+++ b/include/trace/events/writeback.h
@@ -53,7 +53,7 @@
struct wb_writeback_work;
-TRACE_EVENT(writeback_dirty_page,
+DECLARE_EVENT_CLASS(writeback_page_template,
TP_PROTO(struct page *page, struct address_space *mapping),
@@ -79,6 +79,20 @@
)
);
+DEFINE_EVENT(writeback_page_template, writeback_dirty_page,
+
+ TP_PROTO(struct page *page, struct address_space *mapping),
+
+ TP_ARGS(page, mapping)
+);
+
+DEFINE_EVENT(writeback_page_template, wait_on_page_writeback,
+
+ TP_PROTO(struct page *page, struct address_space *mapping),
+
+ TP_ARGS(page, mapping)
+);
+
DECLARE_EVENT_CLASS(writeback_dirty_inode_template,
TP_PROTO(struct inode *inode, int flags),
diff --git a/mm/page-writeback.c b/mm/page-writeback.c
index 9f61dfe..0765648 100644
--- a/mm/page-writeback.c
+++ b/mm/page-writeback.c
@@ -2808,6 +2808,18 @@ int __test_set_page_writeback(struct page *page, bool keep_write)
}
EXPORT_SYMBOL(__test_set_page_writeback);
+/*
+ * Wait for a page to complete writeback
+ */
+void wait_on_page_writeback(struct page *page)
+{
+ if (PageWriteback(page)) {
+ trace_wait_on_page_writeback(page, page_mapping(page));
+ wait_on_page_bit(page, PG_writeback);
+ }
+}
+EXPORT_SYMBOL_GPL(wait_on_page_writeback);
+
/**
* wait_for_stable_page() - wait for writeback to finish, if necessary.
* @page: The page to wait on.
--
1.8.3.1
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] mm/page-writeback: introduce tracepoint for wait_on_page_writeback
2019-04-26 10:26 [PATCH] mm/page-writeback: introduce tracepoint for wait_on_page_writeback Yafang Shao
@ 2019-04-26 18:25 ` Andrew Morton
2019-04-27 0:31 ` Yafang Shao
2019-04-28 21:05 ` Michal Hocko
0 siblings, 2 replies; 5+ messages in thread
From: Andrew Morton @ 2019-04-26 18:25 UTC (permalink / raw)
To: Yafang Shao; +Cc: jack, mhocko, linux-mm, shaoyafang
On Fri, 26 Apr 2019 18:26:42 +0800 Yafang Shao <laoar.shao@gmail.com> wrote:
> Recently there're some hungtasks on our server due to
> wait_on_page_writeback, and we want to know the details of this
> PG_writeback, i.e. this page is writing back to which device.
> But it is not so convenient to get the details.
>
> I think it would be better to introduce a tracepoint for diagnosing
> the writeback details.
Fair enough, I guess.
> --- a/include/linux/pagemap.h
> +++ b/include/linux/pagemap.h
> @@ -537,15 +537,7 @@ static inline int wait_on_page_locked_killable(struct page *page)
>
> extern void put_and_wait_on_page_locked(struct page *page);
>
> -/*
> - * Wait for a page to complete writeback
> - */
> -static inline void wait_on_page_writeback(struct page *page)
> -{
> - if (PageWriteback(page))
> - wait_on_page_bit(page, PG_writeback);
> -}
> -
> +void wait_on_page_writeback(struct page *page);
> extern void end_page_writeback(struct page *page);
> void wait_for_stable_page(struct page *page);
>
> ...
>
> +/*
> + * Wait for a page to complete writeback
> + */
> +void wait_on_page_writeback(struct page *page)
> +{
> + if (PageWriteback(page)) {
> + trace_wait_on_page_writeback(page, page_mapping(page));
> + wait_on_page_bit(page, PG_writeback);
> + }
> +}
> +EXPORT_SYMBOL_GPL(wait_on_page_writeback);
But this is a stealth change to the wait_on_page_writeback() licensing.
I will get sad emails from developers of accidentally-broken
out-of-tree filesystems.
We can discuss changing the licensing, but this isn't the way to do it!
--- a/mm/page-writeback.c~mm-page-writeback-introduce-tracepoint-for-wait_on_page_writeback-fix
+++ a/mm/page-writeback.c
@@ -2818,7 +2818,7 @@ void wait_on_page_writeback(struct page
wait_on_page_bit(page, PG_writeback);
}
}
-EXPORT_SYMBOL_GPL(wait_on_page_writeback);
+EXPORT_SYMBOL(wait_on_page_writeback);
/**
* wait_for_stable_page() - wait for writeback to finish, if necessary.
_
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] mm/page-writeback: introduce tracepoint for wait_on_page_writeback
2019-04-26 18:25 ` Andrew Morton
@ 2019-04-27 0:31 ` Yafang Shao
2019-04-28 21:05 ` Michal Hocko
1 sibling, 0 replies; 5+ messages in thread
From: Yafang Shao @ 2019-04-27 0:31 UTC (permalink / raw)
To: Andrew Morton; +Cc: Jan Kara, Michal Hocko, Linux MM, shaoyafang
On Sat, Apr 27, 2019 at 2:25 AM Andrew Morton <akpm@linux-foundation.org> wrote:
>
> On Fri, 26 Apr 2019 18:26:42 +0800 Yafang Shao <laoar.shao@gmail.com> wrote:
>
> > Recently there're some hungtasks on our server due to
> > wait_on_page_writeback, and we want to know the details of this
> > PG_writeback, i.e. this page is writing back to which device.
> > But it is not so convenient to get the details.
> >
> > I think it would be better to introduce a tracepoint for diagnosing
> > the writeback details.
>
> Fair enough, I guess.
>
> > --- a/include/linux/pagemap.h
> > +++ b/include/linux/pagemap.h
> > @@ -537,15 +537,7 @@ static inline int wait_on_page_locked_killable(struct page *page)
> >
> > extern void put_and_wait_on_page_locked(struct page *page);
> >
> > -/*
> > - * Wait for a page to complete writeback
> > - */
> > -static inline void wait_on_page_writeback(struct page *page)
> > -{
> > - if (PageWriteback(page))
> > - wait_on_page_bit(page, PG_writeback);
> > -}
> > -
> > +void wait_on_page_writeback(struct page *page);
> > extern void end_page_writeback(struct page *page);
> > void wait_for_stable_page(struct page *page);
> >
> > ...
> >
> > +/*
> > + * Wait for a page to complete writeback
> > + */
> > +void wait_on_page_writeback(struct page *page)
> > +{
> > + if (PageWriteback(page)) {
> > + trace_wait_on_page_writeback(page, page_mapping(page));
> > + wait_on_page_bit(page, PG_writeback);
> > + }
> > +}
> > +EXPORT_SYMBOL_GPL(wait_on_page_writeback);
>
> But this is a stealth change to the wait_on_page_writeback() licensing.
> I will get sad emails from developers of accidentally-broken
> out-of-tree filesystems.
>
> We can discuss changing the licensing, but this isn't the way to do it!
>
Got it.
Thanks for your explatation.
Thanks
Yafang
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] mm/page-writeback: introduce tracepoint for wait_on_page_writeback
2019-04-26 18:25 ` Andrew Morton
2019-04-27 0:31 ` Yafang Shao
@ 2019-04-28 21:05 ` Michal Hocko
2019-05-11 23:28 ` Andrew Morton
1 sibling, 1 reply; 5+ messages in thread
From: Michal Hocko @ 2019-04-28 21:05 UTC (permalink / raw)
To: Andrew Morton; +Cc: Yafang Shao, jack, linux-mm, shaoyafang
On Fri 26-04-19 11:25:42, Andrew Morton wrote:
> On Fri, 26 Apr 2019 18:26:42 +0800 Yafang Shao <laoar.shao@gmail.com> wrote:
[...]
> > +/*
> > + * Wait for a page to complete writeback
> > + */
> > +void wait_on_page_writeback(struct page *page)
> > +{
> > + if (PageWriteback(page)) {
> > + trace_wait_on_page_writeback(page, page_mapping(page));
> > + wait_on_page_bit(page, PG_writeback);
> > + }
> > +}
> > +EXPORT_SYMBOL_GPL(wait_on_page_writeback);
>
> But this is a stealth change to the wait_on_page_writeback() licensing.
Why do we have to put that out of line in the first place? Btw.
wait_on_page_bit is EXPORT_SYMBOL...
--
Michal Hocko
SUSE Labs
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] mm/page-writeback: introduce tracepoint for wait_on_page_writeback
2019-04-28 21:05 ` Michal Hocko
@ 2019-05-11 23:28 ` Andrew Morton
0 siblings, 0 replies; 5+ messages in thread
From: Andrew Morton @ 2019-05-11 23:28 UTC (permalink / raw)
To: Michal Hocko; +Cc: Yafang Shao, jack, linux-mm, shaoyafang
On Sun, 28 Apr 2019 23:05:38 +0200 Michal Hocko <mhocko@suse.com> wrote:
> On Fri 26-04-19 11:25:42, Andrew Morton wrote:
> > On Fri, 26 Apr 2019 18:26:42 +0800 Yafang Shao <laoar.shao@gmail.com> wrote:
> [...]
> > > +/*
> > > + * Wait for a page to complete writeback
> > > + */
> > > +void wait_on_page_writeback(struct page *page)
> > > +{
> > > + if (PageWriteback(page)) {
> > > + trace_wait_on_page_writeback(page, page_mapping(page));
> > > + wait_on_page_bit(page, PG_writeback);
> > > + }
> > > +}
> > > +EXPORT_SYMBOL_GPL(wait_on_page_writeback);
> >
> > But this is a stealth change to the wait_on_page_writeback() licensing.
>
> Why do we have to put that out of line in the first place?
Seems like a good thing to do from a size and icache-footprint POV.
wait_on_page_writeback() has a ton of callsites and the allmodconfig
out-of-line version is around 600 bytes of code (gack).
> Btw. wait_on_page_bit is EXPORT_SYMBOL...
OK, I'll leave wait_on_page_writeback() as EXPROT_SYMBOL().
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-05-11 23:29 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-26 10:26 [PATCH] mm/page-writeback: introduce tracepoint for wait_on_page_writeback Yafang Shao
2019-04-26 18:25 ` Andrew Morton
2019-04-27 0:31 ` Yafang Shao
2019-04-28 21:05 ` Michal Hocko
2019-05-11 23:28 ` Andrew Morton
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox