From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932129AbdDRE7Q (ORCPT ); Tue, 18 Apr 2017 00:59:16 -0400 Received: from LGEAMRELO13.lge.com ([156.147.23.53]:49898 "EHLO lgeamrelo13.lge.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932073AbdDRE7O (ORCPT ); Tue, 18 Apr 2017 00:59:14 -0400 X-Original-SENDERIP: 156.147.1.125 X-Original-MAILFROM: minchan@kernel.org X-Original-SENDERIP: 165.244.249.23 X-Original-MAILFROM: minchan@kernel.org X-Original-SENDERIP: 10.177.223.161 X-Original-MAILFROM: minchan@kernel.org Date: Tue, 18 Apr 2017 13:59:09 +0900 From: Minchan Kim To: "Huang, Ying" CC: Andrew Morton , , , Hugh Dickins , Shaohua Li , Rik van Riel Subject: Re: [PATCH -mm -v3] mm, swap: Sort swap entries before free Message-ID: <20170418045909.GA11015@bbox> References: <20170407064901.25398-1-ying.huang@intel.com> MIME-Version: 1.0 In-Reply-To: <20170407064901.25398-1-ying.huang@intel.com> User-Agent: Mutt/1.5.24 (2015-08-30) X-MIMETrack: Itemize by SMTP Server on LGEKRMHUB01/LGE/LG Group(Release 8.5.3FP6|November 21, 2013) at 2017/04/18 13:59:11, Serialize by Router on LGEKRMHUB01/LGE/LG Group(Release 8.5.3FP6|November 21, 2013) at 2017/04/18 13:59:11, Serialize complete at 2017/04/18 13:59:11 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Huang, On Fri, Apr 07, 2017 at 02:49:01PM +0800, Huang, Ying wrote: > From: Huang Ying > > To reduce the lock contention of swap_info_struct->lock when freeing > swap entry. The freed swap entries will be collected in a per-CPU > buffer firstly, and be really freed later in batch. During the batch > freeing, if the consecutive swap entries in the per-CPU buffer belongs > to same swap device, the swap_info_struct->lock needs to be > acquired/released only once, so that the lock contention could be > reduced greatly. But if there are multiple swap devices, it is > possible that the lock may be unnecessarily released/acquired because > the swap entries belong to the same swap device are non-consecutive in > the per-CPU buffer. > > To solve the issue, the per-CPU buffer is sorted according to the swap > device before freeing the swap entries. Test shows that the time > spent by swapcache_free_entries() could be reduced after the patch. > > Test the patch via measuring the run time of swap_cache_free_entries() > during the exit phase of the applications use much swap space. The > results shows that the average run time of swap_cache_free_entries() > reduced about 20% after applying the patch. > > Signed-off-by: Huang Ying > Acked-by: Tim Chen > Cc: Hugh Dickins > Cc: Shaohua Li > Cc: Minchan Kim > Cc: Rik van Riel > > v3: > > - Add some comments in code per Rik's suggestion. > > v2: > > - Avoid sort swap entries if there is only one swap device. > --- > mm/swapfile.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/mm/swapfile.c b/mm/swapfile.c > index 90054f3c2cdc..f23c56e9be39 100644 > --- a/mm/swapfile.c > +++ b/mm/swapfile.c > @@ -37,6 +37,7 @@ > #include > #include > #include > +#include > > #include > #include > @@ -1065,6 +1066,13 @@ void swapcache_free(swp_entry_t entry) > } > } > > +static int swp_entry_cmp(const void *ent1, const void *ent2) > +{ > + const swp_entry_t *e1 = ent1, *e2 = ent2; > + > + return (long)(swp_type(*e1) - swp_type(*e2)); > +} > + > void swapcache_free_entries(swp_entry_t *entries, int n) > { > struct swap_info_struct *p, *prev; > @@ -1075,6 +1083,10 @@ void swapcache_free_entries(swp_entry_t *entries, int n) > > prev = NULL; > p = NULL; > + > + /* Sort swap entries by swap device, so each lock is only taken once. */ > + if (nr_swapfiles > 1) > + sort(entries, n, sizeof(entries[0]), swp_entry_cmp, NULL); Let's think on other cases. There are two swaps and they are configured by priority so a swap's usage would be zero unless other swap used up. In case of that, this sorting is pointless. As well, nr_swapfiles is never decreased so if we enable multiple swaps and then disable until a swap is remained, this sorting is pointelss, too. How about lazy sorting approach? IOW, if we found prev != p and, then we can sort it. Thanks. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pf0-f199.google.com (mail-pf0-f199.google.com [209.85.192.199]) by kanga.kvack.org (Postfix) with ESMTP id 7524B6B0038 for ; Tue, 18 Apr 2017 00:59:14 -0400 (EDT) Received: by mail-pf0-f199.google.com with SMTP id a188so46230057pfa.3 for ; Mon, 17 Apr 2017 21:59:14 -0700 (PDT) Received: from lgeamrelo13.lge.com (LGEAMRELO13.lge.com. [156.147.23.53]) by mx.google.com with ESMTP id d8si13272813pgn.60.2017.04.17.21.59.12 for ; Mon, 17 Apr 2017 21:59:13 -0700 (PDT) Date: Tue, 18 Apr 2017 13:59:09 +0900 From: Minchan Kim Subject: Re: [PATCH -mm -v3] mm, swap: Sort swap entries before free Message-ID: <20170418045909.GA11015@bbox> References: <20170407064901.25398-1-ying.huang@intel.com> MIME-Version: 1.0 In-Reply-To: <20170407064901.25398-1-ying.huang@intel.com> Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline Sender: owner-linux-mm@kvack.org List-ID: To: "Huang, Ying" Cc: Andrew Morton , linux-mm@kvack.org, linux-kernel@vger.kernel.org, Hugh Dickins , Shaohua Li , Rik van Riel Hi Huang, On Fri, Apr 07, 2017 at 02:49:01PM +0800, Huang, Ying wrote: > From: Huang Ying > > To reduce the lock contention of swap_info_struct->lock when freeing > swap entry. The freed swap entries will be collected in a per-CPU > buffer firstly, and be really freed later in batch. During the batch > freeing, if the consecutive swap entries in the per-CPU buffer belongs > to same swap device, the swap_info_struct->lock needs to be > acquired/released only once, so that the lock contention could be > reduced greatly. But if there are multiple swap devices, it is > possible that the lock may be unnecessarily released/acquired because > the swap entries belong to the same swap device are non-consecutive in > the per-CPU buffer. > > To solve the issue, the per-CPU buffer is sorted according to the swap > device before freeing the swap entries. Test shows that the time > spent by swapcache_free_entries() could be reduced after the patch. > > Test the patch via measuring the run time of swap_cache_free_entries() > during the exit phase of the applications use much swap space. The > results shows that the average run time of swap_cache_free_entries() > reduced about 20% after applying the patch. > > Signed-off-by: Huang Ying > Acked-by: Tim Chen > Cc: Hugh Dickins > Cc: Shaohua Li > Cc: Minchan Kim > Cc: Rik van Riel > > v3: > > - Add some comments in code per Rik's suggestion. > > v2: > > - Avoid sort swap entries if there is only one swap device. > --- > mm/swapfile.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/mm/swapfile.c b/mm/swapfile.c > index 90054f3c2cdc..f23c56e9be39 100644 > --- a/mm/swapfile.c > +++ b/mm/swapfile.c > @@ -37,6 +37,7 @@ > #include > #include > #include > +#include > > #include > #include > @@ -1065,6 +1066,13 @@ void swapcache_free(swp_entry_t entry) > } > } > > +static int swp_entry_cmp(const void *ent1, const void *ent2) > +{ > + const swp_entry_t *e1 = ent1, *e2 = ent2; > + > + return (long)(swp_type(*e1) - swp_type(*e2)); > +} > + > void swapcache_free_entries(swp_entry_t *entries, int n) > { > struct swap_info_struct *p, *prev; > @@ -1075,6 +1083,10 @@ void swapcache_free_entries(swp_entry_t *entries, int n) > > prev = NULL; > p = NULL; > + > + /* Sort swap entries by swap device, so each lock is only taken once. */ > + if (nr_swapfiles > 1) > + sort(entries, n, sizeof(entries[0]), swp_entry_cmp, NULL); Let's think on other cases. There are two swaps and they are configured by priority so a swap's usage would be zero unless other swap used up. In case of that, this sorting is pointless. As well, nr_swapfiles is never decreased so if we enable multiple swaps and then disable until a swap is remained, this sorting is pointelss, too. How about lazy sorting approach? IOW, if we found prev != p and, then we can sort it. Thanks. -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org