[PATCH 16/41] whiteout: jffs2 whiteout support

[PATCH 16/41] whiteout: jffs2 whiteout support

[Valerie Aurora]

From: Felix Fietkau <nbd@openwrt.org>

Add support for whiteout dentries to jffs2.

Signed-off-by: Felix Fietkau <nbd@openwrt.org>
Signed-off-by: Valerie Aurora <vaurora@redhat.com>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: linux-mtd@lists.infradead.org
---
 fs/jffs2/dir.c        |   77 +++++++++++++++++++++++++++++++++++++++++++++++-
 fs/jffs2/fs.c         |    4 ++
 fs/jffs2/super.c      |    2 +-
 include/linux/jffs2.h |    2 +
 4 files changed, 82 insertions(+), 3 deletions(-)

diff --git a/fs/jffs2/dir.c b/fs/jffs2/dir.c
index 6f60cc9..46a2e1b 100644
--- a/fs/jffs2/dir.c
+++ b/fs/jffs2/dir.c
@@ -34,6 +34,8 @@ static int jffs2_mknod (struct inode *,struct dentry *,int,dev_t);
 static int jffs2_rename (struct inode *, struct dentry *,
 			 struct inode *, struct dentry *);
 
+static int jffs2_whiteout (struct inode *, struct dentry *, struct dentry *);
+
 const struct file_operations jffs2_dir_operations =
 {
 	.read =		generic_read_dir,
@@ -55,6 +57,7 @@ const struct inode_operations jffs2_dir_inode_operations =
 	.rmdir =	jffs2_rmdir,
 	.mknod =	jffs2_mknod,
 	.rename =	jffs2_rename,
+	.whiteout =     jffs2_whiteout,
 	.permission =	jffs2_permission,
 	.setattr =	jffs2_setattr,
 	.setxattr =	jffs2_setxattr,
@@ -98,8 +101,18 @@ static struct dentry *jffs2_lookup(struct inode *dir_i, struct dentry *target,
 			fd = fd_list;
 		}
 	}
-	if (fd)
-		ino = fd->ino;
+	if (fd) {
+		spin_lock(&target->d_lock);
+		switch(fd->type) {
+		case DT_WHT:
+			target->d_flags |= DCACHE_WHITEOUT;
+			break;
+		default:
+			ino = fd->ino;
+			break;
+		}
+		spin_unlock(&target->d_lock);
+	}
 	mutex_unlock(&dir_f->sem);
 	if (ino) {
 		inode = jffs2_iget(dir_i->i_sb, ino);
@@ -498,6 +511,11 @@ static int jffs2_mkdir (struct inode *dir_i, struct dentry *dentry, int mode)
 		return PTR_ERR(inode);
 	}
 
+	if (dentry->d_flags & DCACHE_WHITEOUT) {
+		inode->i_flags |= S_OPAQUE;
+		ri->flags = cpu_to_je16(JFFS2_INO_FLAG_OPAQUE);
+	}
+
 	inode->i_op = &jffs2_dir_inode_operations;
 	inode->i_fop = &jffs2_dir_operations;
 
@@ -779,6 +797,61 @@ static int jffs2_mknod (struct inode *dir_i, struct dentry *dentry, int mode, de
 	return 0;
 }
 
