Subject: Re: [RFC] Interface to hardware-assisted data movers
To: None <thorpej@wasabisystems.com>
From: None <cgd@broadcom.com>
List: tech-kern
Date: 07/16/2002 19:22:53
At Tue, 16 Jul 2002 05:40:44 +0000 (UTC), "Jason R Thorpe" wrote:
> Also, I just nuked the UIO support from the current specification.  None
> of my implementation uses it, even for userland access to the dmovers (the
> userland interfase uses a "request" queue and a "response" queue, which the
> user writes to and reads from, respectively -- messages are passed on these
> queues).

So... I think that might be a mistake.  Specifically, now there's no
way to have a single 'copy' operation gather disjoint chunks into a
contiguous destination buffer, as one might want for, say, coalescing
packets for a network interface w/ "less-than-desirable" alignment
constraints.


Comments on the new document:

* intro: you might want to enunciate the notion of 'zero or more input
  regions', 'one output region', and 'same lengths for each.' here.

* DMOVER_REQ_WAIT: You probably want an additional flag which forces
  the processing code to spin, rather than sleeping.  Horrible, yes,
  but desirable if you're using it in a place where you can't sleep
  but do know that the data mover will be substantially faster or
  provide other benefits as compared to a CPU-based copy.  (an example
  of this might be a pmap zero page routine.)

* so, all these immediate value things...  should probably be a union,
  or even just a single 64-bit type w/ e.g. fill8 specified to use
  only the low byte value.  maybe if you want to do something even
  better (i heard you talking about byte order problems), maybe an
  array of bytes, and specify how they're copied (and that fill8 uses
  the first one, fill16 uses the first 2, etc.)

* There's still some things missing in session method specification
  interface, from where I sit.  e.g., you might want to specify the
  expected likelyhood of (lack of) reuse of the data, either via
  specifying a vague "locality" value (say, 0-3), or by providing
  explicit flags like, "try to avoid putting this data into l1 cache,
  l2 cache, etc."  The latter type of thing may be translated into,
  say, flags on HW requests by a hardware back-end, or specific
  prefetch ops or a specific pattern of accesses by a SW back-end.

  Maybe you could say "copy-<something>", but after you have a flag or
  two that really does get tedious.

* copy should be defined to not operate on overlapping regions.  more
  generally, methods should be "restartable" in case of HW removal and
  the need to re-assign the request to a different back-end.

* can dmover_request_alloc fail?

* Is it the only interface by which one can get request structures, or
  may they be allocated by other means?  if the latter is OK, specify
  that they must be zeroed so that we can add things in the future w/o
  too much pain.  (e.g., priority, but i don't know if that should go
  on session or request.)

* assymetry between dmover_request_alloc's allocation of input buffers
  and dmover_request_free's freeing of them is ... dangerous.  (i.e.,
  to get it to alloc you pass NULL, to get it to free you have to pass
  the allocated ptr!)  Perhaps: pass inbuf to dmover_request_alloc to
  use pre-existing buffer, else it will allocate.  if you provide
  buffer, you must dispose of it yourself.  if it allocates, it sets
  flag in request and deallocates on free.  don't take buffer ptr in
  free.  if free free's something passed in arbitrarily, you'll lose
  due to malloc types, too.

* what happens if you destroy a session while requests are active?
  What happes if you free a request while it is active?

* puzzled by description of use from interrupt context...  you can use
  it, at, say, IPL_BIO and IPL_SOFTNET, but not IPL_NET (between the
  two)?  does this mean from interrupt context caused by interrupts of
  those types, or called under splFOO()?  (IPL == priority level, not
  "context," but you knew that.  8-)

* should probably say that dmover_algdesc, dmover_backend should have
  all unspecified fields zero-initted (initted in data section, or
  bzero'd before filling in the known ones) so that things can be
  added later safely.

* if you do that, not sure what point there is to have the dmb_speed
  there now, since it's not used for much...



re: assigning requests to back-ends: the more i think about it, the
more i like the low-overhead approach of assigning sessions to
back-ends, but then possibly reassigning them when things get
overloaded.  it's a real bummer to consult each backend -- or even
each which can handle a particular method -- for each request.



chris