All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] iomap: Support inline data with block size < page size
@ 2021-07-29  3:23 ` Matthew Wilcox (Oracle)
  0 siblings, 0 replies; 8+ messages in thread
From: Matthew Wilcox (Oracle) @ 2021-07-29  3:23 UTC (permalink / raw)
  To: linux-erofs, linux-fsdevel, linux-xfs
  Cc: Matthew Wilcox (Oracle), hsiangkao, djwong, hch, agruenba

Remove the restriction that inline data must start on a page boundary
in a file.  This allows, for example, the first 2KiB to be stored out
of line and the trailing 30 bytes to be stored inline.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
v2:
 - Rebase on top of iomap: Support file tail packing v9

 fs/iomap/buffered-io.c | 34 ++++++++++++++++------------------
 1 file changed, 16 insertions(+), 18 deletions(-)

diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
index 66b733537c46..50f18985ed13 100644
--- a/fs/iomap/buffered-io.c
+++ b/fs/iomap/buffered-io.c
@@ -209,28 +209,26 @@ static int iomap_read_inline_data(struct inode *inode, struct page *page,
 		struct iomap *iomap)
 {
 	size_t size = i_size_read(inode) - iomap->offset;
+	size_t poff = offset_in_page(iomap->offset);
 	void *addr;
 
 	if (PageUptodate(page))
-		return 0;
+		return PAGE_SIZE - poff;
 
-	/* inline data must start page aligned in the file */
-	if (WARN_ON_ONCE(offset_in_page(iomap->offset)))
-		return -EIO;
 	if (WARN_ON_ONCE(size > PAGE_SIZE -
 			 offset_in_page(iomap->inline_data)))
 		return -EIO;
 	if (WARN_ON_ONCE(size > iomap->length))
 		return -EIO;
-	if (WARN_ON_ONCE(page_has_private(page)))
-		return -EIO;
+	if (poff > 0)
+		iomap_page_create(inode, page);
 
-	addr = kmap_atomic(page);
+	addr = kmap_atomic(page) + poff;
 	memcpy(addr, iomap->inline_data, size);
-	memset(addr + size, 0, PAGE_SIZE - size);
+	memset(addr + size, 0, PAGE_SIZE - poff - size);
 	kunmap_atomic(addr);
-	SetPageUptodate(page);
-	return 0;
+	iomap_set_range_uptodate(page, poff, PAGE_SIZE - poff);
+	return PAGE_SIZE - poff;
 }
 
 static inline bool iomap_block_needs_zeroing(struct inode *inode,
@@ -252,13 +250,8 @@ iomap_readpage_actor(struct inode *inode, loff_t pos, loff_t length, void *data,
 	unsigned poff, plen;
 	sector_t sector;
 
-	if (iomap->type == IOMAP_INLINE) {
-		int ret = iomap_read_inline_data(inode, page, iomap);
-
-		if (ret)
-			return ret;
-		return PAGE_SIZE;
-	}
+	if (iomap->type == IOMAP_INLINE)
+		return iomap_read_inline_data(inode, page, iomap);
 
 	/* zero post-eof blocks as the page may be mapped */
 	iop = iomap_page_create(inode, page);
@@ -593,10 +586,15 @@ __iomap_write_begin(struct inode *inode, loff_t pos, unsigned len, int flags,
 static int iomap_write_begin_inline(struct inode *inode,
 		struct page *page, struct iomap *srcmap)
 {
+	int ret;
+
 	/* needs more work for the tailpacking case, disable for now */
 	if (WARN_ON_ONCE(srcmap->offset != 0))
 		return -EIO;
-	return iomap_read_inline_data(inode, page, srcmap);
+	ret = iomap_read_inline_data(inode, page, srcmap);
+	if (ret < 0)
+		return ret;
+	return 0;
 }
 
 static int
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 8+ messages in thread
* [PATCH v2] iomap: Support inline data with block size < page size
@ 2021-07-29  3:23 ` Matthew Wilcox (Oracle)
  0 siblings, 0 replies; 8+ messages in thread
