All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@lst.de>
To: cluster-devel.redhat.com
Subject: [Cluster-devel] [PATCH v6 4/9] iomap: Generic inline data handling
Date: Sat, 2 Jun 2018 19:04:29 +0200	[thread overview]
Message-ID: <20180602170429.GC15847@lst.de> (raw)
In-Reply-To: <20180602095717.31641-5-agruenba@redhat.com>

On Sat, Jun 02, 2018 at 11:57:12AM +0200, Andreas Gruenbacher wrote:
> Add generic inline data handling by adding a pointer to the inline data
> region to struct iomap.  When handling a buffered IOMAP_INLINE write,
> iomap_write_begin will copy the current inline data from the inline data
> region into the page cache, and iomap_write_end will copy the changes in
> the page cache back to the inline data region.

This approach looks good.  A few comments below:

> 
> This doesn't cover inline data reads and direct I/O yet because so far,
> we have no users.

I'm fine with that as a first step, but gfs2 should be able to do
proper direct I/O to inline data and use by new iomap_readpage(s)
easily at least for blocksize == PAGE_SIZE, so this should be added
soon.

> -int generic_write_end(struct file *file, struct address_space *mapping,
> +void __generic_write_end(struct file *file, struct address_space *mapping,
>  			loff_t pos, unsigned len, unsigned copied,
> -			struct page *page, void *fsdata)
> +			struct page *page, void *fsdata, bool dirty_inode)

This is going to clash with

http://git.infradead.org/users/hch/xfs.git/commitdiff/2733909d6b40046ce9c7302c2e742c5e993a0108

It should also be a separate prep patch with a good explanation

>  iomap_write_end(struct inode *inode, loff_t pos, unsigned len,
> -		unsigned copied, struct page *page)
> +		unsigned copied, struct page *page, struct iomap *iomap)

Note that I have another patch adding this parameter.  I think we'll need
a common iomap base tree for the gfs2 and xfs changes for the next
merge window.  I'd be happy to one one up.


> diff --git a/include/linux/iomap.h b/include/linux/iomap.h
> index 918f14075702..c61113c71a60 100644
> --- a/include/linux/iomap.h
> +++ b/include/linux/iomap.h
> @@ -47,6 +47,7 @@ struct iomap {
>  	u64			length;	/* length of mapping, bytes */
>  	u16			type;	/* type of mapping */
>  	u16			flags;	/* flags for mapping */
> +	void			*inline_data;  /* inline data buffer */
>  	struct block_device	*bdev;	/* block device for I/O */
>  	struct dax_device	*dax_dev; /* dax_dev for dax operations */
>  };

Eventually we need to find a way to union the inline_data, bdev and
dax_dev fields, but that can be left to later.



WARNING: multiple messages have this Message-ID (diff)
From: Christoph Hellwig <hch@lst.de>
To: Andreas Gruenbacher <agruenba@redhat.com>
Cc: cluster-devel@redhat.com, Christoph Hellwig <hch@lst.de>,
	linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH v6 4/9] iomap: Generic inline data handling
Date: Sat, 2 Jun 2018 19:04:29 +0200	[thread overview]
Message-ID: <20180602170429.GC15847@lst.de> (raw)
In-Reply-To: <20180602095717.31641-5-agruenba@redhat.com>

On Sat, Jun 02, 2018 at 11:57:12AM +0200, Andreas Gruenbacher wrote:
> Add generic inline data handling by adding a pointer to the inline data
> region to struct iomap.  When handling a buffered IOMAP_INLINE write,
> iomap_write_begin will copy the current inline data from the inline data
> region into the page cache, and iomap_write_end will copy the changes in
> the page cache back to the inline data region.

This approach looks good.  A few comments below:

> 
> This doesn't cover inline data reads and direct I/O yet because so far,
> we have no users.

I'm fine with that as a first step, but gfs2 should be able to do
proper direct I/O to inline data and use by new iomap_readpage(s)
easily at least for blocksize == PAGE_SIZE, so this should be added
soon.

> -int generic_write_end(struct file *file, struct address_space *mapping,
> +void __generic_write_end(struct file *file, struct address_space *mapping,
>  			loff_t pos, unsigned len, unsigned copied,
> -			struct page *page, void *fsdata)
> +			struct page *page, void *fsdata, bool dirty_inode)

This is going to clash with

http://git.infradead.org/users/hch/xfs.git/commitdiff/2733909d6b40046ce9c7302c2e742c5e993a0108

It should also be a separate prep patch with a good explanation

