wait: add wait_event_lock_irq() interface
Subsystems (3)
PATCH v3 1/2 wait: add wait_event_lock_irq() interface
New wait_event{_interruptible}_lock_irq{_cmd} macros added. This commit moves the private wait_event_lock_irq() macro from MD to regular wait includes, introduces new macro wait_event_lock_irq_cmd() instead of using the old method with omitting cmd parameter which is ugly and makes a use of new macros in the MD. It also introduces the _interruptible_ variant.
The use of new interface is when one have a special lock to protect data structures used in the condition, or one also needs to invoke "cmd" before putting it to sleep.
All new macros are expected to be called with the lock taken. The lock is released before sleep and is reacquired afterwards. We will leave the macro with the lock held.
Note to DM: IMO this should also fix theoretical race on waitqueue while using simultaneously wait_event_lock_irq() and wait_event() because of lack of locking around current state setting and wait queue removal.
---
v2: add interruptible variant, swap cmd and schedule, use prepare_to_wait
v3: swap cmd and schedule so we call cmd before schedule
drivers/md/md.c | 2 +- drivers/md/md.h | 26 -------- drivers/md/raid1.c | 15 ++--- drivers/md/raid10.c | 15 ++--- drivers/md/raid5.c | 12 ++-- include/linux/wait.h | 164 ++++++++++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 184 insertions(+), 50 deletions(-)
drivers/md/md.c +1 -1
@@ -452,7 +452,7 @@ void md_flush_request(struct mddev *mddev, struct bio *bio)spin_lock_irq (&mddev->write_lock);wait_event_lock_irq (mddev->sb_wait, !mddev->flush_bio, - mddev->write_lock, /*nothing*/); + mddev->write_lock); mddev->flush_bio = bio;spin_unlock_irq (&mddev->write_lock);
drivers/md/md.h +0 -26
@@ -551,32 +551,6 @@ struct md_thread {#define THREAD_WAKEUP 0 -#define __wait_event_lock_irq (wq, condition, lock, cmd) \ -do { \ - wait_queue_t __wait; \ - init_waitqueue_entry(&__wait, current); \ - \ - add_wait_queue(&wq, &__wait); \ - for (;;) { \ - set_current_state(TASK_UNINTERRUPTIBLE); \ - if (condition) \ - break; \ - spin_unlock_irq(&lock); \ - cmd; \ - schedule(); \ - spin_lock_irq(&lock); \ - } \ - current->state = TASK_RUNNING; \ - remove_wait_queue(&wq, &__wait); \ -} while (0) - -#define wait_event_lock_irq (wq, condition, lock, cmd) \ -do { \ - if (condition) \ - break; \ - __wait_event_lock_irq(wq, condition, lock, cmd); \ -} while (0) -static inline void safe_put_page (struct page *p) {if (p)put_page (p);
drivers/md/raid1.c +7 -8
@@ -822,7 +822,7 @@ static void raise_barrier(struct r1conf *conf)/* Wait until no block IO is waiting */ wait_event_lock_irq (conf->wait_barrier, !conf->nr_waiting, - conf->resync_lock, ); + conf->resync_lock);/* block any new IO from starting */ conf->barrier++;
@@ -830,7 +830,7 @@ static void raise_barrier(struct r1conf *conf)/* Now wait for all pending IO to complete */ wait_event_lock_irq (conf->wait_barrier, !conf->nr_pending && conf->barrier <RESYNC_DEPTH , - conf->resync_lock, ); + conf->resync_lock);spin_unlock_irq (&conf->resync_lock); }
@@ -864,8 +864,7 @@ static void wait_barrier(struct r1conf *conf) (conf->nr_pending && current->bio_list && !bio_list_empty (current->bio_list)), - conf->resync_lock, - ); + conf->resync_lock); conf->nr_waiting--; } conf->nr_pending++;
@@ -898,10 +897,10 @@ static void freeze_array(struct r1conf *conf)spin_lock_irq (&conf->resync_lock); conf->barrier++; conf->nr_waiting++; -wait_event_lock_irq (conf->wait_barrier, - conf->nr_pending == conf->nr_queued+1 , - conf->resync_lock, -flush_pending_writes (conf)); +wait_event_lock_irq_cmd (conf->wait_barrier, + conf->nr_pending == conf->nr_queued+1 , + conf->resync_lock, +flush_pending_writes (conf));spin_unlock_irq (&conf->resync_lock); }static void unfreeze_array (struct r1conf *conf)
drivers/md/raid10.c +7 -8
@@ -952,7 +952,7 @@ static void raise_barrier(struct r10conf *conf, int force)/* Wait until no block IO is waiting (unless 'force') */ wait_event_lock_irq (conf->wait_barrier, force || !conf->nr_waiting, - conf->resync_lock, ); + conf->resync_lock);/* block any new IO from starting */ conf->barrier++;
@@ -960,7 +960,7 @@ static void raise_barrier(struct r10conf *conf, int force)/* Now wait for all pending IO to complete */ wait_event_lock_irq (conf->wait_barrier, !conf->nr_pending && conf->barrier <RESYNC_DEPTH , - conf->resync_lock, ); + conf->resync_lock);spin_unlock_irq (&conf->resync_lock); }
@@ -993,8 +993,7 @@ static void wait_barrier(struct r10conf *conf) (conf->nr_pending && current->bio_list && !bio_list_empty (current->bio_list)), - conf->resync_lock, - ); + conf->resync_lock); conf->nr_waiting--; } conf->nr_pending++;
@@ -1027,10 +1026,10 @@ static void freeze_array(struct r10conf *conf)spin_lock_irq (&conf->resync_lock); conf->barrier++; conf->nr_waiting++; -wait_event_lock_irq (conf->wait_barrier, - conf->nr_pending == conf->nr_queued+1 , - conf->resync_lock, -flush_pending_writes (conf)); +wait_event_lock_irq_cmd (conf->wait_barrier, + conf->nr_pending == conf->nr_queued+1 , + conf->resync_lock, +flush_pending_writes (conf));spin_unlock_irq (&conf->resync_lock); }
drivers/md/raid5.c +5 -7
@@ -466,7 +466,7 @@ get_active_stripe(struct r5conf *conf, sector_t sector, do { wait_event_lock_irq(conf->wait_for_stripe, conf->quiesce == 0 || noquiesce, - conf->device_lock, /* nothing */); + conf->device_lock); sh = __find_stripe(conf, sector, conf->generation - previous); if (!sh) { if (!conf->inactive_blocked)
@@ -480,8 +480,7 @@ get_active_stripe(struct r5conf *conf, sector_t sector, (atomic_read (&conf->active_stripes) < (conf->max_nr_stripes *3 /4 ) || !conf->inactive_blocked), - conf->device_lock, - ); + conf->device_lock); conf->inactive_blocked =0 ; }else init_stripe (sh, sector, previous);
@@ -1646,8 +1645,7 @@ static int resize_stripes(struct r5conf *conf, int newsize)spin_lock_irq (&conf->device_lock);wait_event_lock_irq (conf->wait_for_stripe, !list_empty (&conf->inactive_list), - conf->device_lock, - ); + conf->device_lock); osh =get_free_stripe (conf);spin_unlock_irq (&conf->device_lock);atomic_set (&nsh->count,1 );
@@ -4003,7 +4001,7 @@ static int chunk_aligned_read(struct mddev *mddev, struct bio * raid_bio)spin_lock_irq (&conf->device_lock);wait_event_lock_irq (conf->wait_for_stripe, conf->quiesce ==0 , - conf->device_lock, /* nothing */); + conf->device_lock);atomic_inc (&conf->active_aligned_reads);spin_unlock_irq (&conf->device_lock);
@@ -6095,7 +6093,7 @@ static void raid5_quiesce(struct mddev *mddev, int state)wait_event_lock_irq (conf->wait_for_stripe,atomic_read (&conf->active_stripes) ==0 &&atomic_read (&conf->active_aligned_reads) ==0 , - conf->device_lock, /* nothing */); + conf->device_lock); conf->quiesce =1 ;spin_unlock_irq (&conf->device_lock);/* allow reshape to continue */
include/linux/wait.h +164 -0
170 lines not on this page · raw message
Signature · 1 line
1.7.7.6
PATCH v6 2/2 loop: Limit the number of requests in the bio list
Currently there is not limitation of number of requests in the loop bio list. This can lead into some nasty situations when the caller spawns tons of bio requests taking huge amount of memory. This is even more obvious with discard where blkdev_issue_discard() will submit all bios for the range and wait for them to finish afterwards. On really big loop devices and slow backing file system this can lead to OOM situation as reported by Dave Chinner.
With this patch we will wait in loop_make_request() if the number of bios in the loop bio list would exceed 'nr_congestion_on'. We'll wake up the process as we process the bios form the list. Some threshold hysteresis is in place to avoid high frequency oscillation.
---
v2: add threshold hysteresis
v3: Wait uninterruptible, use nr_congestion_off/on
v4: use wait_event_lock_irq()
v5: no change
v6: no change
drivers/block/loop.c | 10 ++++++++++ include/linux/loop.h | 3 +++ 2 files changed, 13 insertions(+), 0 deletions(-)
drivers/block/loop.c +10 -0
@@ -463,6 +463,7 @@ out:*/ static void loop_add_bio (struct loop_device *lo,struct bio *bio) { + lo->lo_bio_count++;bio_list_add (&lo->lo_bio_list, bio); }
@@ -471,6 +472,7 @@ static void loop_add_bio(struct loop_device *lo, struct bio *bio)*/ static struct bio *loop_get_bio (struct loop_device *lo) { + lo->lo_bio_count--;return bio_list_pop (&lo->lo_bio_list); }
@@ -489,6 +491,10 @@ static void loop_make_request(struct request_queue *q, struct bio *old_bio) goto out;if (unlikely (rw ==WRITE && (lo->lo_flags &LO_FLAGS_READ_ONLY ))) goto out; +if (lo->lo_bio_count >= q->nr_congestion_on) +wait_event_lock_irq (lo->lo_req_wait, + lo->lo_bio_count < q->nr_congestion_off, + lo->lo_lock);loop_add_bio (lo, old_bio);wake_up (&lo->lo_event);spin_unlock_irq (&lo->lo_lock);
@@ -546,6 +552,8 @@ static int loop_thread(void *data)continue ;spin_lock_irq (&lo->lo_lock); bio =loop_get_bio (lo); +if (lo->lo_bio_count < lo->lo_queue->nr_congestion_off) +wake_up (&lo->lo_req_wait);spin_unlock_irq (&lo->lo_lock);BUG_ON (!bio);
@@ -873,6 +881,7 @@ static int loop_set_fd(struct loop_device *lo, fmode_t mode, lo->transfer = transfer_none; lo->ioctl =NULL ; lo->lo_sizelimit =0 ; + lo->lo_bio_count =0 ; lo->old_gfp_mask =mapping_gfp_mask (mapping);mapping_set_gfp_mask (mapping, lo->old_gfp_mask & ~(__GFP_IO|__GFP_FS));
@@ -1673,6 +1682,7 @@ static int loop_add(struct loop_device **l, int i) lo->lo_number = i; lo->lo_thread =NULL ;init_waitqueue_head (&lo->lo_event); +init_waitqueue_head (&lo->lo_req_wait);spin_lock_init (&lo->lo_lock); disk->major =LOOP_MAJOR ; disk->first_minor = i << part_shift;
include/linux/loop.h +3 -0
@@ -53,10 +53,13 @@ struct loop_device {spinlock_t lo_lock;struct bio_list lo_bio_list; +unsigned int lo_bio_count;int lo_state;struct mutex lo_ctl_mutex;struct task_struct *lo_thread;wait_queue_head_t lo_event; +/* wait queue for incoming requests */ +wait_queue_head_t lo_req_wait;struct request_queue *lo_queue;struct gendisk *lo_disk;
Signature · 1 line
1.7.7.6
Re: [PATCH 2/2 v6] loop: Limit the number of requests in the bio list
Quoted 8 lines · Lukas Czerner
Currently there is not limitation of number of requests in the loop bio list. This can lead into some nasty situations when the caller spawns tons of bio requests taking huge amount of memory. This is even more obvious with discard where
blkdev_issue_discard()will submit all bios for the range and wait for them to finish afterwards. On really big loop devices and slow backing file system this can lead to OOM situation as reported by Dave Chinner.
Quoted 7 lines
With this patch we will wait in
loop_make_request()if the number of bios in the loop bio list would exceed 'nr_congestion_on'. We'll wake up the process as we process the bios form the list. Some threshold hysteresis is in place to avoid high frequency oscillation.Signed-off-by Lukas Czerner <[email protected]>Reported-by Dave Chinner <[email protected]>
Lukas, please carry forward acked-by tags when there is no change so I don't have to keep acking.
Thanks,
Jeff
Quoted 10 lines · On Fri, 30 Nov 2012 11:42:40 +0100 Lukas Czerner <[email protected]> wrote:
New wait_event{_interruptible}_lock_irq{_cmd} macros added. This commit moves the private
wait_event_lock_irq()macro from MD to regular wait includes, introduces new macrowait_event_lock_irq_cmd()instead of using the old method with omitting cmd parameter which is ugly and makes a use of new macros in the MD. It also introduces the _interruptible_ variant.The use of new interface is when one have a special lock to protect data structures used in the condition, or one also needs to invoke "cmd" before putting it to sleep.
Quoted 7 lines
All new macros are expected to be called with the lock taken. The lock is released before sleep and is reacquired afterwards. We will leave the macro with the lock held.
Note to DM: IMO this should also fix theoretical race on waitqueue while using simultaneously
wait_event_lock_irq()andwait_event()because of lack of locking around current state setting and wait queue removal.
Does this fix the sparse warning which Fengguang just sent us?
Quoted 25 lines · Andrew Morton
Date: Fri, 30 Nov 2012 02:48:42 -0800
From: Andrew Morton <[email protected]>
To: Lukas Czerner <[email protected]>
Cc: [email protected], [email protected], [email protected],
[email protected], Neil Brown <[email protected]>,
David Howells <[email protected]>, Ingo Molnar <[email protected]>, Peter Zijlstra <[email protected]>
Subject: Re: [PATCH 1/2 v3] wait: addwait_event_lock_irq()interfaceQuoted 14 lines · On Fri, 30 Nov 2012 11:42:40 +0100 Lukas Czerner <[email protected]> wrote:
New wait_event{_interruptible}_lock_irq{_cmd} macros added. This commit moves the private
wait_event_lock_irq()macro from MD to regular wait includes, introduces new macrowait_event_lock_irq_cmd()instead of using the old method with omitting cmd parameter which is ugly and makes a use of new macros in the MD. It also introduces the _interruptible_ variant.The use of new interface is when one have a special lock to protect data structures used in the condition, or one also needs to invoke "cmd" before putting it to sleep.
All new macros are expected to be called with the lock taken. The lock is released before sleep and is reacquired afterwards. We will leave the macro with the lock held.
Quoted 5 lines
Quoted 3 lines
Note to DM: IMO this should also fix theoretical race on waitqueue while using simultaneously
wait_event_lock_irq()andwait_event()because of lack of locking around current state setting and wait queue removal.Does this fix the sparse warning which Fengguang just sent us?
Which report from Fengguang do you have in mind ? I do not see any on linux-kernel today.
/me going to see what spare reports
-Lukas
Quoted 38 lines · On Fri, 30 Nov 2012 12:36:35 +0100 (CET) Lukáš Czerner <[email protected]>…
Quoted 25 lines · Andrew Morton
Date: Fri, 30 Nov 2012 02:48:42 -0800
From: Andrew Morton <[email protected]>
To: Lukas Czerner <[email protected]>
Cc: [email protected], [email protected], [email protected],
[email protected], Neil Brown <[email protected]>,
David Howells <[email protected]>, Ingo Molnar <[email protected]>, Peter Zijlstra <[email protected]>
Subject: Re: [PATCH 1/2 v3] wait: addwait_event_lock_irq()interfaceQuoted 14 lines · On Fri, 30 Nov 2012 11:42:40 +0100 Lukas Czerner <[email protected]> wrote:
New wait_event{_interruptible}_lock_irq{_cmd} macros added. This commit moves the private
wait_event_lock_irq()macro from MD to regular wait includes, introduces new macrowait_event_lock_irq_cmd()instead of using the old method with omitting cmd parameter which is ugly and makes a use of new macros in the MD. It also introduces the _interruptible_ variant.The use of new interface is when one have a special lock to protect data structures used in the condition, or one also needs to invoke "cmd" before putting it to sleep.
All new macros are expected to be called with the lock taken. The lock is released before sleep and is reacquired afterwards. We will leave the macro with the lock held.
Quoted 5 lines
Quoted 3 lines
Note to DM: IMO this should also fix theoretical race on waitqueue while using simultaneously
wait_event_lock_irq()andwait_event()because of lack of locking around current state setting and wait queue removal.Does this fix the sparse warning which Fengguang just sent us?
Which report from Fengguang do you have in mind ? I do not see any on linux-kernel today.
/me going to see what spare reports
Quoted 29 lines · On Fri, 30 Nov 2012 16:30:24 +0800 kbuild test robot <[email protected]>…
tree: git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git akpm head: cfb65dadcd079ad4547407a1584bc6b96bd48bb3 commit: 2b29cdb6f98c86a1da4ec5335d6247392b7c6551 [35/476] wait: add wait_event_lock_irq() interfacesparse warnings:
+
drivers/block/drbd/drbd_int.h:2339:9: sparse: preprocessor token__wait_event_lock_irqredefinedinclude/linux/wait.h:554:9: this was the original definition
+drivers/block/drbd/drbd_int.h:2358:9: sparse: preprocessor tokenwait_event_lock_irqredefinedinclude/linux/wait.h:621:9: this was the original definitiondrivers/block/drbd/drbd_interval.h:12:63: sparse: dubious one-bit signed bitfielddrivers/block/drbd/drbd_interval.h:13:22: sparse: dubious one-bit signed bitfielddrivers/block/drbd/drbd_int.h:903:39: sparse: attribute 'require_context': unknown attributedrivers/block/drbd/drbd_int.h:1123:69: sparse: attribute 'require_context': unknown attributedrivers/block/drbd/drbd_int.h:1124:70: sparse: attribute 'require_context': unknown attributedrivers/block/drbd/drbd_int.h:1125:59: sparse: attribute 'require_context': unknown attributedrivers/block/drbd/drbd_int.h:1126:63: sparse: attribute 'require_context': unknown attributedrivers/block/drbd/drbd_int.h:1127:60: sparse: attribute 'require_context': unknown attributedrivers/block/drbd/drbd_int.h:1128:71: sparse: attribute 'require_context': unknown attributedrivers/block/drbd/drbd_int.h:1129:65: sparse: attribute 'require_context': unknown attributedrivers/block/drbd/drbd_int.h:1130:66: sparse: attribute 'require_context': unknown attributedrivers/block/drbd/drbd_int.h:1335:74: sparse: attribute 'require_context': unknown attributedrivers/block/drbd/drbd_int.h:1336:50: sparse: attribute 'require_context': unknown attributedrivers/block/drbd/drbd_int.h:1338:51: sparse: attribute 'require_context': unknown attributedrivers/block/drbd/drbd_int.h:1339:58: sparse: attribute 'require_context': unknown attributedrivers/block/drbd/drbd_int.h:1340:54: sparse: attribute 'require_context': unknown attributedrivers/block/drbd/drbd_int.h:1341:62: sparse: attribute 'require_context': unknown attributedrivers/block/drbd/drbd_int.h:1435:92: sparse: attribute 'require_context': unknown attribute
Quoted 43 lines · Andrew Morton
Date: Fri, 30 Nov 2012 12:13:08 -0800
From: Andrew Morton <[email protected]>
To: Lukáš Czerner <[email protected]>
Cc: [email protected], [email protected], [email protected],
[email protected], Neil Brown <[email protected]>
Subject: Re: [PATCH 1/2 v3] wait: addwait_event_lock_irq()interfaceQuoted 33 lines · On Fri, 30 Nov 2012 12:36:35 +0100 (CET) Lukáš Czerner <[email protected]>…
Quoted 30 lines · Andrew Morton
Date: Fri, 30 Nov 2012 02:48:42 -0800
From: Andrew Morton <[email protected]>
To: Lukas Czerner <[email protected]>
Cc: [email protected], [email protected], [email protected],
[email protected], Neil Brown <[email protected]>,
David Howells <[email protected]>, Ingo Molnar <[email protected]>, Peter Zijlstra <[email protected]>
Subject: Re: [PATCH 1/2 v3] wait: addwait_event_lock_irq()interfaceQuoted 10 lines · On Fri, 30 Nov 2012 11:42:40 +0100 Lukas Czerner <[email protected]> wrote:
New wait_event{_interruptible}_lock_irq{_cmd} macros added. This commit moves the private
wait_event_lock_irq()macro from MD to regular wait includes, introduces new macrowait_event_lock_irq_cmd()instead of using the old method with omitting cmd parameter which is ugly and makes a use of new macros in the MD. It also introduces the _interruptible_ variant.The use of new interface is when one have a special lock to protect data structures used in the condition, or one also needs to invoke "cmd" before putting it to sleep.
Quoted 7 lines
All new macros are expected to be called with the lock taken. The lock is released before sleep and is reacquired afterwards. We will leave the macro with the lock held.
Note to DM: IMO this should also fix theoretical race on waitqueue while using simultaneously
wait_event_lock_irq()andwait_event()because of lack of locking around current state setting and wait queue removal.Does this fix the sparse warning which Fengguang just sent us?
Quoted 39 lines
Quoted 5 lines
Which report from Fengguang do you have in mind ? I do not see any on linux-kernel today.
/me going to see what spare reports
Quoted 29 lines · On Fri, 30 Nov 2012 16:30:24 +0800 kbuild test robot <[email protected]>…
tree: git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git akpm head: cfb65dadcd079ad4547407a1584bc6b96bd48bb3 commit: 2b29cdb6f98c86a1da4ec5335d6247392b7c6551 [35/476] wait: add wait_event_lock_irq() interfacesparse warnings:
+
drivers/block/drbd/drbd_int.h:2339:9: sparse: preprocessor token__wait_event_lock_irqredefinedinclude/linux/wait.h:554:9: this was the original definition
+drivers/block/drbd/drbd_int.h:2358:9: sparse: preprocessor tokenwait_event_lock_irqredefinedinclude/linux/wait.h:621:9: this was the original definitiondrivers/block/drbd/drbd_interval.h:12:63: sparse: dubious one-bit signed bitfielddrivers/block/drbd/drbd_interval.h:13:22: sparse: dubious one-bit signed bitfielddrivers/block/drbd/drbd_int.h:903:39: sparse: attribute 'require_context': unknown attributedrivers/block/drbd/drbd_int.h:1123:69: sparse: attribute 'require_context': unknown attributedrivers/block/drbd/drbd_int.h:1124:70: sparse: attribute 'require_context': unknown attributedrivers/block/drbd/drbd_int.h:1125:59: sparse: attribute 'require_context': unknown attributedrivers/block/drbd/drbd_int.h:1126:63: sparse: attribute 'require_context': unknown attributedrivers/block/drbd/drbd_int.h:1127:60: sparse: attribute 'require_context': unknown attributedrivers/block/drbd/drbd_int.h:1128:71: sparse: attribute 'require_context': unknown attributedrivers/block/drbd/drbd_int.h:1129:65: sparse: attribute 'require_context': unknown attributedrivers/block/drbd/drbd_int.h:1130:66: sparse: attribute 'require_context': unknown attributedrivers/block/drbd/drbd_int.h:1335:74: sparse: attribute 'require_context': unknown attributedrivers/block/drbd/drbd_int.h:1336:50: sparse: attribute 'require_context': unknown attributedrivers/block/drbd/drbd_int.h:1338:51: sparse: attribute 'require_context': unknown attributedrivers/block/drbd/drbd_int.h:1339:58: sparse: attribute 'require_context': unknown attributedrivers/block/drbd/drbd_int.h:1340:54: sparse: attribute 'require_context': unknown attributedrivers/block/drbd/drbd_int.h:1341:62: sparse: attribute 'require_context': unknown attributedrivers/block/drbd/drbd_int.h:1435:92: sparse: attribute 'require_context': unknown attribute
I believe that Lars send patch for that already. So no,
'[PATCH 1/2 v3] wait: add wait_event_lock_irq() interface'
does not fix the issue, but another patch should.
http://www.spinics.net/lists/linux-next/msg23042.html
Thanks!
-Lukas
Quoted 18 lines · On Fri, 30 Nov 2012 11:42:40 +0100 Lukas Czerner <[email protected]> wrote:
New wait_event{_interruptible}_lock_irq{_cmd} macros added. This commit moves the private
wait_event_lock_irq()macro from MD to regular wait includes, introduces new macrowait_event_lock_irq_cmd()instead of using the old method with omitting cmd parameter which is ugly and makes a use of new macros in the MD. It also introduces the _interruptible_ variant.The use of new interface is when one have a special lock to protect data structures used in the condition, or one also needs to invoke "cmd" before putting it to sleep.
All new macros are expected to be called with the lock taken. The lock is released before sleep and is reacquired afterwards. We will leave the macro with the lock held.
Note to DM: IMO this should also fix theoretical race on waitqueue while using simultaneously
wait_event_lock_irq()andwait_event()because of lack of locking around current state setting and wait queue removal.
Quoted 8 lines
Signed-off-by Lukas Czerner <[email protected]>Cc Neil Brown <[email protected]>Cc David Howells <[email protected]>Cc Ingo Molnar <[email protected]>Cc Peter Zijlstra <[email protected]>---
v2: add interruptible variant, swap cmd and schedule, useprepare_to_wait
v3: swap cmd and schedule so we call cmd before schedule
Your v3 patch reverts
http://ozlabs.org/~akpm/mmots/broken-out/wait-add-wait_event_lock_irq-interface-fix.patch.
Here's what I've queued in -mm:
include/linux/wait.h renamed from include/linux/wait.h~wait-add-wait_event_lock_irq-interface-v3 +8 -10
@@ -555,14 +555,13 @@ do { \ do { \ DEFINE_WAIT(__wait); \ \ - cmd; \ for (;;) { \ prepare_to_wait(&wq, &__wait, TASK_UNINTERRUPTIBLE); \ if (condition) \ break; \ spin_unlock_irq(&lock); \ - schedule(); \ cmd; \ + schedule(); \ spin_lock_irq(&lock); \ } \ finish_wait(&wq, &__wait); \
@@ -575,8 +574,8 @@ do { \* taken. * @wq: the waitqueue to wait on * @condition: a C expression for the event to wait for -* @lock: a locked spinlock_t, which will be released before schedule() -* and cmd reacquired afterwards. +* @lock: a locked spinlock_t, which will be released before cmd +* and schedule() and reacquired afterwards. * @cmd: a command which is invoked outside the critical section before * sleep *
@@ -588,7 +587,7 @@ do { \* change the result of the wait condition. * * This is supposed to be called while holding the lock. The lock is -* dropped before going to sleep and invoking the cmd and is reacquired +* dropped before invoking the cmd and going to sleep and is reacquired * afterwards. */ #define wait_event_lock_irq_cmd (wq, condition, lock, cmd) \
@@ -631,7 +630,6 @@ do { \ do { \ DEFINE_WAIT(__wait); \ \ - cmd; \ for (;;) { \ prepare_to_wait(&wq, &__wait, TASK_INTERRUPTIBLE); \ if (condition) \
@@ -641,8 +639,8 @@ do { \break ; \ } \spin_unlock_irq (&lock); \ -schedule (); \ cmd; \ +schedule (); \spin_lock_irq (&lock); \ } \finish_wait (&wq, &__wait); \
@@ -654,8 +652,8 @@ do { \* be called with the lock taken. * @wq: the waitqueue to wait on * @condition: a C expression for the event to wait for -* @lock: a locked spinlock_t, which will be released before schedule() -* and cmd reacquired afterwards. +* @lock: a locked spinlock_t, which will be released before cmd and +* schedule() and reacquired afterwards. * @cmd: a command which is invoked outside the critical section before * sleep *
@@ -667,7 +665,7 @@ do { \* change the result of the wait condition. * * This is supposed to be called while holding the lock. The lock is -* dropped before going to sleep and invoking the cmd and is reacquired +* dropped before invoking the cmd and going to sleep and is reacquired * afterwards. * * The macro will return -ERESTARTSYS if it was interrupted by a signal
_