[PATCH 0/6] use irqsafe variant of refcount_dec_and_lock() / atomic_dec_and_lock()

[PATCH 0/6] use irqsafe variant of refcount_dec_and_lock() / atomic_dec_and_lock()

[Sebastian Andrzej Siewior]

The irqsave variant of refcount_dec_and_lock() and atomic_dec_and_lock()
made it into v4.18-rc2. This is just a repost of the users so that they
can be routed through the individual subsystems.
 
Sebastian
 
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-mm@kvack.org
Cc: Shaohua Li <shli@kernel.org>
Cc: linux-raid@vger.kernel.org

[PATCH 1/6] drivers/md/raid5: Use irqsave variant of atomic_dec_and_lock()

[Sebastian Andrzej Siewior]

From: Anna-Maria Gleixner <anna-maria@linutronix.de>

The irqsave variant of atomic_dec_and_lock handles irqsave/restore when
taking/releasing the spin lock. With this variant the call of
local_irq_save is no longer required.

Cc: Shaohua Li <shli@kernel.org>
Cc: linux-raid@vger.kernel.org
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Anna-Maria Gleixner <anna-maria@linutronix.de>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/md/raid5.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 2031506a0ecd..e933bff9459e 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -409,16 +409,15 @@ void raid5_release_stripe(struct stripe_head *sh)
 		md_wakeup_thread(conf->mddev->thread);
 	return;
 slow_path:
-	local_irq_save(flags);
 	/* we are ok here if STRIPE_ON_RELEASE_LIST is set or not */
-	if (atomic_dec_and_lock(&sh->count, &conf->device_lock)) {
+	if (atomic_dec_and_lock_irqsave(&sh->count, &conf->device_lock, flags)) {
 		INIT_LIST_HEAD(&list);
 		hash = sh->hash_lock_index;
 		do_release_stripe(conf, sh, &list);
 		spin_unlock(&conf->device_lock);
 		release_inactive_stripe_list(conf, &list, hash);
+		local_irq_restore(flags);
 	}
-	local_irq_restore(flags);
 }
 
 static inline void remove_hash(struct stripe_head *sh)
-- 
2.18.0

[PATCH 2/6] drivers/md/raid5: Do not disable irq on release_inactive_stripe_list() call

[Sebastian Andrzej Siewior]

From: Anna-Maria Gleixner <anna-maria@linutronix.de>

There is no need to invoke release_inactive_stripe_list() with interrupts
disabled. All call sites, except raid5_release_stripe(), unlock
->device_lock and enable interrupts before invoking the function.

Make it consistent.

Cc: Shaohua Li <shli@kernel.org>
Cc: linux-raid@vger.kernel.org
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Anna-Maria Gleixner <anna-maria@linutronix.de>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/md/raid5.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index e933bff9459e..ca1dd0cb04c5 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -414,9 +414,8 @@ void raid5_release_stripe(struct stripe_head *sh)
 		INIT_LIST_HEAD(&list);
 		hash = sh->hash_lock_index;
 		do_release_stripe(conf, sh, &list);
-		spin_unlock(&conf->device_lock);
+		spin_unlock_irqrestore(&conf->device_lock, flags);
 		release_inactive_stripe_list(conf, &list, hash);
-		local_irq_restore(flags);
 	}
 }
 
-- 
2.18.0

Re: [PATCH 1/6] drivers/md/raid5: Use irqsave variant of atomic_dec_and_lock()

[Sebastian Andrzej Siewior]

On 2018-07-03 22:01:36 [+0200], To linux-kernel@vger.kernel.org wrote:
> From: Anna-Maria Gleixner <anna-maria@linutronix.de>
> 
> The irqsave variant of atomic_dec_and_lock handles irqsave/restore when
> taking/releasing the spin lock. With this variant the call of
> local_irq_save is no longer required.

Shaohua, are you with this?

