| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Mon, 28 Dec 2009 09:36:06 +0900
From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>
To: Peter Zijlstra <peterz@...radead.org>
Cc: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-mm@...ck.org" <linux-mm@...ck.org>,
"minchan.kim@...il.com" <minchan.kim@...il.com>,
cl@...ux-foundation.org
Subject: Re: [RFC PATCH] asynchronous page fault.
On Sun, 27 Dec 2009 13:03:11 +0100
Peter Zijlstra <peterz@...radead.org> wrote:
> On Fri, 2009-12-25 at 10:51 +0900, KAMEZAWA Hiroyuki wrote:
> > /*
> > + * Returns vma which contains given address. This scans rb-tree in speculative
> > + * way and increment a reference count if found. Even if vma exists in rb-tree,
> > + * this function may return NULL in racy case. So, this function cannot be used
> > + * for checking whether given address is valid or not.
> > + */
> > +struct vm_area_struct *
> > +find_vma_speculative(struct mm_struct *mm, unsigned long addr)
> > +{
> > + struct vm_area_struct *vma = NULL;
> > + struct vm_area_struct *vma_tmp;
> > + struct rb_node *rb_node;
> > +
> > + if (unlikely(!mm))
> > + return NULL;;
> > +
> > + rcu_read_lock();
> > + rb_node = rcu_dereference(mm->mm_rb.rb_node);
> > + vma = NULL;
> > + while (rb_node) {
> > + vma_tmp = rb_entry(rb_node, struct vm_area_struct, vm_rb);
> > +
> > + if (vma_tmp->vm_end > addr) {
> > + vma = vma_tmp;
> > + if (vma_tmp->vm_start <= addr)
> > + break;
> > + rb_node = rcu_dereference(rb_node->rb_left);
> > + } else
> > + rb_node = rcu_dereference(rb_node->rb_right);
> > + }
> > + if (vma) {
> > + if ((vma->vm_start <= addr) && (addr < vma->vm_end)) {
> > + if (!atomic_inc_not_zero(&vma->refcnt))
>
> And here you destroy pretty much all advantage of having done the
> lockless lookup ;-)
>
Hmm ? for single-thread apps ? This patch's purpose is not for lockless
lookup, it's just a part of work. My purpose is avoiding false-sharing.
2.6.33-rc2's score of the same test program is here.
75.42% multi-fault-all [kernel] [k] _raw_spin_lock_irqsav
|
--- _raw_spin_lock_irqsave
|
|--49.13%-- __down_read_trylock
| down_read_trylock
| do_page_fault
| page_fault
| 0x400950
| |
| --100.00%-- (nil)
|
|--46.92%-- __up_read
| up_read
| |
| |--99.99%-- do_page_fault
| | page_fault
| | 0x400950
| | (nil)
| --0.01%-- [...]
Most of time is used for up/down read.
Here is a comparison between
- page fault by 8 threads on one vma
- page fault by 8 threads on 8 vma on x86-64.
== one vma ==
# Samples: 1338964273489
#
# Overhead Command Shared Object Symbol
# ........ ............... ........................ ......
#
26.90% multi-fault-all [kernel] [k] clear_page_c
|
--- clear_page_c
__alloc_pages_nodemask
handle_mm_fault
do_page_fault
page_fault
0x400940
|
--100.00%-- (nil)
20.65% multi-fault-all [kernel] [k] _raw_spin_lock
|
--- _raw_spin_lock
|
|--85.07%-- free_pcppages_bulk
| free_hot_cold_page
....<snip>
3.94% multi-fault-all [kernel] [k] find_vma_speculative
|
--- find_vma_speculative
|
|--99.40%-- do_page_fault
| page_fault
| 0x400940
| |
| --100.00%-- (nil)
|
--0.60%-- page_fault
0x400940
|
--100.00%-- (nil)
==
== 8 vma ==
27.98% multi-fault-all [kernel] [k] clear_page_c
|
--- clear_page_c
__alloc_pages_nodemask
handle_mm_fault
do_page_fault
page_fault
0x400950
|
--100.00%-- (nil)
21.91% multi-fault-all [kernel] [k] _raw_spin_lock
|
--- _raw_spin_lock
|
|--77.01%-- free_pcppages_bulk
| free_hot_cold_page
| __pagevec_free
| release_pages
...<snip>
0.21% multi-fault-all [kernel] [k] find_vma_speculative
|
--- find_vma_speculative
|
|--87.50%-- do_page_fault
| page_fault
| 0x400950
| |
| --100.00%-- (nil)
|
--12.50%-- page_fault
0x400950
|
--100.00%-- (nil)
==
Yes, this atomic_inc_unless adds some overhead. But this isn't as bad as
false sharing in mmap_sem. Anyway, as Minchan pointed out, this code contains
bug. I consider this part again.
> The idea is to let the RCU lock span whatever length you need the vma
> for, the easy way is to simply use PREEMPT_RCU=y for now,
I tried to remove his kind of reference count trick but I can't do that
without synchronize_rcu() somewhere in unmap code. I don't like that and
use this refcnt.
> the hard way
> is to also incorporate the drop-mmap_sem on blocking patches from a
> while ago.
>
"drop-mmap_sem if block" is no help for this false-sharing problem.
Thanks,
-Kame
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists