public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
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
Date: Wed, 14 Dec 2005 02:12:18 -0800	[thread overview]
Message-ID: <20051214021218.0dad1f70.pj@sgi.com> (raw)
In-Reply-To: <8126E4F969BA254AB43EA03C59F44E840431BB3B@pdsmsx404>

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

  parent reply	other threads:[~2005-12-14 10:12 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2005-12-14  3:30 [PATCH 1/2] Export cpu info by sysfs Zhang, Yanmin
2005-12-14  4:14 ` Nathan Lynch
2005-12-14 10:12 ` Paul Jackson [this message]
  -- strict thread matches above, loose matches on Subject: below --
2005-12-15  1:33 Zhang, Yanmin
2005-12-15  2:03 ` Paul Jackson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20051214021218.0dad1f70.pj@sgi.com \
    --to=pj@sgi.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=venkatesh.pallipadi@intel.com \
    --cc=yanmin.zhang@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox