[PATCH v3 01/33] jfs: free uncommitted anonymous tlocks when evicting an inode

[Christian Brauner <brauner@kernel.org>]

jfs doesn't log block allocations made through ->write_begin() and
->writepages() right away. extAlloc() records them in anonymous
transaction locks that hang off the inode and puts the inode on the
global anonymous list. They only get committed when a later transaction
includes the inode, usually writeback calling ->write_inode() which ends
up in jfs_commit_inode(). jfs_evict_inode() relies on that and BUG()s
when the inode is still on the anonymous list.

But nothing guarantees that writeback ran before eviction. With
errors=remount-ro, the default, an error sets SB_RDONLY and
sync_filesystem() skips read-only superblocks. So unmounting the
filesystem after an error evicts every inode that allocated blocks since
it was last written back without committing anything and the BUG_ON()
fires. Writing a file and hitting an error before the flusher got to the
inode is enough:

  kernel BUG at fs/jfs/inode.c:175!
  Oops: invalid opcode: 0000 [#1] SMP KASAN NOPTI
  RIP: 0010:jfs_evict_inode+0x33c/0x440
  Call Trace:
   <TASK>
   evict+0x336/0x8b0
   evict_inodes+0x421/0x850
   generic_shutdown_super+0xaf/0x530
   kill_block_super+0x36/0xa0
   deactivate_locked_super+0x97/0xf0
   cleanup_mnt+0x9a/0x130
   task_work_run+0x135/0x230
   exit_to_user_mode_loop+0x171/0x560
   do_syscall_64+0x410/0x5e0
   entry_SYSCALL_64_after_hwframe+0x77/0x7f
   </TASK>

Just taking the inode off the anonymous list isn't enough. The tlocks
come from TxLock which is shared by all jfs mounts, txLock() pointed
mp->lid at them and pinned the metapages with metapage_nohomeok(). Leak
them and every other jfs filesystem eventually blocks in txBeginAnon()
waiting for tlocks that never come back.

So free them the way txUnlock() frees the tlocks of a committed
transaction: drop the metapage_nohomeok() pin, reset mp->lid and free
the linelocks and the tlock itself. txAbort() isn't a good model here as
it only drops the pin for pages that are on the logsync list and leaves
the linelocks alone. A metapage that carries nothing but the uncommitted
change isn't written home. The superblock is marked dirty after an error
so fsck has to look at the filesystem anyway.

Note that jfs_sync() takes an inode it can't grab off the anonymous list
without freeing anything so check jfs_ip->atlhead instead of the list.

Fixes: 8a9cd6d67672 ("JFS: Fix race in txLock")
Cc: stable@vger.kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 fs/jfs/inode.c      |  9 +++++++
 fs/jfs/jfs_txnmgr.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/jfs/jfs_txnmgr.h |  1 +
 3 files changed, 78 insertions(+)

diff --git a/fs/jfs/inode.c b/fs/jfs/inode.c
index c7914dbc91ed9..6390d915c7f11 100644
--- a/fs/jfs/inode.c
+++ b/fs/jfs/inode.c
@@ -172,6 +172,15 @@ void jfs_evict_inode(struct inode *inode)
 	clear_inode(inode);
 	dquot_drop(inode);
 
+	/*
+	 * Writeback commits the anonymous transactions of allocations. If the
+	 * filesystem went read-only without a sync, e.g. after an error,
+	 * nothing did.
+	 */
+	if (ji->atlhead) {
+		WARN_ON_ONCE(!sb_rdonly(inode->i_sb));
+		txAbortAnon(inode);
+	}
 	BUG_ON(!list_empty(&ji->anon_inode_list));
 
 	spin_lock_irq(&ji->ag_lock);
diff --git a/fs/jfs/jfs_txnmgr.c b/fs/jfs/jfs_txnmgr.c
index 083dbbb0c3268..85192dd4461a4 100644
--- a/fs/jfs/jfs_txnmgr.c
+++ b/fs/jfs/jfs_txnmgr.c
@@ -2581,6 +2581,74 @@ void txFreelock(struct inode *ip)
 	TXN_UNLOCK();
 }
 
+/*
+ *	txAbortAnon()
+ *
+ * function:	free the anonymous tlocks of an inode that is evicted
+ *		before writeback committed them, e.g. because an error made
+ *		the filesystem read-only.
+ *
+ * A page that carries nothing but uncommitted changes isn't written home.
+ * A page that is also waiting to go home with committed changes (mp->lsn)
+ * goes home with both, after an error the superblock is marked dirty.
+ */
+void txAbortAnon(struct inode *ip)
+{
+	struct jfs_inode_info *jfs_ip = JFS_IP(ip);
+	struct linelock *linelock;
+	struct metapage *mp;
+	struct tlock *tlck;
+	lid_t lid, next, llid, k;
+
+	/* txQuiesce() and jfs_sync() commit under the commit_mutex */
+	mutex_lock(&jfs_ip->commit_mutex);
+
+	TXN_LOCK();
+	lid = jfs_ip->atlhead;
+	jfs_ip->atlhead = jfs_ip->atltail = 0;
+	list_del_init(&jfs_ip->anon_inode_list);
+	TXN_UNLOCK();
+
+	for (; lid; lid = next) {
+		tlck = lid_to_tlock(lid);
+		next = tlck->next;
+		mp = tlck->mp;
+
+		if (mp && (tlck->flag & tlckPAGELOCK)) {
+			/* undo txLock()'s metapage_nohomeok() */
+			hold_metapage(mp);
+			if (!mp->lsn)
+				clear_bit(META_dirty, &mp->flag);
+			TXN_LOCK();
+			mp->lid = 0;
+			TXN_UNLOCK();
+			_metapage_homeok(mp);
+			put_metapage(mp);
+		} else if (mp) {
+			/* the xtree root in the inode, it isn't written anymore */
+			TXN_LOCK();
+			if (jfs_ip->xtlid == lid)
+				jfs_ip->xtlid = 0;
+			else
+				mp->lid = 0;
+			TXN_UNLOCK();
+		}
+
+		TXN_LOCK();
+		llid = ((struct linelock *) &tlck->lock)->next;
+		while (llid) {
+			linelock = (struct linelock *) lid_to_tlock(llid);
+			k = linelock->next;
+			txLockFree(llid);
+			llid = k;
+		}
+		txLockFree(lid);
+		TXN_UNLOCK();
+	}
+
+	mutex_unlock(&jfs_ip->commit_mutex);
+}
+
 /*
  *	txAbort()
  *
diff --git a/fs/jfs/jfs_txnmgr.h b/fs/jfs/jfs_txnmgr.h
index ba71eb5ced567..e9a7547afd3f6 100644
--- a/fs/jfs/jfs_txnmgr.h
+++ b/fs/jfs/jfs_txnmgr.h
@@ -288,6 +288,7 @@ extern struct linelock *txLinelock(struct linelock *);
 extern void txFreeMap(struct inode *, struct maplock *, struct tblock *, int);
 extern void txEA(tid_t, struct inode *, dxd_t *, dxd_t *);
 extern void txFreelock(struct inode *);
+extern void txAbortAnon(struct inode *);
 extern int lmLog(struct jfs_log *, struct tblock *, struct lrd *,
 		 struct tlock *);
 extern void txQuiesce(struct super_block *);

-- 
2.53.0


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