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:
diff --git a/fs/io_uring.c b/fs/io_uring.c
index 326695f74b93..90ecd656cc13 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -1257,7 +1257,7 @@ static const struct file_operations
io_uring_fops;
const char *io_uring_get_opcode(u8 opcode)
{
- switch (opcode) {
+ switch ((enum io_uring_op)opcode) {
case IORING_OP_NOP:
return "NOP";
case IORING_OP_READV:
diff --git a/include/uapi/linux/io_uring.h
b/include/uapi/linux/io_uring.h
index 980d82eb196e..a10b216ede3e 100644
--- a/include/uapi/linux/io_uring.h
+++ b/include/uapi/linux/io_uring.h
@@ -103,7 +103,7 @@ enum {
#define IORING_SETUP_R_DISABLED (1U << 6) /* start with
ring disabled */
#define IORING_SETUP_SUBMIT_ALL (1U << 7) /* continue
submit on error */
-enum {
+enum io_uring_op {
IORING_OP_NOP,
IORING_OP_READV,
IORING_OP_WRITEV,
>
> In any case, the LAST one is just a sentinel, both that and beyond
> should return eg INVALID.
>
Will do
