Re: [PATCH v2 2/7] mm/swapfile.c: Replace some #ifdef with IS_ENABLED()

From: Dave Hansen
Date: Wed Jul 18 2018 - 11:16:18 EST


On 07/17/2018 08:25 PM, Huang, Ying wrote:
>> Seriously, though, does it hurt us to add a comment or two to say
>> something like:
>>
>> /*
>> * Should not even be attempting cluster allocations when
>> * huge page swap is disabled. Warn and fail the allocation.
>> */
>> if (!IS_ENABLED(CONFIG_THP_SWAP)) {
>> VM_WARN_ON_ONCE(1);
>> return 0;
>> }
> I totally agree with you that we should add more comments for THP swap
> to improve the code readability. As for this specific case,
> VM_WARN_ON_ONCE() here is just to capture some programming error during
> development. Do we really need comments here?

If it's code in mainline, we need to know what it is doing.

If it's not useful to have in mainline, then let's remove it.