Replace down_trylock() with down_try(), reverse return values.
Subsystems (20)
PATCH 1/1 Replace down_trylock() with down_try(), reverse return values.
down_trylock() returns 1 on failure, 0 on success. This differs from spin_trylock(), mutex_trylock() and common sense. Or as ocfs2 put it "kernel 1, world 0".
Rename it to down_try() (which makes more sense anyway), and reverse it. Fortunately there aren't a huge number of callers left.
I took the liberty of reversing the sense of usb_trylock_device() without renaming it: it's only used in one place anyway.
Documentation/DocBook/kernel-locking.tmpl +1 -1
@@ -1906,7 +1906,7 @@ machines due to caching. <function>down()</function> </para> <para> - There is a <function>down_trylock()</function> which can be + There is a <function>down_try()</function> which can be used inside interrupt context, as it will not sleep. <function>up()</function> will also never sleep. </para>
arch/ia64/kernel/salinfo.c +2 -2
@@ -192,7 +192,7 @@ static voidstatic void salinfo_work_to_do (struct salinfo_data *data) { -down_trylock (&data->mutex); +down_try (&data->mutex);up (&data->mutex); }
@@ -309,7 +309,7 @@ salinfo_event_read(struct file *file, chint i, n, cpu =-1 ; retry: -if (cpus_empty (data->cpu_event) &&down_trylock (&data->mutex)) { +if (cpus_empty (data->cpu_event) && !down_try (&data->mutex)) {if (file->f_flags &O_NONBLOCK )return -EAGAIN ;if (down_interruptible (&data->mutex))
drivers/char/snsc.c +2 -2
@@ -161,7 +161,7 @@ scdrv_read(struct file *file, char __usestruct subch_data_s *sd = (struct subch_data_s *) file->private_data;/* try to get control of the read buffer */ -if (down_trylock (&sd->sd_rbs)) { +if (!down_try (&sd->sd_rbs)) {/* somebody else has it now; * if we're non-blocking, then exit... */
@@ -253,7 +253,7 @@ scdrv_write(struct file *file, const chastruct subch_data_s *sd = (struct subch_data_s *) file->private_data;/* try to get control of the write buffer */ -if (down_trylock (&sd->sd_wbs)) { +if (!down_try (&sd->sd_wbs)) {/* somebody else has it now; * if we're non-blocking, then exit... */
drivers/char/viotape.c +2 -2
@@ -361,7 +361,7 @@ static ssize_t viotap_write(struct file* semaphore */ if (noblock) { -if (down_trylock (&reqSem)) { +if (!down_try (&reqSem)) { ret = -EWOULDBLOCK ; goto free_op; }
@@ -451,7 +451,7 @@ static ssize_t viotap_read(struct file ** semaphore */ if (noblock) { -if (down_trylock (&reqSem)) { +if (!down_try (&reqSem)) { ret = -EWOULDBLOCK ; goto free_op; }
drivers/infiniband/core/user_mad.c +1 -1
@@ -890,7 +890,7 @@ static int ib_umad_sm_open(struct inodereturn -ENXIO ;if (filp->f_flags &O_NONBLOCK ) { -if (down_trylock (&port->sm_sem)) { +if (!down_try (&port->sm_sem)) { ret = -EAGAIN ; goto fail; }
drivers/input/serio/hil_mlc.c +2 -2
@@ -607,7 +607,7 @@ static inline void hilse_setup_input(hildo_gettimeofday (&(mlc->instart)); mlc->icount =15 ;memset (mlc->ipacket,0 ,16 *sizeof (hil_packet)); -BUG_ON (down_trylock (&mlc->isem)); +BUG_ON (!down_try (&mlc->isem)); }#ifdef HIL_MLC_DEBUG
@@ -694,7 +694,7 @@ static int hilse_donode(hil_mlc *mlc) out2:write_unlock_irqrestore (&mlc->lock, flags); -if (down_trylock (&mlc->osem)) { +if (!down_try (&mlc->osem)) { nextidx =HILSEN_DOZE ;break ; }
drivers/input/serio/hp_sdc_mlc.c +7 -7
@@ -148,7 +148,7 @@ static int hp_sdc_mlc_in(hil_mlc *mlc, s priv = mlc->priv;/* Try to down the semaphore */ -if (down_trylock (&mlc->isem)) { +if (!down_try (&mlc->isem)) {struct timeval tv;if (priv->emtestmode) { mlc->ipacket[0] =
@@ -186,13 +186,13 @@ static int hp_sdc_mlc_cts(hil_mlc *mlc) priv = mlc->priv;/* Try to down the semaphores -- they should be up. */ -BUG_ON (down_trylock (&mlc->isem)); -BUG_ON (down_trylock (&mlc->osem)); +BUG_ON (!down_try (&mlc->isem)); +BUG_ON (!down_try (&mlc->osem));up (&mlc->isem);up (&mlc->osem); -if (down_trylock (&mlc->csem)) { +if (!down_try (&mlc->csem)) {if (priv->trans.act.semaphore != &mlc->csem) goto poll;else
@@ -229,7 +229,7 @@ static void hp_sdc_mlc_out(hil_mlc *mlc) priv = mlc->priv;/* Try to down the semaphore -- it should be up. */ -BUG_ON (down_trylock (&mlc->osem)); +BUG_ON (!down_try (&mlc->osem));if (mlc->opacket &HIL_DO_ALTER_CTRL ) goto do_control;
@@ -240,7 +240,7 @@ static void hp_sdc_mlc_out(hil_mlc *mlc)return ; }/* Shouldn't be sending commands when loop may be busy */ -BUG_ON (down_trylock (&mlc->csem)); +BUG_ON (!down_try (&mlc->csem));up (&mlc->csem); priv->trans.actidx =0 ;
@@ -296,7 +296,7 @@ static void hp_sdc_mlc_out(hil_mlc *mlc) priv->tseq[3 ] =0 ;if (mlc->opacket &HIL_CTRL_APE ) { priv->tseq[3 ] |=HP_SDC_LPC_APE_IPF ; -down_trylock (&mlc->csem); +down_try (&mlc->csem); } enqueue:hp_sdc_enqueue_transaction (&priv->trans);
drivers/md/dm-raid1.c +1 -1
@@ -587,7 +587,7 @@ static void rh_recovery_prepare(struct r/* Extra reference to avoid race with rh_stop_recovery */ atomic_inc (&rh->recovery_in_flight); -while (!down_trylock (&rh->recovery_count)) { +while (down_try (&rh->recovery_count)) {atomic_inc (&rh->recovery_in_flight);if (__rh_recovery_prepare (rh) <=0 ) {atomic_dec (&rh->recovery_in_flight);
drivers/net/3c527.c +1 -1
@@ -577,7 +577,7 @@ static int mc32_command_nowait(struct neint ioaddr = dev->base_addr;int ret =-1 ; -if (down_trylock (&lp->cmd_mutex) ==0 ) +if (down_try (&lp->cmd_mutex)) { lp->cmd_nonblocking=1 ; lp->exec_box->mbox=0 ;
drivers/net/irda/sir_dev.c +0 -0
@@ -286,7 +286,7 @@ int sirdev_schedule_request(struct sir_d IRDA_DEBUG(2, "%s - state=0x%04x / param=%u\n", __FUNCTION__, initial_state,
param);
- if (down_trylock(&fsm->sem)) {
+ if (!down_try(&fsm->sem)) {
if (in_interrupt() || in_atomic() || irqs_disabled()) {
IRDA_DEBUG(1, "%s(), state machine busy!\n", __FUNCTION__); return -EWOULDBLOCK;
drivers/net/ps3_gelic_wireless.c +1 -1
@@ -2151,7 +2151,7 @@ static void gelic_wl_disconnect_event(st* As it waits with timeout, just leave assoc_done * uncompleted, then it terminates with timeout */ -if (down_trylock (&wl->assoc_stat_lock)) { +if (!down_try (&wl->assoc_stat_lock)) {pr_debug ("%s: already locked\n" , __func__); lock =0 ; }else {
drivers/net/wireless/airo.c +6 -6
@@ -2138,7 +2138,7 @@ static int airo_start_xmit(struct sk_buf fids[i] |= (len <<16 ); priv->xmit.skb = skb; priv->xmit.fid = i; -if (down_trylock (&priv->sem) !=0 ) { +if (!down_try (&priv->sem)) {set_bit (FLAG_PENDING_XMIT , &priv->flags);netif_stop_queue (dev);set_bit (JOB_XMIT , &priv->jobs);
@@ -2209,7 +2209,7 @@ static int airo_start_xmit11(struct sk_b fids[i] |= (len <<16 ); priv->xmit11.skb = skb; priv->xmit11.fid = i; -if (down_trylock (&priv->sem) !=0 ) { +if (!down_try (&priv->sem)) {set_bit (FLAG_PENDING_XMIT11 , &priv->flags);netif_stop_queue (dev);set_bit (JOB_XMIT11 , &priv->jobs);
@@ -2257,7 +2257,7 @@ static struct net_device_stats *airo_getif (!test_bit (JOB_STATS , &local->jobs)) {/* Get stats out of the card if available */ -if (down_trylock (&local->sem) !=0 ) { +if (!down_try (&local->sem)) {set_bit (JOB_STATS , &local->jobs);wake_up_interruptible (&local->thr_wait); }else
@@ -2284,7 +2284,7 @@ static void airo_set_multicast_list(struif ((dev->flags ^ ai->flags) &IFF_PROMISC ) {change_bit (FLAG_PROMISC , &ai->flags); -if (down_trylock (&ai->sem) !=0 ) { +if (!down_try (&ai->sem)) {set_bit (JOB_PROMISC , &ai->jobs);wake_up_interruptible (&ai->thr_wait); }else
@@ -3211,7 +3211,7 @@ static irqreturn_t airo_interrupt(int irset_bit (FLAG_UPDATE_UNI , &apriv->flags);set_bit (FLAG_UPDATE_MULTI , &apriv->flags); -if (down_trylock (&apriv->sem) !=0 ) { +if (!down_try (&apriv->sem)) {set_bit (JOB_EVENT , &apriv->jobs);wake_up_interruptible (&apriv->thr_wait); }else
@@ -7658,7 +7658,7 @@ static struct iw_statistics *airo_get_wiif (!test_bit (JOB_WSTATS , &local->jobs)) {/* Get stats out of the card if available */ -if (down_trylock (&local->sem) !=0 ) { +if (!down_try (&local->sem)) {set_bit (JOB_WSTATS , &local->jobs);wake_up_interruptible (&local->thr_wait); }else
drivers/scsi/aacraid/commsup.c +1 -1
@@ -490,7 +490,7 @@ int aac_fib_send(u16 command, struct fib* hardware failure has occurred. */ unsigned long count =36000000L ;/* 3 minutes */ -while (down_trylock (&fibptr->event_wait)) { +while (!down_try (&fibptr->event_wait)) {int blink;if (--count ==0 ) {struct aac_queue * q = &dev->queues->queue[AdapNormCmdQueue];
drivers/usb/core/usb.c +1 -1
@@ -477,7 +477,7 @@ int usb_lock_device_for_reset(struct usb } } -while (usb_trylock_device (udev) !=0 ) { +while (!usb_trylock_device (udev)) {/* If we can't acquire the lock after waiting one second, * we're probably deadlocked */
drivers/usb/gadget/inode.c +1 -1
@@ -298,7 +298,7 @@ get_ready_ep (unsigned f_flags, struct eint val;if (f_flags &O_NONBLOCK ) { -if (down_trylock (&epdata->lock) !=0 ) +if (!down_try (&epdata->lock)) goto nonblock;if (epdata->state !=STATE_EP_ENABLED ) {up (&epdata->lock);
drivers/watchdog/ar7_wdt.c +1 -1
@@ -179,7 +179,7 @@ static int ar7_wdt_open(struct inode *instatic int ar7_wdt_open (struct inode *inode,struct file *file) {/* only allow one at a time */ -if (down_trylock (&open_semaphore)) +if (!down_try (&open_semaphore))return -EBUSY ;ar7_wdt_enable_wdt (); expect_close =0 ;
drivers/watchdog/it8712f_wdt.c +1 -1
@@ -306,7 +306,7 @@ it8712f_wdt_open(struct inode *inode, st it8712f_wdt_open(struct inode *inode, struct file *file) {/* only allow one at a time */ -if (down_trylock (&it8712f_wdt_sem)) +if (!down_try (&it8712f_wdt_sem))return -EBUSY ;it8712f_wdt_enable ();
drivers/watchdog/s3c2410_wdt.c +1 -1
@@ -211,7 +211,7 @@ static int s3c2410wdt_set_heartbeat(intstatic int s3c2410wdt_open (struct inode *inode,struct file *file) { -if (down_trylock (&open_lock)) +if (!down_try (&open_lock))return -EBUSY ;if (nowayout)
drivers/watchdog/sc1200wdt.c +1 -1
@@ -151,7 +151,7 @@ static int sc1200wdt_open(struct inode *static int sc1200wdt_open (struct inode *inode,struct file *file) {/* allow one at a time */ -if (down_trylock (&open_sem)) +if (!down_try (&open_sem))return -EBUSY ;if (timeout >MAX_TIMEOUT )
drivers/watchdog/scx200_wdt.c +1 -1
@@ -92,7 +92,7 @@ static int scx200_wdt_open(struct inodestatic int scx200_wdt_open (struct inode *inode,struct file *file) {/* only allow one at a time */ -if (down_trylock (&open_semaphore)) +if (!down_try (&open_semaphore))return -EBUSY ;scx200_wdt_enable ();
drivers/watchdog/wdt_pci.c +1 -1
@@ -426,7 +426,7 @@ static int wdtpci_ioctl(struct inode *instatic int wdtpci_open (struct inode *inode,struct file *file) { -if (down_trylock (&open_sem)) +if (!down_try (&open_sem))return -EBUSY ;if (nowayout) {
fs/ocfs2/inode.c +0 -4
@@ -1062,10 +1062,6 @@ void ocfs2_clear_inode(struct inode *ino (unsigned long long)oi->ip_blkno);mutex_unlock (&oi->ip_io_mutex); -/* -* down_trylock() returns 0, down_write_trylock() returns 1 -* kernel 1, world 0 -*/ mlog_bug_on_msg (!down_write_trylock (&oi->ip_alloc_sem),"Clear inode of %llu, alloc_sem is locked\n" , (unsigned long long )oi->ip_blkno);
fs/reiserfs/journal.c +1 -1
@@ -1412,7 +1412,7 @@ static int flush_journal_list(struct sup/* if flushall == 0, the lock is already held */ if (flushall) {down (&journal->j_flush_sem); - }else if (!down_trylock (&journal->j_flush_sem)) { + }else if (down_try (&journal->j_flush_sem)) {BUG (); }
fs/xfs/linux-2.6/sema.h +3 -5
@@ -36,17 +36,15 @@ typedef struct semaphore sema_t;static inline int issemalocked (sema_t *sp) { -return down_trylock (sp) || (up (sp),0 ); +return !down_try (sp) || (up (sp),0 ); }/* -* Map cpsema (try to get the sema) to down_trylock. We need to switch -* the return values since cpsema returns 1 (acquired) 0 (failed) and -* down_trylock returns the reverse 0 (acquired) 1 (failed). +* Map cpsema (try to get the sema) to down_try. */ static inline int cpsema (sema_t *sp) { -return down_trylock (sp) ?0 :1 ; +return down_try (sp); }#endif /* __XFS_SUPPORT_SEMA_H__ */
fs/xfs/linux-2.6/xfs_buf.c +2 -2
@@ -530,7 +530,7 @@ found:* if this does not work then we need to drop the * spinlock and do a hard attempt on the semaphore. */ -if (down_trylock (&bp->b_sema)) { +if (!down_try (&bp->b_sema)) {if (!(flags &XBF_TRYLOCK )) {/* wait for buffer ownership */ XB_TRACE (bp,"get_lock" ,0 );
@@ -873,7 +873,7 @@ xfs_buf_cond_lock( {int locked; - locked =down_trylock (&bp->b_sema) ==0 ; + locked =down_try (&bp->b_sema);if (locked) {XB_SET_OWNER (bp); }
include/linux/mutex.h +0 -4
@@ -141,10 +141,6 @@ extern int __must_check mutex_lock_killa# define mutex_lock_killable_nested (lock, subclass) mutex_lock_killable(lock)#endif -/* -* NOTE: mutex_trylock() follows the spin_trylock() convention, -* not the down_trylock() convention! -*/ extern int mutex_trylock (struct mutex *lock);extern void mutex_unlock (struct mutex *lock);
include/linux/semaphore.h +1 -1
@@ -44,7 +44,7 @@ extern void down(struct semaphore *sem);extern void down (struct semaphore *sem);extern int __must_check down_interruptible (struct semaphore *sem);extern int __must_check down_killable (struct semaphore *sem); -extern int __must_check down_trylock (struct semaphore *sem); +extern int __must_check down_try (struct semaphore *sem);extern int __must_check down_timeout (struct semaphore *sem,long jiffies);extern void up (struct semaphore *sem);
include/linux/usb.h +1 -1
@@ -491,7 +491,7 @@ extern void usb_put_dev(struct usb_devic/* USB device locking */ #define usb_lock_device (udev) down(&(udev)->dev.sem)#define usb_unlock_device (udev) up(&(udev)->dev.sem) -#define usb_trylock_device (udev) down_trylock(&(udev)->dev.sem) +#define usb_trylock_device (udev) down_try(&(udev)->dev.sem)extern int usb_lock_device_for_reset (struct usb_device *udev,const struct usb_interface *iface);
kernel/mutex.c +2 -2
@@ -373,8 +373,8 @@ static inline int __mutex_trylock_slowpa* Try to acquire the mutex atomically. Returns 1 if the mutex * has been acquired successfully, and 0 on contention. * -* NOTE: this function follows the spin_trylock() convention, so -* it is negated to the down_trylock() return values! Be careful +* NOTE: this function follows the spin_trylock()/down_try() convention, +* so it is negated to the old down_trylock() return values! Be careful * about this when converting semaphore users to mutexes. * * This function must not be used in interrupt context. The
kernel/printk.c +2 -2
@@ -986,7 +986,7 @@ EXPORT_SYMBOL(acquire_console_sem);int try_acquire_console_sem (void ) { -if (down_trylock (&console_sem)) +if (!down_try (&console_sem))return -1 ; console_locked =1 ; console_may_schedule =0 ;
@@ -1083,7 +1083,7 @@ void console_unblank(void)* oops_in_progress is set to 1.. */ if (oops_in_progress) { -if (down_trylock (&console_sem) !=0 ) +if (!down_try (&console_sem))return ; }else acquire_console_sem ();
kernel/semaphore.c +8 -9
@@ -14,7 +14,7 @@* Some notes on the implementation: * * The spinlock controls access to the other members of the semaphore. -* down_trylock() and up() can be called from interrupt context, so we +* down_try() and up() can be called from interrupt context, so we * have to disable interrupts when taking the lock. It turns out various * parts of the kernel expect to be able to use down() on a semaphore in * interrupt context when they know it will succeed, so we have to use
@@ -114,19 +114,18 @@ EXPORT_SYMBOL(down_killable);EXPORT_SYMBOL (down_killable);/** -* down_trylock - try to acquire the semaphore, without waiting +* down_try - try to acquire the semaphore, without waiting * @sem: the semaphore to be acquired * -* Try to acquire the semaphore atomically. Returns 0 if the mutex has -* been acquired successfully or 1 if it it cannot be acquired. +* Try to acquire the semaphore atomically. Returns true if the mutex has +* been acquired successfully or 0 if it it cannot be acquired. * -* NOTE: This return value is inverted from both spin_trylock and -* mutex_trylock! Be careful about this when converting code. +* NOTE: This replaces down_trylock() which returned the reverse. * * Unlike mutex_trylock, this function can be used from interrupt context, * and the semaphore can be released by any task or interrupt. */ -int down_trylock (struct semaphore *sem) +int down_try (struct semaphore *sem) {unsigned long flags;int count;
@@ -137,9 +136,9 @@ int down_trylock(struct semaphore *sem) sem->count = count;spin_unlock_irqrestore (&sem->lock, flags); -return (count <0 ); +return (count >=0 ); } -EXPORT_SYMBOL (down_trylock); +EXPORT_SYMBOL (down_try);/** * down_timeout - acquire the semaphore within a specified time
Quoted 10 lines · Rusty Russell
down_trylock()returns 1 on failure, 0 on success. This differs fromspin_trylock(),mutex_trylock()and common sense. Or as ocfs2 put it "kernel 1, world 0".Rename it to
down_try()(which makes more sense anyway), and reverse it. Fortunately there aren't a huge number of callers left.I took the liberty of reversing the sense of
usb_trylock_device()without renaming it: it's only used in one place anyway.
Given that people are actively trying to kill struct semaphore I don't think doing a big search and rename is a good idea right now.
(And I also really hate the name down_try, but when it goes away that's
rather void and we can spare the discussion)
Quoted 10 lines · On Monday 05 May 2008 15:58:23 Christoph Hellwig wrote:
Quoted 6 lines · Rusty Russell
down_trylock()returns 1 on failure, 0 on success. This differs fromspin_trylock(),mutex_trylock()and common sense. Or as ocfs2 put it "kernel 1, world 0".Rename it to
down_try()(which makes more sense anyway), and reverse it. Fortunately there aren't a huge number of callers left.Given that people are actively trying to kill struct semaphore I don't think doing a big search and rename is a good idea right now.
If it goes away before the 2.6.27 merge window, great. But I don't see that happening, so let's clean up this horror. I cc'd all the people effected in the hope that it will prod some of them towards mutexes anyway.
Quoted 2 lines
(And I also really hate the name
down_try, but when it goes away that's
rather void and we can spare the discussion)
Ideas? down() is pretty bad, down_try() matches it.
Thanks,
Rusty.
Quoted 6 lines · Rusty Russell
Quoted 2 lines
Given that people are actively trying to kill struct semaphore I don't think doing a big search and rename is a good idea right now.
If it goes away before the 2.6.27 merge window, great. But I don't see that happening, so let's clean up this horror. I cc'd all the people effected in the hope that it will prod some of them towards mutexes anyway.
.27 might not be doable but .28 seems probable if willy and co are continuing to churn like they do currently.
Quoted 4 lines
Quoted 2 lines
(And I also really hate the name
down_try, but when it goes away that's
rather void and we can spare the discussion)Ideas?
down()is pretty bad,down_try()matches it.
The trylock is a convention for real locking function, so having one stand out would be nasty. Then again a semaphore is not just a simple lock but a higher level locking primitive, so a down_nowait might make sense because we don't encode the lock anywhere else either
Quoted 11 lines · On Monday 05 May 2008 16:12:17 Christoph Hellwig wrote:
Quoted 7 lines · Rusty Russell
Quoted 2 lines
Given that people are actively trying to kill struct semaphore I don't think doing a big search and rename is a good idea right now.
If it goes away before the 2.6.27 merge window, great. But I don't see that happening, so let's clean up this horror. I cc'd all the people effected in the hope that it will prod some of them towards mutexes anyway.
.27 might not be doable but .28 seems probable if willy and co are continuing to churn like they do currently.
I didn't think he was killing them all, just the ones which are actually mutex wannabes?
Quoted 6 lines
Quoted 1 line
Ideas?
down()is pretty bad,down_try()matches it.The trylock is a convention for real locking function, so having one stand out would be nasty. Then again a semaphore is not just a simple lock but a higher level locking primitive, so a
down_nowaitmight make sense because we don't encode the lock anywhere else either
Yep, down_nowait() it is. I'll roll a new one if willy isn't going to get rid of them all.
Thanks,
Rusty.