From mboxrd@z Thu Jan 1 00:00:00 1970 From: Richard Weinberger 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 Message-ID: <6556017.quyC1DpDMg@blindfold> References: Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="nextPart1960845.MarFMtrmzt" Content-Transfer-Encoding: 7Bit Return-path: In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-mtd" Errors-To: linux-mtd-bounces+gldm-linux-mtd-36=gmane.org@lists.infradead.org To: =?utf-8?B?UmFmYcWCIE1pxYJlY2tp?= Cc: Artem Bityutskiy , Amir Goldstein , Adrian Hunter , linux-unionfs@vger.kernel.org, Miklos Szeredi , linux-mtd@lists.infradead.org, Russell Senior , linux-fsdevel@vger.kernel.org, OpenWrt Development List List-Id: linux-unionfs@vger.kernel.org This is a multi-part message in MIME format. --nextPart1960845.MarFMtrmzt Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="UTF-8" Rafa=C5=82, Am Montag, 22. Oktober 2018, 17:34:44 CEST schrieb Rafa=C5=82 Mi=C5=82ecki: > 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! >=20 > 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=20 understand the problem correctly it should fix the issues you're facing. Thanks, //richard --nextPart1960845.MarFMtrmzt Content-Disposition: attachment; filename="replay_tmpfile_fix.diff" Content-Transfer-Encoding: 7Bit Content-Type: text/x-patch; charset="UTF-8"; name="replay_tmpfile_fix.diff" 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; } --nextPart1960845.MarFMtrmzt Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ --nextPart1960845.MarFMtrmzt-- From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from lithops.sigma-star.at ([195.201.40.130]:48788 "EHLO lithops.sigma-star.at" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725748AbeJ1EQA (ORCPT ); Sun, 28 Oct 2018 00:16:00 -0400 From: Richard Weinberger To: =?utf-8?B?UmFmYcWCIE1pxYJlY2tp?= Cc: Amir Goldstein , Miklos Szeredi , linux-unionfs@vger.kernel.org, linux-fsdevel@vger.kernel.org, Artem Bityutskiy , Adrian Hunter , linux-mtd@lists.infradead.org, Russell Senior , OpenWrt Development List 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 Message-ID: <6556017.quyC1DpDMg@blindfold> In-Reply-To: References: MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="nextPart1960845.MarFMtrmzt" Content-Transfer-Encoding: 7Bit Sender: linux-fsdevel-owner@vger.kernel.org List-ID: This is a multi-part message in MIME format. --nextPart1960845.MarFMtrmzt Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="UTF-8" Rafa=C5=82, Am Montag, 22. Oktober 2018, 17:34:44 CEST schrieb Rafa=C5=82 Mi=C5=82ecki: > 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! >=20 > 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=20 understand the problem correctly it should fix the issues you're facing. Thanks, //richard --nextPart1960845.MarFMtrmzt Content-Disposition: attachment; filename="replay_tmpfile_fix.diff" Content-Transfer-Encoding: 7Bit Content-Type: text/x-patch; charset="UTF-8"; name="replay_tmpfile_fix.diff" 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; } --nextPart1960845.MarFMtrmzt--