Re: [PATCH 11/35] whiteout: jffs2 whiteout support

[Valerie Aurora]

On Mon, Apr 19, 2010 at 02:03:27PM +0100, David Woodhouse wrote:
> On Thu, 2010-04-15 at 16:04 -0700, Valerie Aurora wrote:
> > From: Felix Fietkau <nbd@xxxxxxxxxxx>
> > 
> > Add support for whiteout dentries to jffs2. 
> 
> This doesn't seem to have incorporated my feedback from the attached...

Hm, I'm not sure whether I lost the patch in a rebase or didn't have
time to test it or what.  I was hoping someone who actually knows
JFFS2 like Felix or you would get to it first - in general, I'd like
the underlying file system maintainers to implement whiteouts and
fallthrus since they know them best.  Felix, if you implemented it and
I lost the patch, my apologies to you.

Thanks David,

-VAL


> -- 
> David Woodhouse                            Open Source Technology Centre
> David.Woodhouse@xxxxxxxxx                              Intel Corporation

Content-Description: Attached message - Re: [PATCH 16/41] whiteout: jffs2 whiteout support
> From: David Woodhouse <dwmw2@xxxxxxxxxxxxx>
> To: Valerie Aurora <vaurora@xxxxxxxxxx>
> Cc: Jan Blunck <jblunck@xxxxxxx>, Alexander Viro <viro@xxxxxxxxxxxxxxxxxx>,  Christoph Hellwig <hch@xxxxxxxxxxxxx>, Andy Whitcroft <apw@xxxxxxxxxxxxx>, Scott James Remnant <scott@xxxxxxxxxxxxx>, Sandu Popa Marius <sandupopamarius@xxxxxxxxx>, Jan Rekorajski <baggins@xxxxxxxxxxxxxxxxx>, "J. R. Okajima" <hooanon05@xxxxxxxxxxx>, Arnd Bergmann <arnd@xxxxxxxx>, Vladimir Dronnikov <dronnikov@xxxxxxxxx>, Felix Fietkau <nbd@xxxxxxxxxxx>, linux-fsdevel@xxxxxxxxxxxxxxx, linux-kernel@xxxxxxxxxxxxxxx,  linux-mtd@xxxxxxxxxxxxxxxxxxx
> Date: Thu, 22 Oct 2009 07:50:58 +0900
> Subject: Re: [PATCH 16/41] whiteout: jffs2 whiteout support
> 
> On Wed, 2009-10-21 at 12:19 -0700, Valerie Aurora wrote:
> > From: Felix Fietkau <nbd@xxxxxxxxxxx>
> > 
> > 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@xxxxxxxxx                              Intel Corporation

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/