Change synchronize_kernel to _rcu and _sched
Subsystems (9)
RFC PATCH 3/4 Change synchronize_kernel to _rcu and _sched
This patch changes calls to synchronize_kernel(), deprecated in the earlier "Deprecate synchronize_kernel, GPL replacement" patch to instead call the new synchronize_rcu() and synchronize_sched() APIs.
---
arch/i386/oprofile/nmi_timer_int.c | 2 +- arch/ppc64/kernel/HvLpEvent.c | 2 +- arch/x86_64/kernel/mce.c | 2 +- drivers/acpi/processor_idle.c | 2 +- drivers/char/ipmi/ipmi_si_intf.c | 4 ++-- drivers/input/keyboard/atkbd.c | 2 +- drivers/md/multipath.c | 2 +- drivers/md/raid1.c | 2 +- drivers/md/raid10.c | 2 +- drivers/md/raid5.c | 2 +- drivers/md/raid6main.c | 2 +- drivers/net/r8169.c | 2 +- drivers/s390/cio/airq.c | 4 ++-- kernel/module.c | 2 +- kernel/profile.c | 2 +- mm/slab.c | 2 +- net/core/dev.c | 2 +- 17 files changed, 19 insertions(+), 19 deletions(-)
arch/i386/oprofile/nmi_timer_int.c +1 -1
@@ -36,7 +36,7 @@ static void timer_stop(void) {enable_timer_nmi_watchdog ();unset_nmi_callback (); -synchronize_kernel (); +synchronize_sched ();/* Allow already-started NMIs to complete. */ }
arch/ppc64/kernel/HvLpEvent.c +1 -1
@@ -45,7 +45,7 @@ int HvLpEvent_unregisterHandler( HvLpEve/* We now sleep until all other CPUs have scheduled. This ensures that * the deletion is seen by all other CPUs, and that the deleted handler * isn't still running on another CPU when we return. */ -synchronize_kernel (); +synchronize_rcu (); } }return rc;
arch/x86_64/kernel/mce.c +1 -1
@@ -392,7 +392,7 @@ static ssize_t mce_read(struct file *filmemset (mcelog.entry,0 , next *sizeof (struct mce )); mcelog.next =0 ; -synchronize_kernel (); +synchronize_sched ();/* Allow all already-started MCEs to complete. */ /* Collect entries that were still getting written before the synchronize. */
drivers/acpi/processor_idle.c +1 -1
@@ -838,7 +838,7 @@ int acpi_processor_cst_has_changed (stru/* Fall back to the default idle loop */ pm_idle = pm_idle_save; -synchronize_kernel (); +synchronize_sched ();/* Relies on interrupts forcing exit from idle. */ pr->flags.power =0 ; result =acpi_processor_get_power_info (pr);
drivers/char/ipmi/ipmi_si_intf.c +2 -2
@@ -2194,7 +2194,7 @@ static int init_one_smi(int intf_num, st/* Wait until we know that we are out of any interrupt handlers might have been running before we freed the interrupt. */ -synchronize_kernel (); +synchronize_sched ();if (new_smi->si_sm) {if (new_smi->handlers)
@@ -2307,7 +2307,7 @@ static void __exit cleanup_one_si(struct/* Wait until we know that we are out of any interrupt handlers might have been running before we freed the interrupt. */ -synchronize_kernel (); +synchronize_sched ();/* Wait for the timer to stop. This avoids problems with race conditions removing the timer here. */
drivers/input/keyboard/atkbd.c +1 -1
@@ -678,7 +678,7 @@ static void atkbd_disconnect(struct seriatkbd_disable (atkbd);/* make sure we don't have a command in flight */ -synchronize_kernel (); +synchronize_sched ();/* Allow atkbd_interrupt()s to complete. */ flush_scheduled_work ();device_remove_file (&serio->dev, &atkbd_attr_extra);
drivers/md/multipath.c +1 -1
@@ -355,7 +355,7 @@ static int multipath_remove_disk(mddev_t goto abort; } p->rdev =NULL ; -synchronize_kernel (); +synchronize_rcu ();if (atomic_read (&rdev->nr_pending)) {/* lost the race, try later */ err = -EBUSY ;
drivers/md/raid1.c +1 -1
@@ -797,7 +797,7 @@ static int raid1_remove_disk(mddev_t *md goto abort; } p->rdev =NULL ; -synchronize_kernel (); +synchronize_rcu ();if (atomic_read (&rdev->nr_pending)) {/* lost the race, try later */ err = -EBUSY ;
drivers/md/raid10.c +1 -1
@@ -977,7 +977,7 @@ static int raid10_remove_disk(mddev_t *m goto abort; } p->rdev =NULL ; -synchronize_kernel (); +synchronize_rcu ();if (atomic_read (&rdev->nr_pending)) {/* lost the race, try later */ err = -EBUSY ;
drivers/md/raid5.c +1 -1
@@ -1873,7 +1873,7 @@ static int raid5_remove_disk(mddev_t *md goto abort; } p->rdev =NULL ; -synchronize_kernel (); +synchronize_rcu ();if (atomic_read (&rdev->nr_pending)) {/* lost the race, try later */ err = -EBUSY ;
drivers/md/raid6main.c +1 -1
@@ -2038,7 +2038,7 @@ static int raid6_remove_disk(mddev_t *md goto abort; } p->rdev =NULL ; -synchronize_kernel (); +synchronize_rcu ();if (atomic_read (&rdev->nr_pending)) {/* lost the race, try later */ err = -EBUSY ;
drivers/net/r8169.c +1 -1
@@ -2385,7 +2385,7 @@ core_down: }/* Give a racing hard_start_xmit a few cycles to complete. */ -synchronize_kernel (); +synchronize_sched ();/* FIXME: should this be synchronize_irq()? */ /* * And now for the 50k$ question: are IRQ disabled or not ?
drivers/s390/cio/airq.c +2 -2
@@ -45,7 +45,7 @@ s390_register_adapter_interrupt (adapterelse ret = (cmpxchg (&adapter_handler,NULL , handler) ? -EBUSY :0 );if (!ret) -synchronize_kernel (); +synchronize_sched ();/* Allow interrupts to complete. */ sprintf (dbf_txt,"ret:%d" , ret);CIO_TRACE_EVENT (4 , dbf_txt);
@@ -65,7 +65,7 @@ s390_unregister_adapter_interrupt (adapt ret = -EINVAL ; else { adapter_handler =NULL ; -synchronize_kernel (); +synchronize_sched ();/* Allow interrupts to complete. */ ret =0 ; }sprintf (dbf_txt,"ret:%d" , ret);
kernel/module.c +1 -1
@@ -1801,7 +1801,7 @@ sys_init_module(void __user *umod,/* Init routine failed: abort. Try to protect us from buggy refcounters. */ mod->state =MODULE_STATE_GOING ; -synchronize_kernel (); +synchronize_sched ();if (mod->unsafe)printk (KERN_ERR "%s: module is now stuck!\n" , mod->name);
kernel/profile.c +1 -1
@@ -184,7 +184,7 @@ void unregister_timer_hook(int (*hook)(sWARN_ON (hook != timer_hook); timer_hook =NULL ;/* make sure all CPUs see the NULL hook */ -synchronize_kernel (); +synchronize_sched ();/* Allow ongoing interrupts to complete. */ }EXPORT_SYMBOL_GPL (register_timer_hook);
mm/slab.c +1 -1
@@ -1656,7 +1656,7 @@ int kmem_cache_destroy (kmem_cache_t * c }if (unlikely (cachep->flags &SLAB_DESTROY_BY_RCU )) -synchronize_kernel (); +synchronize_rcu ();/* no cpu_online check required here since we clear the percpu * array on cpu offline and set this to NULL.
net/core/dev.c +1 -1
@@ -3077,7 +3077,7 @@ void free_netdev(struct net_device *dev)void synchronize_net (void ) {might_sleep (); -synchronize_kernel (); +synchronize_rcu (); }/**
Paul E. McKenney <[email protected]> :
Quoted 3 lines
This patch changes calls to
synchronize_kernel(), deprecated in the earlier "Deprecatesynchronize_kernel, GPL replacement" patch to instead call the newsynchronize_rcu()andsynchronize_sched()APIs.
[...]
drivers/net/r8169.c core_down: · line 2391
Quoted 12 lines
diff -urpN -X dontdiff
linux-2.6.12-rc1/drivers/net/r8169.clinux-2.6.12-rc1-bettersk/drivers/net/r8169.cdrivers/net/r8169.c +1
-1@@ -2385,7 +2385,7 @@ core_down: }/* Give a racing hard_start_xmit a few cycles to complete. */ -synchronize_kernel (); +synchronize_sched ();/* FIXME: should this be synchronize_irq()? */ /* * And now for the 50k$ question: are IRQ disabled or not ?
(answering the FIXME)
The race with the irq is handled somewhere else. As the comment suggests, this part is racing with the hard_start_xmit() handler. If I read correctly net/core/dev.c::dev_queue_xmit, the code above simply needs the new synchronize_rcu().
--
Ueimor
Quoted 20 lines · Francois Romieu
Paul E. McKenney <[email protected]> :
Quoted 3 lines
This patch changes calls to
synchronize_kernel(), deprecated in the earlier "Deprecatesynchronize_kernel, GPL replacement" patch to instead call the newsynchronize_rcu()andsynchronize_sched()APIs.[...]
drivers/net/r8169.c core_down: · line 2391
Quoted 12 lines
diff -urpN -X dontdiff
linux-2.6.12-rc1/drivers/net/r8169.clinux-2.6.12-rc1-bettersk/drivers/net/r8169.cdrivers/net/r8169.c +1
-1@@ -2385,7 +2385,7 @@ core_down: }/* Give a racing hard_start_xmit a few cycles to complete. */ -synchronize_kernel (); +synchronize_sched ();/* FIXME: should this be synchronize_irq()? */ /* * And now for the 50k$ question: are IRQ disabled or not ? (answering the FIXME)
Quoted 4 lines
The race with the irq is handled somewhere else. As the comment suggests, this part is racing with the
hard_start_xmit()handler. If I read correctly net/core/dev.c::dev_queue_xmit, the code above simply needs the newsynchronize_rcu().
That would be good! In your reading of the code, did you verify that all instances of calls to hard_start_xmit() are in fact under either rcu_read_lock() or rcu_read_lock_bh()?
Thanx, Paul
[Jeff added to cc list - it's a network driver question]
Quoted 3 lines · Paul E. McKenney
Quoted 3 lines
drivers/net/r8169.c core_down: · line 2388
Quoted 3 lines
drivers/net/r8169.c +0
-1/* Give a racing hard_start_xmit a few cycles to complete. */ -synchronize_kernel ();
I haven't read the whole driver, but what about
spin_unlock_wait(&dev->xmit_lock);
?hard_start_xmit is called under dev->xmit_lock, waiting until the lock is free would guarantee that all running instances of hard_start_xmit have completed.
Jeff: Is that still correct?
--
Manfred
Manfred Spraul <[email protected]> :
Quoted 1 line
[Jeff added to cc list - it's a network driver question]
[...]
Quoted 3 lines
I haven't read the whole driver, but what about
spin_unlock_wait(&dev->xmit_lock);?
The race here is a dev->close() against dev->hard_start_xmit() one where dev->hard_start_xmit() does not do any locking at all.
(so far, so good...)
--
Ueimor
Quoted 10 lines · Francois Romieu
Manfred Spraul <[email protected]> :
Quoted 1 line
[Jeff added to cc list - it's a network driver question]
[...]
Quoted 3 lines
I haven't read the whole driver, but what about
spin_unlock_wait(&dev->xmit_lock);?
The race here is a
dev->close()againstdev->hard_start_xmit()one wheredev->hard_start_xmit()does not do any locking at all.
I always thought that all callers of dev->hard_start_xmit() acquire dev->xmit_lock before calling hard_start_xmit().
Is that assumption wrong? I think I even rely on that in one of my drivers.
--
Manfred
Manfred Spraul <[email protected]> :
[...]
Quoted 5 lines
I always thought that all callers of
dev->hard_start_xmit()acquiredev->xmit_lockbefore callinghard_start_xmit().Is that assumption wrong? I think I even rely on that in one of my drivers.
Afaik, no, it is right.
This part of the r8169 driver handles the window starting where qdisc_run() tests netif_queue_stopped() to the spin_trylock(&dev->xmit_lock) inqdisc_restart: the lock is not taken and dev->hard_start_xmit() will happen in the near future ("near" or "preempt safe near" is not relevant here). The driver must wait for the late newcomer to do its whole work before it can release the Tx ring, whence the former synchronize_kernel().
r8169_down() looks a bit baroque because it can race with the irq handler or dev->poll() or the recovery timer and none of those spin_locks either.
I may have missed something though. Feel free to educate.
--
Ueimor