On Mon, Sep 20, 2021 at 02:23:29PM +0200, David Sterba wrote:

On Mon, Sep 20, 2021 at 10:04:10AM +0300, Nikolay Borisov wrote:

quoted

On 15.09.21 г. 10:17, Qu Wenruo wrote:

quoted

Previously we have "struct btrfs_bio", which records IO context for
mirrored IO and RAID56, and "strcut btrfs_io_bio", which records extra
btrfs specific info for logical bytenr bio.

With "strcut btrfs_bio" renamed to "struct btrfs_io_context", we are
safe to rename "strcut btrfs_io_bio" to "strcut btrfs_logical_bio" which
is a more suitable name now.

Although the name, "btrfs_logical_bio", is a little long and name
"btrfs_bio" can be much shorter, "btrfs_bio" conflicts with previous
"btrfs_bio" structure and can cause a lot of problems for backports.

Thus here we choose the name "btrfs_logical_bio", which also emphasis
those bios all work at logical bytenr.

Signed-off-by: Qu Wenruo <redacted>

So thinking a bit more about the renaming we are trading "awkwardness"
for future generations so that we make backporting easier or rather more
fool proof.

What if we backport a patch that does BUILD_BUG_ON predicated on the
size of the btrfs_io_bio. That way if a patch backports cleanly and
automatically but in fact git got confused by btrfs_bio vs btrfs_io_bio
then a build failure would ensue due to mismatched sizes and that would
be a clear indication something has gone wrong so whoever is doing the
backport can go and correct the backport? David what do you think about
this?

So you want to call the structure btrfs_bio and add build protections?  I'm not
sure how exactly you want to do the sizeof check, one way would be to add a
stub structure and compare sizeof against that, because a hardcoded value won't
work due to padding, or we'd have to have a 32bit assertion version.

I'd like to see the code, but otherwise I think it's reasonable, the shorter
name would be better. I don't expect many backports regarding the bio
related code, it could be referenced in the diff context but that we can
handle fine. I'm a bit cautious because I've seen patches to other
subystems that did changes like swapping parameters or repurposing
structures like here we do and Linus did not like that at all. It's
trade off if we'll suffer a naming we don't like or would cause a bug
because we'd forget about the change.

For the record, we had a chat about that and found that explicit build
checks won't be necessary as the old/new structure has no overlap of the
members so the build would fail anyway.

I did the rename from btrfs_logical_bio* to btrfs_bio again in
misc-next, please have a look, it's basically what Qu sent as v2.