Hello, On Tue, Jan 31, 2023 at 10:31:31PM -0800, Eric Biggers wrote: > > These can usually be handled by explicitly associating the bio's to the > > desired cgroups using one of bio_associate_blkg*() or > > bio_clone_blkg_association(). > > Here that already happens in wbc_init_bio(), called from io_submit_init_bio() in > fs/ext4/page-io.c. Yeah, without bouncing, that's usually how writeback IOs are associated with their cgroups. > > It is possible to go through memcg ownership > > too using set_active_memcg() so that the page is owned by the target cgroup; > > however, the page ownership doesn't directly map to IO ownership as the > > relationship depends on the type of the page (e.g. IO ownership for > > pagecache writeback is determined per-inode, not per-page). If the in-flight > > pages are limited, it probably is better to set bio association directly. > > ext4 also calls wbc_account_cgroup_owner() for each pagecache page that's > written out. It seems this is for a different purpose -- it looks like the > fs-writeback code is trying to figure out which cgroup "owns" the inode based on > which cgroup "owns" most of the pagecache pages? Yeah, there's a difference between how memory and IO track cgroup ownership. Memory ownership is per-page but IO ownership is per-inode. This is because splitting writeback IOs of the same inode can perform really badly, so we try to find the majority dirty page owner cgroup of a given inode and associate the whole inode to that cgroup. So, something like md / dm, which gets a bio from filesystem and then bounces it to another bio, would use either bio_clone_blkg_association() to copy the association of the original bio (which probably is set through wbc_init_bio()) or determine the cgroup the bio should belong to somehow and set it explicitly with bio_associate_blkg(). However, here, as the filesystem is the one bouncing I guess it can be simpler. > The bug we're discussing here is that when ext4 writes out a pagecache page in > an encrypted file, it first encrypts the data into a bounce page, then passes > the bounce page (which don't have a memcg) to wbc_account_cgroup_owner(). Maybe > the proper fix is to just pass the pagecache page to wbc_account_cgroup_owner() > instead? See below for ext4 (a separate patch would be needed for f2fs): Yeah, this makes sense to me and is the right thing to do no matter what. wbc_account_cgroup_owner() should be fed the origin page so that the IO can be blamed on the owner of that page. Thanks. -- tejun
