Re: [PATCH v3 3/3] btrfs: rename struct btrfs_io_bio to btrfs_logical_bio

[David Sterba <dsterba@suse.cz>]

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:
> > On 15.09.21 г. 10:17, Qu Wenruo wrote:
> > > 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 <wqu@suse.com>
> > 
> > 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.