* Re: [bug report] mm: memcg: move charge migration code to memcontrol-v1.c [not found] <cf6a89aa-449b-4dad-a1a4-40c56a40d258@stanley.mountain> @ 2024-07-12 18:56 ` Roman Gushchin 2024-07-15 8:19 ` Michal Hocko 0 siblings, 1 reply; 2+ messages in thread From: Roman Gushchin @ 2024-07-12 18:56 UTC (permalink / raw) To: Dan Carpenter, Andrew Morton Cc: cgroups, Johannes Weiner, Michal Hocko, Shakeel Butt, linux-mm, linux-kernel On Fri, Jul 12, 2024 at 09:07:45AM -0500, Dan Carpenter wrote: > Hello Roman Gushchin, > > Commit e548ad4a7cbf ("mm: memcg: move charge migration code to > memcontrol-v1.c") from Jun 24, 2024 (linux-next), leads to the > following Smatch static checker warning: > > mm/memcontrol-v1.c:609 mem_cgroup_move_charge_write() > warn: was expecting a 64 bit value instead of '~(1 | 2)' > > mm/memcontrol-v1.c > 599 #ifdef CONFIG_MMU > 600 static int mem_cgroup_move_charge_write(struct cgroup_subsys_state *css, > 601 struct cftype *cft, u64 val) > 602 { > 603 struct mem_cgroup *memcg = mem_cgroup_from_css(css); > 604 > 605 pr_warn_once("Cgroup memory moving (move_charge_at_immigrate) is deprecated. " > 606 "Please report your usecase to linux-mm@kvack.org if you " > 607 "depend on this functionality.\n"); > 608 > --> 609 if (val & ~MOVE_MASK) > > val is a u64 and MOVE_MASK is a u32. This only checks if something in the low > 32 bits is set and it ignores the high 32 bits. Hi Dan, thank you for the report! The mentioned commit just moved to code from memcontrol.c to memcontrol-v1.c, so the bug is actually much much older. Anyway, the fix is attached below. Andrew, can you please pick it up? I don't think the problem and the fix deserve a stable backport. Thank you! -- From 8b3d1ebb8d99cd9d3ab331f69ba9170f09a5c9b6 Mon Sep 17 00:00:00 2001 From: Roman Gushchin <roman.gushchin@linux.dev> Date: Fri, 12 Jul 2024 18:35:14 +0000 Subject: [PATCH] mm: memcg1: convert charge move flags to unsigned long long Currently MOVE_ANON and MOVE_FILE flags are defined as integers and it leads to the following Smatch static checker warning: mm/memcontrol-v1.c:609 mem_cgroup_move_charge_write() warn: was expecting a 64 bit value instead of '~(1 | 2)' Fix this be redefining them as unsigned long long. Even though the issue allows to set high 32 bits of mc.flags to an arbitrary number, these bits are never used, so it doesn't have any significant consequences. Reported-by: Dan Carpenter <dan.carpenter@linaro.org> Signed-off-by: Roman Gushchin <roman.gushchin@linux.dev> --- mm/memcontrol-v1.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mm/memcontrol-v1.c b/mm/memcontrol-v1.c index 6b3e56e88a8a..2aeea4d8bf8e 100644 --- a/mm/memcontrol-v1.c +++ b/mm/memcontrol-v1.c @@ -44,8 +44,8 @@ static struct mem_cgroup_tree soft_limit_tree __read_mostly; /* * Types of charges to be moved. */ -#define MOVE_ANON 0x1U -#define MOVE_FILE 0x2U +#define MOVE_ANON 0x1ULL +#define MOVE_FILE 0x2ULL #define MOVE_MASK (MOVE_ANON | MOVE_FILE) /* "mc" and its members are protected by cgroup_mutex */ -- 2.45.2.993.g49e7a77208-goog ^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [bug report] mm: memcg: move charge migration code to memcontrol-v1.c 2024-07-12 18:56 ` [bug report] mm: memcg: move charge migration code to memcontrol-v1.c Roman Gushchin @ 2024-07-15 8:19 ` Michal Hocko 0 siblings, 0 replies; 2+ messages in thread From: Michal Hocko @ 2024-07-15 8:19 UTC (permalink / raw) To: Roman Gushchin Cc: Dan Carpenter, Andrew Morton, cgroups, Johannes Weiner, Shakeel Butt, linux-mm, linux-kernel On Fri 12-07-24 18:56:03, Roman Gushchin wrote: > On Fri, Jul 12, 2024 at 09:07:45AM -0500, Dan Carpenter wrote: > > Hello Roman Gushchin, > > > > Commit e548ad4a7cbf ("mm: memcg: move charge migration code to > > memcontrol-v1.c") from Jun 24, 2024 (linux-next), leads to the > > following Smatch static checker warning: > > > > mm/memcontrol-v1.c:609 mem_cgroup_move_charge_write() > > warn: was expecting a 64 bit value instead of '~(1 | 2)' > > > > mm/memcontrol-v1.c > > 599 #ifdef CONFIG_MMU > > 600 static int mem_cgroup_move_charge_write(struct cgroup_subsys_state *css, > > 601 struct cftype *cft, u64 val) > > 602 { > > 603 struct mem_cgroup *memcg = mem_cgroup_from_css(css); > > 604 > > 605 pr_warn_once("Cgroup memory moving (move_charge_at_immigrate) is deprecated. " > > 606 "Please report your usecase to linux-mm@kvack.org if you " > > 607 "depend on this functionality.\n"); > > 608 > > --> 609 if (val & ~MOVE_MASK) > > > > val is a u64 and MOVE_MASK is a u32. This only checks if something in the low > > 32 bits is set and it ignores the high 32 bits. > > Hi Dan, > > thank you for the report! > > The mentioned commit just moved to code from memcontrol.c to memcontrol-v1.c, > so the bug is actually much much older. Anyway, the fix is attached below. > > Andrew, can you please pick it up? > > I don't think the problem and the fix deserve a stable backport. Agreed! > Thank you! > > -- > > >From 8b3d1ebb8d99cd9d3ab331f69ba9170f09a5c9b6 Mon Sep 17 00:00:00 2001 > From: Roman Gushchin <roman.gushchin@linux.dev> > Date: Fri, 12 Jul 2024 18:35:14 +0000 > Subject: [PATCH] mm: memcg1: convert charge move flags to unsigned long long > > Currently MOVE_ANON and MOVE_FILE flags are defined as integers > and it leads to the following Smatch static checker warning: > mm/memcontrol-v1.c:609 mem_cgroup_move_charge_write() > warn: was expecting a 64 bit value instead of '~(1 | 2)' > > Fix this be redefining them as unsigned long long. > > Even though the issue allows to set high 32 bits of mc.flags > to an arbitrary number, these bits are never used, so it doesn't > have any significant consequences. > > Reported-by: Dan Carpenter <dan.carpenter@linaro.org> > Signed-off-by: Roman Gushchin <roman.gushchin@linux.dev> Acked-by: Michal Hocko <mhocko@suse.com> Thanks! > --- > mm/memcontrol-v1.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/mm/memcontrol-v1.c b/mm/memcontrol-v1.c > index 6b3e56e88a8a..2aeea4d8bf8e 100644 > --- a/mm/memcontrol-v1.c > +++ b/mm/memcontrol-v1.c > @@ -44,8 +44,8 @@ static struct mem_cgroup_tree soft_limit_tree __read_mostly; > /* > * Types of charges to be moved. > */ > -#define MOVE_ANON 0x1U > -#define MOVE_FILE 0x2U > +#define MOVE_ANON 0x1ULL > +#define MOVE_FILE 0x2ULL > #define MOVE_MASK (MOVE_ANON | MOVE_FILE) > > /* "mc" and its members are protected by cgroup_mutex */ > -- > 2.45.2.993.g49e7a77208-goog -- Michal Hocko SUSE Labs ^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2024-07-15 8:19 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <cf6a89aa-449b-4dad-a1a4-40c56a40d258@stanley.mountain>
2024-07-12 18:56 ` [bug report] mm: memcg: move charge migration code to memcontrol-v1.c Roman Gushchin
2024-07-15 8:19 ` Michal Hocko
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).