Re: [RFC 0/3] Add support of iopoll for dm device

[Mike Snitzer <snitzer@redhat.com>]

On Sat, Nov 07 2020 at  8:09pm -0500,
JeffleXu <jefflexu@linux.alibaba.com> wrote:

> 
> On 11/7/20 1:45 AM, Mike Snitzer wrote:
> >On Thu, Nov 05 2020 at  9:51pm -0500,
> >JeffleXu <jefflexu@linux.alibaba.com> wrote:
> >
> >>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.
> 
> Make sense.
> 
> >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?
> 
> Actually I think it's a common issue to design the cookie returned
> by submit_bio() whenever it's implemented in a recursive or
> non-recursive way. After all you need to translate the returned cookie
> to the original bio even if it's implemented in a recursive way as you
> described.

Yes.

> Or how could you walk through the bio graph when the returned cookie
> is only 'unsigned int' type?

You could store a pointer (to orig bio, or per-bio-data stored in split
clone, or whatever makes sense for how you're associating poll data with
split bios) in 'unsigned int' type but that's very clumsy -- as I
discovered when trying to do that ;)

> How about this:
> 
> 
> ```
> 
> typedef uintptr_t blk_qc_t;
> 
> ```
> 
> 
> or something like union
> 
> ```
> 
> typedef union {
> 
>     unsigned int cookie;
> 
>     struct bio *orig; // the original bio of submit_bio()
> 
> } blk_qc_t;
> 
> ```
> When serving for blk-mq, the integer part of blk_qc_t is used and it
> stores the valid cookie, while it stores a pointer to the original bio
> when serving bio-based device.

Union looks ideal, but maybe make it a void *?  Initial implementation
may store bio pointer but it _could_ evolve to be 'struct blk_poll_data
*' or whatever.  But not a big deal either way.

> By the way, would you mind sharing your plan and progress on this
> work, I mean, supporting iopoll for dm device. To be honest, I don't
> want to re-invent the wheel as you have started on this work, but I do
> want to participate in somehow. Please let me know if there's something
> I could do here.

I thought I said as much before but: I really don't have anything
meaningful to share.  My early exploration was more rough pseudo code
that served to try to get my arms around the scope of the design
problem.

Please feel free to own all aspects of this work.

I will gladly help analyze/test/refine your approach once you reach the
point of sharing RFC patches.

Thanks,
Mike