Re: [bug report] mm: reduce noise in show_mem for lowmem allocations

[Dan Carpenter <dan.carpenter@oracle.com>]

On Tue, Aug 30, 2022 at 09:11:03AM +0200, Michal Hocko wrote:
> On Tue 30-08-22 10:02:05, Dan Carpenter wrote:
> > On Tue, Aug 30, 2022 at 08:46:58AM +0200, Michal Hocko wrote:
> > > On Tue 30-08-22 09:30:26, Dan Carpenter wrote:
> > > > Hello Michal Hocko,
> > > > 
> > > > The patch e8fedfea3dea: "mm: reduce noise in show_mem for lowmem
> > > > allocations" from Aug 23, 2022, leads to the following Smatch static
> > > > checker warning:
> > > > 
> > > > 	kernel/panic.c:190 panic_print_sys_info()
> > > > 	warn: sleeping in atomic context
> > > 
> > > What is this warning saying?
> > > 
> > 
> > This is a Smatch warning.
> > 
> > > >     189         if (panic_print & PANIC_PRINT_MEM_INFO)
> > > > --> 190                 show_mem(0, NULL, GFP_HIGHUSER_MOVABLE);
> > > >                                           ^^^^^^^^^^^^^^^^^^^^
> > > > This obviously seems very deliberate and a lot of weird stuff happens
> > > > during panic().  But the panic() function disables preemption so
> > > > shouldn't this be GFP_ATOMIC?  GFP_HIGHUSER_MOVABLE has __GFP_RECLAIM
> > > > and triggering swap during a panic seems bad.
> > > 
> > > This function shouldn't ever be allocating any memory. The flag is
> > > solely to infer which memory zones should be displayed. It acts as a
> > > filter. Is it possible that the checker misinterprets the parameter's
> > > meaning?
> > 
> > Ah.  Yes.  Smatch sees every gfp_t as a sleep/no sleep marker.  I
> > didn't realize it wasn't used like that here.  Thanks!
> 
> OK, fair enough and I can actually see how that can turn out into a real
> allocation in a distant future when the original intention has been lost
> in the past. Let me re-open the discussion for that patch and CC you
> there.

No no.  I can silence this in Smatch.  I hadn't really wanted to bother
because this one false positive doesn't stand out from all the other
false positives.

The sleeping in atomic check in Smatch is generally pretty good, but
when the call tree is five or more functions deep between disabling
preemption and the sleep then that's the main source of false positives.

regards,
dan carpenter