Re: [PATCH v2] powerpc/mm/cxl: Add barrier when setting mm cpumask
From: Andrew Donnellan <hidden>
Date: 2017-08-28 09:30:43
On 28/08/17 18:35, Aneesh Kumar K.V wrote:
We need to add memory barrier so that the page table walk doesn't happen
before the cpumask is set and made visible to the other cpus. We need
to use a sync here instead of lwsync because lwsync is not sufficient for
store/load ordering.
We also need to add an if (mm) check so that we do the right thing when called
with a kernel context. For kernel context, we have mm = NULL. W.r.t kernel
address we can skip setting the mm cpumask.
Fixes: 0f4bc0932e ("powerpc/mm/cxl: Add the fault handling cpu to mm cpumask")
Cc: Andrew Donnellan <redacted>
Reported-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Reported-by: Dan Carpenter <redacted>
Signed-off-by: Aneesh Kumar K.V <redacted>Acked-by: Andrew Donnellan <redacted>
quoted hunk ↗ jump to hunk
--- drivers/misc/cxl/fault.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-)diff --git a/drivers/misc/cxl/fault.c b/drivers/misc/cxl/fault.c index ab507e4ed69b..f17f72ea0545 100644 --- a/drivers/misc/cxl/fault.c +++ b/drivers/misc/cxl/fault.c@@ -141,9 +141,19 @@ int cxl_handle_mm_fault(struct mm_struct *mm, u64 dsisr, u64 dar) /* * Add the fault handling cpu to task mm cpumask so that we * can do a safe lockless page table walk when inserting the - * hash page table entry. + * hash page table entry. This function get called with a + * valid mm for user space addresses. Hence using the if (mm) + * check is sufficient here. */ - cpumask_set_cpu(smp_processor_id(), mm_cpumask(mm)); + if (mm && !cpumask_test_cpu(smp_processor_id(), mm_cpumask(mm))) { + cpumask_set_cpu(smp_processor_id(), mm_cpumask(mm)); + /* + * We need to make sure we walk the table only after + * we update the cpumask. The other side of the barrier + * is explained in serialize_against_pte_lookup() + */ + smp_mb(); + } if ((result = copro_handle_mm_fault(mm, dar, dsisr, &flt))) { pr_devel("copro_handle_mm_fault failed: %#x\n", result); return result;
-- Andrew Donnellan OzLabs, ADL Canberra andrew.donnellan@au1.ibm.com IBM Australia Limited