| Message ID | 20230706112820.2393447-1-linmiaohe@huawei.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm/memcg: remove definition of MEM_CGROUP_ID_MAX when !CONFIG_MEMCG | expand |
> 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.
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.
> 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. > > >
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.
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) {
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(-)