Re: [PATCH 2/2] page table iterators

[Nick Piggin]

Hugh Dickins wrote:
> On Sun, 20 Feb 2005, Nick Piggin wrote:

> > > Open coding is probably the smaller evil.
> > > And they're really not changed that often.
>
>
> My opinion FWIW: I'm all for regularizing the pagetable loops to
> work the same way, changing their variables to use the same names,
> improving their efficiency; but I do like to see what a loop is up to.
>
> list_for_each and friends are very widely used, they're fine, and I'm
> quite glad to have their prefetching hidden away from me; but usually
> I groan, grin and bear it, each time someone devises a clever new
> for_each macro concealing half the details of some specialist loop.
>
> In a minority?

OK, I think Andrew is now sitting on the fence after seeing the
code. So you (and Andi?) are the ones with remaining reservations
about this.

I don't disagree with your stance entirely, Hugh. I think these
macros are close to being too complicated... But I don't think
they is hiding too much detail: we all know that conceptually,
walking a page table page is reasonably simple. There are just a
few tricky bits like wrapping and termination that caused such
a divergent range of implementations - I would argue that hiding
these details is OK, because they are basically inconsequencial
to the job at hand. I think that actually makes the high level
intention of the code clearer, if anything.

If you are reading just the patch, that doesn't quite do it
justice IMO - in that case, have a look at the code after the
patch is applied (I can send you one which applies to current
kernels if you'd like).

Also, the implementation of the macros is not insanely difficult
to understand, so the details are still accessible.

Lastly, they fold to 2 and 3 levels easily, which is something
that couldn't sanely be done with the open-coded implementation.
I think with an infinitely smart compiler, there shouldn't need
to be any folding here. But in practice I see quite a large
speedup, which is something we shouldn't ignore.

I do think that they are probably not ideal candidates for a
more general abstraction that would allow for example the
transparent drop in of Dave's bitmap walking functions (it
would be possible, but would not be pretty AFAIKS). I have some
other ideas to work towards those goals, but before that I
think these macros do help with the deficiencies of the current
situation.

Nick

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