+static int jffs2_whiteout (struct inode *dir, struct dentry *old_dentry,
+			   struct dentry *new_dentry)
+{
+	struct jffs2_sb_info *c = JFFS2_SB_INFO(dir->i_sb);
+	struct jffs2_inode_info *victim_f = NULL;
+	uint32_t now;
+	int ret;
+
+	/* If it's a directory, then check whether it is really empty
+	 */
+	if (new_dentry->d_inode) {
+		victim_f = JFFS2_INODE_INFO(old_dentry->d_inode);
+		if (S_ISDIR(old_dentry->d_inode->i_mode)) {
+			struct jffs2_full_dirent *fd;
+
+			mutex_lock(&victim_f->sem);
+			for (fd = victim_f->dents; fd; fd = fd->next) {
+				if (fd->ino) {
+					mutex_unlock(&victim_f->sem);
+					return -ENOTEMPTY;
+				}
+			}
+			mutex_unlock(&victim_f->sem);
+		}
+	}
+
+	now = get_seconds();
+	ret = jffs2_do_link(c, JFFS2_INODE_INFO(dir), 0, DT_WHT,
+			    new_dentry->d_name.name, new_dentry->d_name.len, now);
+	if (ret)
+		return ret;
+
+	spin_lock(&new_dentry->d_lock);
+	new_dentry->d_flags |= DCACHE_WHITEOUT;
+	spin_unlock(&new_dentry->d_lock);
+	d_add(new_dentry, NULL);
+
+	if (victim_f) {
+		/* There was a victim. Kill it off nicely */
+		drop_nlink(old_dentry->d_inode);
+		/* Don't oops if the victim was a dirent pointing to an
+		   inode which didn't exist. */
+		if (victim_f->inocache) {
+			mutex_lock(&victim_f->sem);
+			if (S_ISDIR(old_dentry->d_inode->i_mode))
+				victim_f->inocache->pino_nlink = 0;
+			else
+				victim_f->inocache->pino_nlink--;
+			mutex_unlock(&victim_f->sem);
+		}
+	}
+
+	return 0;
+}
+
 static int jffs2_rename (struct inode *old_dir_i, struct dentry *old_dentry,
 			 struct inode *new_dir_i, struct dentry *new_dentry)
 {
diff --git a/fs/jffs2/fs.c b/fs/jffs2/fs.c
index 3451a81..c1e333c 100644
--- a/fs/jffs2/fs.c
+++ b/fs/jffs2/fs.c
@@ -301,6 +301,10 @@ struct inode *jffs2_iget(struct super_block *sb, unsigned long ino)
 
 		inode->i_op = &jffs2_dir_inode_operations;
 		inode->i_fop = &jffs2_dir_operations;
+
+		if (je16_to_cpu(latest_node.flags) & JFFS2_INO_FLAG_OPAQUE)
+			inode->i_flags |= S_OPAQUE;
+
 		break;
 	}
 	case S_IFREG:
diff --git a/fs/jffs2/super.c b/fs/jffs2/super.c
index 0035c02..6607f0b 100644
--- a/fs/jffs2/super.c
+++ b/fs/jffs2/super.c
@@ -172,7 +172,7 @@ static int jffs2_fill_super(struct super_block *sb, void *data, int silent)
 
 	sb->s_op = &jffs2_super_operations;
 	sb->s_export_op = &jffs2_export_ops;
-	sb->s_flags = sb->s_flags | MS_NOATIME;
+	sb->s_flags = sb->s_flags | MS_NOATIME | MS_WHITEOUT;
 	sb->s_xattr = jffs2_xattr_handlers;
 #ifdef CONFIG_JFFS2_FS_POSIX_ACL
 	sb->s_flags |= MS_POSIXACL;
diff --git a/include/linux/jffs2.h b/include/linux/jffs2.h
index 2b32d63..65533bb 100644
--- a/include/linux/jffs2.h
+++ b/include/linux/jffs2.h
@@ -87,6 +87,8 @@
 #define JFFS2_INO_FLAG_USERCOMPR  2	/* User has requested a specific
 					   compression type */
 
+#define JFFS2_INO_FLAG_OPAQUE     4	/* Directory is opaque (for union mounts) */
+
 
 /* These can go once we've made sure we've caught all uses without
    byteswapping */
-- 
1.6.3.3

[PATCH 33/41] fallthru: jffs2 fallthru support

[Valerie Aurora]

From: Felix Fietkau <nbd@openwrt.org>

Add support for fallthru dentries to jffs2.

Cc: David Woodhouse <dwmw2@infradead.org>
Cc: linux-mtd@lists.infradead.org
Signed-off-by: Felix Fietkau <nbd@openwrt.org>
Signed-off-by: Valerie Aurora <vaurora@redhat.com>
---
 fs/jffs2/dir.c        |   31 ++++++++++++++++++++++++++++++-
 include/linux/jffs2.h |    6 ++++++
 2 files changed, 36 insertions(+), 1 deletions(-)

