| Message ID | 20170828083544.16514-1-aneesh.kumar@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 22259a6e800cdb8e06e65432fcd019983214be0c |
| Headers | show |
| Series | [v2] powerpc/mm/cxl: Add barrier when setting mm cpumask | expand |
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 <andrew.donnellan@au1.ibm.com> > Reported-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> > Reported-by: Dan Carpenter <dan.carpenter@oracle.com> > Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com> Acked-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> > --- > 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; >
On Mon, 2017-08-28 at 08:35:44 UTC, "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 <andrew.donnellan@au1.ibm.com> > Reported-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> > Reported-by: Dan Carpenter <dan.carpenter@oracle.com> > Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com> > Acked-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> Applied to powerpc next, thanks. https://git.kernel.org/powerpc/c/22259a6e800cdb8e06e65432fcd019 cheers
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;
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 <andrew.donnellan@au1.ibm.com> Reported-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> Reported-by: Dan Carpenter <dan.carpenter@oracle.com> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com> --- drivers/misc/cxl/fault.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-)