Re: [PATCH] btrfs: tag all metadata I/O with REQ_META

[Mark Korenberg]

On 2026/11/5 19:37, Qu Wenruo wrote:
> > - RAID5/6 rbios build new per-device bios from a bare op, so all tree
> >    block writes on RAID5/6, and recovery reads, lose the flag;
>
> I think this can lead to very cursed combinations.
>
> A RAID56 mixed block groups. In that case, how do you handle recovery?
>
> A vertical stripe can contain both data and metadata, should the
> recovery be counted as data or meta?

In v1 it is counted as data: btrfs_chunk_req_meta() returns 0 for any
chunk with the DATA bit, and a mixed chunk has it.

I don't think that is right, though.  My view is that everything that
is not pure data is metadata: if an I/O may carry tree blocks, it is a
metadata I/O.  A device that caches or places metadata differently is
better off seeing a few data blocks tagged as metadata than missing
tree blocks.  So in v2 mixed chunks get REQ_META too, including RAID56
recovery.

On a mixed RAID56 chunk this tags file data writes as well: a
per-device bio of an rbio can cover sectors of several original bios
and the parity, so only the chunk type tells it.  I find that
acceptable for such a rare setup, and v2 says so in a comment.

> > - scrub and dev-replace read, repair and copy metadata block groups
> >    without it;
>
> Again, what if a block group is mixed?

Same as above: tagged in v2.

> > Tag these by the chunk type: metadata and system chunks get REQ_META,
> > mixed chunks do not,
>
> That almost kill the meaning of re-introduce this flag.

Agreed, see above.

> I'm more interested in how other dm devices, like LVM/dmraid handle
> those REQ_META flags.
>
> E.g. a btrfs over a LVM/mdraid, and btrfs passes REQ_META for a tree
> block read, then should the LVM/mdraid pass REQ_META to the underlying
> devices?

dm clones keep bi_opf, so LVM and dm-crypt pass REQ_META down.  md
raid1/raid10 pass it too since e879a0d9cb08 ("md/raid1,raid10: don't
ignore IO flags"), which names the priority inversion when REQ_META is
cleared.  md raid5 drops it: ops_run_io() builds the per-device bios
with FUA and SYNC only.  I see that as a gap in raid5 that the
raid1/raid10 fix did not cover, not as something to follow.

> If not, I see no particular reason to strictly follow that for
> scrub/dev-replace/raid56 recovery.

md resync cannot know what it copies, btrfs scrub and dev-replace do.

> I'm more interested in what you really want to archieve?
>
> Just to make the REQ_META more consistent?

Not only.  The ublk zoned device I use sees REQ_META as
UBLK_IO_F_META, and its server caches and places metadata differently
from data.  A tree block read issued by scrub or by RAID56 recovery is
still a metadata read for it.  The same holds for the in-kernel users:
BFQ prefers REQ_META requests, blk-cgroup issues them as root to avoid
priority inversion, and wbt does not throttle them.

> At least I do not think it's a good idea to touch RAID56, unless you can
> prove that mdraid is also passing those flags.
>
> I want to align btrfs to existing RAID56 implementation, not the opposite.

I'll move the RAID56 part to a separate patch in v2, so the rest does
not wait on it.

Still, I don't see what it can break: it changes no algorithm, only
the hint.  And within btrfs, metadata writes on DUP, RAID1, RAID1C3/4
and RAID10 already carry REQ_META, because the per-mirror bios are
clones of the original.  RAID56 is the only profile that builds new
per-device bios and loses it.  md raid1/raid10 pass it as well, see
above.

> > +static inline blk_opf_t btrfs_chunk_req_meta(u64 type)
> > +{
> > +	return (type & BTRFS_BLOCK_GROUP_DATA) ? 0 : REQ_META;
>
> Missing BLOCK_GROUP_METADATA_REMAP and also handling BLOCK_GROUP_SYSTEM.

Both get REQ_META: the check is for the DATA bit, and tree-checker
allows only five chunk types (DATA, METADATA, SYSTEM, METADATA_REMAP
and DATA|METADATA), so METADATA, SYSTEM and METADATA_REMAP are all
tagged.  The SYSTEM rows in my test show it, METADATA_REMAP I did not
test on its own.

That is clearly too implicit, and it leaves mixed chunks out.  In v2
the helper returns 0 only for a pure data chunk:

	return (type & BTRFS_BLOCK_GROUP_TYPE_MASK) == BTRFS_BLOCK_GROUP_DATA ?
	       0 : REQ_META;

and the comment lists what gets tagged: METADATA, SYSTEM,
METADATA_REMAP and mixed chunks.