From: Gao Xiang <hsiangkao@linux.alibaba.com> To: Christoph Hellwig <hch@infradead.org> Cc: Andreas Gr??nbacher <andreas.gruenbacher@gmail.com>, Matthew Wilcox <willy@infradead.org>, linux-erofs@lists.ozlabs.org, Linux FS-devel Mailing List <linux-fsdevel@vger.kernel.org>, LKML <linux-kernel@vger.kernel.org>, "Darrick J. Wong" <djwong@kernel.org>, Chao Yu <chao@kernel.org>, Liu Bo <bo.liu@linux.alibaba.com>, Joseph Qi <joseph.qi@linux.alibaba.com>, Liu Jiang <gerry@linux.alibaba.com> Subject: Re: [PATCH 1/2] iomap: support tail packing inline read Date: Mon, 19 Jul 2021 21:31:15 +0800 [thread overview] Message-ID: <YPV+o+V7wU32j6m3@B-P7TQMD6M-0146.local> (raw) In-Reply-To: <YPVe41YqpfGLNsBS@infradead.org> On Mon, Jul 19, 2021 at 12:15:47PM +0100, Christoph Hellwig wrote: > On Sat, Jul 17, 2021 at 09:38:18PM +0800, Gao Xiang wrote: > > >From 62f367245492e389711bcebbf7da5adae586299f Mon Sep 17 00:00:00 2001 > > From: Christoph Hellwig <hch@lst.de> > > Date: Fri, 16 Jul 2021 10:52:48 +0200 > > Subject: [PATCH] iomap: support tail packing inline read > > I'd still credit this to you as you did all the hard work. Ok. > > > + /* inline source data must be inside a single page */ > > + BUG_ON(iomap->length > PAGE_SIZE - offset_in_page(iomap->inline_data)); > > + /* handle tail-packing blocks cross the current page into the next */ > > + if (size > PAGE_SIZE - poff) > > + size = PAGE_SIZE - poff; > > This should probably use min or min_t. Okay, will update. > > > > > addr = kmap_atomic(page); > > - memcpy(addr, iomap->inline_data, size); > > - memset(addr + size, 0, PAGE_SIZE - size); > > + memcpy(addr + poff, iomap->inline_data - iomap->offset + pos, size); > > + memset(addr + poff + size, 0, PAGE_SIZE - poff - size); > > kunmap_atomic(addr); > > - SetPageUptodate(page); > > + flush_dcache_page(page); > > The flush_dcache_page addition should be a separate patch. I wondered what is the target order of recent iomap patches, since this is a quite small change, so I'd like to just rebase on for-next for now . So ok, I won't touch this in this patchset and I think flush_dcache_page() does no harm to x86(_64) and arm(64) at least if I remember correctly. > > > > > if (dio->flags & IOMAP_DIO_WRITE) { > > loff_t size = inode->i_size; > > @@ -394,7 +395,8 @@ iomap_dio_inline_actor(struct inode *inode, loff_t pos, loff_t length, > > mark_inode_dirty(inode); > > } > > } else { > > - copied = copy_to_iter(iomap->inline_data + pos, length, iter); > > + copied = copy_to_iter(iomap->inline_data + pos - iomap->offset, > > + length, iter); > > We also need to take the offset into account for the write side. > I guess it would be nice to have a local variable for the inline > address to not duplicate that calculation multiple times. ok. Will update. Thanks, Gao Xiang
WARNING: multiple messages have this Message-ID (diff)
From: Gao Xiang <hsiangkao@linux.alibaba.com> To: Christoph Hellwig <hch@infradead.org> Cc: "Darrick J. Wong" <djwong@kernel.org>, Andreas Gr??nbacher <andreas.gruenbacher@gmail.com>, LKML <linux-kernel@vger.kernel.org>, Matthew Wilcox <willy@infradead.org>, Joseph Qi <joseph.qi@linux.alibaba.com>, Liu Bo <bo.liu@linux.alibaba.com>, Linux FS-devel Mailing List <linux-fsdevel@vger.kernel.org>, Liu Jiang <gerry@linux.alibaba.com>, linux-erofs@lists.ozlabs.org Subject: Re: [PATCH 1/2] iomap: support tail packing inline read Date: Mon, 19 Jul 2021 21:31:15 +0800 [thread overview] Message-ID: <YPV+o+V7wU32j6m3@B-P7TQMD6M-0146.local> (raw) In-Reply-To: <YPVe41YqpfGLNsBS@infradead.org> On Mon, Jul 19, 2021 at 12:15:47PM +0100, Christoph Hellwig wrote: > On Sat, Jul 17, 2021 at 09:38:18PM +0800, Gao Xiang wrote: > > >From 62f367245492e389711bcebbf7da5adae586299f Mon Sep 17 00:00:00 2001 > > From: Christoph Hellwig <hch@lst.de> > > Date: Fri, 16 Jul 2021 10:52:48 +0200 > > Subject: [PATCH] iomap: support tail packing inline read > > I'd still credit this to you as you did all the hard work. Ok. > > > + /* inline source data must be inside a single page */ > > + BUG_ON(iomap->length > PAGE_SIZE - offset_in_page(iomap->inline_data)); > > + /* handle tail-packing blocks cross the current page into the next */ > > + if (size > PAGE_SIZE - poff) > > + size = PAGE_SIZE - poff; > > This should probably use min or min_t. Okay, will update. > > > > > addr = kmap_atomic(page); > > - memcpy(addr, iomap->inline_data, size); > > - memset(addr + size, 0, PAGE_SIZE - size); > > + memcpy(addr + poff, iomap->inline_data - iomap->offset + pos, size); > > + memset(addr + poff + size, 0, PAGE_SIZE - poff - size); > > kunmap_atomic(addr); > > - SetPageUptodate(page); > > + flush_dcache_page(page); > > The flush_dcache_page addition should be a separate patch. I wondered what is the target order of recent iomap patches, since this is a quite small change, so I'd like to just rebase on for-next for now . So ok, I won't touch this in this patchset and I think flush_dcache_page() does no harm to x86(_64) and arm(64) at least if I remember correctly. > > > > > if (dio->flags & IOMAP_DIO_WRITE) { > > loff_t size = inode->i_size; > > @@ -394,7 +395,8 @@ iomap_dio_inline_actor(struct inode *inode, loff_t pos, loff_t length, > > mark_inode_dirty(inode); > > } > > } else { > > - copied = copy_to_iter(iomap->inline_data + pos, length, iter); > > + copied = copy_to_iter(iomap->inline_data + pos - iomap->offset, > > + length, iter); > > We also need to take the offset into account for the write side. > I guess it would be nice to have a local variable for the inline > address to not duplicate that calculation multiple times. ok. Will update. Thanks, Gao Xiang
next prev parent reply other threads:[~2021-07-19 13:31 UTC|newest] Thread overview: 39+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-07-16 5:07 [PATCH 0/2] erofs: iomap support for tailpacking cases Gao Xiang 2021-07-16 5:07 ` Gao Xiang 2021-07-16 5:07 ` [PATCH 1/2] iomap: support tail packing inline read Gao Xiang 2021-07-16 5:07 ` Gao Xiang 2021-07-16 9:19 ` Christoph Hellwig 2021-07-16 9:19 ` Christoph Hellwig 2021-07-16 9:46 ` Gao Xiang 2021-07-16 9:46 ` Gao Xiang 2021-07-16 13:47 ` Matthew Wilcox 2021-07-16 13:47 ` Matthew Wilcox 2021-07-16 14:38 ` Matthew Wilcox 2021-07-16 14:38 ` Matthew Wilcox 2021-07-16 13:02 ` Matthew Wilcox 2021-07-16 13:02 ` Matthew Wilcox 2021-07-16 13:56 ` Gao Xiang 2021-07-16 13:56 ` Gao Xiang 2021-07-16 14:44 ` Matthew Wilcox 2021-07-16 15:03 ` Gao Xiang 2021-07-16 15:03 ` Gao Xiang 2021-07-16 15:53 ` Andreas Grünbacher 2021-07-16 15:53 ` Andreas Grünbacher 2021-07-17 13:38 ` Gao Xiang 2021-07-17 13:38 ` Gao Xiang 2021-07-17 15:01 ` Matthew Wilcox 2021-07-17 15:15 ` Gao Xiang 2021-07-17 15:15 ` Gao Xiang 2021-07-17 18:40 ` Matthew Wilcox 2021-07-19 11:19 ` Christoph Hellwig 2021-07-19 11:19 ` Christoph Hellwig 2021-07-19 13:45 ` Gao Xiang 2021-07-19 13:45 ` Gao Xiang 2021-07-19 11:15 ` Christoph Hellwig 2021-07-19 13:31 ` Gao Xiang [this message] 2021-07-19 13:31 ` Gao Xiang 2021-07-16 5:07 ` [PATCH 2/2] erofs: convert all uncompressed cases to iomap Gao Xiang 2021-07-16 5:07 ` Gao Xiang 2021-07-18 5:30 ` kernel test robot 2021-07-18 5:30 ` [RFC PATCH] erofs: erofs_iomap_end() can be static kernel test robot 2021-07-18 16:53 ` Gao Xiang
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=YPV+o+V7wU32j6m3@B-P7TQMD6M-0146.local \ --to=hsiangkao@linux.alibaba.com \ --cc=andreas.gruenbacher@gmail.com \ --cc=bo.liu@linux.alibaba.com \ --cc=chao@kernel.org \ --cc=djwong@kernel.org \ --cc=gerry@linux.alibaba.com \ --cc=hch@infradead.org \ --cc=joseph.qi@linux.alibaba.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.