Tim Chen <[email protected]> writes: > On Wed, 2017-03-08 at 15:26 +0800, Huang, Ying wrote: >> From: Huang Ying <[email protected]> >> >> 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 <[email protected]> >> Cc: Kirill A. Shutemov <[email protected]> >> Cc: Hugh Dickins <[email protected]> >> Cc: Shaohua Li <[email protected]> >> Cc: Minchan Kim <[email protected]> >> Cc: Rik van Riel <[email protected]> >> Signed-off-by: "Huang, Ying" <[email protected]> >> --- >> 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
