On Fri, 6 Dec 2013 03:33:34 +0100
Nicholas Mc Guire <[email protected]> wrote:
> > > - migrate_disable();
> > > if (rt_mutex_owner(lock) != current) {
> > > ret = rt_mutex_trylock(lock);
> > > - if (ret)
> > > + if (ret) {
> > > + migrate_disable();
> > > rwlock_acquire(&rwlock->dep_map, 0, 1, _RET_IP_);
> > > + }
> >
> > I like this patch in general, but I'm nervous about this part.
> >
> > What was the original reason to disable migration before checking the
> > mutex owner? Seems like we should have only disabled it on success. I'm
> > assuming there was some subtle reason not to.
> >
>
> I think that it was simply being treated semantically as a lock - but it
> is actually not.
> hold a
> migrate_disable/lock per_cpu unlock/migrate_enable
> lock/migrate_disable per_cpu migrate_enable/unlcok
> lock/migrate_disable per_cpu unlock/migrate_enable
> migrate_disable/lock per_cpu migrate_enable/unlcok
> reference
>
> are all equivalent - the only thing you need to ensure that the per cpu
> object will not be accessed before both lock and migration_disable have
> been sucessful. So from my understanding this is safe.
I think you may be right, but I'm still a little nervous about this
code. But that's good. We all should be nervous about any locking
code ;-)
>
> if we get migrated after a succesful trylock what would go wrong ?
> the protected object will not be accessed until after the spin_trylock
> returned so migration is disabled
I agree.
>
> > If there was some strange reason, I'm not convinced that your change
> > makes that reason go away.
> >
> IF there is a reason then this patch is broken - my conclusion up to now
> is that there is no such reason.
>
Let me analyze the original code first. I'll poke peterz and tglx too
to make sure this modification is OK.
Thanks,
-- Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
