2022-09-21 09:45:45

by Siddh Raman Pant

[permalink] [raw]
Subject: [RESEND PATCH v2 0/2] watch_queue: Clean up some code

There is a dangling reference to pipe in a watch_queue after clearing it.
Thus, NULL that pointer while clearing.

This change renders wqueue->defunct superfluous, as the latter is only used
to check if watch_queue is cleared. With this change, the pipe is NULLed
while clearing, so we can just check if the pipe is NULL.

Extending comment for watch_queue->pipe in the definition of watch_queue
made the comment conventionally too long (it was already past 80 chars),
so I have changed the struct annotations to be kerneldoc-styled, so that
I can extend the comment mentioning that the pipe is NULL when watch_queue
is cleared. In the process, I have also hopefully improved documentation
by documenting things which weren't documented before.

Changes in v2:
- Merged the NULLing and removing defunct patches.
- Removed READ_ONCE barrier in lock_wqueue().
- Improved and fixed errors in struct docs.
- Better commit messages.

Siddh Raman Pant (2):
include/linux/watch_queue: Improve documentation
kernel/watch_queue: NULL the dangling *pipe, and use it for clear
check

include/linux/watch_queue.h | 100 ++++++++++++++++++++++++++----------
kernel/watch_queue.c | 12 ++---
2 files changed, 79 insertions(+), 33 deletions(-)

--
2.35.1



2022-09-21 10:44:11

by Siddh Raman Pant

[permalink] [raw]
Subject: [RESEND PATCH v2 1/2] include/linux/watch_queue: Improve documentation

Introduce kerneldoc-style comments, and document a couple of things
explicitly.

Signed-off-by: Siddh Raman Pant <[email protected]>
---
include/linux/watch_queue.h | 102 ++++++++++++++++++++++++++----------
1 file changed, 75 insertions(+), 27 deletions(-)

diff --git a/include/linux/watch_queue.h b/include/linux/watch_queue.h
index fc6bba20273b..7f8b1f15634b 100644
--- a/include/linux/watch_queue.h
+++ b/include/linux/watch_queue.h
@@ -18,57 +18,103 @@

struct cred;

+/**
+ * struct 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;
};

+/**
+ * struct watch_filter - Filter on watch
+ *
+ * @rcu: RCU head (in union with type_filter)
+ * @type_filter: Bitmask of accepted types (in union with rcu)
+ * @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[];
};

+/**
+ * struct watch_queue - General notification queue
+ *
+ * @rcu: RCU head
+ * @filter: Filter to use on watches
+ * @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: To serialize accesses and removes
+ * @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.
+/**
+ * struct watch - Representation of a watch on an object
+ *
+ * @rcu: RCU head (in union with info_id)
+ * @info_id: ID to be OR'd in to info field (in union with rcu)
+ * @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.
+/**
+ * struct watch_list - List of watches on an object
+ *
+ * @rcu: RCU head
+ * @watchers: List head
+ * @release_watch: Function to release watch
+ * @lock: To protect addition and removal of watches
*/
struct watch_list {
struct rcu_head rcu;
@@ -118,8 +164,10 @@ static inline void remove_watch_list(struct watch_list *wlist, u64 id)
}

/**
- * watch_sizeof - Calculate the information part of the size of a watch record,
- * given the structure size.
+ * watch_sizeof() - Calculate the information part of the size of a watch
+ * record, given the structure size.
+ *
+ * @STRUCT: The structure whose size is to be given
*/
#define watch_sizeof(STRUCT) (sizeof(STRUCT) << WATCH_INFO_LENGTH__SHIFT)

--
2.35.1