lkml.org 
[lkml]   [2021]   [Jul]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH 1/2] iomap: support tail packing inline read
    On Sat, Jul 17, 2021 at 11:15:58PM +0800, Gao Xiang wrote:
    > Hi Matthew,
    >
    > On Sat, Jul 17, 2021 at 04:01:38PM +0100, Matthew Wilcox wrote:
    > > On Sat, Jul 17, 2021 at 09:38:18PM +0800, Gao Xiang wrote:
    > > > Sorry about some late. I've revised a version based on Christoph's
    > > > version and Matthew's thought above. I've preliminary checked with
    > > > EROFS, if it does make sense, please kindly help check on the gfs2
    > > > side as well..
    > >
    > > I don't understand how this bit works:
    >
    > This part inherited from the Christoph version without change.
    > The following thoughts are just my own understanding...
    >
    > >
    > > > struct page *page = ctx->cur_page;
    > > > - struct iomap_page *iop;
    > > > + struct iomap_page *iop = NULL;
    > > > bool same_page = false, is_contig = false;
    > > > loff_t orig_pos = pos;
    > > > unsigned poff, plen;
    > > > sector_t sector;
    > > >
    > > > - if (iomap->type == IOMAP_INLINE) {
    > > > - WARN_ON_ONCE(pos);
    > > > - iomap_read_inline_data(inode, page, iomap);
    > > > - return PAGE_SIZE;
    > > > - }
    > > > + if (iomap->type == IOMAP_INLINE && !pos)
    > > > + WARN_ON_ONCE(to_iomap_page(page) != NULL);
    > > > + else
    > > > + iop = iomap_page_create(inode, page);
    > >
    > > Imagine you have a file with bytes 0-2047 in an extent which is !INLINE
    > > and bytes 2048-2051 in the INLINE extent. When you read the page, first
    > > you create an iop for the !INLINE extent. Then this function is called
    >
    > Yes, it first created an iop for the !INLINE extent.
    >
    > > again for the INLINE extent and you'll hit the WARN_ON_ONCE. No?
    >
    > If it is called again with another INLINE extent, pos will be non-0?
    > so (!pos) == false. Am I missing something?

    Well, either sense of a WARN_ON is wrong.

    For a file which is PAGE_SIZE + 3 bytes in size, to_iomap_page() will
    be NULL. For a file which is PAGE_SIZE/2 + 3 bytes in size,
    to_iomap_page() will not be NULL. (assuming the block size is <=
    PAGE_SIZE / 2).

    I think we need a prep patch that looks something like this:

    +++ b/fs/iomap/buffered-io.c
    @@ -252,8 +252,12 @@ iomap_readpage_actor(struct inode *inode, loff_t pos, loff_t length, void *data,
    return PAGE_SIZE;
    }

    + if (offset_in_page(pos) || length < PAGE_SIZE)
    + iop = iomap_page_create(inode, page);
    + else
    + iop = NULL;
    +
    /* zero post-eof blocks as the page may be mapped */
    - iop = iomap_page_create(inode, page);
    iomap_adjust_read_range(inode, iop, &pos, length, &poff, &plen);
    if (plen == 0)
    goto done;

    ie first get the conditions right under which we should create an iop.

    \
     
     \ /
      Last update: 2021-07-17 20:43    [W:2.142 / U:0.016 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site