Message-ID: <alpine.LSU.2.11.1402232344280.1890@eggly.anvils>
Date:	Mon, 24 Feb 2014 00:28:01 -0800 (PST)
From:	Hugh Dickins <hughd@...gle.com>
To:	Weijie Yang <weijieut@...il.com>
cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Mel Gorman <mgorman@...e.de>, Michal Hocko <mhocko@...e.cz>,
	Christian Ehrhardt <ehrhardt@...ux.vnet.ibm.com>,
	linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] mm: swap: Use swapfiles in priority order

On Sun, 16 Feb 2014, Weijie Yang wrote:
>  On Fri, Feb 14, 2014 at 9:10 PM, Christian Ehrhardt
> <ehrhardt@...ux.vnet.ibm.com> wrote:
> > Weijie Yang <weijie.yang.kh <at> gmail.com> writes:
> >
> >>
> >> On Thu, Feb 13, 2014 at 6:42 PM, Mel Gorman <mgorman <at> suse.de> wrote:
> > [...]
> >> > -       for (type = swap_list.next; type >= 0 && wrapped < 2; type = next) {
> >> > +       for (type = swap_list.head; type >= 0 && wrapped < 2; type = next) {
> >>
> > [...]
> >> Does it lead to a "schlemiel the painter's algorithm"?
> >> (please forgive my rude words, but I can't find a precise word to describe it
> >>
> >> How about modify it like this?
> >>
> > [...]
> >> - next = swap_list.head;
> >> + next = type;
> > [...]
> >
> > Hi,
> > unfortunately withou studying the code more thoroughly I'm not even sure if
> > you meant you code to extend or replace Mels patch.
> >
> > To be sure about your intention.  You refered to algorithm scaling because
> > you were afraid the new code would scan the full list all the time right ?
> >
> > But simply letting the machines give a try for both options I can now
> > qualify both.
> >
> > Just your patch creates a behaviour of jumping over priorities (see the
> > following example), so I hope you meant combining both patches.
> > With that in mind the patch I eventually tested the combined patch looking
> > like this:
> 
> Hi Christian,
> 
> My patch is not appropriate, so there is no need to combine it with Mel's patch.
> 
> What I worried about Mel's patch is not only the search efficiency,
> actually it has
> negligible impact on system, but also the following scenario:
> 
> If two swapfiles have the same priority, in ordinary semantic, they
> should be used
> in balance. But with Mel's patch, it will always get the free
> swap_entry from the
> swap_list.head in priority order, I worry it could break the balance.
> 
> I think you can test this scenario if you have available test machines.
> 
> Appreciate for your done.

Weijie, I agree with you on both points: Schlemiel effect of repeatedly
restarting from head (already an unintended defect before Mel's patch),
and more importantly the breakage of swapfiles at the same priority.

Sorry, it has to be a Nak to Mel's patch, which fixes one behavior
at the expense of another.  And if we were to go that way, better
just to rip out all of swap_list.next and highest_priority_index.

I had hoped to respond today with a better patch; but I just haven't
got it right yet either.  I think we don't need to rush to fix it,
but fix it we certainly should.

Christian, congratulations on discovering this wrong behavior: at
first I assumed it came from Shaohua's 3.9 highest_priority_index
changes, but no; then I assumed it came from my 2.6.14 swap_lock
changes; but now I think it goes back even before 2.4.0, probably
ever since there have been swap priorities.

Hugh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists