Linux-NFS Archive on lore.kernel.org
 help / color / Atom feed
From: Nixiaoming <nixiaoming@huawei.com>
To: Vasily Averin <vvs@virtuozzo.com>,
	"adobriyan@gmail.com" <adobriyan@gmail.com>,
	"akpm@linux-foundation.org" <akpm@linux-foundation.org>,
	"anna.schumaker@netapp.com" <anna.schumaker@netapp.com>,
	"arjan@linux.intel.com" <arjan@linux.intel.com>,
	"bfields@fieldses.org" <bfields@fieldses.org>,
	"chuck.lever@oracle.com" <chuck.lever@oracle.com>,
	"davem@davemloft.net" <davem@davemloft.net>,
	"gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
	"jlayton@kernel.org" <jlayton@kernel.org>,
	"luto@kernel.org" <luto@kernel.org>,
	"mingo@kernel.org" <mingo@kernel.org>,
	"Nadia.Derbey@bull.net" <Nadia.Derbey@bull.net>,
	"paulmck@linux.vnet.ibm.com" <paulmck@linux.vnet.ibm.com>,
	"semen.protsenko@linaro.org" <semen.protsenko@linaro.org>,
	"stable@kernel.org" <stable@kernel.org>,
	"stern@rowland.harvard.edu" <stern@rowland.harvard.edu>,
	"tglx@linutronix.de" <tglx@linutronix.de>,
	"torvalds@linux-foundation.org" <torvalds@linux-foundation.org>,
	"trond.myklebust@hammerspace.com"
	<trond.myklebust@hammerspace.com>,
	"viresh.kumar@linaro.org" <viresh.kumar@linaro.org>
Cc: "Huangjianhui (Alex)" <alex.huangjianhui@huawei.com>,
	Dailei <dylix.dailei@huawei.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-nfs@vger.kernel.org" <linux-nfs@vger.kernel.org>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>
Subject: RE: [PATCH v3 0/3] kernel/notifier.c: avoid duplicate registration
Date: Thu, 11 Jul 2019 01:55:55 +0000
Message-ID: <E490CD805F7529488761C40FD9D26EF12AC9D068@dggemm507-mbx.china.huawei.com> (raw)
In-Reply-To: <f628ff03-eb47-62f3-465b-fe4ed046b30c@virtuozzo.com>

On Wed, July 10, 2019 1:49 PM Vasily Averin wrote:
>On 7/10/19 6:09 AM, Xiaoming Ni 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.
>
>I think is not enough to _prevent_ 2nd register attempt,
>it's enough to detect just attempt and generate warning to mark host in bad state.
>

Duplicate registration is prevented in my patch, not just "mark host in bad state"

Duplicate registration is checked and exited in notifier_chain_cond_register()

Duplicate registration was checked in notifier_chain_register() but only 
the alarm was triggered without exiting. added by commit 831246570d34692e 
("kernel/notifier.c: double register detection")

My patch is like a combination of 831246570d34692e and notifier_chain_cond_register(),
 which triggers an alarm and exits when a duplicate registration is detected.

>Unexpected 2nd register of the same hook most likely will lead to 2nd unregister,
>and it can lead to host crash in any time: 
>you can unregister notifier on first attempt it can be too early, it can be still in use.
>on the other hand you can never call 2nd unregister at all.

Since the member was not added to the linked list at the time of the second registration, 
no linked list ring was formed. 
The member is released on the first unregistration and -ENOENT on the second unregistration.
After patching, the fault has been alleviated

It may be more helpful to return an error code when someone tries to register the same
notification program a second time.
But I noticed that notifier_chain_cond_register() returns 0 when duplicate registration 
is detected. At the same time, in all the existing export function comments of notify,
"Currently always returns zero"

I am a bit confused: which is better?

>
>Unfortunately I do not see any ways to handle such cases properly,
>and it seems for me your patches does not resolve this problem.
>
>Am I missed something probably?
> 
>> case1: An infinite loop in notifier_chain_register() can cause soft lockup
>>         atomic_notifier_chain_register(&test_notifier_list, &test1);
>>         atomic_notifier_chain_register(&test_notifier_list, &test1);
>>         atomic_notifier_chain_register(&test_notifier_list, &test2);

Thanks

Xiaoming Ni

  reply index

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-10  3:09 Xiaoming Ni
2019-07-10  5:49 ` Vasily Averin
2019-07-11  1:55   ` Nixiaoming [this message]
2019-07-11 13:57     ` Vasily Averin
2019-07-12 13:11       ` Xiaoming Ni
2019-07-12 14:07         ` gregkh
2019-07-14  2:45           ` Xiaoming Ni
2019-07-15  5:38             ` Vasily Averin
2019-07-16  2:00               ` Xiaoming Ni
2019-07-16 10:20                 ` Vasily Averin
2019-07-16 14:07                   ` Xiaoming Ni
2019-07-17 11:15                     ` Vasily Averin
2019-09-10  3:57                       ` Xiaoming Ni
2019-07-10  5:56 ` Greg KH
2019-07-11  1:32   ` Nixiaoming

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=E490CD805F7529488761C40FD9D26EF12AC9D068@dggemm507-mbx.china.huawei.com \
    --to=nixiaoming@huawei.com \
    --cc=Nadia.Derbey@bull.net \
    --cc=adobriyan@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=alex.huangjianhui@huawei.com \
    --cc=anna.schumaker@netapp.com \
    --cc=arjan@linux.intel.com \
    --cc=bfields@fieldses.org \
    --cc=chuck.lever@oracle.com \
    --cc=davem@davemloft.net \
    --cc=dylix.dailei@huawei.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jlayton@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=mingo@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=semen.protsenko@linaro.org \
    --cc=stable@kernel.org \
    --cc=stern@rowland.harvard.edu \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    --cc=trond.myklebust@hammerspace.com \
    --cc=viresh.kumar@linaro.org \
    --cc=vvs@virtuozzo.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-NFS Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-nfs/0 linux-nfs/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-nfs linux-nfs/ https://lore.kernel.org/linux-nfs \
		linux-nfs@vger.kernel.org
	public-inbox-index linux-nfs

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-nfs


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git