Re: [PATCH 5/9] btrfs: Use ->iomap_end() instead of btrfs_dio_data

[Goldwyn Rodrigues <rgoldwyn@suse.de>]

On  1:16 27/03, Christoph Hellwig wrote:
> On Thu, Mar 26, 2020 at 04:02:50PM -0500, Goldwyn Rodrigues wrote:
> > From: Goldwyn Rodrigues <rgoldwyn@suse.com>
> > 
> > Use iomap->iomap_end() to check for failed or incomplete writes and call
> > __endio_write_update_ordered(). We don't need btrfs_dio_data anymore so
> > remove that. The bonus is we don't abuse current->journal_info anymore.
> > 
> > A new structure btrfs_iomap is used to keep a count of submitted I/O
> > for writes.
> 
> I don't think you need a new structure.  As writes are limited to a
> size_t (aka long) you can just case iomap->private.  That is a little
> ugly, but we can just switch the private field to an union, something
> like the patch below.  If I'm missing a reason why it has to be 64-bit
> even on 32-bit kernels we can also grow the size a little on 32-bit
> kernels, but right now I don't think that is needed unless I'm missing
> something.

This would be an better approach as opposed to allocating and
deallocating. I was trying not to disrupt the iomap code ;)
Yes, 32-bits works just as well.

> 
> ---
> From e496cd3db3e7420050be19c5fe68e4675f5a2abc Mon Sep 17 00:00:00 2001
> From: Christoph Hellwig <hch@lst.de>
> Date: Fri, 27 Mar 2020 09:14:34 +0100
> Subject: iomap: turn iomap->private into an union
> 
> Make using the union a little easier for scalar values.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  include/linux/iomap.h | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/iomap.h b/include/linux/iomap.h
> index 8b09463dae0d..61fea687d93d 100644
> --- a/include/linux/iomap.h
> +++ b/include/linux/iomap.h
> @@ -85,7 +85,10 @@ struct iomap {
>  	struct block_device	*bdev;	/* block device for I/O */
>  	struct dax_device	*dax_dev; /* dax_dev for dax operations */
>  	void			*inline_data;
> -	void			*private; /* filesystem private */
> +	union {				/* filesystem private data */
> +		void		*ptr;
> +		uintptr_t	uint;
> +	} private;
>  	const struct iomap_page_ops *page_ops;
>  };
>  


Thanks. Will include this.

-- 
Goldwyn