Hi Jani,
[...]
> > * upon acquiring the wakeref.
> > */
> > mutex_lock_nested(&wf->mutex, SINGLE_DEPTH_NESTING);
> > - if (!atomic_read(&wf->count)) {
> > - int err;
> >
> > - rpm_get(wf);
> > + if (likely(!atomic_read(&wf->count))) {
>
> Adding the likely should be a separate patch with rationale, not a
> random drive-by change. (And maybe it just should not be added at all.)
Agree, this can be made in a separate patch.
> > + INTEL_WAKEREF_BUG_ON(wf->wakeref);
> > + wf->wakeref = fetch_and_zero(&wakeref);
>
> fetch_and_zero() should just die. All it does here is make things more
> confusing, not less. Please don't add new users.
>
> The get and put helpers could probably stay, modified, to make this more
> readable.
it actually looks straight forward to me and even more
understandable. get/put are OK if there are multiple users, but
when used in such simple context it looks a bit of an overkill.
Especially when we don't need anymore the actions taken bu get
and put.
So that replacing the pointer with NULL is a natural process, no?
Andi
