What is the point of the wbc->for_kupdate check in btree_writepages

What is the point of the wbc->for_kupdate check in btree_writepages

[Christoph Hellwig]

Hi Chris,

you added a wbc->for_kupdate check to btree_writepages that skips the
write in this case in commit 448d640b668d ("Btrfs: Fine tune the btree
writeback exclusion some more") which is not associated without any
information but the subject line.

Do you by any chance remember why it was doing that just for kupdate
and not any other background writeback?  I'm asking because it is one
of only two checks for this in file system code, and the other one
in nfs at least checks for all background writeback and thus makes
some sense to me.

Re: What is the point of the wbc->for_kupdate check in btree_writepages

[Chris Mason]

On 5/8/25 2:06 AM, Christoph Hellwig wrote:
> Hi Chris,
> 
> you added a wbc->for_kupdate check to btree_writepages that skips the
> write in this case in commit 448d640b668d ("Btrfs: Fine tune the btree
> writeback exclusion some more") which is not associated without any
> information but the subject line.
> 
> Do you by any chance remember why it was doing that just for kupdate
> and not any other background writeback?  I'm asking because it is one
> of only two checks for this in file system code, and the other one
> in nfs at least checks for all background writeback and thus makes
> some sense to me.

Hi Christoph,

btrfs can keep changing dirty tree blocks in memory, but once we write
them, we have to recow.  Between transaction writeback kicking in every
30 seconds and us calling balance_dirty_pages on the btree inode,
kupdate was doing more harm than good (back in 2007).

Is the goal to get rid of for_kupdate?  I wonder if we can just flag the
btree inode to exclude from kupdate, or keep it off whatever list
kupdate cares about etc.

-chris

Re: What is the point of the wbc->for_kupdate check in btree_writepages

[Christoph Hellwig]

On Thu, May 08, 2025 at 10:33:24AM -0400, Chris Mason wrote:
> btrfs can keep changing dirty tree blocks in memory, but once we write
> them, we have to recow.  Between transaction writeback kicking in every
> 30 seconds and us calling balance_dirty_pages on the btree inode,
> kupdate was doing more harm than good (back in 2007).

I totally understand why you'd want to avoid background writeback.  I
just don't understand why it singles out for_kupdate.

> Is the goal to get rid of for_kupdate?

At least getting rid of exposing it to file systems, yes.

> I wonder if we can just flag the
> btree inode to exclude from kupdate, or keep it off whatever list
> kupdate cares about etc.

Not having the VM do writeback on metadata but running it from a fs
LRU was a huge win in XFS.  I'm not sure we have interfaces that keep
data in the pagecache but never do any background writeback.  But
if you are fine with treating all background writeback equal that
would be exactly where I'd like to go to.

Re: What is the point of the wbc->for_kupdate check in btree_writepages

[Chris Mason]

On 5/8/25 10:41 AM, Christoph Hellwig wrote:
> On Thu, May 08, 2025 at 10:33:24AM -0400, Chris Mason wrote:
>> btrfs can keep changing dirty tree blocks in memory, but once we write
>> them, we have to recow.  Between transaction writeback kicking in every
>> 30 seconds and us calling balance_dirty_pages on the btree inode,
>> kupdate was doing more harm than good (back in 2007).
> 
> I totally understand why you'd want to avoid background writeback.  I
> just don't understand why it singles out for_kupdate.
> 
>> Is the goal to get rid of for_kupdate?
> 
> At least getting rid of exposing it to file systems, yes.
> 
>> I wonder if we can just flag the
>> btree inode to exclude from kupdate, or keep it off whatever list
>> kupdate cares about etc.
> 
> Not having the VM do writeback on metadata but running it from a fs
> LRU was a huge win in XFS.  I'm not sure we have interfaces that keep
> data in the pagecache but never do any background writeback.  But
> if you are fine with treating all background writeback equal that
> would be exactly where I'd like to go to.
> 

I'm not sure how much dirty metadata can accumulate on XFS, but btrfs
isn't bound by any logs, so the only limit on dirty metadata is the size
of the filesystem and/or the size of ram.

If we skipped all background writeout, we'd end up letting metadata grow
until the full dirty limit was hit, which feels too bursty to me (please
correct me if I've got that part wrong).

Josef spent a bunch of time on metadata-in-slab and our own LRU, but
with the volume of dirty tree blocks, my memory is that he basically
would have needed to recreate a bunch of the dirty balance plumbing, and
decided it wasn't worth it.

I'd certainly be in favor of making balance_dirty_pages() and background
writeback able to integrate with private LRUs.  If it's already possible
that's going to be a much better solution than our !for_kupdate hack.

-chris