Re: [PATCH] [RFC] iomap: zeroing needs to be pagecache aware

[Christoph Hellwig <hch@xxxxxxxxxxxxx>]

On Thu, Dec 01, 2022 at 11:52:14AM +1100, Dave Chinner wrote:
>  	folio = __filemap_get_folio(iter->inode->i_mapping, pos >> PAGE_SHIFT,
>  			fgp, mapping_gfp_mask(iter->inode->i_mapping));
>  	if (!folio) {
> -		status = (iter->flags & IOMAP_NOWAIT) ? -EAGAIN : -ENOMEM;
> +		if (!(fgp & FGP_CREAT))
> +			status = -ENODATA;
> +		else if (iter->flags & IOMAP_NOWAIT)
> +			status = -EAGAIN;
> +		else
> +			status = -ENOMEM;

Trying to reverse engineer the error case based on the flags passed
seems a bit problematic, even if I think for now it mostl still
works as !FGP_CREAT is exclusive vs IOMAP_NOWAIT.

Matthew, what do you think of switching __filemap_get_folio to an
ERR_PTR return and return actual cause?  Or do we have too many
callers and need ____filemap_get_folio (urrg..)?

> +		status = iomap_write_begin(iter, pos, bytes, &folio, fgp);
> +		if (status) {
> +			if (status == -ENODATA) {
> +				/*
> +				 * No folio was found, so skip to the start of
> +				 * the next potential entry in the page cache
> +				 * and continue from there.
> +				 */
> +				if (bytes > PAGE_SIZE - offset_in_page(pos))
> +					bytes = PAGE_SIZE - offset_in_page(pos);
> +				goto loop_continue;
> +			}
>  			return status;
> +		}

Nit: I'd check for -ENODATA one level of identation out to keep
the nesting limited:

		status = iomap_write_begin(iter, pos, bytes, &folio, fgp);
		if (status == -ENODATA) {
			...
			goto loop_continue;
		}
		if (status)
 			return status;