Re: [PATCH v4 03/25] notifier: Add atomic/blocking_notifier_has_unique_priority()

Dmitry Osipenko
2021-12-10T18:52:09Z

10.12.2021 21:19, Rafael J. Wysocki пишет:
...
>> +bool atomic_notifier_has_unique_priority(struct atomic_notifier_head *nh,
>> + struct notifier_block *n)
>> +{
>> + unsigned long flags;
>> + bool ret;
>> +
>> + spin_lock_irqsave(&nh->lock, flags);
>> + ret = notifier_has_unique_priority(&nh->head, n);
>> + spin_unlock_irqrestore(&nh->lock, flags);
>
> This only works if the caller can prevent new entries from being added
> to the list at this point or if the caller knows that they cannot be
> added for some reason, but the kerneldoc doesn't mention this
> limitation.

I'll update the comment.

..
>> +bool blocking_notifier_has_unique_priority(struct blocking_notifier_head *nh,
>> + struct notifier_block *n)
>> +{
>> + bool ret;
>> +
>> + /*
>> + * This code gets used during boot-up, when task switching is
>> + * not yet working and interrupts must remain disabled. At such
>> + * times we must not call down_read().
>> + */
>> + if (system_state != SYSTEM_BOOTING)
>
> No, please don't do this, it makes the whole thing error-prone.

What should I do then?

>> + down_read(&nh->rwsem);
>> +
>> + ret = notifier_has_unique_priority(&nh->head, n);
>> +
>> + if (system_state != SYSTEM_BOOTING)
>> + up_read(&nh->rwsem);
>
> And still what if a new entry with a non-unique priority is added to
> the chain at this point?

If entry with a non-unique priority is added after the check, then
obviously it won't be detected. I don't understand the question. These
down/up_read() are the locks that prevent the race, if that's the question.