Re: [PATCH 1/4] io_uring: add IORING_OP_READ{WRITE}V_PI cmd

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

 



On Wed, Feb 26, 2020 at 08:58:46AM -0700, Jens Axboe wrote:
> On 2/26/20 8:57 AM, Christoph Hellwig wrote:
> > On Wed, Feb 26, 2020 at 07:24:06AM -0700, Jens Axboe wrote:
> >> On 2/26/20 1:37 AM, Bob Liu wrote:
> >>> diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
> >>> index a3300e1..98fa3f1 100644
> >>> --- a/include/uapi/linux/io_uring.h
> >>> +++ b/include/uapi/linux/io_uring.h
> >>> @@ -62,6 +62,8 @@ enum {
> >>>  	IORING_OP_NOP,
> >>>  	IORING_OP_READV,
> >>>  	IORING_OP_WRITEV,
> >>> +	IORING_OP_READV_PI,
> >>> +	IORING_OP_WRITEV_PI,
> >>>  	IORING_OP_FSYNC,
> >>>  	IORING_OP_READ_FIXED,
> >>>  	IORING_OP_WRITE_FIXED,
> >>
> >> So this one renumbers everything past IORING_OP_WRITEV, breaking the
> >> ABI in a very bad way. I'm guessing that was entirely unintentional?
> >> Any new command must go at the end of the list.
> >>
> >> You're also not updating io_op_defs[] with the two new commands,
> >> which means it won't compile at all. I'm guessing you tested this on
> >> an older version of the kernel which didn't have io_op_defs[]?
> > 
> > And the real question is why we need ops insted of just a flag and
> > using previously reserved fields for the PI pointer.
> 
> Yeah, should probably be a RWF_ flag instead, and a 64-bit SQE field
> for the PI data. The 'last iovec is PI' is kind of icky.

Heh, I was about to send in nearly the same comment. :)

--D

> -- 
> Jens Axboe
> 



[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