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:   Mon, 16 May 2022 18:21:09 -1000
From:   Tejun Heo <tj@kernel.org>
To:     Imran Khan <imran.f.khan@oracle.com>
Cc:     viro@zeniv.linux.org.uk, gregkh@linuxfoundation.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/5] kernfs: make ->attr.open RCU protected.
Message-ID: <YoMitbmf5hIm7QRh@slm.duckdns.org>
References: <20220428055431.3826852-1-imran.f.khan@oracle.com>
 <20220428055431.3826852-3-imran.f.khan@oracle.com>
 <YnQtL7+GYHwpo4n2@slm.duckdns.org>
 <f5ff2554-580c-c817-b77f-25e2ef46411f@oracle.com>
 <Yn0/RrgMRie2YPEb@slm.duckdns.org>
 <526b46a7-7daa-9050-1276-e19836816991@oracle.com>
 <YoKna/Ty+k9O+pjt@slm.duckdns.org>
 <4532123f-4c03-0303-1480-a51faabca806@oracle.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <4532123f-4c03-0303-1480-a51faabca806@oracle.com>
Precedence: bulk

On Tue, May 17, 2022 at 12:30:26PM +1000, Imran Khan wrote:
> +/**
> + *     kernfs_deref_open_node - Get kernfs_open_node corresponding to @kn.
> + *
> + *     @kn: target kernfs_node.
> + *     @val: RCU dereference will take place only if val is true.
> + *
> + *     Fetch and return ->attr.open of @kn when current updater of ->attr.open
> + *     ensures that as long as @val is true, other updater(s) of ->attr.open
> + *     can not change it. The caller needs to pass value of the condition
> + *     (@val) that prevents value of ->attr.open from changing.
> + *
> + *     This should ONLY be used by updaters of ->attr.open.
> + */
> +static struct kernfs_open_node *
> +kernfs_deref_open_node(struct kernfs_node *kn, bool val)
> +{
> +       return rcu_dereference_protected(kn->attr.open, val);
> +}
> +
> +/**
> + *     kernfs_check_open_node - Get kernfs_open_node corresponding to @kn.
> + *
> + *     @kn: target kernfs_node.
> + *     @val: RCU dereference will take place only if val is true.
> + *
> + *     rcu_dereference and return ->attr.open of @kn. This is used in cases
> + *     where we can access ->attr.open outside RCU read-side critical section
> + *     as long as specified conditions are correct i.e @val is true.
> + *
> + *     This should ONLY be used by readers of ->attr.open.
> + */
> +static struct kernfs_open_node *
> +kernfs_check_open_node(struct kernfs_node *kn, bool val)
> +{
> +       return rcu_dereference_check(kn->attr.open, val);
> +}

You gotta put the conditions inside these wrappers. These wrappers are
supposed to describe as strict as reasonably possible so that the checks can
catch people making mistakes. It doesn't make much sense to defer the
condition to the callers. Just describe what the expected conditions are in
the wrappers as best as you reasonably can.

Thanks.

-- 
tejun