lkml.org 
[lkml]   [2007]   [Oct]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 09/10] Change table chaining layout
    On Tue, Oct 23 2007, Boaz Harrosh wrote:
    > On Tue, Oct 23 2007 at 11:55 +0200, Jens Axboe <jens.axboe@oracle.com> wrote:
    > > On Tue, Oct 23 2007, Boaz Harrosh wrote:
    > >> On Tue, Oct 23 2007 at 11:41 +0200, Jens Axboe <jens.axboe@oracle.com> wrote:
    > >>> On Tue, Oct 23 2007, Boaz Harrosh wrote:
    > >>>> On Mon, Oct 22 2007 at 23:47 +0200, Linus Torvalds <torvalds@linux-foundation.org> wrote:
    > >>>>> On Mon, 22 Oct 2007, Alan Cox wrote:
    > >>>>>
    > >>>>>> For structures, not array elements or stack objects. Does gcc now get
    > >>>>>> aligned correct as an attribute on a stack object ?
    > >>>>> I think m68k stack layout still guarantees 4-byte-alignment, no?
    > >>>>>
    > >>>>>> Still doesn't answer the rather more important question - why not just
    > >>>>>> stick a NULL on the end instead of all the nutty hacks ?
    > >>>>> You still do need one bit for the discontiguous case, so it's not like you
    > >>>>> can avoid the hacks anyway (unless you just blow up the structure
    > >>>>> entirely) and make it a separate member). So once you have that
    > >>>>> bit+pointer, using a separate NULL entry isn't exactly prettier.
    > >>>>>
    > >>>>> Especially as we actally want to see the difference between
    > >>>>> "end-of-allocation" and "not yet filled in", so you shouldn't use NULL
    > >>>>> anyway, you should probably use something like "all-ones".
    > >>>>>
    > >>>>> Linus
    > >>>>> -
    > >>>> Every one is so hysterical about this sg-chaining problem. And massive
    > >>>> patches produced, that when a simple none intrusive solution is proposed
    > >>>> it is totally ignored because every one thinks, "I can not be that stupid".
    > >>>> Well Einstein said: "Simplicity is the ultimate sophistication". So no one
    > >>>> need to feel bad.
    > >>> It's all about the end goal - having maintainable and resilient code.
    > >>> And I think the sg code will be better once we get past the next day or
    > >>> so, and it'll be more robust. That is what matters to me, not the
    > >>> simplicity of the patch itself.
    > >>>
    > >> But that is exactly what his patch is. Much more robust. Because you do not
    > >> relay on sglist content but on outside information, that you already have.
    > >> Have you had an hard look at his solution? It just simply falls into place.
    > >> Please try it out for yourself. I did, and it works.
    > >
    > > Sure, I looked at it, it's not exactly rocket science, I do understand
    > > what it achieves. I don't think the patch is bad as such, I'm merely
    > > trying to state that I think the end code AND interface will be much
    > > nicer with the current direction that the sg helpers are moving.
    > >
    > > It does rely on outside context, because you need to pass in the sglist
    > > number. In my opinion, this patch would be a bandaid for the original
    > > chain code until we got around to fixing the PAGEALLOC crash. Which we
    > > did, it's now merged. The patch doesn't make the code cleaner, it makes
    > > it uglier. It'll work, but that still doesn't mean I have to agree it's
    > > a nice design.
    > >
    > A nice design is to have an struct like BIO. That holds a pointer to the
    > array of scatterlists, size, ..., and a next and prev pointers to the next
    > chunks. Than have all kernel code that now accepts scatterlist* and size
    > accept a pointer to such structure. And all is clear and defined.
    >
    > But since we do not do that, and every single API in the kernel that
    > receives a scatterlist pointer also receives an sg_count parameter,
    > than I do not see what is so hacky about giving that sg_count parameter
    > to the one that needs it the most. sg_next();

    Not all paths need to know the exact number though, and with the changes
    you could legitimately pass in just the header and iteration would work
    fine.

    > OK I guess this is all a matter of taste so there is no point arguing
    > about it any more. I can see your view, and the work has been done so
    > I guess there is no point going back. If it all works than it's for the
    > best.

    Yes agreed, debating taste is usually not very interesting as we wont
    get far ;-)

    > Thanks Jens for doing all this, The performance gain is substantial
    > and we will all enjoy it.

    My pleasure, I just wish it could have been a little less painful. But
    in a day or two, it should all be behind us and we can move forward with
    making good use of it.

    --
    Jens Axboe

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

    \
     
     \ /
      Last update: 2007-10-23 12:33    [W:0.027 / U:0.756 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site