It is currently not possible for various wait_on_bit functions to
implement a timeout.
While the "action" function that is called to do the waiting could
certainly use schedule_timeout(), there is no way to carry forward the
remaining timeout after a false wake-up.
As false-wakeups a clearly possible at least due to possible
hash collisions in bit_waitqueue(), this is a real problem.
The 'action' function is currently passed a pointer to the word
containing the bit being waited on. Of the 27 currently defined
action functions, zero of them use this pointer.
So changing it to something else will be a little noisy but will have
no immediate effect.
This patch changes the 'action' function to take a pointer to the
"struct wait_bit_key", which contains a pointer to the word
containing the bit so nothing is really lost.
It also adds a 'private' field to "struct wait_bit_key", which is
initialized to zero.
An action function can now implement a timeout with something like
static int timed_out_waiter(struct wait_bit_key *key)
{
unsigned long waited;
if (key->private == 0) {
key->private = jiffies;
if (key->private == 0)
key->private -= 1;
}
waited = jiffies - key->private;
if (waited > 10 * HZ)
return -EAGAIN;
schedule_timeout(waited - 10 * HZ);
return 0;
}
If any other need for context in a waiter were found it would be easy
to use ->private for some other purpose, or even extend "struct
wait_bit_key".
My particular need is to support timeouts in nfs_release_page() to
avoid deadlocks with loopback mounted NFS.
Signed-off-by: NeilBrown <[email protected]>
diff --git a/drivers/md/dm-bufio.c b/drivers/md/dm-bufio.c
index 66c5d130c8c2..3f51affd7d0e 100644
--- a/drivers/md/dm-bufio.c
+++ b/drivers/md/dm-bufio.c
@@ -617,7 +617,7 @@ static void write_endio(struct bio *bio, int error)
/*
* This function is called when wait_on_bit is actually waiting.
*/
-static int do_io_schedule(void *word)
+static int do_io_schedule(struct wait_bit_key *key)
{
io_schedule();
diff --git a/drivers/md/dm-snap.c b/drivers/md/dm-snap.c
index ebddef5237e4..efd2c6753e10 100644
--- a/drivers/md/dm-snap.c
+++ b/drivers/md/dm-snap.c
@@ -1032,7 +1032,7 @@ static void start_merge(struct dm_snapshot *s)
snapshot_merge_next_chunks(s);
}
-static int wait_schedule(void *ptr)
+static int wait_schedule(struct wait_bit_key *key)
{
schedule();
diff --git a/drivers/media/usb/dvb-usb-v2/dvb_usb_core.c b/drivers/media/usb/dvb-usb-v2/dvb_usb_core.c
index 8a054d66e708..c2677deaf83c 100644
--- a/drivers/media/usb/dvb-usb-v2/dvb_usb_core.c
+++ b/drivers/media/usb/dvb-usb-v2/dvb_usb_core.c
@@ -253,7 +253,7 @@ static int dvb_usbv2_adapter_stream_exit(struct dvb_usb_adapter *adap)
return usb_urb_exitv2(&adap->stream);
}
-static int wait_schedule(void *ptr)
+static int wait_schedule(struct wait_bit_key *key)
{
schedule();
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 85bbd01f1271..de1926efeb43 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -3366,7 +3366,7 @@ done_unlocked:
return 0;
}
-static int eb_wait(void *word)
+static int eb_wait(struct wait_bit_key *key)
{
io_schedule();
return 0;
diff --git a/fs/buffer.c b/fs/buffer.c
index 27265a8b43c1..75c22932fd35 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -61,7 +61,7 @@ inline void touch_buffer(struct buffer_head *bh)
}
EXPORT_SYMBOL(touch_buffer);
-static int sleep_on_buffer(void *word)
+static int sleep_on_buffer(struct wait_bit_key *key)
{
io_schedule();
return 0;
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 8813ff776ba3..b2e1a2ff8991 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -3932,7 +3932,7 @@ cifs_sb_master_tcon(struct cifs_sb_info *cifs_sb)
}
static int
-cifs_sb_tcon_pending_wait(void *unused)
+cifs_sb_tcon_pending_wait(struct wait_bit_key *unused)
{
schedule();
return signal_pending(current) ? -ERESTARTSYS : 0;
diff --git a/fs/fscache/internal.h b/fs/fscache/internal.h
index 4226f6680b06..0988cf622f72 100644
--- a/fs/fscache/internal.h
+++ b/fs/fscache/internal.h
@@ -91,8 +91,8 @@ static inline bool fscache_object_congested(void)
return workqueue_congested(WORK_CPU_UNBOUND, fscache_object_wq);
}
-extern int fscache_wait_bit(void *);
-extern int fscache_wait_bit_interruptible(void *);
+extern int fscache_wait_bit(struct wait_bit_key *);
+extern int fscache_wait_bit_interruptible(struct wait_bit_key *);
extern int fscache_wait_atomic_t(atomic_t *);
/*
diff --git a/fs/fscache/main.c b/fs/fscache/main.c
index 7c27907e650c..b393201c1388 100644
--- a/fs/fscache/main.c
+++ b/fs/fscache/main.c
@@ -200,7 +200,7 @@ module_exit(fscache_exit);
/*
* wait_on_bit() sleep function for uninterruptible waiting
*/
-int fscache_wait_bit(void *flags)
+int fscache_wait_bit(struct wait_bit_key *key)
{
schedule();
return 0;
@@ -209,7 +209,7 @@ int fscache_wait_bit(void *flags)
/*
* wait_on_bit() sleep function for interruptible waiting
*/
-int fscache_wait_bit_interruptible(void *flags)
+int fscache_wait_bit_interruptible(struct wait_bit_key *key)
{
schedule();
return signal_pending(current);
diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c
index ca0be6c69a26..25ff6e95b814 100644
--- a/fs/gfs2/glock.c
+++ b/fs/gfs2/glock.c
@@ -862,13 +862,13 @@ void gfs2_holder_uninit(struct gfs2_holder *gh)
* order to be more informative to the user.
*/
-static int gfs2_glock_holder_wait(void *word)
+static int gfs2_glock_holder_wait(struct wait_bit_key *key)
{
schedule();
return 0;
}
-static int gfs2_glock_demote_wait(void *word)
+static int gfs2_glock_demote_wait(struct wait_bit_key *key)
{
schedule();
return 0;
diff --git a/fs/gfs2/lock_dlm.c b/fs/gfs2/lock_dlm.c
index 2a6ba06bee6f..10bfdfec9fa3 100644
--- a/fs/gfs2/lock_dlm.c
+++ b/fs/gfs2/lock_dlm.c
@@ -934,7 +934,7 @@ fail:
return error;
}
-static int dlm_recovery_wait(void *word)
+static int dlm_recovery_wait(struct wait_bit_key *key)
{
schedule();
return 0;
diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
index c6872d09561a..e7ec580da091 100644
--- a/fs/gfs2/ops_fstype.c
+++ b/fs/gfs2/ops_fstype.c
@@ -1071,7 +1071,7 @@ void gfs2_lm_unmount(struct gfs2_sbd *sdp)
lm->lm_unmount(sdp);
}
-static int gfs2_journalid_wait(void *word)
+static int gfs2_journalid_wait(struct wait_bit_key *key)
{
if (signal_pending(current))
return -EINTR;
diff --git a/fs/gfs2/recovery.c b/fs/gfs2/recovery.c
index 963b2d75200c..63c371e27228 100644
--- a/fs/gfs2/recovery.c
+++ b/fs/gfs2/recovery.c
@@ -591,7 +591,7 @@ done:
wake_up_bit(&jd->jd_flags, JDF_RECOVERY);
}
-static int gfs2_recovery_wait(void *word)
+static int gfs2_recovery_wait(struct wait_bit_key *key)
{
schedule();
return 0;
diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c
index 60f60f6181f3..daa0a3a64c28 100644
--- a/fs/gfs2/super.c
+++ b/fs/gfs2/super.c
@@ -858,7 +858,7 @@ static int gfs2_make_fs_ro(struct gfs2_sbd *sdp)
return error;
}
-static int gfs2_umount_recovery_wait(void *word)
+static int gfs2_umount_recovery_wait(struct wait_bit_key *key)
{
schedule();
return 0;
diff --git a/fs/inode.c b/fs/inode.c
index 4bcdad3c9361..2ba2da54eebc 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -1696,7 +1696,7 @@ int inode_needs_sync(struct inode *inode)
}
EXPORT_SYMBOL(inode_needs_sync);
-int inode_wait(void *word)
+int inode_wait(struct wait_bit_key *key)
{
schedule();
return 0;
diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c
index 60bb365f54a5..b7ed6e229bf3 100644
--- a/fs/jbd2/transaction.c
+++ b/fs/jbd2/transaction.c
@@ -763,7 +763,7 @@ static void warn_dirty_buffer(struct buffer_head *bh)
bdevname(bh->b_bdev, b), (unsigned long long)bh->b_blocknr);
}
-static int sleep_on_shadow_bh(void *word)
+static int sleep_on_shadow_bh(struct wait_bit_key *key)
{
io_schedule();
return 0;
diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index 91c4746fdab1..c26c55e1ad25 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -75,7 +75,7 @@ nfs_fattr_to_ino_t(struct nfs_fattr *fattr)
* nfs_wait_bit_killable - helper for functions that are sleeping on bit locks
* @word: long word containing the bit lock
*/
-int nfs_wait_bit_killable(void *word)
+int nfs_wait_bit_killable(struct wait_bit_key *key)
{
if (fatal_signal_pending(current))
return -ERESTARTSYS;
diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
index b46cf5a67329..b4f65ca80d65 100644
--- a/fs/nfs/internal.h
+++ b/fs/nfs/internal.h
@@ -341,7 +341,7 @@ extern int nfs_drop_inode(struct inode *);
extern void nfs_clear_inode(struct inode *);
extern void nfs_evict_inode(struct inode *);
void nfs_zap_acl_cache(struct inode *inode);
-extern int nfs_wait_bit_killable(void *word);
+extern int nfs_wait_bit_killable(struct wait_bit_key *key);
/* super.c */
extern const struct super_operations nfs_sops;
diff --git a/fs/nfs/pagelist.c b/fs/nfs/pagelist.c
index 2ffebf2081ce..d48c1001c29c 100644
--- a/fs/nfs/pagelist.c
+++ b/fs/nfs/pagelist.c
@@ -258,7 +258,7 @@ void nfs_release_request(struct nfs_page *req)
kref_put(&req->wb_kref, nfs_free_request);
}
-static int nfs_wait_bit_uninterruptible(void *word)
+static int nfs_wait_bit_uninterruptible(struct wait_bit_key *key)
{
io_schedule();
return 0;
diff --git a/include/linux/sunrpc/sched.h b/include/linux/sunrpc/sched.h
index 3a847de83fab..592be588ce62 100644
--- a/include/linux/sunrpc/sched.h
+++ b/include/linux/sunrpc/sched.h
@@ -236,7 +236,7 @@ void * rpc_malloc(struct rpc_task *, size_t);
void rpc_free(void *);
int rpciod_up(void);
void rpciod_down(void);
-int __rpc_wait_for_completion_task(struct rpc_task *task, int (*)(void *));
+int __rpc_wait_for_completion_task(struct rpc_task *task, int (*)(struct wait_bit_key *));
#ifdef RPC_DEBUG
struct net;
void rpc_show_tasks(struct net *);
diff --git a/include/linux/wait.h b/include/linux/wait.h
index 559044c79232..d7ce4c908352 100644
--- a/include/linux/wait.h
+++ b/include/linux/wait.h
@@ -25,6 +25,7 @@ struct wait_bit_key {
void *flags;
int bit_nr;
#define WAIT_ATOMIC_T_BIT_NR -1
+ unsigned long private;
};
struct wait_bit_queue {
@@ -147,12 +148,12 @@ void __wake_up_sync_key(wait_queue_head_t *q, unsigned int mode, int nr, void *k
void __wake_up_locked(wait_queue_head_t *q, unsigned int mode, int nr);
void __wake_up_sync(wait_queue_head_t *q, unsigned int mode, int nr);
void __wake_up_bit(wait_queue_head_t *, void *, int);
-int __wait_on_bit(wait_queue_head_t *, struct wait_bit_queue *, int (*)(void *), unsigned);
-int __wait_on_bit_lock(wait_queue_head_t *, struct wait_bit_queue *, int (*)(void *), unsigned);
+int __wait_on_bit(wait_queue_head_t *, struct wait_bit_queue *, int (*)(struct wait_bit_key *), unsigned);
+int __wait_on_bit_lock(wait_queue_head_t *, struct wait_bit_queue *, int (*)(struct wait_bit_key *), unsigned);
void wake_up_bit(void *, int);
void wake_up_atomic_t(atomic_t *);
-int out_of_line_wait_on_bit(void *, int, int (*)(void *), unsigned);
-int out_of_line_wait_on_bit_lock(void *, int, int (*)(void *), unsigned);
+int out_of_line_wait_on_bit(void *, int, int (*)(struct wait_bit_key *), unsigned);
+int out_of_line_wait_on_bit_lock(void *, int, int (*)(struct wait_bit_key *), unsigned);
int out_of_line_wait_on_atomic_t(atomic_t *, int (*)(atomic_t *), unsigned);
wait_queue_head_t *bit_waitqueue(void *, int);
@@ -868,7 +869,7 @@ int wake_bit_function(wait_queue_t *wait, unsigned mode, int sync, void *key);
* but has no intention of setting it.
*/
static inline int
-wait_on_bit(void *word, int bit, int (*action)(void *), unsigned mode)
+wait_on_bit(void *word, int bit, int (*action)(struct wait_bit_key *), unsigned mode)
{
if (!test_bit(bit, word))
return 0;
@@ -892,7 +893,7 @@ wait_on_bit(void *word, int bit, int (*action)(void *), unsigned mode)
* clear with the intention of setting it, and when done, clearing it.
*/
static inline int
-wait_on_bit_lock(void *word, int bit, int (*action)(void *), unsigned mode)
+wait_on_bit_lock(void *word, int bit, int (*action)(struct wait_bit_key *), unsigned mode)
{
if (!test_and_set_bit(bit, word))
return 0;
diff --git a/include/linux/writeback.h b/include/linux/writeback.h
index 021b8a319b9e..0aeac2801479 100644
--- a/include/linux/writeback.h
+++ b/include/linux/writeback.h
@@ -90,7 +90,7 @@ struct writeback_control {
* fs/fs-writeback.c
*/
struct bdi_writeback;
-int inode_wait(void *);
+int inode_wait(struct wait_bit_key *);
void writeback_inodes_sb(struct super_block *, enum wb_reason reason);
void writeback_inodes_sb_nr(struct super_block *, unsigned long nr,
enum wb_reason reason);
diff --git a/kernel/ptrace.c b/kernel/ptrace.c
index 1f4bcb3cc21c..64e2183d9e1c 100644
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -28,7 +28,7 @@
#include <linux/compat.h>
-static int ptrace_trapping_sleep_fn(void *flags)
+static int ptrace_trapping_sleep_fn(struct wait_bit_key *key)
{
schedule();
return 0;
diff --git a/kernel/sched/wait.c b/kernel/sched/wait.c
index 7d50f794e248..5add4ffe815c 100644
--- a/kernel/sched/wait.c
+++ b/kernel/sched/wait.c
@@ -319,14 +319,14 @@ EXPORT_SYMBOL(wake_bit_function);
*/
int __sched
__wait_on_bit(wait_queue_head_t *wq, struct wait_bit_queue *q,
- int (*action)(void *), unsigned mode)
+ int (*action)(struct wait_bit_key *), unsigned mode)
{
int ret = 0;
do {
prepare_to_wait(wq, &q->wait, mode);
if (test_bit(q->key.bit_nr, q->key.flags))
- ret = (*action)(q->key.flags);
+ ret = (*action)(&q->key);
} while (test_bit(q->key.bit_nr, q->key.flags) && !ret);
finish_wait(wq, &q->wait);
return ret;
@@ -334,7 +334,7 @@ __wait_on_bit(wait_queue_head_t *wq, struct wait_bit_queue *q,
EXPORT_SYMBOL(__wait_on_bit);
int __sched out_of_line_wait_on_bit(void *word, int bit,
- int (*action)(void *), unsigned mode)
+ int (*action)(struct wait_bit_key *), unsigned mode)
{
wait_queue_head_t *wq = bit_waitqueue(word, bit);
DEFINE_WAIT_BIT(wait, word, bit);
@@ -345,7 +345,7 @@ EXPORT_SYMBOL(out_of_line_wait_on_bit);
int __sched
__wait_on_bit_lock(wait_queue_head_t *wq, struct wait_bit_queue *q,
- int (*action)(void *), unsigned mode)
+ int (*action)(struct wait_bit_key *), unsigned mode)
{
do {
int ret;
@@ -353,7 +353,7 @@ __wait_on_bit_lock(wait_queue_head_t *wq, struct wait_bit_queue *q,
prepare_to_wait_exclusive(wq, &q->wait, mode);
if (!test_bit(q->key.bit_nr, q->key.flags))
continue;
- ret = action(q->key.flags);
+ ret = action(&q->key);
if (!ret)
continue;
abort_exclusive_wait(wq, &q->wait, mode, &q->key);
@@ -365,7 +365,7 @@ __wait_on_bit_lock(wait_queue_head_t *wq, struct wait_bit_queue *q,
EXPORT_SYMBOL(__wait_on_bit_lock);
int __sched out_of_line_wait_on_bit_lock(void *word, int bit,
- int (*action)(void *), unsigned mode)
+ int (*action)(struct wait_bit_key *), unsigned mode)
{
wait_queue_head_t *wq = bit_waitqueue(word, bit);
DEFINE_WAIT_BIT(wait, word, bit);
diff --git a/mm/filemap.c b/mm/filemap.c
index 7a13f6ac5421..4e9ed37e566d 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -176,15 +176,15 @@ void delete_from_page_cache(struct page *page)
}
EXPORT_SYMBOL(delete_from_page_cache);
-static int sleep_on_page(void *word)
+static int sleep_on_page(struct wait_bit_key *key)
{
io_schedule();
return 0;
}
-static int sleep_on_page_killable(void *word)
+static int sleep_on_page_killable(struct wait_bit_key *key)
{
- sleep_on_page(word);
+ sleep_on_page(key);
return fatal_signal_pending(current) ? -EINTR : 0;
}
diff --git a/mm/ksm.c b/mm/ksm.c
index 68710e80994a..997b07993e77 100644
--- a/mm/ksm.c
+++ b/mm/ksm.c
@@ -1979,7 +1979,7 @@ void ksm_migrate_page(struct page *newpage, struct page *oldpage)
#endif /* CONFIG_MIGRATION */
#ifdef CONFIG_MEMORY_HOTREMOVE
-static int just_wait(void *word)
+static int just_wait(struct wait_bit_key *key)
{
schedule();
return 0;
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 5e8663c194c1..9d4ecf66202f 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -1753,7 +1753,7 @@ static void hci_inq_req(struct hci_request *req, unsigned long opt)
hci_req_add(req, HCI_OP_INQUIRY, sizeof(cp), &cp);
}
-static int wait_inquiry(void *word)
+static int wait_inquiry(struct wait_bit_key *key)
{
schedule();
return signal_pending(current);
diff --git a/net/sunrpc/sched.c b/net/sunrpc/sched.c
index ff3cc4bf4b24..132a5e93207a 100644
--- a/net/sunrpc/sched.c
+++ b/net/sunrpc/sched.c
@@ -250,7 +250,7 @@ void rpc_destroy_wait_queue(struct rpc_wait_queue *queue)
}
EXPORT_SYMBOL_GPL(rpc_destroy_wait_queue);
-static int rpc_wait_bit_killable(void *word)
+static int rpc_wait_bit_killable(struct wait_bit_key *key)
{
if (fatal_signal_pending(current))
return -ERESTARTSYS;
@@ -309,7 +309,7 @@ static int rpc_complete_task(struct rpc_task *task)
* to enforce taking of the wq->lock and hence avoid races with
* rpc_complete_task().
*/
-int __rpc_wait_for_completion_task(struct rpc_task *task, int (*action)(void *))
+int __rpc_wait_for_completion_task(struct rpc_task *task, int (*action)(struct wait_bit_key *))
{
if (action == NULL)
action = rpc_wait_bit_killable;
diff --git a/security/keys/gc.c b/security/keys/gc.c
index d3222b6d7d59..3b5375cf558e 100644
--- a/security/keys/gc.c
+++ b/security/keys/gc.c
@@ -94,7 +94,7 @@ static void key_gc_timer_func(unsigned long data)
/*
* wait_on_bit() sleep function for uninterruptible waiting
*/
-static int key_gc_wait_bit(void *flags)
+static int key_gc_wait_bit(struct wait_bit_key *key)
{
schedule();
return 0;
diff --git a/security/keys/request_key.c b/security/keys/request_key.c
index 381411941cc1..ed43d04db4fd 100644
--- a/security/keys/request_key.c
+++ b/security/keys/request_key.c
@@ -24,7 +24,7 @@
/*
* wait_on_bit() sleep function for uninterruptible waiting
*/
-static int key_wait_bit(void *flags)
+static int key_wait_bit(struct wait_bit_key *key)
{
schedule();
return 0;
@@ -33,7 +33,7 @@ static int key_wait_bit(void *flags)
/*
* wait_on_bit() sleep function for interruptible waiting
*/
-static int key_wait_bit_intr(void *flags)
+static int key_wait_bit_intr(struct wait_bit_key *key)
{
schedule();
return signal_pending(current) ? -ERESTARTSYS : 0;
On Tue, Apr 29, 2014 at 07:44:06PM +1000, NeilBrown wrote:
>
>
> It is currently not possible for various wait_on_bit functions to
> implement a timeout.
> While the "action" function that is called to do the waiting could
> certainly use schedule_timeout(), there is no way to carry forward the
> remaining timeout after a false wake-up.
> As false-wakeups a clearly possible at least due to possible
> hash collisions in bit_waitqueue(), this is a real problem.
>
> The 'action' function is currently passed a pointer to the word
> containing the bit being waited on. Of the 27 currently defined
> action functions, zero of them use this pointer.
> So changing it to something else will be a little noisy but will have
> no immediate effect.
>
> This patch changes the 'action' function to take a pointer to the
> "struct wait_bit_key", which contains a pointer to the word
> containing the bit so nothing is really lost.
>
> It also adds a 'private' field to "struct wait_bit_key", which is
> initialized to zero.
>
> An action function can now implement a timeout with something like
>
> static int timed_out_waiter(struct wait_bit_key *key)
> {
> unsigned long waited;
> if (key->private == 0) {
> key->private = jiffies;
> if (key->private == 0)
> key->private -= 1;
> }
> waited = jiffies - key->private;
> if (waited > 10 * HZ)
> return -EAGAIN;
> schedule_timeout(waited - 10 * HZ);
> return 0;
> }
>
> If any other need for context in a waiter were found it would be easy
> to use ->private for some other purpose, or even extend "struct
> wait_bit_key".
>
> My particular need is to support timeouts in nfs_release_page() to
> avoid deadlocks with loopback mounted NFS.
So I'm sure I'm not getting it; but why is all the wait_bit crap so
entirely different from the normal wait stuff?
Surely something like:
wait_event_timeout(&wq, test_bit(bit, word), timeout);
Is pretty much the same, no? The only thing that's different is the wake
function, but surely we can thread that into there somehow without all
this silly repetition.
Furthermore, I count about 23 action functions, of which there appear to
be only like 4 actual variants. Surely such repetition sucks arse and
should be avoided?
On Tue, 29 Apr 2014 12:32:17 +0200 Peter Zijlstra <[email protected]>
wrote:
> On Tue, Apr 29, 2014 at 07:44:06PM +1000, NeilBrown wrote:
> >
> >
> > It is currently not possible for various wait_on_bit functions to
> > implement a timeout.
> > While the "action" function that is called to do the waiting could
> > certainly use schedule_timeout(), there is no way to carry forward the
> > remaining timeout after a false wake-up.
> > As false-wakeups a clearly possible at least due to possible
> > hash collisions in bit_waitqueue(), this is a real problem.
> >
> > The 'action' function is currently passed a pointer to the word
> > containing the bit being waited on. Of the 27 currently defined
> > action functions, zero of them use this pointer.
> > So changing it to something else will be a little noisy but will have
> > no immediate effect.
> >
> > This patch changes the 'action' function to take a pointer to the
> > "struct wait_bit_key", which contains a pointer to the word
> > containing the bit so nothing is really lost.
> >
> > It also adds a 'private' field to "struct wait_bit_key", which is
> > initialized to zero.
> >
> > An action function can now implement a timeout with something like
> >
> > static int timed_out_waiter(struct wait_bit_key *key)
> > {
> > unsigned long waited;
> > if (key->private == 0) {
> > key->private = jiffies;
> > if (key->private == 0)
> > key->private -= 1;
> > }
> > waited = jiffies - key->private;
> > if (waited > 10 * HZ)
> > return -EAGAIN;
> > schedule_timeout(waited - 10 * HZ);
> > return 0;
> > }
> >
> > If any other need for context in a waiter were found it would be easy
> > to use ->private for some other purpose, or even extend "struct
> > wait_bit_key".
> >
> > My particular need is to support timeouts in nfs_release_page() to
> > avoid deadlocks with loopback mounted NFS.
>
> So I'm sure I'm not getting it; but why is all the wait_bit crap so
> entirely different from the normal wait stuff?
>
> Surely something like:
>
> wait_event_timeout(&wq, test_bit(bit, word), timeout);
>
> Is pretty much the same, no? The only thing that's different is the wake
> function, but surely we can thread that into there somehow without all
> this silly repetition.
>
> Furthermore, I count about 23 action functions, of which there appear to
> be only like 4 actual variants. Surely such repetition sucks arse and
> should be avoided?
>
Sure, we could replace the interface with something that matches a more
common pattern.
The wait_queue is chosen based on a hash of the bit and the word, and we
sometimes want "test_and_set_bit", and sometimes "test_bit" but we could
probably come up with reasonable definitions for
wait_bit{,_lock}{,_interruptible,_killable}{,_io}{,_freezable}{,_timeout}(
bit, word [, timeout]);
there are 48 functions there. We don't need all of them of course.
My particular use case (as currently designed) wouldn't actually be met by
these. In the 'action' function I current check to see if the connection
that the NFS client has is to the local machine or a remote machine and
adjust the timeout accordingly (and this state can change while waiting).
So I guess we add a "_cmd" set of interfaces too.
I'm not sure it's worth the effort - can we just stick with my idea?
Maybe defined and export action wrappers for
io_schedule schedule schedule(interruptible),
that covers everything except a couple of NFS/RPC things which which should
probably stay local to NFS/RPC.
NeilBrown
On Tue, 29 Apr 2014 12:32:17 +0200 Peter Zijlstra <[email protected]>
wrote:
> So I'm sure I'm not getting it; but why is all the wait_bit crap so
> entirely different from the normal wait stuff?
>
> Surely something like:
>
> wait_event_timeout(&wq, test_bit(bit, word), timeout);
>
> Is pretty much the same, no? The only thing that's different is the wake
> function, but surely we can thread that into there somehow without all
> this silly repetition.
>
> Furthermore, I count about 23 action functions, of which there appear to
> be only like 4 actual variants. Surely such repetition sucks arse and
> should be avoided?
>
What do you think of the following as a cleanup prior to my patch. It will
make my patch a lot smaller and removes a lot of the duplication that you
don't like.
It renames the current wait_on_bit{_lock} function to have a "_cmd" suffix
and introduces versions which are not passed an action function (maybe it
should be an "_action" suffix...).
wait_on_bit() and wait_on_bit_lock() use schedule(),
wait_on_bit_io() and wait_on_bit_lock_io() use io_schedule().
The bit_wait() and bit_wait_io() function that these pass through use
signal_pending_state() to determine if they should abort or [io_]schedule().
I think the code is right, but it is the bit I would particularly like you
to check.
The return value from wait_on_bit{,_lock} is 0 or non-zero.
Functions which wanted a particular error code need to interpolate that
themselves.
(somewhat amusingly, fs/fscache/cookie.c currently has
wait_on_bit(&cookie->flags, FSCACHE_COOKIE_INVALIDATING,
fscache_wait_bit_interruptible,
TASK_UNINTERRUPTIBLE);
which is a little bit interruptible, but mostly not...)
If you think it is a good cleanup I'll post a proper patch with all the right
Cc:s.
Thanks,
NeilBrown
diff --git a/include/linux/wait.h b/include/linux/wait.h
index 559044c79232..13e6fd9acf09 100644
--- a/include/linux/wait.h
+++ b/include/linux/wait.h
@@ -854,7 +854,7 @@ int wake_bit_function(wait_queue_t *wait, unsigned mode, int sync, void *key);
} while (0)
/**
- * wait_on_bit - wait for a bit to be cleared
+ * wait_on_bit_cmd - wait for a bit to be cleared
* @word: the word being waited on, a kernel virtual address
* @bit: the bit of the word being waited on
* @action: the function used to sleep, which may take special actions
@@ -868,15 +868,51 @@ int wake_bit_function(wait_queue_t *wait, unsigned mode, int sync, void *key);
* but has no intention of setting it.
*/
static inline int
-wait_on_bit(void *word, int bit, int (*action)(void *), unsigned mode)
+wait_on_bit_cmd(void *word, int bit, int (*action)(void *), unsigned mode)
{
if (!test_bit(bit, word))
return 0;
return out_of_line_wait_on_bit(word, bit, action, mode);
}
+extern int bit_wait(void *);
+extern int bit_wait_io(void *);
+
/**
- * wait_on_bit_lock - wait for a bit to be cleared, when wanting to set it
+ * wait_on_bit - wait for a bit to be cleared
+ * @word: the word being waited on, a kernel virtual address
+ * @bit: the bit of the word being waited on
+ * @mode: the task state to sleep in
+ *
+ * There is a standard hashed waitqueue table for generic use. This
+ * is the part of the hashtable's accessor API that waits on a bit.
+ * For instance, if one were to have waiters on a bitflag, one would
+ * call wait_on_bit() in threads waiting for the bit to clear.
+ * One uses wait_on_bit() where one is waiting for the bit to clear,
+ * but has no intention of setting it.
+ */
+static inline int
+wait_on_bit(void *word, int bit, unsigned mode)
+{
+ if (!test_bit(bit, word))
+ return 0;
+ return out_of_line_wait_on_bit(word, bit,
+ bit_wait,
+ mode & 65535);
+}
+
+static inline int
+wait_on_bit_io(void *word, int bit, unsigned mode)
+{
+ if (!test_bit(bit, word))
+ return 0;
+ return out_of_line_wait_on_bit(word, bit,
+ bit_wait_io,
+ mode & 65535);
+}
+
+/**
+ * wait_on_bit_lock_cmd - wait for a bit to be cleared, when wanting to set it
* @word: the word being waited on, a kernel virtual address
* @bit: the bit of the word being waited on
* @action: the function used to sleep, which may take special actions
@@ -892,7 +928,7 @@ wait_on_bit(void *word, int bit, int (*action)(void *), unsigned mode)
* clear with the intention of setting it, and when done, clearing it.
*/
static inline int
-wait_on_bit_lock(void *word, int bit, int (*action)(void *), unsigned mode)
+wait_on_bit_lock_cmd(void *word, int bit, int (*action)(void *), unsigned mode)
{
if (!test_and_set_bit(bit, word))
return 0;
@@ -900,6 +936,41 @@ wait_on_bit_lock(void *word, int bit, int (*action)(void *), unsigned mode)
}
/**
+ * wait_on_bit_lock - wait for a bit to be cleared, when wanting to set it
+ * @word: the word being waited on, a kernel virtual address
+ * @bit: the bit of the word being waited on
+ * @mode: the task state to sleep in
+ *
+ * There is a standard hashed waitqueue table for generic use. This
+ * is the part of the hashtable's accessor API that waits on a bit
+ * when one intends to set it, for instance, trying to lock bitflags.
+ * For instance, if one were to have waiters trying to set bitflag
+ * and waiting for it to clear before setting it, one would call
+ * wait_on_bit() in threads waiting to be able to set the bit.
+ * One uses wait_on_bit_lock() where one is waiting for the bit to
+ * clear with the intention of setting it, and when done, clearing it.
+ */
+static inline int
+wait_on_bit_lock(void *word, int bit, unsigned mode)
+{
+ if (!test_and_set_bit(bit, word))
+ return 0;
+ return out_of_line_wait_on_bit_lock(word, bit,
+ bit_wait,
+ mode & 65536);
+}
+
+static inline int
+wait_on_bit_lock_io(void *word, int bit, unsigned mode)
+{
+ if (!test_and_set_bit(bit, word))
+ return 0;
+ return out_of_line_wait_on_bit_lock(word, bit,
+ bit_wait_io,
+ mode & 65536);
+}
+
+/**
* wait_on_atomic_t - Wait for an atomic_t to become 0
* @val: The atomic value being waited on, a kernel virtual address
* @action: the function used to sleep, which may take special actions
diff --git a/kernel/sched/wait.c b/kernel/sched/wait.c
index 7d50f794e248..0c0795002f56 100644
--- a/kernel/sched/wait.c
+++ b/kernel/sched/wait.c
@@ -502,3 +502,21 @@ void wake_up_atomic_t(atomic_t *p)
__wake_up_bit(atomic_t_waitqueue(p), p, WAIT_ATOMIC_T_BIT_NR);
}
EXPORT_SYMBOL(wake_up_atomic_t);
+
+__sched int bit_wait(void *word)
+{
+ if (signal_pending_state(current->state, current))
+ return 1;
+ schedule();
+ return 0;
+}
+EXPORT_SYMBOL(bit_wait);
+
+__sched int bit_wait_io(void *word)
+{
+ if (signal_pending_state(current->state, current))
+ return 1;
+ io_schedule();
+ return 0;
+}
+EXPORT_SYMBOL(bit_wait_io);
diff --git a/Documentation/filesystems/caching/operations.txt b/Documentation/filesystems/caching/operations.txt
index bee2a5f93d60..a1c052cbba35 100644
--- a/Documentation/filesystems/caching/operations.txt
+++ b/Documentation/filesystems/caching/operations.txt
@@ -90,7 +90,7 @@ operations:
to be cleared before proceeding:
wait_on_bit(&op->flags, FSCACHE_OP_WAITING,
- fscache_wait_bit, TASK_UNINTERRUPTIBLE);
+ TASK_UNINTERRUPTIBLE);
(2) The operation may be fast asynchronous (FSCACHE_OP_FAST), in which case it
diff --git a/drivers/md/dm-bufio.c b/drivers/md/dm-bufio.c
index 66c5d130c8c2..c6b692dd3b88 100644
--- a/drivers/md/dm-bufio.c
+++ b/drivers/md/dm-bufio.c
@@ -615,16 +615,6 @@ static void write_endio(struct bio *bio, int error)
}
/*
- * This function is called when wait_on_bit is actually waiting.
- */
-static int do_io_schedule(void *word)
-{
- io_schedule();
-
- return 0;
-}
-
-/*
* Initiate a write on a dirty buffer, but don't wait for it.
*
* - If the buffer is not dirty, exit.
@@ -640,8 +630,8 @@ static void __write_dirty_buffer(struct dm_buffer *b,
return;
clear_bit(B_DIRTY, &b->state);
- wait_on_bit_lock(&b->state, B_WRITING,
- do_io_schedule, TASK_UNINTERRUPTIBLE);
+ wait_on_bit_lock_io(&b->state, B_WRITING,
+ TASK_UNINTERRUPTIBLE);
if (!write_list)
submit_io(b, WRITE, b->block, write_endio);
@@ -675,9 +665,9 @@ static void __make_buffer_clean(struct dm_buffer *b)
if (!b->state) /* fast case */
return;
- wait_on_bit(&b->state, B_READING, do_io_schedule, TASK_UNINTERRUPTIBLE);
+ wait_on_bit_io(&b->state, B_READING, TASK_UNINTERRUPTIBLE);
__write_dirty_buffer(b, NULL);
- wait_on_bit(&b->state, B_WRITING, do_io_schedule, TASK_UNINTERRUPTIBLE);
+ wait_on_bit_io(&b->state, B_WRITING, TASK_UNINTERRUPTIBLE);
}
/*
@@ -1030,7 +1020,7 @@ static void *new_read(struct dm_bufio_client *c, sector_t block,
if (need_submit)
submit_io(b, READ, b->block, read_endio);
- wait_on_bit(&b->state, B_READING, do_io_schedule, TASK_UNINTERRUPTIBLE);
+ wait_on_bit_io(&b->state, B_READING, TASK_UNINTERRUPTIBLE);
if (b->read_error) {
int error = b->read_error;
@@ -1209,15 +1199,13 @@ again:
dropped_lock = 1;
b->hold_count++;
dm_bufio_unlock(c);
- wait_on_bit(&b->state, B_WRITING,
- do_io_schedule,
- TASK_UNINTERRUPTIBLE);
+ wait_on_bit_io(&b->state, B_WRITING,
+ TASK_UNINTERRUPTIBLE);
dm_bufio_lock(c);
b->hold_count--;
} else
- wait_on_bit(&b->state, B_WRITING,
- do_io_schedule,
- TASK_UNINTERRUPTIBLE);
+ wait_on_bit_io(&b->state, B_WRITING,
+ TASK_UNINTERRUPTIBLE);
}
if (!test_bit(B_DIRTY, &b->state) &&
@@ -1321,15 +1309,15 @@ retry:
__write_dirty_buffer(b, NULL);
if (b->hold_count == 1) {
- wait_on_bit(&b->state, B_WRITING,
- do_io_schedule, TASK_UNINTERRUPTIBLE);
+ wait_on_bit_io(&b->state, B_WRITING,
+ TASK_UNINTERRUPTIBLE);
set_bit(B_DIRTY, &b->state);
__unlink_buffer(b);
__link_buffer(b, new_block, LIST_DIRTY);
} else {
sector_t old_block;
- wait_on_bit_lock(&b->state, B_WRITING,
- do_io_schedule, TASK_UNINTERRUPTIBLE);
+ wait_on_bit_lock_io(&b->state, B_WRITING,
+ TASK_UNINTERRUPTIBLE);
/*
* Relink buffer to "new_block" so that write_callback
* sees "new_block" as a block number.
@@ -1341,8 +1329,8 @@ retry:
__unlink_buffer(b);
__link_buffer(b, new_block, b->list_mode);
submit_io(b, WRITE, new_block, write_endio);
- wait_on_bit(&b->state, B_WRITING,
- do_io_schedule, TASK_UNINTERRUPTIBLE);
+ wait_on_bit_io(&b->state, B_WRITING,
+ TASK_UNINTERRUPTIBLE);
__unlink_buffer(b);
__link_buffer(b, old_block, b->list_mode);
}
diff --git a/drivers/md/dm-snap.c b/drivers/md/dm-snap.c
index ebddef5237e4..172ba0d6e4e0 100644
--- a/drivers/md/dm-snap.c
+++ b/drivers/md/dm-snap.c
@@ -1032,20 +1032,13 @@ static void start_merge(struct dm_snapshot *s)
snapshot_merge_next_chunks(s);
}
-static int wait_schedule(void *ptr)
-{
- schedule();
-
- return 0;
-}
-
/*
* Stop the merging process and wait until it finishes.
*/
static void stop_merge(struct dm_snapshot *s)
{
set_bit(SHUTDOWN_MERGE, &s->state_bits);
- wait_on_bit(&s->state_bits, RUNNING_MERGE, wait_schedule,
+ wait_on_bit(&s->state_bits, RUNNING_MERGE,
TASK_UNINTERRUPTIBLE);
clear_bit(SHUTDOWN_MERGE, &s->state_bits);
}
diff --git a/drivers/media/usb/dvb-usb-v2/dvb_usb_core.c b/drivers/media/usb/dvb-usb-v2/dvb_usb_core.c
index 8a054d66e708..f65ecdc79cbb 100644
--- a/drivers/media/usb/dvb-usb-v2/dvb_usb_core.c
+++ b/drivers/media/usb/dvb-usb-v2/dvb_usb_core.c
@@ -253,13 +253,6 @@ static int dvb_usbv2_adapter_stream_exit(struct dvb_usb_adapter *adap)
return usb_urb_exitv2(&adap->stream);
}
-static int wait_schedule(void *ptr)
-{
- schedule();
-
- return 0;
-}
-
static int dvb_usb_start_feed(struct dvb_demux_feed *dvbdmxfeed)
{
struct dvb_usb_adapter *adap = dvbdmxfeed->demux->priv;
@@ -273,7 +266,7 @@ static int dvb_usb_start_feed(struct dvb_demux_feed *dvbdmxfeed)
dvbdmxfeed->pid, dvbdmxfeed->index);
/* wait init is done */
- wait_on_bit(&adap->state_bits, ADAP_INIT, wait_schedule,
+ wait_on_bit(&adap->state_bits, ADAP_INIT,
TASK_UNINTERRUPTIBLE);
if (adap->active_fe == -1)
@@ -568,7 +561,7 @@ static int dvb_usb_fe_sleep(struct dvb_frontend *fe)
if (!adap->suspend_resume_active) {
set_bit(ADAP_SLEEP, &adap->state_bits);
- wait_on_bit(&adap->state_bits, ADAP_STREAMING, wait_schedule,
+ wait_on_bit(&adap->state_bits, ADAP_STREAMING,
TASK_UNINTERRUPTIBLE);
}
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 85bbd01f1271..34011f6be939 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -3366,16 +3366,10 @@ done_unlocked:
return 0;
}
-static int eb_wait(void *word)
-{
- io_schedule();
- return 0;
-}
-
void wait_on_extent_buffer_writeback(struct extent_buffer *eb)
{
- wait_on_bit(&eb->bflags, EXTENT_BUFFER_WRITEBACK, eb_wait,
- TASK_UNINTERRUPTIBLE);
+ wait_on_bit_io(&eb->bflags, EXTENT_BUFFER_WRITEBACK,
+ TASK_UNINTERRUPTIBLE);
}
static int lock_extent_buffer_for_io(struct extent_buffer *eb,
diff --git a/fs/buffer.c b/fs/buffer.c
index 27265a8b43c1..a92a1f51716a 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -61,16 +61,9 @@ inline void touch_buffer(struct buffer_head *bh)
}
EXPORT_SYMBOL(touch_buffer);
-static int sleep_on_buffer(void *word)
-{
- io_schedule();
- return 0;
-}
-
void __lock_buffer(struct buffer_head *bh)
{
- wait_on_bit_lock(&bh->b_state, BH_Lock, sleep_on_buffer,
- TASK_UNINTERRUPTIBLE);
+ wait_on_bit_lock_io(&bh->b_state, BH_Lock, TASK_UNINTERRUPTIBLE);
}
EXPORT_SYMBOL(__lock_buffer);
@@ -123,7 +116,7 @@ EXPORT_SYMBOL(buffer_check_dirty_writeback);
*/
void __wait_on_buffer(struct buffer_head * bh)
{
- wait_on_bit(&bh->b_state, BH_Lock, sleep_on_buffer, TASK_UNINTERRUPTIBLE);
+ wait_on_bit_io(&bh->b_state, BH_Lock, TASK_UNINTERRUPTIBLE);
}
EXPORT_SYMBOL(__wait_on_buffer);
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 8813ff776ba3..d4a24ef95647 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -3931,13 +3931,6 @@ cifs_sb_master_tcon(struct cifs_sb_info *cifs_sb)
return tlink_tcon(cifs_sb_master_tlink(cifs_sb));
}
-static int
-cifs_sb_tcon_pending_wait(void *unused)
-{
- schedule();
- return signal_pending(current) ? -ERESTARTSYS : 0;
-}
-
/* find and return a tlink with given uid */
static struct tcon_link *
tlink_rb_search(struct rb_root *root, kuid_t uid)
@@ -4036,11 +4029,10 @@ cifs_sb_tlink(struct cifs_sb_info *cifs_sb)
} else {
wait_for_construction:
ret = wait_on_bit(&tlink->tl_flags, TCON_LINK_PENDING,
- cifs_sb_tcon_pending_wait,
TASK_INTERRUPTIBLE);
if (ret) {
cifs_put_tlink(tlink);
- return ERR_PTR(ret);
+ return ERR_PTR(-ERESTARTSYS);
}
/* if it's good, return it */
diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
index d754e3cf99a8..268e25885d96 100644
--- a/fs/fs-writeback.c
+++ b/fs/fs-writeback.c
@@ -327,7 +327,8 @@ static void __inode_wait_for_writeback(struct inode *inode)
wqh = bit_waitqueue(&inode->i_state, __I_SYNC);
while (inode->i_state & I_SYNC) {
spin_unlock(&inode->i_lock);
- __wait_on_bit(wqh, &wq, inode_wait, TASK_UNINTERRUPTIBLE);
+ __wait_on_bit(wqh, &wq, bit_wait,
+ TASK_UNINTERRUPTIBLE);
spin_lock(&inode->i_lock);
}
}
diff --git a/fs/fscache/cookie.c b/fs/fscache/cookie.c
index 29d7feb62cf7..9a494d8e1f1b 100644
--- a/fs/fscache/cookie.c
+++ b/fs/fscache/cookie.c
@@ -160,7 +160,7 @@ void __fscache_enable_cookie(struct fscache_cookie *cookie,
_enter("%p", cookie);
wait_on_bit_lock(&cookie->flags, FSCACHE_COOKIE_ENABLEMENT_LOCK,
- fscache_wait_bit, TASK_UNINTERRUPTIBLE);
+ TASK_UNINTERRUPTIBLE);
if (test_bit(FSCACHE_COOKIE_ENABLED, &cookie->flags))
goto out_unlock;
@@ -255,7 +255,7 @@ static int fscache_acquire_non_index_cookie(struct fscache_cookie *cookie)
if (!fscache_defer_lookup) {
_debug("non-deferred lookup %p", &cookie->flags);
wait_on_bit(&cookie->flags, FSCACHE_COOKIE_LOOKING_UP,
- fscache_wait_bit, TASK_UNINTERRUPTIBLE);
+ TASK_UNINTERRUPTIBLE);
_debug("complete");
if (test_bit(FSCACHE_COOKIE_UNAVAILABLE, &cookie->flags))
goto unavailable;
@@ -463,8 +463,7 @@ void __fscache_wait_on_invalidate(struct fscache_cookie *cookie)
_enter("%p", cookie);
wait_on_bit(&cookie->flags, FSCACHE_COOKIE_INVALIDATING,
- fscache_wait_bit_interruptible,
- TASK_UNINTERRUPTIBLE);
+ TASK_INTERRUPTIBLE);
_leave("");
}
@@ -525,7 +524,7 @@ void __fscache_disable_cookie(struct fscache_cookie *cookie, bool invalidate)
}
wait_on_bit_lock(&cookie->flags, FSCACHE_COOKIE_ENABLEMENT_LOCK,
- fscache_wait_bit, TASK_UNINTERRUPTIBLE);
+ TASK_UNINTERRUPTIBLE);
if (!test_and_clear_bit(FSCACHE_COOKIE_ENABLED, &cookie->flags))
goto out_unlock_enable;
diff --git a/fs/fscache/main.c b/fs/fscache/main.c
index 7c27907e650c..818057de05c6 100644
--- a/fs/fscache/main.c
+++ b/fs/fscache/main.c
@@ -198,24 +198,6 @@ static void __exit fscache_exit(void)
module_exit(fscache_exit);
/*
- * wait_on_bit() sleep function for uninterruptible waiting
- */
-int fscache_wait_bit(void *flags)
-{
- schedule();
- return 0;
-}
-
-/*
- * wait_on_bit() sleep function for interruptible waiting
- */
-int fscache_wait_bit_interruptible(void *flags)
-{
- schedule();
- return signal_pending(current);
-}
-
-/*
* wait_on_atomic_t() sleep function for uninterruptible waiting
*/
int fscache_wait_atomic_t(atomic_t *p)
diff --git a/fs/fscache/page.c b/fs/fscache/page.c
index 7f5c658af755..e9bb50c391db 100644
--- a/fs/fscache/page.c
+++ b/fs/fscache/page.c
@@ -298,7 +298,6 @@ int fscache_wait_for_deferred_lookup(struct fscache_cookie *cookie)
jif = jiffies;
if (wait_on_bit(&cookie->flags, FSCACHE_COOKIE_LOOKING_UP,
- fscache_wait_bit_interruptible,
TASK_INTERRUPTIBLE) != 0) {
fscache_stat(&fscache_n_retrievals_intr);
_leave(" = -ERESTARTSYS");
@@ -342,7 +341,6 @@ int fscache_wait_for_operation_activation(struct fscache_object *object,
if (stat_op_waits)
fscache_stat(stat_op_waits);
if (wait_on_bit(&op->flags, FSCACHE_OP_WAITING,
- fscache_wait_bit_interruptible,
TASK_INTERRUPTIBLE) != 0) {
ret = fscache_cancel_op(op, do_cancel);
if (ret == 0)
@@ -351,7 +349,7 @@ int fscache_wait_for_operation_activation(struct fscache_object *object,
/* it's been removed from the pending queue by another party,
* so we should get to run shortly */
wait_on_bit(&op->flags, FSCACHE_OP_WAITING,
- fscache_wait_bit, TASK_UNINTERRUPTIBLE);
+ TASK_UNINTERRUPTIBLE);
}
_debug("<<< GO");
diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c
index ca0be6c69a26..43a54163d99e 100644
--- a/fs/gfs2/glock.c
+++ b/fs/gfs2/glock.c
@@ -854,27 +854,6 @@ void gfs2_holder_uninit(struct gfs2_holder *gh)
}
/**
- * gfs2_glock_holder_wait
- * @word: unused
- *
- * This function and gfs2_glock_demote_wait both show up in the WCHAN
- * field. Thus I've separated these otherwise identical functions in
- * order to be more informative to the user.
- */
-
-static int gfs2_glock_holder_wait(void *word)
-{
- schedule();
- return 0;
-}
-
-static int gfs2_glock_demote_wait(void *word)
-{
- schedule();
- return 0;
-}
-
-/**
* gfs2_glock_wait - wait on a glock acquisition
* @gh: the glock holder
*
@@ -886,7 +865,7 @@ int gfs2_glock_wait(struct gfs2_holder *gh)
unsigned long time1 = jiffies;
might_sleep();
- wait_on_bit(&gh->gh_iflags, HIF_WAIT, gfs2_glock_holder_wait, TASK_UNINTERRUPTIBLE);
+ wait_on_bit(&gh->gh_iflags, HIF_WAIT, TASK_UNINTERRUPTIBLE);
if (time_after(jiffies, time1 + HZ)) /* have we waited > a second? */
/* Lengthen the minimum hold time. */
gh->gh_gl->gl_hold_time = min(gh->gh_gl->gl_hold_time +
@@ -1122,7 +1101,7 @@ void gfs2_glock_dq_wait(struct gfs2_holder *gh)
struct gfs2_glock *gl = gh->gh_gl;
gfs2_glock_dq(gh);
might_sleep();
- wait_on_bit(&gl->gl_flags, GLF_DEMOTE, gfs2_glock_demote_wait, TASK_UNINTERRUPTIBLE);
+ wait_on_bit(&gl->gl_flags, GLF_DEMOTE, TASK_UNINTERRUPTIBLE);
}
/**
diff --git a/fs/gfs2/lock_dlm.c b/fs/gfs2/lock_dlm.c
index 2a6ba06bee6f..e7b029356355 100644
--- a/fs/gfs2/lock_dlm.c
+++ b/fs/gfs2/lock_dlm.c
@@ -934,12 +934,6 @@ fail:
return error;
}
-static int dlm_recovery_wait(void *word)
-{
- schedule();
- return 0;
-}
-
static int control_first_done(struct gfs2_sbd *sdp)
{
struct lm_lockstruct *ls = &sdp->sd_lockstruct;
@@ -974,7 +968,7 @@ restart:
fs_info(sdp, "control_first_done wait gen %u\n", start_gen);
wait_on_bit(&ls->ls_recover_flags, DFL_DLM_RECOVERY,
- dlm_recovery_wait, TASK_UNINTERRUPTIBLE);
+ TASK_UNINTERRUPTIBLE);
goto restart;
}
diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
index c6872d09561a..0a086acf0835 100644
--- a/fs/gfs2/ops_fstype.c
+++ b/fs/gfs2/ops_fstype.c
@@ -1071,20 +1071,13 @@ void gfs2_lm_unmount(struct gfs2_sbd *sdp)
lm->lm_unmount(sdp);
}
-static int gfs2_journalid_wait(void *word)
-{
- if (signal_pending(current))
- return -EINTR;
- schedule();
- return 0;
-}
-
static int wait_on_journal(struct gfs2_sbd *sdp)
{
if (sdp->sd_lockstruct.ls_ops->lm_mount == NULL)
return 0;
- return wait_on_bit(&sdp->sd_flags, SDF_NOJOURNALID, gfs2_journalid_wait, TASK_INTERRUPTIBLE);
+ return wait_on_bit(&sdp->sd_flags, SDF_NOJOURNALID, TASK_INTERRUPTIBLE)
+ ? -EINTR : 0;
}
void gfs2_online_uevent(struct gfs2_sbd *sdp)
diff --git a/fs/gfs2/recovery.c b/fs/gfs2/recovery.c
index 963b2d75200c..da8853f49609 100644
--- a/fs/gfs2/recovery.c
+++ b/fs/gfs2/recovery.c
@@ -591,12 +591,6 @@ done:
wake_up_bit(&jd->jd_flags, JDF_RECOVERY);
}
-static int gfs2_recovery_wait(void *word)
-{
- schedule();
- return 0;
-}
-
int gfs2_recover_journal(struct gfs2_jdesc *jd, bool wait)
{
int rv;
@@ -609,7 +603,7 @@ int gfs2_recover_journal(struct gfs2_jdesc *jd, bool wait)
BUG_ON(!rv);
if (wait)
- wait_on_bit(&jd->jd_flags, JDF_RECOVERY, gfs2_recovery_wait,
+ wait_on_bit(&jd->jd_flags, JDF_RECOVERY,
TASK_UNINTERRUPTIBLE);
return wait ? jd->jd_recover_error : 0;
diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c
index 60f60f6181f3..58c22af73967 100644
--- a/fs/gfs2/super.c
+++ b/fs/gfs2/super.c
@@ -858,12 +858,6 @@ static int gfs2_make_fs_ro(struct gfs2_sbd *sdp)
return error;
}
-static int gfs2_umount_recovery_wait(void *word)
-{
- schedule();
- return 0;
-}
-
/**
* gfs2_put_super - Unmount the filesystem
* @sb: The VFS superblock
@@ -888,7 +882,7 @@ restart:
continue;
spin_unlock(&sdp->sd_jindex_spin);
wait_on_bit(&jd->jd_flags, JDF_RECOVERY,
- gfs2_umount_recovery_wait, TASK_UNINTERRUPTIBLE);
+ TASK_UNINTERRUPTIBLE);
goto restart;
}
spin_unlock(&sdp->sd_jindex_spin);
diff --git a/fs/inode.c b/fs/inode.c
index 4bcdad3c9361..e5b968ddf79e 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -1696,13 +1696,6 @@ int inode_needs_sync(struct inode *inode)
}
EXPORT_SYMBOL(inode_needs_sync);
-int inode_wait(void *word)
-{
- schedule();
- return 0;
-}
-EXPORT_SYMBOL(inode_wait);
-
/*
* If we try to find an inode in the inode hash while it is being
* deleted, we have to wait until the filesystem completes its
diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c
index 60bb365f54a5..02a675b5d815 100644
--- a/fs/jbd2/transaction.c
+++ b/fs/jbd2/transaction.c
@@ -763,12 +763,6 @@ static void warn_dirty_buffer(struct buffer_head *bh)
bdevname(bh->b_bdev, b), (unsigned long long)bh->b_blocknr);
}
-static int sleep_on_shadow_bh(void *word)
-{
- io_schedule();
- return 0;
-}
-
/*
* If the buffer is already part of the current transaction, then there
* is nothing we need to do. If it is already part of a prior
@@ -906,8 +900,8 @@ repeat:
if (buffer_shadow(bh)) {
JBUFFER_TRACE(jh, "on shadow: sleep");
jbd_unlock_bh_state(bh);
- wait_on_bit(&bh->b_state, BH_Shadow,
- sleep_on_shadow_bh, TASK_UNINTERRUPTIBLE);
+ wait_on_bit_io(&bh->b_state, BH_Shadow,
+ TASK_UNINTERRUPTIBLE);
goto repeat;
}
diff --git a/fs/nfs/file.c b/fs/nfs/file.c
index 5bb790a69c71..d470019fddfa 100644
--- a/fs/nfs/file.c
+++ b/fs/nfs/file.c
@@ -362,8 +362,8 @@ start:
* Prevent starvation issues if someone is doing a consistency
* sync-to-disk
*/
- ret = wait_on_bit(&NFS_I(mapping->host)->flags, NFS_INO_FLUSHING,
- nfs_wait_bit_killable, TASK_KILLABLE);
+ ret = wait_on_bit_cmd(&NFS_I(mapping->host)->flags, NFS_INO_FLUSHING,
+ nfs_wait_bit_killable, TASK_KILLABLE);
if (ret)
return ret;
diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index 91c4746fdab1..82444eb25785 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -1050,8 +1050,8 @@ int nfs_revalidate_mapping(struct inode *inode, struct address_space *mapping)
* the bit lock here if it looks like we're going to be doing that.
*/
for (;;) {
- ret = wait_on_bit(bitlock, NFS_INO_INVALIDATING,
- nfs_wait_bit_killable, TASK_KILLABLE);
+ ret = wait_on_bit_cmd(bitlock, NFS_INO_INVALIDATING,
+ nfs_wait_bit_killable, TASK_KILLABLE);
if (ret)
goto out;
spin_lock(&inode->i_lock);
diff --git a/fs/nfs/nfs4filelayoutdev.c b/fs/nfs/nfs4filelayoutdev.c
index efac602edb37..8a2346f4e598 100644
--- a/fs/nfs/nfs4filelayoutdev.c
+++ b/fs/nfs/nfs4filelayoutdev.c
@@ -783,7 +783,7 @@ nfs4_fl_select_ds_fh(struct pnfs_layout_segment *lseg, u32 j)
static void nfs4_wait_ds_connect(struct nfs4_pnfs_ds *ds)
{
might_sleep();
- wait_on_bit(&ds->ds_state, NFS4DS_CONNECTING,
+ wait_on_bit_cmd(&ds->ds_state, NFS4DS_CONNECTING,
nfs_wait_bit_killable, TASK_KILLABLE);
}
diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index 0deb32105ccf..1a7a8da9cde8 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -1251,8 +1251,8 @@ int nfs4_wait_clnt_recover(struct nfs_client *clp)
might_sleep();
atomic_inc(&clp->cl_count);
- res = wait_on_bit(&clp->cl_state, NFS4CLNT_MANAGER_RUNNING,
- nfs_wait_bit_killable, TASK_KILLABLE);
+ res = wait_on_bit_cmd(&clp->cl_state, NFS4CLNT_MANAGER_RUNNING,
+ nfs_wait_bit_killable, TASK_KILLABLE);
if (res)
goto out;
if (clp->cl_cons_state < 0)
diff --git a/fs/nfs/pagelist.c b/fs/nfs/pagelist.c
index 2ffebf2081ce..f369a74f2b31 100644
--- a/fs/nfs/pagelist.c
+++ b/fs/nfs/pagelist.c
@@ -258,12 +258,6 @@ void nfs_release_request(struct nfs_page *req)
kref_put(&req->wb_kref, nfs_free_request);
}
-static int nfs_wait_bit_uninterruptible(void *word)
-{
- io_schedule();
- return 0;
-}
-
/**
* nfs_wait_on_request - Wait for a request to complete.
* @req: request to wait upon.
@@ -274,9 +268,8 @@ static int nfs_wait_bit_uninterruptible(void *word)
int
nfs_wait_on_request(struct nfs_page *req)
{
- return wait_on_bit(&req->wb_flags, PG_BUSY,
- nfs_wait_bit_uninterruptible,
- TASK_UNINTERRUPTIBLE);
+ return wait_on_bit_io(&req->wb_flags, PG_BUSY,
+ TASK_UNINTERRUPTIBLE);
}
bool nfs_generic_pg_test(struct nfs_pageio_descriptor *desc, struct nfs_page *prev, struct nfs_page *req)
diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
index 4755858e37a0..c1536c503649 100644
--- a/fs/nfs/pnfs.c
+++ b/fs/nfs/pnfs.c
@@ -1898,7 +1898,7 @@ pnfs_layoutcommit_inode(struct inode *inode, bool sync)
if (test_and_set_bit(NFS_INO_LAYOUTCOMMITTING, &nfsi->flags)) {
if (!sync)
goto out;
- status = wait_on_bit_lock(&nfsi->flags,
+ status = wait_on_bit_lock_cmd(&nfsi->flags,
NFS_INO_LAYOUTCOMMITTING,
nfs_wait_bit_killable,
TASK_KILLABLE);
diff --git a/fs/nfs/write.c b/fs/nfs/write.c
index 9a3b6a4cd6b9..790f94514793 100644
--- a/fs/nfs/write.c
+++ b/fs/nfs/write.c
@@ -393,7 +393,7 @@ int nfs_writepages(struct address_space *mapping, struct writeback_control *wbc)
int err;
/* Stop dirtying of new pages while we sync */
- err = wait_on_bit_lock(bitlock, NFS_INO_FLUSHING,
+ err = wait_on_bit_lock_cmd(bitlock, NFS_INO_FLUSHING,
nfs_wait_bit_killable, TASK_KILLABLE);
if (err)
goto out_err;
@@ -1694,7 +1694,7 @@ int nfs_commit_inode(struct inode *inode, int how)
return error;
if (!may_wait)
goto out_mark_dirty;
- error = wait_on_bit(&NFS_I(inode)->flags,
+ error = wait_on_bit_cmd(&NFS_I(inode)->flags,
NFS_INO_COMMIT,
nfs_wait_bit_killable,
TASK_KILLABLE);
diff --git a/include/linux/writeback.h b/include/linux/writeback.h
index 021b8a319b9e..f1bc4fd07b2f 100644
--- a/include/linux/writeback.h
+++ b/include/linux/writeback.h
@@ -90,7 +90,6 @@ struct writeback_control {
* fs/fs-writeback.c
*/
struct bdi_writeback;
-int inode_wait(void *);
void writeback_inodes_sb(struct super_block *, enum wb_reason reason);
void writeback_inodes_sb_nr(struct super_block *, unsigned long nr,
enum wb_reason reason);
@@ -105,7 +104,7 @@ void inode_wait_for_writeback(struct inode *inode);
static inline void wait_on_inode(struct inode *inode)
{
might_sleep();
- wait_on_bit(&inode->i_state, __I_NEW, inode_wait, TASK_UNINTERRUPTIBLE);
+ wait_on_bit(&inode->i_state, __I_NEW, TASK_UNINTERRUPTIBLE);
}
/*
diff --git a/kernel/ptrace.c b/kernel/ptrace.c
index 1f4bcb3cc21c..23acf4b4e421 100644
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -28,12 +28,6 @@
#include <linux/compat.h>
-static int ptrace_trapping_sleep_fn(void *flags)
-{
- schedule();
- return 0;
-}
-
/*
* ptrace a task: make the debugger its new parent and
* move it to the ptrace list.
@@ -371,7 +365,7 @@ unlock_creds:
out:
if (!retval) {
wait_on_bit(&task->jobctl, JOBCTL_TRAPPING_BIT,
- ptrace_trapping_sleep_fn, TASK_UNINTERRUPTIBLE);
+ TASK_UNINTERRUPTIBLE);
proc_ptrace_connector(task, PTRACE_ATTACH);
}
diff --git a/mm/ksm.c b/mm/ksm.c
index 68710e80994a..33c8b475df65 100644
--- a/mm/ksm.c
+++ b/mm/ksm.c
@@ -1979,18 +1979,12 @@ void ksm_migrate_page(struct page *newpage, struct page *oldpage)
#endif /* CONFIG_MIGRATION */
#ifdef CONFIG_MEMORY_HOTREMOVE
-static int just_wait(void *word)
-{
- schedule();
- return 0;
-}
-
static void wait_while_offlining(void)
{
while (ksm_run & KSM_RUN_OFFLINE) {
mutex_unlock(&ksm_thread_mutex);
wait_on_bit(&ksm_run, ilog2(KSM_RUN_OFFLINE),
- just_wait, TASK_UNINTERRUPTIBLE);
+ TASK_UNINTERRUPTIBLE);
mutex_lock(&ksm_thread_mutex);
}
}
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 5e8663c194c1..29bd326d1a0f 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -1753,12 +1753,6 @@ static void hci_inq_req(struct hci_request *req, unsigned long opt)
hci_req_add(req, HCI_OP_INQUIRY, sizeof(cp), &cp);
}
-static int wait_inquiry(void *word)
-{
- schedule();
- return signal_pending(current);
-}
-
int hci_inquiry(void __user *arg)
{
__u8 __user *ptr = arg;
@@ -1809,7 +1803,7 @@ int hci_inquiry(void __user *arg)
/* Wait until Inquiry procedure finishes (HCI_INQUIRY flag is
* cleared). If it is interrupted by a signal, return -EINTR.
*/
- if (wait_on_bit(&hdev->flags, HCI_INQUIRY, wait_inquiry,
+ if (wait_on_bit(&hdev->flags, HCI_INQUIRY,
TASK_INTERRUPTIBLE))
return -EINTR;
}
diff --git a/security/keys/gc.c b/security/keys/gc.c
index d3222b6d7d59..9609a7f0faea 100644
--- a/security/keys/gc.c
+++ b/security/keys/gc.c
@@ -92,15 +92,6 @@ static void key_gc_timer_func(unsigned long data)
}
/*
- * wait_on_bit() sleep function for uninterruptible waiting
- */
-static int key_gc_wait_bit(void *flags)
-{
- schedule();
- return 0;
-}
-
-/*
* Reap keys of dead type.
*
* We use three flags to make sure we see three complete cycles of the garbage
@@ -123,7 +114,7 @@ void key_gc_keytype(struct key_type *ktype)
schedule_work(&key_gc_work);
kdebug("sleep");
- wait_on_bit(&key_gc_flags, KEY_GC_REAPING_KEYTYPE, key_gc_wait_bit,
+ wait_on_bit(&key_gc_flags, KEY_GC_REAPING_KEYTYPE,
TASK_UNINTERRUPTIBLE);
key_gc_dead_keytype = NULL;
diff --git a/security/keys/request_key.c b/security/keys/request_key.c
index 381411941cc1..26a94f18af94 100644
--- a/security/keys/request_key.c
+++ b/security/keys/request_key.c
@@ -21,24 +21,6 @@
#define key_negative_timeout 60 /* default timeout on a negative key's existence */
-/*
- * wait_on_bit() sleep function for uninterruptible waiting
- */
-static int key_wait_bit(void *flags)
-{
- schedule();
- return 0;
-}
-
-/*
- * wait_on_bit() sleep function for interruptible waiting
- */
-static int key_wait_bit_intr(void *flags)
-{
- schedule();
- return signal_pending(current) ? -ERESTARTSYS : 0;
-}
-
/**
* complete_request_key - Complete the construction of a key.
* @cons: The key construction record.
@@ -592,10 +574,9 @@ int wait_for_key_construction(struct key *key, bool intr)
int ret;
ret = wait_on_bit(&key->flags, KEY_FLAG_USER_CONSTRUCT,
- intr ? key_wait_bit_intr : key_wait_bit,
intr ? TASK_INTERRUPTIBLE : TASK_UNINTERRUPTIBLE);
- if (ret < 0)
- return ret;
+ if (ret)
+ return -ERESTARTSYS;
if (test_bit(KEY_FLAG_NEGATIVE, &key->flags)) {
smp_rmb();
return key->type_data.reject_error;
On Wed, Apr 30, 2014 at 12:29:26PM +1000, NeilBrown wrote:
> If you think it is a good cleanup I'll post a proper patch with all the right
> Cc:s.
Yeah, its a good cleanup. Thanks!
> +static inline int
> +wait_on_bit(void *word, int bit, unsigned mode)
> +{
> + if (!test_bit(bit, word))
> + return 0;
> + return out_of_line_wait_on_bit(word, bit,
> + bit_wait,
> + mode & 65535);
> +}
> +
> +static inline int
> +wait_on_bit_io(void *word, int bit, unsigned mode)
> +{
> + if (!test_bit(bit, word))
> + return 0;
> + return out_of_line_wait_on_bit(word, bit,
> + bit_wait_io,
> + mode & 65535);
> +}
That actually fits on one <80 line. Also, where does the 16 bit mask
come from? On which, I would write that in hex, 0xFFFF is slightly
easier to recognise as (1<<16)-1.
On Wed, Apr 30, 2014 at 09:31:54AM +0200, Peter Zijlstra wrote:
> On Wed, Apr 30, 2014 at 12:29:26PM +1000, NeilBrown wrote:
> > If you think it is a good cleanup I'll post a proper patch with all the right
> > Cc:s.
>
> Yeah, its a good cleanup. Thanks!
Also, please add Oleg to the Cc, he was involved in the last big wait
cleanup too.
On Wed, 30 Apr 2014 09:31:54 +0200 Peter Zijlstra <[email protected]>
wrote:
> On Wed, Apr 30, 2014 at 12:29:26PM +1000, NeilBrown wrote:
> > If you think it is a good cleanup I'll post a proper patch with all the right
> > Cc:s.
>
> Yeah, its a good cleanup. Thanks!
>
> > +static inline int
> > +wait_on_bit(void *word, int bit, unsigned mode)
> > +{
> > + if (!test_bit(bit, word))
> > + return 0;
> > + return out_of_line_wait_on_bit(word, bit,
> > + bit_wait,
> > + mode & 65535);
> > +}
> > +
> > +static inline int
> > +wait_on_bit_io(void *word, int bit, unsigned mode)
> > +{
> > + if (!test_bit(bit, word))
> > + return 0;
> > + return out_of_line_wait_on_bit(word, bit,
> > + bit_wait_io,
> > + mode & 65535);
> > +}
>
> That actually fits on one <80 line. Also, where does the 16 bit mask
> come from? On which, I would write that in hex, 0xFFFF is slightly
> easier to recognise as (1<<16)-1.
That is a hangover from an earlier attempt which didn't work. Thanks for
catching it.
I'll refresh and do some basic testing tomorrow and send it out, including to
Oleg.
Thanks,
NeilBrown