From: Andres Freund <andres@anarazel.de>
To: Jens Axboe <axboe@kernel.dk>
Cc: io-uring@vger.kernel.org
Subject: Re: [PATCH 2/6] io_uring: add IORING_OP_PROVIDE_BUFFERS
Date: Mon, 9 Mar 2020 10:28:46 -0700 [thread overview]
Message-ID: <20200309172846.ilh27woo7tsaqadf@alap3.anarazel.de> (raw)
In-Reply-To: <a2283eb6-4b86-b858-a440-af4a8a7c2ba9@kernel.dk>
Hi,
On 2020-03-09 11:17:46 -0600, Jens Axboe wrote:
> >> +static int io_add_buffers(struct io_provide_buf *pbuf, struct list_head *list)
> >> +{
> >> + struct io_buffer *buf;
> >> + u64 addr = pbuf->addr;
> >> + int i, bid = pbuf->bid;
> >> +
> >> + for (i = 0; i < pbuf->nbufs; i++) {
> >> + buf = kmalloc(sizeof(*buf), GFP_KERNEL);
> >> + if (!buf)
> >> + break;
> >> +
> >> + buf->addr = addr;
> >> + buf->len = pbuf->len;
> >> + buf->bid = bid;
> >> + list_add(&buf->list, list);
> >> + addr += pbuf->len;
> >> + bid++;
> >> + }
> >> +
> >> + return i;
> >> +}
> >
> > Hm, aren't you loosing an error here if you kmalloc fails for i > 0?
> > Afaict io_provide_buffes() only checks for ret != 0. I think userland
> > should know that a PROVIDE_BUFFERS failed, even if just partially (I'd
> > just make it fail wholesale).
>
> The above one does have the issue that we're losing the error for i ==
> 0, current one does:
>
> return i ? i : -ENOMEM;
>
> But this is what most interfaces end up doing, return the number
> processed, if any, or error if none of them were added. Like a short
> read, for example, and you'd get EIO if you forwarded and tried again.
> So I tend to prefer doing it like that, at least to me it seems more
> logical than unwinding. The application won't know what buffer caused
> the error if you unwind, whereas it's perfectly clear if you asked to
> add 128 and we return 64 that the issue is with the 65th buffer.
Fair enough. I was/am thinking that this'd pretty much always be a fatal
error for the application. Which does seem a bit different from the
short read/write case, where there are plenty reasons to handle them
"silently" during normal operation.
But I can error out with the current interface, so ...
Greetings,
Andres Freund
next prev parent reply other threads:[~2020-03-09 17:28 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-28 20:30 [PATCHSET v3] io_uring support for automatic buffers Jens Axboe
2020-02-28 20:30 ` [PATCH 1/6] io_uring: buffer registration infrastructure Jens Axboe
2020-02-28 20:30 ` [PATCH 2/6] io_uring: add IORING_OP_PROVIDE_BUFFERS Jens Axboe
2020-02-29 0:43 ` Pavel Begunkov
2020-02-29 4:50 ` Jens Axboe
2020-02-29 11:36 ` Pavel Begunkov
2020-02-29 17:32 ` Jens Axboe
2020-02-29 12:08 ` Pavel Begunkov
2020-02-29 17:34 ` Jens Axboe
2020-02-29 18:11 ` Jens Axboe
2020-03-09 17:03 ` Andres Freund
2020-03-09 17:17 ` Jens Axboe
2020-03-09 17:28 ` Andres Freund [this message]
2020-03-10 13:33 ` Jens Axboe
2020-02-28 20:30 ` [PATCH 3/6] io_uring: support buffer selection Jens Axboe
2020-02-29 12:21 ` Pavel Begunkov
2020-02-29 17:35 ` Jens Axboe
2020-03-09 17:21 ` Andres Freund
2020-03-10 13:37 ` Jens Axboe
2020-02-28 20:30 ` [PATCH 4/6] io_uring: add IOSQE_BUFFER_SELECT support for IORING_OP_READV Jens Axboe
2020-02-28 20:30 ` [PATCH 5/6] net: abstract out normal and compat msghdr import Jens Axboe
2020-02-28 20:30 ` [PATCH 6/6] io_uring: add IOSQE_BUFFER_SELECT support for IORING_OP_RECVMSG Jens Axboe
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=20200309172846.ilh27woo7tsaqadf@alap3.anarazel.de \
--to=andres@anarazel.de \
--cc=axboe@kernel.dk \
--cc=io-uring@vger.kernel.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.