Re: [PATCH 17/19] [not for merge] print more debug info in mm

[Glenn Washburn]

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
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?)

Glenn