On Thu, Nov 05 2020 at 9:51pm -0500,
JeffleXu <jefflexu@xxxxxxxxxxxxxxxxx> wrote:
>
> On 11/4/20 11:08 PM, Mike Snitzer wrote:
> >>I'm doubted if this should be implemented in block layer like:
> >>
> >>```
> >>
> >>struct bio {
> >>
> >> ...
> >>
> >> struct list_head cookies;
> >>
> >>};
> >>
> >>```
> >>
> >>After all it's only used by bio-based queue, or more specifically
> >>only dm device currently.
> >I do think this line of work really should be handled in block core
> >because I cannot see any reason why MD or bcache or whatever bio-based
> >device wouldn't want the ability to better support io_uring (with IO
> >poll).
> >
> >>Another design I can come up with is to maintain a global data
> >>structure for the very beginning
> >>original bio. Currently the blocking point is that now one original
> >>bio to the dm device (@bio of dm_submit()) can correspond to multiple
> >>dm_io and thus we have nowhere to place the @cookies list.
> >Yes, and that will always be the case. We need the design to handle an
> >arbitrary sprawl of splitting from a given bio. The graph of bios
> >resulting from that fan-out needs to be walked at various levels -- be
> >it the top-level original bio's submit_bio() returned cookie or some
> >intermediate point in the chain of bios.
> >
> >The problem is the lifetime of the data structure created for a given
> >split bio versus layering boundaries (that come from block core's
> >simplistic recursion via bio using submit_bio).
> >
> >>Now we have to maintain one data structure for every original bio,
> >>something like
> >>
> >>```
> >>
> >>struct dm_poll_instance {
> >>
> >> ...
> >>
> >> struct list_head cookies;
> >>
> >>};
> >>
> >>```
> >I do think we need a hybrid where at the point of recursion we're able
> >to make the associated data structure available across the recursion
> >boundary so that modeling the association in a chain of split bios is
> >possible. (e.g. struct dm_poll_data or dm_poll_instance as you named it,
> >_but_ that struct definition would live in block core, but would be part
> >of per-bio-data; so 'struct blk_poll_data' is more logical name when
> >elevated to block core).
> >
> >It _might_ be worthwhile to see if a new BIO_ flag could be added to
> >allow augmenting the bio_split + bio_chain pattern to also track this
> >additional case of carrying additional data per-bio while creating
> >bio-chains. I may not be clear yet, said differently: augmenting
> >bio_chain to not only chain bios, but to _also_ thread/chain together
> >per-bio-data that lives within those chained bios. SO you have the
> >chain of bios _and_ the chain of potentially opaque void * that happens
> >to point to a list head for a list of 'struct blk_poll_data'.
> >
> >Does that make sense?
>
>
> I'm doubted if it really makes sense to maintain a *complete* bio
> chain across the recursive
>
> call boundary.
>
>
> Considering the following device stack:
>
> ```
>
> dm0
>
> dm1 dm2 dm3
>
> nvme0 nvme1 .... ....
>
> ```
>
>
> We have the following bio graph (please let me know if it's wrong or
> the image can't display)
>
>
> For example, for dm1 there are three bios containing valid cookie in
> the end, that is
>
> bio 9/10/11. We only need to insert the corresponding blk_poll_data
> (containing
>
> request_queue, cookie, etc) of these three bios into the very
> beginning original
>
> bio (that is bio0). Of course we can track all the sub-bios down
> through the device
>
> stack, layer by layer, e.g.,
>
> - get bio 1/2/3 from bio 0
>
> - get bio 4 from bio 3
>
> - finally get bio 9 from bio 4
>
> But I'm doubted if it's overkill to just implement the iopoll.
>
>
> Another issue still unclear is that, if we should implement the
> iopoll in a recursive way.
>
> In a recursive way, to poll dm 0, we should poll all its
> sub-devices, that is, bio 4/5/7/8.
>
> Oppositely we can insert only the bottom bio (bio 9/10/11 which have
> valid cookie) at
>
> the very beginning (at submit_bio() phase), and to poll dm 0, we
> only need to poll bio 9/10/11.
I feel we need the ability to walk the entire graph and call down to
next level. The lowest level would be what you call a "valid cookie"
that blk-mq returned. But the preceding cookies need to be made valid
and used when walking the graph (from highest to lowest) and calling
down to the underlying layers.
>
>
> To implement this non-recursive design, we can add a field in struct bio
>
> ```
>
> struct bio {
>
> ...
>
> struct bio *orig;
>
> }
>
> ```
>
> @orig points to the original bio inputted into submit_bio(), that is, bio 0.
>
>
> @orig field is transmitted through bio splitting.
>
> ```
>
> bio_split()
>
> split->orig = bio->orig ? : bio
>
>
> dm.c: __clone_and_map_data_bio
>
> clone->orig = bio->orig ? : bio
>
> ```
>
>
> Finally bio 9/10/11 can be inserted into bio 0.
>
> ```
>
> blk-mq.c: blk_mq_submit_bio
>
> if (bio->orig)
>
> init blk_poll_data and insert it into bio->orig's @cookies list
>
> ```
If you feel that is doable: certainly give it a shot.
But it is not clear to me how you intend to translate from cookie passed
in to ->blk_poll hook (returned from submit_bio) to the saved off
bio->orig.
What is your cookie strategy in this non-recursive implementation? What
will you be returning? Where will you be storing it?
Mike
