Commit 250df6ed removed wake_up_inode() (in particular a memory barrier before
wake_up_bit()) on the basis that i_state transitions are protected by i_lock.
That would be fine if all the readers of i_state were using i_lock as well. But
wait_on_inode() doesn't use i_lock and thus the following can happen due to
reordering:
CPU 1 CPU 2
unlock_new_inode()
spin_lock(&inode->i_lock);
wake_up_bit(&inode->i_state, __I_NEW);
wait_on_inode()
wait_on_bit(&inode->i_state, __I_NEW);
inode->i_state &= ~I_NEW;
^^^ this store was reordered
spin_unlock(&inode->i_lock);
And waiter on CPU2 sleeps forever (or for a really long time).
We fix the issue by using i_lock in wait_on_inode() in the spirit of commit
250df6ed.
CC: Dave Chinner <dchinner@xxxxxxxxxx>
Reported-by: Eric Buddington <ebuddington@xxxxxxxxxxxx>
Signed-off-by: Jan Kara <jack@xxxxxxx>
---
fs/inode.c | 22 ++++++++++++++++++++++
include/linux/writeback.h | 5 -----
2 files changed, 22 insertions(+), 5 deletions(-)
Al, could you please merge this patch? It fixes a real deadlock...
diff --git a/fs/inode.c b/fs/inode.c
index fb10d86..e768f9e 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -954,6 +954,28 @@ EXPORT_SYMBOL(lockdep_annotate_inode_mutex_key);
#endif
/**
+ * wait_on_inode - wait while inode is in I_NEW state
+ * @inode: inode to wait for
+ *
+ * This function waits until inode is fully initialized and exits new state
+ */
+static void wait_on_inode(struct inode *inode)
+{
+ DEFINE_WAIT_BIT(wq, &inode->i_state, __I_NEW);
+ wait_queue_head_t *wqh;
+
+ might_sleep();
+ wqh = bit_waitqueue(&inode->i_state, __I_NEW);
+ spin_lock(&inode->i_lock);
+ while (inode->i_state & I_NEW) {
+ spin_unlock(&inode->i_lock);
+ __wait_on_bit(wqh, &wq, inode_wait, TASK_UNINTERRUPTIBLE);
+ spin_lock(&inode->i_lock);
+ }
+ spin_unlock(&inode->i_lock);
+}
+
+/**
* unlock_new_inode - clear the I_NEW state and wake up any waiters
* @inode: new inode to unlock
*
diff --git a/include/linux/writeback.h b/include/linux/writeback.h
index 995b8bf..e2dbc70 100644
--- a/include/linux/writeback.h
+++ b/include/linux/writeback.h
@@ -96,11 +96,6 @@ long wb_do_writeback(struct bdi_writeback *wb, int force_wait);
void wakeup_flusher_threads(long nr_pages, enum wb_reason reason);
/* writeback.h requires fs.h; it, too, is not included from here. */
-static inline void wait_on_inode(struct inode *inode)
-{
- might_sleep();
- wait_on_bit(&inode->i_state, __I_NEW, inode_wait, TASK_UNINTERRUPTIBLE);
-}
static inline void inode_sync_wait(struct inode *inode)
{
might_sleep();
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
