Re: [Xen-devel] [PATCH RFC 1/3] x86/vpt: execute callbacks for masked interrupts

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 26.02.18 at 13:48, <roger.pau@xxxxxxxxxx> wrote:
> On Mon, Feb 26, 2018 at 12:35:54PM +0000, Wei Liu wrote:
>> On Fri, Feb 23, 2018 at 01:27:41PM +0000, Roger Pau Monne wrote:
>> >  int pt_update_irq(struct vcpu *v)
>> >  {
>> >      struct list_head *head = &v->arch.hvm_vcpu.tm_list;
>> > +    LIST_HEAD(purged);
>> 
>> to_purge?
> 
> My point is that they have already been purged from the pt->list, but
> I really don't have a preference.
> 
>> >      struct periodic_time *pt, *temp, *earliest_pt;
>> >      uint64_t max_lag;
>> >      int irq, is_lapic, pt_vector;
>> > @@ -267,7 +289,10 @@ int pt_update_irq(struct vcpu *v)
>> >              {
>> >                  /* suspend timer emulation */
>> >                  list_del(&pt->list);
>> > -                pt->on_list = 0;
>> > +                if ( pt->cb )
>> > +                    list_add(&pt->list, &purged);
>> > +                else
>> > +                    pt->on_list = 0;
>> >              }
>> >              else
>> >              {
>> > @@ -283,6 +308,7 @@ int pt_update_irq(struct vcpu *v)
>> >      if ( earliest_pt == NULL )
>> >      {
>> >          spin_unlock(&v->arch.hvm_vcpu.tm_lock);
>> > +        execute_callbacks(v, &purged);
>> 
>> It would be better to check if the list is not empty before calling the
>> function to avoid the extra lock / unlock.
> 
> The lock is also protecting the 'purged' list, so I think that for
> consistency the lock needs to be held before accessing it.

But that's a local list, isn't it? No-one else can access it.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel