Re: [PATCH] amdkfd: Drop interrupt SW ring buffer

["Michel Dänzer" <michel@daenzer.net>]

On 07.01.2015 21:24, Oded Gabbay wrote:
> Hi Michel,
> So your patch is quite, hmm, *drastic* :)
> 
> Instead, could I suggest to only remove the calls to kfd_interrupt_init()
> and kfd_interrupt_exit() ? It will also require a minor modification to the
> logic in kgd2kfd_interrupt() but it is much less intrusive than what you are
> suggesting.
> 
> Alternatively, we could take just this hunk:
> 
>> @@ -296,13 +286,5 @@ int kgd2kfd_resume(struct kfd_dev *kfd)
>>  /* This is called directly from KGD at ISR. */
>>  void kgd2kfd_interrupt(struct kfd_dev *kfd, const void *ih_ring_entry)
>>  {
>> -	if (kfd->init_complete) {
>> -		spin_lock(&kfd->interrupt_lock);
>> -
>> -		if (kfd->interrupts_active
>> -		    && enqueue_ih_ring_entry(kfd, ih_ring_entry))
>> -			schedule_work(&kfd->interrupt_work);
>> -
>> -		spin_unlock(&kfd->interrupt_lock);
>> -	}
>> +	/* Process interrupts / schedule work as necessary */
>>  }
> 
> 
> After all, we do need this feature eventually and most of it is fine,

Actually, I'm not sure I can agree with 'most of it is fine'. The whole
design of the SW ring buffer which is processed by a work queue seems
rather flawed, I'm not sure it's possible to reliably prevent the ring
buffer from overflowing like that, even while the hardware interrupt
ring is being processed without problems.


> so I don't want to take it all out. As I said, it is *drastic*.

Why do you think so? It's removing code which is currently essentially
dead but which is causing user visible problems. If it does turn out to
be useful later and the problems can be fixed, it'll be easy to
reinstate it from the Git history.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel