Re: [PATCH RFC] Btrfs: only subtract from len_to_oe_boundary when it is tracking an extent

[Chris Mason <clm@meta.com>]

On 7/31/23 3:02 AM, Christoph Hellwig wrote:
> On Mon, Jul 31, 2023 at 10:27:02AM +0800, Qu Wenruo wrote:
>> Personally speaking, I think we'd better moving the ordered extent based
>> split (only for zoned devices) to btrfs bio layer.
> 
> That goes completely counter the direction I've been working to.  The
> ordered extent is the "container" for writeback, so having a bio
> that spawns them creates all kinds of problems.  Thats's the reason
> why we now have a bbio->ordered pointer now.
> 
>> Another concern is, how we could hit a bio which has a size larger than
>> U32_MAX?
>>
>> The bio->bi_iter.size is only unsigned int, it should never exceed U32_MAX.
>>
>> It would help a lot if you can provide a backtrace of such unaligned bio.
> 
> That's indeed a bit weird.
> 

bio_full() is using a slightly different test:

        if (bio->bi_iter.bi_size > UINT_MAX - len)
                return true;

We're doing:

                /* Cap to the current ordered extent boundary if there is one. */
                if (len > bio_ctrl->len_to_oe_boundary) {
                        ASSERT(bio_ctrl->compress_type == BTRFS_COMPRESS_NONE);
                        ASSERT(is_data_inode(&inode->vfs_inode));
                        len = bio_ctrl->len_to_oe_boundary;
                }

The end result of this is that when we get to a 
bio sized U32_MAX - PAGE_SIZE, both our len_to_oe_boundary and the
bio_add_page() check will correctly decide we can only fit 4095 bytes.

The difference is bio_add_page() would just say no triggering bio
submission.  But submit_extent_page()'s check is first, cutting the
IO down to 4095 bytes instead.

(I do have the stack trace, it's just a boring filemap_fdata_write_and_wait() path off of file release)

-chris