Re: [RFC PATCH 5/7] ARM: KVM: kill stage2_set_pte

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Feb 21, 2013 at 6:05 AM, Marc Zyngier <marc.zyngier@xxxxxxx> wrote:
> On Wed, 20 Feb 2013 15:37:49 -0800, Christoffer Dall
> <cdall@xxxxxxxxxxxxxxx> wrote:
>> On Wed, Feb 13, 2013 at 03:47:02PM +0000, Marc Zyngier wrote:
>>> kvm_set_spte_handler is the last user of stage2_set_pte.
>>> Convert it to the new get/set_at operations, and kill stage2_set_pte.
>>>
>>> Signed-off-by: Marc Zyngier <marc.zyngier@xxxxxxx>
>>> ---
>>>  arch/arm/kvm/mmu.c | 17 ++++-------------
>>>  1 file changed, 4 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c
>>> index d033344..633b546 100644
>>> --- a/arch/arm/kvm/mmu.c
>>> +++ b/arch/arm/kvm/mmu.c
>>> @@ -458,18 +458,6 @@ static void stage2_set_pte_at(struct kvm *kvm,
>>> phys_addr_t addr,
>>>              get_page(virt_to_page(pte));
>>>  }
>>>
>>> -static int stage2_set_pte(struct kvm *kvm, struct kvm_mmu_memory_cache
>>> *cache,
>>> -                      phys_addr_t addr, const pte_t *new_pte)
>>> -{
>>> -    pte_t *pte;
>>> -
>>> -    pte = stage2_get_pte(kvm, cache, addr);
>>> -    if (pte)
>>> -            stage2_set_pte_at(kvm, addr, pte, *new_pte);
>>> -
>>> -    return 0;
>>> -}
>>> -
>>
>> Ah, you did this later on. I don't see why you want to keep the name
>> stage2_set_pte_at though. Rationale?
>
> set_pte_at is the "normal" kernel interface (for an admittedly twisted
> definition of "normal"). It really helps to have similar function names
> (specially when you're trying to explain something to a kernel hacked that
> is not familiar with KVM's internal stuff).
>
I know that part, but set_pte_at also has the other twisted idea of
setting random other bits.

For consistency, why don't we call the whole thing stage2_get_pte_at
and stage2_get_pmd_at, ...

Honestly it sounds to me like the _at postfix was added because
someone wanted some other level of indirection and couldn't come up
with a better naming standard. This function is different from the
standard kernel function and calling it something with a connotation
that indicates that it's largely the same, instead of using the more
"natural" name, is just confusing to me.

Maybe this is brain damage from the voodoo bug, but I'm no longer
friends with set_pte_at :)
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm


[Index of Archives]     [Linux KVM]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux