From: Jens Axboe <axboe@kernel.dk>
To: Pavel Begunkov <asml.silence@gmail.com>,
JeffleXu <jefflexu@linux.alibaba.com>,
Dave Chinner <david@fromorbit.com>
Cc: Hao Xu <haoxu@linux.alibaba.com>,
Alexander Viro <viro@zeniv.linux.org.uk>,
Christoph Hellwig <hch@infradead.org>,
"Darrick J. Wong" <darrick.wong@oracle.com>,
linux-fsdevel@vger.kernel.org,
Konstantin Khlebnikov <khlebnikov@yandex-team.ru>,
io-uring@vger.kernel.org, Joseph Qi <joseph.qi@linux.alibaba.com>
Subject: Re: [PATCH v3 RESEND] iomap: set REQ_NOWAIT according to IOCB_NOWAIT in Direct IO
Date: Fri, 2 Apr 2021 10:26:42 -0600 [thread overview]
Message-ID: <62452618-d44f-fb88-18b6-80bcf5c8b81d@kernel.dk> (raw)
In-Reply-To: <1e687bef-3d96-69ad-ec98-c674f5a88ca2@gmail.com>
On 4/2/21 8:32 AM, Pavel Begunkov wrote:
> On 15/12/2020 09:43, JeffleXu wrote:
>> Thanks for your explanation, again.
>
> Got stale, let's bring it up again.
How about something like this - check upfront if we're going to be
using multiple bios, and -EAGAIN for NOWAIT being set if that is
the case. That avoids the partial problem, and still retains (what
I consider) proper NOWAIT behavior for O_DIRECT with IOCB_NOWAIT
set.
It's also worth nothing that this condition exists already for
polled IO. If the bio is marked as polled, then we implicitly
set NOWAIT as well, as there's no way to support polled IO with
sleeping request allocations. Hence it's worth considering this
a fix for that case, too.
diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c
index e2c4991833b8..6f932fe99440 100644
--- a/fs/iomap/direct-io.c
+++ b/fs/iomap/direct-io.c
@@ -66,6 +66,8 @@ static void iomap_dio_submit_bio(struct iomap_dio *dio, struct iomap *iomap,
if (dio->iocb->ki_flags & IOCB_HIPRI)
bio_set_polled(bio, dio->iocb);
+ if (dio->iocb->ki_flags & IOCB_NOWAIT)
+ bio->bi_opf |= REQ_NOWAIT;
dio->submit.last_queue = bdev_get_queue(iomap->bdev);
if (dio->dops && dio->dops->submit_io)
@@ -236,6 +238,7 @@ iomap_dio_bio_actor(struct inode *inode, loff_t pos, loff_t length,
unsigned int blkbits = blksize_bits(bdev_logical_block_size(iomap->bdev));
unsigned int fs_block_size = i_blocksize(inode), pad;
unsigned int align = iov_iter_alignment(dio->submit.iter);
+ bool nowait = dio->iocb->ki_flags & (IOCB_HIPRI | IOCB_NOWAIT);
unsigned int bio_opf;
struct bio *bio;
bool need_zeroout = false;
@@ -296,7 +299,17 @@ iomap_dio_bio_actor(struct inode *inode, loff_t pos, loff_t length,
*/
bio_opf = iomap_dio_bio_opflags(dio, iomap, use_fua);
- nr_pages = bio_iov_vecs_to_alloc(dio->submit.iter, BIO_MAX_PAGES);
+ nr_pages = bio_iov_vecs_to_alloc(dio->submit.iter, INT_MAX);
+
+ /* Can't handle IOCB_NOWAIT for split bios */
+ if (nr_pages > BIO_MAX_PAGES) {
+ if (nowait) {
+ ret = -EAGAIN;
+ goto out;
+ }
+ nr_pages = BIO_MAX_PAGES;
+ }
+
do {
size_t n;
if (dio->error) {
@@ -326,6 +339,19 @@ iomap_dio_bio_actor(struct inode *inode, loff_t pos, loff_t length,
goto zero_tail;
}
+ /*
+ * If there are leftover pages, bail if nowait is set to avoid
+ * multiple bios and potentially having one of them -EAGAIN
+ * with the other succeeding.
+ */
+ nr_pages = bio_iov_vecs_to_alloc(dio->submit.iter,
+ BIO_MAX_PAGES);
+ if (nr_pages && nowait) {
+ ret = -EAGAIN;
+ bio_put(bio);
+ goto out;
+ }
+
n = bio->bi_iter.bi_size;
if (dio->flags & IOMAP_DIO_WRITE) {
task_io_account_write(n);
@@ -337,8 +363,6 @@ iomap_dio_bio_actor(struct inode *inode, loff_t pos, loff_t length,
dio->size += n;
copied += n;
- nr_pages = bio_iov_vecs_to_alloc(dio->submit.iter,
- BIO_MAX_PAGES);
iomap_dio_submit_bio(dio, iomap, bio, pos);
pos += n;
} while (nr_pages);
--
Jens Axboe
prev parent reply other threads:[~2021-04-02 16:26 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-12-04 9:44 [PATCH v3 RESEND] iomap: set REQ_NOWAIT according to IOCB_NOWAIT in Direct IO Hao Xu
2020-12-04 11:44 ` Pavel Begunkov
2020-12-07 2:21 ` Dave Chinner
2020-12-07 23:40 ` Jens Axboe
2020-12-09 21:15 ` Dave Chinner
2020-12-10 2:33 ` JeffleXu
2020-12-08 5:46 ` JeffleXu
2020-12-09 21:23 ` Dave Chinner
2020-12-10 1:55 ` JeffleXu
2020-12-10 5:18 ` Dave Chinner
2020-12-11 2:50 ` JeffleXu
2020-12-14 2:56 ` Dave Chinner
2020-12-15 9:43 ` JeffleXu
2021-04-02 14:32 ` Pavel Begunkov
2021-04-02 16:26 ` Jens Axboe [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=62452618-d44f-fb88-18b6-80bcf5c8b81d@kernel.dk \
--to=axboe@kernel.dk \
--cc=asml.silence@gmail.com \
--cc=darrick.wong@oracle.com \
--cc=david@fromorbit.com \
--cc=haoxu@linux.alibaba.com \
--cc=hch@infradead.org \
--cc=io-uring@vger.kernel.org \
--cc=jefflexu@linux.alibaba.com \
--cc=joseph.qi@linux.alibaba.com \
--cc=khlebnikov@yandex-team.ru \
--cc=linux-fsdevel@vger.kernel.org \
--cc=viro@zeniv.linux.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).