extract two functions from update_cpumask() and update_nodemask().They will be
used later for updating tasks' cpus_allowed and mems_allowed after CPU/NODE
offline/online.

NOTE: In original code of update_cpumask(), the variable heap's memory space was
allocated before changing the cpuset's cpus_allowed. Now we do it in the
function update_tasks_cpumask after changing the cpuset's cpus_allowed. Similar
to update_nodemask().

Signed-off-by: Miao Xie <miaox@cn.fujitsu.com>

---
 kernel/cpuset.c |  180 +++++++++++++++++++++++++++++++++---------------------
 1 files changed, 110 insertions(+), 70 deletions(-)

diff --git a/kernel/cpuset.c b/kernel/cpuset.c
index 86ea9e3..2c2aafa 100644
--- a/kernel/cpuset.c
+++ b/kernel/cpuset.c
@@ -766,6 +766,38 @@ static void cpuset_change_cpumask(struct task_struct *tsk,
 }
 
 /**
+ * update_tasks_cpumask - Scan tasks in cpuset cs, and update the cpumasks of
+ * any that need an update.
+ * @cs: the cpuset in which each task's cpus_allowed mask needs to be changed
+ *
+ * Called with cgroup_mutex held
+ *
+ * The cgroup_scan_tasks() function will scan all the tasks in a cgroup,
+ * calling callback functions for each.
+ *
+ * Return 0 if successful, -errno if not.
+ */
+static int update_tasks_cpumask(struct cpuset *cs)
+{
+	struct cgroup_scanner scan;
+	struct ptr_heap heap;
+	int retval = 0;
+
+	retval = heap_init(&heap, PAGE_SIZE, GFP_KERNEL, &started_after);
+	if (retval)
+		return retval;
+
+	scan.cg = cs->css.cgroup;
+	scan.test_task = cpuset_test_cpumask;
+	scan.process_task = cpuset_change_cpumask;
+	scan.heap = &heap;
+	retval = cgroup_scan_tasks(&scan);
+
+	heap_free(&heap);
+	return retval;
+}
+
+/**
  * update_cpumask - update the cpus_allowed mask of a cpuset and all tasks in it
  * @cs: the cpuset to consider
  * @buf: buffer of cpu numbers written to this cpuset
@@ -773,8 +805,6 @@ static void cpuset_change_cpumask(struct task_struct *tsk,
 static int update_cpumask(struct cpuset *cs, char *buf)
 {
 ...
[ message continues ]

Nice work.

As usual, I have a few minor items to note.

 1) No need to initialize 'retval = 0' since retval is set
    unconditionally in the next line of update_tasks_cpumask():

	+static int update_tasks_cpumask(struct cpuset *cs)
	+{
	+	struct cgroup_scanner scan;
	+	struct ptr_heap heap;
	+	int retval = 0;
	+
	+	retval = heap_init(&heap, PAGE_SIZE, GFP_KERNEL, &started_after);

 2) Same thing in update_tasks_nodemask - no need to initialize
    retval = 0.
    
    These initializations use an extra instruction or two, so we
    avoid them, if it is really clear from the code that the
    variable is set before used anyway.
    

 3) The special style for documenting functions, starting with the
    "/**" comment, which is documented in:
        Documentation/kernel-doc-nano-HOWTO.txt
    results in adding that functions documentation to various man pages
    and documents that are generated from these comment blocks.
    Typically, we do not document file static routines this way,
    because such routines cannot be called from outside that file,
    so are of little use to others.  Best not to clutter the more
    widely distributed documentation with details of functions that
    others can't call anyway.

    So I would suggest -removing- the special commenting convention
    for the routines update_tasks_cpumask() and update_tasks_nodemask().
    
    For example, in the update_tasks_nodemask() case, this means
    changing from:

    > /**
    >  * update_tasks_nodemask - Scan tasks in cpuset cs, and update the nodemasks of
    >  * any that need an update.
    >  * @cs: the cpuset in which each task's mems_allowed mask needs to be changed
    >  * @oldmem: old mems_allowed of cpuset cs
    >  *
    >  * Called with cgroup_mutex held
    >  * Return 0 if successful, -errno if not.
    >  */
    > static int update_tasks_nodemask(struct cpuset *cs, const nodemask_t *oldmem)

    to:


    > /*
    >  * update_tasks_nodemask - Scan tasks in ...
[ message continues ]

on 2008-5-29 16:16 Paul Jackson wrote:

I think that it is unnecessary to change cpuset_cpus_allowed_locked()'s comment
because it isn't a static function, it is a extern function and it is called by



--

[ message continues ]

Yes, you are correct.  My mistake.  Sorry.

-- 
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <pj@sgi.com> 1.940.382.4214
--

[ message continues ]

I check kernel/cpuset.c and find many static functions with "/**" comment.
So I want to remove the special commenting convention for them.

--

[ message continues ]

Right you are.  Offhand, in kernel/cpuset.c, I see:

    static int cpuset_test_cpumask(struct task_struct *tsk,
    static void cpuset_change_cpumask(struct task_struct *tsk,
    static int update_cpumask(struct cpuset *cs, char *buf)
    static void cpuset_do_move_task(struct task_struct *tsk,
    static void move_member_tasks_to_cpuset(struct cpuset *from, struct cpuset *to)

all having "/**" header comments.  I would be glad to Ack a patch
from you to fix such comments.  Thank-you.

-- 
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <pj@sgi.com> 1.940.382.4214
--

[ message continues ]

Uh.. We strongly want non-static functions to be documented via kernel-doc.
For static functions, it's up to the maintainer/developer whether to do that
or not.  But if the functions already have kernel-doc, there's no strong
reason to remove it.  And these look good currently, so I see no
good reason to change them.



---
~Randy
"He closes his eyes and drops the goggles.  You can't get hurt
by looking at a bitmap.  Or can you?"
--

[ message continues ]

Ok - thank-you for the explanation.

I had this vague recollection that kernel-doc was just for non-static
functions, so when I saw Miao put kernel-doc comments on a static
routine, I looked around to see if that was normal.

I didn't see mention of static functions in kernel-doc-nano-HOWTO.txt,
and in a brief survey, didn't happen to see any static functions with
kernel-doc comments.  However my survey was so brief I even missed five
such functions in the file I maintain, kernel/cpuset.c.  So from that
I came to the (apparently flawed) conclusion that it didn't make sense
to kernel-doc static functions, and so advised Miao.

I don't quite see what purpose kernel-doc would have for static
functions, and am still concerned that such would (mildly) clutter the
presentation of the externally visible cpuset functions in which those
who just use cpusets would be interested.

It remains, of course, important to have good documentation of
functions, static or not, but I would expect developers working inside
the kernel/cpuset.c file to look for documentation of functions
static to that file within the code and comments of that file, not in
kernel-doc.

However I am just the cpuset maintainer, not a kernel-doc expert.

I remain inclined toward removing the kernel-doc markings on static
functions in kernel/cpuset.c, but if you wish, Randy, to make a renewed
appeal for me not to do so, I will happily honor that appeal.

Life is good either way when we are discussing how to annotate
documentation, rather than lamenting its absence.

If you, Randy, felt inclined to explain the value of kernel-doc comments
on file static functions in kernel-doc-nano-HOWTO.txt, that might be
valuable for the next person who travels this path.

Thank-you.

-- 
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <pj@sgi.com> 1.940.382.4214
--

[ message continues ]

Well, they won't clutter the generated kernel-doc docbook output unless
you/we/someone requests that both non-static and static functions be listed


a.  I looked at those 5 functions.  They should be documented, whether
it's done with kernel-doc or some other way.  I don't see why there would
be a problem with using kernel-doc instead of something else.


It might need a little tweaking.  Basically what is already there says:

"""
!E<filename> is replaced by the documentation, in <filename>, for
functions that are exported using EXPORT_SYMBOL: the function list is
collected from files listed in Documentation/DocBook/Makefile.

!I<filename> is replaced by the documentation for functions that are
_not_ exported using EXPORT_SYMBOL.
"""

so if a docbook (like one generated from Documentation/DocBook/kernel-api.tmpl)
uses both of these:
!Ekernel/cpuset.c
!Ikernel/cpuset.c

then that docbook will contain both exported functions and non-exported ones.
To produce a docbook with only exported symbols, use only
!Ekernel/cpuset.c



Same to you, sir.

Does that help?  your understanding of kernel-doc or your decision?

---
~Randy
"He closes his eyes and drops the goggles.  You can't get hurt
by looking at a bitmap.  Or can you?"
--

[ message continues ]

Ok ... hmmm ... how to say this...

The documentation in Documentation/kernel-doc-nano-HOWTO.txt of
kernel-doc is mostly focused on the details of the apparatus that
converts the "/**" comments into various documenation formats, and has
only somewhat buried and non-inviting documentation for kernel hackers
wanting the "Kernel-Doc for Dummies" summary of how and when and where
to create such "/**" comments.

For example, the distinction you note between:
  !E<filename> external (EXPORT_SYMBOL), and
  !I<filename> internal (not exported)
is buried in the "How to make new SGML template files" section, which I
would hope that I never had to read.

However ... in accordance with the usual rule that he who complains
gets to fix it, I find this kernel-doc documentation to be PERFECT! ;).

-- 
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <pj@sgi.com> 1.940.382.4214
--

[ message continues ]

Well, I get the difference between E (exported) and I (non-exported)
now.  And I see that one could prepare documents using SGML templates
that contained one, or the other of these, for any kernel source files
of interest.

I'm stuck on the next step of this decision.

Usually, when I am preparing documens, I know what document I am
preparing and have an idea who is in its audience.

I have never seen or heard of a document using the "/**" kernel-doc
entries of kernel/cpuset.c, and I have no idea who actually has (in
the past or present, not just hypothetically) read such or why.

So I'm kinda shootin in the dark here.

So, mostly just to be consistent with my previous call, because I enjoy
being a stubborn retard, I continue to prefer that file static routines
in kernel/cpuset.c not have "/**" kernel-doc markers on their comments,
and I would still welcome a patch from Miao removing the ones already
there.

Enough of this discussion, from me at least.

-- 
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <pj@sgi.com> 1.940.382.4214
--

[ message continues ]

And I'll join that by stubbonly NAKing such an approach. We want
consistent clear formats for documentation extraction. Simply getting
people to use the kerneldoc format materially improves the documentation
quality, so please don't set bad examples ;)

Alan

--

[ message continues ]

> And I'll join that by stubbonly NAKing such an approach. 

That's good - thanks, Alan.

I'll ack Alan's nak of my ack ;).

-- 
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <pj@sgi.com> 1.940.382.4214
--

[ message continues ]

You are certainly welcome to add kernel/cpuset.c to kernel-api.tmpl or any other


---
~Randy
"He closes his eyes and drops the goggles.  You can't get hurt
by looking at a bitmap.  Or can you?"
--

[ message continues ]

Well, that's some reason for your decision.  At least you aren't just being
arbitrary.  I agree with Alan, of course.  I don't think that you have stated
a Good reason for removing the kernel-doc on the static functions, but it is


---
~Randy
"He closes his eyes and drops the goggles.  You can't get hurt
by looking at a bitmap.  Or can you?"
--

[ message continues ]

So be it ... more kernel-doc comments are good,
static or not.

-- 
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <pj@sgi.com> 1.940.382.4214
--

[ message continues ]