[v3,2/2] lib/scatterlist: use page iterator in the mapping iterator

Message ID 1360768224-18163-2-git-send-email-imre.deak@intel.com
State New
Headers show

Commit Message

Imre Deak Feb. 13, 2013, 3:10 p.m.
For better code reuse use the newly added page iterator to iterate
through the pages. The offset, length within the page is still
calculated by the mapping iterator as well as the actual mapping.
Idea from Tejun Heo <tj@kernel.org>.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 include/linux/scatterlist.h |    6 +++---
 lib/scatterlist.c           |   46 ++++++++++++++++++++-----------------------
 2 files changed, 24 insertions(+), 28 deletions(-)

Comments

Stephen Warren Feb. 23, 2013, 4:29 a.m. | #1
On 02/13/2013 08:10 AM, Imre Deak wrote:
> For better code reuse use the newly added page iterator to iterate
> through the pages. The offset, length within the page is still
> calculated by the mapping iterator as well as the actual mapping.
> Idea from Tejun Heo <tj@kernel.org>.

This patch appears in linux-next since next-20130220. It breaks mounting
a root filesystem on an SD card on the Raspberry Pi ARM platform, with
errors such as those shown below.

next-20130222 with just this patch reverted works fine.

> [    0.708426] VFS: Mounted root (ext4 filesystem) on device 179:2.
> [    0.723742] devtmpfs: mounted
> [    0.733064] Freeing init memory: 204K
> [    0.777992] EXT4-fs error (device mmcblk0p2): ext4_iget:3814: inode #4259: comm swapper: bad extra_isize (57200 != 256)
> [    0.815172] EXT4-fs error (device mmcblk0p2): ext4_lookup:1428: inode #8198: comm swapper: deleted inode referenced: 487
> [    0.826179] Kernel panic - not syncing: No init found.  Try passing init= option to kernel. See Linux Documentation/init.txt for guidance.

> [    0.719365] VFS: Mounted root (ext4 filesystem) on device 179:2.
> [    0.740918] devtmpfs: mounted
> [    0.745219] Freeing init memory: 204K
> ERROR: ld.so: object '/usr/lib/arm-linux-gnueabihf/libcofi_rpi.so' from /etc/ld.so.preload cannot be preloaded: ignored.
> [    0.906840] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b

> [    0.724018] VFS: Mounted root (ext4 filesystem) on device 179:2.
> [    0.739404] devtmpfs: mounted
> [    0.748741] Freeing init memory: 204K
> [    0.793603] EXT4-fs error (device mmcblk0p2): ext4_iget:3814: inode #4259: comm swapper: bad extra_isize (57200 != 256)
> [    0.822138] Kernel panic - not syncing: No init found.  Try passing init= option to kernel. See Linux Documentation/init.txt for guidance.
Stephen Warren Feb. 23, 2013, 11:45 p.m. | #2
On Sat, 23 Feb 2013 22:04:06 +0200, Imre Deak wrote:
> On Fri, 2013-02-22 at 21:29 -0700, Stephen Warren wrote:
>> On 02/13/2013 08:10 AM, Imre Deak wrote:
>>> For better code reuse use the newly added page iterator to iterate
>>> through the pages. The offset, length within the page is still
>>> calculated by the mapping iterator as well as the actual mapping.
>>> Idea from Tejun Heo <tj@kernel.org>.
>>
>> This patch appears in linux-next since next-20130220. It breaks mounting
>> a root filesystem on an SD card on the Raspberry Pi ARM platform, with
>> errors such as those shown below.
>>
>> next-20130222 with just this patch reverted works fine.
> 
> ...
> the following might fix the
> problem you saw. Could you give it a try? It applies on top of
> v4 of the patch [1]:

Yes, thanks very much. That incremental patch you gave solves the
problem perfectly.

Patch

diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
index 788a853..a6cd692 100644
--- a/include/linux/scatterlist.h
+++ b/include/linux/scatterlist.h
@@ -295,9 +295,9 @@  struct sg_mapping_iter {
 	size_t			consumed;	/* number of consumed bytes */
 
 	/* these are internal states, keep away */
-	struct scatterlist	*__sg;		/* current entry */
-	unsigned int		__nents;	/* nr of remaining entries */
-	unsigned int		__offset;	/* offset within sg */
+	unsigned int		__offset;	/* offset within page */
+	struct sg_page_iter	__piter;	/* page iterator */
+	unsigned int		__remaining;	/* remaining bytes on page */
 	unsigned int		__flags;
 };
 
diff --git a/lib/scatterlist.c b/lib/scatterlist.c
index a1d1564..4e4974a 100644
--- a/lib/scatterlist.c
+++ b/lib/scatterlist.c
@@ -449,9 +449,7 @@  void sg_miter_start(struct sg_mapping_iter *miter, struct scatterlist *sgl,
 {
 	memset(miter, 0, sizeof(struct sg_mapping_iter));
 
-	miter->__sg = sgl;
-	miter->__nents = nents;
-	miter->__offset = 0;
+	__sg_page_iter_start(&miter->__piter, sgl, nents, 0);
 	WARN_ON(!(flags & (SG_MITER_TO_SG | SG_MITER_FROM_SG)));
 	miter->__flags = flags;
 }
@@ -476,36 +474,33 @@  EXPORT_SYMBOL(sg_miter_start);
  */
 bool sg_miter_next(struct sg_mapping_iter *miter)
 {
-	unsigned int off, len;
-
-	/* check for end and drop resources from the last iteration */
-	if (!miter->__nents)
-		return false;
-
 	sg_miter_stop(miter);
 
-	/* get to the next sg if necessary.  __offset is adjusted by stop */
-	while (miter->__offset == miter->__sg->length) {
-		if (--miter->__nents) {
-			miter->__sg = sg_next(miter->__sg);
-			miter->__offset = 0;
-		} else
+	/*
+	 * Get to the next page if necessary.
+	 * __remaining, __offset is adjusted by sg_miter_stop
+	 */
+	if (!miter->__remaining) {
+		struct scatterlist *sg;
+		unsigned long pgoffset;
+
+		if (!__sg_page_iter_next(&miter->__piter))
 			return false;
-	}
 
-	/* map the next page */
-	off = miter->__sg->offset + miter->__offset;
-	len = miter->__sg->length - miter->__offset;
+		sg = miter->__piter.sg;
+		pgoffset = miter->__piter.sg_pgoffset;
 
-	miter->page = nth_page(sg_page(miter->__sg), off >> PAGE_SHIFT);
-	off &= ~PAGE_MASK;
-	miter->length = min_t(unsigned int, len, PAGE_SIZE - off);
-	miter->consumed = miter->length;
+		miter->__offset = pgoffset ? 0 : sg->offset;
+		miter->__remaining = sg->offset + sg->length -
+				(pgoffset << PAGE_SHIFT) - miter->__offset;
+	}
+	miter->page = miter->__piter.page;
+	miter->consumed = miter->length = miter->__remaining;
 
 	if (miter->__flags & SG_MITER_ATOMIC)
-		miter->addr = kmap_atomic(miter->page) + off;
+		miter->addr = kmap_atomic(miter->page) + miter->__offset;
 	else
-		miter->addr = kmap(miter->page) + off;
+		miter->addr = kmap(miter->page) + miter->__offset;
 
 	return true;
 }
@@ -532,6 +527,7 @@  void sg_miter_stop(struct sg_mapping_iter *miter)
 	/* drop resources from the last iteration */
 	if (miter->addr) {
 		miter->__offset += miter->consumed;
+		miter->__remaining -= miter->consumed;
 
 		if (miter->__flags & SG_MITER_TO_SG)
 			flush_kernel_dcache_page(miter->page);