Re: [PATCH RFC v2 5/6] inode: port __I_LRU_ISOLATING to var event

[Christian Brauner <brauner@kernel.org>]

On Wed, Aug 21, 2024 at 03:41:45PM GMT, Jeff Layton wrote:
> On Wed, 2024-08-21 at 17:47 +0200, Christian Brauner wrote:
> > Port the __I_LRU_ISOLATING mechanism to use the new var event mechanism.
> > 
> > Signed-off-by: Christian Brauner <brauner@kernel.org>
> > ---
> >  fs/inode.c | 26 +++++++++++++++++---------
> >  1 file changed, 17 insertions(+), 9 deletions(-)
> > 
> > diff --git a/fs/inode.c b/fs/inode.c
> > index d18e1567c487..c8a5c63dc980 100644
> > --- a/fs/inode.c
> > +++ b/fs/inode.c
> > @@ -510,8 +510,7 @@ static void inode_unpin_lru_isolating(struct inode *inode)
> >  	spin_lock(&inode->i_lock);
> >  	WARN_ON(!(inode->i_state & I_LRU_ISOLATING));
> >  	inode->i_state &= ~I_LRU_ISOLATING;
> > -	smp_mb();
> > -	wake_up_bit(&inode->i_state, __I_LRU_ISOLATING);
> > +	inode_wake_up_bit(inode, __I_LRU_ISOLATING);
> >  	spin_unlock(&inode->i_lock);
> >  }
> >  
> > @@ -519,13 +518,22 @@ static void inode_wait_for_lru_isolating(struct inode *inode)
> >  {
> >  	lockdep_assert_held(&inode->i_lock);
> >  	if (inode->i_state & I_LRU_ISOLATING) {
> > -		DEFINE_WAIT_BIT(wq, &inode->i_state, __I_LRU_ISOLATING);
> > -		wait_queue_head_t *wqh;
> > -
> > -		wqh = bit_waitqueue(&inode->i_state, __I_LRU_ISOLATING);
> > -		spin_unlock(&inode->i_lock);
> > -		__wait_on_bit(wqh, &wq, bit_wait, TASK_UNINTERRUPTIBLE);
> > -		spin_lock(&inode->i_lock);
> > +		struct wait_bit_queue_entry wqe;
> > +		struct wait_queue_head *wq_head;
> > +
> > +		wq_head = inode_bit_waitqueue(&wqe, inode, __I_LRU_ISOLATING);
> > +		for (;;) {
> > +			prepare_to_wait_event(wq_head, &wqe.wq_entry,
> > +					      TASK_UNINTERRUPTIBLE);
> > +			if (inode->i_state & I_LRU_ISOLATING) {
> > +				spin_unlock(&inode->i_lock);
> > +				schedule();
> > +				spin_lock(&inode->i_lock);
> > +				continue;
> > +			}
> > +			break;
> > +		}
> 
> nit: personally, I'd prefer this, since you wouldn't need the brackets
> or the continue:
> 
> 			if (!(inode->i_state & LRU_ISOLATING))
> 				break;
> 			spin_unlock();
> 			schedule();

Yeah, that's nicer. I'll use that.