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:   Tue, 14 Jun 2022 06:20:21 -1000
From:   Tejun Heo <tj@kernel.org>
To:     Imran Khan <imran.f.khan@oracle.com>
Cc:     gregkh@linuxfoundation.org, viro@zeniv.linux.org.uk,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v6 1/4] kernfs: make ->attr.open RCU protected.
Message-ID: <Yqi1RVRK2XahPWlg@slm.duckdns.org>
References: <20220614070346.810168-1-imran.f.khan@oracle.com>
 <20220614070346.810168-2-imran.f.khan@oracle.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220614070346.810168-2-imran.f.khan@oracle.com>
Precedence: bulk

On Tue, Jun 14, 2022 at 05:03:43PM +1000, Imran Khan wrote:
> +/**
> + * kernfs_deref_open_node_protected - Get kernfs_open_node corresponding to @kn
> + *
> + * @kn: target kernfs_node.
> + *
> + * Fetch and return ->attr.open of @kn when caller holds the
> + * kernfs_open_file_mutex.
> + *
> + * Update of ->attr.open happens under kernfs_open_file_mutex. So when
> + * the caller guarantees that this mutex is being held, other updaters can't
> + * change ->attr.open and this means that we can safely deref ->attr.open
> + * outside RCU read-side critical section.
> + *
> + * The caller needs to make sure that kernfs_open_file_mutex is held.
> + */
> +static struct kernfs_open_node *
> +kernfs_deref_open_node_protected(struct kernfs_node *kn)
> +{
> +	return rcu_dereference_check(kn->attr.open,
> +				lockdep_is_held(&kernfs_open_file_mutex));

Hey, so, the difference between rcu_dereference_check() and
rcu_dereference_protected() is that the former can be called either with rcu
read locked or under the extra condition (here, open_file_mutex held) while
the latter can't be used under rcu read lock. The two can generate different
codes too - the former enforces dependency ordering which makes accesses
under rcu read lock safe, while the latter doesn't.

In the above, you're saying that the accessor is only to be used while
holding kernfs_open_file_mutex but then using rcu_dereference_check() which
is odd. There are two ways you can go 1. ensure that the accessor is always
used under the mutex and use rcu_dereference_protected() or 2. if the
function can be used under rcu read lock, rename so that the differentiation
between the two accessors is based on the parameter type, not whether
they're protected or not.

Can you please post the updated patch as a reply to this one? No need to
post the whole thing over and over again.

Thanks.

-- 
tejun