lkml.org 
[lkml]   [2005]   [Mar]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [patch 0/8] CKRM: Core patch set
Date

On Tue, 29 Mar 2005 22:05:30 PST, Paul Jackson wrote:
> gerrit wrote:
> > This is the core patch set for CKRM
>
> Welcome.

Hi Paul.

> Newcomers to CKRM might want to start reading these patches with "[patch
> 8/8] CKRM: Documentation". Starting with patch 0/8 or 1/8 will be
> difficult, at least if you're as dimm witted as I am.
>
> Even the documentation included in patch 8/8 is missing the motivation
> and context essential to understanding this patch set. It might have
> helped if the Introduction text at http://ckrm.sourceforge.net/ had been
> included in some form, as part of patch 0/8. I'm just a little penguin
> here (lkml), but from what I can tell by watching how things work,
> you're going to have to "make the case" -- explain what this is, how
> it's put togeher, and why it's needed. This is a sizable patch, in
> lines of code, in hooks in critical places, and in amount of "new
> concepts." I presume (unless you've managed to bribe or blackmail some
> big penguin) you're going to have convince some others that this is
> worth having. I for one am a CKRM skeptic, so won't be much help to you
> in that quest. Good luck.

Good point on including the pointer to the web site. As you probably
noticed, there is a history of the design, papers presented, etc.
Also, Jonathan Corbet did a nice write up from the discussion at the
2004 Kernel summit which is archived here: http://lwn.net/Articles/94573/
which may be of use.

The OLS and LinuxTag papers are archived at the site that you pointed
to and there will be a tutorial on configuring, using and writing
controllers for CKRM at OLS this year. You may also want to see the
previous postings of this code to LKML for more background.

In short, CKRM provides very basic desktop to server workload management
capabilities similar to those provided by most of the old fashioned
operating systems. The code provides a fairly simple mechanism for
adding controllers for any resource type and the code is currently
widely deployed by PlanetLab, a part of Novell/SuSE's distro, and
the capabilities are requested by a fair number of Linux users and
customers.

> I don't see any performance numbers, either on small systems, or
> scalability on large systems. Certainly this patch does not fall under
> the "obviously no performance impact" exclusion.

Fair point. We have been running some of the smaller benchmarks but
have not yet had a chance to do any kind of performance comparison
based on the current code. However, when configured out, it will
have zero impact. We do have some performance analysis of the code
with CONFIG_CKRM set to y but no rules configured planned for the
very near future.

> A couple of nits:
>
> 1) Instead of disabling routines with #defines:
> #define numtasks_put_ref(core_class) do {} while (0)
> one can do it with static inlines, preserving more compiler
> checking.

Yeah - that works well in some cases but it turns out to not do so
well when an argument to a function refers to a structure element
which is not configured in. In that case, the compiler emits a
reference to an undefined structure value in the case of the static
inline, where otherwise the entire set of code is pre-processed
away. I think we've gone through the code and used the correct
balance of static inlines and #define constructs as appropriate.
If we've missed any, I'm more than willing to accept a patch to
correct a specific instance.

> 2) I take it that the following constitutes the 'documentation'
> for what is in /proc/<pid>/delay. Perhaps I missed something.
>
> + res = sprintf(buffer,"%u %llu %llu %u %llu %u %llu\n",
> + (unsigned int) get_delay(task,runs),
> + (uint64_t) get_delay(task,runcpu_total),
> + (uint64_t) get_delay(task,waitcpu_total),
> + (unsigned int) get_delay(task,num_iowaits),
> + (uint64_t) get_delay(task,iowait_total),
> + (unsigned int) get_delay(task,num_memwaits),
> + (uint64_t) get_delay(task,mem_iowait_total)

The code is the documentation? :)

There is probably some documentation on /proc/<pid>/ in general and
we'll see if we can get it updated appropriately. Vivek?

> 3) Typo in init/Kconfig "atleast":
>
> If you say Y here, enable the Resource Class File System and atleast

Got it - thanks! Someone liked the new word "atleast" - at least
three occurences removed.

Oh - and uniformly updated diffstats - I probably missed some when
I was playing with quilt originally.

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

\
 
 \ /
  Last update: 2005-04-06 13:31    [from the cache]
©2003-2014 Jasper Spaans. Advertise on this site