Re: [RFC PATCH 2/2] watch_queue: Fix pipe accounting

[Eric Sandeen <sandeen@sandeen.net>]

On 2/14/25 10:58 AM, Eric Sandeen wrote:
> On 2/14/25 10:49 AM, David Howells wrote:
>> Eric Sandeen <sandeen@redhat.com> wrote:
>>
>>> -	if (!pipe_has_watch_queue(pipe)) {
>>> -		pipe->max_usage = nr_slots;
>>> -		pipe->nr_accounted = nr_slots;
>>> -	}
>>> +	pipe->max_usage = nr_slots;
>>> +	pipe->nr_accounted = nr_slots;
>>
>> Hmmm...   The pipe ring is supposed to have some spare capacity when used as a
>> watch queue so that you can bung at least a few messages into it.
> 
> If the pipe has an original number of buffers on it, and
> then watch_queue_set_size adjusts that number - where does
> the spare capacity come from? Who adds it / where?

If that was resizing the ring to nr_notes not nr_pages, then ... I guess we
could keep the pipe_has_watch_queue() test as above, skip updating nr_accounted
there, and set it manually to the user-charged number in watch_queue_set_size()?

Something like this? I'm not sure about max_usage, maybe it should be set
as well.

diff --git a/kernel/watch_queue.c b/kernel/watch_queue.c
index 5267adeaa403..07a161ecc8a8 100644
--- a/kernel/watch_queue.c
+++ b/kernel/watch_queue.c
@@ -269,6 +269,14 @@ long watch_queue_set_size(struct pipe_inode_info *pipe, unsigned int nr_notes)
 	if (ret < 0)
 		goto error;
 
+	/*
+	 * pipe_resize_ring does not update nr_accounted for watch_queue pipes,
+	 * because the above vastly overprovisions. Set nr_accounted on
+	 * this pipe to the number that was charged to the user above
+	 * here manually.
+	 */
+	pipe->nr_accounted = nr_pages;
+
 	ret = -ENOMEM;
 	pages = kcalloc(nr_pages, sizeof(struct page *), GFP_KERNEL);
 	if (!pages)




> Thanks,
> -Eric
> 
>> David
>>
>>
> 
>