| Message ID | 20210109080943.34832-1-linmiaohe@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mm/swap_slots.c: Remove unnecessary NULL pointer check | expand |
On Sat, Jan 09, 2021 at 03:09:43AM -0500, Miaohe Lin wrote: > The cache->slots and cache->slots_ret is already checked before we try to > drain it. And kvfree can handle the NULL pointer itself. So remove the > NULL pointer check here. > @@ -178,7 +178,7 @@ static void drain_slots_cache_cpu(unsigned int cpu, unsigned int type, > swapcache_free_entries(cache->slots + cache->cur, cache->nr); > cache->cur = 0; > cache->nr = 0; > - if (free_slots && cache->slots) { > + if (free_slots) { Prove that swapcache_free_entries() doesn't change cache->slots. > @@ -188,13 +188,12 @@ static void drain_slots_cache_cpu(unsigned int cpu, unsigned int type, > spin_lock_irq(&cache->free_lock); > swapcache_free_entries(cache->slots_ret, cache->n_ret); > cache->n_ret = 0; > - if (free_slots && cache->slots_ret) { > + if (free_slots) { ... or ->slots_ret > - if (slots) > - kvfree(slots); > + kvfree(slots); This is fine.
Hi: On 2021/1/10 1:40, Matthew Wilcox wrote: > On Sat, Jan 09, 2021 at 03:09:43AM -0500, Miaohe Lin wrote: >> The cache->slots and cache->slots_ret is already checked before we try to >> drain it. And kvfree can handle the NULL pointer itself. So remove the >> NULL pointer check here. > >> @@ -178,7 +178,7 @@ static void drain_slots_cache_cpu(unsigned int cpu, unsigned int type, >> swapcache_free_entries(cache->slots + cache->cur, cache->nr); >> cache->cur = 0; >> cache->nr = 0; >> - if (free_slots && cache->slots) { >> + if (free_slots) { > > Prove that swapcache_free_entries() doesn't change cache->slots. > Yeh... I see. I thought swap_slots_cache_mutex could totally guard against this. >> @@ -188,13 +188,12 @@ static void drain_slots_cache_cpu(unsigned int cpu, unsigned int type, >> spin_lock_irq(&cache->free_lock); >> swapcache_free_entries(cache->slots_ret, cache->n_ret); >> cache->n_ret = 0; >> - if (free_slots && cache->slots_ret) { >> + if (free_slots) { > > ... or ->slots_ret > >> - if (slots) >> - kvfree(slots); >> + kvfree(slots); > > This is fine. > . > Many thanks for your review and reply!
diff --git a/mm/swap_slots.c b/mm/swap_slots.c index 0357fbe70645..4cf99ce033d0 100644 --- a/mm/swap_slots.c +++ b/mm/swap_slots.c @@ -178,7 +178,7 @@ static void drain_slots_cache_cpu(unsigned int cpu, unsigned int type, swapcache_free_entries(cache->slots + cache->cur, cache->nr); cache->cur = 0; cache->nr = 0; - if (free_slots && cache->slots) { + if (free_slots) { kvfree(cache->slots); cache->slots = NULL; } @@ -188,13 +188,12 @@ static void drain_slots_cache_cpu(unsigned int cpu, unsigned int type, spin_lock_irq(&cache->free_lock); swapcache_free_entries(cache->slots_ret, cache->n_ret); cache->n_ret = 0; - if (free_slots && cache->slots_ret) { + if (free_slots) { slots = cache->slots_ret; cache->slots_ret = NULL; } spin_unlock_irq(&cache->free_lock); - if (slots) - kvfree(slots); + kvfree(slots); } }
The cache->slots and cache->slots_ret is already checked before we try to drain it. And kvfree can handle the NULL pointer itself. So remove the NULL pointer check here. Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> --- mm/swap_slots.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)