public inbox for [email protected]
 help / color / mirror / Atom feed
From: Jens Axboe <[email protected]>
To: Linus Torvalds <[email protected]>
Cc: io-uring <[email protected]>,
	"[email protected]" <[email protected]>,
	"[email protected]" <[email protected]>
Subject: Re: [GIT PULL] io_uring passthrough support
Date: Mon, 23 May 2022 14:19:38 -0600	[thread overview]
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAHk-=whfi3FE3O7KrziqPbyGvAmNFas3xxLz2O+ttzBkCOQmfw@mail.gmail.com>

On 5/23/22 2:15 PM, Linus Torvalds wrote:
> On Sun, May 22, 2022 at 2:26 PM Jens Axboe <[email protected]> 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


  reply	other threads:[~2022-05-23 20:19 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-22 21:26 [GIT PULL] io_uring passthrough support Jens Axboe
2022-05-23 20:15 ` Linus Torvalds
2022-05-23 20:19   ` Jens Axboe [this message]
2022-05-24  5:15     ` Christoph Hellwig
2022-05-23 20:42 ` pr-tracker-bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox