From d958324f97f4668a2a6e4a6ce3e5ca09b71b31d9 Mon Sep 17 00:00:00 2001 From: Brian Behlendorf Date: Tue, 6 Jan 2015 16:54:57 -0800 Subject: [PATCH] Fix zfs_putpage() lock inversion (again) This is a follow up commit to 74328ee which correctly resolved a lock inversion between zfs_putpage() and zfs_free_range(). Unfortunately, in the process it accidentally introduced another inversion between zfs_putpage() and zfs_read(). The page must be unlocked before taking the range lock. This patch corrects that issue. In addition, because the locking rules here are subtle a block comment has been added clearly explaining why the ordering here is critical. Signed-off-by: Brian Behlendorf Signed-off-by: Ned Bass Issue #2976 --- module/zfs/zfs_vnops.c | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/module/zfs/zfs_vnops.c b/module/zfs/zfs_vnops.c index a048aeb36c..33cdbb3f54 100644 --- a/module/zfs/zfs_vnops.c +++ b/module/zfs/zfs_vnops.c @@ -3899,15 +3899,28 @@ zfs_putpage(struct inode *ip, struct page *pp, struct writeback_control *wbc) } #endif - rl = zfs_range_lock(zp, pgoff, pglen, RL_WRITER); - - set_page_writeback(pp); + /* + * The ordering here is critical and must adhere to the following + * rules in order to avoid deadlocking in either zfs_read() or + * zfs_free_range() due to a lock inversion. + * + * 1) The page must be unlocked prior to acquiring the range lock. + * This is critical because zfs_read() calls find_lock_page() + * which may block on the page lock while holding the range lock. + * + * 2) Before setting or clearing write back on a page the range lock + * must be held in order to prevent a lock inversion with the + * zfs_free_range() function. + */ unlock_page(pp); + rl = zfs_range_lock(zp, pgoff, pglen, RL_WRITER); + set_page_writeback(pp); tx = dmu_tx_create(zsb->z_os); dmu_tx_hold_write(tx, zp->z_id, pgoff, pglen); dmu_tx_hold_sa(tx, zp->z_sa_hdl, B_FALSE); zfs_sa_upgrade_txholds(tx, zp); + err = dmu_tx_assign(tx, TXG_NOWAIT); if (err != 0) { if (err == ERESTART)