Re: [PATCH 02/10] mm: system wide ALLOC_NO_WATERMARK

Daniel Phillips
2007-08-10T08:16:16Z

On 8/9/07, Christoph Lameter <clameter@sgi.com> wrote:
> > If you believe that the deadlock problems we address here can be
> > better fixed by making reclaim more intelligent then please post a
> > patch and we will test it. I am highly skeptical, but the proof is in
> > the patch.
>
> Then please test the patch that I posted here earlier to reclaim even if
> PF_MEMALLOC is set. It may require some fixups but it should address your
> issues in most vm load situations.

It is quite clear what is in your patch. Instead of just grabbing a
page off the buddy free lists in a critical allocation situation you
go invoke shrink_caches. Why oh why? All the memory needed to get
through these crunches is already sitting right there on the buddy
free lists, ready to be used, why would you go off scanning instead?
And this does not work in atomic contexts at all, that is a whole
thing you would have to develop, and why? You just offered us
functionality that we already have, except your idea has issues.

You do not do anything to prevent mixing of ordinary slab allocations
of unknown duration with critical allocations of controlled duration.
This is _very important_ for sk_alloc. How are you going to take
care of that?

In short, you provide a piece we don't need because we already have it
in a more efficient form, your approach does not work in atomic
context, and you need to solve the slab object problem. You also need
integration with sk_alloc. That is just what I noticed on a
once-over-lightly. Your patch has a _long_ way to go before it is
ready to try.

We have already presented a patch set that is tested and is known to
solve the deadlocks. This patch set has been more than two years in
development. It covers problems you have not even begun to think
about, which we have been aware of for years. Your idea is not
anywhere close to working. Why don't you just work with us instead?
There are certainly improvements that can be made to the posted patch
set. Running off and learning from scratch how to do this is not
really helpful.

Regards,

Daniel