Messages in this thread | | | Date | Wed, 3 Feb 2010 21:12:46 +0100 | Subject | Re: [RFC][PATCH] vmscan: balance local_irq_disable() and local_irq_enable() | From | John Kacur <> |
| |
On Wed, Feb 3, 2010 at 9:09 PM, Steven Rostedt <rostedt@goodmis.org> wrote: > t On Wed, 2010-02-03 at 20:53 +0100, John Kacur wrote: >> Balance local_irq_disable() and local_irq_enable() as well as >> spin_lock_irq() and spin_lock_unlock_irq >> >> Signed-off-by: John Kacur <jkacur@redhat.com> >> --- >> mm/vmscan.c | 3 ++- >> 1 files changed, 2 insertions(+), 1 deletions(-) >> >> diff --git a/mm/vmscan.c b/mm/vmscan.c >> index c26986c..b895025 100644 >> --- a/mm/vmscan.c >> +++ b/mm/vmscan.c >> @@ -1200,8 +1200,9 @@ static unsigned long shrink_inactive_list(unsigned long max_scan, >> if (current_is_kswapd()) >> __count_vm_events(KSWAPD_STEAL, nr_freed); >> __count_zone_vm_events(PGSTEAL, zone, nr_freed); >> + local_irq_enable(); >> >> - spin_lock(&zone->lru_lock); >> + spin_lock_irq(&zone->lru_lock); >> /* >> * Put back any unfreeable pages. >> */ > > > The above looks wrong. I don't know the code, but just by looking at > where the locking and interrupts are, I can take a guess. > > Lets add a little more of the code: > > local_irq_disable(); > if (current_is_kswapd()) > __count_vm_events(KSWAPD_STEAL, nr_freed); > __count_zone_vm_events(PGSTEAL, zone, nr_freed); > > spin_lock(&zone->lru_lock); > /* > > I'm guessing the __count_zone_vm_events and friends need interrupts > disabled here, probably due to per cpu stuff. But if you enable > interrupts before the spin_lock() you may let an interrupt come in and > invalidate what was done above it. > > So no, I do not think enabling interrupts here is a good thing. >
okay, and since we have already done local_irq_disable(), then that is why we only need the spin_lock() and not the spin_lock_irq() flavour? -- 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/
| |