Re: [PATCH] pipe: cache 2 pages instead of 1

[Oleg Nesterov <oleg@xxxxxxxxxx>]

I already had a lot of beer, can't read this patch, just one nit...

On 02/27, Mateusz Guzik wrote:
>
> User data is kept in a circular buffer backed by pages allocated as
> needed. Only having space for one spare is still prone to having to
> resort to allocation / freeing.
>
> In my testing this decreases page allocs by 60% during a -j 20 kernel
> build.

So this is performance improvement?

> +static struct page *anon_pipe_get_page(struct pipe_inode_info *pipe)
> +{
> +	struct page *page;
> +
> +	if (pipe->tmp_page[0]) {
> +		page = pipe->tmp_page[0];
> +		pipe->tmp_page[0] = NULL;
> +	} else if (pipe->tmp_page[1]) {
> +		page = pipe->tmp_page[1];
> +		pipe->tmp_page[1] = NULL;
> +	} else {
> +		page = alloc_page(GFP_HIGHUSER | __GFP_ACCOUNT);
> +	}
> +
> +	return page;
> +}

Perhaps something like

	for (i = 0; i < ARRAY_SIZE(pipe->tmp_page); i++) {
		if (pipe->tmp_page[i]) {
			struct page *page = pipe->tmp_page[i];
			pipe->tmp_page[i] = NULL;
			return page;
		}
	}

	return alloc_page(GFP_HIGHUSER | __GFP_ACCOUNT);
?

Same for anon_pipe_put_page() and free_pipe_info().

This avoids the code duplication and allows to change the size of
pipe->tmp_page[] array without other changes.

Oleg.