Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Sender: Tejun Heo <htejun@gmail.com>
Date:   Sat, 2 Jul 2022 05:11:02 +0900
From:   Tejun Heo <tj@kernel.org>
To:     Imran Khan <imran.f.khan@oracle.com>
Cc:     gregkh@linuxfoundation.org, viro@zeniv.linux.org.uk,
        m.szyprowski@samsung.com, nathan@kernel.org, michael@walle.cc,
        robh@kernel.org, linux-serial@vger.kernel.org,
        linux-kernel@vger.kernel.org, guillaume.tucker@collabora.com,
        pmladek@suse.com
Subject: Re: [RESEND PATCH] kernfs: Avoid re-adding kernfs_node into
 kernfs_notify_list.
Message-ID: <Yr9U1q0BBinCgyrT@mtj.duckdns.org>
References: <20220701154604.2211008-1-imran.f.khan@oracle.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220701154604.2211008-1-imran.f.khan@oracle.com>
Precedence: bulk

Hello,

On Sat, Jul 02, 2022 at 01:46:04AM +1000, Imran Khan wrote:
> @@ -992,9 +993,11 @@ void kernfs_notify(struct kernfs_node *kn)
>  	rcu_read_unlock();
>  
>  	/* schedule work to kick fsnotify */
> -	kernfs_get(kn);
> -	llist_add(&kn->attr.notify_next, &kernfs_notify_list);
> -	schedule_work(&kernfs_notify_work);
> +	if (kn->attr.notify_next.next != NULL) {
> +		kernfs_get(kn);
> +		llist_add(&kn->attr.notify_next, &kernfs_notify_list);
> +		schedule_work(&kernfs_notify_work);
> +	}

Aren't you just narrowing the race window here? What prevents two
threads simultaneously testing for non NULL and then entering the
addition block together?

Looked at the llist code and it doesn't support multiple producers
trying to add the same node, unfortunately, so I'm not sure llist is
gonna work here. For now, the right thing to do prolly is reverting
it.

Al, is there something I'm missing about llist?

Thanks.

-- 
tejun