Re: [PATCH 2/2] x86: modify show_shared_cpu_map in intel_cacheinfo

Bert Wesarg wrote:
On Fri, Mar 28, 2008 at 3:40 PM, Mike Travis <travis@xxxxxxx> wrote:
Bert Wesarg wrote:
> On Fri, Mar 28, 2008 at 12:16 AM, Mike Travis <travis@xxxxxxx> wrote:
>> Used cpulist_scnprintf to print cpus on a leaf instead of requiring
>> a new "cpumask_scnprintf_len" function to determine the size of
>> the temporary buffer. cpulist_scnprintf can be used to print directly
>> to the output buffer, eliminating the need for the temporary buffer.
>>
>> Based on:
>> git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
>> git://git.kernel.org/pub/scm/linux/kernel/git/x86/linux-2.6-x86.git
>>
>> Signed-off-by: Mike Travis <travis@xxxxxxx>
>> ---
>> arch/x86/kernel/cpu/intel_cacheinfo.c | 25 +++++++++++++++++--------
>> 1 file changed, 17 insertions(+), 8 deletions(-)
>>
>> --- linux.trees.git.orig/arch/x86/kernel/cpu/intel_cacheinfo.c
>> +++ linux.trees.git/arch/x86/kernel/cpu/intel_cacheinfo.c
>> @@ -593,14 +593,23 @@ static ssize_t show_size(struct _cpuid4_
>>
>> static ssize_t show_shared_cpu_map(struct _cpuid4_info *this_leaf, char *buf)
>> {
>> - int n = 0;
>> - int len = cpumask_scnprintf_len(nr_cpu_ids);
>> - char *mask_str = kmalloc(len, GFP_KERNEL);
>> -
>> - if (mask_str) {
>> - cpumask_scnprintf(mask_str, len, this_leaf->shared_cpu_map);
>> - n = sprintf(buf, "%s\n", mask_str);
>> - kfree(mask_str);
>> + /*
>> + * cpulist_scnprintf() has the advantage of compressing
>> + * consecutive cpu numbers into a single range which seems
>> + * appropriate for cpus on a leaf. This will change what is
>> + * output so scripts that process the output will have to change.
> So this breaks user space?
>
> Bert

Potentially, yes. But I suspect with 4096 cpus, user scripts will have
to change anyways. Currently it is represented as sets of 32 bit mask
outputs with comma separators, so 1152 characters would be output.
But you can declare it as a programming error on user space side. If
you change the format, the brown-paper-bag is yours.

Is there a special notice I should provide to announce this change?
I hope so. At least lwn.net has an API changes site:

http://lwn.net/Articles/2.6-kernel-api/

I did look at that site. Besides being "kind of" out of date (last mod: 10/19/07),
it didn't appear to track changes in information displayed by proc/sysfs functions.


I also looked into MAINTAINERS, but it seems there is no official API
'maintainer'.

(And this output does conform with other syntax for printing and parsing
strings of bits.)
Aren't the most cpumaps (like cpu/cpu*/topology/*_siblings or
node/node*/cpumap) bitmasks?

I did an informal survey and you are right, the majority of references do use
cpumask_scnprintf instead of cpulist_scnprintf. Maybe the later function was
added later?

To me though, it would seem that:

240-255

is more readable than:

00000000,00000000,00000000,00000000,00000000,00000000,00000000,0000ffff

And as I mentioned, bitmask_parselist() [libbitmask(3)] does parse the output.

Thanks,
Mike
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Relevant Pages