[lkml]   [2009]   [May]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: New TRIM/UNMAP tree published (2009-05-02)
    James Bottomley wrote:
    > On Sun, 2009-05-03 at 14:40 -0400, Jeff Garzik wrote:
    >> Jeff Garzik wrote:
    >>> (2) determine at init if queue (a) supports explicit DISCARD and/or (b)
    >>> supports DISCARD flag passed with READ or WRITE
    >> As an aside -- does any existing command set support case #b, above?
    > Not to my knowledge.
    > I think discard was modelled on barrier (which can be associated with
    > data or stand on its own).

    Yeah -- I am thinking we can reduce complexity if no real hardware
    supports "associated with data."

    >> AFAICT, ATA, SCSI and NVMHCI all have a single, explicit hardware
    >> command to discard/deallocate unused sectors.
    >> Therefore, creating REQ_TYPE_DISCARD seems to eliminate any need for new
    >> hook ->prepare_discard().
    > Well, yes and no ... in the SCSI implementation of either UNMAP or WRITE
    > SAME, we still need a data buffer to store either the extents or the
    > actual same data.

    Sure, UNMAP's extents would quite naturally travel with the
    REQ_TYPE_DISCARD struct request, and can be set up at struct request
    allocation time. In all of ATA, SCSI and NVMHCI, they appear to want
    the extents.

    Is WRITE SAME associated with this current DISCARD work, or is that just
    a similar example? I'm unfamiliar with its issues...

    >> This provides a 1:1 correspondence between hardware and struct request,
    >> most closely matching the setup of known hardware.
    > Agreed ... but we still have to allocate the adjunct buffer
    > somewhere ....

    Agreed. I merely argue ->prep_rq_fn() would be a better place -- it can
    at least return a useful return value -- than ->prepare_discard(), which
    would be the natural disposition for a REQ_TYPE_DISCARD -- or
    ->request_fn() itself, if a block driver so chooses.

    The extents would be an argument to REQ_TYPE_DISCARD, and should be
    associated with struct request somehow, by struct request's creator.
    The extent info needs to be in some generic form, because all of
    ATA|SCSI|NVMHCI seem to want it.


    Does make you wonder if a ->init_rq_fn() would be helpful, one that
    could perform gfp_t allocations rather than GFP_ATOMIC? The idea being
    to call ->init_rq_fn() almost immediately after creation of struct
    request, by the struct request creator.

    I obviously have not thought in depth about this, but it does seem that
    init_rq_fn(), called earlier in struct request lifetime, could eliminate
    the need for ->prepare_flush, ->prepare_discard, and perhaps could be a
    better place for some of the ->prep_rq_fn logic.

    The creator of struct request generally has more freedom to sleep, and
    it seems logical to give low-level drivers a "fill in LLD-specific info"
    hook BEFORE the request is ever added to a request_queue.

    Who knows...


     \ /
      Last update: 2009-05-03 21:57    [W:2.727 / U:0.832 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site