lkml.org 
[lkml]   [2013]   [Dec]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 2/3] read_lock migrate_disable pushdown to rt_read_lock
On Fri, 6 Dec 2013 03:33:34 +0100
Nicholas Mc Guire <der.herr@hofr.at> 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


\
 
 \ /
  Last update: 2013-12-06 17:01    [W:0.105 / U:0.492 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site