Skip to content

Commit

Permalink
block: don't use un-ordered __set_current_state(TASK_UNINTERRUPTIBLE)
Browse files Browse the repository at this point in the history
This mostly reverts commit 849a370 ("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 <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
  • Loading branch information
torvalds committed Jan 2, 2019
1 parent d9a7fa6 commit 1ac5cd4
Show file tree
Hide file tree
Showing 3 changed files with 4 additions and 9 deletions.
7 changes: 2 additions & 5 deletions fs/block_dev.c
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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;

Expand Down
3 changes: 1 addition & 2 deletions fs/iomap.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
3 changes: 1 addition & 2 deletions mm/page_io.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down

0 comments on commit 1ac5cd4

Please sign in to comment.