[PATCH RT] mm/memcontrol: Move misplaced local_unlock_irqrestore()

[PATCH RT] mm/memcontrol: Move misplaced local_unlock_irqrestore()

[Matt Fleming]

There's no need to leave interrupts disabled when calling css_put_many().

Cc: Daniel Wagner <wagi@monom.org>
Signed-off-by: Matt Fleming <matt@codeblueprint.co.uk>
---
 mm/memcontrol.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 7b6f208c5a6b..1120b9d8dd86 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -7062,10 +7062,10 @@ void mem_cgroup_swapout(struct page *page, swp_entry_t entry)
 	mem_cgroup_charge_statistics(memcg, page, PageTransHuge(page),
 				     -nr_entries);
 	memcg_check_events(memcg, page);
+	local_unlock_irqrestore(event_lock, flags);
 
 	if (!mem_cgroup_is_root(memcg))
 		css_put_many(&memcg->css, nr_entries);
-	local_unlock_irqrestore(event_lock, flags);
 }
 
 /**
-- 
2.16.4

Re: [PATCH RT] mm/memcontrol: Move misplaced local_unlock_irqrestore()

[Sebastian Andrzej Siewior]

On 2020-01-26 21:19:45 [+0000], Matt Fleming wrote:
> There's no need to leave interrupts disabled when calling css_put_many().

For RT the interrupts are never actually disabled and for !RT they are
disabled with or without the change.
The comment about the disable function mentions just the counters and
css_put_many()'s callback just invokes a worker so it is probably save
to move the function as suggested.

May I ask how on earth you managed to open that file on a Sunday
evening?

> Cc: Daniel Wagner <wagi@monom.org>
> Signed-off-by: Matt Fleming <matt@codeblueprint.co.uk>
> ---
>  mm/memcontrol.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 7b6f208c5a6b..1120b9d8dd86 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -7062,10 +7062,10 @@ void mem_cgroup_swapout(struct page *page, swp_entry_t entry)
>  	mem_cgroup_charge_statistics(memcg, page, PageTransHuge(page),
>  				     -nr_entries);
>  	memcg_check_events(memcg, page);
> +	local_unlock_irqrestore(event_lock, flags);
>  
>  	if (!mem_cgroup_is_root(memcg))
>  		css_put_many(&memcg->css, nr_entries);
> -	local_unlock_irqrestore(event_lock, flags);
>  }
>  
>  /**

Sebastian

Re: [PATCH RT] mm/memcontrol: Move misplaced local_unlock_irqrestore()

[Matt Fleming]

On Mon, 03 Feb, at 07:17:46PM, Sebastian Andrzej Siewior wrote:
> On 2020-01-26 21:19:45 [+0000], Matt Fleming wrote:
> > There's no need to leave interrupts disabled when calling css_put_many().
> 
> For RT the interrupts are never actually disabled and for !RT they are
> disabled with or without the change.
> The comment about the disable function mentions just the counters and
> css_put_many()'s callback just invokes a worker so it is probably save
> to move the function as suggested.
> 
> May I ask how on earth you managed to open that file on a Sunday
> evening?

We're carrying it in some of our older SUSE RT kernels and I'd really
like to get it upstream or sent to /dev/null ;)

Re: [PATCH RT] mm/memcontrol: Move misplaced local_unlock_irqrestore()

[Mike Galbraith]

On Tue, 2020-02-04 at 09:35 +0000, Matt Fleming wrote:
> On Mon, 03 Feb, at 07:17:46PM, Sebastian Andrzej Siewior wrote:
> > On 2020-01-26 21:19:45 [+0000], Matt Fleming wrote:
> > > There's no need to leave interrupts disabled when calling css_put_many().
> >
> > For RT the interrupts are never actually disabled and for !RT they are
> > disabled with or without the change.
> > The comment about the disable function mentions just the counters and
> > css_put_many()'s callback just invokes a worker so it is probably save
> > to move the function as suggested.
> >
> > May I ask how on earth you managed to open that file on a Sunday
> > evening?
>
> We're carrying it in some of our older SUSE RT kernels and I'd really
> like to get it upstream or sent to /dev/null ;)

My recollection of the reason for that patchlet existing was simply
because while rummaging around one day, unlock placement offended my
eye a bit, so I did a dinky on the spot correction.

	-Mike