[bug report] powerpc/mm/radix: Avoid flushing the PWC on every flush_tlb_range

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[bug report] powerpc/mm/radix: Avoid flushing the PWC on every flush_tlb_range

Dan Carpenter-2
Hello Benjamin Herrenschmidt,

This is a semi-automatic email about new static checker warnings.

The patch 424de9c6e3f8: "powerpc/mm/radix: Avoid flushing the PWC on
every flush_tlb_range" from Jul 19, 2017, leads to the following
Smatch complaint:

arch/powerpc/mm/tlb-radix.c:368 radix__flush_tlb_collapsed_pmd()
         error: we previously assumed 'mm' could be null (see line 362)

arch/powerpc/mm/tlb-radix.c
   361
   362 pid = mm ? mm->context.id : 0;
                      ^^
Check for NULL.

   363 if (unlikely(pid == MMU_NO_CONTEXT))
   364 goto no_context;
   365
   366 /* 4k page size, just blow the world */
   367 if (PAGE_SIZE == 0x1000) {
   368 radix__flush_all_mm(mm);
                                            ^^
Unchecked dereference.

   369 return;
   370 }

regards,
dan carpenter
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [bug report] powerpc/mm/radix: Avoid flushing the PWC on every flush_tlb_range

Tyrel Datwyler
On 08/11/2017 01:15 PM, Dan Carpenter wrote:

> Hello Benjamin Herrenschmidt,
>
> This is a semi-automatic email about new static checker warnings.
>
> The patch 424de9c6e3f8: "powerpc/mm/radix: Avoid flushing the PWC on
> every flush_tlb_range" from Jul 19, 2017, leads to the following
> Smatch complaint:
>
> arch/powerpc/mm/tlb-radix.c:368 radix__flush_tlb_collapsed_pmd()
> error: we previously assumed 'mm' could be null (see line 362)
>
> arch/powerpc/mm/tlb-radix.c
>    361
>    362 pid = mm ? mm->context.id : 0;
>                       ^^
> Check for NULL.
>
>    363 if (unlikely(pid == MMU_NO_CONTEXT))
>    364 goto no_context;
>    365
>    366 /* 4k page size, just blow the world */
>    367 if (PAGE_SIZE == 0x1000) {
>    368 radix__flush_all_mm(mm);
>                                             ^^
> Unchecked dereference.

Appears to be a false positive. MMU_NO_CONTEXT I believe is defined as "0". So, maybe it
would be clearer that we take the goto branch if this line read:

362 pid = mm ? mm->context.id : MMU_NO_CONTEXT;

-Tyrel

>
>    369 return;
>    370 }
>
> regards,
> dan carpenter
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [bug report] powerpc/mm/radix: Avoid flushing the PWC on every flush_tlb_range

Dan Carpenter-2
On Fri, Aug 11, 2017 at 03:16:08PM -0700, Tyrel Datwyler wrote:

> On 08/11/2017 01:15 PM, Dan Carpenter wrote:
> > Hello Benjamin Herrenschmidt,
> >
> > This is a semi-automatic email about new static checker warnings.
> >
> > The patch 424de9c6e3f8: "powerpc/mm/radix: Avoid flushing the PWC on
> > every flush_tlb_range" from Jul 19, 2017, leads to the following
> > Smatch complaint:
> >
> > arch/powerpc/mm/tlb-radix.c:368 radix__flush_tlb_collapsed_pmd()
> > error: we previously assumed 'mm' could be null (see line 362)
> >
> > arch/powerpc/mm/tlb-radix.c
> >    361
> >    362 pid = mm ? mm->context.id : 0;
> >                       ^^
> > Check for NULL.
> >
> >    363 if (unlikely(pid == MMU_NO_CONTEXT))
> >    364 goto no_context;
> >    365
> >    366 /* 4k page size, just blow the world */
> >    367 if (PAGE_SIZE == 0x1000) {
> >    368 radix__flush_all_mm(mm);
> >                                             ^^
> > Unchecked dereference.
>
> Appears to be a false positive. MMU_NO_CONTEXT I believe is defined as "0". So, maybe it
> would be clearer that we take the goto branch if this line read:
>
> 362 pid = mm ? mm->context.id : MMU_NO_CONTEXT;
>

Ah...  This is because I'm compiling code for other arches in a "best
effort" way.  It doesn't pull in the right headers so it doesn't know
the value of MMU_NO_CONTEXT.  Otherwise it would read the code correctly
and not complain.

Sorry for that.

regards,
dan carpenter
Loading...