> Cc: Shaohua Li <shli@kernel.org>
> Cc: linux-raid@vger.kernel.org
> Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> Signed-off-by: Anna-Maria Gleixner <anna-maria@linutronix.de>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
>  drivers/md/raid5.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index 2031506a0ecd..e933bff9459e 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -409,16 +409,15 @@ void raid5_release_stripe(struct stripe_head *sh)
>  		md_wakeup_thread(conf->mddev->thread);
>  	return;
>  slow_path:
> -	local_irq_save(flags);
>  	/* we are ok here if STRIPE_ON_RELEASE_LIST is set or not */
> -	if (atomic_dec_and_lock(&sh->count, &conf->device_lock)) {
> +	if (atomic_dec_and_lock_irqsave(&sh->count, &conf->device_lock, flags)) {
>  		INIT_LIST_HEAD(&list);
>  		hash = sh->hash_lock_index;
>  		do_release_stripe(conf, sh, &list);
>  		spin_unlock(&conf->device_lock);
>  		release_inactive_stripe_list(conf, &list, hash);
> +		local_irq_restore(flags);
>  	}
> -	local_irq_restore(flags);
>  }
>  
>  static inline void remove_hash(struct stripe_head *sh)
> -- 
> 2.18.0
> 

Re: [PATCH 1/6] drivers/md/raid5: Use irqsave variant of atomic_dec_and_lock()

[Shaohua Li]

On Mon, Jul 16, 2018 at 02:27:40PM +0200, Sebastian Andrzej Siewior wrote:
> On 2018-07-03 22:01:36 [+0200], To linux-kernel@vger.kernel.org wrote:
> > From: Anna-Maria Gleixner <anna-maria@linutronix.de>
> > 
> > The irqsave variant of atomic_dec_and_lock handles irqsave/restore when
> > taking/releasing the spin lock. With this variant the call of
> > local_irq_save is no longer required.
> 
> Shaohua, are you with this?

Acked-by: Shaohua Li <shli@kernel.org> 
> > Cc: Shaohua Li <shli@kernel.org>
> > Cc: linux-raid@vger.kernel.org
> > Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> > Signed-off-by: Anna-Maria Gleixner <anna-maria@linutronix.de>
> > Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> > ---
> >  drivers/md/raid5.c | 5 ++---
> >  1 file changed, 2 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> > index 2031506a0ecd..e933bff9459e 100644
> > --- a/drivers/md/raid5.c
> > +++ b/drivers/md/raid5.c
> > @@ -409,16 +409,15 @@ void raid5_release_stripe(struct stripe_head *sh)
> >  		md_wakeup_thread(conf->mddev->thread);
> >  	return;
> >  slow_path:
> > -	local_irq_save(flags);
> >  	/* we are ok here if STRIPE_ON_RELEASE_LIST is set or not */
> > -	if (atomic_dec_and_lock(&sh->count, &conf->device_lock)) {
> > +	if (atomic_dec_and_lock_irqsave(&sh->count, &conf->device_lock, flags)) {
> >  		INIT_LIST_HEAD(&list);
> >  		hash = sh->hash_lock_index;
> >  		do_release_stripe(conf, sh, &list);
> >  		spin_unlock(&conf->device_lock);
> >  		release_inactive_stripe_list(conf, &list, hash);
> > +		local_irq_restore(flags);
> >  	}
> > -	local_irq_restore(flags);
> >  }
> >  
> >  static inline void remove_hash(struct stripe_head *sh)
> > -- 
> > 2.18.0
> > 

Re: [PATCH 1/6] drivers/md/raid5: Use irqsave variant of atomic_dec_and_lock()

[Sebastian Andrzej Siewior]

