Re: Regression in handling power cuts since 3a1e819b4e80 ("ovl: store file handle of lower inode on copy up")

From: Richard Weinberger <richard@nod.at>
To: "Rafał Miłecki" <zajec5@gmail.com>
Cc: Artem Bityutskiy <dedekind1@gmail.com>,
	Amir Goldstein <amir73il@gmail.com>,
	Adrian Hunter <adrian.hunter@intel.com>,
	linux-unionfs@vger.kernel.org, Miklos Szeredi <miklos@szeredi.hu>,
	linux-mtd@lists.infradead.org,
	Russell Senior <russell@personaltelco.net>,
	linux-fsdevel@vger.kernel.org,
	OpenWrt Development List <openwrt-devel@lists.openwrt.org>
Subject: Re: Regression in handling power cuts since 3a1e819b4e80 ("ovl: store file handle of lower inode on copy up")
Date: Sat, 27 Oct 2018 21:33:56 +0200	[thread overview]
Message-ID: <6556017.quyC1DpDMg@blindfold> (raw)
In-Reply-To: <CACna6rzvzc-_X3VDeA-nwtTW7KHaQGtbvzjVNoOFEQuHKi3PJg@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 1088 bytes --]

Rafał,

Am Montag, 22. Oktober 2018, 17:34:44 CEST schrieb Rafał Miłecki:
> Then I took a close look at ovl_copy_up_locked() and it seems above
> info isn't accurate. It seems to me that setxattr() happens between
> fsync and link. I modified my C app to follow that order (open, write,
> fsync, setxattr, link) and I can reproduce the problem now!
> 
> Steps to reproduce the problem:
> 1) compile tmptest.c
> 2) tmptest /overlay/upper/foo.txt user.bar baz
> 3) wait 5 seconds (so ubifs writes to flash)
> 4) power cut
> 5) boot again and check content of /overlay/upper/foo.txt
> 6) in my case content appears to be 00 00 00 00

Just returned from Edinburgh and had a chance to look at the problem.
The problem is not that no write-back happens, in fact it happens just fine.
But we have a problem upon journal replay if an unlinked file (O_TMPFILE)
gets relinked in combination with xattrs.

Can you please give the attached patch a try? It is not perfect but if I 
understand the problem correctly it should fix the issues you're facing.

Thanks,
//richard

[-- Attachment #2: replay_tmpfile_fix.diff --]
[-- Type: text/x-patch, Size: 1263 bytes --]

diff --git a/fs/ubifs/replay.c b/fs/ubifs/replay.c
index 4844538eb926..82b1244e4138 100644
--- a/fs/ubifs/replay.c
+++ b/fs/ubifs/replay.c
@@ -209,6 +209,19 @@ static int trun_remove_range(struct ubifs_info *c, struct replay_entry *r)
 	return ubifs_tnc_remove_range(c, &min_key, &max_key);
 }
 
+static bool inode_comes_back(struct ubifs_info *c, struct replay_entry *rino)
+{
+	struct replay_entry *r;
+
+	list_for_each_entry(r, &c->replay_list, list) {
+		if (keys_cmp(c, &rino->key, &r->key) == 0 &&
+		    r->deletion == 0)
+			return true;
+	}
+
+	return false;
+}
+
 /**
  * apply_replay_entry - apply a replay entry to the TNC.
  * @c: UBIFS file-system description object
@@ -218,7 +231,7 @@ static int trun_remove_range(struct ubifs_info *c, struct replay_entry *r)
  */
 static int apply_replay_entry(struct ubifs_info *c, struct replay_entry *r)
 {
-	int err;
+	int err = 0;
 
 	dbg_mntk(&r->key, "LEB %d:%d len %d deletion %d sqnum %llu key ",
 		 r->lnum, r->offs, r->len, r->deletion, r->sqnum);
@@ -236,6 +249,9 @@ static int apply_replay_entry(struct ubifs_info *c, struct replay_entry *r)
 			{
 				ino_t inum = key_inum(c, &r->key);
 
+				if (inode_comes_back(c, r))
+					break;
+
 				err = ubifs_tnc_remove_ino(c, inum);
 				break;
 			}

[-- Attachment #3: Type: text/plain, Size: 144 bytes --]

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
