Re: [PATCH -mm 1/5] Blackfin: blackfin architecture patch update
- From: Robin Getz <rgetz@xxxxxxxxxxxxxxxxxxxx>
- Date: Mon, 5 Mar 2007 17:06:10 -0500
On Mon 5 Mar 2007 12:32, Paul Mundt pondered:
On Mon, Mar 05, 2007 at 11:29:19AM -0500, Robin Getz wrote:
On Mon 5 Mar 2007 09:00, Paul Mundt pondered:
Throwing this all at the user simply shows that the functions being
relocated haven't been profiled adequately with real workloads.
Actually - that is not true at all - we have profiled these
extensively
across many workloads on various real world applications from fax
machine to bar code scanner, to software radio, to internet radio -
and that is the point - it all depends on what the user
application/drivers
that are loaded are doing. I can't decided what is best for the user,
because I don't know what they are doing.
And there are just as many cases that are common regardless of the
workload that are simply good decisions. Exception vectors are a good
example, either you get a boost from having them in L1 memory or you
don't, the workload really doesn't matter.
Then you have not profiled the same applications I have. I see different
results in on this platform.
Workloads will of course have variations, but it's optimizing the hot
paths that end up being quite common that end up being the most useful.
These are not things that should be required from the end user.
The end user is the only one who knows what is work load is.
If a user wants to use on-chip memory, presumably they have a target
application in mind, and they know how much space they need. Beyond
that, they expect the kernel to do the best it can with the space
that's left over for it to play with.
And that is the problem - the kernel doesn't know how much on chip
memory an application which will be loaded in the future might need.
The user needs to be able to control the amount of on-chip memory
that the kernel uses - hence the knobs.
That's crap, for the exact reason that these are not run-time
configurable knobs, they are static build-time components. If a user has
to scale back the kernel users until their application fits, then they
already know how much space they're going to need. Once you know that,
reserving a chunk for userspace and limiting the kernel's usage is quite
simple.
OK - you want me to remove 14 selections (yeah - I agree that is alot), and
replace them with one selection, and a header file, that the user has to
maintain separately from the .config, which everyone does already? IMHO -
that is harder for an end user to maintain than our existing proposal.
If a user has to sit around profiling their workload to
figure out what config options to set to chew through the rest of
the L1 memory, you've completely lost at intuitive design.
Embedded has never been intuitive.
Embedded has also never been about throwing everything at the user
that's possible. Flexibility is good. Intelligent design is good.
Blindly tossing everything the user's way and hoping for the best
is not.
default settings?
Hope is not a strategy that we use. Careful design is (most of the time).
Normally, the majority of end users leave things set by the default,
meaning their application doesn't use the on-chip memory, and the
kernel is allowed to. If they want to start using on-chip memory,
they are forced to do a re-compile, with the kernel functions
using off-chip memory, and see how much of on-chip memory
their application uses. They calculate the difference, and
do a little system level profiling, and put the most used kernel
functions back.
Yes, precisely. This is all something that should be done by the kernel
_for_ the user. They can of course add their own instrumentation to the
hot paths that their workload hits that aren't generally handled, but
this should not be a required step.
If I'm working on an embedded application, I expect my kernel to do some
sensible things with the hardware available. I don't want to have to
manually configure ever possible thing in order to get a system that
behaves in a reasonable manner, and I certainly don't want to have to do
this multiple times with a profiler for paths that are inevitably going
to be quite common anyways.
People do all kinds of things to make their application work better on a
processor that cost less than lunch for one person.
The applications always work without system level tuning - but they can work
up to 2x faster with a little performance tweaking. To most people, spending
the day or two - doing the tweaking to get max performance that the chip is
capable of is worth the effort.
You are asking me to try to decide what is best for all the potential
end users, with out knowing what the end user is doing. What ends up
happening is that people make modifications (normally poor ones) and
then still ask for help when their modifications break.
No, I'm asking you to set some sensible defaults, provide an interface
that works for the common cases, and allow the special cases to remain
simply that.
In my opinion - that is what we have.
I would rather give them a supported (but arguablely confusing) methodThere's also a substantial differences between what customers are
of customisation. (This is why it was added - end users were asking for
it, and when we didn't have it, they would try to make the
modifications themselves, break it, and report a bug. After telling
8 different people the same thing,
telling
you they want, what they're trying to accomplish, and what they actually
need. If you want to add something simple, setup an attribute tag for
the
section and tell the customers to flag the cases they're most concerned
with. Obviously you expect them to be sitting there with a profiler open
anyways, so this should not be a problem.
This is what people do today (without our proposal). This is what was causing
all the problems. This is what we are trying to avoid.
Also note that we are not required to accept bad code in the kernel
because you happen to have customer requirements for it.
I am not asking you to accept bad code. I'm trying to explain why we came to a
specific design decision, and went down a specific path, after thinking about
and a similar idea as your suggestion.
The point is, doing this as a config option design is just broken. It
doesn't scale for new code, and it doesn't scale as soon as you start
throwing in more blocks of SRAM with varying locality, latencies, etc.
Littering around ifdefs for this stuff all over the code is equally
ugly.
It is equally bad to force end users to maintain a patch set, to change around
our best guess of what their fast paths might be in a .h file.
I'll reiterate my original suggestion, allow the user to cap the amount
of memory that's used by the kernel, then add something akin to initcall
levels that you can use for prioritizing items to be relocated in
on-chip memory, while spilling the rest to RAM. With this sort of scheme
you can also trivially spill in to additional SRAM pools and whatever
else you have handy as you go along.
I don't understand how this is radically different than what we have today.
You are suggesting that we take the knobs that are easy to modify, and hide
them, to create pain for a user who wants to/needs to use them. I don't think
this is good design either.
You can of course argue that as an embedded platform the user must make
all of these decisions on their own, but there can be a compromise if
the kernel's at least half-way intelligent about matters.
I guess it is an disagreement in overall design philosophy - I thought the
kernel was suppost to be flexible enough to handle the corner cases, and
robust enough to ensure proper operation in all cases. What we are attempting
to do (maybe poorly - jury is still out), is continue to add flexibility,
putting the burden on us - the maintainers of all the files where the #ifdefs
exist (all in arch/blackfin or include/asm-blackfin), to make it easier (both
from use and maintenance) on end users.
If your disagreement is - this doesn't make it easier, it makes it harder -
then OK - but I don't think moving the controls somewhere else makes it
easier. Plus I am not sure how I would explain things - "there are n levels
of prioritizing if a function will be in slow or fast SRAM. Setting things to
the highest priority may not mean it will go in the fastest SRAM, and you
won't know until runtime, when your application doesn't run as fast as it
should or by digging through the map file and looking at the address of the
function".
This is a worst case explanation, and I am sure that you could think of
something better. I tend to be negative.
What we have today - is a switch (not a knob). If you select too much, link
fails. There is no "spill over" - by design. It is in on-chip SRAM, or not.
While it may not count for much, until something is done about this and
the other issues that were raised earlier in the thread, I'd have to NAK
this patch. If not wanting to manually select everything that goes in to
L1 memory makes me too unsophisticated by blackfin standards, so be it
;-)
It's not that we don't appreciate the thoughts and time that you have looking
into this - we do and Thanks - I guess I would rather have something that:
- hard failure, rather than soft failure. (it works, or it doesn't boot)
- is maintained in .config file
We are all about worrying about good design, proper code, and future
maintenance/extendability. I think that this specific part of the patch meet
those goals.
As for the other pieces of the patch, that you pointed out - most need to be
fixed per your comments. Thanks again for the time.
-Robin
-
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/
- References:
- [PATCH -mm 1/5] Blackfin: blackfin architecture patch update
- From: Wu, Bryan
- Re: [PATCH -mm 1/5] Blackfin: blackfin architecture patch update
- From: Robin Getz
- Re: [PATCH -mm 1/5] Blackfin: blackfin architecture patch update
- From: Paul Mundt
- [PATCH -mm 1/5] Blackfin: blackfin architecture patch update
- Prev by Date: Re: 2.6.19: ACPI reports AC not present after resume from STD
- Next by Date: Re: [PATCH] pata_jmicron: remove spurious #endif
- Previous by thread: Re: [PATCH -mm 1/5] Blackfin: blackfin architecture patch update
- Next by thread: Re: [PATCH -mm 1/5] Blackfin: blackfin architecture patch update
- Index(es):
Relevant Pages
|
