lkml.org 
[lkml]   [2009]   [Jun]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [dm-devel] REQUEST for new 'topology' metrics to be moved out of the 'queue' sysfs directory.
    On Fri, Jun 26 2009, Neil Brown wrote:
    > On Thursday June 25, jens.axboe@oracle.com wrote:
    > > On Thu, Jun 25 2009, NeilBrown wrote:
    > > > > You seem to be hung up on the fact that you don't queue things. I think
    > > > > that's beside the point. You *do* have a request_queue thanks to
    > > > > calling blk_queue_make_request() in md.c. And there is more to
    > > > > request_queue than the values you brought up. Like the callback
    > > > > functions. I'm not saying that all the values in request_queue apply to
    > > > > MD, but I really don't understand what all the fuss is about. Other
    > > > > than the presence of the string "queue" in the choice of naming.
    > > > >
    > > >
    > > > Well names are very important. And as I said later we could possibly keep
    > > > them in 'queue' and make 'queue' a more generic directory. I don't like
    > > > that but it is probably better than the current situation.
    > >
    > > Sorry to ask the obvious question, but what would the point of all this
    > > pain be? The existing values can't go anywhere else, so you'd have to
    > > add symlinks back into queue/ anyway.
    >
    > Why cannot the existing values go any where else? I don't understand
    > that assertion at all.

    Because it's an exported interface, we can't just move things around at
    will.

    > > > As you say, I do currently have a request_queue, but that is an internal
    > > > detail, not part of the externally visible interface, and it is something
    > > > that is very much in my sights as something I want to change. I'm
    > > > still working out the details so I'm a fair way from a concrete proposal
    > > > and a long way from some code. That change certainly doesn't have
    > > > to happen in any rush. But we should get the externally visible
    > > > names "right" if we can.
    > >
    > > What crack are you smoking? :-)
    >
    > I have a special mix of crack that helps me see Patterns everywhere,
    > even in C code. Some patterns are bright, shiny, and elegant. Others
    > are muddy and confused. struct request_queue has a distinct shadow
    > over it just now.
    >
    > >
    > > A block device must have a request_queue, that's pretty much spread
    > > throughout the kernel. The fact that md/dm is only using a subset of the
    > > functionality is not a very good reason for re-writing large parts of
    > > that design. We could save some space, but whether the queue is 200
    > > bytes or 1400 bytes doesn't really make a whole lot of real-world
    > > difference. It's not like we allocate/deallocate these all the time,
    > > they are mostly static structures.
    >
    > It isn't about saving space. It is about maintainability. To be
    > maintainable, the code must be easy to understand. For that, it must
    > be well structured.
    >
    > Every block device has a 'gendisk' (aka generic disk).
    > Every block device also (currently) has a request_queue.

    I don't know why you keep saying currently. It has always had a queue,
    and I don't see a good reason why that should change for "special" block
    devices like md/dm/loop/whatnot.

    > If I have generic data that is applicable to all disks, where should I
    > put it? One would think "gendisk".
    > If I have data that is related to request handling (as in uses of
    > 'struct request'), where should that go? in request_queue I suspect.
    > But there are several fields in request_queue that are not related to
    > requests, and are generic to all disks.

    It is indeed a bit of a toss-up there, since we do a queue associated
    with each gendisk.

    > I think "generic data goes in gendisk" is something that it would be
    > easy for people to understand.
    > The current 'topology' values are intended to be generic to all disks
    > so they should ideally go in gendisk. I'm not pushing for that now.
    > Longer term, that would be my aim, but for now I'm just focussing on
    > restoring the 'queue' subdirectory to it's previous non-generic state.
    >
    > i.e. revert the change that made 'queue' appear for md and dm and loop
    > and nbd and .... which have all never needed it before.
    > And find somewhere else to put the topology data - probably just the
    > top level.
    > i.e. add the names as DEVICE_ATTRs in genhd.c, and write the 'show'
    > routine to dereference ->queue carefully, just like
    > disk_alignment_offset_show.
    > (oooo... just noticed that the 'alignment_offset' attribute can
    > contain the string '-1'.... while I have been guilty of that sort of
    > thing myself, I would much rather it said "misaligned".)
    >
    >
    > As for how intrusive vs beneficial it would be to move all the generic
    > fields out of request_queue and allow md to not have a request queue,
    > that will have to be a discussion for another day. I do hope to
    > eventually present you with a series of patches which does just that.
    > My aim would be to make sure each one was clearly beneficial. And I
    > do have a grand vision involving this which is more than just tidying
    > up some small in-elegances. Only time will tell how it eventuates.
    >
    >
    > But for now, please please please can we revert the change which made
    > 'queue' appear in md and dm devices, (and loop and ...) and put these
    > generic values somewhere ... generic?

    No we cannot, not without a time machine. 2.6.30 is released, so it's
    too late to revert things like that, even if we wanted.

    --
    Jens Axboe



    \
     
     \ /
      Last update: 2009-06-26 14:55    [W:4.953 / U:0.500 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site