Re: [PATCH 2/3] ceph: Uninline the data on a file opened for writing

[Matthew Wilcox <willy@xxxxxxxxxxxxx>]

On Mon, Jan 17, 2022 at 04:26:36PM +0000, David Howells wrote:
> +	folio = read_mapping_folio(inode->i_mapping, 0, file);
> +	if (IS_ERR(folio))
> +		goto out;

... you need to set 'err' here, right?

> +	if (folio_test_uptodate(folio))
> +		goto out_put_folio;

Er ... if (!folio_test_uptodate(folio)), perhaps?  And is it even
worth testing if read_mapping_folio() returned success?  I feel like
we should take ->readpage()'s word for it that success means the
folio is now uptodate.

> +	err = folio_lock_killable(folio);
> +	if (err < 0)
> +		goto out_put_folio;
> +
> +	if (inline_version == 1 || /* initial version, no data */
> +	    inline_version == CEPH_INLINE_NONE)
> +		goto out_unlock;
> +
> +	len = i_size_read(inode);
> +	if (len >  folio_size(folio))

extra space.  Plus, you're hardcoding 4096 below, but using folio_size()
here which is a bit weird to me.