Re: [PATCH v4 2/3] btrfs: inhibit extent buffer writeback to prevent COW amplification

[David Sterba <dsterba@suse.cz>]

On Thu, Feb 26, 2026 at 01:51:07AM -0800, Leo Martins wrote:
> +/*
> + * btrfs_inhibit_eb_writeback - Inhibit writeback on buffer during transaction.
> + * @trans: transaction handle that will own the inhibitor
> + * @eb: extent buffer to inhibit writeback on
> + *
> + * Attempts to track this extent buffer in the transaction's inhibited set.
> + * If memory allocation fails, the buffer is simply not tracked. It may
> + * be written back and need re-COW, which is the original behavior.
> + * This is acceptable since inhibiting writeback is an optimization.
> + */

Minor thing, please check how functions should be commented, with the
parameter block.

https://btrfs.readthedocs.io/en/latest/dev/Development-notes.html#comments

> +/*
> + * btrfs_uninhibit_all_eb_writeback - Uninhibit writeback on all buffers.
> + * @trans: transaction handle to clean up
> + */

Same

> @@ -102,6 +102,8 @@ struct extent_buffer {
>  	/* >= 0 if eb belongs to a log tree, -1 otherwise */
>  	s8 log_index;
>  	u8 folio_shift;
> +	/* Inhibits WB_SYNC_NONE writeback when > 0. */
> +	atomic_t writeback_inhibitors;

The extent buffer is a sensitive data structure so the layout and size
is closely watched. You increase the size to 248 bytes which is still
under 256 so it fits fine to the slabs. What is not good is the
placement after the s8/u8 members as this leaves 2 byte hole before and
4 byte hole after it

@@ -5666,15 +5669,19 @@ struct extent_buffer {
 
        /* XXX 2 bytes hole, try to pack */
 
-       struct callback_head       callback_head __attribute__((__aligned__(8))); /*    56    16 */
-       /* --- cacheline 1 boundary (64 bytes) was 8 bytes ago --- */
-       struct rw_semaphore        lock __attribute__((__aligned__(8))); /*    72    40 */
-       struct folio *             folios[16];           /*   112   128 */
+       atomic_t                   writeback_inhibitors __attribute__((__aligned__(4))); /*    56     4 */
 
-       /* size: 240, cachelines: 4, members: 14 */
-       /* sum members: 238, holes: 1, sum holes: 2 */
-       /* forced alignments: 4, forced holes: 1, sum forced holes: 2 */
-       /* last cacheline: 48 bytes */
+       /* XXX 4 bytes hole, try to pack */
+
+       /* --- cacheline 1 boundary (64 bytes) --- */
+       struct callback_head       callback_head __attribute__((__aligned__(8))); /*    64    16 */
+       struct rw_semaphore        lock __attribute__((__aligned__(8))); /*    80    40 */
+       struct folio *             folios[16];           /*   120   128 */
+
+       /* size: 248, cachelines: 4, members: 15 */
+       /* sum members: 242, holes: 2, sum holes: 6 */
+       /* forced alignments: 5, forced holes: 2, sum forced holes: 6 */
+       /* last cacheline: 56 bytes */
 } __attribute__((__aligned__(8)));

We can't get rid of the holes unfortunately but at least placing it after
mirror_num the hole becomes contiguous:

@@ -5664,14 +5664,11 @@ struct extent_buffer {
        spinlock_t                 refs_lock __attribute__((__aligned__(4))); /*    40     4 */
        refcount_t                 refs __attribute__((__aligned__(4))); /*    44     4 */
        int                        read_mirror;          /*    48     4 */
-       s8                         log_index;            /*    52     1 */
-       u8                         folio_shift;          /*    53     1 */
+       atomic_t                   writeback_inhibitors __attribute__((__aligned__(4))); /*    52     4 */
+       s8                         log_index;            /*    56     1 */
+       u8                         folio_shift;          /*    57     1 */
 
-       /* XXX 2 bytes hole, try to pack */
-
-       atomic_t                   writeback_inhibitors __attribute__((__aligned__(4))); /*    56     4 */
-
-       /* XXX 4 bytes hole, try to pack */
+       /* XXX 6 bytes hole, try to pack */
 
        /* --- cacheline 1 boundary (64 bytes) --- */
        struct callback_head       callback_head __attribute__((__aligned__(8))); /*    64    16 */
@@ -5679,8 +5676,8 @@ struct extent_buffer {
        struct folio *             folios[16];           /*   120   128 */
 
        /* size: 248, cachelines: 4, members: 15 */
-       /* sum members: 242, holes: 2, sum holes: 6 */
-       /* forced alignments: 5, forced holes: 2, sum forced holes: 6 */
+       /* sum members: 242, holes: 1, sum holes: 6 */
+       /* forced alignments: 5, forced holes: 1, sum forced holes: 6 */
        /* last cacheline: 56 bytes */
 } __attribute__((__aligned__(8)));

The placement to first cacheline cannot be avoided but the access pattern seems
consistent with the other members like refs or lock so this should be OK.