[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 17/19] [not for merge] print more debug info in mm
From: |
Daniel Kiper |
Subject: |
Re: [PATCH 17/19] [not for merge] print more debug info in mm |
Date: |
Wed, 10 Nov 2021 23:17:54 +0100 |
User-agent: |
NeoMutt/20170113 (1.7.2) |
On Wed, Nov 10, 2021 at 01:35:12PM -0600, Glenn Washburn wrote:
> On Wed, 10 Nov 2021 14:47:07 +0100
> Daniel Kiper <dkiper@net-space.pl> wrote:
>
> > CC-ing Glenn...
> >
> > On Tue, Oct 12, 2021 at 06:30:06PM +1100, Daniel Axtens wrote:
> > > This is handy for debugging - I'm including it in case anyone else hacking
> > > on this area finds it helpful.
> > >
> > > Signed-off-by: Daniel Axtens <dja@axtens.net>
> > > ---
> > > grub-core/kern/mm.c | 8 ++++++++
> > > 1 file changed, 8 insertions(+)
> > >
> > > diff --git a/grub-core/kern/mm.c b/grub-core/kern/mm.c
> > > index 58d5b89e8860..811df1ab5ebb 100644
> > > --- a/grub-core/kern/mm.c
> > > +++ b/grub-core/kern/mm.c
> > > @@ -135,6 +135,9 @@ grub_mm_init_region (void *addr, grub_size_t size)
> > > for (p = &grub_mm_base, q = *p; q; p = &(q->next), q = *p)
> > > {
> > > /* Does this region come _before_ an existing region? */
> > > + grub_printf ("Extending w/ before %p + %" PRIxGRUB_SIZE " + %"
> > > PRIxGRUB_SIZE " = %p ? %s\n",
> > > + (grub_uint8_t *)addr, size, q->pre_size, (grub_uint8_t *)q,
> > > + (grub_uint8_t *)addr + size + q->pre_size == (grub_uint8_t *)
> > > q ? "yes" : "no");
> >
> > I think this kind of messages can be useful. Same applies to patch #18.
> > Though I would use grub_dprintf() instead which should be wrapped with
> > #ifdef MM_DEBUG. However, we have to be very careful with printing any
> > messages from mm and do not exeecec 255 chars message length. If we go
> > above that limit then we will trigger dynamic allocation in grub_dprintf()
> > from mm which may lead to a recursion...
> >
> > Additionally, I think Glenn's patch allowing us to disable logging from
> > certain subsystem would be useful here. Glenn, could you take a look at
> > it once again?
>
> Yes, I'm planning on it. Its been low priority for me and this month is
Cool! Thanks!
> very busy fo me IRL. I don't think my patches should hold this up
> though (or would this cause way to much logs if one didn't want them?)
No rush...
Daniel