| Message ID | 1673235231-30302-9-git-send-email-byungchul.park@lge.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | DEPT(Dependency Tracker) | expand |
On 9 Jan 2023 12:33:36 +0900 Byungchul Park <byungchul.park@lge.com> > Makes Dept able to track dependencies by PG_{locked,writeback} waits. > > Signed-off-by: Byungchul Park <byungchul.park@lge.com> > --- Hey Byungchul Is DEPT able to get deadlock reported for the syzbot report [1]? Hillf [1] https://lore.kernel.org/lkml/6383cde5-cf4b-facf-6e07-1378a485657d@I-love.SAKURA.ne.jp/ > mm/filemap.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/mm/filemap.c b/mm/filemap.c > index c4d4ace..b013a5b 100644 > --- a/mm/filemap.c > +++ b/mm/filemap.c > @@ -42,6 +42,7 @@ > #include <linux/ramfs.h> > #include <linux/page_idle.h> > #include <linux/migrate.h> > +#include <linux/dept_sdt.h> > #include <asm/pgalloc.h> > #include <asm/tlbflush.h> > #include "internal.h" > @@ -1215,6 +1216,9 @@ static inline bool folio_trylock_flag(struct folio *folio, int bit_nr, > /* How many times do we accept lock stealing from under a waiter? */ > int sysctl_page_lock_unfairness = 5; > > +static struct dept_map __maybe_unused PG_locked_map = DEPT_MAP_INITIALIZER(PG_locked_map, NULL); > +static struct dept_map __maybe_unused PG_writeback_map = DEPT_MAP_INITIALIZER(PG_writeback_map, NULL); > + > static inline int folio_wait_bit_common(struct folio *folio, int bit_nr, > int state, enum behavior behavior) > { > @@ -1226,6 +1230,11 @@ static inline int folio_wait_bit_common(struct folio *folio, int bit_nr, > unsigned long pflags; > bool in_thrashing; > > + if (bit_nr == PG_locked) > + sdt_might_sleep_strong(&PG_locked_map); > + else if (bit_nr == PG_writeback) > + sdt_might_sleep_strong(&PG_writeback_map); > + > if (bit_nr == PG_locked && > !folio_test_uptodate(folio) && folio_test_workingset(folio)) { > delayacct_thrashing_start(&in_thrashing); > @@ -1327,6 +1336,8 @@ static inline int folio_wait_bit_common(struct folio *folio, int bit_nr, > */ > finish_wait(q, wait); > > + sdt_might_sleep_finish(); > + > if (thrashing) { > delayacct_thrashing_end(&in_thrashing); > psi_memstall_leave(&pflags); > -- > 1.9.1
Hillf wrote: > On 9 Jan 2023 12:33:36 +0900 Byungchul Park <byungchul.park@lge.com> >> Makes Dept able to track dependencies by PG_{locked,writeback} waits. >> >> Signed-off-by: Byungchul Park <byungchul.park@lge.com> >> --- > > Hey Byungchul +cc max.byungchul.park@gmail.com Hi, This email never reached to me. > Is DEPT able to get deadlock reported for the syzbot report [1]? DEPT can detect the case 100% *IF* the folio_trylock() is released within the same context since DEPT tracks folio_trylock(), folio_lock() and folio_unlock(), and it's definitely a deadlock. But as we know, because folio_trylock() can be released by another context like irq, it might be either just a severe slowdown of the context triggering folio_unlock() or a literal deadlock where the context is involved. I dunno which one is the case. In short, DEPT can detect this case too *IF* it's a literal deadlock, but it doesn't if it's just a slowdown. I'm planning to warn it even if there is a slowdown tho, not for now. Let me reproduce the following issue. I will share the result. > Hillf > > [1] https://lore.kernel.org/lkml/6383cde5-cf4b-facf-6e07-1378a485657d@I-love.SAKURA.ne.jp/ Byungchul
Byungchul wrote: > Hillf wrote: > > On 9 Jan 2023 12:33:36 +0900 Byungchul Park <byungchul.park@lge.com> > > > Makes Dept able to track dependencies by PG_{locked,writeback} waits. > > > > > > Signed-off-by: Byungchul Park <byungchul.park@lge.com> > > > --- > > > > Hey Byungchul > > +cc max.byungchul.park@gmail.com > > Hi, > > This email never reached to me. > > > Is DEPT able to get deadlock reported for the syzbot report [1]? > > DEPT can detect the case 100% *IF* the folio_trylock() is released > within the same context since DEPT tracks folio_trylock(), folio_lock() > and folio_unlock(), and it's definitely a deadlock. > > But as we know, because folio_trylock() can be released by another > context like irq, it might be either just a severe slowdown of the > context triggering folio_unlock() or a literal deadlock where the > context is involved. I dunno which one is the case. > > In short, DEPT can detect this case too *IF* it's a literal deadlock, > but it doesn't if it's just a slowdown. I'm planning to warn it even if > there is a slowdown tho, not for now. > > Let me reproduce the following issue. I will share the result. Hi Hillf, Can we talk about the DEPT report for the hang issue in here? https://lore.kernel.org/lkml/1673235231-30302-1-git-send-email-byungchul.park@lge.com/T/#m458f4d5f3da06a28c7fbb39b392d05e4c016603b Thanks, Byungchul > > Hillf > > > > [1] https://lore.kernel.org/lkml/6383cde5-cf4b-facf-6e07-1378a485657d@I-love.SAKURA.ne.jp/ > > Byungchul >
On Sat, 21 Jan 2023 12:35:09 +0900 Byungchul Park <byungchul.park@lge.com> > > Can we talk about the DEPT report for the hang issue in here? > > https://lore.kernel.org/lkml/1673235231-30302-1-git-send-email-byungchul.park@lge.com/T/#m458f4d5f3da06a28c7fbb39b392d05e4c016603b Yeah I am all ears after seeing your post. [ 227.857551] context A [ 227.857803] [S] lock(&ni->ni_lock:0) [ 227.858175] [W] folio_wait_bit_common(PG_locked_map:0) [ 227.858658] [E] unlock(&ni->ni_lock:0) [ 227.859233] context B [ 227.859484] [S] (unknown)(PG_locked_map:0) [ 227.859906] [W] lock(&ni->ni_lock:0) [ 227.860277] [E] folio_unlock(PG_locked_map:0)
diff --git a/mm/filemap.c b/mm/filemap.c index c4d4ace..b013a5b 100644 --- a/mm/filemap.c +++ b/mm/filemap.c @@ -42,6 +42,7 @@ #include <linux/ramfs.h> #include <linux/page_idle.h> #include <linux/migrate.h> +#include <linux/dept_sdt.h> #include <asm/pgalloc.h> #include <asm/tlbflush.h> #include "internal.h" @@ -1215,6 +1216,9 @@ static inline bool folio_trylock_flag(struct folio *folio, int bit_nr, /* How many times do we accept lock stealing from under a waiter? */ int sysctl_page_lock_unfairness = 5; +static struct dept_map __maybe_unused PG_locked_map = DEPT_MAP_INITIALIZER(PG_locked_map, NULL); +static struct dept_map __maybe_unused PG_writeback_map = DEPT_MAP_INITIALIZER(PG_writeback_map, NULL); + static inline int folio_wait_bit_common(struct folio *folio, int bit_nr, int state, enum behavior behavior) { @@ -1226,6 +1230,11 @@ static inline int folio_wait_bit_common(struct folio *folio, int bit_nr, unsigned long pflags; bool in_thrashing; + if (bit_nr == PG_locked) + sdt_might_sleep_strong(&PG_locked_map); + else if (bit_nr == PG_writeback) + sdt_might_sleep_strong(&PG_writeback_map); + if (bit_nr == PG_locked && !folio_test_uptodate(folio) && folio_test_workingset(folio)) { delayacct_thrashing_start(&in_thrashing); @@ -1327,6 +1336,8 @@ static inline int folio_wait_bit_common(struct folio *folio, int bit_nr, */ finish_wait(q, wait); + sdt_might_sleep_finish(); + if (thrashing) { delayacct_thrashing_end(&in_thrashing); psi_memstall_leave(&pflags);
Makes Dept able to track dependencies by PG_{locked,writeback} waits. Signed-off-by: Byungchul Park <byungchul.park@lge.com> --- mm/filemap.c | 11 +++++++++++ 1 file changed, 11 insertions(+)