Re: [PATCH v3 3/6] inode: port __I_SYNC to var event

[Jan Kara <jack@suse.cz>]

On Fri 23-08-24 14:47:37, Christian Brauner wrote:
> Port the __I_SYNC mechanism to use the new var event mechanism.
> 
> Signed-off-by: Christian Brauner <brauner@kernel.org>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/fs-writeback.c | 45 +++++++++++++++++++++++++++++----------------
>  1 file changed, 29 insertions(+), 16 deletions(-)
> 
> diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
> index 1a5006329f6f..d8bec3c1bb1f 100644
> --- a/fs/fs-writeback.c
> +++ b/fs/fs-writeback.c
> @@ -1386,12 +1386,13 @@ static void requeue_io(struct inode *inode, struct bdi_writeback *wb)
>  
>  static void inode_sync_complete(struct inode *inode)
>  {
> +	assert_spin_locked(&inode->i_lock);
> +
>  	inode->i_state &= ~I_SYNC;
>  	/* If inode is clean an unused, put it into LRU now... */
>  	inode_add_lru(inode);
> -	/* Waiters must see I_SYNC cleared before being woken up */
> -	smp_mb();
> -	wake_up_bit(&inode->i_state, __I_SYNC);
> +	/* Called with inode->i_lock which ensures memory ordering. */
> +	inode_wake_up_bit(inode, __I_SYNC);
>  }
>  
>  static bool inode_dirtied_after(struct inode *inode, unsigned long t)
> @@ -1512,17 +1513,25 @@ static int write_inode(struct inode *inode, struct writeback_control *wbc)
>   */
>  void inode_wait_for_writeback(struct inode *inode)
>  {
> -	DEFINE_WAIT_BIT(wq, &inode->i_state, __I_SYNC);
> -	wait_queue_head_t *wqh;
> +	struct wait_bit_queue_entry wqe;
> +	struct wait_queue_head *wq_head;
> +
> +	assert_spin_locked(&inode->i_lock);
> +
> +	if (!(inode->i_state & I_SYNC))
> +		return;
>  
> -	lockdep_assert_held(&inode->i_lock);
> -	wqh = bit_waitqueue(&inode->i_state, __I_SYNC);
> -	while (inode->i_state & I_SYNC) {
> +	wq_head = inode_bit_waitqueue(&wqe, inode, __I_SYNC);
> +	for (;;) {
> +		prepare_to_wait_event(wq_head, &wqe.wq_entry, TASK_UNINTERRUPTIBLE);
> +		/* Checking I_SYNC with inode->i_lock guarantees memory ordering. */
> +		if (!(inode->i_state & I_SYNC))
> +			break;
>  		spin_unlock(&inode->i_lock);
> -		__wait_on_bit(wqh, &wq, bit_wait,
> -			      TASK_UNINTERRUPTIBLE);
> +		schedule();
>  		spin_lock(&inode->i_lock);
>  	}
> +	finish_wait(wq_head, &wqe.wq_entry);
>  }
>  
>  /*
> @@ -1533,16 +1542,20 @@ void inode_wait_for_writeback(struct inode *inode)
>  static void inode_sleep_on_writeback(struct inode *inode)
>  	__releases(inode->i_lock)
>  {
> -	DEFINE_WAIT(wait);
> -	wait_queue_head_t *wqh = bit_waitqueue(&inode->i_state, __I_SYNC);
> -	int sleep;
> +	struct wait_bit_queue_entry wqe;
> +	struct wait_queue_head *wq_head;
> +	bool sleep;
> +
> +	assert_spin_locked(&inode->i_lock);
>  
> -	prepare_to_wait(wqh, &wait, TASK_UNINTERRUPTIBLE);
> -	sleep = inode->i_state & I_SYNC;
> +	wq_head = inode_bit_waitqueue(&wqe, inode, __I_SYNC);
> +	prepare_to_wait_event(wq_head, &wqe.wq_entry, TASK_UNINTERRUPTIBLE);
> +	/* Checking I_SYNC with inode->i_lock guarantees memory ordering. */
> +	sleep = !!(inode->i_state & I_SYNC);
>  	spin_unlock(&inode->i_lock);
>  	if (sleep)
>  		schedule();
> -	finish_wait(wqh, &wait);
> +	finish_wait(wq_head, &wqe.wq_entry);
>  }
>  
>  /*
> 
> -- 
> 2.43.0
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR