On 4/25/22 8:48 AM, Dylan Yudaken wrote:
> On Mon, 2022-04-25 at 08:29 -0600, Jens Axboe wrote:
>> On 4/25/22 7:21 AM, Dylan Yudaken wrote:
>>> On Mon, 2022-04-25 at 06:38 -0600, Jens Axboe wrote:
>>>> On 4/25/22 3:36 AM, Dylan Yudaken wrote:
>>>>> In some debug scenarios it is useful to have the text
>>>>> representation
>>>>> of
>>>>> the opcode. Add this function in preparation.
>>>>>
>>>>> Signed-off-by: Dylan Yudaken <dylany@xxxxxx>
>>>>> ---
>>>>> fs/io_uring.c | 91
>>>>> ++++++++++++++++++++++++++++++++++++++++
>>>>> include/linux/io_uring.h | 5 +++
>>>>> 2 files changed, 96 insertions(+)
>>>>>
>>>>> diff --git a/fs/io_uring.c b/fs/io_uring.c
>>>>> index e57d47a23682..326695f74b93 100644
>>>>> --- a/fs/io_uring.c
>>>>> +++ b/fs/io_uring.c
>>>>> @@ -1255,6 +1255,97 @@ static struct kmem_cache *req_cachep;
>>>>>
>>>>> static const struct file_operations io_uring_fops;
>>>>>
>>>>> +const char *io_uring_get_opcode(u8 opcode)
>>>>> +{
>>>>> + switch (opcode) {
>>>>> + case IORING_OP_NOP:
>>>>> + return "NOP";
>>>>> + case IORING_OP_READV:
>>>>> + return "READV";
>>>>> + case IORING_OP_WRITEV:
>>>>> + return "WRITEV";
>>>>> + case IORING_OP_FSYNC:
>>>>> + return "FSYNC";
>>>>> + case IORING_OP_READ_FIXED:
>>>>> + return "READ_FIXED";
>>>>> + case IORING_OP_WRITE_FIXED:
>>>>> + return "WRITE_FIXED";
>>>>> + case IORING_OP_POLL_ADD:
>>>>> + return "POLL_ADD";
>>>>> + case IORING_OP_POLL_REMOVE:
>>>>> + return "POLL_REMOVE";
>>>>> + case IORING_OP_SYNC_FILE_RANGE:
>>>>> + return "SYNC_FILE_RANGE";
>>>>> + case IORING_OP_SENDMSG:
>>>>> + return "SENDMSG";
>>>>> + case IORING_OP_RECVMSG:
>>>>> + return "RECVMSG";
>>>>> + case IORING_OP_TIMEOUT:
>>>>> + return "TIMEOUT";
>>>>> + case IORING_OP_TIMEOUT_REMOVE:
>>>>> + return "TIMEOUT_REMOVE";
>>>>> + case IORING_OP_ACCEPT:
>>>>> + return "ACCEPT";
>>>>> + case IORING_OP_ASYNC_CANCEL:
>>>>> + return "ASYNC_CANCEL";
>>>>> + case IORING_OP_LINK_TIMEOUT:
>>>>> + return "LINK_TIMEOUT";
>>>>> + case IORING_OP_CONNECT:
>>>>> + return "CONNECT";
>>>>> + case IORING_OP_FALLOCATE:
>>>>> + return "FALLOCATE";
>>>>> + case IORING_OP_OPENAT:
>>>>> + return "OPENAT";
>>>>> + case IORING_OP_CLOSE:
>>>>> + return "CLOSE";
>>>>> + case IORING_OP_FILES_UPDATE:
>>>>> + return "FILES_UPDATE";
>>>>> + case IORING_OP_STATX:
>>>>> + return "STATX";
>>>>> + case IORING_OP_READ:
>>>>> + return "READ";
>>>>> + case IORING_OP_WRITE:
>>>>> + return "WRITE";
>>>>> + case IORING_OP_FADVISE:
>>>>> + return "FADVISE";
>>>>> + case IORING_OP_MADVISE:
>>>>> + return "MADVISE";
>>>>> + case IORING_OP_SEND:
>>>>> + return "SEND";
>>>>> + case IORING_OP_RECV:
>>>>> + return "RECV";
>>>>> + case IORING_OP_OPENAT2:
>>>>> + return "OPENAT2";
>>>>> + case IORING_OP_EPOLL_CTL:
>>>>> + return "EPOLL_CTL";
>>>>> + case IORING_OP_SPLICE:
>>>>> + return "SPLICE";
>>>>> + case IORING_OP_PROVIDE_BUFFERS:
>>>>> + return "PROVIDE_BUFFERS";
>>>>> + case IORING_OP_REMOVE_BUFFERS:
>>>>> + return "REMOVE_BUFFERS";
>>>>> + case IORING_OP_TEE:
>>>>> + return "TEE";
>>>>> + case IORING_OP_SHUTDOWN:
>>>>> + return "SHUTDOWN";
>>>>> + case IORING_OP_RENAMEAT:
>>>>> + return "RENAMEAT";
>>>>> + case IORING_OP_UNLINKAT:
>>>>> + return "UNLINKAT";
>>>>> + case IORING_OP_MKDIRAT:
>>>>> + return "MKDIRAT";
>>>>> + case IORING_OP_SYMLINKAT:
>>>>> + return "SYMLINKAT";
>>>>> + case IORING_OP_LINKAT:
>>>>> + return "LINKAT";
>>>>> + case IORING_OP_MSG_RING:
>>>>> + return "MSG_RING";
>>>>> + case IORING_OP_LAST:
>>>>> + return "LAST";
>>>>> + }
>>>>> + return "UNKNOWN";
>>>>> +}
>>>>
>>>> My only worry here is that it's another place to touch when
>>>> adding an
>>>> opcode. I'm assuming the compiler doesn't warn if you're missing
>>>> one
>>>> since it's not strongly typed?
>>>
>>> It doesn't complain, but we could strongly type it to get it to? I
>>> don't think it will break anything (certainly does not locally).
>>> What
>>> about something like this:
>>
>> I think this would be fine. Would probably be cleaner if you just
>> make
>> io_uring_get_opcode() take an enum io_uring_op and just fwd declare
>> it
>> in io_uring.h?
>
> I cannot do that bit, as there is a separate function for
> CONFIG_IO_URING=n. That would either require different signatures (u8
> vs enum io_uring_op) or including the enum even when not enabled in
> config. Both of these feel ugly.
>
> I'll send a new series giving the enum a type.
Agree, let's keep the cast then.
--
Jens Axboe
