Re: [PATCH v2 1/4] fs: document mapping helpers

From: Christian Brauner <christian.brauner@ubuntu.com>
To: Matthew Wilcox <willy@infradead.org>
Cc: Christoph Hellwig <hch@lst.de>, Al Viro <viro@zeniv.linux.org.uk>,
	Vivek Goyal <vgoyal@redhat.com>,
	"Darrick J . Wong" <djwong@kernel.org>,
	linux-fsdevel@vger.kernel.org, linux-xfs@vger.kernel.org
Subject: Re: [PATCH v2 1/4] fs: document mapping helpers
Date: Mon, 22 Mar 2021 09:50:54 +0100	[thread overview]
Message-ID: <20210322085054.dicpuv2oub3lyzaf@wittgenstein> (raw)
In-Reply-To: <20210322073546.GH1719932@casper.infradead.org>

On Mon, Mar 22, 2021 at 07:35:46AM +0000, Matthew Wilcox wrote:
> On Sat, Mar 20, 2021 at 01:26:21PM +0100, Christian Brauner wrote:
> > +/**
> > + * kuid_into_mnt - map a kuid down into a mnt_userns
> > + * @mnt_userns: user namespace of the relevant mount
> > + * @kuid: kuid to be mapped
> > + *
> > + * Return @kuid mapped according to @mnt_userns.
> > + * If @kuid has no mapping INVALID_UID is returned.
> > + */
> 
> If you could just put the ':' after 'Return', htmldoc would put this into
> a nice section for you.

I'll fix that up in my tree. Thanks!

> 
> I also like to include a Context: section which lists whether the
> function takes locks / requires locks to be held / can be called in
> hard or soft interrupt context / may sleep / requires refcounts be held /
> ...  Generally, what do you expect from your callers, and what your callers
> can expect from you.

Thanks for the hint about "Context:". The functions don't take any
locks, they don't require locks to be held and they don't sleep and
don't manipulate refcounts (The lifetime of @mnt_userns is tied to the
respective mnt it's from. It can't be altered anymore once a non-initial
@mnt_userns has been attached to the mnt so it can't go away behind the
caller's back.). Internally only smp_rmb and smp_wmb are used and so
they should be fine to call from soft and hard irq.
Because of that it seemed not explicitly mentioning all that is more
correct then describing all of that. I always thought "Context:"
sections are "Here's things to keep in mind." less then "Here's how it
behaves in all those contexts.", i.e. point out pitfalls, not describe
regular behavior. I might be wrong though or it's a matter of
preference.

(Should we also aim for all other fs.h helpers to have similar comments?)

Christian