Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <365fe273-ba11-eb12-4d80-a2e6a17bf0fa@redhat.com>
Date:   Fri, 23 Dec 2022 16:32:53 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.5.1
Subject: Re: [PATCH] KVM: use unified srcu interface function
Content-Language: en-US
To:     Hao Peng <flyingpenghao@gmail.com>,
        Sean Christopherson <seanjc@google.com>
Cc:     kvm@vger.kernel.org, linux-kernel@vger.kernel.org
References: <CAPm50aJTh7optC=gBXfj+1HKVu+9U0165mYH0sjj3Jqgf8Aivg@mail.gmail.com>
 <Y5KNvgzakT1Vvxy4@google.com>
 <CAPm50aJv2_6321BgLXB6SWH1CcoYM4733fsovtB_5zhoP_7x+Q@mail.gmail.com>
From:   Paolo Bonzini <pbonzini@redhat.com>
In-Reply-To: <CAPm50aJv2_6321BgLXB6SWH1CcoYM4733fsovtB_5zhoP_7x+Q@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 12/20/22 08:47, Hao Peng wrote:
>>> +       old = srcu_dereference_check(kvm->irq_routing, &kvm->irq_srcu,
>>> +                                       lockdep_is_held(&kvm->irq_lock));
>> Readers of irq_routing are protected via kvm->irq_srcu, but this writer is never
>> called with kvm->irq_srcu held.  I do like the of replacing '1' with
>> lockdep_is_held(&kvm->irq_lock) to document the protection, so what about just
>> doing that?  I.e.
>>
> Sorry for the long delay in replying. Although kvm->irq_srcu is not required
> to protect irq_routing here, this interface function srcu_dereference_check
> indicates that irq_routing is protected by kvm->irq_srcu in the kvm subsystem.
> Thanks.
> 

I agree, the last two arguments basically are alternative conditions to 
satisfy the check:

#define srcu_dereference_check(p, ssp, c) \
         __rcu_dereference_check((p), __UNIQUE_ID(rcu), \
                                 (c) || srcu_read_lock_held(ssp), __rcu)

The idea is to share the code between readers and writers, so what do 
you think of adding a

#define kvm_get_irq_routing(kvm) srcu_dereference_check(...)

macro at the top of virt/kvm/irqchip.c?

Thanks,

Paolo