Messages in this thread | | | From | "Huang\, Ying" <> | Subject | Re: [PATCH v2 2/7] mm/swapfile.c: Replace some #ifdef with IS_ENABLED() | Date | Thu, 19 Jul 2018 12:42:14 +0800 |
| |
Dave Hansen <dave.hansen@linux.intel.com> writes:
> 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.
OK. Will add the comments.
Best Regards, Huang, Ying
| |