On Fri, 14 Jun 2019 03:38 AM Andrew Morton <[email protected]> wrote:
>On Thu, 13 Jun 2019 22:07:44 +0800 Xiaoming Ni <[email protected]> wrote:
>
>> Registering the same notifier to a hook repeatedly can cause the hook
>> list to form a ring or lose other members of the list.
>> .....
>>
>> diff --git a/kernel/notifier.c b/kernel/notifier.c
>> index d9f5081..56efd54 100644
>> --- a/kernel/notifier.c
>> +++ b/kernel/notifier.c
>> @@ -19,20 +19,6 @@
>> * are layered on top of these, with appropriate locking added.
>> */
>>
>> -static int notifier_chain_register(struct notifier_block **nl,
>> - struct notifier_block *n)
>> -{
>> - while ((*nl) != NULL) {
>> - WARN_ONCE(((*nl) == n), "double register detected");
>> - if (n->priority > (*nl)->priority)
>> - break;
>> - nl = &((*nl)->next);
>> - }
>> - n->next = *nl;
>> - rcu_assign_pointer(*nl, n);
>> - return 0;
>> -}
>
>Registering an already-registered notifier is a bug (except for in
>net/sunrpc/rpc_pipe.c, apparently). The effect of this change is to
>remove the warning about the presence of the bug, so the bug is less
>likely to get fixed.
>
thanks for your guidance,
Should I modify this way
1 notifier_chain_cond_register() and notifier_chain_register() should be
combined into one function.
2 The warning information needs to be displayed while prohibiting duplicate
registration.
@@ -23,7 +23,10 @@ static int notifier_chain_register(struct
notifier_block **nl,
struct notifier_block *n)
{
while ((*nl) != NULL) {
- WARN_ONCE(((*nl) == n), "double register detected");
+ if (unlikely((*nl) == n)) {
+ WARN(1, "double register detected");
+ return 0;
+ }
if (n->priority >
(*nl)->priority)
break;
>I think it would be better to remove notifier_chain_cond_register() and
>blocking_notifier_chain_cond_register() and to figure out why
>net/sunrpc/rpc_pipe.c is using it and to redo the rpc code so it no
>longer has that need.
>
thanks for your guidance,
I re-examine the submission record and analyze it as follows
notifier_chain_cond_register() was introduced by commit 6546bc4279241e8fa43
("ipc: re-enable msgmni automatic recomputing msgmni if set to negative")
From the patch description information, it should be done to avoid repeated
registrations,
but I don't know why not directly modify notifier_chain_cond_register().
notifier_chain_cond_register() is only called by
blocking_notifier_chain_cond_register()
blocking_notifier_chain_cond_register() has less processing of the
SYSTEM_BOOTING state
than blocking_notifier_chain_egister().
may also be a bug.
ipc/ipcns_notifier.c and the call to blocking_notifier_chain_cond_register()
are removed
in commit 0050ee059f7fc86b1df252 ("ipc/msg: increase MSGMNI, remove scaling").
now blocking_notifier_chain_cond_register() is only used in
net/sunrpc/rpc_pipe.c,
commit 2d00131acc641b2cb6 ("SUNRPC: send notification events on pipefs sb
creation and destruction")
Using blocking_notifier_chain_cond_register() may also be to avoid duplicate
registrations??
thanks
