All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kevin Wolf <kwolf@redhat.com>
To: Christoph Hellwig <hch@lst.de>
Cc: qemu-devel@nongnu.org
Subject: [Qemu-devel] Re: [PATCH] raw/linux-aio: Also initialize POSIX AIO
Date: Thu, 22 Oct 2009 11:05:55 +0200	[thread overview]
Message-ID: <4AE02073.6030403@redhat.com> (raw)
In-Reply-To: <20091022083156.GC27577@lst.de>

Am 22.10.2009 10:31, schrieb Christoph Hellwig:
> On Tue, Oct 20, 2009 at 12:12:55PM +0200, Kevin Wolf wrote:
>> On that note, falling back to POSIX AIO means that paio_submit is called
>> with a Linux AIO aio_ctx. Which works because this parameter is unused
>> anyway, but am I the only one to find this ugly?
>>
>> What is the public interface of paio_submit meant to look like at all?
>> If aio_ctx is guaranteed to be unused, why not drop it or pass NULL at
>> least? And if it could be used some time in the future, the raw block
>> driver needs to be fixed.
> 
> Agreed.  Cared to send a patch?

Will do so if we agree to do it this way instead of doing the overkill
suggested below.

>> That said, I don't even think that the raw block driver is the right
>> place to distinguish between different AIO variants. Having a generic
>> aio_submit that calls the right AIO driver depending on the context
>> would be much cleaner. This would also mean that laio_submit handles the
>> fallback to paio_submit on its own, which I think is much cleaner than
>> teaching raw about the capabilities of each driver.
> 
> Seems a bit overkill until we get even more AIO variants at least.  And
> yes, that whole area is really ugly.

Yes, it might look like overkill to introduce a abstraction for exactly
two backends. I felt the same way. But then, the current implementation
just feels totally wrong. It absolutely intransparent when we fall back
to paio, and before debugging the bdrv_read/write emulation I didn't
even know that we're doing it. And, like I said, why should a block
format driver know what AIO method works which way?

I'm not insisting on restructuring though if you think that the effort
would outweigh the benefit of a cleanup.

Kevin

  reply	other threads:[~2009-10-22  9:07 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-10-20  9:33 [Qemu-devel] [PATCH] raw/linux-aio: Also initialize POSIX AIO Kevin Wolf
2009-10-20 10:12 ` [Qemu-devel] " Kevin Wolf
2009-10-22  8:31   ` Christoph Hellwig
2009-10-22  9:05     ` Kevin Wolf [this message]
2009-10-25  7:19       ` Christoph Hellwig
2009-10-26  8:36         ` Kevin Wolf
2009-10-28  8:37           ` Christoph Hellwig

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 \
    --in-reply-to=4AE02073.6030403@redhat.com \
    --to=kwolf@redhat.com \
    --cc=hch@lst.de \
    --cc=qemu-devel@nongnu.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.