Re: freevxfs: hp-ux support. patchset r3 3/4

["Krzysztof Błaszkowski" <kb@sysmikro.com.pl>]

>From d3f13779deb42928ad1652604b941801fd8444cd Mon Sep 17 00:00:00 2001
From: KB <kb@sysmikro.com.pl>
Date: Wed, 1 Jun 2016 09:17:43 +0200
Subject: [PATCH 3/3] readdir fix

the patch makes algorithm of walking over directory records simple
because it is linear. The idea is to evaluate page relative number,
file system block number inside the mapped page, and finally current
directory record offset from single index (pos or ctx->pos) advanced 
by size of directory record (and the length of dirblk record at 
beginning of every fs block). Thus the inner "for" (old code)
is not necessary. Furthermore, lack of the inner for fixes the issue
with accessing kaddr one block size beyond mapped page.
correct "for" should look like this:
 "for ( ; ... block < pblocks ; block++)" : "<" vs "<="
let's suppose pblocks is 4, thus last block would be 4 (old "for")
which stands for that 5th block would be parsed and this resulted in
adding some random garbage to e.g. ls output. it used to happen that
some files were missing in the directory list.

anyway the "for" is no longer needed and the vxfs directory holding
almost 2900 files was useful to verify the readdir().


Signed-off-by: Krzysztof Błaszkowski <kb@sysmikro.com.pl>
---
 fs/freevxfs/vxfs_lookup.c |  268 +++++++++++++++++++++------------------------
 1 files changed, 125 insertions(+), 143 deletions(-)

diff --git a/fs/freevxfs/vxfs_lookup.c b/fs/freevxfs/vxfs_lookup.c
index 31bcebe..c9de653 100644
--- a/fs/freevxfs/vxfs_lookup.c
+++ b/fs/freevxfs/vxfs_lookup.c
@@ -61,42 +61,8 @@ const struct file_operations vxfs_dir_operations = {
 	.iterate =		vxfs_readdir,
 };
 
- 
-static inline u_long
-dir_pages(struct inode *inode)
-{
-	return (inode->i_size + PAGE_CACHE_SIZE - 1) >> PAGE_CACHE_SHIFT;
-}
- 
-static inline u_long
-dir_blocks(struct inode *ip)
-{
-	u_long			bsize = ip->i_sb->s_blocksize;
-	return (ip->i_size + bsize - 1) & ~(bsize - 1);
-}
 
-/*
- * NOTE! unlike strncmp, vxfs_match returns 1 for success, 0 for failure.
- *
- * len <= VXFS_NAMELEN and de != NULL are guaranteed by caller.
- */
-static inline int
-vxfs_match(struct vxfs_sb_info *sbi, int len, const char *const name,
-		struct vxfs_direct *de)
-{
-	if (len != fs16_to_cpu(sbi, de->d_namelen))
-		return 0;
-	if (!de->d_ino)
-		return 0;
-	return !memcmp(name, de->d_name, len);
-}
 
