[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v5 0/6] block/rbd: migrate to coroutines and add write zeroes
From: |
Peter Lieven |
Subject: |
Re: [PATCH v5 0/6] block/rbd: migrate to coroutines and add write zeroes support |
Date: |
Wed, 7 Jul 2021 20:13:54 +0200 |
> Am 06.07.2021 um 17:25 schrieb Kevin Wolf <kwolf@redhat.com>:
>
> 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!
As I had to fire up my laptop to look into another issue anyway, I have sent
two patches for updating MAINTAINERS and to fix the int vs. bool mix for
task->complete. As Paolos fix (5f50be9b5) is relatively new and there are maybe
other non obvious problems when removing the BH indirection and we are close to
soft freeze I would leave the BH removal change for 6.2.
Best,
Peter
- [PATCH v5 2/6] block/rbd: store object_size in BDRVRBDState, (continued)
- [PATCH v5 2/6] block/rbd: store object_size in BDRVRBDState, Ilya Dryomov, 2021/07/02
- [PATCH v5 1/6] block/rbd: bump librbd requirement to luminous release, Ilya Dryomov, 2021/07/02
- [PATCH v5 6/6] block/rbd: drop qemu_rbd_refresh_limits, Ilya Dryomov, 2021/07/02
- [PATCH v5 4/6] block/rbd: migrate from aio to coroutines, Ilya Dryomov, 2021/07/02
- [PATCH v5 5/6] block/rbd: add write zeroes support, Ilya Dryomov, 2021/07/02
- Re: [PATCH v5 0/6] block/rbd: migrate to coroutines and add write zeroes support, Ilya Dryomov, 2021/07/02
- Re: [PATCH v5 0/6] block/rbd: migrate to coroutines and add write zeroes support, Kevin Wolf, 2021/07/06
- Re: [PATCH v5 0/6] block/rbd: migrate to coroutines and add write zeroes support, Peter Lieven, 2021/07/06
- Re: [PATCH v5 0/6] block/rbd: migrate to coroutines and add write zeroes support, Kevin Wolf, 2021/07/06
- Re: [PATCH v5 0/6] block/rbd: migrate to coroutines and add write zeroes support, Peter Lieven, 2021/07/06
- Re: [PATCH v5 0/6] block/rbd: migrate to coroutines and add write zeroes support,
Peter Lieven <=
- Re: [PATCH v5 0/6] block/rbd: migrate to coroutines and add write zeroes support, Kevin Wolf, 2021/07/08
- Re: [PATCH v5 0/6] block/rbd: migrate to coroutines and add write zeroes support, Peter Lieven, 2021/07/08
- Re: [PATCH v5 0/6] block/rbd: migrate to coroutines and add write zeroes support, Kevin Wolf, 2021/07/09