On 3/27/23 6:25 PM, Al Viro wrote:
> On Tue, Mar 28, 2023 at 01:08:11AM +0100, Al Viro wrote:
>> On Mon, Mar 27, 2023 at 05:27:13PM -0600, Jens Axboe wrote:
>>> Add a special case to __import_iovec(), which imports a single segment
>>> iovec as an ITER_UBUF rather than an ITER_IOVEC. ITER_UBUF is cheaper
>>> to iterate than ITER_IOVEC, and for a single segment iovec, there's no
>>> point in using a segmented iterator.
>>
>> Won't that enforce the "single-segment readv() is always identical to
>> read()"? We'd been through that before - some of infinibarf drvivers
>> have two different command sets, one reached via read(), another - via
>> readv(). It's a userland ABI. Misdesigned one, but that's infinibarf
>> for you.
>>
>> Does anyone really need this particular microoptimization?
>
> static ssize_t qib_write_iter(struct kiocb *iocb, struct iov_iter *from)
> {
> struct qib_filedata *fp = iocb->ki_filp->private_data;
> struct qib_ctxtdata *rcd = ctxt_fp(iocb->ki_filp);
> struct qib_user_sdma_queue *pq = fp->pq;
>
> if (!iter_is_iovec(from) || !from->nr_segs || !pq)
> return -EINVAL;
>
> return qib_user_sdma_writev(rcd, pq, from->iov, from->nr_segs);
> }
>
> Hit this with single-segment writev() and you've got yourself -EINVAL.
> Sure, that could be adjusted for (check for user_backed_iter(), then
> if it's ubuf form an iovec and pass that to qib_user_sdma_writev()),
> but that's a clear regression.
>
> Found by simple grepping for iter_is_iovec()...
Looks like there are a few of those, in fact. Guess we need to do
an iter->user_backed check for these.
--
Jens Axboe
