RE: [PATCH 1/2] Export cpu info by sysfs

[Zhang, Yanmin]

>>-----Original Message-----
>>From: Paul Jackson [mailto:pj@xxxxxxx]
>>Sent: 2005年12月14日 18:12
>>To: Zhang, Yanmin
>>Cc: linux-kernel@xxxxxxxxxxxxxxx; Pallipadi, Venkatesh
>>Subject: Re: [PATCH 1/2] Export cpu info by sysfs
>>
>>Some comments on your patch ...
I really appreciate your comments.

>>
>>1) It's easier for others to read patches if they are inline text,
>>   or at least attached as text, not as base64.  See further the
>>   kernel source file: Documentation/SubmittingPatches.  If your
>>   company's email client has difficulty attaching patches without
>>   mangling them, then perhaps you will have better luck with a
>>   dedicated patch sending program, such as one I support:
>>	http://www.speakeasy.org/~pj99/sgi/sendpatchset
Thanks. I use outlook and tried many approaches before, but patches always have unexpected line breaks when I attach them as inline text. Anyway, I found a new approach to avoid that. Next time, I will paste patches as inline.

>>
>>2) > cpumask_scnprintf(buf, NR_CPUS+1, cpu_core_map[cpu]);
>>
>>   The 2nd arg, "NR_CPUS+1", is wrong.  It should be the length
>>   of the buffer (1st arg, "buf").  Unfortunately, you probably
>>   aren't passed that length by sysfs.  Your routine was likely
>>   passed a page, so assuming a size of PAGE_SIZE/2 would work
>>   (big enough to print a cpumask, small enough not to allow
>>   buffer overrun.)
In theory, it's a problem which doesn't exist in fact. The smallest page size on IA64 is 4KB (default is 16KB) and cpumask_scnprintf uses hex format, so one page could store cpumask of (4K-1)*4 cpu. I can't imagine a machine has more than (4K-1)*4 cpu.

>>
>>3) The patch needs to include reasonable documentation (not
>>   just the patch header that goes in the source control log,
>>   but also documentation that will into the source file and/or
>>   into the Documentation directory.)  Unfortunately, it seems
>>   that the rest of /sys/devices/system is not directly documented
>>   under Documentation, except as it pertains to such subjects as
>>   cpufreq, laptop, numastat and hotplug.  So until someone takes
>>   on the challenge of documenting the rest of this /sys hierarchy,
>>   I see no obvious place to add such items as you propose.
I agree with you.

-
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/