On Thu, Jul 16, 2020 at 03:20:53PM -0600, Jens Axboe wrote: > On 7/16/20 2:51 PM, Jens Axboe wrote: > > On 7/16/20 2:47 PM, Pavel Begunkov wrote: > >> On 16/07/2020 23:42, Jens Axboe wrote: > >>> On 7/16/20 2:16 PM, Pavel Begunkov wrote: > >>>> On 16/07/2020 15:48, Stefano Garzarella wrote: > >>>>> The enumeration allows us to keep track of the last > >>>>> io_uring_register(2) opcode available. > >>>>> > >>>>> Behaviour and opcodes names don't change. > >>>>> > >>>>> Signed-off-by: Stefano Garzarella <[email protected]> > >>>>> --- > >>>>> include/uapi/linux/io_uring.h | 27 ++++++++++++++++----------- > >>>>> 1 file changed, 16 insertions(+), 11 deletions(-) > >>>>> > >>>>> diff --git a/include/uapi/linux/io_uring.h > >>>>> b/include/uapi/linux/io_uring.h > >>>>> index 7843742b8b74..efc50bd0af34 100644 > >>>>> --- a/include/uapi/linux/io_uring.h > >>>>> +++ b/include/uapi/linux/io_uring.h > >>>>> @@ -253,17 +253,22 @@ struct io_uring_params { > >>>>> /* > >>>>> * io_uring_register(2) opcodes and arguments > >>>>> */ > >>>>> -#define IORING_REGISTER_BUFFERS 0 > >>>>> -#define IORING_UNREGISTER_BUFFERS 1 > >>>>> -#define IORING_REGISTER_FILES 2 > >>>>> -#define IORING_UNREGISTER_FILES 3 > >>>>> -#define IORING_REGISTER_EVENTFD 4 > >>>>> -#define IORING_UNREGISTER_EVENTFD 5 > >>>>> -#define IORING_REGISTER_FILES_UPDATE 6 > >>>>> -#define IORING_REGISTER_EVENTFD_ASYNC 7 > >>>>> -#define IORING_REGISTER_PROBE 8 > >>>>> -#define IORING_REGISTER_PERSONALITY 9 > >>>>> -#define IORING_UNREGISTER_PERSONALITY 10 > >>>>> +enum { > >>>>> + IORING_REGISTER_BUFFERS, > >>>>> + IORING_UNREGISTER_BUFFERS, > >>>>> + IORING_REGISTER_FILES, > >>>>> + IORING_UNREGISTER_FILES, > >>>>> + IORING_REGISTER_EVENTFD, > >>>>> + IORING_UNREGISTER_EVENTFD, > >>>>> + IORING_REGISTER_FILES_UPDATE, > >>>>> + IORING_REGISTER_EVENTFD_ASYNC, > >>>>> + IORING_REGISTER_PROBE, > >>>>> + IORING_REGISTER_PERSONALITY, > >>>>> + IORING_UNREGISTER_PERSONALITY, > >>>>> + > >>>>> + /* this goes last */ > >>>>> + IORING_REGISTER_LAST > >>>>> +}; > >>>> > >>>> It breaks userspace API. E.g. > >>>> > >>>> #ifdef IORING_REGISTER_BUFFERS > >>> > >>> It can, yes, but we have done that in the past. In this one, for > >> > >> Ok, if nobody on the userspace side cares, then better to do that > >> sooner than later. > > I actually don't think it's a huge issue. Normally if applications > do this, it's because they are using it and need it. Ala: > > #ifndef IORING_REGISTER_SOMETHING > #define IORING_REGISTER_SOMETHING fooval > #endif > > and that'll still work just fine, even if an identical enum is there. >
Thank you both for the review! Then if you agree, I'll leave this patch as it is by introducing the enum. Stefano
