* Re: [RFC PATCH 1/2] net: ethtool: Add capability to retrieve plug-in module EEPROM
From: Ben Hutchings @ 2012-04-11 18:16 UTC (permalink / raw)
To: Stuart Hodgson
Cc: netdev, bruce.w.allan, mirq-linux, decot, amit.salecha,
alexander.h.duyck, davem, linux-kernel
In-Reply-To: <4F85B639.5090900@solarflare.com>
On Wed, 2012-04-11 at 17:50 +0100, Stuart Hodgson wrote:
> On 02/04/12 18:52, Ben Hutchings wrote:
[...]
> >> --- a/net/core/ethtool.c
> >> +++ b/net/core/ethtool.c
[...]
> >> + if (eeprom.offset + eeprom.len> modinfo.eeprom_len)
> >> + return -EINVAL;
> >> +
> >> + data = kmalloc(PAGE_SIZE, GFP_USER);
> >> + if (!data)
> >> + return -ENOMEM;
> >
> > What if some device has a larger EEPROM? Surely this length should be
> > eeprom.len.
> >
>
> Do you mean what if the eeprom length in te device is larger than
> PAGE_SIZE?
Yes.
> If so then it should really use modinfo.eeprom_len since
> this the size of the data. eeprom.len could be arbitary.
No, eeprom.len is the size of the data and we've already validated it at
this point.
Ben.
^ permalink raw reply
* Re: [PATCH net-next] rtnetlink & bonding: change args got get_tx_queues
From: Stephen Hemminger @ 2012-04-11 18:21 UTC (permalink / raw)
To: Jay Vosburgh
Cc: Eric Dumazet, Ben Hutchings, Andy Gospodarek, David Miller,
netdev
In-Reply-To: <4576.1334168015@death.nxdomain>
On Wed, 11 Apr 2012 11:13:35 -0700
Jay Vosburgh <fubar@us.ibm.com> wrote:
> Stephen Hemminger <shemminger@vyatta.com> wrote:
>
> >On Wed, 11 Apr 2012 07:55:47 +0200
> >Eric Dumazet <eric.dumazet@gmail.com> wrote:
> >
> >> On Tue, 2012-04-10 at 21:34 -0700, Stephen Hemminger wrote:
> >> > Change get_tx_queues, drop unsused arg/return value real_tx_queues,
> >> > and use return by value (with error) rather than call by reference.
> >> >
> >> > Probably bonding should just change to LLTX and the whole get_tx_queues
> >> > API could disappear!
> >>
> >> Absolutely ;)
> >>
> >>
> >
> >It is more complex than that (actually the bonding driver is a mess).
> >The bonding device is already using Lockless Transmit and transmit queue length
> >of zero (good), but it then does some queue mapping of it's own which
> >is unnecessary.
> >
> >Multiqueue only makes sense if there is a queue, otherwise the skb
> >can transparently pass through the layered device (vlan, bridge, bond)
> >and get queued on the real physical device.
> >
> >Right now, trying to see if there is any impact by just leaving
> >bond device as single queue.
>
> The multiqueue support in bonding is intended to permit
> individual slaves to be assigned a particular queue id, which then
> permits tc filter actions to steer traffic to particular slaves.
>
> The relevant part of Documentation/networking/bonding.txt:
>
> The queue_id for a slave can be set using the command:
>
> # echo "eth1:2" > /sys/class/net/bond0/bonding/queue_id
>
> Any interface that needs a queue_id set should set it with multiple calls
> like the one above until proper priorities are set for all interfaces. On
> distributions that allow configuration via initscripts, multiple 'queue_id'
> arguments can be added to BONDING_OPTS to set all needed slave queues.
>
> These queue id's can be used in conjunction with the tc utility to configure
> a multiqueue qdisc and filters to bias certain traffic to transmit on certain
> slave devices. For instance, say we wanted, in the above configuration to
> force all traffic bound to 192.168.1.100 to use eth1 in the bond as its output
> device. The following commands would accomplish this:
>
> # tc qdisc add dev bond0 handle 1 root multiq
>
> # tc filter add dev bond0 protocol ip parent 1: prio 1 u32 match ip dst \
> 192.168.1.100 action skbedit queue_mapping 2
>
> These commands tell the kernel to attach a multiqueue queue discipline to the
> bond0 interface and filter traffic enqueued to it, such that packets with a dst
> ip of 192.168.1.100 have their output queue mapping value overwritten to 2.
> This value is then passed into the driver, causing the normal output path
> selection policy to be overridden, selecting instead qid 2, which maps to eth1.
>
> Note that qid values begin at 1. Qid 0 is reserved to initiate to the driver
> that normal output policy selection should take place. One benefit to simply
> leaving the qid for a slave to 0 is the multiqueue awareness in the bonding
> driver that is now present. This awareness allows tc filters to be placed on
> slave devices as well as bond devices and the bonding driver will simply act as
> a pass-through for selecting output queues on the slave device rather than
> output port selection.
>
But that choice makes performance worse for the simple case of bonding
2 10G NIC's on a 64 core system. I think you are overloading the concept of
queue id to make a classification value. Wasn't marking intended for that?
^ permalink raw reply
* Re: [RFC PATCH 2/2] net: ethtool: Add capability to retrieve plug-in module EEPROM
From: Ben Hutchings @ 2012-04-11 18:31 UTC (permalink / raw)
To: Stuart Hodgson; +Cc: netdev, davem, linux-kernel
In-Reply-To: <4F85C22C.30707@solarflare.com>
On Wed, 2012-04-11 at 18:41 +0100, Stuart Hodgson wrote:
> On 02/04/12 19:18, Ben Hutchings wrote:
> > On Tue, 2012-03-27 at 18:51 +0100, Stuart Hodgson wrote:
[...]
> >> --- a/drivers/net/ethernet/sfc/ethtool.c
> >> +++ b/drivers/net/ethernet/sfc/ethtool.c
[...]
> >> + payload_len = MCDI_DWORD(outbuf,
> >> + GET_PHY_MEDIA_INFO_OUT_DATALEN);
> >> +
> >> + to_copy = (space_remaining< payload_len) ?
> >> + space_remaining : payload_len;
> >> +
> >> + to_copy -= page_off;
> >
> > page_off is the number of bytes we need to discard from payload_len, but
> > we don't want do discard that from space_remaining. I think the last
> > two statements should be changed to:
> >
> > payload_len -= page_off;
> > to_copy = (space_remaining< payload_len) ?
> > space_remaining : payload_len;
> >
>
> I am pretty sure that these two pieces of code are the same
Suppose we start with ee->offset = 64, ee->len = 32. After the MCDI
request returns and we assign payload_len from that, I believe we will
have page_off = 64, payload_len = 128, space_remaining = 32.
Work through the calculations from there and you'll see the problem.
[...]
> >> + phy_cfg = efx->phy_data;
> >> + modinfo->eeprom_len = mcdi_to_module_eeprom_len(phy_cfg->media);
> >> + modinfo->type = SFF_8079;
> > [...]
> >
> > I don't think this makes sense. If we're fixing the type as SFF_8079
> > then why are we calling a function to get the length?
> >
> > Ben.
> >
>
> What about adding an mcdi_to_module_eeprom_type in the same manner
> as mcdi_to_module_eeprom_len and the other mapping functions?
Could do, but I don't see the point of breaking this out into separate
functions that are only used once. It's actually going to increase code
duplication because you'll have to put the same PHY type checks in both
of them.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [PATCH v17 01/15] Add PR_{GET,SET}_NO_NEW_PRIVS to prevent execve from granting privs
From: Michael Kerrisk (man-pages) @ 2012-04-11 19:31 UTC (permalink / raw)
To: Jonathan Corbet
Cc: Andrew Lutomirski, Andrew Morton, Will Drewry, linux-kernel,
linux-security-module, linux-arch, linux-doc, kernel-hardening,
netdev, x86, arnd, davem, hpa, mingo, oleg, peterz, rdunlap,
mcgrathr, tglx, eparis, serge.hallyn, djm, scarybeasts, indan,
pmoore, eric.dumazet, markus, coreyb, keescook, jmorris,
Andy Lutomirski, linux-man
In-Reply-To: <20120406142824.61d8ca3b@lwn.net>
On Sat, Apr 7, 2012 at 8:28 AM, Jonathan Corbet <corbet@lwn.net> wrote:
> On Fri, 6 Apr 2012 13:01:17 -0700
> Andrew Lutomirski <luto@mit.edu> wrote:
>
>> This has been bugging me for awhile. Is there any interest in moving
>> the manpages into the kernel source tree? Then there could be a
>> general requirement that new APIs get documented when they're written.
>
> Man page (or other documentation) requirements for patch acceptance are a
> regular kernel summit feature. People seem to think it's a good idea, but
> actual enforcement of such requirements always seems to be lacking. Lots
> of people have kind of given up trying. I don't really see that adding
> the man pages to the tree would help, but I could be wrong...
I largely consider this (moving man pages to kernel.org) a technical
solution to what is fundamentally a social problem (developers
reluctant to write documentation), and doubt that the technical
solution would make much difference. I'd love to be proved wrong, but
the experiment would require significant start-up effort. (My
collected thoughts on this can be found here:
http://www.kernel.org/doc/man-pages/todo.html#migrate_to_kernel_source.
Note the alternative idea of patch tags mentioned at the end of that
text.)
Unless, or until there's a paid maintainer, I don't expect things to
get significantly better than what they currently are. The quite
significant improvements in man-pages since 2004, when I became
maintainer were in small part due to the fact that I was for a short
period paid to do the work, but in much larger part due to a huge
private effort over those years which over the last couple of years is
no longer unsustainable for me (man-pages is in competition with
requirements for my attention from family, working life, and
(seriously!) seismic events),
Cheers,
Michael
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Author of "The Linux Programming Interface"; http://man7.org/tlpi/
^ permalink raw reply
* Re: [3.4-rc1][Regression][network wireless] 'task xxxx blocked for more than 120 seconds' after resume
From: Maciej Rutecki @ 2012-04-11 19:37 UTC (permalink / raw)
To: Francois Romieu
Cc: Johannes Berg, Stanislaw Gruszka, Linux Kernel Mailing List,
Rafael J. Wysocki, linux-wireless, netdev
In-Reply-To: <20120405210122.GA12135@electric-eye.fr.zoreil.com>
On czwartek, 5 kwietnia 2012 o 23:01:22 Francois Romieu wrote:
> Johannes Berg <johannes@sipsolutions.net> :
> > On Thu, 2012-04-05 at 22:08 +0200, Maciej Rutecki wrote:
> > > Dmesg when CONFIG_LOCKDEP is set:
> > >
> > > http://mrutecki.pl/download/kernel/3.4-rc1/netdev_resume/dmesg-lockdep.
> > > txt
> >
> > Looks like rtl8169's fault:
> Fix is on the way to mainline, see http://patchwork.ozlabs.org/patch/150541
Seems be resolved in -rc2. Only in dmesg still I have messages "netlink: 140
bytes leftover after parsing attributes.". It seems be another issue.
Thanks
--
Maciej Rutecki
http://www.mrutecki.pl
^ permalink raw reply
* Re: [PATCH 5/9] ipvs: use adaptive pause in master thread
From: Julian Anastasov @ 2012-04-11 20:02 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: Simon Horman, lvs-devel, netdev, netfilter-devel, Wensong Zhang
In-Reply-To: <20120409230803.GB27514@1984>
Hello,
On Tue, 10 Apr 2012, Pablo Neira Ayuso wrote:
> You can still use kthread_should_stop inside a wrapper function
> that calls kthread_stop and up() the semaphore.
>
> sync_stop:
> kthread_stop(k)
> up(s)
>
> kthread_routine:
> while(1) {
> down(s)
> if (kthread_should_stop(k))
> break;
>
> get sync msg
> send sync msg
> }
>
> BTW, each up() does not necessarily mean one wakeup event. up() will
> delivery only one wakeup event for one process that has been already
> awaken.
OK, now I added up(). It will be called when
32 messages are queued after last sent by thread.
> > I'm still thinking if sndbuf value should be exported,
> > currently users have to modify the global default/max value.
>
> I think it's a good idea.
Done, used same value both for rcvbuf and sndbuf.
> > But in below version I'm trying to handle the sndbuf overflow
> > by blocking for write_space event. By this way we should work
> > with any sndbuf configuration.
>
> You seem to be defering the overrun problem by using a longer
> intermediate queue than the socket buffer. Then, that queue can be
> tuned by the user via sysctl. It may happen under heavy stress that
> your intermediate queue gets full again, then you'll have to drop
> packets at some point.
Yes, both values are for same thing, the problem is
that queue size is in messages while socket buffer is in bytes.
And as sndbuf config is optional, I'm not trying to derive
sync_qlen_max from sndbuf. May be we can do it after
socket is created but it will cause problem for systems
that do not configure sync_sock_size, they before now used
unlimited queue and may be default socket size, so using
some small default sndbuf as sync_qlen_max can cause message
drops. They will use reduced limits. So, now we provide
some large sync_qlen_max as default configuration
which probably exceeds the default socket buffer.
Still, I think the down/up idea is not better.
We are adding two new vars: master_stopped and
master_sem.
The problem is that kthread_stop() is a blocking
function. It waits thread to terminate. It can not wakeup
thread blocked in down(), so we add master_stopped flag
that will unblock the down() loop while kthread_stop() will also
unblock thread if waiting for write_space. I.e. up()+kthread_stop()
is racy without additional flag while kthread_stop()+up()
is not possible to work.
I'm appending untested version with up+down but I think
we should use wake_up_process and schedule_timeout instead,
as in previous version.
diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
index 2bdee51..8ed41eb 100644
--- a/include/net/ip_vs.h
+++ b/include/net/ip_vs.h
@@ -870,6 +870,8 @@ struct netns_ipvs {
#endif
int sysctl_snat_reroute;
int sysctl_sync_ver;
+ int sysctl_sync_qlen_max;
+ int sysctl_sync_sock_size;
int sysctl_cache_bypass;
int sysctl_expire_nodest_conn;
int sysctl_expire_quiescent_template;
@@ -890,6 +892,10 @@ struct netns_ipvs {
struct timer_list est_timer; /* Estimation timer */
/* ip_vs_sync */
struct list_head sync_queue;
+ int sync_queue_len;
+ unsigned int sync_queue_delay;
+ int master_stopped;
+ struct semaphore master_sem;
spinlock_t sync_lock;
struct ip_vs_sync_buff *sync_buff;
spinlock_t sync_buff_lock;
@@ -912,6 +918,8 @@ struct netns_ipvs {
#define DEFAULT_SYNC_THRESHOLD 3
#define DEFAULT_SYNC_PERIOD 50
#define DEFAULT_SYNC_VER 1
+#define IPVS_SYNC_WAKEUP_RATE 32
+#define IPVS_SYNC_QLEN_MAX (IPVS_SYNC_WAKEUP_RATE * 4)
#ifdef CONFIG_SYSCTL
@@ -930,6 +938,16 @@ static inline int sysctl_sync_ver(struct netns_ipvs *ipvs)
return ipvs->sysctl_sync_ver;
}
+static inline int sysctl_sync_qlen_max(struct netns_ipvs *ipvs)
+{
+ return ipvs->sysctl_sync_qlen_max;
+}
+
+static inline int sysctl_sync_sock_size(struct netns_ipvs *ipvs)
+{
+ return ipvs->sysctl_sync_sock_size;
+}
+
#else
static inline int sysctl_sync_threshold(struct netns_ipvs *ipvs)
@@ -947,6 +965,16 @@ static inline int sysctl_sync_ver(struct netns_ipvs *ipvs)
return DEFAULT_SYNC_VER;
}
+static inline int sysctl_sync_qlen_max(struct netns_ipvs *ipvs)
+{
+ return IPVS_SYNC_QLEN_MAX;
+}
+
+static inline int sysctl_sync_sock_size(struct netns_ipvs *ipvs)
+{
+ return 0;
+}
+
#endif
/*
diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
index 964d426..2172fcc 100644
--- a/net/netfilter/ipvs/ip_vs_ctl.c
+++ b/net/netfilter/ipvs/ip_vs_ctl.c
@@ -1718,6 +1718,18 @@ static struct ctl_table vs_vars[] = {
.proc_handler = &proc_do_sync_mode,
},
{
+ .procname = "sync_qlen_max",
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ .proc_handler = proc_dointvec,
+ },
+ {
+ .procname = "sync_sock_size",
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ .proc_handler = proc_dointvec,
+ },
+ {
.procname = "cache_bypass",
.maxlen = sizeof(int),
.mode = 0644,
@@ -3662,6 +3674,10 @@ int __net_init ip_vs_control_net_init_sysctl(struct net *net)
tbl[idx++].data = &ipvs->sysctl_snat_reroute;
ipvs->sysctl_sync_ver = 1;
tbl[idx++].data = &ipvs->sysctl_sync_ver;
+ ipvs->sysctl_sync_qlen_max = nr_free_buffer_pages() / 32;
+ tbl[idx++].data = &ipvs->sysctl_sync_qlen_max;
+ ipvs->sysctl_sync_sock_size = 0;
+ tbl[idx++].data = &ipvs->sysctl_sync_sock_size;
tbl[idx++].data = &ipvs->sysctl_cache_bypass;
tbl[idx++].data = &ipvs->sysctl_expire_nodest_conn;
tbl[idx++].data = &ipvs->sysctl_expire_quiescent_template;
diff --git a/net/netfilter/ipvs/ip_vs_sync.c b/net/netfilter/ipvs/ip_vs_sync.c
index 0e36679..0bd473c 100644
--- a/net/netfilter/ipvs/ip_vs_sync.c
+++ b/net/netfilter/ipvs/ip_vs_sync.c
@@ -312,6 +312,9 @@ static inline struct ip_vs_sync_buff *sb_dequeue(struct netns_ipvs *ipvs)
struct ip_vs_sync_buff,
list);
list_del(&sb->list);
+ ipvs->sync_queue_len--;
+ if (!ipvs->sync_queue_len)
+ ipvs->sync_queue_delay = 0;
}
spin_unlock_bh(&ipvs->sync_lock);
@@ -358,9 +361,13 @@ static inline void sb_queue_tail(struct netns_ipvs *ipvs)
struct ip_vs_sync_buff *sb = ipvs->sync_buff;
spin_lock(&ipvs->sync_lock);
- if (ipvs->sync_state & IP_VS_STATE_MASTER)
+ if (ipvs->sync_state & IP_VS_STATE_MASTER &&
+ ipvs->sync_queue_len < sysctl_sync_qlen_max(ipvs)) {
list_add_tail(&sb->list, &ipvs->sync_queue);
- else
+ ipvs->sync_queue_len++;
+ if ((++ipvs->sync_queue_delay) == IPVS_SYNC_WAKEUP_RATE)
+ up(&ipvs->master_sem);
+ } else
ip_vs_sync_buff_release(sb);
spin_unlock(&ipvs->sync_lock);
}
@@ -405,10 +412,11 @@ void ip_vs_sync_switch_mode(struct net *net, int mode)
ipvs->sync_buff = NULL;
} else {
spin_lock_bh(&ipvs->sync_lock);
- if (ipvs->sync_state & IP_VS_STATE_MASTER)
+ if (ipvs->sync_state & IP_VS_STATE_MASTER) {
list_add_tail(&ipvs->sync_buff->list,
&ipvs->sync_queue);
- else
+ ipvs->sync_queue_len++;
+ } else
ip_vs_sync_buff_release(ipvs->sync_buff);
spin_unlock_bh(&ipvs->sync_lock);
}
@@ -1130,6 +1138,28 @@ static void ip_vs_process_message(struct net *net, __u8 *buffer,
/*
+ * Setup sndbuf (mode=1) or rcvbuf (mode=0)
+ */
+static void set_sock_size(struct sock *sk, int mode, int val)
+{
+ /* setsockopt(sock, SOL_SOCKET, SO_SNDBUF, &val, sizeof(val)); */
+ /* setsockopt(sock, SOL_SOCKET, SO_RCVBUF, &val, sizeof(val)); */
+ lock_sock(sk);
+ if (mode) {
+ val = clamp_t(int, val, (SOCK_MIN_SNDBUF + 1) / 2,
+ sysctl_wmem_max);
+ sk->sk_sndbuf = val * 2;
+ sk->sk_userlocks |= SOCK_SNDBUF_LOCK;
+ } else {
+ val = clamp_t(int, val, (SOCK_MIN_RCVBUF + 1) / 2,
+ sysctl_rmem_max);
+ sk->sk_rcvbuf = val * 2;
+ sk->sk_userlocks |= SOCK_RCVBUF_LOCK;
+ }
+ release_sock(sk);
+}
+
+/*
* Setup loopback of outgoing multicasts on a sending socket
*/
static void set_mcast_loop(struct sock *sk, u_char loop)
@@ -1305,6 +1335,9 @@ static struct socket *make_send_sock(struct net *net)
set_mcast_loop(sock->sk, 0);
set_mcast_ttl(sock->sk, 1);
+ result = sysctl_sync_sock_size(ipvs);
+ if (result > 0)
+ set_sock_size(sock->sk, 1, result);
result = bind_mcastif_addr(sock, ipvs->master_mcast_ifn);
if (result < 0) {
@@ -1350,6 +1383,9 @@ static struct socket *make_receive_sock(struct net *net)
sk_change_net(sock->sk, net);
/* it is equivalent to the REUSEADDR option in user-space */
sock->sk->sk_reuse = 1;
+ result = sysctl_sync_sock_size(ipvs);
+ if (result > 0)
+ set_sock_size(sock->sk, 0, result);
result = sock->ops->bind(sock, (struct sockaddr *) &mcast_addr,
sizeof(struct sockaddr));
@@ -1392,18 +1428,22 @@ ip_vs_send_async(struct socket *sock, const char *buffer, const size_t length)
return len;
}
-static void
+static int
ip_vs_send_sync_msg(struct socket *sock, struct ip_vs_sync_mesg *msg)
{
int msize;
+ int ret;
msize = msg->size;
/* Put size in network byte order */
msg->size = htons(msg->size);
- if (ip_vs_send_async(sock, (char *)msg, msize) != msize)
- pr_err("ip_vs_send_async error\n");
+ ret = ip_vs_send_async(sock, (char *)msg, msize);
+ if (ret >= 0 || ret == -EAGAIN)
+ return ret;
+ pr_err("ip_vs_send_async error %d\n", ret);
+ return 0;
}
static int
@@ -1428,33 +1468,57 @@ ip_vs_receive(struct socket *sock, char *buffer, const size_t buflen)
return len;
}
+/* Get next buffer to send */
+static inline struct ip_vs_sync_buff *
+next_sync_buff(struct netns_ipvs *ipvs)
+{
+ struct ip_vs_sync_buff *sb;
+
+ sb = sb_dequeue(ipvs);
+ if (sb)
+ return sb;
+ /* Do not delay entries in buffer for more than 2 seconds */
+ return get_curr_sync_buff(ipvs, 2 * HZ);
+}
static int sync_thread_master(void *data)
{
struct ip_vs_sync_thread_data *tinfo = data;
struct netns_ipvs *ipvs = net_ipvs(tinfo->net);
+ struct sock *sk = tinfo->sock->sk;
struct ip_vs_sync_buff *sb;
pr_info("sync thread started: state = MASTER, mcast_ifn = %s, "
"syncid = %d\n",
ipvs->master_mcast_ifn, ipvs->master_syncid);
- while (!kthread_should_stop()) {
- while ((sb = sb_dequeue(ipvs))) {
- ip_vs_send_sync_msg(tinfo->sock, sb->mesg);
- ip_vs_sync_buff_release(sb);
+ for (;;) {
+ sb = next_sync_buff(ipvs);
+ if (!sb) {
+ down_timeout(&ipvs->master_sem, HZ / 5);
+ if (ipvs->master_stopped)
+ break;
+ continue;
}
- /* check if entries stay in ipvs->sync_buff for 2 seconds */
- sb = get_curr_sync_buff(ipvs, 2 * HZ);
- if (sb) {
- ip_vs_send_sync_msg(tinfo->sock, sb->mesg);
- ip_vs_sync_buff_release(sb);
+retry:
+ if (unlikely(ipvs->master_stopped))
+ break;
+ if (ip_vs_send_sync_msg(tinfo->sock, sb->mesg) < 0) {
+ int ret = 0;
+
+ __wait_event_interruptible(*sk_sleep(sk),
+ sock_writeable(sk) ||
+ kthread_should_stop(),
+ ret);
+ goto retry;
}
-
- schedule_timeout_interruptible(HZ);
+ ip_vs_sync_buff_release(sb);
}
+ if (sb)
+ ip_vs_sync_buff_release(sb);
+
/* clean up the sync_buff queue */
while ((sb = sb_dequeue(ipvs)))
ip_vs_sync_buff_release(sb);
@@ -1538,6 +1602,10 @@ int start_sync_thread(struct net *net, int state, char *mcast_ifn, __u8 syncid)
realtask = &ipvs->master_thread;
name = "ipvs_master:%d";
threadfn = sync_thread_master;
+ ipvs->sync_queue_len = 0;
+ ipvs->sync_queue_delay = 0;
+ ipvs->master_stopped = 0;
+ sema_init(&ipvs->master_sem, 0);
sock = make_send_sock(net);
} else if (state == IP_VS_STATE_BACKUP) {
if (ipvs->backup_thread)
@@ -1623,6 +1691,8 @@ int stop_sync_thread(struct net *net, int state)
spin_lock_bh(&ipvs->sync_lock);
ipvs->sync_state &= ~IP_VS_STATE_MASTER;
spin_unlock_bh(&ipvs->sync_lock);
+ ipvs->master_stopped = 1;
+ up(&ipvs->master_sem);
retc = kthread_stop(ipvs->master_thread);
ipvs->master_thread = NULL;
} else if (state == IP_VS_STATE_BACKUP) {
^ permalink raw reply related
* [PATCH] ath6kl: fix memory leak in ath6kl_fwlog_block_read()
From: Jesper Juhl @ 2012-04-11 20:41 UTC (permalink / raw)
To: linux-wireless
Cc: netdev, linux-kernel, Robert Love, John W. Linville, Kalle Valo
If, in
drivers/net/wireless/ath/ath6kl/debug.c::ath6kl_fwlog_block_read(),
the call to wait_for_completion_interruptible() returns -ERESTARTSYS
then we'll return without freeing the (as yet unused) memory we
allocated for 'buf' - thus leaking it.
Signed-off-by: Jesper Juhl <jj@chaosbits.net>
---
drivers/net/wireless/ath/ath6kl/debug.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/ath/ath6kl/debug.c b/drivers/net/wireless/ath/ath6kl/debug.c
index d01403a..34eb927 100644
--- a/drivers/net/wireless/ath/ath6kl/debug.c
+++ b/drivers/net/wireless/ath/ath6kl/debug.c
@@ -401,8 +401,10 @@ static ssize_t ath6kl_fwlog_block_read(struct file *file,
ret = wait_for_completion_interruptible(
&ar->debug.fwlog_completion);
- if (ret == -ERESTARTSYS)
+ if (ret == -ERESTARTSYS) {
+ vfree(buf);
return ret;
+ }
spin_lock(&ar->debug.fwlog_queue.lock);
}
--
1.7.10
PS. Please CC me on replies.
--
Jesper Juhl <jj@chaosbits.net> http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.
^ permalink raw reply related
* [PATCH] can: usb: PCAN-USB Pro: fix mem leaks in pcan_usb_pro_init() on error paths
From: Jesper Juhl @ 2012-04-11 21:23 UTC (permalink / raw)
To: linux-can
Cc: Stephane Grosjean, Wolfgang Grandegger, Marc Kleine-Budde, netdev,
linux-kernel
If either call to pcan_usb_pro_send_req() in
drivers/net/can/usb/peak_usb/pcan_usb_pro.c::pcan_usb_pro_init()
fails, we'll leak the memory we allocated to 'usb_if' with kzalloc()
when the 'usb_if' variable goes out of scope without having been
assigned to anything as we 'return err;'.
Fix this by adding appropriate kfree(usb_if) calls to the error paths.
Signed-off-by: Jesper Juhl <jj@chaosbits.net>
---
drivers/net/can/usb/peak_usb/pcan_usb_pro.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_pro.c b/drivers/net/can/usb/peak_usb/pcan_usb_pro.c
index 5234586..629c4ba 100644
--- a/drivers/net/can/usb/peak_usb/pcan_usb_pro.c
+++ b/drivers/net/can/usb/peak_usb/pcan_usb_pro.c
@@ -875,6 +875,7 @@ static int pcan_usb_pro_init(struct peak_usb_device *dev)
PCAN_USBPRO_INFO_FW,
&fi, sizeof(fi));
if (err) {
+ kfree(usb_if);
dev_err(dev->netdev->dev.parent,
"unable to read %s firmware info (err %d)\n",
pcan_usb_pro.name, err);
@@ -885,6 +886,7 @@ static int pcan_usb_pro_init(struct peak_usb_device *dev)
PCAN_USBPRO_INFO_BL,
&bi, sizeof(bi));
if (err) {
+ kfree(usb_if);
dev_err(dev->netdev->dev.parent,
"unable to read %s bootloader info (err %d)\n",
pcan_usb_pro.name, err);
--
1.7.10
(please CC me on replies)
--
Jesper Juhl <jj@chaosbits.net> http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.
^ permalink raw reply related
* Re: [PATCH v2] Lockd: pass network namespace to creation and destruction routines
From: J. Bruce Fields @ 2012-04-11 21:32 UTC (permalink / raw)
To: Stanislav Kinsbursky
Cc: Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA@public.gmane.org,
linux-nfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Pavel Emelianov, neilb-l3A5Bk7waGM@public.gmane.org,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
James Bottomley, davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org,
devel-GEFAQzZX7r8dnm+yROfE0A@public.gmane.org
In-Reply-To: <4F85AD54.6000806-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
On Wed, Apr 11, 2012 at 08:12:04PM +0400, Stanislav Kinsbursky wrote:
> 11.04.2012 20:11, J. Bruce Fields пишет:
> >On Thu, Mar 29, 2012 at 06:54:33PM +0400, Stanislav Kinsbursky wrote:
> >>v2: dereference of most probably already released nlm_host removed in
> >>nlmclnt_done() and reclaimer().
> >
> >Did you want this in Trond's tree or mine?
> >
>
> Your tree is preferred since I'm working with it.
OK, applying.--b.
>
> >--b.
> >
> >>
> >>These routines are called from locks reclaimer() kernel thread. This thread
> >>works in "init_net" network context and currently relays on persence on lockd
> >>thread and it's per-net resources. Thus lockd_up() and lockd_down() can't relay
> >>on current network context. So let's pass corrent one into them.
> >>
> >>Signed-off-by: Stanislav Kinsbursky<skinsbursky-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
> >>
> >>Signed-off-by: Stanislav Kinsbursky<skinsbursky-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
> >>
> >>---
> >> fs/lockd/clntlock.c | 13 ++++++++-----
> >> fs/lockd/svc.c | 7 +++----
> >> fs/nfsd/nfssvc.c | 6 +++---
> >> include/linux/lockd/bind.h | 4 ++--
> >> 4 files changed, 16 insertions(+), 14 deletions(-)
> >>
> >>diff --git a/fs/lockd/clntlock.c b/fs/lockd/clntlock.c
> >>index ba1dc2e..ca0a080 100644
> >>--- a/fs/lockd/clntlock.c
> >>+++ b/fs/lockd/clntlock.c
> >>@@ -56,7 +56,7 @@ struct nlm_host *nlmclnt_init(const struct nlmclnt_initdata *nlm_init)
> >> u32 nlm_version = (nlm_init->nfs_version == 2) ? 1 : 4;
> >> int status;
> >>
> >>- status = lockd_up();
> >>+ status = lockd_up(nlm_init->net);
> >> if (status< 0)
> >> return ERR_PTR(status);
> >>
> >>@@ -65,7 +65,7 @@ struct nlm_host *nlmclnt_init(const struct nlmclnt_initdata *nlm_init)
> >> nlm_init->hostname, nlm_init->noresvport,
> >> nlm_init->net);
> >> if (host == NULL) {
> >>- lockd_down();
> >>+ lockd_down(nlm_init->net);
> >> return ERR_PTR(-ENOLCK);
> >> }
> >>
> >>@@ -80,8 +80,10 @@ EXPORT_SYMBOL_GPL(nlmclnt_init);
> >> */
> >> void nlmclnt_done(struct nlm_host *host)
> >> {
> >>+ struct net *net = host->net;
> >>+
> >> nlmclnt_release_host(host);
> >>- lockd_down();
> >>+ lockd_down(net);
> >> }
> >> EXPORT_SYMBOL_GPL(nlmclnt_done);
> >>
> >>@@ -220,11 +222,12 @@ reclaimer(void *ptr)
> >> struct nlm_wait *block;
> >> struct file_lock *fl, *next;
> >> u32 nsmstate;
> >>+ struct net *net = host->net;
> >>
> >> allow_signal(SIGKILL);
> >>
> >> down_write(&host->h_rwsem);
> >>- lockd_up(); /* note: this cannot fail as lockd is already running */
> >>+ lockd_up(net); /* note: this cannot fail as lockd is already running */
> >>
> >> dprintk("lockd: reclaiming locks for host %s\n", host->h_name);
> >>
> >>@@ -275,6 +278,6 @@ restart:
> >>
> >> /* Release host handle after use */
> >> nlmclnt_release_host(host);
> >>- lockd_down();
> >>+ lockd_down(net);
> >> return 0;
> >> }
> >>diff --git a/fs/lockd/svc.c b/fs/lockd/svc.c
> >>index b34100e..ce4c80e 100644
> >>--- a/fs/lockd/svc.c
> >>+++ b/fs/lockd/svc.c
> >>@@ -295,11 +295,10 @@ static void lockd_down_net(struct net *net)
> >> /*
> >> * Bring up the lockd process if it's not already up.
> >> */
> >>-int lockd_up(void)
> >>+int lockd_up(struct net *net)
> >> {
> >> struct svc_serv *serv;
> >> int error = 0;
> >>- struct net *net = current->nsproxy->net_ns;
> >>
> >> mutex_lock(&nlmsvc_mutex);
> >> /*
> >>@@ -377,12 +376,12 @@ EXPORT_SYMBOL_GPL(lockd_up);
> >> * Decrement the user count and bring down lockd if we're the last.
> >> */
> >> void
> >>-lockd_down(void)
> >>+lockd_down(struct net *net)
> >> {
> >> mutex_lock(&nlmsvc_mutex);
> >> if (nlmsvc_users) {
> >> if (--nlmsvc_users) {
> >>- lockd_down_net(current->nsproxy->net_ns);
> >>+ lockd_down_net(net);
> >> goto out;
> >> }
> >> } else {
> >>diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
> >>index fce472f..0f3e35b 100644
> >>--- a/fs/nfsd/nfssvc.c
> >>+++ b/fs/nfsd/nfssvc.c
> >>@@ -220,7 +220,7 @@ static int nfsd_startup(unsigned short port, int nrservs)
> >> ret = nfsd_init_socks(port);
> >> if (ret)
> >> goto out_racache;
> >>- ret = lockd_up();
> >>+ ret = lockd_up(&init_net);
> >> if (ret)
> >> goto out_racache;
> >> ret = nfs4_state_start();
> >>@@ -229,7 +229,7 @@ static int nfsd_startup(unsigned short port, int nrservs)
> >> nfsd_up = true;
> >> return 0;
> >> out_lockd:
> >>- lockd_down();
> >>+ lockd_down(&init_net);
> >> out_racache:
> >> nfsd_racache_shutdown();
> >> return ret;
> >>@@ -246,7 +246,7 @@ static void nfsd_shutdown(void)
> >> if (!nfsd_up)
> >> return;
> >> nfs4_state_shutdown();
> >>- lockd_down();
> >>+ lockd_down(&init_net);
> >> nfsd_racache_shutdown();
> >> nfsd_up = false;
> >> }
> >>diff --git a/include/linux/lockd/bind.h b/include/linux/lockd/bind.h
> >>index 11a966e..4d24d64 100644
> >>--- a/include/linux/lockd/bind.h
> >>+++ b/include/linux/lockd/bind.h
> >>@@ -54,7 +54,7 @@ extern void nlmclnt_done(struct nlm_host *host);
> >>
> >> extern int nlmclnt_proc(struct nlm_host *host, int cmd,
> >> struct file_lock *fl);
> >>-extern int lockd_up(void);
> >>-extern void lockd_down(void);
> >>+extern int lockd_up(struct net *net);
> >>+extern void lockd_down(struct net *net);
> >>
> >> #endif /* LINUX_LOCKD_BIND_H */
> >>
>
>
> --
> Best regards,
> Stanislav Kinsbursky
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] net: orphan queued skbs if device tx can stall
From: Michael S. Tsirkin @ 2012-04-11 21:52 UTC (permalink / raw)
To: Eric Dumazet
Cc: netdev, linux-kernel, David S. Miller, Jamal Hadi Salim,
Stephen Hemminger, Jason Wang, Neil Horman, Jiri Pirko,
Jeff Kirsher, Michał Mirosław, Ben Hutchings,
Herbert Xu
In-Reply-To: <1334065929.5300.40.camel@edumazet-glaptop>
On Tue, Apr 10, 2012 at 03:52:09PM +0200, Eric Dumazet wrote:
> On Tue, 2012-04-10 at 15:41 +0300, Michael S. Tsirkin wrote:
>
> > I think it's a bad interface too but it's in a userspace ABI
> > now so I suspect we are stuck with it for now. We can try deprecating
> > but we can't just drop it.
> >
>
> By the way, skb orphaning should already be done in skb_orphan_try(),
> not sure why its done again in tun_net_xmit(). Note we perform orphaning
> right before giving skb to device on premise it'll be sent (and freed)
> in a reasonable amount of time.
>
>
> With following patch, no more qdisc on top of tun device, yet user can
> change the limit (I would be curious to know if anybody changes tun
> txqueuelen and why)
Following would makes the default visible with SIOCGIFTXQLEN
same as it was + avoid branch on data path.
Didn't have time to test yet - it should achieve
same purpose, shouldn't it?
Or do I misunderstand how it's designed to work?
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index bb8c72c..418abd1 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -521,7 +521,6 @@ static void tun_net_init(struct net_device *dev)
/* Zero header length */
dev->type = ARPHRD_NONE;
dev->flags = IFF_POINTOPOINT | IFF_NOARP | IFF_MULTICAST;
- dev->tx_queue_len = TUN_READQ_SIZE; /* We prefer our own queue length */
break;
case TUN_TAP_DEV:
@@ -531,10 +530,9 @@ static void tun_net_init(struct net_device *dev)
dev->priv_flags &= ~IFF_TX_SKB_SHARING;
eth_hw_addr_random(dev);
-
- dev->tx_queue_len = TUN_READQ_SIZE; /* We prefer our own queue length */
break;
}
+ dev->tx_queue_len = 0; /* Disable qdisc queueing */
}
/* Character device part */
@@ -1143,6 +1141,12 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
if (err < 0)
goto err_free_sk;
+ /*
+ * We (ab)use queue length to limit our amount of buffering.
+ * Set our own default queue length.
+ */
+ dev->tx_queue_len = TUN_READQ_SIZE;
+
if (device_create_file(&tun->dev->dev, &dev_attr_tun_flags) ||
device_create_file(&tun->dev->dev, &dev_attr_owner) ||
device_create_file(&tun->dev->dev, &dev_attr_group))
^ permalink raw reply related
* Re: via-rhine: Problem with lost link after a while
From: Francois Romieu @ 2012-04-11 22:21 UTC (permalink / raw)
To: Svenning Sørensen
Cc: Bjarke Istrup Pedersen, linux-kernel, netdev, Andreas Mohr
In-Reply-To: <4F8553BF.6030303@secomea.com>
Svenning Sørensen <sss@secomea.com> :
[...]
> Regarding the "Tx descriptor busy" test: no, I didn't see it
> trigger, I just put it there just in case because I suspected there
> could be a race due to the lock-free tx path.
> But I'm glad if you're confident that it can't happen :)
Almost :o)
Without the patch it may happen in mainline. Once the napi Tx completion
function checks if queueing is stopped before enabling it, it should not
happen though. The patch should fix the race where queuing is not enabled
as well.
That being said, I would welcome a pony^W^W some testing on a multi-core
system with lots of Tx and enough irq from a different (non via-rhine)
source to starve the softirq processing.
--
Ueimor
^ permalink raw reply
* Re: suspicious RCU usage warnings in 3.3.0
From: Paul E. McKenney @ 2012-04-11 23:08 UTC (permalink / raw)
To: Meelis Roos; +Cc: David Miller, linux-kernel, netdev
In-Reply-To: <alpine.SOC.1.00.1204111807420.26884@math.ut.ee>
On Wed, Apr 11, 2012 at 06:08:53PM +0300, Meelis Roos wrote:
> > > Is this the same RCU problem that was fixed after 3.3 (fix a potential
> > > rcu_read_lock() imbalance in rt6_fill_node())? My problem does not seem
> > > to be ipv6-only, most traces are from IPv6 but some for ip.
> >
> > It's hard to say because the ipv6 RCU problem causes the warning to
> > trigger somewhere away from the ipv6 code that had the RCU locking
> > bug.
>
> Tested todays 3.4.0-rc2-00016-ga9e1e53 on the same with flood ping and
> still got RCU warning:
Hmmm... What CPU family is this running on? From the look of the
stack, it is sneaking out of idle into softirq without telling RCU.
This would cause RCU to complain bitterly about being invoked from
the idle loop -- and RCU ignores CPUs in the idle loop.
Thanx, Paul
> [36456.693191]
> [36456.712658] ===============================
> [36456.767614] [ INFO: suspicious RCU usage. ]
> [36456.822588] 3.4.0-rc2-00016-ga9e1e53 #36 Not tainted
> [36456.887835] -------------------------------
> [36456.942804] include/linux/netpoll.h:70 suspicious rcu_dereference_check() usage!
> [36457.040083]
> [36457.040089] other info that might help us debug this:
> [36457.040098]
> [36457.145306]
> [36457.145312] RCU used illegally from idle CPU!
> [36457.145320] rcu_scheduler_active = 1, debug_locks = 0
> [36457.288293] RCU used illegally from extended quiescent state!
> [36457.363834] no locks held by swapper/0.
> [36457.414221]
> [36457.414227] stack backtrace:
> [36457.471471] Call Trace:
> [36457.503600] [0000000000489834] lockdep_rcu_suspicious+0xd4/0x100
> [36457.583727] [00000000006755a8] __netif_receive_skb+0x368/0xa80
> [36457.661536] [0000000000675e6c] netif_receive_skb+0x4c/0x60
> [36457.734787] [000000000063fd74] tulip_poll+0x3b4/0x6a0
> [36457.802327] [00000000006794d8] net_rx_action+0x118/0x1e0
> [36457.873299] [00000000004560fc] __do_softirq+0x9c/0x140
> [36457.941984] [000000000042b1c4] do_softirq+0x84/0xc0
> [36458.007229] [0000000000404a40] __handle_softirq+0x0/0x10
> [36458.078199] [000000000042b688] cpu_idle+0x48/0x100
> [36458.142314] [0000000000722db8] rest_init+0x160/0x188
> [36458.208711] [00000000008c87b0] start_kernel+0x32c/0x33c
> [36458.278530] [0000000000722c50] tlb_fixup_done+0x88/0x90
> [36458.348346] [0000000000000000] (null)
>
>
> --
> Meelis Roos (mroos@linux.ee)
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
^ permalink raw reply
* Re: [PATCH 1/1] netfilter: xt_recent: Add optional mask option for xt_recent
From: Pablo Neira Ayuso @ 2012-04-11 23:14 UTC (permalink / raw)
To: Denys Fedoryshchenko
Cc: Patrick McHardy, David S. Miller, netfilter-devel, netfilter,
coreteam, linux-kernel, netdev
In-Reply-To: <1331033084-23803-1-git-send-email-denys@visp.net.lb>
Hi Denys,
On Tue, Mar 06, 2012 at 01:24:44PM +0200, Denys Fedoryshchenko wrote:
> Use case for this feature:
> 1)In some occasions if you need to allow,block,match specific subnet.
> 2)I can use recent as a trigger when netfilter rule matches, with mask 0.0.0.0
>
> Example:
>
> If you ping 8.8.8.8, after that you can't ping 2.2.2.10
Could you provide an useful example for this new feature?
I also think you can make this with hashlimit, that allows you to
set the network mask.
^ permalink raw reply
* Re: [PATCH] netfilter: ipvs: Verify that IP_VS protocol has been registered
From: Pablo Neira Ayuso @ 2012-04-11 23:22 UTC (permalink / raw)
To: Sasha Levin
Cc: wensong, horms, ja, kaber, davem, davej, netdev, lvs-devel,
netfilter-devel, linux-kernel
In-Reply-To: <1333668273-8873-1-git-send-email-levinsasha928@gmail.com>
On Thu, Apr 05, 2012 at 07:24:33PM -0400, Sasha Levin wrote:
> The registration of a protocol might fail, there were no checks
> and all registrations were assumed to be correct. This lead to
> NULL ptr dereferences when apps tried registering.
>
> For example:
>
> [ 1293.226051] BUG: unable to handle kernel NULL pointer dereference at 0000000000000018
> [ 1293.227038] IP: [<ffffffff822aacb0>] tcp_register_app+0x60/0xb0
> [ 1293.227038] PGD 391de067 PUD 6c20b067 PMD 0
> [ 1293.227038] Oops: 0000 [#1] PREEMPT SMP
> [ 1293.227038] CPU 1
> [ 1293.227038] Pid: 19609, comm: trinity Tainted: G W 3.4.0-rc1-next-20120405-sasha-dirty #57
> [ 1293.227038] RIP: 0010:[<ffffffff822aacb0>] [<ffffffff822aacb0>] tcp_register_app+0x60/0xb0
> [ 1293.227038] RSP: 0018:ffff880038c1dd18 EFLAGS: 00010286
> [ 1293.227038] RAX: ffffffffffffffc0 RBX: 0000000000001500 RCX: 0000000000010000
> [ 1293.227038] RDX: 0000000000000000 RSI: ffff88003a2d5888 RDI: 0000000000000282
> [ 1293.227038] RBP: ffff880038c1dd48 R08: 0000000000000000 R09: 0000000000000000
> [ 1293.227038] R10: 0000000000000000 R11: 0000000000000000 R12: ffff88003a2d5668
> [ 1293.227038] R13: ffff88003a2d5988 R14: ffff8800696a8ff8 R15: 0000000000000000
> [ 1293.227038] FS: 00007f01930d9700(0000) GS:ffff88007ce00000(0000) knlGS:0000000000000000
> [ 1293.227038] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
> [ 1293.227038] CR2: 0000000000000018 CR3: 0000000065dfc000 CR4: 00000000000406e0
> [ 1293.227038] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [ 1293.227038] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
> [ 1293.227038] Process trinity (pid: 19609, threadinfo ffff880038c1c000, task ffff88002dc73000)
> [ 1293.227038] Stack:
> [ 1293.227038] ffff880038c1dd48 00000000fffffff4 ffff8800696aada0 ffff8800694f5580
> [ 1293.227038] ffffffff8369f1e0 0000000000001500 ffff880038c1dd98 ffffffff822a716b
> [ 1293.227038] 0000000000000000 ffff8800696a8ff8 0000000000000015 ffff8800694f5580
> [ 1293.227038] Call Trace:
> [ 1293.227038] [<ffffffff822a716b>] ip_vs_app_inc_new+0xdb/0x180
> [ 1293.227038] [<ffffffff822a7258>] register_ip_vs_app_inc+0x48/0x70
> [ 1293.227038] [<ffffffff822b2fea>] __ip_vs_ftp_init+0xba/0x140
> [ 1293.227038] [<ffffffff821c9060>] ops_init+0x80/0x90
> [ 1293.227038] [<ffffffff821c90cb>] setup_net+0x5b/0xe0
> [ 1293.227038] [<ffffffff821c9416>] copy_net_ns+0x76/0x100
> [ 1293.227038] [<ffffffff810dc92b>] create_new_namespaces+0xfb/0x190
> [ 1293.227038] [<ffffffff810dca21>] unshare_nsproxy_namespaces+0x61/0x80
> [ 1293.227038] [<ffffffff810afd1f>] sys_unshare+0xff/0x290
> [ 1293.227038] [<ffffffff8187622e>] ? trace_hardirqs_on_thunk+0x3a/0x3f
> [ 1293.227038] [<ffffffff82665539>] system_call_fastpath+0x16/0x1b
> [ 1293.227038] Code: 89 c7 e8 34 91 3b 00 89 de 66 c1 ee 04 31 de 83 e6 0f 48 83 c6 22 48 c1 e6 04 4a 8b 14 26 49 8d 34 34 48 8d 42 c0 48 39 d6 74 13 <66> 39 58 58 74 22 48 8b 48 40 48 8d 41 c0 48 39 ce 75 ed 49 8d
> [ 1293.227038] RIP [<ffffffff822aacb0>] tcp_register_app+0x60/0xb0
> [ 1293.227038] RSP <ffff880038c1dd18>
> [ 1293.227038] CR2: 0000000000000018
> [ 1293.379284] ---[ end trace 364ab40c7011a009 ]---
> [ 1293.381182] Kernel panic - not syncing: Fatal exception in interrupt
>
> Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
> ---
> net/netfilter/ipvs/ip_vs_proto.c | 21 ++++++++++++++++-----
> 1 files changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/net/netfilter/ipvs/ip_vs_proto.c b/net/netfilter/ipvs/ip_vs_proto.c
> index f843a88..43893ba 100644
> --- a/net/netfilter/ipvs/ip_vs_proto.c
> +++ b/net/netfilter/ipvs/ip_vs_proto.c
> @@ -316,20 +316,31 @@ ip_vs_tcpudp_debug_packet(int af, struct ip_vs_protocol *pp,
> */
> int __net_init ip_vs_protocol_net_init(struct net *net)
> {
> + int ret;
> #ifdef CONFIG_IP_VS_PROTO_TCP
> - register_ip_vs_proto_netns(net, &ip_vs_protocol_tcp);
> + ret = register_ip_vs_proto_netns(net, &ip_vs_protocol_tcp);
> + if (ret < 0)
> + return ret;
> #endif
> #ifdef CONFIG_IP_VS_PROTO_UDP
> - register_ip_vs_proto_netns(net, &ip_vs_protocol_udp);
> + ret = register_ip_vs_proto_netns(net, &ip_vs_protocol_udp);
> + if (ret < 0)
> + return ret;
If you return here, I think you'll leave things in inconsistent state,
ie. the tcp protocol is registered. You have to unregister it before
leaving.
ret = register_ip_vs_proto_netns(net, &ip_vs_protocol_tcp);
if (ret < 0)
goto err_tcp;
...
err_tcp:
unregister_ip_vs_proto_netns(net, &ip_vs_protocol_tcp);
^ permalink raw reply
* Re: [RFC PATCH 1/2] net: ethtool: Add capability to retrieve plug-in module EEPROM
From: Ben Hutchings @ 2012-04-11 23:42 UTC (permalink / raw)
To: Stuart Hodgson
Cc: netdev, bruce.w.allan, mirq-linux, decot, amit.salecha,
alexander.h.duyck, davem, linux-kernel
In-Reply-To: <1334168217.2552.5.camel@bwh-desktop.uk.solarflarecom.com>
On Wed, 2012-04-11 at 19:16 +0100, Ben Hutchings wrote:
> On Wed, 2012-04-11 at 17:50 +0100, Stuart Hodgson wrote:
> > On 02/04/12 18:52, Ben Hutchings wrote:
> [...]
> > >> --- a/net/core/ethtool.c
> > >> +++ b/net/core/ethtool.c
> [...]
> > >> + if (eeprom.offset + eeprom.len> modinfo.eeprom_len)
> > >> + return -EINVAL;
> > >> +
> > >> + data = kmalloc(PAGE_SIZE, GFP_USER);
> > >> + if (!data)
> > >> + return -ENOMEM;
> > >
> > > What if some device has a larger EEPROM? Surely this length should be
> > > eeprom.len.
> > >
> >
> > Do you mean what if the eeprom length in te device is larger than
> > PAGE_SIZE?
>
> Yes.
>
> > If so then it should really use modinfo.eeprom_len since
> > this the size of the data. eeprom.len could be arbitary.
>
> No, eeprom.len is the size of the data and we've already validated it at
> this point.
Maybe we should start by refactoring ethtool_get_eeprom() so we can
reuse most of its code in ethtool_get_module_eeprom(), rather than
having to worry about what the maximum size of a module EEPROM might be
and whether we need a loop:
Subject: ethtool: Split ethtool_get_eeprom() to allow for additional EEPROM accessors
We want to support reading module (SFP+, XFP, ...) EEPROMs as well as
NIC EEPROMs. They will need a different command number and driver
operation, but the structure and arguments will be the same and so we
can share most of the code here.
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
net/core/ethtool.c | 24 +++++++++++++++++-------
1 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index beacdd9..ca7698f 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -751,18 +751,17 @@ static int ethtool_get_link(struct net_device *dev, char __user *useraddr)
return 0;
}
-static int ethtool_get_eeprom(struct net_device *dev, void __user *useraddr)
+static int ethtool_get_any_eeprom(struct net_device *dev, void __user *useraddr,
+ int (*getter)(struct net_device *,
+ struct ethtool_eeprom *, u8 *),
+ u32 total_len)
{
struct ethtool_eeprom eeprom;
- const struct ethtool_ops *ops = dev->ethtool_ops;
void __user *userbuf = useraddr + sizeof(eeprom);
u32 bytes_remaining;
u8 *data;
int ret = 0;
- if (!ops->get_eeprom || !ops->get_eeprom_len)
- return -EOPNOTSUPP;
-
if (copy_from_user(&eeprom, useraddr, sizeof(eeprom)))
return -EFAULT;
@@ -771,7 +770,7 @@ static int ethtool_get_eeprom(struct net_device *dev, void __user *useraddr)
return -EINVAL;
/* Check for exceeding total eeprom len */
- if (eeprom.offset + eeprom.len > ops->get_eeprom_len(dev))
+ if (eeprom.offset + eeprom.len > total_len)
return -EINVAL;
data = kmalloc(PAGE_SIZE, GFP_USER);
@@ -782,7 +781,7 @@ static int ethtool_get_eeprom(struct net_device *dev, void __user *useraddr)
while (bytes_remaining > 0) {
eeprom.len = min(bytes_remaining, (u32)PAGE_SIZE);
- ret = ops->get_eeprom(dev, &eeprom, data);
+ ret = getter(dev, &eeprom, data);
if (ret)
break;
if (copy_to_user(userbuf, data, eeprom.len)) {
@@ -803,6 +802,17 @@ static int ethtool_get_eeprom(struct net_device *dev, void __user *useraddr)
return ret;
}
+static int ethtool_get_eeprom(struct net_device *dev, void __user *useraddr)
+{
+ const struct ethtool_ops *ops = dev->ethtool_ops;
+
+ if (!ops->get_eeprom || !ops->get_eeprom_len)
+ return -EOPNOTSUPP;
+
+ return ethtool_get_any_eeprom(dev, useraddr, ops->get_eeprom,
+ ops->get_eeprom_len(dev));
+}
+
static int ethtool_set_eeprom(struct net_device *dev, void __user *useraddr)
{
struct ethtool_eeprom eeprom;
--
1.7.7.6
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply related
* Re: [PATCH] netfilter: ipvs: Verify that IP_VS protocol has been registered
From: Pablo Neira Ayuso @ 2012-04-11 23:46 UTC (permalink / raw)
To: Sasha Levin
Cc: wensong, horms, ja, kaber, davem, davej, netdev, lvs-devel,
netfilter-devel, linux-kernel
In-Reply-To: <20120411232243.GB7038@1984>
On Thu, Apr 12, 2012 at 01:22:43AM +0200, Pablo Neira Ayuso wrote:
> On Thu, Apr 05, 2012 at 07:24:33PM -0400, Sasha Levin wrote:
> > The registration of a protocol might fail, there were no checks
> > and all registrations were assumed to be correct. This lead to
> > NULL ptr dereferences when apps tried registering.
> >
> > For example:
> >
> > [ 1293.226051] BUG: unable to handle kernel NULL pointer dereference at 0000000000000018
> > [ 1293.227038] IP: [<ffffffff822aacb0>] tcp_register_app+0x60/0xb0
> > [ 1293.227038] PGD 391de067 PUD 6c20b067 PMD 0
> > [ 1293.227038] Oops: 0000 [#1] PREEMPT SMP
> > [ 1293.227038] CPU 1
> > [ 1293.227038] Pid: 19609, comm: trinity Tainted: G W 3.4.0-rc1-next-20120405-sasha-dirty #57
> > [ 1293.227038] RIP: 0010:[<ffffffff822aacb0>] [<ffffffff822aacb0>] tcp_register_app+0x60/0xb0
> > [ 1293.227038] RSP: 0018:ffff880038c1dd18 EFLAGS: 00010286
> > [ 1293.227038] RAX: ffffffffffffffc0 RBX: 0000000000001500 RCX: 0000000000010000
> > [ 1293.227038] RDX: 0000000000000000 RSI: ffff88003a2d5888 RDI: 0000000000000282
> > [ 1293.227038] RBP: ffff880038c1dd48 R08: 0000000000000000 R09: 0000000000000000
> > [ 1293.227038] R10: 0000000000000000 R11: 0000000000000000 R12: ffff88003a2d5668
> > [ 1293.227038] R13: ffff88003a2d5988 R14: ffff8800696a8ff8 R15: 0000000000000000
> > [ 1293.227038] FS: 00007f01930d9700(0000) GS:ffff88007ce00000(0000) knlGS:0000000000000000
> > [ 1293.227038] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
> > [ 1293.227038] CR2: 0000000000000018 CR3: 0000000065dfc000 CR4: 00000000000406e0
> > [ 1293.227038] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > [ 1293.227038] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
> > [ 1293.227038] Process trinity (pid: 19609, threadinfo ffff880038c1c000, task ffff88002dc73000)
> > [ 1293.227038] Stack:
> > [ 1293.227038] ffff880038c1dd48 00000000fffffff4 ffff8800696aada0 ffff8800694f5580
> > [ 1293.227038] ffffffff8369f1e0 0000000000001500 ffff880038c1dd98 ffffffff822a716b
> > [ 1293.227038] 0000000000000000 ffff8800696a8ff8 0000000000000015 ffff8800694f5580
> > [ 1293.227038] Call Trace:
> > [ 1293.227038] [<ffffffff822a716b>] ip_vs_app_inc_new+0xdb/0x180
> > [ 1293.227038] [<ffffffff822a7258>] register_ip_vs_app_inc+0x48/0x70
> > [ 1293.227038] [<ffffffff822b2fea>] __ip_vs_ftp_init+0xba/0x140
> > [ 1293.227038] [<ffffffff821c9060>] ops_init+0x80/0x90
> > [ 1293.227038] [<ffffffff821c90cb>] setup_net+0x5b/0xe0
> > [ 1293.227038] [<ffffffff821c9416>] copy_net_ns+0x76/0x100
> > [ 1293.227038] [<ffffffff810dc92b>] create_new_namespaces+0xfb/0x190
> > [ 1293.227038] [<ffffffff810dca21>] unshare_nsproxy_namespaces+0x61/0x80
> > [ 1293.227038] [<ffffffff810afd1f>] sys_unshare+0xff/0x290
> > [ 1293.227038] [<ffffffff8187622e>] ? trace_hardirqs_on_thunk+0x3a/0x3f
> > [ 1293.227038] [<ffffffff82665539>] system_call_fastpath+0x16/0x1b
> > [ 1293.227038] Code: 89 c7 e8 34 91 3b 00 89 de 66 c1 ee 04 31 de 83 e6 0f 48 83 c6 22 48 c1 e6 04 4a 8b 14 26 49 8d 34 34 48 8d 42 c0 48 39 d6 74 13 <66> 39 58 58 74 22 48 8b 48 40 48 8d 41 c0 48 39 ce 75 ed 49 8d
> > [ 1293.227038] RIP [<ffffffff822aacb0>] tcp_register_app+0x60/0xb0
> > [ 1293.227038] RSP <ffff880038c1dd18>
> > [ 1293.227038] CR2: 0000000000000018
> > [ 1293.379284] ---[ end trace 364ab40c7011a009 ]---
> > [ 1293.381182] Kernel panic - not syncing: Fatal exception in interrupt
> >
> > Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
> > ---
> > net/netfilter/ipvs/ip_vs_proto.c | 21 ++++++++++++++++-----
> > 1 files changed, 16 insertions(+), 5 deletions(-)
> >
> > diff --git a/net/netfilter/ipvs/ip_vs_proto.c b/net/netfilter/ipvs/ip_vs_proto.c
> > index f843a88..43893ba 100644
> > --- a/net/netfilter/ipvs/ip_vs_proto.c
> > +++ b/net/netfilter/ipvs/ip_vs_proto.c
> > @@ -316,20 +316,31 @@ ip_vs_tcpudp_debug_packet(int af, struct ip_vs_protocol *pp,
> > */
> > int __net_init ip_vs_protocol_net_init(struct net *net)
> > {
> > + int ret;
> > #ifdef CONFIG_IP_VS_PROTO_TCP
> > - register_ip_vs_proto_netns(net, &ip_vs_protocol_tcp);
> > + ret = register_ip_vs_proto_netns(net, &ip_vs_protocol_tcp);
> > + if (ret < 0)
> > + return ret;
> > #endif
> > #ifdef CONFIG_IP_VS_PROTO_UDP
> > - register_ip_vs_proto_netns(net, &ip_vs_protocol_udp);
> > + ret = register_ip_vs_proto_netns(net, &ip_vs_protocol_udp);
> > + if (ret < 0)
> > + return ret;
>
> If you return here, I think you'll leave things in inconsistent state,
> ie. the tcp protocol is registered. You have to unregister it before
> leaving.
>
> ret = register_ip_vs_proto_netns(net, &ip_vs_protocol_tcp);
^^^
sorry, I meant to say udp here.
> if (ret < 0)
> goto err_tcp;
^^^
and here.
> ...
> err_tcp:
> unregister_ip_vs_proto_netns(net, &ip_vs_protocol_tcp);
^ permalink raw reply
* Re: suspicious RCU usage warnings in 3.3.0
From: Stephen Hemminger @ 2012-04-12 0:10 UTC (permalink / raw)
To: paulmck; +Cc: Meelis Roos, David Miller, linux-kernel, netdev
In-Reply-To: <20120411230837.GC2473@linux.vnet.ibm.com>
On Wed, 11 Apr 2012 16:08:37 -0700
"Paul E. McKenney" <paulmck@linux.vnet.ibm.com> wrote:
> Hmmm... What CPU family is this running on? From the look of the
> stack, it is sneaking out of idle into softirq without telling RCU.
> This would cause RCU to complain bitterly about being invoked from
> the idle loop -- and RCU ignores CPUs in the idle loop.
>
> Thanx, Paul
Sun4... Ping David.
^ permalink raw reply
* Re: [PATCH 5/9] ipvs: use adaptive pause in master thread
From: Pablo Neira Ayuso @ 2012-04-12 0:13 UTC (permalink / raw)
To: Julian Anastasov
Cc: Simon Horman, lvs-devel, netdev, netfilter-devel, Wensong Zhang
In-Reply-To: <alpine.LFD.2.00.1204112221250.2000@ja.ssi.bg>
Hi Julian,
On Wed, Apr 11, 2012 at 11:02:39PM +0300, Julian Anastasov wrote:
>
> Hello,
>
> On Tue, 10 Apr 2012, Pablo Neira Ayuso wrote:
>
> > You can still use kthread_should_stop inside a wrapper function
> > that calls kthread_stop and up() the semaphore.
> >
> > sync_stop:
> > kthread_stop(k)
> > up(s)
> >
> > kthread_routine:
> > while(1) {
> > down(s)
> > if (kthread_should_stop(k))
> > break;
> >
> > get sync msg
> > send sync msg
> > }
> >
> > BTW, each up() does not necessarily mean one wakeup event. up() will
> > delivery only one wakeup event for one process that has been already
> > awaken.
>
> OK, now I added up(). It will be called when
> 32 messages are queued after last sent by thread.
Why 32?
If you do up() once per message, you will still get an arbitrary
number of messages in the queue until the scheduler selects your
thread to enter the running state.
In other works, if you do up() once per 32 messages, your thread will
get N+32 messages in its queue by the time the scheduler makes it
enter the running state. Being N that amount of arbitrary messages.
This seems to me like more chances to overrun the socket buffer under
high stress.
> > > I'm still thinking if sndbuf value should be exported,
> > > currently users have to modify the global default/max value.
> >
> > I think it's a good idea.
>
> Done, used same value both for rcvbuf and sndbuf.
>
> > > But in below version I'm trying to handle the sndbuf overflow
> > > by blocking for write_space event. By this way we should work
> > > with any sndbuf configuration.
> >
> > You seem to be defering the overrun problem by using a longer
> > intermediate queue than the socket buffer. Then, that queue can be
> > tuned by the user via sysctl. It may happen under heavy stress that
> > your intermediate queue gets full again, then you'll have to drop
> > packets at some point.
>
> Yes, both values are for same thing, the problem is
> that queue size is in messages while socket buffer is in bytes.
> And as sndbuf config is optional, I'm not trying to derive
> sync_qlen_max from sndbuf. May be we can do it after
> socket is created but it will cause problem for systems
> that do not configure sync_sock_size, they before now used
> unlimited queue and may be default socket size, so using
> some small default sndbuf as sync_qlen_max can cause message
> drops. They will use reduced limits. So, now we provide
> some large sync_qlen_max as default configuration
> which probably exceeds the default socket buffer.
>
> Still, I think the down/up idea is not better.
> We are adding two new vars: master_stopped and
> master_sem.
Well, this is not exactly the idea I had in mind.
> The problem is that kthread_stop() is a blocking
> function. It waits thread to terminate. It can not wakeup
> thread blocked in down(), so we add master_stopped flag
> that will unblock the down() loop while kthread_stop() will also
> unblock thread if waiting for write_space. I.e. up()+kthread_stop()
> is racy without additional flag while kthread_stop()+up()
> is not possible to work.
I don't see the up+kthread_stop() race you mention.
> I'm appending untested version with up+down but I think
> we should use wake_up_process and schedule_timeout instead,
> as in previous version.
OK.
I still think that using an intermediate queue is *not* the way
to achieve reliability and congestion control, sorry.
But, you seem to persist on the idea and I don't want to block your
developments, I just wanted to show my point and provide some ideas.
After all, you maintain that part of the code.
Please, tell me what patch you want me to apply and I'll take it.
^ permalink raw reply
* Re: [PATCH v17 01/15] Add PR_{GET,SET}_NO_NEW_PRIVS to prevent execve from granting privs
From: Michael Kerrisk (man-pages) @ 2012-04-12 0:15 UTC (permalink / raw)
To: Jonathan Corbet
Cc: Andrew Lutomirski, Andrew Morton, Will Drewry, linux-kernel,
linux-security-module, linux-arch, linux-doc, kernel-hardening,
netdev, x86, arnd, davem, hpa, mingo, oleg, peterz, rdunlap,
mcgrathr, tglx, eparis, serge.hallyn, djm, scarybeasts, indan,
pmoore, eric.dumazet, markus, coreyb, keescook, jmorris,
Andy Lutomirski, linux-man
In-Reply-To: <CAKgNAkhhnC1+vdh7vTYKkngzxHccDwHH_aKF5wCNhga33BGhyQ@mail.gmail.com>
> no longer unsustainable for me (man-pages is in competition with
^no longer unsustainable^no longer sustainable
^ permalink raw reply
* Re: suspicious RCU usage warnings in 3.3.0
From: David Miller @ 2012-04-12 0:18 UTC (permalink / raw)
To: shemminger; +Cc: paulmck, mroos, linux-kernel, netdev
In-Reply-To: <20120411171004.016ddd95@nehalam.linuxnetplumber.net>
From: Stephen Hemminger <shemminger@vyatta.com>
Date: Wed, 11 Apr 2012 17:10:04 -0700
> On Wed, 11 Apr 2012 16:08:37 -0700
> "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> wrote:
>
>> Hmmm... What CPU family is this running on? From the look of the
>> stack, it is sneaking out of idle into softirq without telling RCU.
>> This would cause RCU to complain bitterly about being invoked from
>> the idle loop -- and RCU ignores CPUs in the idle loop.
>>
>> Thanx, Paul
>
> Sun4... Ping David.
So is there anything specific I need to do in the sparc64
idle loop?
^ permalink raw reply
* Re: suspicious RCU usage warnings in 3.3.0
From: Paul E. McKenney @ 2012-04-12 0:45 UTC (permalink / raw)
To: David Miller; +Cc: shemminger, mroos, linux-kernel, netdev
In-Reply-To: <20120411.201854.1070083308359208025.davem@davemloft.net>
On Wed, Apr 11, 2012 at 08:18:54PM -0400, David Miller wrote:
> From: Stephen Hemminger <shemminger@vyatta.com>
> Date: Wed, 11 Apr 2012 17:10:04 -0700
>
> > On Wed, 11 Apr 2012 16:08:37 -0700
> > "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> wrote:
> >
> >> Hmmm... What CPU family is this running on? From the look of the
> >> stack, it is sneaking out of idle into softirq without telling RCU.
> >> This would cause RCU to complain bitterly about being invoked from
> >> the idle loop -- and RCU ignores CPUs in the idle loop.
> >>
> >> Thanx, Paul
> >
> > Sun4... Ping David.
>
> So is there anything specific I need to do in the sparc64
> idle loop?
Hmmm... I must confess that I don't immediately see how control
is passing from cpu_idle() in arch/sparc/kernel/process_64.c to
__handle_softirq().
But it looks like a simple function call in the call trace:
[36457.471471] Call Trace:
[36457.503600] [0000000000489834] lockdep_rcu_suspicious+0xd4/0x100
[36457.583727] [00000000006755a8] __netif_receive_skb+0x368/0xa80
[36457.661536] [0000000000675e6c] netif_receive_skb+0x4c/0x60
[36457.734787] [000000000063fd74] tulip_poll+0x3b4/0x6a0
[36457.802327] [00000000006794d8] net_rx_action+0x118/0x1e0
[36457.873299] [00000000004560fc] __do_softirq+0x9c/0x140
[36457.941984] [000000000042b1c4] do_softirq+0x84/0xc0
[36458.007229] [0000000000404a40] __handle_softirq+0x0/0x10
[36458.078199] [000000000042b688] cpu_idle+0x48/0x100
[36458.142314] [0000000000722db8] rest_init+0x160/0x188
[36458.208711] [00000000008c87b0] start_kernel+0x32c/0x33c
[36458.278530] [0000000000722c50] tlb_fixup_done+0x88/0x90
[36458.348346] [0000000000000000] (null)
If it really is a simple function call, the trick is to wrap a RCU_NONIDLE()
around the call point, for example, fancifully:
RCU_NONIDLE(__handle_softirq());
This places an rcu_idle_enter() before the argument and an
rcu_idle_enter() after it. So it might be sufficient to adjust the
positions of the rcu_idle_enter() and rcu_idle_exit() calls in sparc64's
cpu_idle() function, for example, into the sparc64_yield() function
(if that is what is needed -- I can't see how sparc64_yield() calls
__handle_softirq(), either).
If I am confused about the simple function call, and if control is really
passing via an interrupt or exception, then rcu_irq_enter() should be
called on entry to the interrupt or exception and rcu_irq_exit() should
be called on exit.
Otherwise, RCU will happily ignore any RCU read-side critical sections
that are in what it believes to the the idle loop.
Thanx, Paul
^ permalink raw reply
* RE:
From: Felicia @ 2012-04-12 0:45 UTC (permalink / raw)
Do you need a fast approved loan with low
interest rate? Are you tired of looking for a
loan? Just reply and we are here to help.
^ permalink raw reply
* Re: [PATCH v17 01/15] Add PR_{GET,SET}_NO_NEW_PRIVS to prevent execve from granting privs
From: Andrew Lutomirski @ 2012-04-12 0:50 UTC (permalink / raw)
To: mtk.manpages
Cc: Jonathan Corbet, Andrew Morton, Will Drewry, linux-kernel,
linux-security-module, linux-arch, linux-doc, kernel-hardening,
netdev, x86, arnd, davem, hpa, mingo, oleg, peterz, rdunlap,
mcgrathr, tglx, eparis, serge.hallyn, djm, scarybeasts, indan,
pmoore, eric.dumazet, markus, coreyb, keescook, jmorris,
Andy Lutomirski, linux-man
In-Reply-To: <CAKgNAkhhnC1+vdh7vTYKkngzxHccDwHH_aKF5wCNhga33BGhyQ@mail.gmail.com>
On Wed, Apr 11, 2012 at 12:31 PM, Michael Kerrisk (man-pages)
<mtk.manpages@gmail.com> wrote:
> On Sat, Apr 7, 2012 at 8:28 AM, Jonathan Corbet <corbet@lwn.net> wrote:
>> On Fri, 6 Apr 2012 13:01:17 -0700
>> Andrew Lutomirski <luto@mit.edu> wrote:
>>
>>> This has been bugging me for awhile. Is there any interest in moving
>>> the manpages into the kernel source tree? Then there could be a
>>> general requirement that new APIs get documented when they're written.
>>
>> Man page (or other documentation) requirements for patch acceptance are a
>> regular kernel summit feature. People seem to think it's a good idea, but
>> actual enforcement of such requirements always seems to be lacking. Lots
>> of people have kind of given up trying. I don't really see that adding
>> the man pages to the tree would help, but I could be wrong...
>
> I largely consider this (moving man pages to kernel.org) a technical
> solution to what is fundamentally a social problem (developers
> reluctant to write documentation), and doubt that the technical
> solution would make much difference. I'd love to be proved wrong, but
> the experiment would require significant start-up effort. (My
> collected thoughts on this can be found here:
> http://www.kernel.org/doc/man-pages/todo.html#migrate_to_kernel_source.
> Note the alternative idea of patch tags mentioned at the end of that
> text.)
>
> Unless, or until there's a paid maintainer, I don't expect things to
> get significantly better than what they currently are. The quite
> significant improvements in man-pages since 2004, when I became
> maintainer were in small part due to the fact that I was for a short
> period paid to do the work, but in much larger part due to a huge
> private effort over those years which over the last couple of years is
> no longer unsustainable for me (man-pages is in competition with
> requirements for my attention from family, working life, and
> (seriously!) seismic events),
Hrm. Maybe someone could convince Andrew and Linus not to pull new
syscalls or major ABI features unless the patchset includes full docs.
Anyway, I'll write up a detailed description of PR_SET_NO_NEW_PRIVS,
stick it in the changelog, and cc linux-doc.
--Andy
^ permalink raw reply
* Re: suspicious RCU usage warnings in 3.3.0
From: David Miller @ 2012-04-12 1:03 UTC (permalink / raw)
To: paulmck; +Cc: shemminger, mroos, linux-kernel, netdev
In-Reply-To: <20120412004507.GF2473@linux.vnet.ibm.com>
From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Date: Wed, 11 Apr 2012 17:45:07 -0700
> On Wed, Apr 11, 2012 at 08:18:54PM -0400, David Miller wrote:
>> From: Stephen Hemminger <shemminger@vyatta.com>
>> Date: Wed, 11 Apr 2012 17:10:04 -0700
>>
>> > On Wed, 11 Apr 2012 16:08:37 -0700
>> > "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> wrote:
>> >
>> >> Hmmm... What CPU family is this running on? From the look of the
>> >> stack, it is sneaking out of idle into softirq without telling RCU.
>> >> This would cause RCU to complain bitterly about being invoked from
>> >> the idle loop -- and RCU ignores CPUs in the idle loop.
>> >>
>> >> Thanx, Paul
>> >
>> > Sun4... Ping David.
>>
>> So is there anything specific I need to do in the sparc64
>> idle loop?
>
> Hmmm... I must confess that I don't immediately see how control
> is passing from cpu_idle() in arch/sparc/kernel/process_64.c to
> __handle_softirq().
>
> But it looks like a simple function call in the call trace:
It's coming from the trap return code path at the end of hardware
interrupt processing, on the IRQ stack.
^ permalink raw reply
* Re: suspicious RCU usage warnings in 3.3.0
From: David Miller @ 2012-04-12 1:08 UTC (permalink / raw)
To: paulmck; +Cc: shemminger, mroos, linux-kernel, netdev
In-Reply-To: <20120412004507.GF2473@linux.vnet.ibm.com>
From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Date: Wed, 11 Apr 2012 17:45:07 -0700
> If I am confused about the simple function call, and if control is really
> passing via an interrupt or exception, then rcu_irq_enter() should be
> called on entry to the interrupt or exception and rcu_irq_exit() should
> be called on exit.
Hmm, it seems the convention changed such that platforms aren't
supposed to invoke do_softirq() from their trap return trap any more.
It's handled completely by irq_exit().
When did that start happening? :-)
Anyways I bet that's the problem, sparc64 invokes do_softirq() in it's
trap return path if softirqs are pending, and that doesn't do any
of the RCU frobbing you mention.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox