Sorry for that, I will send a new version later. On 11/24/20 7:25 PM, Christoph Hellwig wrote: > On Fri, Nov 20, 2020 at 06:06:54PM +0800, JeffleXu wrote: >> >> On 11/20/20 1:55 AM, Christoph Hellwig wrote: >>>> diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c >>>> index 933f234d5bec..396ac0f91a43 100644 >>>> --- a/fs/iomap/direct-io.c >>>> +++ b/fs/iomap/direct-io.c >>>> @@ -309,6 +309,16 @@ iomap_dio_bio_actor(struct inode *inode, loff_t pos, loff_t length, >>>> copied += n; >>>> nr_pages = iov_iter_npages(dio->submit.iter, BIO_MAX_PAGES); >>>> + /* >>>> + * The current dio needs to be split into multiple bios here. >>>> + * iopoll for split bio will cause subtle trouble such as >>>> + * hang when doing sync polling, while iopoll is initially >>>> + * for small size, latency sensitive IO. Thus disable iopoll >>>> + * if split needed. >>>> + */ >>>> + if (nr_pages) >>>> + dio->iocb->ki_flags &= ~IOCB_HIPRI; >>> I think this is confusing two things. >> >> Indeed there's two level of split concerning this issue when doing sync >> iopoll. >> >> >> The first is that one bio got split in block-core, and patch 1 of this patch >> set just fixes this. >> >> >> Second is that one dio got split into multiple bios in fs layer, and patch 2 >> fixes this. >> >> >>> One is that we don't handle >>> polling well when there are multiple bios. For this I think we should >>> only call bio_set_polled when we know there is a single bio. >> >> >> How about the following patch: >> >> >> --- a/fs/iomap/direct-io.c >> +++ b/fs/iomap/direct-io.c >> @@ -60,12 +60,12 @@ int iomap_dio_iopoll(struct kiocb *kiocb, bool spin) >> ??EXPORT_SYMBOL_GPL(iomap_dio_iopoll); >> >> ??static void iomap_dio_submit_bio(struct iomap_dio *dio, struct iomap >> *iomap, >> -???????????????????????????? struct bio *bio, loff_t pos) >> +???????????????????????????? struct bio *bio, loff_t pos, bool split) > > This seems pretty messed up by your mailer and I have a hard time > reading it. Can you resend it? > -- Thanks, Jeffle
