Re: [patch 1/2] ufd v1 - unsequential O(1) fdmap core

From: Davide Libenzi
Date: Mon Jun 04 2007 - 10:54:16 EST


On Mon, 4 Jun 2007, Eric Dumazet wrote:

> On Mon, 4 Jun 2007 06:35:16 -0700 (PDT)
> Davide Libenzi <davidel@xxxxxxxxxxxxxxx> wrote:
>
> > On Mon, 4 Jun 2007, Eric Dumazet wrote:
> >
> > > You add conditional branches on very hot spots.
> >
> > Keep BS for the ones you argue usually, and that are not able to reply.
> > You *still* two bitmaps, because allocation spaces are far apart. So the
> > "if" will still be there.
>
> I actually read your patches and spent time to see the pros and cons.
>
> If you dont need reviewers, please dont post your patches on lkml.

I *do like* reviews, and if you go back in all the reviews my code had
during the last few months for example, you'll notice that I *almost always*
ACK the reviewers suggestions. I'm a believer that in computer science,
there are many ways to do the same thing, and many of those are very close
as far as pros&cons. So, if some reviewer suggests me something, I'm
inclined to ACK it, even if the solution I wrote (or that I had in mind)
was different.
In this case though, your solution and mine are *really* different. You
are trying to fit a structure to a problem that is not meant to. And it is
so clearly evident, that it blows my mind how this could even be argued.
As I said to Andrew, we can use fdmap for both allocations (fdmap already
supports all the allocation modes the current fdtable does - modulo adding
a bitmap), and cut a lot of code out.
This is the way I'm currently heading, so all the code from fs/file.c (256
lines) can go. locate_fd will go. set_close_on_exec and get_close_on_exec
will use the existing function in fdmap.c, etc...



> If I am not mistaken, you added a test in fget()/fget_light(), which is
> a known hot point for said huge processes.

There is one "if", that you cannot avoid, since allocation areas are far
apart (and one of them is even random). Unless you plan to have a single
contiguous bitmap with *a lot* of wasted space in the middle.



- Davide


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