Re: [patch] Useless locking in mm/numa.c
From: Matthew Dobson <hidden>
Date: 2002-07-19 20:30:58
Kanoj Sarcar wrote:
I think I put in the locks in the initial version of the file becase the idea was that show_free_areas_node() could be invoked from any cpu in a multinode system (via the sysrq keys or other intr sources), and the spin lock would provide sanity in the print out.
As Bill mentioned, a grep through the source shows that show_free_areas_node() is never called, and since it boils down to *just* a call to show_free_areas_core() w/out the locking, the revised patch pulls it out entirely.
For nonnuma discontig machines, isn't the spin lock providing protection in the pgdat list chain walking in _alloc_pages()?
Uhh... kinda? Since *next is static, it means that at best case, 2 processes walking the pgdat_list chain will hip-hop over nodes... If it is racy code, the *best* that lock is currently doing is making it mildy less racy, and at worst, hiding the fact that there is a race there. I'm sure the lock was useful at some point, but it no longer is... Attatched is the new version, please apply.. Cheers! -Matt
quoted hunk ↗ jump to hunk
Kanoj--- Matthew Dobson <colpatch@us.ibm.com> wrote:quoted
There is a lock that is apparently protecting nothing. The node_lock spinlock in mm/numa.c is protecting read-only accesses to pgdat_list. Here is a patch to get rid of it. Cheers! -Mattquoted
--- linux-2.5.26-vanilla/mm/numa.c Tue Jul 1616:49:30 2002+++ linux-2.5.26-vanilla/mm/numa.c.fixed Thu Jul 1817:59:35 2002@@ -44,15 +44,11 @@#define LONG_ALIGN(x) (((x)+(sizeof(long))-1)&~((sizeof(long))-1)) -static spinlock_t node_lock = SPIN_LOCK_UNLOCKED; - void show_free_areas_node(pg_data_t *pgdat) { unsigned long flags; - spin_lock_irqsave(&node_lock, flags); show_free_areas_core(pgdat); - spin_unlock_irqrestore(&node_lock, flags); } /*@@ -106,11 +102,9 @@#ifdef CONFIG_NUMA temp = NODE_DATA(numa_node_id()); #else - spin_lock_irqsave(&node_lock, flags); if (!next) next = pgdat_list; temp = next; next = next->node_next; - spin_unlock_irqrestore(&node_lock, flags); #endif start = temp; while (temp) {__________________________________________________ Do You Yahoo!? Yahoo! Autos - Get free new car price quotes http://autos.yahoo.com
Attachments
- node_lock.patch [text/plain] 843 bytes · preview