Re: [PATCH v2] MIPS: Fix incorrect NULL check in local_flush_tlb_page()

[Emil Goode]

Hello Maciej,

I didn't know it was possible to go beyond commit 1da177e,
thank you for the info.

Best regards,

Emil

On Sun, Jul 06, 2014 at 12:16:38PM +0100, Maciej W. Rozycki wrote:
> On Sun, 6 Jul 2014, Emil Goode wrote:
> 
> > > > diff --git a/arch/mips/mm/tlb-r3k.c b/arch/mips/mm/tlb-r3k.c
> > > > index d657493..6546758 100644
> > > > --- a/arch/mips/mm/tlb-r3k.c
> > > > +++ b/arch/mips/mm/tlb-r3k.c
> > > > @@ -158,7 +158,7 @@ void local_flush_tlb_page(struct vm_area_struct *vma, unsigned long page)
> > > >  {
> > > >         int cpu = smp_processor_id();
> > > >
> > > > -       if (!vma || cpu_context(cpu, vma->vm_mm) != 0) {
> > > > +       if (vma && cpu_context(cpu, vma->vm_mm) != 0) {
> > > 
> > > Sorry for replying "too late", but grepping through the kernel code I
> > > fail to find any caller that does not dereference vma before calling
> > > (local)flush_tlb_page(). Also both tlb-4k and tlb-8k assume vma cannot
> > > be NULL, so I would say it is safe to assume vma is never NULL, and
> > > the NULL check can be removed completely.
> > > 
> > > Also it looks like this "bug" was there since at least 2.6.12, and
> > > never seem to have bitten anyone.
> > 
> > Yes, the bug pre-dates GIT history and I agree that it is most unlikely
> > that it ever caused a problem. I will send a new patch that removes the
> > NULL check of vma.
> 
>  Well, as at 2.4.26 (picked randomly) code in arch/mips/mm/tlb-r4k.c had 
> the same check:
> 
> void local_flush_tlb_page(struct vm_area_struct *vma, unsigned long page)
> {
> 	int cpu = smp_processor_id();
> 
> 	if (!vma || cpu_context(cpu, vma->vm_mm) != 0) {
> 		unsigned long flags;
> 		[...]
> 
> but code in arch/mips64/mm/tlb-r4k.c did not:
> 
> void local_flush_tlb_page(struct vm_area_struct *vma, unsigned long page)
> {
> 	int cpu = smp_processor_id();
> 
> 	if (cpu_context(cpu, vma->vm_mm) != 0) {
> 		unsigned long flags;
> 		[...]
> 
> and the current situation is an artefact from the 32-bit and 64-bit port 
> unification:
> 
> commit efc2f9a8a100511399309a50a33b46ff099f3454
> Author: Ralf Baechle <ralf@xxxxxxxxxxxxxx>
> Date:   Mon Jul 21 03:03:15 2003 +0000
> 
>     Unify tlb-r4k.c.
> 
> -- at which point tlb-r3k.c wasn't updated accordingly.  The condition was 
> eventually removed from arch/mips/mm/tlb-r4k.c in 2.4 too with:
> 
> commit fd8917812546ef2278e006237a7cc38497bfbf52
> Author: Ralf Baechle <ralf@xxxxxxxxxxxxxx>
> Date:   Thu Nov 25 22:18:38 2004 +0000
> 
>     Try to glue the hazard avoidance stuff the same way it was done for
>     2.6 before the TLB synthesizer.  Previously the code for some CPUs
>     such as the RM9000 was completly bogus nonsense ...  I guess we may
>     eventually want to backport the tlb synthesizer to 2.4 once the dust
>     has settled.
> 
> And arch/mips64/mm/tlb-r4k.c was introduced with:
> 
> commit 49be6d9f37bbac0a0c413a2a63dcf7cc497d144a
> Author: Ralf Baechle <ralf@xxxxxxxxxxxxxx>
> Date:   Wed Jul 24 16:12:03 2002 +0000
> 
>     Random 64-bit updates and fixes.
> 
> and never had the condition in the first place.
> 
>  We do have full 2.6 GIT history, beyond 2.6.12.  It seems cut off at 
> 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2 for some operations, but not 
> plain `git log' for example, and I was able to peek beyond that by 
> checking out the immediately preceding commit, that is 
> 66f0a432564b5f0ebf632263ceac84a10a99de09:
> 
> commit 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2
> Author: Linus Torvalds <torvalds@xxxxxxxxxxxxxxx>
> Date:   Sat Apr 16 15:20:36 2005 -0700
> 
>     Linux-2.6.12-rc2
> 
>     Initial git repository build. I'm not bothering with the full history,
>     even though we have it. We can create a separate "historical" git
>     archive of that later if we want to, and in the meantime it's about
>     3.2GB when imported into git - space that would just make the early
>     git days unnecessarily complicated, when we don't have a lot of good
>     infrastructure for it.
> 
>     Let it rip!
> 
> commit 66f0a432564b5f0ebf632263ceac84a10a99de09
> Author: Ralf Baechle <ralf@xxxxxxxxxxxxxx>
> Date:   Fri Apr 15 14:40:46 2005 +0000
> 
>     Add resource managment.
> 
> There may be a better way, I don't claim GIT expertise.
> 
>   Maciej
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/