On Thu, 13 Nov 2014, Pranith Kumar wrote: > Recently lockless_dereference() was added which can be used in place of > hard-coding smp_read_barrier_depends(). The following PATCH makes the change. > > Signed-off-by: Pranith Kumar <bobby.prani@xxxxxxxxx> Sorry, I don't think your patch is buggy, but I do think it makes this tricky piece of code harder to follow, not easier. It is certainly not a standard use of lockless_dereference() (kpfn is not a pointer), and it both hides and moves where the barrier is. And then at the end of the function, there's still explicit barriers and ACCESS_ONCE comparison with kpfn, which this makes more obscure. Unless you are actually fixing a bug (I don't pretend to have tested this on Alpha, and I can get barriers wrong as we all do), or smp_read_barrier_depends() is about to be withdrawn from use, I'd rather say NAK to this patch. Hugh > --- > mm/ksm.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/mm/ksm.c b/mm/ksm.c > index d247efa..a67de79 100644 > --- a/mm/ksm.c > +++ b/mm/ksm.c > @@ -542,15 +542,14 @@ static struct page *get_ksm_page(struct stable_node *stable_node, bool lock_it) > expected_mapping = (void *)stable_node + > (PAGE_MAPPING_ANON | PAGE_MAPPING_KSM); > again: > - kpfn = ACCESS_ONCE(stable_node->kpfn); > - page = pfn_to_page(kpfn); > - > /* > * page is computed from kpfn, so on most architectures reading > * page->mapping is naturally ordered after reading node->kpfn, > * but on Alpha we need to be more careful. > */ > - smp_read_barrier_depends(); > + kpfn = lockless_dereference(stable_node->kpfn); > + page = pfn_to_page(kpfn); > + > if (ACCESS_ONCE(page->mapping) != expected_mapping) > goto stale; > > -- > 1.9.1 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>