On 12/06/2019 09:16, Julien Thierry wrote:
> Hi Marc,
>
> On 11/06/2019 18:03, Marc Zyngier wrote:
[...]
>> +
>> +void vgic_lpi_translation_cache_init(struct kvm *kvm)
>> +{
>> + struct vgic_dist *dist = &kvm->arch.vgic;
>> + unsigned int sz;
>> + int i;
>> +
>> + if (!list_empty(&dist->lpi_translation_cache))
>> + return;
>> +
>> + sz = atomic_read(&kvm->online_vcpus) * LPI_DEFAULT_PCPU_CACHE_SIZE;
>> +
>> + for (i = 0; i < sz; i++) {
>> + struct vgic_translation_cache_entry *cte;
>> +
>> + /* An allocation failure is not fatal */
>> + cte = kzalloc(sizeof(*cte), GFP_KERNEL);
>> + if (WARN_ON(!cte))
>> + break;
>> +
>> + INIT_LIST_HEAD(&cte->entry);
>> + list_add(&cte->entry, &dist->lpi_translation_cache);
>
> Going through the series, it looks like this list is either empty
> (before the cache init) or has a fixed number
> (LPI_DEFAULT_PCPU_CACHE_SIZE * nr_cpus) of entries. And the list never
> grows nor shrinks throughout the series, so it seems odd to be using a
> list here.
>
> Is there a reason for not using a dynamically allocated array instead of
> the list? (does list_move() provide a big perf advantage over swapping
> the data from one array entry to another? Or is there some other
> facility I am missing?
>
Scratch that, I realized having the list makes it easier to implement
the LRU policy later in the series.
--
Julien Thierry
