| Message ID | 20210320122623.599086-2-christian.brauner@ubuntu.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | tweak fs mapping helpers | expand |
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
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.
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 --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) {
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(+)