diff mbox

[-mm,-v4,02/21] mm, THP, swap: Make CONFIG_THP_SWAP depends on CONFIG_SWAP

Message ID 20180622035151.6676-3-ying.huang@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Huang, Ying June 22, 2018, 3:51 a.m. UTC 
From: Huang Ying <ying.huang@intel.com>

It's unreasonable to optimize swapping for THP without basic swapping
support.  And this will cause build errors when THP_SWAP functions are
defined in swapfile.c and called elsewhere.

The comments are fixed too to reflect the latest progress.

Signed-off-by: "Huang, Ying" <ying.huang@intel.com>
Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
Cc: Andrea Arcangeli <aarcange@redhat.com>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: Shaohua Li <shli@kernel.org>
Cc: Hugh Dickins <hughd@google.com>
Cc: Minchan Kim <minchan@kernel.org>
Cc: Rik van Riel <riel@redhat.com>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
Cc: Zi Yan <zi.yan@cs.rutgers.edu>
Cc: Daniel Jordan <daniel.m.jordan@oracle.com>
---
 mm/Kconfig | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Comments

Dan Williams July 7, 2018, 9:12 p.m. UTC | #1 
On Thu, Jun 21, 2018 at 8:55 PM Huang, Ying <ying.huang@intel.com> wrote:
>
> From: Huang Ying <ying.huang@intel.com>
>
> It's unreasonable to optimize swapping for THP without basic swapping
> support.  And this will cause build errors when THP_SWAP functions are
> defined in swapfile.c and called elsewhere.
>
> The comments are fixed too to reflect the latest progress.

Looks good to me:

Reviewed-by: Dan Williams <dan.j.williams@intel.com>
Huang, Ying July 9, 2018, 6:34 a.m. UTC | #2 
Dan Williams <dan.j.williams@gmail.com> writes:

> On Thu, Jun 21, 2018 at 8:55 PM Huang, Ying <ying.huang@intel.com> wrote:
>>
>> From: Huang Ying <ying.huang@intel.com>
>>
>> It's unreasonable to optimize swapping for THP without basic swapping
>> support.  And this will cause build errors when THP_SWAP functions are
>> defined in swapfile.c and called elsewhere.
>>
>> The comments are fixed too to reflect the latest progress.
>
> Looks good to me:
>
> Reviewed-by: Dan Williams <dan.j.williams@intel.com>

Thanks!

Best Regards,
Huang, Ying
Dave Hansen July 9, 2018, 4 p.m. UTC | #3 
>  config THP_SWAP
>  	def_bool y
> -	depends on TRANSPARENT_HUGEPAGE && ARCH_WANTS_THP_SWAP
> +	depends on TRANSPARENT_HUGEPAGE && ARCH_WANTS_THP_SWAP && SWAP
>  	help


This seems like a bug-fix.  Is there a reason this didn't cause problems
up to now?
Huang, Ying July 10, 2018, 1:19 a.m. UTC | #4 
Dave Hansen <dave.hansen@linux.intel.com> writes:

>>  config THP_SWAP
>>  	def_bool y
>> -	depends on TRANSPARENT_HUGEPAGE && ARCH_WANTS_THP_SWAP
>> +	depends on TRANSPARENT_HUGEPAGE && ARCH_WANTS_THP_SWAP && SWAP
>>  	help
>
>
> This seems like a bug-fix.  Is there a reason this didn't cause problems
> up to now?

Yes.  The original code has some problem in theory, but not in practice
because all code enclosed by

#ifdef CONFIG_THP_SWAP
#endif

are in swapfile.c.  But that will be not true in this patchset.

Best Regards,
Huang, Ying
Dave Hansen July 10, 2018, 1:59 a.m. UTC | #5 
On 07/09/2018 06:19 PM, Huang, Ying wrote:
> Dave Hansen <dave.hansen@linux.intel.com> writes:
> 
>>>  config THP_SWAP
>>>  	def_bool y
>>> -	depends on TRANSPARENT_HUGEPAGE && ARCH_WANTS_THP_SWAP
>>> +	depends on TRANSPARENT_HUGEPAGE && ARCH_WANTS_THP_SWAP && SWAP
>>>  	help
>>
>> This seems like a bug-fix.  Is there a reason this didn't cause problems
>> up to now?
> Yes.  The original code has some problem in theory, but not in practice
> because all code enclosed by
> 
> #ifdef CONFIG_THP_SWAP
> #endif
> 
> are in swapfile.c.  But that will be not true in this patchset.

That's great info for the changelog.
Huang, Ying July 10, 2018, 5:26 a.m. UTC | #6 
Dave Hansen <dave.hansen@linux.intel.com> writes:

> On 07/09/2018 06:19 PM, Huang, Ying wrote:
>> Dave Hansen <dave.hansen@linux.intel.com> writes:
>> 
>>>>  config THP_SWAP
>>>>  	def_bool y
>>>> -	depends on TRANSPARENT_HUGEPAGE && ARCH_WANTS_THP_SWAP
>>>> +	depends on TRANSPARENT_HUGEPAGE && ARCH_WANTS_THP_SWAP && SWAP
>>>>  	help
>>>
>>> This seems like a bug-fix.  Is there a reason this didn't cause problems
>>> up to now?
>> Yes.  The original code has some problem in theory, but not in practice
>> because all code enclosed by
>> 
>> #ifdef CONFIG_THP_SWAP
>> #endif
>> 
>> are in swapfile.c.  But that will be not true in this patchset.
>
> That's great info for the changelog.

Sure.  Will add it.

Best Regards,
Huang, Ying
diff mbox

Patch

diff --git a/mm/Kconfig b/mm/Kconfig
index ce95491abd6a..cee958bb6002 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -420,10 +420,9 @@  config ARCH_WANTS_THP_SWAP
 
 config THP_SWAP
 	def_bool y
-	depends on TRANSPARENT_HUGEPAGE && ARCH_WANTS_THP_SWAP
+	depends on TRANSPARENT_HUGEPAGE && ARCH_WANTS_THP_SWAP && SWAP
 	help
 	  Swap transparent huge pages in one piece, without splitting.
-	  XXX: For now this only does clustered swap space allocation.
 
 	  For selection by architectures with reasonable THP sizes.