On 2018-07-16 17:37:27 [-0700], Shaohua Li wrote:
> On Mon, Jul 16, 2018 at 02:27:40PM +0200, Sebastian Andrzej Siewior wrote:
> > On 2018-07-03 22:01:36 [+0200], To linux-kernel@vger.kernel.org wrote:
> > > From: Anna-Maria Gleixner <anna-maria@linutronix.de>
> > > 
> > > The irqsave variant of atomic_dec_and_lock handles irqsave/restore when
> > > taking/releasing the spin lock. With this variant the call of
> > > local_irq_save is no longer required.
> > 
> > Shaohua, are you with this?
> 
> Acked-by: Shaohua Li <shli@kernel.org> 

Thank you. 
Is there a reason why you did not apply it or are you too busy now and
do it later (and just signal the ack that you are fine with it)?

Sebastian

Re: [PATCH 1/6] drivers/md/raid5: Use irqsave variant of atomic_dec_and_lock()

[Shaohua Li]

On Wed, Jul 18, 2018 at 12:57:21PM +0200, Sebastian Andrzej Siewior wrote:
> On 2018-07-16 17:37:27 [-0700], Shaohua Li wrote:
> > On Mon, Jul 16, 2018 at 02:27:40PM +0200, Sebastian Andrzej Siewior wrote:
> > > On 2018-07-03 22:01:36 [+0200], To linux-kernel@vger.kernel.org wrote:
> > > > From: Anna-Maria Gleixner <anna-maria@linutronix.de>
> > > > 
> > > > The irqsave variant of atomic_dec_and_lock handles irqsave/restore when
> > > > taking/releasing the spin lock. With this variant the call of
> > > > local_irq_save is no longer required.
> > > 
> > > Shaohua, are you with this?
> > 
> > Acked-by: Shaohua Li <shli@kernel.org> 
> 
> Thank you. 
> Is there a reason why you did not apply it or are you too busy now and
> do it later (and just signal the ack that you are fine with it)?

Since you sent a series, I suppose you want someone else to take it. But I can
take it for sure, will do soon.

Thanks,
Shaohua

Re: [PATCH 1/6] drivers/md/raid5: Use irqsave variant of atomic_dec_and_lock()

[Sebastian Andrzej Siewior]

On 2018-07-18 19:44:05 [-0700], Shaohua Li wrote:
> Since you sent a series, I suppose you want someone else to take it. But I can
> take it for sure, will do soon.

I'm sorry for the miss understanding. I tried to explain this in the
cover letter. The prerequisites are already merged so then the actual
changes can be merged via the individual maintainer.
Thank you Shaohua.

> Thanks,
> Shaohua

Sebastian

Re: [PATCH 2/6] drivers/md/raid5: Do not disable irq on release_inactive_stripe_list() call

[Sebastian Andrzej Siewior]

On 2018-07-03 22:01:37 [+0200], To linux-kernel@vger.kernel.org wrote:
> From: Anna-Maria Gleixner <anna-maria@linutronix.de>
> 
> There is no need to invoke release_inactive_stripe_list() with interrupts
> disabled. All call sites, except raid5_release_stripe(), unlock
> ->device_lock and enable interrupts before invoking the function.
> 
> Make it consistent.

Shaohua, I've seen that you already applied 1/6. Could you also please
apply this one? This is the only remaining raid5 patch :)

> Cc: Shaohua Li <shli@kernel.org>
> Cc: linux-raid@vger.kernel.org
> Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> Signed-off-by: Anna-Maria Gleixner <anna-maria@linutronix.de>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
>  drivers/md/raid5.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index e933bff9459e..ca1dd0cb04c5 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -414,9 +414,8 @@ void raid5_release_stripe(struct stripe_head *sh)
>  		INIT_LIST_HEAD(&list);
>  		hash = sh->hash_lock_index;
>  		do_release_stripe(conf, sh, &list);
> -		spin_unlock(&conf->device_lock);
> +		spin_unlock_irqrestore(&conf->device_lock, flags);
>  		release_inactive_stripe_list(conf, &list, hash);
> -		local_irq_restore(flags);
>  	}
>  }
>  

Sebastian