On 2/14/25 10:58 AM, Eric Sandeen wrote:
> On 2/14/25 10:49 AM, David Howells wrote:
>> Eric Sandeen <sandeen@xxxxxxxxxx> 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
>>
>>
>
>