-static inline struct vxfs_direct *
-vxfs_next_entry(struct vxfs_sb_info *sbi, struct vxfs_direct *de)
-{
-	return ((struct vxfs_direct *)
-		((char *)de + fs16_to_cpu(sbi, de->d_reclen)));
-}
 
 /**
  * vxfs_find_entry - find a mathing directory entry for a dentry
@@ -115,53 +81,65 @@ vxfs_next_entry(struct vxfs_sb_info *sbi, struct vxfs_direct *de)
 static struct vxfs_direct *
 vxfs_find_entry(struct inode *ip, struct dentry *dp, struct page **ppp)
 {
-	struct vxfs_sb_info		*sbi = VXFS_SBI(ip->i_sb);
-	u_long				npages, page, nblocks, pblocks, block;
-	u_long				bsize = ip->i_sb->s_blocksize;
-	const char			*name = dp->d_name.name;
-	int				namelen = dp->d_name.len;
-
-	npages = dir_pages(ip);
-	nblocks = dir_blocks(ip);
-	pblocks = VXFS_BLOCK_PER_PAGE(ip->i_sb);
-	
-	for (page = 0; page < npages; page++) {
-		caddr_t			kaddr;
-		struct page		*pp;
-
-		pp = vxfs_get_page(ip->i_mapping, page);
-		if (IS_ERR(pp))
-			continue;
-		kaddr = (caddr_t)page_address(pp);
-
-		for (block = 0; block <= nblocks && block <= pblocks; block++) {
-			caddr_t			baddr, limit;
-			struct vxfs_dirblk	*dbp;
-			struct vxfs_direct	*de;
-
-			baddr = kaddr + (block * bsize);
-			limit = baddr + bsize - VXFS_DIRLEN(1);
-			
-			dbp = (struct vxfs_dirblk *)baddr;
-			de = (struct vxfs_direct *)
-				(baddr + VXFS_DIRBLKOV(sbi, dbp));
-
-			for (; (caddr_t)de <= limit;
-					de = vxfs_next_entry(sbi, de)) {
-				if (!de->d_reclen)
-					break;
-				if (!de->d_ino)
-					continue;
-				if (vxfs_match(sbi, namelen, name, de)) {
-					*ppp = pp;
-					return (de);
-				}
+	u_long bsize = ip->i_sb->s_blocksize;
+	const char *name = dp->d_name.name;
+	int namelen = dp->d_name.len;
+	loff_t limit = VXFS_DIRROUND(ip->i_size);
+	struct vxfs_direct *de_exit = NULL;
+	loff_t pos = 0;
+	struct vxfs_sb_info *sbi = VXFS_SBI(ip->i_sb);
+
+	while (pos < limit) {
+		struct page *pp;
+		char *kaddr;
+		int pg_ofs = pos & ~PAGE_CACHE_MASK;
+
+		pp = vxfs_get_page(ip->i_mapping, pos >> PAGE_CACHE_SHIFT);
+		if (IS_ERR(pp)) {
+			return NULL;
+		}
+		kaddr = (char *)page_address(pp);
+
+		while (pg_ofs < PAGE_SIZE && pos < limit) {
+			struct vxfs_direct *de;
+
+			if ((pos & (bsize - 1)) < 4) {
+				struct vxfs_dirblk *dbp =
+				    (struct vxfs_dirblk *)(kaddr + (pos & ~PAGE_CACHE_MASK));
+				int overhead = (sizeof(short) * fs16_to_cpu(sbi, dbp->d_nhash)) + 4;
+
+				pos += overhead;
+				pg_ofs += overhead;
+			}
+			de = (struct vxfs_direct *)(kaddr + pg_ofs);
+
+			if (!de->d_reclen) {
+				pos += bsize - 1;
+				pos &= ~(bsize - 1);
+				break;
+			}
+
+			pg_ofs += fs16_to_cpu(sbi, de->d_reclen);
+			pos += fs16_to_cpu(sbi, de->d_reclen);
+			if (!de->d_ino) {
+				continue;
+			}
+
+			if (namelen != fs16_to_cpu(sbi, de->d_namelen))
+				continue;
+			if (!memcmp(name, de->d_name, namelen)) {
+				*ppp = pp;
+				de_exit = de;
+				break;
 			}
 		}
-		vxfs_put_page(pp);
+		if (!de_exit)
+			vxfs_put_page(pp);
+		else
+			break;
 	}
 
-	return NULL;
+	return de_exit;
 }
 
 /**
@@ -181,7 +159,7 @@ vxfs_inode_by_name(struct inode *dip, struct dentry *dp)
 {
 	struct vxfs_direct		*de;
 	struct page			*pp;
-	ino_t				ino = 0;
+	ino_t ino = 0;
 
 	de = vxfs_find_entry(dip, dp, &pp);
 	if (de) {
@@ -189,7 +167,7 @@ vxfs_inode_by_name(struct inode *dip, struct dentry *dp)
 		kunmap(pp);
 		page_cache_release(pp);
 	}
-	
+
 	return (ino);
 }
 
@@ -212,17 +190,17 @@ vxfs_lookup(struct inode *dip, struct dentry *dp, unsigned int flags)
 {
 	struct inode		*ip = NULL;
 	ino_t			ino;
-			 
+
 	if (dp->d_name.len > VXFS_NAMELEN)
 		return ERR_PTR(-ENAMETOOLONG);
-				 
+
 	ino = vxfs_inode_by_name(dip, dp);
 	if (ino) {
 		ip = vxfs_iget(dip->i_sb, ino);
 		if (IS_ERR(ip))
 			return ERR_CAST(ip);
+		d_add(dp, ip);
 	}
-	d_add(dp, ip);
 	return NULL;
 }
 
@@ -244,80 +222,84 @@ vxfs_readdir(struct file *fp, struct dir_context *ctx)
 {
 	struct inode		*ip = file_inode(fp);
 	struct super_block	*sbp = ip->i_sb;
-	struct vxfs_sb_info	*sbi = VXFS_SBI(sbp);
 	u_long			bsize = sbp->s_blocksize;
-	u_long			page, npages, block, pblocks, nblocks, offset;
-	loff_t			pos;
-
+	loff_t			pos, limit;
+	struct vxfs_sb_info	*sbi = VXFS_SBI(sbp);
 
 	if (ctx->pos == 0) {
 		if (!dir_emit_dot(fp, ctx))
-			return 0;
-		ctx->pos = 1;
+			goto out;
+		ctx->pos++;
 	}
 	if (ctx->pos == 1) {
 		if (!dir_emit(ctx, "..", 2, VXFS_INO(ip)->vii_dotdot, DT_DIR))
-			return 0;
-		ctx->pos = 2;
+			goto out;
+		ctx->pos++;
+	}
+
+	limit = VXFS_DIRROUND(ip->i_size);
+	if (ctx->pos > limit) {
+#if 0
+		ctx->pos = 0;
+#endif
+		goto out;
 	}
-	pos = ctx->pos - 2;
-	
-	if (pos > VXFS_DIRROUND(ip->i_size))
-		return 0;
-
-	npages = dir_pages(ip);
-	nblocks = dir_blocks(ip);
-	pblocks = VXFS_BLOCK_PER_PAGE(sbp);
-
-	page = pos >> PAGE_CACHE_SHIFT;
-	offset = pos & ~PAGE_CACHE_MASK;
-	block = (u_long)(pos >> sbp->s_blocksize_bits) % pblocks;
-
-	for (; page < npages; page++, block = 0) {
-		char			*kaddr;
-		struct page		*pp;
-
-		pp = vxfs_get_page(ip->i_mapping, page);
-		if (IS_ERR(pp))
-			continue;
+
+	pos = ctx->pos & ~3L;
+
+	while (pos < limit) {
+		struct page *pp;
+		char *kaddr;
+		int pg_ofs = pos & ~PAGE_CACHE_MASK;
+		int rc = 0;
+
+		pp = vxfs_get_page(ip->i_mapping, pos >> PAGE_CACHE_SHIFT);
+		if (IS_ERR(pp)) {
+			return -ENOMEM;
+		}
 		kaddr = (char *)page_address(pp);
 
-		for (; block <= nblocks && block <= pblocks; block++) {
-			char			*baddr, *limit;
-			struct vxfs_dirblk	*dbp;
-			struct vxfs_direct	*de;
-
-			baddr = kaddr + (block * bsize);
-			limit = baddr + bsize - VXFS_DIRLEN(1);
-	
-			dbp = (struct vxfs_dirblk *)baddr;
-			de = (struct vxfs_direct *)
-				(offset ?
-				 (kaddr + offset) :
-				 (baddr + VXFS_DIRBLKOV(sbi, dbp)));
-
-			for (; (char *)de <= limit;
-					de = vxfs_next_entry(sbi, de)) {
-				if (!de->d_reclen)
-					break;
-				if (!de->d_ino)
-					continue;
-
-				offset = (char *)de - kaddr;
-				ctx->pos = ((page << PAGE_CACHE_SHIFT) | offset) + 2;
-				if (!dir_emit(ctx, de->d_name,
-						fs16_to_cpu(sbi, de->d_namelen),
-						fs32_to_cpu(sbi, de->d_ino),
-						DT_UNKNOWN)) {
-					vxfs_put_page(pp);
-					return 0;
-				}
+		while (pg_ofs < PAGE_SIZE && pos < limit) {
+			struct vxfs_direct *de;
+
+			if ((pos & (bsize - 1)) < 4) {
+				struct vxfs_dirblk *dbp =
+				    (struct vxfs_dirblk *)(kaddr + (pos & ~PAGE_CACHE_MASK));
+				int overhead = (sizeof(short) * fs16_to_cpu(sbi, dbp->d_nhash)) + 4;
+
+				pos += overhead;
+				pg_ofs += overhead;
+			}
+			de = (struct vxfs_direct *)(kaddr + pg_ofs);
+
+			if (!de->d_reclen) {
+				pos += bsize - 1;
+				pos &= ~(bsize - 1);
+				break;
+			}
+
+			pg_ofs += fs16_to_cpu(sbi, de->d_reclen);
+			pos += fs16_to_cpu(sbi, de->d_reclen);
+			if (!de->d_ino) {
+				continue;
+			}
+
+			rc = dir_emit(ctx, de->d_name, fs16_to_cpu(sbi, de->d_namelen),
+				fs32_to_cpu(sbi, de->d_ino), DT_UNKNOWN);
+			if (!rc) {
+				/* the dir entry was not submitted, so fix pos. */
+				pos -= fs16_to_cpu(sbi, de->d_reclen);
+				break;
 			}
-			offset = 0;
 		}
 		vxfs_put_page(pp);
-		offset = 0;
+		if (!rc)
+			break;
 	}
-	ctx->pos = ((page << PAGE_CACHE_SHIFT) | offset) + 2;
+
+	ctx->pos = pos | 2;
+
+out:
 	return 0;
 }
+
-- 
1.7.3.4



> 
> Thanks. yes, the old readdir has a bug. this time my change logs are
> more verbose.
> 
> 
> -- 
> Krzysztof Blaszkowski
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Krzysztof Blaszkowski