qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Kevin Wolf <kwolf@redhat.com>
To: Peter Lieven <pl@kamp.de>
Cc: "Daniel P. Berrange" <berrange@redhat.com>,
	qemu-block@nongnu.org, ct@flyingcircus.io, qemu-devel@nongnu.org,
	Max Reitz <mreitz@redhat.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Ilya Dryomov <idryomov@gmail.com>
Subject: Re: [PATCH v5 0/6] block/rbd: migrate to coroutines and add write zeroes support
Date: Tue, 6 Jul 2021 17:25:20 +0200	[thread overview]
Message-ID: <YOR14GzLqr3EKzcm@redhat.com> (raw)
In-Reply-To: <2F1CA650-9CE2-4AD9-BAF0-AD07829973B7@kamp.de>

Am 06.07.2021 um 16:55 hat Peter Lieven geschrieben:
> > Am 06.07.2021 um 15:19 schrieb Kevin Wolf <kwolf@redhat.com>:
> > 
> > Am 02.07.2021 um 19:23 hat Ilya Dryomov geschrieben:
> >> This series migrates the qemu rbd driver from the old aio emulation
> >> to native coroutines and adds write zeroes support which is important
> >> for block operations.
> >> 
> >> To achieve this we first bump the librbd requirement to the already
> >> outdated luminous release of ceph to get rid of some wrappers and
> >> ifdef'ry in the code.
> > 
> > Thanks, applied to the block branch.
> > 
> > I've only had a very quick look at the patches, but I think there is one
> > suggestion for a cleanup I can make: The qemu_rbd_finish_bh()
> > indirection is probably unnecessary now because aio_co_wake() is thread
> > safe.
> 
> But this is new, isn’t it?

Not sure in what sense you mean. aio_co_wake() has always been thread
safe, as far as I know.

Obviously, the old code didn't use aio_co_wake(), but directly called
some callbacks, so the part that is new is your coroutine conversion
that enables getting rid of the BH.

> We also have this indirection in iscsi and nfs drivers I think.

Indeed, the resulting codes look the same. In iscsi and nfs it doesn't
come from an incomplete converstion to coroutines, but they both used
qemu_coroutine_enter() originally, which resumes the coroutine in the
current thread...

> Does it matter that the completion callback is called from an librbd
> thread? Will the coroutine continue to run in the right thread?

...whereas aio_co_wake() resumes the coroutine in the thread where it
was running before.

(Before commit 5f50be9b5, this would have been buggy from an librbd
thread, but now it should work correctly even for threads that are
neither iothreads nor vcpu threads.)

> I will have a decent look after my vacation.

Sounds good, thanks. Enjoy your vacation!

Kevin



  reply	other threads:[~2021-07-06 15:27 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-02 17:23 [PATCH v5 0/6] block/rbd: migrate to coroutines and add write zeroes support Ilya Dryomov
2021-07-02 17:23 ` [PATCH v5 1/6] block/rbd: bump librbd requirement to luminous release Ilya Dryomov
2021-07-02 17:23 ` [PATCH v5 2/6] block/rbd: store object_size in BDRVRBDState Ilya Dryomov
2021-07-02 17:23 ` [PATCH v5 3/6] block/rbd: update s->image_size in qemu_rbd_getlength Ilya Dryomov
2021-07-02 17:23 ` [PATCH v5 4/6] block/rbd: migrate from aio to coroutines Ilya Dryomov
2021-07-02 17:23 ` [PATCH v5 5/6] block/rbd: add write zeroes support Ilya Dryomov
2021-07-02 17:23 ` [PATCH v5 6/6] block/rbd: drop qemu_rbd_refresh_limits Ilya Dryomov
2021-07-02 19:55 ` [PATCH v5 0/6] block/rbd: migrate to coroutines and add write zeroes support Ilya Dryomov
2021-07-06 13:19 ` Kevin Wolf
2021-07-06 14:55   ` Peter Lieven
2021-07-06 15:25     ` Kevin Wolf [this message]
2021-07-06 15:48       ` Peter Lieven
2021-07-07 18:13       ` Peter Lieven
2021-07-08 12:18         ` Kevin Wolf
2021-07-08 18:23           ` Peter Lieven
2021-07-09 10:21             ` Kevin Wolf
2021-09-16 12:34               ` Peter Lieven
2021-10-25 11:39                 ` Peter Lieven
2021-10-25 12:58                   ` Kevin Wolf
2021-10-26 14:53                     ` Peter Lieven
2021-11-15 11:17                       ` Peter Lieven

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=YOR14GzLqr3EKzcm@redhat.com \
    --to=kwolf@redhat.com \
    --cc=berrange@redhat.com \
    --cc=ct@flyingcircus.io \
    --cc=idryomov@gmail.com \
    --cc=mreitz@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=pl@kamp.de \
    --cc=qemu-block@nongnu.org \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).