diff mbox series

[4/9] Use wait_var_event() instead of I_DIO_WAKEUP

Message ID 20240819053605.11706-5-neilb@suse.de (mailing list archive)
State New
Headers show
Series Make wake_up_{bit,var} less fragile | expand

Commit Message

NeilBrown Aug. 19, 2024, 5:20 a.m. UTC 
inode_dio_wait() is essentially an open-coded version of
wait_var_event().  Similarly inode_dio_wait_interruptible() is an
open-coded version of wait_var_event_interruptible().

If we switch to waiting on the var, instead of an imaginary bit, the
code is more transparent, is shorter, and we can discard I_DIO_WAKEUP.

Signed-off-by: NeilBrown <neilb@suse.de>
---
 fs/inode.c         | 16 ++--------------
 fs/netfs/locking.c | 19 ++-----------------
 include/linux/fs.h |  7 +------
 3 files changed, 5 insertions(+), 37 deletions(-)

Comments

Christian Brauner Aug. 20, 2024, 7:22 a.m. UTC | #1 
On Mon, Aug 19, 2024 at 03:20:38PM GMT, NeilBrown wrote:
> inode_dio_wait() is essentially an open-coded version of
> wait_var_event().  Similarly inode_dio_wait_interruptible() is an
> open-coded version of wait_var_event_interruptible().
> 
> If we switch to waiting on the var, instead of an imaginary bit, the
> code is more transparent, is shorter, and we can discard I_DIO_WAKEUP.
> 
> Signed-off-by: NeilBrown <neilb@suse.de>
> ---

Neil, I've sent a patch for this last week already removing
__I_DIO_WAKEUP and it's in -next as
0009dc756e81 ("inode: remove __I_DIO_WAKEUP"). So you can drop this
patch, please.
Christian Brauner Aug. 20, 2024, 7:12 p.m. UTC | #2 
On Tue, Aug 20, 2024 at 09:22:33AM GMT, Christian Brauner wrote:
> On Mon, Aug 19, 2024 at 03:20:38PM GMT, NeilBrown wrote:
> > inode_dio_wait() is essentially an open-coded version of
> > wait_var_event().  Similarly inode_dio_wait_interruptible() is an
> > open-coded version of wait_var_event_interruptible().
> > 
> > If we switch to waiting on the var, instead of an imaginary bit, the
> > code is more transparent, is shorter, and we can discard I_DIO_WAKEUP.
> > 
> > Signed-off-by: NeilBrown <neilb@suse.de>
> > ---
> 
> Neil, I've sent a patch for this last week already removing
> __I_DIO_WAKEUP and it's in -next as
> 0009dc756e81 ("inode: remove __I_DIO_WAKEUP"). So you can drop this

Today's the day of getting things slightly wrong it seems...
2726a7a8477d8c0 is what I meant.
diff mbox series

Patch

diff --git a/fs/inode.c b/fs/inode.c
index 86670941884b..91bb2f80fa03 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -2467,18 +2467,6 @@  EXPORT_SYMBOL(inode_owner_or_capable);
 /*
  * Direct i/o helper functions
  */
-static void __inode_dio_wait(struct inode *inode)
-{
-	wait_queue_head_t *wq = bit_waitqueue(&inode->i_state, __I_DIO_WAKEUP);
-	DEFINE_WAIT_BIT(q, &inode->i_state, __I_DIO_WAKEUP);
-
-	do {
-		prepare_to_wait(wq, &q.wq_entry, TASK_UNINTERRUPTIBLE);
-		if (atomic_read(&inode->i_dio_count))
-			schedule();
-	} while (atomic_read(&inode->i_dio_count));
-	finish_wait(wq, &q.wq_entry);
-}
 
 /**
  * inode_dio_wait - wait for outstanding DIO requests to finish
@@ -2492,8 +2480,8 @@  static void __inode_dio_wait(struct inode *inode)
  */
 void inode_dio_wait(struct inode *inode)
 {
-	if (atomic_read(&inode->i_dio_count))
-		__inode_dio_wait(inode);
+	wait_var_event(&inode->i_dio_count,
+		       atomic_read(&inode->i_dio_count) == 0);
 }
 EXPORT_SYMBOL(inode_dio_wait);
 
diff --git a/fs/netfs/locking.c b/fs/netfs/locking.c
index 75dc52a49b3a..c171a0a48ed0 100644
--- a/fs/netfs/locking.c
+++ b/fs/netfs/locking.c
@@ -21,23 +21,8 @@ 
  */
 static int inode_dio_wait_interruptible(struct inode *inode)
 {
-	if (!atomic_read(&inode->i_dio_count))
-		return 0;
-
-	wait_queue_head_t *wq = bit_waitqueue(&inode->i_state, __I_DIO_WAKEUP);
-	DEFINE_WAIT_BIT(q, &inode->i_state, __I_DIO_WAKEUP);
-
-	for (;;) {
-		prepare_to_wait(wq, &q.wq_entry, TASK_INTERRUPTIBLE);
-		if (!atomic_read(&inode->i_dio_count))
-			break;
-		if (signal_pending(current))
-			break;
-		schedule();
-	}
-	finish_wait(wq, &q.wq_entry);
-
-	return atomic_read(&inode->i_dio_count) ? -ERESTARTSYS : 0;
+	return wait_var_event_interruptible(&inode->i_dio_count,
+					    !atomic_read(&inode->i_dio_count));
 }
 
 /* Call with exclusively locked inode->i_rwsem */
diff --git a/include/linux/fs.h b/include/linux/fs.h
index fd34b5755c0b..fc1b68134cbf 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2372,8 +2372,6 @@  static inline void kiocb_clone(struct kiocb *kiocb, struct kiocb *kiocb_src,
  *
  * I_REFERENCED		Marks the inode as recently references on the LRU list.
  *
- * I_DIO_WAKEUP		Never set.  Only used as a key for wait_on_bit().
- *
  * I_WB_SWITCH		Cgroup bdi_writeback switching in progress.  Used to
  *			synchronize competing switching instances and to tell
  *			wb stat updates to grab the i_pages lock.  See
@@ -2405,8 +2403,6 @@  static inline void kiocb_clone(struct kiocb *kiocb, struct kiocb *kiocb_src,
 #define __I_SYNC		7
 #define I_SYNC			(1 << __I_SYNC)
 #define I_REFERENCED		(1 << 8)
-#define __I_DIO_WAKEUP		9
-#define I_DIO_WAKEUP		(1 << __I_DIO_WAKEUP)
 #define I_LINKABLE		(1 << 10)
 #define I_DIRTY_TIME		(1 << 11)
 #define I_WB_SWITCH		(1 << 13)
@@ -3237,8 +3233,7 @@  static inline void inode_dio_begin(struct inode *inode)
  */
 static inline void inode_dio_end(struct inode *inode)
 {
-	if (atomic_dec_and_test(&inode->i_dio_count))
-		wake_up_bit(&inode->i_state, __I_DIO_WAKEUP);
+	atomic_dec_and_wake_up_var(&inode->i_dio_count);
 }
 
 extern void inode_set_flags(struct inode *inode, unsigned int flags,