From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751151AbdCOBTc (ORCPT ); Tue, 14 Mar 2017 21:19:32 -0400 Received: from mga14.intel.com ([192.55.52.115]:51439 "EHLO mga14.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750805AbdCOBTb (ORCPT ); Tue, 14 Mar 2017 21:19:31 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.36,166,1486454400"; d="scan'208";a="236214913" From: "Huang\, Ying" To: Tim Chen Cc: "Huang\, Ying" , Andrew Morton , , , Andrea Arcangeli , "Kirill A . Shutemov" , Hugh Dickins , "Shaohua Li" , Minchan Kim , Rik van Riel Subject: Re: [PATCH -mm -v6 3/9] mm, THP, swap: Add swap cluster allocate/free functions References: <20170308072613.17634-1-ying.huang@intel.com> <20170308072613.17634-4-ying.huang@intel.com> <1489533213.2733.33.camel@linux.intel.com> Date: Wed, 15 Mar 2017 09:19:27 +0800 In-Reply-To: <1489533213.2733.33.camel@linux.intel.com> (Tim Chen's message of "Tue, 14 Mar 2017 16:13:33 -0700") Message-ID: <87wpbrcp5s.fsf@yhuang-dev.intel.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Tim Chen writes: > On Wed, 2017-03-08 at 15:26 +0800, Huang, Ying wrote: >> From: Huang Ying >> >> The swap cluster allocation/free functions are added based on the >> existing swap cluster management mechanism for SSD.  These functions >> don't work for the rotating hard disks because the existing swap cluster >> management mechanism doesn't work for them.  The hard disks support may >> be added if someone really need it.  But that needn't be included in >> this patchset. >> >> This will be used for the THP (Transparent Huge Page) swap support. >> Where one swap cluster will hold the contents of each THP swapped out. >> >> Cc: Andrea Arcangeli >> Cc: Kirill A. Shutemov >> Cc: Hugh Dickins >> Cc: Shaohua Li >> Cc: Minchan Kim >> Cc: Rik van Riel >> Signed-off-by: "Huang, Ying" >> --- >>  mm/swapfile.c | 217 +++++++++++++++++++++++++++++++++++++++++----------------- >>  1 file changed, 156 insertions(+), 61 deletions(-) >> >> diff --git a/mm/swapfile.c b/mm/swapfile.c >> index a744604384ff..91876c33114b 100644 >> --- a/mm/swapfile.c >> +++ b/mm/swapfile.c >> @@ -378,6 +378,14 @@ static void swap_cluster_schedule_discard(struct swap_info_struct *si, >>   schedule_work(&si->discard_work); >>  } >>   >> +static void __free_cluster(struct swap_info_struct *si, unsigned long idx) >> +{ >> + struct swap_cluster_info *ci = si->cluster_info; >> + >> + cluster_set_flag(ci + idx, CLUSTER_FLAG_FREE); >> + cluster_list_add_tail(&si->free_clusters, ci, idx); >> +} >> + >>  /* >>   * Doing discard actually. After a cluster discard is finished, the cluster >>   * will be added to free cluster list. caller should hold si->lock. >> @@ -398,10 +406,7 @@ static void swap_do_scheduled_discard(struct swap_info_struct *si) >>   >>   spin_lock(&si->lock); >>   ci = lock_cluster(si, idx * SWAPFILE_CLUSTER); >> - cluster_set_flag(ci, CLUSTER_FLAG_FREE); >> - unlock_cluster(ci); >> - cluster_list_add_tail(&si->free_clusters, info, idx); >> - ci = lock_cluster(si, idx * SWAPFILE_CLUSTER); >> + __free_cluster(si, idx); >>   memset(si->swap_map + idx * SWAPFILE_CLUSTER, >>   0, SWAPFILE_CLUSTER); >>   unlock_cluster(ci); > > The __free_cluster definition and the above change to eliminate > the extra unlock_cluster and lock_cluster can perhaps be broken up > as a separate patch.  It can be independent of THP changes. I think the change may have no value by itself without THP changes. There will be only 1 user of __free_cluster() and the lock change is trivial too. So I think it may be better just to keep it as that? Best Regards, Huang, Ying