On Thu, Dec 10 2020 at 11:32am -0500,
Mike Snitzer [off-list ref] wrote:

On Thu, Dec 10 2020 at  9:58am -0500,
Sergei Shtepa [off-list ref] wrote:

quoted

The 12/09/2020 16:51, Mike Snitzer wrote:

quoted

On Wed, Dec 09 2020 at  8:01am -0500,
Sergei Shtepa [off-list ref] wrote:

quoted

Hi all.

I try to suggest the Block Layer Interposer (blk_interposer) again.
It`s allows to intercept bio requests, remap bio to another devices
or add new bios.

Initially, blk_interposer was designed to be compatible with
device mapper. Our (my and Hannes) previous attempt to offer
blk_interposer integrated with device mapper did not receive
any comments from the dm-devel team, and without their help
I will not be able to make a full implementation. I hope later
they will have time to support blk_interposer in device mapper.

Excuse me?  I gave you quite a bit of early feedback!  I then went on
PTO for ~10 days, when I returned last week I had to deal with fixing
some 5.10 dm/block bio splitting regressions that only got resolved this
past Saturday in time for 5.10-rc7.

Mike,

I would like to clarify some points that I've made, and also try 
to repair the damage from the misunderstandings that I think have occured.

First of all, I actually meant the feedback on Hannes's patch which was
sent on the 19th of November:
https://lore.kernel.org/linux-block/20201119164924.74401-1-hare@suse.de/ (local)

Your feedback from the 18th of November ("[PATCH 4/4] dm_interposer - 
Try to use blk_interpose in dm") is very valuable, but I am not sure that
I am currently capable of implementing the proposed DM changes.
The overall architecture of DM is still not clear to me, and I am studying
it right now.

This new patch (the one that Hannes sent on the 19th of November) is also
compatibile with DM and should not pose any problems - the architecture is
the same. There are some changes that make blk_interposer simpler and better,
plus the sample is added.

quoted

blk_interposer was/is on my short list to review closer (Hannes' version
that refined yours a bit more).. primarily to see if I could avoid the
extra clone and endio hooking.

Glad to hear that! In order to avoid the additional copying one can only
change an intercepted bio, which might be dangerous.

quoted

The development window for 5.11 is past, so you pushing this without
using the approach discussed (in terms of DM) doesn't help your cause.

quoted

And my blk-snap module requires an architecture change to
support blk_interposer.

This time I offer it along with a sample.
Despite the fact that blk_interposer is quite simple,
there are a few non-obvious points that I would like to clarify.

However, I suggest the blk_interposer now so that people
could discuss it and use it in their projects as soon as possible.

So you weren't willing to put the work in on something DM oriented
because you expected me to do the work for you?  And now you're looking
to side-step the consensus that was built because I didn't contribute
quick enough?  That's pretty messed up.

I just think that no one can implement integration of DM with
blk_interposer better than dm-devel team. I will certainly try my best,
but I am afraid that such efforts will only produce incongruous
DM patches that will be a waste of time (yours and mine).

quoted

In the time-scale that is Linux kernel development.. you've really
jumped the gun and undercut my enthusiasm to work through the details.
I'd rather not read into your actions more than I already have here, but
I'm not liking what you've done here.  Kind of left in dismay with how
you decided to go down this path without a followup note to me or others
(that I'm aware of).

I am very sorry that I undercut your enthusiasm, but, as you rightly
pointed out, another development windows is closing, and my product
is still not able to work on newer Linux versions starting from 5.8.
That fact makes me particularly sad and forces me to search for different
means to draw some attention to blk_interposer. I've seen an RHEL 8.4
alpha recently, all looks very cool there but made me even more sad ...

Made you more sad because you don't have a working solution for upstream
or RHEL 8.4?

I may have missed it in your past emails but how were you able to
provide blk-snap support for kernels prior to 5.8?

I now clearly understand that the 5.8 block changes to do away with
->make_request_fn in favor of a more optimized ->submit_bio (that isn't
applicable for all devices) is why you're pursuing a "fix" so urgently.

quoted

quoted

But I'm still willing to make blk_interposer work as we all discussed
(in terms of DM).

I want it too. However, there is a certain difficulty with usage of DM
for backup copying. For instance, there is no changed block tracking (CBT)
and right now I don't have any clue how it could be implemented
considering the existing architecture. I still hope that sometime
in future I could be offer my blk-snap module which was specifically
created for backup copying purposes.

I apologize for causing all that confusion and mess and making you upset.
I hope that all of the above makes sense to you and you will not think
that I was slacking and tried to offload all the work to your team.

My primary concern is that blk_interposer be correct from the start.  To
validate its correctness it needs to be fully implemented and vetted in
terms on upstream Linux kernel code.  DM can easily serve as the primary
upstream consumer until if/when your blk-snap module is proposed for
upstream inclusion.

But short of having an actual upstream consumer of blk_interposer (not
just samples/ code) it cannot go upstream.  Otherwise there are too many
risks of misuse and problems in the longrun.  That and it'd be baggage
block core would need to carry for no upstream Linux benefit.

As I shared in private: I have some urgent Red Hat business critical
work I need to do and unfortunately cannot put my near-term focus to
implementing a fully baked blk_interposer for DM.  But I can come back
to it (sadly unlikely to do so until the new year though).

While I still think there needs to be a proper _upstream_ consumer of
blk_interposer as a condition of it going in.. I'll let others make the
call.

As such, I'll defer to Jens, Christoph and others on whether your
minimalist blk_interposer hook is acceptable in the near-term.

Mike