Re: [PATCH v2 2/3] fs/ufs: Change the signature of ufs_get_page()

[Al Viro]

On Tue, Dec 13, 2022 at 12:19:05AM +0100, Fabio M. De Francesco wrote:
> +static void *ufs_get_page(struct inode *dir, unsigned long n, struct page **page)
>  {
>  	struct address_space *mapping = dir->i_mapping;
> -	struct page *page = read_mapping_page(mapping, n, NULL);
> -	if (!IS_ERR(page)) {
> -		kmap(page);
> -		if (unlikely(!PageChecked(page))) {
> -			if (!ufs_check_page(page))
> +	*page = read_mapping_page(mapping, n, NULL);
> +	if (!IS_ERR(*page)) {
> +		kmap(*page);
> +		if (unlikely(!PageChecked(*page))) {
> +			if (!ufs_check_page(*page))
>  				goto fail;
>  		}
>  	}
> -	return page;
> +	return page_address(*page);

Er...   You really don't want to do that when you've got ERR_PTR()
from read_mapping_page().
>  
>  fail:
> -	ufs_put_page(page);
> +	ufs_put_page(*page);
>  	return ERR_PTR(-EIO);
>  }

IDGI...

static void *ufs_get_page(struct inode *dir, unsigned long n, struct page **p)
{
	struct address_space *mapping = dir->i_mapping;
	struct page *page = read_mapping_page(mapping, n, NULL);

	if (!IS_ERR(page)) {
		kmap(page);
		if (unlikely(!PageChecked(page))) {
			if (!ufs_check_page(page))
				goto fail;
		}
		*p = page;
		return page_address(page);
	}
	return ERR_CAST(page);

fail:
	ufs_put_page(page);
	return ERR_PTR(-EIO);
}

all there is to it...  The only things you need to change are
1) type of function
2) make sure to store the page into that pointer to pointer to page on success
3) return page_address(page) instead of page on success
4) use ERR_CAST() to convert ERR_PTR() that is struct page * into equal
ERR_PTR() that is void * (the last one is optional, just makes the intent
more clear).