lkml.org 
[lkml]   [2011]   [May]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [patch] net/core/filter.c: Fix build error

* David Miller <davem@davemloft.net> wrote:

> From: Joe Perches <joe@perches.com>
> Date: Thu, 26 May 2011 08:31:06 -0700
>
> > My suggestion would be to see about again adding
> > #include <linux/ratelimit.h> somehow
> > back to kernel.h which commit 3fff4c42bd0a removed
> > in 2009 because of the spinlock issues.
> >
> > Any suggestion on how best to fix it generically?
>
> I don't think we want spinlock_t's definition being sucked
> into kernel.h's dependency food chain.

Agreed.

Also, i don't think it's unreasonable to require code that uses
DEFINE_RATELIMIT_STATE() to #include ratelimit.h. That's what we
require from DEFINE_MUTEX() and DEFINE_SPINLOCK() users and many
other users as well.

The only exception is printk_ratelimit() itself - that should not -
and currently does not - require the inclusion of ratelimit.h.

The *real* solution here would be to remove the spurious and .config
dependent inclusion of ratelimit.h in include/linux/net.h. It's fine
to provide the net_ratelimit() interface (it'sanalogous to
printk_ratelimit()), but it's not fine to declare net_ratelimit_state
in that header and include that header in half of all networking
code, bringing in ratelimit.h implicitly.

So no, i don't think your patch is the real solution. The problem was
that the .config's you tested had CONFIG_SYSCTL=y, which brought in
ratelimit.h into half of the networking code. That's unnecessary and
problematic - the interface between net/core/sysctl_net_core.c and
net/core/utils.c should be done via a dedicated header (not included
by other code), or via explicit extern declared in the .c file (more
dangerous, but used by pretty much all sysctl code in the kernel).

Thanks,

Ingo


\
 
 \ /
  Last update: 2011-05-26 21:13    [W:0.060 / U:7.204 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site