Commit e1aab161e013 ("socket: initial cgroup code.") causes a build error
when CONFIG_INET is disabled in Linus' tree:
net/built-in.o: In function `sk_update_clone':
net/core/sock.c:1336: undefined reference to `sock_update_memcg'
sock_update_memcg() is only defined when CONFIG_INET is enabled, so fix it
by defining the dummy function without this option.
Reported-by: Randy Dunlap <[email protected]>
Signed-off-by: David Rientjes <[email protected]>
---
Checking the logs, Randy reported this in an email to LKML on
September 24 and didn't get a response...
include/linux/memcontrol.h | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -396,7 +396,7 @@ enum {
};
struct sock;
-#ifdef CONFIG_MEMCG_KMEM
+#if defined(CONFIG_INET) && defined(CONFIG_MEMCG_KMEM)
void sock_update_memcg(struct sock *sk);
void sock_release_memcg(struct sock *sk);
#else
@@ -406,6 +406,6 @@ static inline void sock_update_memcg(struct sock *sk)
static inline void sock_release_memcg(struct sock *sk)
{
}
-#endif /* CONFIG_MEMCG_KMEM */
+#endif /* CONFIG_INET && CONFIG_MEMCG_KMEM */
#endif /* _LINUX_MEMCONTROL_H */
On 10/10/2012 10:32 AM, David Rientjes wrote:
> Commit e1aab161e013 ("socket: initial cgroup code.") causes a build error
> when CONFIG_INET is disabled in Linus' tree:
>
unlikely that something that old would cause a build bug now, specially
that commit, that actually wraps things inside CONFIG_INET.
More likely caused by the recently merged
"memcg-cleanup-kmem-tcp-ifdefs.patch" in -mm by mhocko (CC'd)
As a matter of fact, I just tested, and it indeed start failing after
that patch.
Michal, since it is just a cleanup patch, I'd prefer just reverting if
you are okay with it.
On 10/10/2012 01:27 PM, Michal Hocko wrote:
> On Wed 10-10-12 12:56:26, Glauber Costa wrote:
>> On 10/10/2012 10:32 AM, David Rientjes wrote:
>>> Commit e1aab161e013 ("socket: initial cgroup code.") causes a build error
>>> when CONFIG_INET is disabled in Linus' tree:
>>>
>> unlikely that something that old would cause a build bug now, specially
>> that commit, that actually wraps things inside CONFIG_INET.
>>
>> More likely caused by the recently merged
>> "memcg-cleanup-kmem-tcp-ifdefs.patch" in -mm by mhocko (CC'd)
>
> Strange it didn't trigger during my (and Fenguang) build testing.
Fengguang mentioned to me while testing my kmemcg tree that were a build
error occurring in the base tree, IOW, yours.
Fengguang, was that this error? Why hasn't it showed up before in the
test system?
On Wed 10-10-12 12:56:26, Glauber Costa wrote:
> On 10/10/2012 10:32 AM, David Rientjes wrote:
> > Commit e1aab161e013 ("socket: initial cgroup code.") causes a build error
> > when CONFIG_INET is disabled in Linus' tree:
> >
> unlikely that something that old would cause a build bug now, specially
> that commit, that actually wraps things inside CONFIG_INET.
>
> More likely caused by the recently merged
> "memcg-cleanup-kmem-tcp-ifdefs.patch" in -mm by mhocko (CC'd)
Strange it didn't trigger during my (and Fenguang) build testing.
> As a matter of fact, I just tested, and it indeed start failing after
> that patch.
>
> Michal, since it is just a cleanup patch, I'd prefer just reverting if
> you are okay with it.
I think that taking David's patch makes more sense.
--
Michal Hocko
SUSE Labs
On Tue 09-10-12 23:32:35, David Rientjes wrote:
> Commit e1aab161e013 ("socket: initial cgroup code.") causes a build error
> when CONFIG_INET is disabled in Linus' tree:
>
> net/built-in.o: In function `sk_update_clone':
> net/core/sock.c:1336: undefined reference to `sock_update_memcg'
>
> sock_update_memcg() is only defined when CONFIG_INET is enabled, so fix it
> by defining the dummy function without this option.
>
> Reported-by: Randy Dunlap <[email protected]>
> Signed-off-by: David Rientjes <[email protected]>
Acked-by: Michal Hocko <[email protected]>
Thanks!
> ---
> Checking the logs, Randy reported this in an email to LKML on
> September 24 and didn't get a response...
>
> include/linux/memcontrol.h | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
> --- a/include/linux/memcontrol.h
> +++ b/include/linux/memcontrol.h
> @@ -396,7 +396,7 @@ enum {
> };
>
> struct sock;
> -#ifdef CONFIG_MEMCG_KMEM
> +#if defined(CONFIG_INET) && defined(CONFIG_MEMCG_KMEM)
> void sock_update_memcg(struct sock *sk);
> void sock_release_memcg(struct sock *sk);
> #else
> @@ -406,6 +406,6 @@ static inline void sock_update_memcg(struct sock *sk)
> static inline void sock_release_memcg(struct sock *sk)
> {
> }
> -#endif /* CONFIG_MEMCG_KMEM */
> +#endif /* CONFIG_INET && CONFIG_MEMCG_KMEM */
> #endif /* _LINUX_MEMCONTROL_H */
>
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to [email protected]. For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
--
Michal Hocko
SUSE Labs
On Wed, Oct 10, 2012 at 01:29:35PM +0400, Glauber Costa wrote:
> On 10/10/2012 01:27 PM, Michal Hocko wrote:
> > On Wed 10-10-12 12:56:26, Glauber Costa wrote:
> >> On 10/10/2012 10:32 AM, David Rientjes wrote:
> >>> Commit e1aab161e013 ("socket: initial cgroup code.") causes a build error
> >>> when CONFIG_INET is disabled in Linus' tree:
> >>>
> >> unlikely that something that old would cause a build bug now, specially
> >> that commit, that actually wraps things inside CONFIG_INET.
> >>
> >> More likely caused by the recently merged
> >> "memcg-cleanup-kmem-tcp-ifdefs.patch" in -mm by mhocko (CC'd)
> >
> > Strange it didn't trigger during my (and Fenguang) build testing.
>
> Fengguang mentioned to me while testing my kmemcg tree that were a build
> error occurring in the base tree, IOW, yours.
Yes, and the errors were sent to/cc Michal and [email protected]
> Fengguang, was that this error? Why hasn't it showed up before in the
> test system?
I do find this error in the build error log:
(.text+0x867f): undefined reference to `sock_update_memcg'
2012-09-24 04:54:53 snb next:akpm:69921c3 x86_64-randconfig-s005 0a7f618
Unfortunately it was not reported because the build system could
miss/ignore build bugs due to various reasons/imperfections. It has
since then undergo lots of enhancements and as a result, the daily
reported errors have more than doubled. :-)
Thanks,
Fengguang
On Wed, 10 Oct 2012, Fengguang Wu wrote:
> > Fengguang, was that this error? Why hasn't it showed up before in the
> > test system?
>
> I do find this error in the build error log:
>
> (.text+0x867f): undefined reference to `sock_update_memcg'
> 2012-09-24 04:54:53 snb next:akpm:69921c3 x86_64-randconfig-s005 0a7f618
>
> Unfortunately it was not reported because the build system could
> miss/ignore build bugs due to various reasons/imperfections. It has
> since then undergo lots of enhancements and as a result, the daily
> reported errors have more than doubled. :-)
>
Not sure where this discussion is going. Do people who can't build their
kernel and have a fix for it need to verify that your build system shows
the same thing first? This isn't a false positive.
As I said in the first message, Randy reported this on September 24 (the
same date you're reporting above) and received no response when he
reported it to LKML here:
http://marc.info/?l=linux-kernel&m=134852557320089
Regardless, Linus' tree is messed up and I don't think we need to go back
reverting patches out of his tree when it's trivial to fix with my patch,
which Michal acked. Sheesh.
On 10/11/2012 12:17 AM, David Rientjes wrote:
> On Wed, 10 Oct 2012, Fengguang Wu wrote:
>
>>> Fengguang, was that this error? Why hasn't it showed up before in the
>>> test system?
>>
>> I do find this error in the build error log:
>>
>> (.text+0x867f): undefined reference to `sock_update_memcg'
>> 2012-09-24 04:54:53 snb next:akpm:69921c3 x86_64-randconfig-s005 0a7f618
>>
>> Unfortunately it was not reported because the build system could
>> miss/ignore build bugs due to various reasons/imperfections. It has
>> since then undergo lots of enhancements and as a result, the daily
>> reported errors have more than doubled. :-)
>>
>
> Not sure where this discussion is going. Do people who can't build their
> kernel and have a fix for it need to verify that your build system shows
> the same thing first? This isn't a false positive.
>
> As I said in the first message, Randy reported this on September 24 (the
> same date you're reporting above) and received no response when he
> reported it to LKML here:
> http://marc.info/?l=linux-kernel&m=134852557320089
>
> Regardless, Linus' tree is messed up and I don't think we need to go back
> reverting patches out of his tree when it's trivial to fix with my patch,
> which Michal acked. Sheesh.
>
I am perfectly fine with your patch.
As I said when he first posted it: I see no reason to oppose cleanup
patches, as long as they don't break anything. Unfortunately it did, but
that is water under the bridge.
Again, since the goal of Michal patches was just to move things around,
I don't really care if you patch is applied or if Michal's is reverted.