From: Matthew Wilcox (Oracle) @ 2021-07-29  3:23 UTC (permalink / raw)
  To: linux-erofs, linux-fsdevel, linux-xfs
  Cc: agruenba, hch, Matthew Wilcox (Oracle), djwong

Remove the restriction that inline data must start on a page boundary
in a file.  This allows, for example, the first 2KiB to be stored out
of line and the trailing 30 bytes to be stored inline.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
v2:
 - Rebase on top of iomap: Support file tail packing v9

 fs/iomap/buffered-io.c | 34 ++++++++++++++++------------------
 1 file changed, 16 insertions(+), 18 deletions(-)

diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
index 66b733537c46..50f18985ed13 100644
--- a/fs/iomap/buffered-io.c
+++ b/fs/iomap/buffered-io.c
@@ -209,28 +209,26 @@ static int iomap_read_inline_data(struct inode *inode, struct page *page,
 		struct iomap *iomap)
 {
 	size_t size = i_size_read(inode) - iomap->offset;
+	size_t poff = offset_in_page(iomap->offset);
 	void *addr;
 
 	if (PageUptodate(page))
-		return 0;
+		return PAGE_SIZE - poff;
 
-	/* inline data must start page aligned in the file */
-	if (WARN_ON_ONCE(offset_in_page(iomap->offset)))
-		return -EIO;
 	if (WARN_ON_ONCE(size > PAGE_SIZE -
 			 offset_in_page(iomap->inline_data)))
 		return -EIO;
 	if (WARN_ON_ONCE(size > iomap->length))
 		return -EIO;
-	if (WARN_ON_ONCE(page_has_private(page)))
-		return -EIO;
+	if (poff > 0)
+		iomap_page_create(inode, page);
 
-	addr = kmap_atomic(page);
+	addr = kmap_atomic(page) + poff;
 	memcpy(addr, iomap->inline_data, size);
-	memset(addr + size, 0, PAGE_SIZE - size);
+	memset(addr + size, 0, PAGE_SIZE - poff - size);
 	kunmap_atomic(addr);
-	SetPageUptodate(page);
-	return 0;
+	iomap_set_range_uptodate(page, poff, PAGE_SIZE - poff);
+	return PAGE_SIZE - poff;
 }
 
 static inline bool iomap_block_needs_zeroing(struct inode *inode,
@@ -252,13 +250,8 @@ iomap_readpage_actor(struct inode *inode, loff_t pos, loff_t length, void *data,
 	unsigned poff, plen;
 	sector_t sector;
 
-	if (iomap->type == IOMAP_INLINE) {
-		int ret = iomap_read_inline_data(inode, page, iomap);
-
-		if (ret)
-			return ret;
-		return PAGE_SIZE;
-	}
+	if (iomap->type == IOMAP_INLINE)
+		return iomap_read_inline_data(inode, page, iomap);
 
 	/* zero post-eof blocks as the page may be mapped */
 	iop = iomap_page_create(inode, page);
@@ -593,10 +586,15 @@ __iomap_write_begin(struct inode *inode, loff_t pos, unsigned len, int flags,
 static int iomap_write_begin_inline(struct inode *inode,
 		struct page *page, struct iomap *srcmap)
 {
+	int ret;
+
 	/* needs more work for the tailpacking case, disable for now */
 	if (WARN_ON_ONCE(srcmap->offset != 0))
 		return -EIO;
-	return iomap_read_inline_data(inode, page, srcmap);
+	ret = iomap_read_inline_data(inode, page, srcmap);
+	if (ret < 0)
+		return ret;
+	return 0;
 }
 
 static int
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 8+ messages in thread
* Re: [PATCH v2] iomap: Support inline data with block size < page size
  2021-07-29  3:23 ` Matthew Wilcox (Oracle)
@ 2021-07-29  3:54   ` Andreas Gruenbacher
  -1 siblings, 0 replies; 8+ messages in thread
From: Andreas Gruenbacher @ 2021-07-29  3:54 UTC (permalink / raw)
  To: Matthew Wilcox (Oracle)
  Cc: linux-erofs, linux-fsdevel, linux-xfs, Gao Xiang,
	Darrick J. Wong, Christoph Hellwig

On Thu, Jul 29, 2021 at 5:25 AM Matthew Wilcox (Oracle)
<willy@infradead.org> wrote:
> Remove the restriction that inline data must start on a page boundary
> in a file.  This allows, for example, the first 2KiB to be stored out
> of line and the trailing 30 bytes to be stored inline.
>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
> v2:
>  - Rebase on top of iomap: Support file tail packing v9
>
>  fs/iomap/buffered-io.c | 34 ++++++++++++++++------------------
>  1 file changed, 16 insertions(+), 18 deletions(-)
>
> diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
> index 66b733537c46..50f18985ed13 100644
> --- a/fs/iomap/buffered-io.c
> +++ b/fs/iomap/buffered-io.c
> @@ -209,28 +209,26 @@ static int iomap_read_inline_data(struct inode *inode, struct page *page,
>                 struct iomap *iomap)
>  {
>         size_t size = i_size_read(inode) - iomap->offset;
> +       size_t poff = offset_in_page(iomap->offset);
>         void *addr;
>
>         if (PageUptodate(page))
> -               return 0;
> +               return PAGE_SIZE - poff;
>
> -       /* inline data must start page aligned in the file */
> -       if (WARN_ON_ONCE(offset_in_page(iomap->offset)))
> -               return -EIO;

Maybe add a WARN_ON_ONCE(size > PAGE_SIZE - poff) here?

>         if (WARN_ON_ONCE(size > PAGE_SIZE -
>                          offset_in_page(iomap->inline_data)))
>                 return -EIO;
>         if (WARN_ON_ONCE(size > iomap->length))
>                 return -EIO;
> -       if (WARN_ON_ONCE(page_has_private(page)))
> -               return -EIO;
> +       if (poff > 0)
> +               iomap_page_create(inode, page);
>
> -       addr = kmap_atomic(page);
> +       addr = kmap_atomic(page) + poff;

Maybe kmap_local_page?

>         memcpy(addr, iomap->inline_data, size);
> -       memset(addr + size, 0, PAGE_SIZE - size);
> +       memset(addr + size, 0, PAGE_SIZE - poff - size);
>         kunmap_atomic(addr);
> -       SetPageUptodate(page);
> -       return 0;
> +       iomap_set_range_uptodate(page, poff, PAGE_SIZE - poff);
> +       return PAGE_SIZE - poff;
>  }
>
>  static inline bool iomap_block_needs_zeroing(struct inode *inode,
> @@ -252,13 +250,8 @@ iomap_readpage_actor(struct inode *inode, loff_t pos, loff_t length, void *data,
>         unsigned poff, plen;
>         sector_t sector;
>
> -       if (iomap->type == IOMAP_INLINE) {
> -               int ret = iomap_read_inline_data(inode, page, iomap);
> -
> -               if (ret)
> -                       return ret;
> -               return PAGE_SIZE;
> -       }
> +       if (iomap->type == IOMAP_INLINE)
> +               return iomap_read_inline_data(inode, page, iomap);
>
>         /* zero post-eof blocks as the page may be mapped */
>         iop = iomap_page_create(inode, page);
> @@ -593,10 +586,15 @@ __iomap_write_begin(struct inode *inode, loff_t pos, unsigned len, int flags,
>  static int iomap_write_begin_inline(struct inode *inode,
>                 struct page *page, struct iomap *srcmap)
>  {
> +       int ret;
> +
>         /* needs more work for the tailpacking case, disable for now */
>         if (WARN_ON_ONCE(srcmap->offset != 0))
>                 return -EIO;
> -       return iomap_read_inline_data(inode, page, srcmap);
> +       ret = iomap_read_inline_data(inode, page, srcmap);
> +       if (ret < 0)
> +               return ret;
> +       return 0;
>  }
>
>  static int
> --
> 2.30.2
>

Thanks,
Andreas


^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH v2] iomap: Support inline data with block size < page size
@ 2021-07-29  3:54   ` Andreas Gruenbacher
  0 siblings, 0 replies; 8+ messages in thread
From: Andreas Gruenbacher @ 2021-07-29  3:54 UTC (permalink / raw)
  To: Matthew Wilcox (Oracle)
  Cc: Darrick J. Wong, linux-xfs, linux-fsdevel, linux-erofs,
	Christoph Hellwig

On Thu, Jul 29, 2021 at 5:25 AM Matthew Wilcox (Oracle)
<willy@infradead.org> wrote:
> Remove the restriction that inline data must start on a page boundary
> in a file.  This allows, for example, the first 2KiB to be stored out
> of line and the trailing 30 bytes to be stored inline.
>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
> v2:
>  - Rebase on top of iomap: Support file tail packing v9
>
>  fs/iomap/buffered-io.c | 34 ++++++++++++++++------------------
>  1 file changed, 16 insertions(+), 18 deletions(-)
>
> diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
> index 66b733537c46..50f18985ed13 100644
> --- a/fs/iomap/buffered-io.c
> +++ b/fs/iomap/buffered-io.c
> @@ -209,28 +209,26 @@ static int iomap_read_inline_data(struct inode *inode, struct page *page,
>                 struct iomap *iomap)
>  {
>         size_t size = i_size_read(inode) - iomap->offset;
> +       size_t poff = offset_in_page(iomap->offset);
>         void *addr;
>
>         if (PageUptodate(page))
> -               return 0;
> +               return PAGE_SIZE - poff;
>
> -       /* inline data must start page aligned in the file */
> -       if (WARN_ON_ONCE(offset_in_page(iomap->offset)))
> -               return -EIO;

Maybe add a WARN_ON_ONCE(size > PAGE_SIZE - poff) here?

>         if (WARN_ON_ONCE(size > PAGE_SIZE -
>                          offset_in_page(iomap->inline_data)))
>                 return -EIO;
>         if (WARN_ON_ONCE(size > iomap->length))
>                 return -EIO;
> -       if (WARN_ON_ONCE(page_has_private(page)))
> -               return -EIO;
> +       if (poff > 0)
> +               iomap_page_create(inode, page);
>
> -       addr = kmap_atomic(page);
> +       addr = kmap_atomic(page) + poff;

Maybe kmap_local_page?

>         memcpy(addr, iomap->inline_data, size);
> -       memset(addr + size, 0, PAGE_SIZE - size);
> +       memset(addr + size, 0, PAGE_SIZE - poff - size);
>         kunmap_atomic(addr);
> -       SetPageUptodate(page);
> -       return 0;
> +       iomap_set_range_uptodate(page, poff, PAGE_SIZE - poff);
> +       return PAGE_SIZE - poff;
>  }
>
>  static inline bool iomap_block_needs_zeroing(struct inode *inode,
> @@ -252,13 +250,8 @@ iomap_readpage_actor(struct inode *inode, loff_t pos, loff_t length, void *data,
>         unsigned poff, plen;
>         sector_t sector;
>
> -       if (iomap->type == IOMAP_INLINE) {
> -               int ret = iomap_read_inline_data(inode, page, iomap);
> -
> -               if (ret)
> -                       return ret;
> -               return PAGE_SIZE;
> -       }
> +       if (iomap->type == IOMAP_INLINE)
> +               return iomap_read_inline_data(inode, page, iomap);
>
>         /* zero post-eof blocks as the page may be mapped */
>         iop = iomap_page_create(inode, page);
> @@ -593,10 +586,15 @@ __iomap_write_begin(struct inode *inode, loff_t pos, unsigned len, int flags,
>  static int iomap_write_begin_inline(struct inode *inode,
>                 struct page *page, struct iomap *srcmap)
>  {
> +       int ret;
> +
>         /* needs more work for the tailpacking case, disable for now */
>         if (WARN_ON_ONCE(srcmap->offset != 0))
>                 return -EIO;
> -       return iomap_read_inline_data(inode, page, srcmap);
> +       ret = iomap_read_inline_data(inode, page, srcmap);
> +       if (ret < 0)
> +               return ret;
> +       return 0;
>  }
>
>  static int
> --
> 2.30.2
>

Thanks,
Andreas


^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH v2] iomap: Support inline data with block size < page size
  2021-07-29  3:54   ` Andreas Gruenbacher
@ 2021-07-29 12:43     ` Matthew Wilcox
  -1 siblings, 0 replies; 8+ messages in thread
From: Matthew Wilcox @ 2021-07-29 12:43 UTC (permalink / raw)
  To: Andreas Gruenbacher
  Cc: linux-erofs, linux-fsdevel, linux-xfs, Gao Xiang,
	Darrick J. Wong, Christoph Hellwig

On Thu, Jul 29, 2021 at 05:54:56AM +0200, Andreas Gruenbacher wrote:
> > -       /* inline data must start page aligned in the file */
> > -       if (WARN_ON_ONCE(offset_in_page(iomap->offset)))
> > -               return -EIO;
> 
> Maybe add a WARN_ON_ONCE(size > PAGE_SIZE - poff) here?

Sure!

> >         if (WARN_ON_ONCE(size > PAGE_SIZE -
> >                          offset_in_page(iomap->inline_data)))
> >                 return -EIO;
> >         if (WARN_ON_ONCE(size > iomap->length))
> >                 return -EIO;
> > -       if (WARN_ON_ONCE(page_has_private(page)))
> > -               return -EIO;
> > +       if (poff > 0)
> > +               iomap_page_create(inode, page);
> >
> > -       addr = kmap_atomic(page);
> > +       addr = kmap_atomic(page) + poff;
> 
> Maybe kmap_local_page?

Heh, I do that later when I convert to folios (there is no
kmap_atomic_folio(), only kmap_local_folio()).  But I can throw that
in here too.


^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH v2] iomap: Support inline data with block size < page size
@ 2021-07-29 12:43     ` Matthew Wilcox
  0 siblings, 0 replies; 8+ messages in thread
From: Matthew Wilcox @ 2021-07-29 12:43 UTC (permalink / raw)
  To: Andreas Gruenbacher
  Cc: Darrick J. Wong, linux-xfs, linux-fsdevel, linux-erofs,
	Christoph Hellwig

On Thu, Jul 29, 2021 at 05:54:56AM +0200, Andreas Gruenbacher wrote:
> > -       /* inline data must start page aligned in the file */
> > -       if (WARN_ON_ONCE(offset_in_page(iomap->offset)))
> > -               return -EIO;
> 
> Maybe add a WARN_ON_ONCE(size > PAGE_SIZE - poff) here?

Sure!

> >         if (WARN_ON_ONCE(size > PAGE_SIZE -
> >                          offset_in_page(iomap->inline_data)))
> >                 return -EIO;
> >         if (WARN_ON_ONCE(size > iomap->length))
> >                 return -EIO;
> > -       if (WARN_ON_ONCE(page_has_private(page)))
> > -               return -EIO;
> > +       if (poff > 0)
> > +               iomap_page_create(inode, page);
> >
> > -       addr = kmap_atomic(page);
> > +       addr = kmap_atomic(page) + poff;
> 
> Maybe kmap_local_page?

Heh, I do that later when I convert to folios (there is no
kmap_atomic_folio(), only kmap_local_folio()).  But I can throw that
in here too.


^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH v2] iomap: Support inline data with block size < page size
  2021-07-29 12:43     ` Matthew Wilcox
@ 2021-07-29 17:26       ` Gao Xiang
  -1 siblings, 0 replies; 8+ messages in thread
From: Gao Xiang @ 2021-07-29 17:26 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: Andreas Gruenbacher, linux-erofs, linux-fsdevel, linux-xfs,
	Darrick J. Wong, Christoph Hellwig

Hi Matthew,

On Thu, Jul 29, 2021 at 01:43:38PM +0100, Matthew Wilcox wrote:
> On Thu, Jul 29, 2021 at 05:54:56AM +0200, Andreas Gruenbacher wrote:
> > > -       /* inline data must start page aligned in the file */
> > > -       if (WARN_ON_ONCE(offset_in_page(iomap->offset)))
> > > -               return -EIO;
> > 
> > Maybe add a WARN_ON_ONCE(size > PAGE_SIZE - poff) here?
> 
> Sure!
> 
> > >         if (WARN_ON_ONCE(size > PAGE_SIZE -
> > >                          offset_in_page(iomap->inline_data)))
> > >                 return -EIO;
> > >         if (WARN_ON_ONCE(size > iomap->length))
> > >                 return -EIO;
> > > -       if (WARN_ON_ONCE(page_has_private(page)))
> > > -               return -EIO;
> > > +       if (poff > 0)
> > > +               iomap_page_create(inode, page);
> > >
> > > -       addr = kmap_atomic(page);
> > > +       addr = kmap_atomic(page) + poff;
> > 
> > Maybe kmap_local_page?
> 
> Heh, I do that later when I convert to folios (there is no
> kmap_atomic_folio(), only kmap_local_folio()).  But I can throw that
> in here too.

I don't find any critical point with this patch (and agree with Andreas'
suggestion), maybe some followup folio work could get more input about
this. I'll evaluate them all together later.

Thanks,
Gao Xiang

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH v2] iomap: Support inline data with block size < page size
@ 2021-07-29 17:26       ` Gao Xiang
  0 siblings, 0 replies; 8+ messages in thread
From: Gao Xiang @ 2021-07-29 17:26 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: Andreas Gruenbacher, Darrick J. Wong, linux-xfs, linux-fsdevel,
	linux-erofs, Christoph Hellwig

Hi Matthew,

On Thu, Jul 29, 2021 at 01:43:38PM +0100, Matthew Wilcox wrote:
> On Thu, Jul 29, 2021 at 05:54:56AM +0200, Andreas Gruenbacher wrote:
> > > -       /* inline data must start page aligned in the file */
> > > -       if (WARN_ON_ONCE(offset_in_page(iomap->offset)))
> > > -               return -EIO;
> > 
> > Maybe add a WARN_ON_ONCE(size > PAGE_SIZE - poff) here?
> 
> Sure!
> 
> > >         if (WARN_ON_ONCE(size > PAGE_SIZE -
> > >                          offset_in_page(iomap->inline_data)))
> > >                 return -EIO;
> > >         if (WARN_ON_ONCE(size > iomap->length))
> > >                 return -EIO;
> > > -       if (WARN_ON_ONCE(page_has_private(page)))
> > > -               return -EIO;
> > > +       if (poff > 0)
> > > +               iomap_page_create(inode, page);
> > >
> > > -       addr = kmap_atomic(page);
> > > +       addr = kmap_atomic(page) + poff;
> > 
> > Maybe kmap_local_page?
> 
> Heh, I do that later when I convert to folios (there is no
> kmap_atomic_folio(), only kmap_local_folio()).  But I can throw that
> in here too.

I don't find any critical point with this patch (and agree with Andreas'
suggestion), maybe some followup folio work could get more input about
this. I'll evaluate them all together later.

Thanks,
Gao Xiang

^ permalink raw reply	[flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-07-29 17:34 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-29  3:23 [PATCH v2] iomap: Support inline data with block size < page size Matthew Wilcox (Oracle)
2021-07-29  3:23 ` Matthew Wilcox (Oracle)
2021-07-29  3:54 ` Andreas Gruenbacher
2021-07-29  3:54   ` Andreas Gruenbacher
2021-07-29 12:43   ` Matthew Wilcox
2021-07-29 12:43     ` Matthew Wilcox
2021-07-29 17:26     ` Gao Xiang
2021-07-29 17:26       ` Gao Xiang

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.