Messages in this thread | | | Subject | Re: [PATCH v2 2/7] mm/swapfile.c: Replace some #ifdef with IS_ENABLED() | From | Dave Hansen <> | Date | Wed, 18 Jul 2018 08:15:58 -0700 |
| |
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.
| |