On Wed, Aug 27, 2025 at 05:20:17PM +0200, Christian Brauner wrote: > On Tue, Aug 26, 2025 at 11:39:15AM -0400, Josef Bacik wrote: > > Currently we have relied on dirty inodes and inodes with cache on them > > to simply be left hanging around on the system outside of an LRU. The > > only way to make sure these inodes are eventually reclaimed is because > > dirty writeback will grab a reference on the inode and then iput it when > > it's done, potentially getting it on the LRU. For the cached case the > > page cache deletion path will call inode_add_lru when the inode no > > longer has cached pages in order to make sure the inode object can be > > freed eventually. In the unmount case we walk all inodes and free them > > so this all works out fine. > > > > But we want to eliminate 0 i_count objects as a concept, so we need a > > mechanism to hold a reference on these pinned inodes. To that end, add a > > list to the super block that contains any inodes that are cached for one > > reason or another. > > > > When we call inode_add_lru(), if the inode falls into one of these > > categories, we will add it to the cached inode list and hold an > > i_obj_count reference. If the inode does not fall into one of these > > categories it will be moved to the normal LRU, which is already holds an > > i_obj_count reference. > > > > The dirty case we will delete it from the LRU if it is on one, and then > > the iput after the writeout will make sure it's placed onto the correct > > list at that point. > > > > The page cache case will migrate it when it calls inode_add_lru() when > > deleting pages from the page cache. > > > > Signed-off-by: Josef Bacik <josef@xxxxxxxxxxxxxx> > > --- > > Ok, I'm trying to wrap my head around the justification for this new > list. Currently we have inodes with a zero reference counts that aren't > on any LRU. They just appear on sb->i_sb_list and are e.g., dealt with > during umount (sync_filesystem() followed by evict_inodes()). > > So they're either dealt with by writeback or by the page cache and are > eventually put on the regular LRU or the filesystem shuts down before > that happens. > > They're easy to handle and recognize because their inode->i_count is > zero. > > Now you make the LRUs hold a full reference so it can be grabbed from > the LRU again avoiding the zombie resurrection from zero. So to > recognize inodes that are pinned internally due to being dirty or having > pagecache pages attached to it you need to track them in a new list > otherwise you can't really differentiate them and when to move them onto > the LRU after writeback and pagecache is done with them. > Exactly. We need to put them somewhere so we can account for their reference. We could technically just use a flag and not have a list for this, and just use the flag to indicate that the inode is pinned and the flag has a full reference associated with it. I did it this way because if I had a nickel for every time I needed to figure out where a zombie inode was and had to do the most grotesque drgn magic to find it, I'd have like 15 cents, which isn't a lot but weird that it's happened 3 times. Having a list makes it easier from a debugging perspective. But again, we have ->s_inodes, and I can just scan that list and look for I_LRU_CACHED. We'd still need to hold a full reference for that, but it would eliminate the need for another list if that's more preferable? Thanks, Josef
