Date: Wed, 14 Dec 2005 02:12:18 -0800
From: Paul Jackson <pj@sgi.com>
To: "Zhang, Yanmin" <yanmin.zhang@intel.com>
Cc: linux-kernel@vger.kernel.org, venkatesh.pallipadi@intel.com
Subject: Re: [PATCH 1/2] Export cpu info by sysfs
Message-Id: <20051214021218.0dad1f70.pj@sgi.com>
In-Reply-To: <8126E4F969BA254AB43EA03C59F44E840431BB3B@pdsmsx404>
References: <8126E4F969BA254AB43EA03C59F44E840431BB3B@pdsmsx404>
Organization: SGI
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1853
Lines: 38

Some comments on your patch ...

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

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.)

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 won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <pj@sgi.com> 1.925.600.0401
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/