On 5/27/22 8:54 AM, Jens Axboe wrote: > On 5/27/22 8:45 AM, Vincent Fu wrote: >>> -----Original Message----- >>> From: Ankit Kumar [mailto:ankit.kumar@xxxxxxxxxxx] >> >>> + >>> + if (io_u->ddir == DDIR_READ) >>> + cmd->opcode = nvme_cmd_read; >>> + if (io_u->ddir == DDIR_WRITE) >>> + cmd->opcode = nvme_cmd_write; >> >> Consider changing this to a switch statement and adding a default: >> case in case someone tries to send an unsupported command. >> >> Since this is in the fast path a switch statement would also reduce >> the number of times ddir is checked. > > A switch or if/else won't make any difference there, the compiler should > generate the same code. But I do agree that it's nicer to use a switch > so that unhandled cases are done properly. How I'd do this: make the prep handler return an error, and then do: if (io_u->ddir == DDIR_READ) cmd->opcode = nvme_cmd_read; else if (io_u->ddir == DDIR_WRITE) cmd->opcode = nvme_cmd_write; else return appropriate error; after you do the command memset, before all the other logic. Now make sure that whoever calls the prep handler will check for an error and error the io_u in that case. -- Jens Axboe
