Re: [PATCH v2] btrfs: try to search for data csums in commit root

[David Sterba <dsterba@suse.cz>]

On Fri, Oct 11, 2024 at 12:48:31PM -0700, Boris Burkov wrote:
> > Can you please find another way how to store this? Maybe the bio flags
> > have some bits free. Otherwise this adds 8 bytes to btrfs_bio, while
> > this structure should be optimized for size.  It's ok to use bool for
> > simplicity in the first versions when you're testing the locking.
> 
> Ooh, good point!
> 
> I started looking into it some more and it's tricky but I have a few
> ideas, curious what you think:
> 
> 1. Export the btrfs_bio_ctrl and optionally wire it through the
>    callstack. For data reads it is still live on the stack, we just can't
>    be sure that containerof will work in general. Or just wire the bool
>    through the calls. This is pretty "trivial" but also ugly.
> 2. We already allocate an io context in multi-device scenarios. we could
>    allocate a smaller one for single. That doesn't seem that different
>    than adding a flags to btrfs_bio.
> 3. Figure out how to stuff it into struct btrfs_bio. For example, I
>    think we aren't using btrfs_bio->private until later, so we could put
>    a to-be-overwritten submit control struct in there.

Maybe this could work but seems fragile as the private pointer is used
for other strucutres and purposes. All in a void pointer. We need to
store only a single bit of information, so this can work with some stub
pointer type that can be at least recognized when mistakenly
dereference.

> 4. Figure out how to stuff it into struct bio. This would be your
>    bi_flags idea. However, I'm confused how to do that safely. Doesn't
>    the block layer own those bits? It seems aggressive for me to try
>    to use them. bio has a bi_private as well, which might be unset in
>    the context we care about.

#define BTRFS_BIO_FLAG_SEARCH_COMMIT_ROOT	(1U << (BIO_FLAG_LAST + 1)

I don't see any code in block/* that would verify the bio bits. Also the
REQ_ bits could be used, we already do that with REQ_BTRFS_CGROUP_PUNT
and at some point the REQ_DRV was used (removed in a39da514eba81e687db).

> I'm leaning towards option 3: taking advantage of the yet unset
> btrfs_bio->private