Re: [PATCH 4/6] kvm tools: Add rwlock wrapper

Paul E. McKenney
2011-05-29T15:31:35Z

On Sun, May 29, 2011 at 09:19:48AM +0200, Ingo Molnar wrote:
>
> * Avi Kivity <avi@redhat.com> wrote:
>
> > Yes, this is equivalent to the kernel's stop_machine_run(). It's a
> > heavyweight method but it should work just fine.
>
> Yeah. It is fine for reconfiguration/configuration-only kind of write
> paths - i.e. the mmio lookup path should be ok.
>
> There's only one thing i'd like to warn about: please still shape it
> as a br_read_lock()/unlock(), br_write_lock()/unlock() operation.
>
> This way all the SMP read paths remain identified properly, even if
> br_write_lock() does a stop_machine_run() equivalent. This still
> leaves options open for an easy conversion to rwlock or urcu (should
> any particular write path become more than just occasional).

This is very important even if no write path ever becomes more than
just occasional. If you don't mark the read paths like Ingo suggests,
your one-year-from-now self will be very annoyed at you, as the code
will be quite difficult to understand. And anyone else trying to
read your code will be even more annoyed. ;-)

Thanx, Paul