diff mbox series

[v2,1/4] fs: document mapping helpers

Message ID 20210320122623.599086-2-christian.brauner@ubuntu.com (mailing list archive)
State New
Headers show
Series tweak fs mapping helpers | expand

Commit Message

Christian Brauner March 20, 2021, 12:26 p.m. UTC
Document new helpers we introduced this cycle.

Suggested-by: Christoph Hellwig <hch@lst.de>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: linux-fsdevel@vger.kernel.org
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
/* v2 */
patch introduced
- Christoph Hellwig <hch@lst.de>:
  - Add kernel docs to helpers.
---
 include/linux/fs.h | 48 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)

Comments

Christoph Hellwig March 22, 2021, 7:03 a.m. UTC | #1
Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>
Matthew Wilcox March 22, 2021, 7:35 a.m. UTC | #2
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 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.

I don't understand the thing you're documenting, so it may not make sense
to talk about interrupt context, for example.
Christian Brauner March 22, 2021, 8:50 a.m. UTC | #3
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
diff mbox series

Patch

diff --git a/include/linux/fs.h b/include/linux/fs.h
index ec8f3ddf4a6a..4795a632ef0d 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1574,36 +1574,84 @@  static inline void i_gid_write(struct inode *inode, gid_t gid)
 	inode->i_gid = make_kgid(inode->i_sb->s_user_ns, gid);
 }
 
+/**
+ * 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.
+ */
 static inline kuid_t kuid_into_mnt(struct user_namespace *mnt_userns,
 				   kuid_t kuid)
 {
 	return make_kuid(mnt_userns, __kuid_val(kuid));
 }
 
+/**
+ * kgid_into_mnt - map a kgid down into a mnt_userns
+ * @mnt_userns: user namespace of the relevant mount
+ * @kgid: kgid to be mapped
+ *
+ * Return @kgid mapped according to @mnt_userns.
+ * If @kgid has no mapping INVALID_GID is returned.
+ */
 static inline kgid_t kgid_into_mnt(struct user_namespace *mnt_userns,
 				   kgid_t kgid)
 {
 	return make_kgid(mnt_userns, __kgid_val(kgid));
 }
 
+/**
+ * i_uid_into_mnt - map an inode's i_uid down into a mnt_userns
+ * @mnt_userns: user namespace of the mount the inode was found from
+ * @inode: inode to map
+ *
+ * Return the inode's i_uid mapped down according to @mnt_userns.
+ * If the inode's i_uid has no mapping INVALID_UID is returned.
+ */
 static inline kuid_t i_uid_into_mnt(struct user_namespace *mnt_userns,
 				    const struct inode *inode)
 {
 	return kuid_into_mnt(mnt_userns, inode->i_uid);
 }
 
+/**
+ * i_gid_into_mnt - map an inode's i_gid down into a mnt_userns
+ * @mnt_userns: user namespace of the mount the inode was found from
+ * @inode: inode to map
+ *
+ * Return the inode's i_gid mapped down according to @mnt_userns.
+ * If the inode's i_gid has no mapping INVALID_GID is returned.
+ */
 static inline kgid_t i_gid_into_mnt(struct user_namespace *mnt_userns,
 				    const struct inode *inode)
 {
 	return kgid_into_mnt(mnt_userns, inode->i_gid);
 }
 
+/**
+ * kuid_from_mnt - map a kuid up into a mnt_userns
+ * @mnt_userns: user namespace of the relevant mount
+ * @kuid: kuid to be mapped
+ *
+ * Return @kuid mapped up according to @mnt_userns.
+ * If @kuid has no mapping INVALID_UID is returned.
+ */
 static inline kuid_t kuid_from_mnt(struct user_namespace *mnt_userns,
 				   kuid_t kuid)
 {
 	return KUIDT_INIT(from_kuid(mnt_userns, kuid));
 }
 
+/**
+ * kgid_from_mnt - map a kgid up into a mnt_userns
+ * @mnt_userns: user namespace of the relevant mount
+ * @kgid: kgid to be mapped
+ *
+ * Return @kgid mapped up according to @mnt_userns.
+ * If @kgid has no mapping INVALID_GID is returned.
+ */
 static inline kgid_t kgid_from_mnt(struct user_namespace *mnt_userns,
 				   kgid_t kgid)
 {