Skip to contentSkip to conversation

Replace down_trylock() with down_try(), reverse return values.

PATCH In review Rusty Russell 2 participants 5 messages May 5, 2008

Subsystems (20)
lore
Share & tools
lore
#0 lore raw

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
diff -r c4dfb28595bc Documentation/DocBook/kernel-locking.tmpl
@@ -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
diff -r c4dfb28595bc arch/ia64/kernel/salinfo.c
@@ -192,7 +192,7 @@ static void
 static 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, ch
 	int 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
diff -r c4dfb28595bc drivers/char/snsc.c
@@ -161,7 +161,7 @@ scdrv_read(struct file *file, char __use
 	struct 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 cha
 	struct 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
diff -r c4dfb28595bc drivers/char/viotape.c
@@ -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
diff -r c4dfb28595bc drivers/infiniband/core/user_mad.c
@@ -890,7 +890,7 @@ static int ib_umad_sm_open(struct inode 
 		return -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
diff -r c4dfb28595bc drivers/input/serio/hil_mlc.c
@@ -607,7 +607,7 @@ static inline void hilse_setup_input(hil
 	do_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
diff -r c4dfb28595bc drivers/input/serio/hp_sdc_mlc.c
@@ -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
diff -r c4dfb28595bc drivers/md/dm-raid1.c
@@ -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
diff -r c4dfb28595bc drivers/net/3c527.c
@@ -577,7 +577,7 @@ static int mc32_command_nowait(struct ne
 	int 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
diff -r c4dfb28595bc drivers/net/irda/sir_dev.c
@@ -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
diff -r c4dfb28595bc drivers/net/ps3_gelic_wireless.c
@@ -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
diff -r c4dfb28595bc drivers/net/wireless/airo.c
@@ -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_get
 
 	if (!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(stru
 
 	if ((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 ir
 				set_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_wi
 
 	if (!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
diff -r c4dfb28595bc drivers/scsi/aacraid/commsup.c
@@ -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
diff -r c4dfb28595bc drivers/usb/core/usb.c
@@ -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
diff -r c4dfb28595bc drivers/usb/gadget/inode.c
@@ -298,7 +298,7 @@ get_ready_ep (unsigned f_flags, struct e
 	int	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
diff -r c4dfb28595bc drivers/watchdog/ar7_wdt.c
@@ -179,7 +179,7 @@ static int ar7_wdt_open(struct inode *in
 static 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
diff -r c4dfb28595bc drivers/watchdog/it8712f_wdt.c
@@ -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
diff -r c4dfb28595bc drivers/watchdog/s3c2410_wdt.c
@@ -211,7 +211,7 @@ static int s3c2410wdt_set_heartbeat(int 
 
 static 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
diff -r c4dfb28595bc drivers/watchdog/sc1200wdt.c
@@ -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
diff -r c4dfb28595bc drivers/watchdog/scx200_wdt.c
@@ -92,7 +92,7 @@ static int scx200_wdt_open(struct inode 
 static 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
diff -r c4dfb28595bc drivers/watchdog/wdt_pci.c
@@ -426,7 +426,7 @@ static int wdtpci_ioctl(struct inode *in
 
 static 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
diff -r c4dfb28595bc fs/ocfs2/inode.c
@@ -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
diff -r c4dfb28595bc fs/reiserfs/journal.c
@@ -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
diff -r c4dfb28595bc fs/xfs/linux-2.6/sema.h
@@ -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
diff -r c4dfb28595bc fs/xfs/linux-2.6/xfs_buf.c
@@ -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
diff -r c4dfb28595bc include/linux/mutex.h
@@ -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
diff -r c4dfb28595bc include/linux/semaphore.h
@@ -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
diff -r c4dfb28595bc include/linux/usb.h
@@ -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
diff -r c4dfb28595bc kernel/mutex.c
@@ -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
diff -r c4dfb28595bc kernel/printk.c
@@ -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
diff -r c4dfb28595bc kernel/semaphore.c
@@ -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
#1 lore raw
Quoted 10 lines · Rusty Russell

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.

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)

#2 lore raw
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 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.

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.

#3 lore raw
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

#4 lore raw
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_nowait might 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.