| Message ID | 20181127083603.39041-1-heiko.carstens@de.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mm: warn only once if page table misaccounting is detected | expand |
On Tue, Nov 27, 2018 at 09:36:03AM +0100, Heiko Carstens wrote: > Use pr_alert_once() instead of pr_alert() if page table misaccounting > has been detected. > > If this happens once it is very likely that there will be numerous > other occurrence as well, which would flood dmesg and the console with > hardly any added information. Therefore print the warning only once. > > Cc: Kirill A. Shutemov <kirill@shutemov.name> > Cc: Martin Schwidefsky <schwidefsky@de.ibm.com> > Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com> Fair enough. Acked-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
On Tue 27-11-18 09:36:03, Heiko Carstens wrote: > Use pr_alert_once() instead of pr_alert() if page table misaccounting > has been detected. > > If this happens once it is very likely that there will be numerous > other occurrence as well, which would flood dmesg and the console with > hardly any added information. Therefore print the warning only once. Have you actually experience a flood of these messages? Is one per mm message really that much? If yes why rss counters do not exhibit the same problem? > Cc: Kirill A. Shutemov <kirill@shutemov.name> > Cc: Martin Schwidefsky <schwidefsky@de.ibm.com> > Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com> > --- > kernel/fork.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/kernel/fork.c b/kernel/fork.c > index 07cddff89c7b..c887e9eba89f 100644 > --- a/kernel/fork.c > +++ b/kernel/fork.c > @@ -647,8 +647,8 @@ static void check_mm(struct mm_struct *mm) > } > > if (mm_pgtables_bytes(mm)) > - pr_alert("BUG: non-zero pgtables_bytes on freeing mm: %ld\n", > - mm_pgtables_bytes(mm)); > + pr_alert_once("BUG: non-zero pgtables_bytes on freeing mm: %ld\n", > + mm_pgtables_bytes(mm)); > > #if defined(CONFIG_TRANSPARENT_HUGEPAGE) && !USE_SPLIT_PMD_PTLOCKS > VM_BUG_ON_MM(mm->pmd_huge_pte, mm); > -- > 2.16.4
On Tue, Nov 27, 2018 at 02:19:16PM +0100, Michal Hocko wrote: > On Tue 27-11-18 09:36:03, Heiko Carstens wrote: > > Use pr_alert_once() instead of pr_alert() if page table misaccounting > > has been detected. > > > > If this happens once it is very likely that there will be numerous > > other occurrence as well, which would flood dmesg and the console with > > hardly any added information. Therefore print the warning only once. > > Have you actually experience a flood of these messages? Is one per mm > message really that much? Yes, I did. Since in this case all compat processes caused the message to appear, I saw thousands of these messages. > If yes why rss counters do not exhibit the same problem? No rss counter messages appeared. Or do you suggest that the other pr_alert() within check_mm() should also be changed?
On Tue, Nov 27, 2018 at 09:36:03AM +0100, Heiko Carstens wrote: > Use pr_alert_once() instead of pr_alert() if page table misaccounting > has been detected. > > If this happens once it is very likely that there will be numerous > other occurrence as well, which would flood dmesg and the console with > hardly any added information. Therefore print the warning only once. > > Cc: Kirill A. Shutemov <kirill@shutemov.name> > Cc: Martin Schwidefsky <schwidefsky@de.ibm.com> > Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com> > --- > kernel/fork.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/kernel/fork.c b/kernel/fork.c > index 07cddff89c7b..c887e9eba89f 100644 > --- a/kernel/fork.c > +++ b/kernel/fork.c > @@ -647,8 +647,8 @@ static void check_mm(struct mm_struct *mm) > } > > if (mm_pgtables_bytes(mm)) > - pr_alert("BUG: non-zero pgtables_bytes on freeing mm: %ld\n", > - mm_pgtables_bytes(mm)); > + pr_alert_once("BUG: non-zero pgtables_bytes on freeing mm: %ld\n", > + mm_pgtables_bytes(mm)); I found the print-always behavior to be useful when developing a driver that mucked with PTEs directly via vmf_insert_pfn() and had issues with racing against exit_mmap(). It was nice to be able to recompile only the driver and rely on dmesg to let me know when I messed up yet again. Would pr_alert_ratelimited() suffice? > #if defined(CONFIG_TRANSPARENT_HUGEPAGE) && !USE_SPLIT_PMD_PTLOCKS > VM_BUG_ON_MM(mm->pmd_huge_pte, mm); > -- > 2.16.4 >
> On Nov 27, 2018, at 8:52 AM, Sean Christopherson <sean.j.christopherson@intel.com> wrote: > > On Tue, Nov 27, 2018 at 09:36:03AM +0100, Heiko Carstens wrote: >> Use pr_alert_once() instead of pr_alert() if page table misaccounting >> has been detected. >> >> If this happens once it is very likely that there will be numerous >> other occurrence as well, which would flood dmesg and the console with >> hardly any added information. Therefore print the warning only once. >> >> Cc: Kirill A. Shutemov <kirill@shutemov.name> >> Cc: Martin Schwidefsky <schwidefsky@de.ibm.com> >> Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com> >> --- >> kernel/fork.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/kernel/fork.c b/kernel/fork.c >> index 07cddff89c7b..c887e9eba89f 100644 >> --- a/kernel/fork.c >> +++ b/kernel/fork.c >> @@ -647,8 +647,8 @@ static void check_mm(struct mm_struct *mm) >> } >> >> if (mm_pgtables_bytes(mm)) >> - pr_alert("BUG: non-zero pgtables_bytes on freeing mm: %ld\n", >> - mm_pgtables_bytes(mm)); >> + pr_alert_once("BUG: non-zero pgtables_bytes on freeing mm: %ld\n", >> + mm_pgtables_bytes(mm)); > > I found the print-always behavior to be useful when developing a driver > that mucked with PTEs directly via vmf_insert_pfn() and had issues with > racing against exit_mmap(). It was nice to be able to recompile only > the driver and rely on dmesg to let me know when I messed up yet again. > > Would pr_alert_ratelimited() suffice? Actually, I really like that idea. There are certainly times when it is useful to see a cascade of messages, within reason; one there are so many they overflow the dmesg buffer they're of limited usefulness. Something like a pr_alert() that could rate limit to a preset value, perhaps a default of fifty or so, could prove quite useful indeed without being an all or once choice. William Kucharski
On Tue 27-11-18 15:36:38, Heiko Carstens wrote: > On Tue, Nov 27, 2018 at 02:19:16PM +0100, Michal Hocko wrote: > > On Tue 27-11-18 09:36:03, Heiko Carstens wrote: > > > Use pr_alert_once() instead of pr_alert() if page table misaccounting > > > has been detected. > > > > > > If this happens once it is very likely that there will be numerous > > > other occurrence as well, which would flood dmesg and the console with > > > hardly any added information. Therefore print the warning only once. > > > > Have you actually experience a flood of these messages? Is one per mm > > message really that much? > > Yes, I did. Since in this case all compat processes caused the message > to appear, I saw thousands of these messages. This means something went colossally wrong and seeing an avalanche of messages might be actually helpful because you can at least see the pattern. I wonder whether the underlying issue would be obvious from a single instance. Maybe we want ratelimit instead? > > If yes why rss counters do not exhibit the same problem? > > No rss counter messages appeared. Or do you suggest that the other > pr_alert() within check_mm() should also be changed? Whatever we go with (and I do not have a strong opinion here) we should be consistent I believe.
diff --git a/kernel/fork.c b/kernel/fork.c index 07cddff89c7b..c887e9eba89f 100644 --- a/kernel/fork.c +++ b/kernel/fork.c @@ -647,8 +647,8 @@ static void check_mm(struct mm_struct *mm) } if (mm_pgtables_bytes(mm)) - pr_alert("BUG: non-zero pgtables_bytes on freeing mm: %ld\n", - mm_pgtables_bytes(mm)); + pr_alert_once("BUG: non-zero pgtables_bytes on freeing mm: %ld\n", + mm_pgtables_bytes(mm)); #if defined(CONFIG_TRANSPARENT_HUGEPAGE) && !USE_SPLIT_PMD_PTLOCKS VM_BUG_ON_MM(mm->pmd_huge_pte, mm);
Use pr_alert_once() instead of pr_alert() if page table misaccounting has been detected. If this happens once it is very likely that there will be numerous other occurrence as well, which would flood dmesg and the console with hardly any added information. Therefore print the warning only once. Cc: Kirill A. Shutemov <kirill@shutemov.name> Cc: Martin Schwidefsky <schwidefsky@de.ibm.com> Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com> --- kernel/fork.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)