[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH for-1.4 qom-cpu 6/9] pc: Set fw_cfg data based o
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH for-1.4 qom-cpu 6/9] pc: Set fw_cfg data based on APIC ID calculation |
Date: |
Wed, 23 Jan 2013 18:32:29 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.1 (gnu/linux) |
Eduardo Habkost <address@hidden> writes:
> On Wed, Jan 23, 2013 at 05:33:25PM +0100, Andreas Färber wrote:
>> Am 22.01.2013 21:25, schrieb Eduardo Habkost:
[...]
>> > + for (cpu_idx = 0; cpu_idx < max_cpus; cpu_idx++) {
>> > + unsigned int apic_id = x86_cpu_apic_id_from_index(cpu_idx);
>> > + assert(apic_id < apic_id_limit);
>> > for (j = 0; j < nb_numa_nodes; j++) {
>> > - if (test_bit(i, node_cpumask[j])) {
>> > - numa_fw_cfg[i + 1] = cpu_to_le64(j);
>> > + if (test_bit(cpu_idx, node_cpumask[j])) {
>> > + numa_fw_cfg[apic_id + 1] = cpu_to_le64(j);
>> > break;
>> > }
>> > }
>> > }
>>
>> Why can't we keep using i here? That would leave the "for (..." and
>> "test_bit" lines unchanged and let us spot the actual changes of i vs.
>> apic_id more easily.
>
> It would make the patch simpler, but at the cost of keeping variable
> names opaque for people reading the code in the future. I believe
> readable code is more important than making patches smaller.
Both are important, and you can have both at the same time: put the
rename in its own patch. Whether that's justified in this case is
debatable.
- [Qemu-devel] [PATCH for-1.4 qom-cpu 4/9] target-i386/cpu: Introduce x86_cpu_apic_id_from_index() function, (continued)
[Qemu-devel] [PATCH for-1.4 qom-cpu 7/9] tests: Support target-specific unit tests, Eduardo Habkost, 2013/01/22
[Qemu-devel] [PATCH for-1.4 qom-cpu 3/9] fw_cfg: Remove FW_CFG_MAX_CPUS from fw_cfg_init(), Eduardo Habkost, 2013/01/22
[Qemu-devel] [PATCH for-1.4 qom-cpu 5/9] cpus.h: Make constant smp_cores/smp_threads available on *-user, Eduardo Habkost, 2013/01/22
[Qemu-devel] [PATCH for-1.4 qom-cpu 8/9] target-i386: Topology & APIC ID utility functions, Eduardo Habkost, 2013/01/22