On Tue, Aug 12, 2025 at 05:00:35PM +0200, Igor Mammedov wrote:
> > Do they better as well be converted to use store_release too?
> 
> alternatively, for consistency sake we can add a helper to set interrupt
> with store_release included and do a blanket tree wide change like with
> cpu_test_interrupt().

Yep, that sounds like the simplest. Unless there's any concern on possible
performance regressions due to the rest conversions on store_release, those
can be special treated with open-code, tagged with reasoning as comment.

>  
> > The other nitpick is this patch introduces the reader helper but without
> > using it.
> > 
> > It can be squashed into the other patch where the reader helper will be
> > applied tree-wide.  IMHO that would be better explaining the helper on its
> > own.
> 
> I can merge it with 7/10 that adds 1st user for the helper in kvm/i386 code.
> That has less chances for the store/acquire change to be drowned in
> tree wide patch (9/10)

For mem barrier changes, IMHO the two sides are better in one patch, hence
the two helpers need better to appear in one patch to show the pairing of
them.  That's what this patche does.

Then, from any helper POV it's better one helper introduced together in the
patch where it will be used to justify the helper with the applicable context.

>From that POV, the clean way, IMHO, should be that we have one prior patch
introducing the reader/writter helpers, together using it globally with
existing users.  Then the kvm patch can use the new helpers.

No strong opinion here, though..

Thanks,

-- 
Peter Xu


Reply via email to