RE: [GIT PULL] mm: frontswap (for 3.2 window)

[Dan Magenheimer]

> From: Pekka Enberg [mailto:penberg@xxxxxxxxxx]
> Subject: Re: [GIT PULL] mm: frontswap (for 3.2 window)
> 
> On Fri, Oct 28, 2011 at 7:37 PM, Dan Magenheimer
> <dan.magenheimer@xxxxxxxxxx> wrote:
> >> Why do you feel that it's OK to ask Linus to pull them?
> >
> > Frontswap is essentially the second half of the cleancache
> > patchset (or, more accurately, both are halves of the
> > transcendent memory patchset).  They are similar in that
> > the hooks in core MM code are fairly trivial and the
> > real value/functionality lies outside of the core kernel;
> > as a result core MM maintainers don't have much interest
> > I guess.
> 
> I would not call this commit trivial:
> 
> http://oss.oracle.com/git/djm/tmem.git/?p=djm/tmem.git;a=commitdiff;h=6ce5607c1edf80f168d1e1f22dc7a852
> 90cf094a
> 
> You are exporting bunch of mm/swapfile.c variables (including locks)
> and adding hooks to mm/page_io.c and mm/swapfile.c.

Oh, good, some real patch discussion! :-)

You'll note that those exports previously were global and
were made static in the recent past.  The rationale for
this is discussed in the FAQ in frontswap.txt which is
part of the patchset.

The swapfile.c changes are really the meat of the patch.
The page_io.c hooks ARE trivial, don't you think?

> Furthermore, code
> like this:
> 
> > +               if (frontswap) {
> > +                       if (frontswap_test(si, i))
> > +                               break;
> > +                       else
> > +                               continue;
> > +               }
> 
> does not really help your case.

I don't like that much either, but I didn't see a better way
to write it without duplicating a bunch of rather obtuse
code.  Suggestions welcome.

Thanks,
Dan
--
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/