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-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
