Re: [PATCH v4 0/2] block, iomap: disable iopoll for split bio

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 11/17/20 8:51 PM, JeffleXu wrote:

On 11/17/20 3:56 PM, Jeffle Xu wrote:
This patchset is to fix the potential hang occurred in sync polling.

Please refer the following link for background info and the v1 patch:
https://patchwork.kernel.org/project/linux-block/patch/20201013084051.27255-1-jefflexu@xxxxxxxxxxxxxxxxx/

The first patch disables iopoll for split bio in block layer, which is
suggested by Ming Lei.


The second patch disables iopoll when one dio need to be split into
multiple bios. As for this patch, Ming Lei had ever asked what's the
expected behaviour of upper layers when simply clear IOCB_HIPRI in
the direct routine of blkdev fs, iomap-based fs. Currently there are
two parts concerning IOCB_HIPRI (or io polling). One is the sync
polling logic embedded in the direct IO routine. In this case, sync
polling won't be executed any more since IOCB_HIPRI flag has been
cleared from iocb->ki_flags. Consider the following code snippet:

fs/block_dev.c: __blkdev_direct_IO
    for (;;) {
        ...
        if (!(iocb->ki_flags & IOCB_HIPRI) ||
            !blk_poll(bdev_get_queue(bdev), qc, true))
            blk_io_schedule();
    }

fs/iomap/direct-io.c: __iomap_dio_rw
    for (;;) {
        ...
        if (!(iocb->ki_flags & IOCB_HIPRI) ||
            !dio->submit.last_queue ||
            !blk_poll(dio->submit.last_queue,
                 dio->submit.cookie, true))
            blk_io_schedule();
    }


The other part is io_uring.

fs/io_uring.c:
io_iopoll_getevents
   io_do_iopoll
     list_for_each_entry_safe(...) {
       ret = kiocb->ki_filp->f_op->iopoll(kiocb, spin);
     }

In this case, though the split bios have been enqueued into DEFAULT
hw queues, io_uring will still poll POLL hw queues. When polling on
the cookie returned by split bio, blk_poll() will return 0 immediately
since the hw queue type check added in patch 1. If there's no other
bio in the POLL hw queues, io_do_iopoll() will loop indefinitely
until the split bio is completed by interrupt of DEFAULT queue. Indeed
there may be a pulse of high CPU sys in this time window here, but it
is no worse than before. After all io_do_iopoll() will still get stuck
in this loop when there's only one bio (that we are polling on) in POLL
hw queue, before this patch applied.

The situation described above may be less impossible. As long as there
are other bios in POLL hw queue, work of io_do_iopoll() is still
meaningful as it *helps* reap these other bios in POLL hw queue, while
the split bios are still completed by interrupt of DEFAULT hw queue.

ops, this design could still be problematic. Once the cookie of split bio is iterated in io_do_iopoll(),

io_do_iopoll() will get stuck in indefinite loop doing nothing until the split bio is completed by the interrupt of

DEFAULT hw queue, even when there may be other bios in POLL hw queue waiting to be reaped.

This shouldn't be a problem. After this patch applied, blk_poll() will return 0 immediately

since the hw queue type check added in patch 1, and thus io_do_iopoll() will iterate next

kiocb in @iopoll_list. There will be no indefinite loop. Sorry for the noise...

--
Thanks,
Jeffle




[Index of Archives]     [Linux RAID]     [Linux SCSI]     [Linux ATA RAID]     [IDE]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Device Mapper]

  Powered by Linux