From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752667AbdGXCPi (ORCPT ); Sun, 23 Jul 2017 22:15:38 -0400 Received: from mga05.intel.com ([192.55.52.43]:37758 "EHLO mga05.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751782AbdGXCPb (ORCPT ); Sun, 23 Jul 2017 22:15:31 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.40,404,1496127600"; d="scan'208";a="881915932" From: "Huang\, Ying" To: Tim Chen Cc: Andrew Morton , Ying Huang , Wenwei Tao , , , Minchan Kim , "Rik van Riel" , Andrea Arcangeli , "Johannes Weiner" , Michal Hocko , Hillf Danton Subject: Re: [PATCH 2/2] mm/swap: Remove lock_initialized flag from swap_slots_cache References: <65a9d0f133f63e66bba37b53b2fd0464b7cae771.1500677066.git.tim.c.chen@linux.intel.com> <867d1fb070644e6d5f0ac7780f63e75259b82cc3.1500677066.git.tim.c.chen@linux.intel.com> Date: Mon, 24 Jul 2017 10:15:29 +0800 In-Reply-To: <867d1fb070644e6d5f0ac7780f63e75259b82cc3.1500677066.git.tim.c.chen@linux.intel.com> (Tim Chen's message of "Fri, 21 Jul 2017 15:45:01 -0700") Message-ID: <878tjeh96m.fsf@yhuang-dev.intel.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=ascii Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, Tim, Tim Chen writes: > We will only reach the lock initialization code > in alloc_swap_slot_cache when the cpu's swap_slots_cache's slots > have not been allocated and swap_slots_cache has not been initialized > previously. So the lock_initialized check is redundant and unnecessary. > Remove lock_initialized flag from swap_slots_cache to save memory. Is there a race condition with CPU offline/online when preempt is enabled? CPU A CPU B ----- ----- get_swap_page() get cache[B], cache[B]->slots != NULL preempted and moved to CPU A be offlined be onlined alloc_swap_slot_cache() mutex_lock(cache[B]->alloc_lock) mutex_init(cache[B]->alloc_lock) !!! The cache[B]->alloc_lock will be reinitialized when it is still held. Best Regards, Huang, Ying > Reported-by: Wenwei Tao > Signed-off-by: Tim Chen > --- > include/linux/swap_slots.h | 1 - > mm/swap_slots.c | 9 ++++----- > 2 files changed, 4 insertions(+), 6 deletions(-) > > diff --git a/include/linux/swap_slots.h b/include/linux/swap_slots.h > index 6ef92d1..a75c30b 100644 > --- a/include/linux/swap_slots.h > +++ b/include/linux/swap_slots.h > @@ -10,7 +10,6 @@ > #define THRESHOLD_DEACTIVATE_SWAP_SLOTS_CACHE (2*SWAP_SLOTS_CACHE_SIZE) > > struct swap_slots_cache { > - bool lock_initialized; > struct mutex alloc_lock; /* protects slots, nr, cur */ > swp_entry_t *slots; > int nr; > diff --git a/mm/swap_slots.c b/mm/swap_slots.c > index 4c5457c..c039e6c 100644 > --- a/mm/swap_slots.c > +++ b/mm/swap_slots.c > @@ -140,11 +140,10 @@ static int alloc_swap_slot_cache(unsigned int cpu) > if (cache->slots || cache->slots_ret) > /* cache already allocated */ > goto out; > - if (!cache->lock_initialized) { > - mutex_init(&cache->alloc_lock); > - spin_lock_init(&cache->free_lock); > - cache->lock_initialized = true; > - } > + > + mutex_init(&cache->alloc_lock); > + spin_lock_init(&cache->free_lock); > + > cache->nr = 0; > cache->cur = 0; > cache->n_ret = 0;