[PATCH 2/3] kernel/watch_queue: Improve struct annotation formatting

Siddh Raman Pant posted 3 patches 3 years, 8 months ago
[PATCH 2/3] kernel/watch_queue: Improve struct annotation formatting
Posted by Siddh Raman Pant 3 years, 8 months ago
Improve formatting struct annotations in watch_queue.h, so that they
fall in the preferred 80 character limit.

Signed-off-by: Siddh Raman Pant <code@siddh.me>
---
 include/linux/watch_queue.h | 96 +++++++++++++++++++++++++++----------
 1 file changed, 71 insertions(+), 25 deletions(-)

diff --git a/include/linux/watch_queue.h b/include/linux/watch_queue.h
index fc6bba20273b..c99c39ec6548 100644
--- a/include/linux/watch_queue.h
+++ b/include/linux/watch_queue.h
@@ -18,57 +18,103 @@
 
 struct cred;
 
+/**
+ * watch_type_filter - Filter on watch type
+ *
+ * @type: Type of watch_notification
+ * @subtype_filter: Bitmask of subtypes to filter on
+ * @info_filter: Filter on watch_notification::info
+ * @info_mask: Mask of relevant bits in info_filter
+ */
 struct watch_type_filter {
 	enum watch_notification_type type;
-	__u32		subtype_filter[1];	/* Bitmask of subtypes to filter on */
-	__u32		info_filter;		/* Filter on watch_notification::info */
-	__u32		info_mask;		/* Mask of relevant bits in info_filter */
+	__u32		subtype_filter[1];
+	__u32		info_filter;
+	__u32		info_mask;
 };
 
+/**
+ * watch_filter - Filter on watch
+ *
+ * @rcu: (union) RCU head
+ * @type_filter: (union) Bitmask of accepted types
+ * @nr_filters: Number of filters
+ * @filters: Array of watch_type_filter
+ */
 struct watch_filter {
 	union {
 		struct rcu_head	rcu;
-		/* Bitmask of accepted types */
 		DECLARE_BITMAP(type_filter, WATCH_TYPE__NR);
 	};
-	u32			nr_filters;	/* Number of filters */
+	u32			 nr_filters;
 	struct watch_type_filter filters[];
 };
 
+/**
+ * watch_queue - General notification queue
+ *
+ * @rcu: RCU head
+ * @filter: Filter on watch_notification::info
+ * @pipe: The pipe we're using as a buffer.
+ * @watches: Contributory watches
+ * @notes: Preallocated notifications
+ * @notes_bitmap: Allocation bitmap for notes
+ * @usage: Object usage count
+ * @lock: Spinlock
+ * @nr_notes: Number of notes
+ * @nr_pages: Number of pages in notes[]
+ * @defunct: True when queues closed
+ */
 struct watch_queue {
 	struct rcu_head		rcu;
 	struct watch_filter __rcu *filter;
-	struct pipe_inode_info	*pipe;		/* The pipe we're using as a buffer */
-	struct hlist_head	watches;	/* Contributory watches */
-	struct page		**notes;	/* Preallocated notifications */
-	unsigned long		*notes_bitmap;	/* Allocation bitmap for notes */
-	struct kref		usage;		/* Object usage count */
+	struct pipe_inode_info	*pipe;
+	struct hlist_head	watches;
+	struct page		**notes;
+	unsigned long		*notes_bitmap;
+	struct kref		usage;
 	spinlock_t		lock;
-	unsigned int		nr_notes;	/* Number of notes */
-	unsigned int		nr_pages;	/* Number of pages in notes[] */
-	bool			defunct;	/* T when queues closed */
+	unsigned int		nr_notes;
+	unsigned int		nr_pages;
+	bool			defunct;
 };
 
