[PATCH] mm/memcg: remove definition of MEM_CGROUP_ID_MAX when !CONFIG_MEMCG

[PATCH] mm/memcg: remove definition of MEM_CGROUP_ID_MAX when !CONFIG_MEMCG

[Miaohe Lin]

MEM_CGROUP_ID_MAX is only used when CONFIG_MEMCG is configured. Remove
unneeded !CONFIG_MEMCG variant.

Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 include/linux/memcontrol.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index 5818af8eca5a..634a282099bf 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -1158,7 +1158,6 @@ unsigned long mem_cgroup_soft_limit_reclaim(pg_data_t *pgdat, int order,
 #else /* CONFIG_MEMCG */
 
 #define MEM_CGROUP_ID_SHIFT	0
-#define MEM_CGROUP_ID_MAX	0
 
 static inline struct mem_cgroup *folio_memcg(struct folio *folio)
 {
-- 
2.33.0

Re: [PATCH] mm/memcg: remove definition of MEM_CGROUP_ID_MAX when !CONFIG_MEMCG

[Muchun Song]

> On Jul 6, 2023, at 19:28, Miaohe Lin <linmiaohe@huawei.com> wrote:
> 
> MEM_CGROUP_ID_MAX is only used when CONFIG_MEMCG is configured. Remove
> unneeded !CONFIG_MEMCG variant.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>

MEM_CGROUP_ID_MAX is also only used in mem_cgroup_alloc(), maybe you also
could move it from memcontrol.h to memcontrol.c. And define it as:

#define MEM_CGROUP_ID_MAX ((1U << MEM_CGROUP_ID_SHIFT) - 1)

I am not suggesting defining it as USHRT_MAX, because if someone changes
MEM_CGROUP_ID_SHIFT in the future, then MEM_CGROUP_ID_MAX will not updated
accordingly.

For this patch, LGTM.

Reviewed-by: Muchun Song <songmuchun@bytedance.com>

Thanks.

Re: [PATCH] mm/memcg: remove definition of MEM_CGROUP_ID_MAX when !CONFIG_MEMCG

[Miaohe Lin]

On 2023/7/7 9:47, Muchun Song wrote:
> 
> 
>> On Jul 6, 2023, at 19:28, Miaohe Lin <linmiaohe@huawei.com> wrote:
>>
>> MEM_CGROUP_ID_MAX is only used when CONFIG_MEMCG is configured. Remove
>> unneeded !CONFIG_MEMCG variant.
>>
>> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> 
> MEM_CGROUP_ID_MAX is also only used in mem_cgroup_alloc(), maybe you also
> could move it from memcontrol.h to memcontrol.c. And define it as:
> 
> #define MEM_CGROUP_ID_MAX ((1U << MEM_CGROUP_ID_SHIFT) - 1)
> 
> I am not suggesting defining it as USHRT_MAX, because if someone changes
> MEM_CGROUP_ID_SHIFT in the future, then MEM_CGROUP_ID_MAX will not updated
> accordingly.

Looks sensible to me. Do you suggest squashing above changes into the current patch
or a separate patch is preferred?

> 
> For this patch, LGTM.
> 
> Reviewed-by: Muchun Song <songmuchun@bytedance.com>

Thanks for review and suggestion.

Re: [PATCH] mm/memcg: remove definition of MEM_CGROUP_ID_MAX when !CONFIG_MEMCG

[Muchun Song]

> On Jul 7, 2023, at 10:06, Miaohe Lin <linmiaohe@huawei.com> wrote:
> 
> On 2023/7/7 9:47, Muchun Song wrote:
>> 
>> 
>>> On Jul 6, 2023, at 19:28, Miaohe Lin <linmiaohe@huawei.com> wrote:
>>> 
>>> MEM_CGROUP_ID_MAX is only used when CONFIG_MEMCG is configured. Remove
>>> unneeded !CONFIG_MEMCG variant.
>>> 
>>> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
>> 
>> MEM_CGROUP_ID_MAX is also only used in mem_cgroup_alloc(), maybe you also
>> could move it from memcontrol.h to memcontrol.c. And define it as:
>> 
>> #define MEM_CGROUP_ID_MAX ((1U << MEM_CGROUP_ID_SHIFT) - 1)
>> 
>> I am not suggesting defining it as USHRT_MAX, because if someone changes
>> MEM_CGROUP_ID_SHIFT in the future, then MEM_CGROUP_ID_MAX will not updated
>> accordingly.
> 
> Looks sensible to me. Do you suggest squashing above changes into the current patch
> or a separate patch is preferred?

I think it's better to squash.

> 
>> 
>> For this patch, LGTM.
>> 
>> Reviewed-by: Muchun Song <songmuchun@bytedance.com>
> 
> Thanks for review and suggestion.
> 
> 
> 

Re: [PATCH] mm/memcg: remove definition of MEM_CGROUP_ID_MAX when !CONFIG_MEMCG

[Miaohe Lin]

On 2023/7/7 10:25, Muchun Song wrote:
> 
> 
>> On Jul 7, 2023, at 10:06, Miaohe Lin <linmiaohe@huawei.com> wrote:
>>
>> On 2023/7/7 9:47, Muchun Song wrote:
>>>
>>>
>>>> On Jul 6, 2023, at 19:28, Miaohe Lin <linmiaohe@huawei.com> wrote:
>>>>
>>>> MEM_CGROUP_ID_MAX is only used when CONFIG_MEMCG is configured. Remove
>>>> unneeded !CONFIG_MEMCG variant.
>>>>
>>>> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
>>>
>>> MEM_CGROUP_ID_MAX is also only used in mem_cgroup_alloc(), maybe you also
>>> could move it from memcontrol.h to memcontrol.c. And define it as:
>>>
>>> #define MEM_CGROUP_ID_MAX ((1U << MEM_CGROUP_ID_SHIFT) - 1)
>>>
>>> I am not suggesting defining it as USHRT_MAX, because if someone changes
>>> MEM_CGROUP_ID_SHIFT in the future, then MEM_CGROUP_ID_MAX will not updated
>>> accordingly.
>>
>> Looks sensible to me. Do you suggest squashing above changes into the current patch
>> or a separate patch is preferred?
> 
> I think it's better to squash.

Will do if no objection. Thanks.