[dm-devel] [PATCH v2 1/7] libmultipath: fix tur checker locking

[Martin Wilck]

On Mon, 2018-02-12 at 12:44 -0600, Benjamin Marzinski wrote:
> On Sat, Feb 10, 2018 at 08:42:31PM +0100, Martin Wilck wrote:
> > On Sat, 2018-02-10 at 17:11 +0100, Martin Wilck wrote:
> > > On Fri, 2018-02-09 at 18:36 -0600, Benjamin Marzinski wrote:
> > > > On Sat, Feb 10, 2018 at 12:36:05AM +0100, Martin Wilck wrote:
> > > > > On Sat, 2018-02-10 at 00:28 +0100, Martin Wilck wrote:
> > > > > > Maybe it's easier than we thought. Attached is a patch on
> > > > > > top
> > > > > > of
> > > > > > yours that I think might work, please have a look. 
> > > > > > 
> > > > > 
> > > > > That one didn't even compile. This one is better.
> > > > > 
> > > > > Martin
> > > > 
> > > > How about this one instead. The idea is that once we are in the
> > > > cleanup
> > > > handler, we just cleanup and exit. But before we enter it, we
> > > > atomically
> > > > exchange running, and if running was 0, we pause(), since the
> > > > checker
> > > > is
> > > > either about to cancel us, or already has.
> > > > 
> > > 
> > > Yes, that should work. Nice.
> > 
> > ... but I just realized that we don't rcu_register_thread() the TUR
> > thread. Maybe we should if we use RCU primitives?
> 
> Looking online, I see this.
> 
> "void rcu_register_thread(void): For all RCU flavors other than
> bullet-proof RCU, each thread must invoke this function before its
> first
> call to rcu_read_lock() or call_rcu(). If a given thread never
> invokes
> any RCU read-side functions, it need not invoke
> rcu_register_thread(void)."
> 
> That makes it sound like this is unnecessary for using the atomic
> operations. Did you see something that makes you think differently? I
> probably won't hurt to add it anyway. But, if it's fuzzy how to
> correctly use the rcu operations, we could just go back to wrapping
> normal operations in a spin_lock, now that we have a method that's
> not
> excessively complicated, and would only require holding the lock for
> simple variable manipulations.

OK, it's not strictly necessary then. Thanks for checking.
I wouldn't prefer going back to spinlocks. I think the atomic
operations are a step in the right direction.

Martin

-- 
Dr. Martin Wilck <mwilck at suse.com>, Tel. +49 (0)911 74053 2107
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)