On 8/22/2018 1:49 PM, Jason Gunthorpe wrote:
> On Tue, Aug 21, 2018 at 04:01:09PM -0500, Steve Wise wrote:
>> Hey RDMAers,
>>
>> I see that a uverbs completion event is added to the ev_queue event_list
>> as well as the associated uobject comp_list in ib_uverbs_comp_handler():
>>
>> list_add_tail(&entry->list, &ev_queue->event_list);
>> list_add_tail(&entry->obj_list, &uobj->comp_list);
>> spin_unlock_irqrestore(&ev_queue->lock, flags);
>>
>>
>> But in ib_uverbs_comp_event_close(), it looks like the entry could be
>> left on the uobj comp_list and then the event structure is freed!
>
> I don't think we can get entries on the ev_queue for the
> uverbs_event_fops FD that do not have counter set.
>
> It should be changed, it is confusing, but the change should be to
> delete this:
>
> @@ -436,8 +436,7 @@ static int ib_uverbs_comp_event_close(struct inode *inode, struct file *filp)
>
> spin_lock_irq(&file->ev_queue.lock);
> list_for_each_entry_safe(entry, tmp, &file->ev_queue.event_list, list) {
> - if (entry->counter)
> - list_del(&entry->obj_list);
> + list_del(&entry->obj_list);
> kfree(entry);
> }
>
> The only events that should be pushed there are completion events, via
> ib_uverbs_comp_handler and they always have non-NULL counter.
>
> Ie this in ib_uverbs_comp_handler:
>
> entry->counter = &uobj->comp_events_reported;
>
> Is never NULL, it is the address of a structure member.
>
> Jason
>
Thanks Jason,
This then leaves me with the mystery of why I see "duplicate add"
warnings from list_add_tail() called from ib_uverbs_comp_handler(). I
wonder what other condition/state can cause this?
Hmm...I'll keep digging.
Steve.
