On 04/03/2013 11:42 PM, Paul Mackerras wrote:
> On Mon, Mar 25, 2013 at 01:57:08PM -0500, Nathan Fontenot wrote:
>> From: Jesse Larrew <jlar...@linux.vnet.ibm.com>
>>
>> Platform events such as partition migration or the new PRRN firmware
>> feature can cause the NUMA characteristics of a CPU to change, and these
>> changes will be reflected in the device tree nodes for the affected
>> CPUs.
>>
>> This patch registers a handler for Open Firmware device tree updates
>> and reconfigures the CPU and node maps whenever the associativity
>> changes. Currently, this is accomplished by marking the affected CPUs in
>> the cpu_associativity_changes_mask and allowing
>> arch_update_cpu_topology() to retrieve the new associativity information
>> using hcall_vphn().
>>
>> Protecting the NUMA cpu maps from concurrent access during an update
>> operation will be addressed in a subsequent patch in this series.
>>
>> Signed-off-by: Nathan Fontenot <nf...@linux.vnet.ibm.com>
>
> [snip]
>
>> + if (firmware_has_feature(OV5_PRRN)) {
>
> Shouldn't this be FW_FEATURE_PRRN? How well has this patch been
> tested? :-/
Yes this should have been FW_FEATURE_PRRN.
I know I tested this and it took some digging to find out why my test succeeded
even though I used the wrong value in the call to firmware_has_feature. The
value
for OV5_PRRN (0x0540) just happens to match some of he bits that are set in
powerpc_firmware_features bit field and cause the check to return true. My test
worked out of sheer luck. I'll update this patch and re-test to ensure it works
with the real value.
This does make me think, should we update firmware_has_feature() to avoid this
kind of false positive in the future. something like
#define firmware_has_feature(feature)
\
((FW_FEATURE_ALWAYS & (feature)) == (feature) ||
\
(FW_FEATURE_POSSIBLE & powerpc_firmware_features & (feature)) ==
(feature)
-Nathan
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
