On Mon, Jun 13, 2022 at 11:28:34PM +0100, Al Viro wrote: > Dave, could you explain what's going on there? Note that pipe_write() > does *not* use that thing at all; it's only splice (i.e. ITER_PIPE > stuff) that is using it. > > What's wrong with > p_occupancy = pipe_occupancy(head, tail); > if (p_occupancy >= pipe->max_usage) > return 0; > else > return pipe->max_usage - p_occupancy; > > which would match the way you are using ->max_usage in pipe_write() > et.al. Including the use in copy_page_to_iter_pipe(), BTW...
The more I'm looking at that thing, the more it smells like a bug; it had the same 3 callers since the time it had been introduced. 1) pipe_get_pages(). We are about to try and allocate up to that many pipe buffers. Allocation (done in push_pipe()) is done only if we have !pipe_full(pipe->head, pipe->tail, pipe->max_usage). It simply won't give you more than max_usage - occupancy. Your function returns min(ring_size - occupancy, max_usage), which is always greater than or equal to that (ring_size >= max_usage). 2) pipe_get_pages_alloc(). Same story, same push_pipe() being called, same "we'll never get that much - it'll hit the limit first". 3) iov_iter_npages() in case of ITER_PIPE. Again, the value is bogus - it should not be greater than the amount of pages we would be able to write there. AFAICS, 6718b6f855a0 "pipe: Allow pipes to have kernel-reserved slots" broke it for cases when ring_size != max_usage...
