Re: Various changes "backportability"

[Stefan Hajnoczi]

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 <nivanov@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