[PATCH v4] clk: allow reentrant calls into the clk framework

Mike Turquette mturquette at linaro.org
Wed Mar 27 23:00:38 EDT 2013


Quoting Russell King - ARM Linux (2013-03-27 15:56:10)
> On Wed, Mar 27, 2013 at 12:09:43AM -0700, Mike Turquette wrote:
> > +     /* set context for any reentrant calls */
> > +     atomic_set(&prepare_context, (int) get_current());
> ...
> > +     if (mutex_is_locked(&prepare_lock))
> > +             if ((void *) atomic_read(&prepare_context) == get_current())
> > +                     return true;
> 
> If you really want to do it like this, then the _correct_ way to do it
> is:
> 
>                 if (atomic_read(&prepare_context) == (int)get_current())
> 
> So that any effects from the cast are the same in both parts.  Otherwise,
> you will be running into the possibility that a cast could do something
> like truncate the returned value, resulting in the test condition using
> pointers always being false.
> 
> It's not that much of a problem on ARM, but it's a matter of good
> programming discpline that such issues are avoided.
> 
> Even better would be to cast it to unsigned long - this is the value
> which the atomic types use, and that is less likely to be truncated.
> It also helps to avoid the possibility of compilers complaining about
> a narrowing cast too - especially as the entire Linux kernel assumes
> that pointers can be cast to unsigned long and back again with no loss.

The atomics and casts will go away in the next version but the insights
are useful.  Thanks for the review.

Regards,
Mike



More information about the linux-arm-kernel mailing list