On Wed, 23 Aug 2023 18:01:59 +0200
Alexander Gordeev <agordeev@xxxxxxxxxxxxx> wrote:
> On Wed, Aug 23, 2023 at 04:09:26PM +0200, Michael Mueller wrote:
> >
> >
> > On 23.08.23 15:23, Alexander Gordeev wrote:
> > > On Wed, Aug 23, 2023 at 02:41:40PM +0200, Michael Mueller wrote:
> > > ...
> > > > diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
> > > > index 9bd0a873f3b1..73153bea6c24 100644
> > > > --- a/arch/s390/kvm/interrupt.c
> > > > +++ b/arch/s390/kvm/interrupt.c
> > > > @@ -3205,8 +3205,10 @@ void kvm_s390_gisa_destroy(struct kvm *kvm)
> > > > if (gi->alert.mask)
> > > > KVM_EVENT(3, "vm 0x%pK has unexpected iam 0x%02x",
> > > > kvm, gi->alert.mask);
> > > > - while (gisa_in_alert_list(gi->origin))
> > > > - cpu_relax();
> > > > + while (gisa_in_alert_list(gi->origin)) {
> > > > + KVM_EVENT(3, "vm 0x%pK gisa in alert list during destroy", kvm);
> > > > + process_gib_alert_list();
> > >
> > > process_gib_alert_list() has two nested loops and neither of them
> > > does cpu_relax(). I guess, those are needed instead of one you remove?
> >
> > Calling function process_gib_alert_list() guarantees the gisa
> > is taken out of the alert list immediately and thus the potential
> > endless loop on gisa_in_alert_list() is solved. The issue surfaced
> > with the following patch that accidently disabled the GAL interrupt
> > processing on the host that normaly handles the alert list.
> > The patch has been reverted from devel and will be re-applied in v2.
> >
> > 88a096a7a460 Revert "s390/airq: remove lsi_mask from airq_struct"
> > a9d17c5d8813 s390/airq: remove lsi_mask from airq_struct
> >
> > Does that make sense for you?
>
> Not really. If process_gib_alert_list() does guarantee the removal,
> then it should be a condition, not the loop.
this is actually a good question. why is it still a loop?
>
> But I am actually not into this code. Just wanted to point out that
> cpu_relax() is removed from this loop and the two other loops within
> process_gib_alert_list() do not have it either.
>
> So up to Christian, Janosch and Claudio.
