All of lore.kernel.org
 help / color / mirror / Atom feed
From: Matthias Kaehlcke <mka@chromium.org>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Johannes Weiner <hannes@cmpxchg.org>,
	Michal Hocko <mhocko@kernel.org>,
	Vladimir Davydov <vdavydov.dev@gmail.com>,
	linux-kernel@vger.kernel.org, cgroups@vger.kernel.org,
	linux-mm@kvack.org, Doug Anderson <dianders@chromium.org>
Subject: Re: [PATCH] mm: memcontrol: Cast mismatched enum types passed to memcg state and event functions
Date: Wed, 26 Jul 2017 14:49:14 -0700	[thread overview]
Message-ID: <20170726214914.GA84665@google.com> (raw)
In-Reply-To: <20170726142309.ac40faf5eb99568e6edb064c@linux-foundation.org>

El Wed, Jul 26, 2017 at 02:23:09PM -0700 Andrew Morton ha dit:

> On Wed, 26 Jul 2017 12:23:56 -0700 Matthias Kaehlcke <mka@chromium.org> wrote:
> 
> > In multiple instances enum values of an incorrect type are passed to
> > mod_memcg_state() and other memcg functions. Apparently this is
> > intentional, however clang rightfully generates tons of warnings about
> > the mismatched types. Cast the offending values to the type expected
> > by the called function. The casts add noise, but this seems preferable
> > over losing the typesafe interface or/and disabling the warning.
> > 
> > ...
> >
> > --- a/include/linux/memcontrol.h
> > +++ b/include/linux/memcontrol.h
> > @@ -576,7 +576,7 @@ static inline void __mod_lruvec_state(struct lruvec *lruvec,
> >  	if (mem_cgroup_disabled())
> >  		return;
> >  	pn = container_of(lruvec, struct mem_cgroup_per_node, lruvec);
> > -	__mod_memcg_state(pn->memcg, idx, val);
> > +	__mod_memcg_state(pn->memcg, (enum memcg_stat_item)idx, val);
> >  	__this_cpu_add(pn->lruvec_stat->count[idx], val);
> >  }
> 
> __mod_memcg_state()'s `idx' arg can be either enum memcg_stat_item or
> enum memcg_stat_item.  I think it would be better to just admit to
> ourselves that __mod_memcg_state() is more general than it appears, and
> change it to take `int idx'.  I assume that this implicit cast of an
> enum to an int will not trigger a clang warning?

Sure, no warnings are raised for implicit casts from enum to
int.

__mod_memcg_state() is not the only function though, all these
functions are called with conflicting types:

memcg_page_state()
__mod_memcg_state()
mod_memcg_state()
count_memcg_events()
count_memcg_page_event()
memcg_sum_events()

Should we change all of them to reveice an int instead of an enum?

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

WARNING: multiple messages have this Message-ID (diff)
From: Matthias Kaehlcke <mka@chromium.org>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Johannes Weiner <hannes@cmpxchg.org>,
	Michal Hocko <mhocko@kernel.org>,
	Vladimir Davydov <vdavydov.dev@gmail.com>,
	linux-kernel@vger.kernel.org, cgroups@vger.kernel.org,
	linux-mm@kvack.org, Doug Anderson <dianders@chromium.org>
Subject: Re: [PATCH] mm: memcontrol: Cast mismatched enum types passed to memcg state and event functions
Date: Wed, 26 Jul 2017 14:49:14 -0700	[thread overview]
Message-ID: <20170726214914.GA84665@google.com> (raw)
In-Reply-To: <20170726142309.ac40faf5eb99568e6edb064c@linux-foundation.org>

El Wed, Jul 26, 2017 at 02:23:09PM -0700 Andrew Morton ha dit:

> On Wed, 26 Jul 2017 12:23:56 -0700 Matthias Kaehlcke <mka@chromium.org> wrote:
> 
> > In multiple instances enum values of an incorrect type are passed to
> > mod_memcg_state() and other memcg functions. Apparently this is
> > intentional, however clang rightfully generates tons of warnings about
> > the mismatched types. Cast the offending values to the type expected
> > by the called function. The casts add noise, but this seems preferable
> > over losing the typesafe interface or/and disabling the warning.
> > 
> > ...
> >
> > --- a/include/linux/memcontrol.h
> > +++ b/include/linux/memcontrol.h
> > @@ -576,7 +576,7 @@ static inline void __mod_lruvec_state(struct lruvec *lruvec,
> >  	if (mem_cgroup_disabled())
> >  		return;
> >  	pn = container_of(lruvec, struct mem_cgroup_per_node, lruvec);
> > -	__mod_memcg_state(pn->memcg, idx, val);
> > +	__mod_memcg_state(pn->memcg, (enum memcg_stat_item)idx, val);
> >  	__this_cpu_add(pn->lruvec_stat->count[idx], val);
> >  }
> 
> __mod_memcg_state()'s `idx' arg can be either enum memcg_stat_item or
> enum memcg_stat_item.  I think it would be better to just admit to
> ourselves that __mod_memcg_state() is more general than it appears, and
> change it to take `int idx'.  I assume that this implicit cast of an
> enum to an int will not trigger a clang warning?

Sure, no warnings are raised for implicit casts from enum to
int.

__mod_memcg_state() is not the only function though, all these
functions are called with conflicting types:

memcg_page_state()
__mod_memcg_state()
mod_memcg_state()
count_memcg_events()
count_memcg_page_event()
memcg_sum_events()

Should we change all of them to reveice an int instead of an enum?

  reply	other threads:[~2017-07-26 21:49 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-07-26 19:23 [PATCH] mm: memcontrol: Cast mismatched enum types passed to memcg state and event functions Matthias Kaehlcke
2017-07-26 19:23 ` Matthias Kaehlcke
2017-07-26 21:23 ` Andrew Morton
2017-07-26 21:23   ` Andrew Morton
2017-07-26 21:49   ` Matthias Kaehlcke [this message]
2017-07-26 21:49     ` Matthias Kaehlcke
     [not found]     ` <20170726214914.GA84665-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2017-07-26 22:03       ` Andrew Morton
2017-07-26 22:03         ` Andrew Morton
2017-07-26 22:03         ` Andrew Morton
     [not found]         ` <20170726150332.313e48837d97046924ddaa16-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
2017-07-27  7:24           ` Michal Hocko
2017-07-27  7:24             ` Michal Hocko
2017-07-27  7:24             ` Michal Hocko
     [not found]             ` <20170727072451.GH20970-2MMpYkNvuYDjFM9bn6wA6Q@public.gmane.org>
2017-07-27 14:17               ` Johannes Weiner
2017-07-27 14:17                 ` Johannes Weiner
2017-07-27 14:17                 ` Johannes Weiner
     [not found]                 ` <20170727141742.GA19738-druUgvl0LCNAfugRpC6u6w@public.gmane.org>
2017-07-27 19:46                   ` Matthias Kaehlcke
2017-07-27 19:46                     ` Matthias Kaehlcke
2017-07-27 19:46                     ` Matthias Kaehlcke

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170726214914.GA84665@google.com \
    --to=mka@chromium.org \
    --cc=akpm@linux-foundation.org \
    --cc=cgroups@vger.kernel.org \
    --cc=dianders@chromium.org \
    --cc=hannes@cmpxchg.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@kernel.org \
    --cc=vdavydov.dev@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.