diff --git a/fs/jffs2/dir.c b/fs/jffs2/dir.c
index 46a2e1b..544d6c5 100644
--- a/fs/jffs2/dir.c
+++ b/fs/jffs2/dir.c
@@ -35,6 +35,7 @@ static int jffs2_rename (struct inode *, struct dentry *,
 			 struct inode *, struct dentry *);
 
 static int jffs2_whiteout (struct inode *, struct dentry *, struct dentry *);
+static int jffs2_fallthru (struct inode *, struct dentry *);
 
 const struct file_operations jffs2_dir_operations =
 {
@@ -57,6 +58,7 @@ const struct inode_operations jffs2_dir_inode_operations =
 	.rmdir =	jffs2_rmdir,
 	.mknod =	jffs2_mknod,
 	.rename =	jffs2_rename,
+	.fallthru =     jffs2_fallthru,
 	.whiteout =     jffs2_whiteout,
 	.permission =	jffs2_permission,
 	.setattr =	jffs2_setattr,
@@ -107,6 +109,9 @@ static struct dentry *jffs2_lookup(struct inode *dir_i, struct dentry *target,
 		case DT_WHT:
 			target->d_flags |= DCACHE_WHITEOUT;
 			break;
+		case JFFS2_DT_FALLTHRU:
+			target->d_flags |= DCACHE_FALLTHRU;
+			break;
 		default:
 			ino = fd->ino;
 			break;
@@ -168,7 +173,10 @@ static int jffs2_readdir(struct file *filp, void *dirent, filldir_t filldir)
 				  fd->name, fd->ino, fd->type, curofs, offset));
 			continue;
 		}
