linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] Improve wait_on_bit interface.
@ 2014-07-07  5:16 NeilBrown
  2014-07-07  5:16 ` [PATCH 2/2] SCHED: allow wait_on_bit_action functions to support a timeout NeilBrown
  2014-07-07  5:16 ` [PATCH 1/2] SCHED: remove proliferation of wait_on_bit action functions NeilBrown
  0 siblings, 2 replies; 6+ messages in thread
From: NeilBrown @ 2014-07-07  5:16 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: linux-cifs, linux-nfs, Peter Zijlstra, Oleg Nesterov,
	linux-kernel, Steve French, David Howells, Ingo Molnar,
	Steven Whitehouse

Hi Linus
 (hoping to get through your spam filter :-)

 I wonder if you would consider applying these patches directly?

 I originally sent them to Peter Zijlstra who was happy with them
 and they went into "tip" for a while
   http://lkml.iu.edu/hypermail/linux/kernel/1405.2/01678.html
 however other code (nfs and cifs) added new users of the old
 wait_on_bit interface causing conflicts, so Ingo had to remove them.
 At that time I suggested:

> How about you drop my patch for now, we wait for -rc1 to come out, then I
> submit a new version against -rc1 and we get that into -rc2.
> That should minimise such conflicts.
> 
> Does that work for you?

and Ingo replied

> Sure, that sounds like a good approach, if Linus doesn't object.
>   

which I took to mean that I could forward revised patches though the
'tip' tree at that time.
I did resend after rc2 (missed rc1 - on leave) and have heard nothing
from Ingo since despite a ping.  So maybe he meant I should submit
them directly to you.

I would really like at least the first of these to go in before 3.16
else other people could add calls using the old interface and cause
the same problems again.  Having both of them go in would make me very
happy as I could then submit the change to NFS which needs the new
wait_on_bit() functionality to device loop-back NFS deadlocks.

Thanks a lot,
NeilBrown


---

NeilBrown (2):
      SCHED: remove proliferation of wait_on_bit action functions.
      SCHED: allow wait_on_bit_action functions to support a timeout.


 Documentation/filesystems/caching/operations.txt |    2 
 drivers/md/dm-bufio.c                            |   41 ++-----
 drivers/md/dm-snap.c                             |   10 --
 drivers/media/usb/dvb-usb-v2/dvb_usb_core.c      |   12 --
 fs/btrfs/extent_io.c                             |   10 --
 fs/buffer.c                                      |   11 --
 fs/cifs/connect.c                                |   10 --
 fs/cifs/file.c                                   |    9 --
 fs/cifs/inode.c                                  |    6 +
 fs/cifs/misc.c                                   |    2 
 fs/fs-writeback.c                                |    3 -
 fs/fscache/cookie.c                              |    7 +
 fs/fscache/internal.h                            |    2 
 fs/fscache/main.c                                |   18 ---
 fs/fscache/page.c                                |    4 -
 fs/gfs2/glock.c                                  |   25 ----
 fs/gfs2/lock_dlm.c                               |    8 -
 fs/gfs2/ops_fstype.c                             |   11 --
 fs/gfs2/recovery.c                               |    8 -
 fs/gfs2/super.c                                  |    8 -
 fs/inode.c                                       |    7 -
 fs/jbd2/transaction.c                            |   10 --
 fs/nfs/file.c                                    |    4 -
 fs/nfs/filelayout/filelayoutdev.c                |    4 -
 fs/nfs/inode.c                                   |    6 +
 fs/nfs/internal.h                                |    2 
 fs/nfs/nfs4state.c                               |    4 -
 fs/nfs/pagelist.c                                |   14 +-
 fs/nfs/pnfs.c                                    |    2 
 fs/nfs/write.c                                   |    4 -
 include/linux/sunrpc/sched.h                     |    2 
 include/linux/wait.h                             |  125 +++++++++++++++++++++-
 include/linux/writeback.h                        |    3 -
 kernel/ptrace.c                                  |    8 -
 kernel/sched/wait.c                              |   30 ++++-
 mm/filemap.c                                     |   20 +---
 mm/ksm.c                                         |    8 -
 net/bluetooth/hci_core.c                         |    8 -
 net/sunrpc/sched.c                               |    4 -
 security/keys/gc.c                               |   11 --
 security/keys/request_key.c                      |   23 ----
 41 files changed, 214 insertions(+), 292 deletions(-)

-- 
Signature


^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH] SCHED: allow wait_on_bit_action functions to support a timeout.
@ 2014-05-01  2:41 NeilBrown
  2014-05-19 13:08 ` [tip:sched/core] sched: Allow " tip-bot for NeilBrown
  0 siblings, 1 reply; 6+ messages in thread
From: NeilBrown @ 2014-05-01  2:41 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar
  Cc: Trond Myklebust, J. Bruce Fields, linux-kernel, linux-nfs

[-- Attachment #1: Type: text/plain, Size: 10343 bytes --]


[[ This patch depends on the previously posted:
    [PATCH] SCHED: remove proliferation of wait_on_bit action functions.

   This version is much smaller then the previous and better tested.

   I'm hoping it too will go though the scheduler tree and the the
   NFS changes won't cause too many conflicts...

]]

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.  No current action functions 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.

While wait_on_bit_timeout() would be a cleaner interface, it will not
meet my need.  I need the timeout to be sensitive to the state of
the connection with the server, which could change.  So I need to
use an 'action' interface.

Signed-off-by: NeilBrown <neilb@suse.de>

diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index cd6e656d839e..5c19408c8345 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 dd8bfc2e2464..9bafea8ecc54 100644
--- a/fs/nfs/internal.h
+++ b/fs/nfs/internal.h
@@ -342,7 +342,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 f369a74f2b31..3f7c25736659 100644
--- a/fs/nfs/pagelist.c
+++ b/fs/nfs/pagelist.c
@@ -112,7 +112,7 @@ __nfs_iocounter_wait(struct nfs_io_counter *c)
 		set_bit(NFS_IO_INPROGRESS, &c->flags);
 		if (atomic_read(&c->io_count) == 0)
 			break;
-		ret = nfs_wait_bit_killable(&c->flags);
+		ret = nfs_wait_bit_killable(&q.key);
 	} while (atomic_read(&c->io_count) != 0);
 	finish_wait(wq, &q.wait);
 	return ret;
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 438dc6044587..162cbcde9dae 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);
 
@@ -855,8 +856,8 @@ int wake_bit_function(wait_queue_t *wait, unsigned mode, int sync, void *key);
 	} while (0)
 
 
-extern int bit_wait(void *);
-extern int bit_wait_io(void *);
+extern int bit_wait(struct wait_bit_key *);
+extern int bit_wait_io(struct wait_bit_key *);
 
 /**
  * wait_on_bit - wait for a bit to be cleared
@@ -925,7 +926,7 @@ wait_on_bit_io(void *word, int bit, unsigned mode)
  * on that signal.
  */
 static inline int
-wait_on_bit_action(void *word, int bit, int (*action)(void *), unsigned mode)
+wait_on_bit_action(void *word, int bit, int (*action)(struct wait_bit_key *), unsigned mode)
 {
 	if (!test_bit(bit, word))
 		return 0;
@@ -1000,7 +1001,7 @@ wait_on_bit_lock_io(void *word, int bit, unsigned mode)
  * the @mode allows that signal to wake the process.
  */
 static inline int
-wait_on_bit_lock_action(void *word, int bit, int (*action)(void *), unsigned mode)
+wait_on_bit_lock_action(void *word, int bit, int (*action)(struct wait_bit_key *), unsigned mode)
 {
 	if (!test_and_set_bit(bit, word))
 		return 0;
diff --git a/kernel/sched/wait.c b/kernel/sched/wait.c
index 0c0795002f56..738fa685fd3d 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);
@@ -503,7 +503,7 @@ void wake_up_atomic_t(atomic_t *p)
 }
 EXPORT_SYMBOL(wake_up_atomic_t);
 
-__sched int bit_wait(void *word)
+__sched int bit_wait(struct wait_bit_key *word)
 {
 	if (signal_pending_state(current->state, current))
 		return 1;
@@ -512,7 +512,7 @@ __sched int bit_wait(void *word)
 }
 EXPORT_SYMBOL(bit_wait);
 
-__sched int bit_wait_io(void *word)
+__sched int bit_wait_io(struct wait_bit_key *word)
 {
 	if (signal_pending_state(current->state, current))
 		return 1;
diff --git a/net/sunrpc/sched.c b/net/sunrpc/sched.c
index 25578afe1548..7b9a673c6adb 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;

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2014-07-16 19:26 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-07  5:16 [PATCH 0/2] Improve wait_on_bit interface NeilBrown
2014-07-07  5:16 ` [PATCH 2/2] SCHED: allow wait_on_bit_action functions to support a timeout NeilBrown
2014-07-16 19:25   ` [tip:sched/core] sched: Allow wait_on_bit_action() " tip-bot for NeilBrown
2014-07-07  5:16 ` [PATCH 1/2] SCHED: remove proliferation of wait_on_bit action functions NeilBrown
2014-07-16 19:25   ` [tip:sched/core] sched: Remove proliferation of wait_on_bit() " tip-bot for NeilBrown
  -- strict thread matches above, loose matches on Subject: below --
2014-05-01  2:41 [PATCH] SCHED: allow wait_on_bit_action functions to support a timeout NeilBrown
2014-05-19 13:08 ` [tip:sched/core] sched: Allow " tip-bot for NeilBrown

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).