Re: [PATCH 3/5] Move irq notifiers lists to its own locking.

[Gregory Haskins <gregory.haskins@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1847 bytes --]

Gleb Natapov wrote:
> On Mon, Jul 13, 2009 at 05:49:20PM +0300, Michael S. Tsirkin wrote:
>   
>> On Mon, Jul 13, 2009 at 05:37:50PM +0300, Gleb Natapov wrote:
>>     
>>> On Mon, Jul 13, 2009 at 05:23:20PM +0300, Michael S. Tsirkin wrote:
>>>       
>>>> On Mon, Jul 13, 2009 at 02:48:44PM +0300, Gleb Natapov wrote:
>>>>         
>>>>> On Mon, Jul 13, 2009 at 02:45:51PM +0300, Michael S. Tsirkin wrote:
>>>>>           
>>>>>> On Mon, Jul 13, 2009 at 12:12:33PM +0300, Gleb Natapov wrote:
>>>>>>             
>>>>>>> Signed-off-by: Gleb Natapov <gleb@redhat.com>
>>>>>>>               
>>>>>> This one is probably better off left as is,
>>>>>>             
>>>>> What do you mean "as is"?
>>>>>           
>>>> This is a slow operation.  It seems that we could use irq_lock or switch
>>>> to slot lock or kvm lock here. Why do we need another one?
>>>>
>>>>         
>>> irq_lock is completely removed. So either we don't remove it and use it
>>> here (and we don't need mutex so we change it to spinlock too), or we add
>>> another lock with the name that actually tell us what its purpose. I prefer
>>> second option. I am not sure you can use kvm lock without deadlock, and
>>> slot lock? How this connected to slots management?!
>>>
>>> And this is not about speed of the operation. It is about making reader
>>> lockless.
>>>       
>> So, to summarize: this patch does not help speed irq injection up, the
>> only reason to change locking here is cosmetical.  Is this a fair
>> summary?
>>
>>     
> The whole series helps to speed irq injection up. This patch is one step
> towards the goal.
>   


FWIW: Improving the injection path in the manner Gleb is proposing will
pave the way to skip the work-queue deferrment in the irqfd signal
path.  This is a good thing.

Regards,
-Greg




[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 266 bytes --]