>  iomap_write_end(struct inode *inode, loff_t pos, unsigned len,
> -		unsigned copied, struct page *page)
> +		unsigned copied, struct page *page, struct iomap *iomap)

Note that I have another patch adding this parameter.  I think we'll need
a common iomap base tree for the gfs2 and xfs changes for the next
merge window.  I'd be happy to one one up.


> diff --git a/include/linux/iomap.h b/include/linux/iomap.h
> index 918f14075702..c61113c71a60 100644
> --- a/include/linux/iomap.h
> +++ b/include/linux/iomap.h
> @@ -47,6 +47,7 @@ struct iomap {
>  	u64			length;	/* length of mapping, bytes */
>  	u16			type;	/* type of mapping */
>  	u16			flags;	/* flags for mapping */
> +	void			*inline_data;  /* inline data buffer */
>  	struct block_device	*bdev;	/* block device for I/O */
>  	struct dax_device	*dax_dev; /* dax_dev for dax operations */
>  };

Eventually we need to find a way to union the inline_data, bdev and
dax_dev fields, but that can be left to later.

  reply	other threads:[~2018-06-02 17:04 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-02  9:57 [Cluster-devel] [PATCH v6 0/9] gfs2 iomap write support Andreas Gruenbacher
2018-06-02  9:57 ` Andreas Gruenbacher
2018-06-02  9:57 ` [Cluster-devel] [PATCH v6 1/9] iomap: inline data should be an iomap type, not a flag Andreas Gruenbacher
2018-06-02  9:57   ` Andreas Gruenbacher
2018-06-02  9:57 ` [Cluster-devel] [PATCH v6 2/9] iomap: Mark newly allocated buffer heads as new Andreas Gruenbacher
2018-06-02  9:57   ` Andreas Gruenbacher
2018-06-02 16:58   ` [Cluster-devel] " Christoph Hellwig
2018-06-02 16:58     ` Christoph Hellwig
2018-06-02  9:57 ` [Cluster-devel] [PATCH v6 3/9] iomap: Complete partial direct I/O writes synchronously Andreas Gruenbacher
2018-06-02  9:57   ` Andreas Gruenbacher
2018-06-02 16:58   ` [Cluster-devel] " Christoph Hellwig
2018-06-02 16:58     ` Christoph Hellwig
2018-06-02  9:57 ` [Cluster-devel] [PATCH v6 4/9] iomap: Generic inline data handling Andreas Gruenbacher
2018-06-02  9:57   ` Andreas Gruenbacher
2018-06-02 17:04   ` Christoph Hellwig [this message]
2018-06-02 17:04     ` Christoph Hellwig
2018-06-04 12:02     ` [Cluster-devel] " Andreas Grünbacher
2018-06-04 12:02       ` Andreas Grünbacher
2018-06-04 12:12       ` [Cluster-devel] " Christoph Hellwig
2018-06-04 12:12         ` Christoph Hellwig
2018-06-04 17:01         ` [Cluster-devel] " Andreas Grünbacher
2018-06-04 17:01           ` Andreas Grünbacher
2018-06-02  9:57 ` [Cluster-devel] [PATCH v6 5/9] iomap: Add write_end iomap operation Andreas Gruenbacher
2018-06-02  9:57   ` Andreas Gruenbacher
2018-06-02 17:06   ` [Cluster-devel] " Christoph Hellwig
2018-06-02 17:06     ` Christoph Hellwig
2018-06-04 12:03     ` [Cluster-devel] " Andreas Grünbacher
2018-06-04 12:03       ` Andreas Grünbacher
2018-06-02  9:57 ` [Cluster-devel] [PATCH v6 6/9] gfs2: iomap buffered write support Andreas Gruenbacher
2018-06-02  9:57   ` Andreas Gruenbacher
2018-06-02  9:57 ` [Cluster-devel] [PATCH v6 7/9] gfs2: gfs2_extent_length cleanup Andreas Gruenbacher
2018-06-02  9:57   ` Andreas Gruenbacher
2018-06-02  9:57 ` [Cluster-devel] [PATCH v6 8/9] gfs2: iomap direct I/O support Andreas Gruenbacher
2018-06-02  9:57   ` Andreas Gruenbacher
2018-06-02  9:57 ` [Cluster-devel] [PATCH v6 9/9] gfs2: Remove gfs2_write_{begin, end} Andreas Gruenbacher
2018-06-02  9:57   ` [PATCH v6 9/9] gfs2: Remove gfs2_write_{begin,end} Andreas Gruenbacher

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180602170429.GC15847@lst.de \
    --to=hch@lst.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.