Hi Matt,
On Fri, Mar 04, 2016 at 01:43:37PM +0000, Matt Redfearn wrote:
> Hi James,
>
> On 04/03/16 10:10, James Hogan wrote:
> > When calculate_cpu_foreign_map() recalculates the cpu_foreign_map
> > cpumask it uses the local variable temp_foreign_map without initialising
> > it to zero. Since the calculation only ever sets bits in this cpumask
> > any existing bits at that memory location will remain set and find their
> > way into cpu_foreign_map too. This could potentially lead to cache
> > operations suboptimally doing smp calls to multiple VPEs in the same
> > core, even though the VPEs share primary caches.
> >
> > Therefore initialise temp_foreign_map using cpumask_clear() before use.
> >
> > Fixes: cccf34e9411c ("MIPS: c-r4k: Fix cache flushing for MT cores")
>
> cccf34e9411c was CC'd to stable 3.15+, should this fix do the same?
I originally didn't think it was needed for stable, since it only causes
a few unnecessary IPIs. However having thought some more about it, I
think it could result in cpu_foreign_map containing VPEs that are
offline, and not ones online, which could result in the IPI *not* being
called on a given core at all when it should.
E.g.
if the initial undefined value is 0x2 (CPU 1) and 2nd VPE of 1st core is
offline, then CPU 0 wouldn't be set (since CPU 1 is a sibling) and
neither VPE in core 0 would get the IPI.
So yeh, maybe it should.
Cheers
James
>
> Thanks,
> Matt
>
>
> > Signed-off-by: James Hogan <james.hogan@xxxxxxxxxx>
> > Cc: Ralf Baechle <ralf@xxxxxxxxxxxxxx>
> > Cc: Paul Burton <paul.burton@xxxxxxxxxx>
> > Cc: linux-mips@xxxxxxxxxxxxxx
> > ---
> > arch/mips/kernel/smp.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/arch/mips/kernel/smp.c b/arch/mips/kernel/smp.c
> > index bd4385a8e6e8..2b521e07b860 100644
> > --- a/arch/mips/kernel/smp.c
> > +++ b/arch/mips/kernel/smp.c
> > @@ -121,6 +121,7 @@ static inline void calculate_cpu_foreign_map(void)
> > cpumask_t temp_foreign_map;
> >
> > /* Re-calculate the mask */
> > + cpumask_clear(&temp_foreign_map);
> > for_each_online_cpu(i) {
> > core_present = 0;
> > for_each_cpu(k, &temp_foreign_map)
>
Attachment:
signature.asc
Description: Digital signature
