[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Wed, 03 Mar 2021 20:03:33 +0530
From: pintu@...eaurora.org
To: Nitin Gupta <nigupta@...dia.com>
Cc: linux-kernel@...r.kernel.org, akpm@...ux-foundation.org,
linux-mm@...ck.org, linux-fsdevel@...r.kernel.org,
iamjoonsoo.kim@....com, sh_def@....com, mateusznosek0@...il.com,
bhe@...hat.com, vbabka@...e.cz, yzaikin@...gle.com,
keescook@...omium.org, mcgrof@...nel.org,
mgorman@...hsingularity.net, pintu.ping@...il.com
Subject: Re: [PATCH] mm/compaction: remove unused variable
sysctl_compact_memory
On 2021-03-03 01:48, Nitin Gupta wrote:
>> -----Original Message-----
>> From: pintu=codeaurora.org@...codeaurora.org
>> <pintu=codeaurora.org@...codeaurora.org> On Behalf Of Pintu Kumar
>> Sent: Tuesday, March 2, 2021 9:56 AM
>> To: linux-kernel@...r.kernel.org; akpm@...ux-foundation.org; linux-
>> mm@...ck.org; linux-fsdevel@...r.kernel.org; pintu@...eaurora.org;
>> iamjoonsoo.kim@....com; sh_def@....com; mateusznosek0@...il.com;
>> bhe@...hat.com; Nitin Gupta <nigupta@...dia.com>; vbabka@...e.cz;
>> yzaikin@...gle.com; keescook@...omium.org; mcgrof@...nel.org;
>> mgorman@...hsingularity.net
>> Cc: pintu.ping@...il.com
>> Subject: [PATCH] mm/compaction: remove unused variable
>> sysctl_compact_memory
>>
>> External email: Use caution opening links or attachments
>>
>>
>> The sysctl_compact_memory is mostly unsed in mm/compaction.c It just
>> acts
>> as a place holder for sysctl.
>>
>> Thus we can remove it from here and move the declaration directly in
>> kernel/sysctl.c itself.
>> This will also eliminate the extern declaration from header file.
>
>
> I prefer keeping the existing pattern of listing all compaction related
> tunables
> together in compaction.h:
>
> extern int sysctl_compact_memory;
> extern unsigned int sysctl_compaction_proactiveness;
> extern int sysctl_extfrag_threshold;
> extern int sysctl_compact_unevictable_allowed;
>
Thanks Nitin for your review.
You mean, you just wanted to retain this extern declaration ?
Any real benefit of keeping this declaration if not used elsewhere ?
>
>> No functionality is broken or changed this way.
>>
>> Signed-off-by: Pintu Kumar <pintu@...eaurora.org>
>> Signed-off-by: Pintu Agarwal <pintu.ping@...il.com>
>> ---
>> include/linux/compaction.h | 1 -
>> kernel/sysctl.c | 1 +
>> mm/compaction.c | 3 ---
>> 3 files changed, 1 insertion(+), 4 deletions(-)
>>
>> diff --git a/include/linux/compaction.h b/include/linux/compaction.h
>> index
>> ed4070e..4221888 100644
>> --- a/include/linux/compaction.h
>> +++ b/include/linux/compaction.h
>> @@ -81,7 +81,6 @@ static inline unsigned long compact_gap(unsigned int
>> order) }
>>
>> #ifdef CONFIG_COMPACTION
>> -extern int sysctl_compact_memory;
>> extern unsigned int sysctl_compaction_proactiveness; extern int
>> sysctl_compaction_handler(struct ctl_table *table, int write,
>> void *buffer, size_t *length, loff_t *ppos);
>> diff --git
>> a/kernel/sysctl.c b/kernel/sysctl.c index c9fbdd8..66aff21 100644
>> --- a/kernel/sysctl.c
>> +++ b/kernel/sysctl.c
>> @@ -198,6 +198,7 @@ static int max_sched_tunable_scaling =
>> SCHED_TUNABLESCALING_END-1; #ifdef CONFIG_COMPACTION static int
>> min_extfrag_threshold; static int max_extfrag_threshold = 1000;
>> +static int sysctl_compact_memory;
>> #endif
>>
>> #endif /* CONFIG_SYSCTL */
>> diff --git a/mm/compaction.c b/mm/compaction.c index 190ccda..ede2886
>> 100644
>> --- a/mm/compaction.c
>> +++ b/mm/compaction.c
>> @@ -2650,9 +2650,6 @@ static void compact_nodes(void)
>> compact_node(nid);
>> }
>>
>> -/* The written value is actually unused, all memory is compacted */
>> -int
>> sysctl_compact_memory;
>> -
>
>
> Please retain this comment for the tunable.
Sorry, I could not understand.
You mean to say just retain this last comment and only remove the
variable ?
Again any real benefit you see in retaining this even if its not used?
Thanks,
Pintu
Powered by blists - more mailing lists