On Wed, Sep 13, 2023 at 05:44:38PM +0300, Michael Tokarev wrote: > 13.09.2023 17:27, Stefan Hajnoczi wrote: > ... > > > For example, recent tpm bugfix, which is trivial by its own, > > > uses RETRY_ON_EINTR helper which were introduced recently and > > > which is now used everywhere. coroutine_fn et al markers is > > > another example, translator_io_start is yet another, and so > > > on and so on. > > > The general concept makes sense to me but I'm not sure what the > > specific issue with adding (?) coroutine_fn was. Can you link to the > > patch that caused difficulties so I can review it? > > There's nothing really exciting here, and coroutine_fn example isn't > a best one really. I'm talking about this: > > https://gitlab.com/mjt0k/qemu/-/commit/c5034f827726f5876234bf4c6a0fab648fd8b020 > > which is a current back-port of 92e2e6a867334a990f8d29f07ca34e3162fdd6ec > "virtio: Drop out of coroutine context in virtio_load()": > > https://gitlab.com/mjt0k/qemu/-/commit/92e2e6a867334a990f8d29f07ca34e3162fdd6ec > > This is a bugfix which I tried to cherry-pick (btw, I dunno yet if it should > go to 8.0 or 7.2 to begin with, asked this in another email, but it still > serves as an example). Original patch adds coroutine_mixed_fn to some > existing > functions and to a newly added function. > > The patch introducing coroutine_mixed_fn marker is v7.2.0-909-g0f3de970 . > This is actually a very good example of a way how things are done best, > an excellent example of what I'm talking here, - this 0f3de970 only introduces > the new concept (to be widely used), not converting everything to it > right away. So it's a good example of how things can be done right. > > But this 0f3de970 change is based on earlier change which split things up > and moved stuff from one place to another, and which is too large to > backport. So even if 0f3de970 did an excellent job, it is still of no > use in this context. > > I decided to drop coroutine_mixed_fn markings in the fix for 7.2 in this > context, - again, if this particular fix is needed there to begin with, > which is a question unrelated to this topic. > > > A better example is a trivial thing with RETRY_ON_EINTR introduction. > A trivial macro which replaced TFR in > > commit 37b0b24e933c18269dddbf6b83f91823cacf8105 > Author: Nikita Ivanov <niva...@cloudlinux.com> > Date: Sun Oct 23 12:04:22 2022 +0300 > > error handling: Use RETRY_ON_EINTR() macro where applicable > > if this change were split into two, first introducing the new macro > and second converting existing code & removing old macro, it'd be > possible to just cherry-pick the first part and thered' be no need > to modify further cherry-picks which uses RETRY_ON_EINTR. > > But once again, this all is definitely not as important as getting > good code into main :)
I see, thank you! Stefan
signature.asc
Description: PGP signature
