From: yangerkun <yangerkun@xxxxxxxxxx>
Combine use of xfs_trans_hold and xfs_trans_set_sync in xfs_sync_sb_buf
can trigger a deadlock once shutdown happened concurrently. xlog_ioend_work
will first unpin the sb(which stuck with xfs_buf_lock), then wakeup
xfs_sync_sb_buf. However, xfs_sync_sb_buf never get the chance to unlock
sb until been wakeup by xlog_ioend_work.
xfs_sync_sb_buf
xfs_trans_getsb // lock sb buf
xfs_trans_bhold // sb buf keep lock until success commit
xfs_trans_commit
...
xfs_log_force_seq
xlog_force_lsn
xlog_wait_on_iclog
xlog_wait(&iclog->ic_force_wait... // shutdown happened
xfs_buf_relse // unlock sb buf
xlog_ioend_work
xlog_force_shutdown
xlog_state_shutdown_callbacks
xlog_cil_process_committed
xlog_cil_committed
...
xfs_buf_item_unpin
xfs_buf_lock // deadlock
wake_up_all(&iclog->ic_force_wait)
xfs_ioc_setlabel use xfs_sync_sb_buf to make sure userspace will see the
change for sb immediately. We can simply call xfs_ail_push_all_sync to
do this and sametime fix the deadlock.
Fixes: f7664b31975b ("xfs: implement online get/set fs label")
Signed-off-by: yangerkun <yangerkun@xxxxxxxxxx>
---
fs/xfs/libxfs/xfs_sb.c | 32 --------------------------------
fs/xfs/libxfs/xfs_sb.h | 1 -
fs/xfs/xfs_ioctl.c | 5 ++++-
3 files changed, 4 insertions(+), 34 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c
index ba0f17bc1dc0..8b89e65c5e1e 100644
--- a/fs/xfs/libxfs/xfs_sb.c
+++ b/fs/xfs/libxfs/xfs_sb.c
@@ -1089,38 +1089,6 @@ xfs_update_secondary_sbs(
return saved_error ? saved_error : error;
}
-/*
- * Same behavior as xfs_sync_sb, except that it is always synchronous and it
- * also writes the superblock buffer to disk sector 0 immediately.
- */
-int
-xfs_sync_sb_buf(
- struct xfs_mount *mp)
-{
- struct xfs_trans *tp;
- struct xfs_buf *bp;
- int error;
-
- error = xfs_trans_alloc(mp, &M_RES(mp)->tr_sb, 0, 0, 0, &tp);
- if (error)
- return error;
-
- bp = xfs_trans_getsb(tp);
- xfs_log_sb(tp);
- xfs_trans_bhold(tp, bp);
- xfs_trans_set_sync(tp);
- error = xfs_trans_commit(tp);
- if (error)
- goto out;
- /*
- * write out the sb buffer to get the changes to disk
- */
- error = xfs_bwrite(bp);
-out:
- xfs_buf_relse(bp);
- return error;
-}
-
void
xfs_fs_geometry(
struct xfs_mount *mp,
diff --git a/fs/xfs/libxfs/xfs_sb.h b/fs/xfs/libxfs/xfs_sb.h
index a5e14740ec9a..fb420312c476 100644
--- a/fs/xfs/libxfs/xfs_sb.h
+++ b/fs/xfs/libxfs/xfs_sb.h
@@ -15,7 +15,6 @@ struct xfs_perag;
extern void xfs_log_sb(struct xfs_trans *tp);
extern int xfs_sync_sb(struct xfs_mount *mp, bool wait);
-extern int xfs_sync_sb_buf(struct xfs_mount *mp);
extern void xfs_sb_mount_common(struct xfs_mount *mp, struct xfs_sb *sbp);
extern void xfs_sb_from_disk(struct xfs_sb *to, struct xfs_dsb *from);
extern void xfs_sb_to_disk(struct xfs_dsb *to, struct xfs_sb *from);
diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index 55bb01173cde..9bacc71d2c9e 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -27,6 +27,7 @@
#include "xfs_trace.h"
#include "xfs_icache.h"
#include "xfs_trans.h"
+#include "xfs_trans_priv.h"
#include "xfs_acl.h"
#include "xfs_btree.h"
#include <linux/fsmap.h>
@@ -1798,9 +1799,11 @@ xfs_ioc_setlabel(
* buffered reads from userspace (i.e. from blkid) are invalidated,
* and userspace will see the newly-written label.
*/
- error = xfs_sync_sb_buf(mp);
+ error = xfs_sync_sb(mp, true);
if (error)
goto out;
+
+ xfs_ail_push_all_sync(mp->m_ail);
/*
* growfs also updates backup supers so lock against that.
*/
--
2.39.2
