Re: [PATCH 07/17] mm/filemap: Change filemap_read_page calling conventions

[Christoph Hellwig <hch@lst.de>]

> +static int filemap_read_page(struct file *file, struct address_space *mapping,
> +		struct page *page)

I still think we should drop the mapping argument as well.

> +	if (iocb->ki_flags & (IOCB_NOIO | IOCB_NOWAIT | IOCB_WAITQ)) {
> +		unlock_page(page);
> +		put_page(page);
> +		return ERR_PTR(-EAGAIN);
> +	}
> +	error = filemap_read_page(iocb->ki_filp, mapping, page);
> +	if (!error)
> +		return page;

I think a goto error for the error cases would be much more useful.
That would allow to also share the error put_page for the flag check
above and the truncated case below, but most importantly make the code
flow obvious vs the early return for the success case.

> -	return filemap_read_page(iocb, filp, mapping, page);
> +	if (!error)
> +		error = filemap_read_page(iocb->ki_filp, mapping, page);
> +	if (!error)
> +		return page;
> +	put_page(page);
> +	if (error == -EEXIST || error == AOP_TRUNCATED_PAGE)
> +		return NULL;
> +	return ERR_PTR(error);

Same here.