-/*
- * Representation of a watch on an object.
+/**
+ * watch - Representation of a watch on an object.
+ *
+ * @rcu: (union) RCU head
+ * @info_id: (union) ID to be OR'd in to info field
+ * @queue: Queue to post events to
+ * @queue_node: Link in queue->watches
+ * @watch_list: Link in watch_list->watchers
+ * @list_node: The list node
+ * @cred: Creds of the owner of the watch
+ * @private: Private data for the watched object
+ * @id: Internal identifier
+ * @usage: Object usage count
  */
 struct watch {
 	union {
 		struct rcu_head	rcu;
-		u32		info_id;	/* ID to be OR'd in to info field */
+		u32		info_id;
 	};
-	struct watch_queue __rcu *queue;	/* Queue to post events to */
-	struct hlist_node	queue_node;	/* Link in queue->watches */
+	struct watch_queue __rcu *queue;
+	struct hlist_node	queue_node;
 	struct watch_list __rcu	*watch_list;
-	struct hlist_node	list_node;	/* Link in watch_list->watchers */
-	const struct cred	*cred;		/* Creds of the owner of the watch */
-	void			*private;	/* Private data for the watched object */
-	u64			id;		/* Internal identifier */
-	struct kref		usage;		/* Object usage count */
+	struct hlist_node	list_node;
+	const struct cred	*cred;
+	void			*private;
+	u64			id;
+	struct kref		usage;
 };
 
-/*
- * List of watches on an object.
+/**
+ * watch_list - List of watches on an object.
+ *
+ * @rcu: RCU head
+ * @watchers: List head
+ * @release_watch: Function to release watch
+ * @lock: Spinlock
  */
 struct watch_list {
 	struct rcu_head		rcu;
-- 
2.35.1
Re: [PATCH 2/3] kernel/watch_queue: Improve struct annotation formatting
Posted by Eric Biggers 3 years, 8 months ago
On Thu, Aug 04, 2022 at 07:00:23PM +0530, Siddh Raman Pant wrote:
> Improve formatting struct annotations in watch_queue.h, so that they
> fall in the preferred 80 character limit.
> 
> Signed-off-by: Siddh Raman Pant <code@siddh.me>

This patch isn't just fixing overly long lines, but rather is introducing
kerneldoc comments and documenting things that weren't documented before.
That's fine, but please make the commit message accurately describe the patch.

> diff --git a/include/linux/watch_queue.h b/include/linux/watch_queue.h
> index fc6bba20273b..c99c39ec6548 100644
> --- a/include/linux/watch_queue.h
> +++ b/include/linux/watch_queue.h
> @@ -18,57 +18,103 @@
>  
>  struct cred;
>  
> +/**
> + * watch_type_filter - Filter on watch type

If you're going to use kerneldoc comments, they should be correctly formatted.
This is not, since it's missing the word struct.  You can run this command to
see the kerneldoc warnings:

	./scripts/kernel-doc -v -none include/linux/watch_queue.h

> + * @lock: Spinlock

Please make sure that comments provide useful information and don't just repeat
what the code says.

- Eric
Re: [PATCH 2/3] kernel/watch_queue: Improve struct annotation formatting
Posted by Siddh Raman Pant 3 years, 8 months ago
On Fri, 05 Aug 2022 12:52:11 +0530  Eric Biggers  wrote:
> On Thu, Aug 04, 2022 at 07:00:23PM +0530, Siddh Raman Pant wrote:
> > Improve formatting struct annotations in watch_queue.h, so that they
> > fall in the preferred 80 character limit.
> > 
> > Signed-off-by: Siddh Raman Pant code@siddh.me>
> 
> This patch isn't just fixing overly long lines, but rather is introducing
> kerneldoc comments and documenting things that weren't documented before.
> That's fine, but please make the commit message accurately describe the patch.
> 
> > diff --git a/include/linux/watch_queue.h b/include/linux/watch_queue.h
> > index fc6bba20273b..c99c39ec6548 100644
> > --- a/include/linux/watch_queue.h
> > +++ b/include/linux/watch_queue.h
> > @@ -18,57 +18,103 @@
> >  
> >  struct cred;
> >  
> > +/**
> > + * watch_type_filter - Filter on watch type
> 
> If you're going to use kerneldoc comments, they should be correctly formatted.
> This is not, since it's missing the word struct.  You can run this command to
> see the kerneldoc warnings:
> 
> 	./scripts/kernel-doc -v -none include/linux/watch_queue.h
> 
> > + * @lock: Spinlock
> 
> Please make sure that comments provide useful information and don't just repeat
> what the code says.
> 
> - Eric
> 

Okay, will do.

Thanks,
Siddh