On 5/23/22 2:15 PM, Linus Torvalds wrote: > On Sun, May 22, 2022 at 2:26 PM Jens Axboe <axboe@xxxxxxxxx> wrote: >> >> This will cause a merge conflict as well, with the provided buffer >> change from the core branch, and adding CQE32 support for NOP in this >> branch. > > Ugh, that really hits home how ugly this CQE32 support was. > > Dammit, it shouldn't have been done this way. That io_nop() code is > disgusting, and how it wants that separate "with extra info" case is > just nasty. > > I've pulled this, but with some swearing. That whole "extra1" and > "extra2" is ugly as hell, and just the naming shows that it has no > sane semantics, much less documentation. > > And the way it's randomly hidden in 'struct io_nop' *and* then a union > with that hash_node is just disgusting beyond words. Why do you need > both fields when you just copy one to the other at cmd start and then > back at cmd end? > > I must be missing something, but that it is incredibly ugly is clear. I think you are! The NOP case is just a sample way of exercising the CQE32 support, with extra1+2 being what is passed back in the bigger CQE. The NOP command exists purely to test things, and the CQE32 support there is a bit forced because NOP just always completes with '0' in the normal res field. We can obviously dump this as it isn't integral to anything, and honestly now that the NVMe is wired up, there's no great need to have a separate test for it. But it doesn't really hurt and there are already regression tests for it. -- Jens Axboe
