* [PATCH 1/3] mm/swap: simplify alloc_swap_slot_cache()
2020-04-30 6:11 [PATCH 0/3] clean up some functions in mm/swap_slots.c Zhen Lei
@ 2020-04-30 6:11 ` Zhen Lei
2020-05-01 21:46 ` Tim Chen
2020-04-30 6:11 ` [PATCH 2/3] mm/swap: simplify enable_swap_slots_cache() Zhen Lei
` (2 subsequent siblings)
3 siblings, 1 reply; 8+ messages in thread
From: Zhen Lei @ 2020-04-30 6:11 UTC (permalink / raw)
To: Tim Chen, Andrew Morton, linux-mm, linux-kernel; +Cc: Zhen Lei
Both "slots" and "slots_ret" are only need to be freed when cache already
allocated. Make them closer, seems more clear.
No functional change.
Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
---
mm/swap_slots.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/mm/swap_slots.c b/mm/swap_slots.c
index 0975adc72253..01609b5f0c55 100644
--- a/mm/swap_slots.c
+++ b/mm/swap_slots.c
@@ -136,9 +136,16 @@ static int alloc_swap_slot_cache(unsigned int cpu)
mutex_lock(&swap_slots_cache_mutex);
cache = &per_cpu(swp_slots, cpu);
- if (cache->slots || cache->slots_ret)
+ if (cache->slots || cache->slots_ret) {
/* cache already allocated */
- goto out;
+ mutex_unlock(&swap_slots_cache_mutex);
+
+ kvfree(slots);
+ kvfree(slots_ret);
+
+ return 0;
+ }
+
if (!cache->lock_initialized) {
mutex_init(&cache->alloc_lock);
spin_lock_init(&cache->free_lock);
@@ -155,15 +162,8 @@ static int alloc_swap_slot_cache(unsigned int cpu)
*/
mb();
cache->slots = slots;
- slots = NULL;
cache->slots_ret = slots_ret;
- slots_ret = NULL;
-out:
mutex_unlock(&swap_slots_cache_mutex);
- if (slots)
- kvfree(slots);
- if (slots_ret)
- kvfree(slots_ret);
return 0;
}
--
2.26.0.106.g9fadedd
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH 1/3] mm/swap: simplify alloc_swap_slot_cache()
2020-04-30 6:11 ` [PATCH 1/3] mm/swap: simplify alloc_swap_slot_cache() Zhen Lei
@ 2020-05-01 21:46 ` Tim Chen
0 siblings, 0 replies; 8+ messages in thread
From: Tim Chen @ 2020-05-01 21:46 UTC (permalink / raw)
To: Zhen Lei, Andrew Morton, linux-mm, linux-kernel
On 4/29/20 11:11 PM, Zhen Lei wrote:
> Both "slots" and "slots_ret" are only need to be freed when cache already
> allocated. Make them closer, seems more clear.
>
> No functional change.
>
> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
> ---
> mm/swap_slots.c | 18 +++++++++---------
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/mm/swap_slots.c b/mm/swap_slots.c
> index 0975adc72253..01609b5f0c55 100644
> --- a/mm/swap_slots.c
> +++ b/mm/swap_slots.c
> @@ -136,9 +136,16 @@ static int alloc_swap_slot_cache(unsigned int cpu)
>
> mutex_lock(&swap_slots_cache_mutex);
> cache = &per_cpu(swp_slots, cpu);
> - if (cache->slots || cache->slots_ret)
> + if (cache->slots || cache->slots_ret) {
> /* cache already allocated */
> - goto out;
> + mutex_unlock(&swap_slots_cache_mutex);
> +
> + kvfree(slots);
> + kvfree(slots_ret);
> +
> + return 0;
> + }
> +
> if (!cache->lock_initialized) {
> mutex_init(&cache->alloc_lock);
> spin_lock_init(&cache->free_lock);
> @@ -155,15 +162,8 @@ static int alloc_swap_slot_cache(unsigned int cpu)
> */
> mb();
> cache->slots = slots;
> - slots = NULL;
> cache->slots_ret = slots_ret;
> - slots_ret = NULL;
> -out:
> mutex_unlock(&swap_slots_cache_mutex);
> - if (slots)
> - kvfree(slots);
> - if (slots_ret)
> - kvfree(slots_ret);
> return 0;
> }
>
>
Acked-by: Tim Chen <tim.c.chen@linux.intel.com>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/3] mm/swap: simplify enable_swap_slots_cache()
2020-04-30 6:11 [PATCH 0/3] clean up some functions in mm/swap_slots.c Zhen Lei
2020-04-30 6:11 ` [PATCH 1/3] mm/swap: simplify alloc_swap_slot_cache() Zhen Lei
@ 2020-04-30 6:11 ` Zhen Lei
2020-05-01 21:59 ` Tim Chen
2020-04-30 6:11 ` [PATCH 3/3] mm/swap: remove redundant check for swap_slot_cache_initialized Zhen Lei
2020-07-08 9:27 ` [PATCH 0/3] clean up some functions in mm/swap_slots.c Leizhen (ThunderTown)
3 siblings, 1 reply; 8+ messages in thread
From: Zhen Lei @ 2020-04-30 6:11 UTC (permalink / raw)
To: Tim Chen, Andrew Morton, linux-mm, linux-kernel; +Cc: Zhen Lei
Whether swap_slot_cache_initialized is true or false,
__reenable_swap_slots_cache() is always called. To make this meaning
clear, leave only one call to __reenable_swap_slots_cache(). This also
make it clearer what extra needs be done when swap_slot_cache_initialized
is false.
No functional change.
Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
---
mm/swap_slots.c | 22 ++++++++++------------
1 file changed, 10 insertions(+), 12 deletions(-)
diff --git a/mm/swap_slots.c b/mm/swap_slots.c
index 01609b5f0c55..b40394473a3c 100644
--- a/mm/swap_slots.c
+++ b/mm/swap_slots.c
@@ -240,21 +240,19 @@ static int free_slot_cache(unsigned int cpu)
int enable_swap_slots_cache(void)
{
- int ret = 0;
-
mutex_lock(&swap_slots_cache_enable_mutex);
- if (swap_slot_cache_initialized) {
- __reenable_swap_slots_cache();
- goto out_unlock;
- }
+ if (!swap_slot_cache_initialized) {
+ int ret;
- ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "swap_slots_cache",
- alloc_swap_slot_cache, free_slot_cache);
- if (WARN_ONCE(ret < 0, "Cache allocation failed (%s), operating "
- "without swap slots cache.\n", __func__))
- goto out_unlock;
+ ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "swap_slots_cache",
+ alloc_swap_slot_cache, free_slot_cache);
+ if (WARN_ONCE(ret < 0, "Cache allocation failed (%s), operating "
+ "without swap slots cache.\n", __func__))
+ goto out_unlock;
+
+ swap_slot_cache_initialized = true;
+ }
- swap_slot_cache_initialized = true;
__reenable_swap_slots_cache();
out_unlock:
mutex_unlock(&swap_slots_cache_enable_mutex);
--
2.26.0.106.g9fadedd
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH 2/3] mm/swap: simplify enable_swap_slots_cache()
2020-04-30 6:11 ` [PATCH 2/3] mm/swap: simplify enable_swap_slots_cache() Zhen Lei
@ 2020-05-01 21:59 ` Tim Chen
0 siblings, 0 replies; 8+ messages in thread
From: Tim Chen @ 2020-05-01 21:59 UTC (permalink / raw)
To: Zhen Lei, Andrew Morton, linux-mm, linux-kernel
On 4/29/20 11:11 PM, Zhen Lei wrote:
> Whether swap_slot_cache_initialized is true or false,
> __reenable_swap_slots_cache() is always called. To make this meaning
> clear, leave only one call to __reenable_swap_slots_cache(). This also
> make it clearer what extra needs be done when swap_slot_cache_initialized
> is false.
>
> No functional change.
>
> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
Acked-by: Tim Chen <tim.c.chen@linux.intel.com>
> ---
> mm/swap_slots.c | 22 ++++++++++------------
> 1 file changed, 10 insertions(+), 12 deletions(-)
>
> diff --git a/mm/swap_slots.c b/mm/swap_slots.c
> index 01609b5f0c55..b40394473a3c 100644
> --- a/mm/swap_slots.c
> +++ b/mm/swap_slots.c
> @@ -240,21 +240,19 @@ static int free_slot_cache(unsigned int cpu)
>
> int enable_swap_slots_cache(void)
> {
> - int ret = 0;
> -
> mutex_lock(&swap_slots_cache_enable_mutex);
> - if (swap_slot_cache_initialized) {
> - __reenable_swap_slots_cache();
> - goto out_unlock;
> - }
> + if (!swap_slot_cache_initialized) {
> + int ret;
>
> - ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "swap_slots_cache",
> - alloc_swap_slot_cache, free_slot_cache);
> - if (WARN_ONCE(ret < 0, "Cache allocation failed (%s), operating "
> - "without swap slots cache.\n", __func__))
> - goto out_unlock;
> + ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "swap_slots_cache",
> + alloc_swap_slot_cache, free_slot_cache);
> + if (WARN_ONCE(ret < 0, "Cache allocation failed (%s), operating "
> + "without swap slots cache.\n", __func__))
> + goto out_unlock;
> +
> + swap_slot_cache_initialized = true;
> + }
>
> - swap_slot_cache_initialized = true;
> __reenable_swap_slots_cache();
> out_unlock:
> mutex_unlock(&swap_slots_cache_enable_mutex);
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 3/3] mm/swap: remove redundant check for swap_slot_cache_initialized
2020-04-30 6:11 [PATCH 0/3] clean up some functions in mm/swap_slots.c Zhen Lei
2020-04-30 6:11 ` [PATCH 1/3] mm/swap: simplify alloc_swap_slot_cache() Zhen Lei
2020-04-30 6:11 ` [PATCH 2/3] mm/swap: simplify enable_swap_slots_cache() Zhen Lei
@ 2020-04-30 6:11 ` Zhen Lei
2020-05-01 22:35 ` Tim Chen
2020-07-08 9:27 ` [PATCH 0/3] clean up some functions in mm/swap_slots.c Leizhen (ThunderTown)
3 siblings, 1 reply; 8+ messages in thread
From: Zhen Lei @ 2020-04-30 6:11 UTC (permalink / raw)
To: Tim Chen, Andrew Morton, linux-mm, linux-kernel; +Cc: Zhen Lei
Because enable_swap_slots_cache can only become true in
enable_swap_slots_cache(), and depends on swap_slot_cache_initialized is
true before. That means, when enable_swap_slots_cache is true,
swap_slot_cache_initialized is true also.
So the condition:
"swap_slot_cache_enabled && swap_slot_cache_initialized"
can be reduced to "swap_slot_cache_enabled"
And in mathematics:
"!swap_slot_cache_enabled || !swap_slot_cache_initialized"
is equal to "!(swap_slot_cache_enabled && swap_slot_cache_initialized)"
So no functional change.
Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
---
mm/swap_slots.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/mm/swap_slots.c b/mm/swap_slots.c
index b40394473a3c..3e6453573a89 100644
--- a/mm/swap_slots.c
+++ b/mm/swap_slots.c
@@ -46,8 +46,7 @@ static void __drain_swap_slots_cache(unsigned int type);
static void deactivate_swap_slots_cache(void);
static void reactivate_swap_slots_cache(void);
-#define use_swap_slot_cache (swap_slot_cache_active && \
- swap_slot_cache_enabled && swap_slot_cache_initialized)
+#define use_swap_slot_cache (swap_slot_cache_active && swap_slot_cache_enabled)
#define SLOTS_CACHE 0x1
#define SLOTS_CACHE_RET 0x2
@@ -94,7 +93,7 @@ static bool check_cache_active(void)
{
long pages;
- if (!swap_slot_cache_enabled || !swap_slot_cache_initialized)
+ if (!swap_slot_cache_enabled)
return false;
pages = get_nr_swap_pages();
--
2.26.0.106.g9fadedd
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH 3/3] mm/swap: remove redundant check for swap_slot_cache_initialized
2020-04-30 6:11 ` [PATCH 3/3] mm/swap: remove redundant check for swap_slot_cache_initialized Zhen Lei
@ 2020-05-01 22:35 ` Tim Chen
0 siblings, 0 replies; 8+ messages in thread
From: Tim Chen @ 2020-05-01 22:35 UTC (permalink / raw)
To: Zhen Lei, Andrew Morton, linux-mm, linux-kernel
On 4/29/20 11:11 PM, Zhen Lei wrote:
> Because enable_swap_slots_cache can only become true in
> enable_swap_slots_cache(), and depends on swap_slot_cache_initialized is
> true before. That means, when enable_swap_slots_cache is true,
> swap_slot_cache_initialized is true also.
>
> So the condition:
> "swap_slot_cache_enabled && swap_slot_cache_initialized"
> can be reduced to "swap_slot_cache_enabled"
>
> And in mathematics:
> "!swap_slot_cache_enabled || !swap_slot_cache_initialized"
> is equal to "!(swap_slot_cache_enabled && swap_slot_cache_initialized)"
>
> So no functional change.
>
> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
> ---
> mm/swap_slots.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/mm/swap_slots.c b/mm/swap_slots.c
> index b40394473a3c..3e6453573a89 100644
> --- a/mm/swap_slots.c
> +++ b/mm/swap_slots.c
> @@ -46,8 +46,7 @@ static void __drain_swap_slots_cache(unsigned int type);
> static void deactivate_swap_slots_cache(void);
> static void reactivate_swap_slots_cache(void);
>
> -#define use_swap_slot_cache (swap_slot_cache_active && \
> - swap_slot_cache_enabled && swap_slot_cache_initialized)
> +#define use_swap_slot_cache (swap_slot_cache_active && swap_slot_cache_enabled)
Yes, swap_slot_cache_enabled does imply swap_slot_cache_initialized
in current code. So checking swap_slot_cache_enabled is
enough here.
> #define SLOTS_CACHE 0x1
> #define SLOTS_CACHE_RET 0x2
>
> @@ -94,7 +93,7 @@ static bool check_cache_active(void)
> {
> long pages;
>
> - if (!swap_slot_cache_enabled || !swap_slot_cache_initialized)
> + if (!swap_slot_cache_enabled)
This simplification is okay. !swap_slot_cache_initialize implies !swap_slot_cache_enabled.
So only !swap_slot_cache_enabled needs to be checked.
> return false;
>
> pages = get_nr_swap_pages();
>
Acked-by: Tim Chen <tim.c.chen@linux.intel.com>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 0/3] clean up some functions in mm/swap_slots.c
2020-04-30 6:11 [PATCH 0/3] clean up some functions in mm/swap_slots.c Zhen Lei
` (2 preceding siblings ...)
2020-04-30 6:11 ` [PATCH 3/3] mm/swap: remove redundant check for swap_slot_cache_initialized Zhen Lei
@ 2020-07-08 9:27 ` Leizhen (ThunderTown)
3 siblings, 0 replies; 8+ messages in thread
From: Leizhen (ThunderTown) @ 2020-07-08 9:27 UTC (permalink / raw)
To: Tim Chen, Andrew Morton, linux-mm, linux-kernel
Hi, all:
Are these patches acceptable？
All these three patches are "Acked-by: Tim Chen <tim.c.chen@linux.intel.com>" two months ago.
On 2020/4/30 14:11, Zhen Lei wrote:
> When I studied the code of mm/swap_slots.c, I found some places can be improved.
>
> Zhen Lei (3):
> mm/swap: simplify alloc_swap_slot_cache()
> mm/swap: simplify enable_swap_slots_cache()
> mm/swap: remove redundant check for swap_slot_cache_initialized
>
> mm/swap_slots.c | 45 +++++++++++++++++++++------------------------
> 1 file changed, 21 insertions(+), 24 deletions(-)
>
^ permalink raw reply [flat|nested] 8+ messages in thread