-		if (!fd->ino) {
+		if (fd->type == JFFS2_DT_FALLTHRU)
+			/* XXX Should really do a lookup for the real inode number here */
+			fd->ino = 100;
+		else if (!fd->ino && (fd->type != DT_WHT)) {
 			D2(printk(KERN_DEBUG "Skipping deletion dirent \"%s\"\n", fd->name));
 			offset++;
 			continue;
@@ -797,6 +805,26 @@ static int jffs2_mknod (struct inode *dir_i, struct dentry *dentry, int mode, de
 	return 0;
 }
 
+static int jffs2_fallthru (struct inode *dir, struct dentry *dentry)
+{
+	struct jffs2_sb_info *c = JFFS2_SB_INFO(dir->i_sb);
+	uint32_t now;
+	int ret;
+
+	now = get_seconds();
+	ret = jffs2_do_link(c, JFFS2_INODE_INFO(dir), 0, DT_UNKNOWN,
+			    dentry->d_name.name, dentry->d_name.len, now);
+	if (ret)
+		return ret;
+
+	d_instantiate(dentry, NULL);
+	spin_lock(&dentry->d_lock);
+	dentry->d_flags |= DCACHE_FALLTHRU;
+	spin_unlock(&dentry->d_lock);
+
+	return 0;
+}
+
 static int jffs2_whiteout (struct inode *dir, struct dentry *old_dentry,
 			   struct dentry *new_dentry)
 {
@@ -830,6 +858,7 @@ static int jffs2_whiteout (struct inode *dir, struct dentry *old_dentry,
 		return ret;
 
 	spin_lock(&new_dentry->d_lock);
+	new_dentry->d_flags &= ~DCACHE_FALLTHRU;
 	new_dentry->d_flags |= DCACHE_WHITEOUT;
 	spin_unlock(&new_dentry->d_lock);
 	d_add(new_dentry, NULL);
diff --git a/include/linux/jffs2.h b/include/linux/jffs2.h
index 65533bb..dbe8c93 100644
--- a/include/linux/jffs2.h
+++ b/include/linux/jffs2.h
@@ -114,6 +114,12 @@ struct jffs2_unknown_node
 	jint32_t hdr_crc;
 };
 
+/*
+ * Non-standard directory entry type(s), for on-disk use
+ */
+
+#define                JFFS2_DT_FALLTHRU       (DT_WHT + 1)
+
 struct jffs2_raw_dirent
 {
 	jint16_t magic;
-- 
1.6.3.3

Re: [PATCH 33/41] fallthru: jffs2 fallthru support

[Erez Zadok]

In message <1256152779-10054-34-git-send-email-vaurora@redhat.com>, Valerie Aurora writes:
> From: Felix Fietkau <nbd@openwrt.org>
> 
> Add support for fallthru dentries to jffs2.
> 
> Cc: David Woodhouse <dwmw2@infradead.org>
> Cc: linux-mtd@lists.infradead.org
> Signed-off-by: Felix Fietkau <nbd@openwrt.org>
> Signed-off-by: Valerie Aurora <vaurora@redhat.com>
> ---
>  fs/jffs2/dir.c        |   31 ++++++++++++++++++++++++++++++-
>  include/linux/jffs2.h |    6 ++++++
>  2 files changed, 36 insertions(+), 1 deletions(-)
> 
> diff --git a/fs/jffs2/dir.c b/fs/jffs2/dir.c
> index 46a2e1b..544d6c5 100644
> --- a/fs/jffs2/dir.c
> +++ b/fs/jffs2/dir.c
> @@ -35,6 +35,7 @@ static int jffs2_rename (struct inode *, struct dentry *,
>  			 struct inode *, struct dentry *);
>  
>  static int jffs2_whiteout (struct inode *, struct dentry *, struct dentry *);
> +static int jffs2_fallthru (struct inode *, struct dentry *);
>  
>  const struct file_operations jffs2_dir_operations =
>  {
> @@ -57,6 +58,7 @@ const struct inode_operations jffs2_dir_inode_operations =
>  	.rmdir =	jffs2_rmdir,
>  	.mknod =	jffs2_mknod,
>  	.rename =	jffs2_rename,
> +	.fallthru =     jffs2_fallthru,
>  	.whiteout =     jffs2_whiteout,
>  	.permission =	jffs2_permission,
>  	.setattr =	jffs2_setattr,
> @@ -107,6 +109,9 @@ static struct dentry *jffs2_lookup(struct inode *dir_i, struct dentry *target,
>  		case DT_WHT:
>  			target->d_flags |= DCACHE_WHITEOUT;
>  			break;
> +		case JFFS2_DT_FALLTHRU:
> +			target->d_flags |= DCACHE_FALLTHRU;
> +			break;
>  		default:
>  			ino = fd->ino;
>  			break;
> @@ -168,7 +173,10 @@ static int jffs2_readdir(struct file *filp, void *dirent, filldir_t filldir)
>  				  fd->name, fd->ino, fd->type, curofs, offset));
>  			continue;
>  		}
> -		if (!fd->ino) {
> +		if (fd->type == JFFS2_DT_FALLTHRU)
> +			/* XXX Should really do a lookup for the real inode number here */
> +			fd->ino = 100;

In the ext2 patch, it was ino=123, here it's 100.  Is there a consistently
useful reserved number to use instead, for jffs2 as well?  If not, maybe at
least we can pick one random inode number and use it for all default inode
numbers for ext2, jffs2, etc.?

> +		else if (!fd->ino && (fd->type != DT_WHT)) {
>  			D2(printk(KERN_DEBUG "Skipping deletion dirent \"%s\"\n", fd->name));
>  			offset++;
>  			continue;
> @@ -797,6 +805,26 @@ static int jffs2_mknod (struct inode *dir_i, struct dentry *dentry, int mode, de
>  	return 0;
>  }
>  
> +static int jffs2_fallthru (struct inode *dir, struct dentry *dentry)
> +{
> +	struct jffs2_sb_info *c = JFFS2_SB_INFO(dir->i_sb);
> +	uint32_t now;
> +	int ret;
> +
> +	now = get_seconds();
> +	ret = jffs2_do_link(c, JFFS2_INODE_INFO(dir), 0, DT_UNKNOWN,
> +			    dentry->d_name.name, dentry->d_name.len, now);
> +	if (ret)
> +		return ret;
> +
> +	d_instantiate(dentry, NULL);
> +	spin_lock(&dentry->d_lock);
> +	dentry->d_flags |= DCACHE_FALLTHRU;
> +	spin_unlock(&dentry->d_lock);
> +
> +	return 0;
> +}
> +
>  static int jffs2_whiteout (struct inode *dir, struct dentry *old_dentry,
>  			   struct dentry *new_dentry)
>  {
> @@ -830,6 +858,7 @@ static int jffs2_whiteout (struct inode *dir, struct dentry *old_dentry,
>  		return ret;
>  
>  	spin_lock(&new_dentry->d_lock);
> +	new_dentry->d_flags &= ~DCACHE_FALLTHRU;
>  	new_dentry->d_flags |= DCACHE_WHITEOUT;
>  	spin_unlock(&new_dentry->d_lock);
>  	d_add(new_dentry, NULL);
> diff --git a/include/linux/jffs2.h b/include/linux/jffs2.h
> index 65533bb..dbe8c93 100644
> --- a/include/linux/jffs2.h
> +++ b/include/linux/jffs2.h
> @@ -114,6 +114,12 @@ struct jffs2_unknown_node
>  	jint32_t hdr_crc;
>  };
>  
> +/*
> + * Non-standard directory entry type(s), for on-disk use
> + */
> +
> +#define                JFFS2_DT_FALLTHRU       (DT_WHT + 1)
> +
>  struct jffs2_raw_dirent
>  {
>  	jint16_t magic;
> -- 
> 1.6.3.3
> 
> --
> 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

Erez.

Re: [PATCH 16/41] whiteout: jffs2 whiteout support

[David Woodhouse]

On Wed, 2009-10-21 at 12:19 -0700, Valerie Aurora wrote:
> From: Felix Fietkau <nbd@openwrt.org>
> 
> Add support for whiteout dentries to jffs2.

As discussed, there are a few places where JFFS2 will assume that a
dirent with fd->ino == 0 is a deletion dirent -- a kind of whiteout of
its own, used internally because it's a log-structured file system and
it needs to mark previously existing dirents as having been unlinked.

You're breaking that assumption. So, for example, your whiteouts are
going to get lost when the eraseblock containing them is garbage
collected -- because they'll be treated like deletion dirents, which
only need to remain on the medium for as long as the _real_ dirents
which they exist to kill.

This completely untested patch addresses some of it.

The other thing to verify is the three places in dir.c which check
whether whiteout/rmdir/rename should return -ENOTEMPTY. Those all do so
by checking whether the directory in question has any dirents with
fd->ino != 0 -- i.e. does it contain any _real_ dirents, or only the
deletion markers for dead stuff.

So that will now be _allowing_ you to remove a directory which contains
whiteouts, since you haven't changed the test. Is that intentional? It
seems sane at first glance.

diff --git a/fs/jffs2/build.c b/fs/jffs2/build.c
index c5e1450..4dc883f 100644
--- a/fs/jffs2/build.c
+++ b/fs/jffs2/build.c
@@ -217,8 +217,9 @@ static void jffs2_build_remove_unlinked_inode(struct jffs2_sb_info *c,
 			ic->scan_dents = fd->next;
 
 			if (!fd->ino) {
-				/* It's a deletion dirent. Ignore it */
-				dbg_fsbuild("child \"%s\" is a deletion dirent, skipping...\n", fd->name);
+				dbg_fsbuild("child \"%s\" is a %s, skipping...\n",
+					    fd->name,
+					    (fd->type == DT_WHT)?"whiteout":"deletion dirent");
 				jffs2_free_full_dirent(fd);
 				continue;
 			}
diff --git a/fs/jffs2/gc.c b/fs/jffs2/gc.c
index 090c556..7f5afbb 100644
--- a/fs/jffs2/gc.c
+++ b/fs/jffs2/gc.c
@@ -516,7 +516,7 @@ static int jffs2_garbage_collect_live(struct jffs2_sb_info *c,  struct jffs2_era
 			break;
 	}
 
-	if (fd && fd->ino) {
+	if (fd && (fd->ino || fd->type == DT_WHT)) {
 		ret = jffs2_garbage_collect_dirent(c, jeb, f, fd);
 	} else if (fd) {
 		ret = jffs2_garbage_collect_deletion_dirent(c, jeb, f, fd);
@@ -895,7 +895,7 @@ static int jffs2_garbage_collect_deletion_dirent(struct jffs2_sb_info *c, struct
 				continue;
 
 			/* If the name length doesn't match, or it's another deletion dirent, skip */
-			if (rd->nsize != name_len || !je32_to_cpu(rd->ino))
+			if (rd->nsize != name_len || (!je32_to_cpu(rd->ino) && rd->type != DT_WHT))
 				continue;
 
 			/* OK, check the actual name now */
diff --git a/fs/jffs2/write.c b/fs/jffs2/write.c
index ca29440..bcd4b86 100644
--- a/fs/jffs2/write.c
+++ b/fs/jffs2/write.c
@@ -629,8 +629,9 @@ int jffs2_do_unlink(struct jffs2_sb_info *c, struct jffs2_inode_info *dir_f,
 					printk(KERN_WARNING "Deleting inode #%u with active dentry \"%s\"->ino #%u\n",
 					       dead_f->inocache->ino, fd->name, fd->ino);
 				} else {
-					D1(printk(KERN_DEBUG "Removing deletion dirent for \"%s\" from dir ino #%u\n",
-						fd->name, dead_f->inocache->ino));
+					D1(printk(KERN_DEBUG "Removing %s for \"%s\" from dir ino #%u\n",
+						  (fd->type == DT_WHT)?"whiteout":"deletion dirent",
+						  fd->name, dead_f->inocache->ino));
 				}
 				if (fd->raw)
 					jffs2_mark_node_obsolete(c, fd->raw);


-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation

Re: [PATCH 16/41] whiteout: jffs2 whiteout support

[Valerie Aurora]

On Thu, Oct 22, 2009 at 07:50:49AM +0900, David Woodhouse wrote:
> On Wed, 2009-10-21 at 12:19 -0700, Valerie Aurora wrote:
> > From: Felix Fietkau <nbd@openwrt.org>
> > 
> > Add support for whiteout dentries to jffs2.
> 
> As discussed, there are a few places where JFFS2 will assume that a
> dirent with fd->ino == 0 is a deletion dirent -- a kind of whiteout of
> its own, used internally because it's a log-structured file system and
> it needs to mark previously existing dirents as having been unlinked.
> 
> You're breaking that assumption. So, for example, your whiteouts are
> going to get lost when the eraseblock containing them is garbage
> collected -- because they'll be treated like deletion dirents, which
> only need to remain on the medium for as long as the _real_ dirents
> which they exist to kill.
>
> This completely untested patch addresses some of it.

I think you are right.  Thanks!  I will add JFFS2 to my test suite
before the next release.  Right now I am testing mostly on UML, which
doesn't support the RAM-based MTD emulator as far I can tell.

> The other thing to verify is the three places in dir.c which check
> whether whiteout/rmdir/rename should return -ENOTEMPTY. Those all do so
> by checking whether the directory in question has any dirents with
> fd->ino != 0 -- i.e. does it contain any _real_ dirents, or only the
> deletion markers for dead stuff.
> 
> So that will now be _allowing_ you to remove a directory which contains
> whiteouts, since you haven't changed the test. Is that intentional? It
> seems sane at first glance.

Yes, you should be able to remove a directory which contains only
union mount-level whiteouts.

-VAL

> diff --git a/fs/jffs2/build.c b/fs/jffs2/build.c
> index c5e1450..4dc883f 100644
> --- a/fs/jffs2/build.c
> +++ b/fs/jffs2/build.c
> @@ -217,8 +217,9 @@ static void jffs2_build_remove_unlinked_inode(struct jffs2_sb_info *c,
>  			ic->scan_dents = fd->next;
>  
>  			if (!fd->ino) {
> -				/* It's a deletion dirent. Ignore it */
> -				dbg_fsbuild("child \"%s\" is a deletion dirent, skipping...\n", fd->name);
> +				dbg_fsbuild("child \"%s\" is a %s, skipping...\n",
> +					    fd->name,
> +					    (fd->type == DT_WHT)?"whiteout":"deletion dirent");
>  				jffs2_free_full_dirent(fd);
>  				continue;
>  			}
> diff --git a/fs/jffs2/gc.c b/fs/jffs2/gc.c
> index 090c556..7f5afbb 100644
> --- a/fs/jffs2/gc.c
> +++ b/fs/jffs2/gc.c
> @@ -516,7 +516,7 @@ static int jffs2_garbage_collect_live(struct jffs2_sb_info *c,  struct jffs2_era
>  			break;
>  	}
>  
> -	if (fd && fd->ino) {
> +	if (fd && (fd->ino || fd->type == DT_WHT)) {
>  		ret = jffs2_garbage_collect_dirent(c, jeb, f, fd);
>  	} else if (fd) {
>  		ret = jffs2_garbage_collect_deletion_dirent(c, jeb, f, fd);
> @@ -895,7 +895,7 @@ static int jffs2_garbage_collect_deletion_dirent(struct jffs2_sb_info *c, struct
>  				continue;
>  
>  			/* If the name length doesn't match, or it's another deletion dirent, skip */
> -			if (rd->nsize != name_len || !je32_to_cpu(rd->ino))
> +			if (rd->nsize != name_len || (!je32_to_cpu(rd->ino) && rd->type != DT_WHT))
>  				continue;
>  
>  			/* OK, check the actual name now */
> diff --git a/fs/jffs2/write.c b/fs/jffs2/write.c
> index ca29440..bcd4b86 100644
> --- a/fs/jffs2/write.c
> +++ b/fs/jffs2/write.c
> @@ -629,8 +629,9 @@ int jffs2_do_unlink(struct jffs2_sb_info *c, struct jffs2_inode_info *dir_f,
>  					printk(KERN_WARNING "Deleting inode #%u with active dentry \"%s\"->ino #%u\n",
>  					       dead_f->inocache->ino, fd->name, fd->ino);
>  				} else {
> -					D1(printk(KERN_DEBUG "Removing deletion dirent for \"%s\" from dir ino #%u\n",
> -						fd->name, dead_f->inocache->ino));
> +					D1(printk(KERN_DEBUG "Removing %s for \"%s\" from dir ino #%u\n",
> +						  (fd->type == DT_WHT)?"whiteout":"deletion dirent",
> +						  fd->name, dead_f->inocache->ino));
>  				}
>  				if (fd->raw)
>  					jffs2_mark_node_obsolete(c, fd->raw);
> 
> 
> -- 
> David Woodhouse                            Open Source Technology Centre
> David.Woodhouse@intel.com                              Intel Corporation
> 

Re: [PATCH 16/41] whiteout: jffs2 whiteout support

[Erez Zadok]

In message <1256152779-10054-17-git-send-email-vaurora@redhat.com>, Valerie Aurora writes:
> From: Felix Fietkau <nbd@openwrt.org>
> 
> Add support for whiteout dentries to jffs2.
> 
> Signed-off-by: Felix Fietkau <nbd@openwrt.org>
> Signed-off-by: Valerie Aurora <vaurora@redhat.com>
> Cc: David Woodhouse <dwmw2@infradead.org>
> Cc: linux-mtd@lists.infradead.org
> ---
>  fs/jffs2/dir.c        |   77 +++++++++++++++++++++++++++++++++++++++++++++++-
>  fs/jffs2/fs.c         |    4 ++
>  fs/jffs2/super.c      |    2 +-
>  include/linux/jffs2.h |    2 +
>  4 files changed, 82 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/jffs2/dir.c b/fs/jffs2/dir.c
> index 6f60cc9..46a2e1b 100644
> --- a/fs/jffs2/dir.c
> +++ b/fs/jffs2/dir.c
> @@ -34,6 +34,8 @@ static int jffs2_mknod (struct inode *,struct dentry *,int,dev_t);
>  static int jffs2_rename (struct inode *, struct dentry *,
>  			 struct inode *, struct dentry *);
>  
> +static int jffs2_whiteout (struct inode *, struct dentry *, struct dentry *);
> +
>  const struct file_operations jffs2_dir_operations =
>  {
>  	.read =		generic_read_dir,
> @@ -55,6 +57,7 @@ const struct inode_operations jffs2_dir_inode_operations =
>  	.rmdir =	jffs2_rmdir,
>  	.mknod =	jffs2_mknod,
>  	.rename =	jffs2_rename,
> +	.whiteout =     jffs2_whiteout,
>  	.permission =	jffs2_permission,
>  	.setattr =	jffs2_setattr,
>  	.setxattr =	jffs2_setxattr,
> @@ -98,8 +101,18 @@ static struct dentry *jffs2_lookup(struct inode *dir_i, struct dentry *target,
>  			fd = fd_list;
>  		}
>  	}
> -	if (fd)
> -		ino = fd->ino;
> +	if (fd) {
> +		spin_lock(&target->d_lock);
> +		switch(fd->type) {
> +		case DT_WHT:
> +			target->d_flags |= DCACHE_WHITEOUT;
> +			break;
> +		default:
> +			ino = fd->ino;
> +			break;
> +		}
> +		spin_unlock(&target->d_lock);
> +	}

The switch statement above should be simplified into this:

 if (fd->type == DT_WHT)
	target->d_flags |= DCACHE_WHITEOUT;
 else
	ino = fd->ino;

> +	/* If it's a directory, then check whether it is really empty
> +	 */

Format above comment on one line.

Erez.

Re: [PATCH 16/41] whiteout: jffs2 whiteout support

[Valerie Aurora]

On Mon, Nov 30, 2009 at 02:51:05AM -0500, Erez Zadok wrote:
> In message <1256152779-10054-17-git-send-email-vaurora@redhat.com>, Valerie Aurora writes:
> > From: Felix Fietkau <nbd@openwrt.org>
> > 
> > Add support for whiteout dentries to jffs2.
> > 
> > Signed-off-by: Felix Fietkau <nbd@openwrt.org>
> > Signed-off-by: Valerie Aurora <vaurora@redhat.com>
> > Cc: David Woodhouse <dwmw2@infradead.org>
> > Cc: linux-mtd@lists.infradead.org
> > ---
> >  fs/jffs2/dir.c        |   77 +++++++++++++++++++++++++++++++++++++++++++++++-
> >  fs/jffs2/fs.c         |    4 ++
> >  fs/jffs2/super.c      |    2 +-
> >  include/linux/jffs2.h |    2 +
> >  4 files changed, 82 insertions(+), 3 deletions(-)
> > 
> > diff --git a/fs/jffs2/dir.c b/fs/jffs2/dir.c
> > index 6f60cc9..46a2e1b 100644
> > --- a/fs/jffs2/dir.c
> > +++ b/fs/jffs2/dir.c
> > @@ -34,6 +34,8 @@ static int jffs2_mknod (struct inode *,struct dentry *,int,dev_t);
> >  static int jffs2_rename (struct inode *, struct dentry *,
> >  			 struct inode *, struct dentry *);
> >  
> > +static int jffs2_whiteout (struct inode *, struct dentry *, struct dentry *);
> > +
> >  const struct file_operations jffs2_dir_operations =
> >  {
> >  	.read =		generic_read_dir,
> > @@ -55,6 +57,7 @@ const struct inode_operations jffs2_dir_inode_operations =
> >  	.rmdir =	jffs2_rmdir,
> >  	.mknod =	jffs2_mknod,
> >  	.rename =	jffs2_rename,
> > +	.whiteout =     jffs2_whiteout,
> >  	.permission =	jffs2_permission,
> >  	.setattr =	jffs2_setattr,
> >  	.setxattr =	jffs2_setxattr,
> > @@ -98,8 +101,18 @@ static struct dentry *jffs2_lookup(struct inode *dir_i, struct dentry *target,
> >  			fd = fd_list;
> >  		}
> >  	}
> > -	if (fd)
> > -		ino = fd->ino;
> > +	if (fd) {
> > +		spin_lock(&target->d_lock);
> > +		switch(fd->type) {
> > +		case DT_WHT:
> > +			target->d_flags |= DCACHE_WHITEOUT;
> > +			break;
> > +		default:
> > +			ino = fd->ino;
> > +			break;
> > +		}
> > +		spin_unlock(&target->d_lock);
> > +	}
> 
> The switch statement above should be simplified into this:
> 
>  if (fd->type == DT_WHT)
> 	target->d_flags |= DCACHE_WHITEOUT;
>  else
> 	ino = fd->ino;

This is because later we add a third case for fallthrus, at which
point a switch statement is easier to read.  But it is confusing and
distracting by itself in this patch, so I changed it as you suggested.


> > +	/* If it's a directory, then check whether it is really empty
> > +	 */
> 
> Format above comment on one line.

Fixxed, thanks.

-VAL