From: Gao Xiang <hsiangkao@linux.alibaba.com> To: Matthew Wilcox <willy@infradead.org> Cc: linux-erofs@lists.ozlabs.org, linux-fsdevel@vger.kernel.org, LKML <linux-kernel@vger.kernel.org>, Christoph Hellwig <hch@lst.de>, "Darrick J . Wong" <djwong@kernel.org>, Andreas Gruenbacher <andreas.gruenbacher@gmail.com>, Huang Jianan <huangjianan@oppo.com> Subject: Re: [PATCH v7] iomap: make inline data support more flexible Date: Sat, 24 Jul 2021 08:54:10 +0800 [thread overview] Message-ID: <YPtksjmvVbcsKwlK@B-P7TQMD6M-0146.local> (raw) In-Reply-To: <YPsbQzcNz+r4V7P2@casper.infradead.org> Hi Matthew, On Fri, Jul 23, 2021 at 08:40:51PM +0100, Matthew Wilcox wrote: > On Sat, Jul 24, 2021 at 01:41:31AM +0800, Gao Xiang wrote: > > Add support for reading inline data content into the page cache from > > nonzero page-aligned file offsets. This enables the EROFS tailpacking > > mode where the last few bytes of the file are stored right after the > > inode. > > > > The buffered write path remains untouched since EROFS cannot be used > > for testing. It'd be better to be implemented if upcoming real users > > care and provide a real pattern rather than leave untested dead code > > around. > > My one complaint with this version is the subject line. It's a bit vague. > I went with: > > iomap: Support file tail packing > > I also wrote a changelog entry that reads: > The existing inline data support only works for cases where the entire > file is stored as inline data. For larger files, EROFS stores the > initial blocks separately and then can pack a small tail adjacent to > the inode. Generalise inline data to allow for tail packing. Tails > may not cross a page boundary in memory. > Yeah, we could complete the commit message like this. Actually EROFS inode base is only 32-byte or 64-byte (so the maximum could not be exactly small), compared to using another tail block or storing other (maybe) irrelevant inodes. According to cache locality principle, a strategy can be selected by mkfs to load tail block with the inode base itself to the page cache by the tail-packing inline and so reduce I/O and fragmentation. > ... but I'm not sure that's necessarily better than what you've written > here. > > > Cc: Christoph Hellwig <hch@lst.de> > > Cc: Darrick J. Wong <djwong@kernel.org> > > Cc: Matthew Wilcox <willy@infradead.org> > > Cc: Andreas Gruenbacher <andreas.gruenbacher@gmail.com> > > Tested-by: Huang Jianan <huangjianan@oppo.com> # erofs > > Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com> > > Reviewed-by: Matthew Wilcox (Oracle) <willy@infradead.org> Many thanks for the review! Thanks, Gao Xiang
WARNING: multiple messages have this Message-ID (diff)
From: Gao Xiang <hsiangkao@linux.alibaba.com> To: Matthew Wilcox <willy@infradead.org> Cc: "Darrick J . Wong" <djwong@kernel.org>, Andreas Gruenbacher <andreas.gruenbacher@gmail.com>, LKML <linux-kernel@vger.kernel.org>, linux-fsdevel@vger.kernel.org, linux-erofs@lists.ozlabs.org, Christoph Hellwig <hch@lst.de> Subject: Re: [PATCH v7] iomap: make inline data support more flexible Date: Sat, 24 Jul 2021 08:54:10 +0800 [thread overview] Message-ID: <YPtksjmvVbcsKwlK@B-P7TQMD6M-0146.local> (raw) In-Reply-To: <YPsbQzcNz+r4V7P2@casper.infradead.org> Hi Matthew, On Fri, Jul 23, 2021 at 08:40:51PM +0100, Matthew Wilcox wrote: > On Sat, Jul 24, 2021 at 01:41:31AM +0800, Gao Xiang wrote: > > Add support for reading inline data content into the page cache from > > nonzero page-aligned file offsets. This enables the EROFS tailpacking > > mode where the last few bytes of the file are stored right after the > > inode. > > > > The buffered write path remains untouched since EROFS cannot be used > > for testing. It'd be better to be implemented if upcoming real users > > care and provide a real pattern rather than leave untested dead code > > around. > > My one complaint with this version is the subject line. It's a bit vague. > I went with: > > iomap: Support file tail packing > > I also wrote a changelog entry that reads: > The existing inline data support only works for cases where the entire > file is stored as inline data. For larger files, EROFS stores the > initial blocks separately and then can pack a small tail adjacent to > the inode. Generalise inline data to allow for tail packing. Tails > may not cross a page boundary in memory. > Yeah, we could complete the commit message like this. Actually EROFS inode base is only 32-byte or 64-byte (so the maximum could not be exactly small), compared to using another tail block or storing other (maybe) irrelevant inodes. According to cache locality principle, a strategy can be selected by mkfs to load tail block with the inode base itself to the page cache by the tail-packing inline and so reduce I/O and fragmentation. > ... but I'm not sure that's necessarily better than what you've written > here. > > > Cc: Christoph Hellwig <hch@lst.de> > > Cc: Darrick J. Wong <djwong@kernel.org> > > Cc: Matthew Wilcox <willy@infradead.org> > > Cc: Andreas Gruenbacher <andreas.gruenbacher@gmail.com> > > Tested-by: Huang Jianan <huangjianan@oppo.com> # erofs > > Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com> > > Reviewed-by: Matthew Wilcox (Oracle) <willy@infradead.org> Many thanks for the review! Thanks, Gao Xiang
next prev parent reply other threads:[~2021-07-24 0:54 UTC|newest] Thread overview: 59+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-07-23 17:41 [PATCH v7] iomap: make inline data support more flexible Gao Xiang 2021-07-23 17:41 ` Gao Xiang 2021-07-23 19:40 ` Matthew Wilcox 2021-07-23 19:40 ` Matthew Wilcox 2021-07-24 0:54 ` Gao Xiang [this message] 2021-07-24 0:54 ` Gao Xiang 2021-07-25 21:39 ` Andreas Grünbacher 2021-07-25 21:39 ` Andreas Grünbacher 2021-07-25 22:16 ` Andreas Gruenbacher 2021-07-25 22:16 ` Andreas Gruenbacher 2021-07-26 2:36 ` Gao Xiang 2021-07-26 2:36 ` Gao Xiang 2021-07-26 7:22 ` Andreas Gruenbacher 2021-07-26 7:22 ` Andreas Gruenbacher 2021-07-26 7:38 ` Gao Xiang 2021-07-26 7:38 ` Gao Xiang 2021-07-26 21:36 ` Darrick J. Wong 2021-07-26 21:36 ` Darrick J. Wong 2021-07-26 22:20 ` Andreas Grünbacher 2021-07-26 22:20 ` Andreas Grünbacher 2021-07-26 3:06 ` Matthew Wilcox 2021-07-26 3:06 ` Matthew Wilcox 2021-07-26 6:56 ` Andreas Gruenbacher 2021-07-26 6:56 ` Andreas Gruenbacher 2021-07-26 4:00 ` Gao Xiang 2021-07-26 4:00 ` Gao Xiang 2021-07-26 8:08 ` Andreas Grünbacher 2021-07-26 8:08 ` Andreas Grünbacher 2021-07-26 8:17 ` Gao Xiang 2021-07-26 8:17 ` Gao Xiang 2021-07-26 11:06 ` Andreas Gruenbacher 2021-07-26 11:06 ` Andreas Gruenbacher 2021-07-26 12:17 ` Christoph Hellwig 2021-07-26 12:17 ` Christoph Hellwig 2021-07-26 12:27 ` Andreas Grünbacher 2021-07-26 12:27 ` Andreas Grünbacher 2021-07-26 12:50 ` Gao Xiang 2021-07-26 12:50 ` Gao Xiang 2021-07-26 13:10 ` Andreas Gruenbacher 2021-07-26 13:27 ` Christoph Hellwig 2021-07-26 13:27 ` Christoph Hellwig 2021-07-27 8:20 ` David Sterba 2021-07-27 8:20 ` David Sterba 2021-07-27 13:35 ` Matthew Wilcox 2021-07-27 15:04 ` Gao Xiang 2021-07-27 15:04 ` Gao Xiang 2021-07-27 16:53 ` David Sterba 2021-07-27 16:53 ` David Sterba 2021-07-26 12:32 ` Matthew Wilcox 2021-07-26 12:32 ` Matthew Wilcox 2021-07-26 13:03 ` Andreas Gruenbacher 2021-07-26 13:03 ` Andreas Gruenbacher 2021-07-26 13:12 ` Gao Xiang 2021-07-26 13:12 ` Gao Xiang 2021-07-26 13:30 ` Christoph Hellwig 2021-07-26 8:08 ` Joseph Qi 2021-07-26 8:08 ` Joseph Qi 2021-08-01 10:29 Andreas Gruenbacher 2021-08-01 10:29 ` 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=YPtksjmvVbcsKwlK@B-P7TQMD6M-0146.local \ --to=hsiangkao@linux.alibaba.com \ --cc=andreas.gruenbacher@gmail.com \ --cc=djwong@kernel.org \ --cc=hch@lst.de \ --cc=huangjianan@oppo.com \ --cc=linux-erofs@lists.ozlabs.org \ --cc=linux-fsdevel@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=willy@infradead.org \ /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: linkBe 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.