Re: [PATCH] openvswitch: make for_each_node loops work with sparse numa systems
On 21.07.2015 [10:32:34 -0500], Chris J Arges wrote: Some architectures like POWER can have a NUMA node_possible_map that contains sparse entries. This causes memory corruption with openvswitch since it allocates flow_cache with a multiple of num_possible_nodes() and Couldn't this also be fixed by just allocationg with a multiple of nr_node_ids (which seems to have been the original intent all along)? You could then make your stats array be sparse or not. assumes the node variable returned by for_each_node will index into flow-stats[node]. For example, if node_possible_map is 0x30003, this patch will map node to node_cnt as follows: 0,1,16,17 = 0,1,2,3 The crash was noticed after 3af229f2 was applied as it changed the node_possible_map to match node_online_map on boot. Fixes: 3af229f2071f5b5cb31664be6109561fbe19c861 My concern with this version of the fix is that you're relying on, implicitly, the order of for_each_node's iteration corresponding to the entries in stats 1:1. But what about node hotplug? It seems better to have the enumeration of the stats array match the topology accurately, rather, or to maintain some sort of internal map in the OVS code between the NUMA node and the entry in the stats array? I'm willing to be convinced otherwise, though :) -Nish Signed-off-by: Chris J Arges chris.j.ar...@canonical.com --- net/openvswitch/flow.c | 10 ++ net/openvswitch/flow_table.c | 18 +++--- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c index bc7b0ab..425d45d 100644 --- a/net/openvswitch/flow.c +++ b/net/openvswitch/flow.c @@ -134,14 +134,14 @@ void ovs_flow_stats_get(const struct sw_flow *flow, struct ovs_flow_stats *ovs_stats, unsigned long *used, __be16 *tcp_flags) { - int node; + int node, node_cnt = 0; *used = 0; *tcp_flags = 0; memset(ovs_stats, 0, sizeof(*ovs_stats)); for_each_node(node) { - struct flow_stats *stats = rcu_dereference_ovsl(flow-stats[node]); + struct flow_stats *stats = rcu_dereference_ovsl(flow-stats[node_cnt]); if (stats) { /* Local CPU may write on non-local stats, so we must @@ -155,16 +155,17 @@ void ovs_flow_stats_get(const struct sw_flow *flow, ovs_stats-n_bytes += stats-byte_count; spin_unlock_bh(stats-lock); } + node_cnt++; } } /* Called with ovs_mutex. */ void ovs_flow_stats_clear(struct sw_flow *flow) { - int node; + int node, node_cnt = 0; for_each_node(node) { - struct flow_stats *stats = ovsl_dereference(flow-stats[node]); + struct flow_stats *stats = ovsl_dereference(flow-stats[node_cnt]); if (stats) { spin_lock_bh(stats-lock); @@ -174,6 +175,7 @@ void ovs_flow_stats_clear(struct sw_flow *flow) stats-tcp_flags = 0; spin_unlock_bh(stats-lock); } + node_cnt++; } } diff --git a/net/openvswitch/flow_table.c b/net/openvswitch/flow_table.c index 4613df8..5d10c54 100644 --- a/net/openvswitch/flow_table.c +++ b/net/openvswitch/flow_table.c @@ -77,7 +77,7 @@ struct sw_flow *ovs_flow_alloc(void) { struct sw_flow *flow; struct flow_stats *stats; - int node; + int node, node_cnt = 0; flow = kmem_cache_alloc(flow_cache, GFP_KERNEL); if (!flow) @@ -99,9 +99,11 @@ struct sw_flow *ovs_flow_alloc(void) RCU_INIT_POINTER(flow-stats[0], stats); - for_each_node(node) + for_each_node(node) { if (node != 0) - RCU_INIT_POINTER(flow-stats[node], NULL); + RCU_INIT_POINTER(flow-stats[node_cnt], NULL); + node_cnt++; + } return flow; err: @@ -139,15 +141,17 @@ static struct flex_array *alloc_buckets(unsigned int n_buckets) static void flow_free(struct sw_flow *flow) { - int node; + int node, node_cnt = 0; if (ovs_identifier_is_key(flow-id)) kfree(flow-id.unmasked_key); kfree((struct sw_flow_actions __force *)flow-sf_acts); - for_each_node(node) - if (flow-stats[node]) + for_each_node(node) { + if (flow-stats[node_cnt]) kmem_cache_free(flow_stats_cache, - (struct flow_stats __force *)flow-stats[node]); + (struct flow_stats __force *)flow-stats[node_cnt]); + node_cnt++; + } kmem_cache_free(flow_cache, flow); } -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to majord...@vger.kernel.org More majordomo info at
Re: [PATCH v2] openvswitch: allocate nr_node_ids flow_stats instead of num_possible_nodes
On 21.07.2015 [12:36:33 -0500], Chris J Arges wrote: Some architectures like POWER can have a NUMA node_possible_map that contains sparse entries. This causes memory corruption with openvswitch since it allocates flow_cache with a multiple of num_possible_nodes() and assumes the node variable returned by for_each_node will index into flow-stats[node]. Use nr_node_ids to allocate a maximal sparse array instead of num_possible_nodes(). The crash was noticed after 3af229f2 was applied as it changed the node_possible_map to match node_online_map on boot. Fixes: 3af229f2071f5b5cb31664be6109561fbe19c861 Signed-off-by: Chris J Arges chris.j.ar...@canonical.com Acked-by: Nishanth Aravamudan n...@linux.vnet.ibm.com --- net/openvswitch/flow_table.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/openvswitch/flow_table.c b/net/openvswitch/flow_table.c index 4613df8..6552394 100644 --- a/net/openvswitch/flow_table.c +++ b/net/openvswitch/flow_table.c @@ -752,7 +752,7 @@ int ovs_flow_init(void) BUILD_BUG_ON(sizeof(struct sw_flow_key) % sizeof(long)); flow_cache = kmem_cache_create(sw_flow, sizeof(struct sw_flow) -+ (num_possible_nodes() ++ (nr_node_ids * sizeof(struct flow_stats *)), 0, 0, NULL); if (flow_cache == NULL) -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] openvswitch: make for_each_node loops work with sparse numa systems
On 21.07.2015 [11:30:58 -0500], Chris J Arges wrote: On Tue, Jul 21, 2015 at 09:24:18AM -0700, Nishanth Aravamudan wrote: On 21.07.2015 [10:32:34 -0500], Chris J Arges wrote: Some architectures like POWER can have a NUMA node_possible_map that contains sparse entries. This causes memory corruption with openvswitch since it allocates flow_cache with a multiple of num_possible_nodes() and Couldn't this also be fixed by just allocationg with a multiple of nr_node_ids (which seems to have been the original intent all along)? You could then make your stats array be sparse or not. Yea originally this is what I did, but I thought it would be wasting memory. assumes the node variable returned by for_each_node will index into flow-stats[node]. For example, if node_possible_map is 0x30003, this patch will map node to node_cnt as follows: 0,1,16,17 = 0,1,2,3 The crash was noticed after 3af229f2 was applied as it changed the node_possible_map to match node_online_map on boot. Fixes: 3af229f2071f5b5cb31664be6109561fbe19c861 My concern with this version of the fix is that you're relying on, implicitly, the order of for_each_node's iteration corresponding to the entries in stats 1:1. But what about node hotplug? It seems better to have the enumeration of the stats array match the topology accurately, rather, or to maintain some sort of internal map in the OVS code between the NUMA node and the entry in the stats array? I'm willing to be convinced otherwise, though :) -Nish Nish, The method I described should work for hotplug since it's using possible map which AFAIK is static rather than the online map. Oh you're right, I'm sorry! -- To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: select(0, ..) is valid ?
On 22.05.2007 [09:16:37 -0500], Steve Fox wrote: On Wed, 2007-05-16 at 17:59 -0700, Badari Pulavarty wrote: Here it is .. Should I do one for poll() also ? Thanks, Badari Optimize select by a using stack space for small fd sets. core_sys_select() already has this optimization. This is for compat version. Signed-off-by: Badari Pulavarty [EMAIL PROTECTED] --- fs/compat.c | 17 +++-- 1 file changed, 11 insertions(+), 6 deletions(-) Index: linux-2.6.22-rc1/fs/compat.c === --- linux-2.6.22-rc1.orig/fs/compat.c 2007-05-12 18:45:56.0 -0700 +++ linux-2.6.22-rc1/fs/compat.c2007-05-16 17:50:39.0 -0700 @@ -1544,9 +1544,10 @@ int compat_core_sys_select(int n, compat compat_ulong_t __user *outp, compat_ulong_t __user *exp, s64 *timeout) { fd_set_bits fds; - char *bits; + void *bits; int size, max_fds, ret = -EINVAL; struct fdtable *fdt; + long stack_fds[SELECT_STACK_ALLOC/sizeof(long)]; if (n 0) goto out_nofds; @@ -1564,11 +1565,14 @@ int compat_core_sys_select(int n, compat * since we used fdset we need to allocate memory in units of * long-words. */ - ret = -ENOMEM; size = FDS_BYTES(n); - bits = kmalloc(6 * size, GFP_KERNEL); - if (!bits) - goto out_nofds; + bits = stack_fds; + if (size sizeof(stack_fds) / 6) { + bits = kmalloc(6 * size, GFP_KERNEL); + ret = -ENOMEM; + if (!bits) + goto out_nofds; + } fds.in = (unsigned long *) bits; fds.out = (unsigned long *) (bits + size); fds.ex = (unsigned long *) (bits + 2*size); @@ -1600,7 +1604,8 @@ int compat_core_sys_select(int n, compat compat_set_fd_set(n, exp, fds.res_ex)) ret = -EFAULT; out: - kfree(bits); + if (bits != stack_fds) + kfree(bits); out_nofds: return ret; } Andy put this through a couple machines on test.kernel.org and elm3b6 was fixed, however elm3b239 still had a boot error. BUG: at mm/slab.c:777 __find_general_cachep() Call Trace: [802729c6] __kmalloc+0xa6/0xe0 [8021d21b] cache_k8_northbridges+0x9b/0x120 I believe this is fixed by: http://lkml.org/lkml/2007/5/18/19 Care to stack it on top and retest? Thanks, Nish -- Nishanth Aravamudan [EMAIL PROTECTED] IBM Linux Technology Center - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
tg3: unable to handle null pointer dereference [Re: Linux 2.6.21-rc6]
[ 14.645283] sp=e01005bc7e00 bsp=e01005bc0ca8 [ 14.658540] [a001008a85f0] init+0x390/0x740 [ 14.658542] sp=e01005bc7e00 bsp=e01005bc0c58 [ 14.671627] [a001000113d0] kernel_thread_helper+0xd0/0x100 [ 14.671629] sp=e01005bc7e30 bsp=e01005bc0c30 [ 14.686023] [a0019140] start_kernel_thread+0x20/0x40 [ 14.686025] sp=e01005bc7e30 bsp=e01005bc0c30 [ 14.700284] Kernel panic - not syncing: Attempted to kill init! on an 8-way IA64. I'm guessing it's one of these: Michael Chan (5): [TG3]: Eliminate the unused TG3_FLAG_SPLIT_MODE flag. [TG3]: Exit irq handler during chip reset. [TG3]: Update version and reldate. probably Exit irq handler during chip reset? Thanks, Nish -- Nishanth Aravamudan [EMAIL PROTECTED] IBM Linux Technology Center - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: tg3: unable to handle null pointer dereference
On 06.04.2007 [17:36:00 -0700], David Miller wrote: From: Michael Chan [EMAIL PROTECTED] Date: Fri, 06 Apr 2007 15:57:13 -0700 On Fri, 2007-04-06 at 14:40 -0700, Nishanth Aravamudan wrote: 2.6.21-rc5 is ok. 2.6.21-rc6 results in [ 14.241665] Unable to handle kernel NULL pointer dereference (address ) Sorry, I think this should fix it: [TG3]: Fix crash during tg3_init_one(). The driver will crash when the chip has been initialized by EFI before tg3_init_one(). In this case, the driver will call tg3_chip_reset() before allocating consistent memory. The bug is fixed by checking for tp-hw_status before accessing it during tg3_chip_reset(). Signed-off-by: Michael Chan [EMAIL PROTECTED] Applied, thanks Michael. FWIW, tested, no panic. Tested-by: Nishanth Aravamudan [EMAIL PROTECTED] Thanks, Nish -- Nishanth Aravamudan [EMAIL PROTECTED] IBM Linux Technology Center - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [KJ] [PATCH] net/core/flow.c: compare data with memcmp
On 31.12.2006 [17:37:05 +0100], Daniel Marjam?ki wrote: From: Daniel Marjamäki This has been tested by me. Signed-off-by: Daniel Marjamäki [EMAIL PROTECTED] --- linux-2.6.20-rc2/net/core/flow.c 2006-12-27 09:59:56.0 +0100 +++ linux/net/core/flow.c 2006-12-31 18:26:06.0 +0100 @@ -144,29 +144,16 @@ typedef u32 flow_compare_t; extern void flowi_is_missized(void); -/* I hear what you're saying, use memcmp. But memcmp cannot make - * important assumptions that we can here, such as alignment and - * constant size. - */ static int flow_key_compare(struct flowi *key1, struct flowi *key2) { - flow_compare_t *k1, *k1_lim, *k2; - const int n_elem = sizeof(struct flowi) / sizeof(flow_compare_t); - if (sizeof(struct flowi) % sizeof(flow_compare_t)) flowi_is_missized(); - k1 = (flow_compare_t *) key1; - k1_lim = k1 + n_elem; - - k2 = (flow_compare_t *) key2; - - do { - if (*k1++ != *k2++) - return 1; - } while (k1 k1_lim); + /* Number of elements to compare */ + const int n_elem = sizeof(struct flowi) / sizeof(flow_compare_t); - return 0; + /* Compare all elements in key1 and key2. */ + return memcmp(key1, key2, n_elem * sizeof(flow_compare_t)); } Small nit, I don't think either of your comments make the code any clearer than the code on its own, so drop them both. Thanks, Nish -- Nishanth Aravamudan [EMAIL PROTECTED] IBM Linux Technology Center - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[-mm PATCH 05/32] net: fix-up schedule_timeout() usage
Description: Use schedule_timeout_{,un}interruptible() instead of set_current_state()/schedule_timeout() to reduce kernel size. Also use human-time conversion functions instead of hard-coded division to avoid rounding issues. Signed-off-by: Nishanth Aravamudan [EMAIL PROTECTED] --- net/core/pktgen.c| 13 + net/dccp/proto.c |2 +- net/ipv4/ipconfig.c |5 ++--- net/irda/ircomm/ircomm_tty.c |9 +++-- net/sunrpc/svcsock.c |3 +-- 5 files changed, 12 insertions(+), 20 deletions(-) --- 2.6.13-rc5-mm1/net/core/pktgen.c2005-08-07 10:05:22.0 -0700 +++ 2.6.13-rc5-mm1-dev/net/core/pktgen.c2005-08-14 13:32:59.0 -0700 @@ -1452,8 +1452,7 @@ static int proc_thread_write(struct file thread_lock(); t-control |= T_REMDEV; thread_unlock(); - current-state = TASK_INTERRUPTIBLE; - schedule_timeout(HZ/8); /* Propagate thread-control */ + schedule_timeout_interruptible(msecs_to_jiffies(125)); /* Propagate thread-control */ ret = count; sprintf(pg_result, OK: rem_device_all); goto out; @@ -1716,10 +1715,9 @@ static void spin(struct pktgen_dev *pkt_ printk(KERN_INFO sleeping for %d\n, (int)(spin_until_us - now)); while (now spin_until_us) { /* TODO: optimise sleeping behavior */ - if (spin_until_us - now (100/HZ)+1) { - current-state = TASK_INTERRUPTIBLE; - schedule_timeout(1); - } else if (spin_until_us - now 100) { + if (spin_until_us - now jiffies_to_usecs(1)+1) + schedule_timeout_interruptible(1); + else if (spin_until_us - now 100) { do_softirq(); if (!pkt_dev-running) return; @@ -2449,8 +2447,7 @@ static void pktgen_run_all_threads(void) } thread_unlock(); - current-state = TASK_INTERRUPTIBLE; - schedule_timeout(HZ/8); /* Propagate thread-control */ + schedule_timeout_interruptible(msecs_to_jiffies(125)); /* Propagate thread-control */ pktgen_wait_all_threads_run(); } diff -urpN 2.6.13-rc5-mm1/net/dccp/proto.c 2.6.13-rc5-mm1-dev/net/dccp/proto.c --- 2.6.13-rc5-mm1/net/dccp/proto.c 2005-08-07 10:05:22.0 -0700 +++ 2.6.13-rc5-mm1-dev/net/dccp/proto.c 2005-08-10 16:10:55.0 -0700 @@ -225,7 +225,7 @@ int dccp_sendmsg(struct kiocb *iocb, str if (delay timeo) goto out_discard; release_sock(sk); - delay = schedule_timeout(delay); + delay = schedule_timeout_interruptible(delay); lock_sock(sk); timeo -= delay; if (signal_pending(current)) diff -urpN 2.6.13-rc5-mm1/net/ipv4/ipconfig.c 2.6.13-rc5-mm1-dev/net/ipv4/ipconfig.c --- 2.6.13-rc5-mm1/net/ipv4/ipconfig.c 2005-08-07 10:05:22.0 -0700 +++ 2.6.13-rc5-mm1-dev/net/ipv4/ipconfig.c 2005-08-10 15:26:57.0 -0700 @@ -1102,10 +1102,9 @@ static int __init ic_dynamic(void) #endif jiff = jiffies + (d-next ? CONF_INTER_TIMEOUT : timeout); - while (time_before(jiffies, jiff) !ic_got_reply) { + while (time_before(jiffies, jiff) !ic_got_reply) set_current_state(TASK_UNINTERRUPTIBLE); - schedule_timeout(1); - } + schedule_timeout_uninterruptible(1); #ifdef IPCONFIG_DHCP /* DHCP isn't done until we get a DHCPACK. */ if ((ic_got_reply IC_BOOTP) diff -urpN 2.6.13-rc5-mm1/net/irda/ircomm/ircomm_tty.c 2.6.13-rc5-mm1-dev/net/irda/ircomm/ircomm_tty.c --- 2.6.13-rc5-mm1/net/irda/ircomm/ircomm_tty.c 2005-08-07 09:57:38.0 -0700 +++ 2.6.13-rc5-mm1-dev/net/irda/ircomm/ircomm_tty.c 2005-08-10 15:27:13.0 -0700 @@ -567,10 +567,8 @@ static void ircomm_tty_close(struct tty_ self-tty = NULL; if (self-blocked_open) { - if (self-close_delay) { - current-state = TASK_INTERRUPTIBLE; - schedule_timeout(self-close_delay); - } + if (self-close_delay) + schedule_timeout_interruptible(self-close_delay); wake_up_interruptible(self-open_wait); } @@ -863,8 +861,7 @@ static void ircomm_tty_wait_until_sent(s spin_lock_irqsave(self-spinlock, flags); while (self-tx_skb self-tx_skb-len) { spin_unlock_irqrestore(self-spinlock, flags); - current-state = TASK_INTERRUPTIBLE; - schedule_timeout(poll_time
[-mm PATCH 20/32] drivers/net: fix-up schedule_timeout() usage
Description: Use schedule_timeout_interruptible() instead of set_current_state()/schedule_timeout() to reduce kernel size. Signed-off-by: Nishanth Aravamudan [EMAIL PROTECTED] --- drivers/net/8139cp.c |3 - drivers/net/hp100.c | 48 ++ drivers/net/irda/stir4200.c |7 +--- drivers/net/ixgb/ixgb_ethtool.c |7 +--- drivers/net/ns83820.c |3 - drivers/net/tokenring/ibmtr.c |9 ++--- drivers/net/tokenring/olympic.c |2 - drivers/net/tokenring/tms380tr.c |3 - drivers/net/typhoon.c |7 +--- drivers/net/wan/cosa.c|6 +-- drivers/net/wan/cycx_drv.c|3 - drivers/net/wan/dscc4.c |9 + drivers/net/wan/farsync.c |3 - drivers/net/wireless/ipw2100.c| 17 +++--- drivers/net/wireless/prism54/islpci_dev.c |6 +-- drivers/net/wireless/prism54/islpci_mgt.c |5 +-- include/linux/ibmtr.h |4 +- include/linux/netdevice.h |6 +-- 18 files changed, 54 insertions(+), 94 deletions(-) diff -urpN 2.6.13-rc5-mm1/drivers/net/8139cp.c 2.6.13-rc5-mm1-dev/drivers/net/8139cp.c --- 2.6.13-rc5-mm1/drivers/net/8139cp.c 2005-08-07 09:58:00.0 -0700 +++ 2.6.13-rc5-mm1-dev/drivers/net/8139cp.c 2005-08-08 15:54:06.0 -0700 @@ -1029,8 +1029,7 @@ static void cp_reset_hw (struct cp_priva if (!(cpr8(Cmd) CmdReset)) return; - set_current_state(TASK_UNINTERRUPTIBLE); - schedule_timeout(10); + schedule_timeout_uninterruptible(10); } printk(KERN_ERR %s: hardware reset timeout\n, cp-dev-name); diff -urpN 2.6.13-rc5-mm1/drivers/net/hp100.c 2.6.13-rc5-mm1-dev/drivers/net/hp100.c --- 2.6.13-rc5-mm1/drivers/net/hp100.c 2005-08-07 09:58:01.0 -0700 +++ 2.6.13-rc5-mm1-dev/drivers/net/hp100.c 2005-08-08 15:55:41.0 -0700 @@ -2517,10 +2517,8 @@ static int hp100_down_vg_link(struct net do { if (hp100_inb(VG_LAN_CFG_1) HP100_LINK_CABLE_ST) break; - if (!in_interrupt()) { - set_current_state(TASK_INTERRUPTIBLE); - schedule_timeout(1); - } + if (!in_interrupt()) + schedule_timeout_interruptible(1); } while (time_after(time, jiffies)); if (time_after_eq(jiffies, time)) /* no signal-no logout */ @@ -2536,10 +2534,8 @@ static int hp100_down_vg_link(struct net do { if (!(hp100_inb(VG_LAN_CFG_1) HP100_LINK_UP_ST)) break; - if (!in_interrupt()) { - set_current_state(TASK_INTERRUPTIBLE); - schedule_timeout(1); - } + if (!in_interrupt()) + schedule_timeout_interruptible(1); } while (time_after(time, jiffies)); #ifdef HP100_DEBUG @@ -2577,10 +2573,8 @@ static int hp100_down_vg_link(struct net do { if (!(hp100_inb(MAC_CFG_4) HP100_MAC_SEL_ST)) break; - if (!in_interrupt()) { - set_current_state(TASK_INTERRUPTIBLE); - schedule_timeout(1); - } + if (!in_interrupt()) + schedule_timeout_interruptible(1); } while (time_after(time, jiffies)); hp100_orb(HP100_AUTO_MODE, MAC_CFG_3); /* Autosel back on */ @@ -2591,10 +2585,8 @@ static int hp100_down_vg_link(struct net do { if ((hp100_inb(VG_LAN_CFG_1) HP100_LINK_CABLE_ST) == 0) break; - if (!in_interrupt()) { - set_current_state(TASK_INTERRUPTIBLE); - schedule_timeout(1); - } + if (!in_interrupt()) + schedule_timeout_interruptible(1); } while (time_after(time, jiffies)); if (time_before_eq(time, jiffies)) { @@ -2606,10 +2598,8 @@ static int hp100_down_vg_link(struct net time = jiffies + (2 * HZ); /* This seems to take a while */ do { - if (!in_interrupt()) { - set_current_state(TASK_INTERRUPTIBLE); - schedule_timeout(1); - } + if (!in_interrupt()) + schedule_timeout_interruptible(1); } while (time_after(time, jiffies)); return 0; @@ -2659,10 +2649,8 @@ static int hp100_login_to_vg_hub(struct do { if (~(hp100_inb(VG_LAN_CFG_1) HP100_LINK_UP_ST
[PATCH] net/sunrpc: fix time conversion error
On 01.08.2005 [15:11:48 -0600], Josip Loncaric wrote: Line 589 of linux-2.6.11.10/net/sunrpc/svcsock.c is obviously wrong: skb-stamp.tv_usec = xtime.tv_nsec * 1000; To convert nsec to usec, one should divide instead of multiplying: skb-stamp.tv_usec = xtime.tv_nsec / 1000; The same bug could be present in the latest kernels, although I haven't checked. This bug makes svc_udp_recvfrom() timestamps incorrect. Agreed, the conversion is wrong. I think the code is buggy period, as it accesses xtime without grabbing the xtime_lock first. Following patch should fix both issues. Description: This function incorrectly multiplies a nanosecond value by 1000, instead of dividing by 1000, to obtain a corresponding microsecond value. Fix the math. Also, the function incorrectly accesses xtime without using the xtime_lock. Fixed as well. Patch is compile-tested. Signed-off-by: Nishanth Aravamudan [EMAIL PROTECTED] --- svcsock.c |8 ++-- 1 files changed, 6 insertions(+), 2 deletions(-) --- 2.6.13-rc4/net/sunrpc/svcsock.c 2005-07-29 14:11:50.0 -0700 +++ 2.6.13-rc4-dev/net/sunrpc/svcsock.c 2005-08-01 15:51:11.0 -0700 @@ -559,6 +559,7 @@ svc_udp_recvfrom(struct svc_rqst *rqstp) struct svc_serv *serv = svsk-sk_server; struct sk_buff *skb; int err, len; +unsigned long seq; if (test_and_clear_bit(SK_CHNGBUF, svsk-sk_flags)) /* udp sockets need large rcvbuf as all pending @@ -585,8 +586,11 @@ svc_udp_recvfrom(struct svc_rqst *rqstp) dprintk(svc: recvfrom returned error %d\n, -err); } if (skb-stamp.tv_sec == 0) { - skb-stamp.tv_sec = xtime.tv_sec; - skb-stamp.tv_usec = xtime.tv_nsec * 1000; + do { + seq = read_seqbegin(xtime_lock); + skb-stamp.tv_sec = xtime.tv_sec; + skb-stamp.tv_usec = xtime.tv_nsec / 1000; + } while (read_seqretry(xtime_lock, seq)); /* Don't enable netstamp, sunrpc doesn't need that much accuracy */ } - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html