block: don't use un-ordered __set_current_state(TASK_UNINTERRUPTIBLE)
authorLinus Torvalds <torvalds@linux-foundation.org>
Wed, 2 Jan 2019 18:46:03 +0000 (10:46 -0800)
committerLinus Torvalds <torvalds@linux-foundation.org>
Wed, 2 Jan 2019 18:46:03 +0000 (10:46 -0800)
This mostly reverts commit 849a370016a5 ("block: avoid ordered task
state change for polled IO").  It was wrongly claiming that the ordering
wasn't necessary.  The memory barrier _is_ necessary.

If something is truly polling and not going to sleep, it's the whole
state setting that is unnecessary, not the memory barrier.  Whenever you
set your state to a sleeping state, you absolutely need the memory
barrier.

Note that sometimes the memory barrier can be elsewhere.  For example,
the ordering might be provided by an external lock, or by setting the
process state to sleeping before adding yourself to the wait queue list
that is used for waking up (where the wait queue lock itself will
guarantee that any wakeup will correctly see the sleeping state).

But none of those cases were true here.

NOTE! Some of the polling paths may indeed be able to drop the state
setting entirely, at which point the memory barrier also goes away.

(Also note that this doesn't revert the TASK_RUNNING cases: there is no
race between a wakeup and setting the process state to TASK_RUNNING,
since the end result doesn't depend on ordering).

Cc: Jens Axboe <axboe@kernel.dk>
Cc: Christoph Hellwig <hch@lst.de>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
fs/block_dev.c
fs/iomap.c
mm/page_io.c

index 450be88cffef3a7e0b9d68973159ac1c2a2b00fc..c546cdce77e6df48ab48df9b84f3cb6c20c4d173 100644 (file)
@@ -237,11 +237,9 @@ __blkdev_direct_IO_simple(struct kiocb *iocb, struct iov_iter *iter,
 
        qc = submit_bio(&bio);
        for (;;) {
-               __set_current_state(TASK_UNINTERRUPTIBLE);
-
+               set_current_state(TASK_UNINTERRUPTIBLE);
                if (!READ_ONCE(bio.bi_private))
                        break;
-
                if (!(iocb->ki_flags & IOCB_HIPRI) ||
                    !blk_poll(bdev_get_queue(bdev), qc, true))
                        io_schedule();
@@ -426,8 +424,7 @@ __blkdev_direct_IO(struct kiocb *iocb, struct iov_iter *iter, int nr_pages)
                return -EIOCBQUEUED;
 
        for (;;) {
-               __set_current_state(TASK_UNINTERRUPTIBLE);
-
+               set_current_state(TASK_UNINTERRUPTIBLE);
                if (!READ_ONCE(dio->waiter))
                        break;
 
index 3a0cd557b4cf8edfa0d068acd77f3ad9c88320e4..a3088fae567ba3092735667e4f9ac2595a1814b1 100644 (file)
@@ -1921,8 +1921,7 @@ iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter,
                        return -EIOCBQUEUED;
 
                for (;;) {
-                       __set_current_state(TASK_UNINTERRUPTIBLE);
-
+                       set_current_state(TASK_UNINTERRUPTIBLE);
                        if (!READ_ONCE(dio->submit.waiter))
                                break;
 
index 3475733b19264ea6c0aa76bbcf031c3aafcec456..d975fa3f02aa565c1bd050b0e2a70e6bedc3af2e 100644 (file)
@@ -405,8 +405,7 @@ int swap_readpage(struct page *page, bool synchronous)
        bio_get(bio);
        qc = submit_bio(bio);
        while (synchronous) {
-               __set_current_state(TASK_UNINTERRUPTIBLE);
-
+               set_current_state(TASK_UNINTERRUPTIBLE);
                if (!READ_ONCE(bio->bi_private))
                        break;