fs: Move metadata bh tracking from address_space#917
Closed
vfsci-bot[bot] wants to merge 42 commits intovfs.base.cifrom
Closed
fs: Move metadata bh tracking from address_space#917vfsci-bot[bot] wants to merge 42 commits intovfs.base.cifrom
vfsci-bot[bot] wants to merge 42 commits intovfs.base.cifrom
Conversation
Instead of checking i_private_list directly use appropriate wrapper inode_has_buffers(). Also delete stale comment. Acked-by: Theodore Ts'o <tytso@mit.edu> Signed-off-by: Jan Kara <jack@suse.cz>
Remove the explicit zeroing of mapping->i_private_data since this field is no longer used. CC: Andreas Gruenbacher <agruenba@redhat.com> CC: gfs2@lists.linux.dev Reviewed-by: Andreas Gruenbacher <agruenba@redhat.com> Signed-off-by: Jan Kara <jack@suse.cz>
…fers() calls ntfs3 never calls mark_buffer_dirty_inode() and thus its metadata buffers list is always empty. Drop the pointless sync_mapping_buffers() and invalidate_inode_buffers() calls. CC: Konstantin Komarov <almaz.alexandrovich@paragon-software.com> CC: ntfs3@lists.linux.dev Signed-off-by: Jan Kara <jack@suse.cz>
ocfs2 never calls mark_buffer_dirty_inode() and thus its metadata buffers list is always empty. Drop the pointless sync_mapping_buffers() calls. CC: Joel Becker <jlbec@evilplan.org> CC: Joseph Qi <joseph.qi@linux.alibaba.com> CC: ocfs2-devel@lists.linux.dev Signed-off-by: Jan Kara <jack@suse.cz> Reviewed-by: Joseph Qi <joseph.qi@linux.alibaba.com>
Nobody is calling mark_buffer_dirty_inode() with internal bdev inode and it doesn't make sense for internal bdev inode to have any metadata buffer heads. Just drop the pointless invalidate_inode_buffers() call and consequently the whole bdev_evict_inode() because generic code takes care of the rest. CC: linux-block@vger.kernel.org Signed-off-by: Jan Kara <jack@suse.cz>
UFS doesn't call mark_buffer_dirty_inode() and thus invalidate_mapping_buffers() never has anything to drop. Remove the pointless call. Signed-off-by: Jan Kara <jack@suse.cz>
EXFAT never calls mark_buffer_dirty_inode() and thus invalidate_inode_buffers() never has anything to evict. Drop the pointless call. Signed-off-by: Jan Kara <jack@suse.cz>
Inode lock in __generic_file_fsync() protects sync_mapping_buffers() and sync_inode_metadata() calls. Neither sync_mapping_buffers() nor sync_inode_metadata() themselves need the protection by inode_lock and both metadata buffer head writeback and inode writeback can happen without inode lock (either in case of background writeback or sync(2) calls). The only protection inode_lock can possibly provide is that write(2) or other inode modifying calls cannot happen in the middle of bh+inode writeout and thus result in writeout of inconsistent metadata. However if writes and fsyncs race, background writeback can submit inconsistent metadata just after fsync completed even with inode_lock protecting fsync so this seems moot as well. So let's remove the apparently pointless inode_lock protection. Signed-off-by: Jan Kara <jack@suse.cz>
UDF uses metadata bh list attached to inode. Switch it to generic_buffers_fsync() instead of generic_file_fsync() as we'll be removing metadata bh handling from generic_file_fsync(). Signed-off-by: Jan Kara <jack@suse.cz>
Minix uses list of metadata bhs attached to an inode. Switch it to generic_buffers_fsync() instead of generic_file_fsync() as we'll be removing metadata bh handling from generic_file_fsync(). Signed-off-by: Jan Kara <jack@suse.cz>
BFS uses list of metadata bhs attached to an inode. Switch it to use generic_buffers_fsync() instead of generic_file_fsync() as we'll be removing metadata bh handling from generic_file_fsync(). Signed-off-by: Jan Kara <jack@suse.cz>
FAT uses a list of metadata bhs attached to an inode. Switch it to use generic_buffers_fsync_noflush() instead of __generic_file_fsync() as we'll be removing metadata bh handling from __generic_file_fsync(). Signed-off-by: Jan Kara <jack@suse.cz>
No filesystem calling __generic_file_fsync() uses metadata bh tracking. Drop sync_mapping_buffers() call from __generic_file_fsync() as it's pointless now which untangles buffer head handling from fs/libfs.c. Signed-off-by: Jan Kara <jack@suse.cz>
The implementation is now really basic so rename generic_file_fsync() simple_fsync() and __generic_file_fsync() to simple_fsync_noflush(). Signed-off-by: Jan Kara <jack@suse.cz>
There are only very few filesystems using generic metadata buffer head tracking and everybody is paying the overhead. When we remove this tracking for inode reclaim code .evict will start to see inodes with metadata buffers attached so write them out and prune them. Signed-off-by: Jan Kara <jack@suse.cz>
There are only very few filesystems using generic metadata buffer head tracking and everybody is paying the overhead. When we remove this tracking for inode reclaim code .evict will start to see inodes with metadata buffers attached so write them out and prune them. Signed-off-by: Jan Kara <jack@suse.cz>
There are only very few filesystems using generic metadata buffer head tracking and everybody is paying the overhead. When we remove this tracking for inode reclaim code .evict will start to see inodes with metadata buffers attached so write them out and prune them. Signed-off-by: Jan Kara <jack@suse.cz>
There are only very few filesystems using generic metadata buffer head tracking and everybody is paying the overhead. When we remove this tracking for inode reclaim code .evict will start to see inodes with metadata buffers attached so write them out and prune them. Signed-off-by: Jan Kara <jack@suse.cz>
There are only very few filesystems using generic metadata buffer head tracking and everybody is paying the overhead. When we remove this tracking for inode reclaim code .evict will start to see inodes with metadata buffers attached so write them out and prune them. Acked-by: Theodore Ts'o <tytso@mit.edu> Signed-off-by: Jan Kara <jack@suse.cz>
There are only very few filesystems using generic metadata buffer head tracking and everybody is paying the overhead. When we remove this tracking for inode reclaim code .evict will start to see inodes with metadata buffers attached so write them out and prune them. Signed-off-by: Jan Kara <jack@suse.cz>
There are only very few filesystems using generic metadata buffer head tracking and everybody is paying the overhead. When we remove this tracking for inode reclaim code .evict will start to see inodes with metadata buffers attached so write them out and prune them. Signed-off-by: Jan Kara <jack@suse.cz>
There are only a few filesystems that use generic tracking of inode metadata buffer heads. As such the logic to reclaim tracked metadata buffer heads in inode_lru_isolate() doesn't bring a benefit big enough to justify intertwining of inode reclaim and metadata buffer head tracking. Just treat tracked metadata buffer heads as any other metadata filesystem has to properly clean up on inode eviction and stop handling it in inode_lru_isolate(). As a result filesystems using generic tracking of metadata buffer heads may now see dirty metadata buffers in their .evict methods more often which can slow down inode reclaim but given these filesystems aren't used in performance demanding setups we should be fine. Signed-off-by: Jan Kara <jack@suse.cz>
All filesystem using generic metadata bh tracking are using bdev mapping as a backing for these bhs. Stop using i_private_data for it and get to bdev mapping directly. Reviewed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Jan Kara <jack@suse.cz>
Instead of using i_private_data for resv_map pointer add the pointer into hugetlbfs private part of the inode. Reviewed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Jan Kara <jack@suse.cz>
Instead of using i_private_data and i_private_lock, just create aio inodes with appropriate necessary fields. Reviewed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Jan Kara <jack@suse.cz>
Nobody is using it anymore. Reviewed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Jan Kara <jack@suse.cz>
Instead of using mapping->i_private_list use a list in private part of the inode. CC: kvm@vger.kernel.org CC: Paolo Bonzini <pbonzini@redhat.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Jan Kara <jack@suse.cz>
The function only waits for already locked buffers in the list of metadata bhs. fsync_buffers_list() has just waited for all outstanding IO on buffers so this isn't adding anything useful. Comment in front of fsync_buffers_list() mentions concerns about buffers being moved out from tmp list back to mappings i_private_list but these days mark_buffer_dirty_inode() doesn't touch inodes with b_assoc_map set so that cannot happen. Just delete the stale code. Reviewed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Jan Kara <jack@suse.cz>
There's only single caller of fsync_buffers_list() so untangle the code a bit by folding fsync_buffers_list() into sync_mapping_buffers(). Also merge the comments and update them to reflect current state of code. Reviewed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Jan Kara <jack@suse.cz>
Instead of tracking metadata bhs for a mapping using i_private_list and i_private_lock create a dedicated mapping_metadata_bhs struct for it. So far this struct is embedded in address_space but that will be switched for per-fs private inode parts later in the series. This also changes the locking from bdev mapping's i_private_lock to a new lock embedded in mapping_metadata_bhs to untangle the i_private_lock locking for maintaining lists of metadata bhs and the locking for looking up / reclaiming bdev's buffer heads. The locking in remove_assoc_map() gets more complex due to this but overall this looks like a reasonable tradeoff. Reviewed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Jan Kara <jack@suse.cz>
Make buffer heads point to mapping_metadata_bhs instead of struct address_space. This makes the code more self contained. For the (only) case of IO error handling where we really need to reach struct address_space add a pointer to the mapping from mapping_metadata_bhs. Reviewed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Jan Kara <jack@suse.cz>
As part of a move towards placing mapping_metadata_bhs in fs-private inode part, switch inode_has_buffers() to take mapping_metadata_bhs and rename the function to mmb_has_buffers(). Reviewed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Jan Kara <jack@suse.cz>
As part of transition toward moving mapping_metadata_bhs to fs-private part of the inode, provide functions for operations on this list directly instead of going through the inode / mapping. Signed-off-by: Jan Kara <jack@suse.cz>
Track metadata bhs for an inode in fs-private part of the inode. Signed-off-by: Jan Kara <jack@suse.cz>
Track metadata bhs for an inode in fs-private part of the inode. Signed-off-by: Jan Kara <jack@suse.cz>
Track metadata bhs for an inode in fs-private part of the inode. Signed-off-by: Jan Kara <jack@suse.cz>
Track metadata bhs for an inode in fs-private part of the inode. Signed-off-by: Jan Kara <jack@suse.cz>
Track metadata bhs for an inode in fs-private part of the inode. Signed-off-by: Jan Kara <jack@suse.cz>
Track metadata bhs for an inode in fs-private part of the inode. Signed-off-by: Jan Kara <jack@suse.cz>
Track metadata bhs for an inode in fs-private part of the inode. We need the tracking only for nojournal mode so this is somewhat wasteful. We can relatively easily make the mapping_metadata_bhs struct dynamically allocated similarly to how we treat jbd2_inode but let's leave that for ext4 specific series once the dust settles a bit. Signed-off-by: Jan Kara <jack@suse.cz>
Nobody uses mapping_metadata_bhs in struct address_space anymore. Just remove it and with it all helper functions using it. Signed-off-by: Jan Kara <jack@suse.cz>
Nobody is using i_private_list anymore. Remove it. Signed-off-by: Jan Kara <jack@suse.cz>
Author
|
This PR is older than 14 days. Closing automatically. If the series is still relevant, a new version will create a new PR. Automated by ml2pr |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Series: https://patchwork.kernel.org/project/linux-fsdevel/list/?series=1072843
Submitter: Jan Kara
Version: 1
Patches: 42/42
Message-ID:
<20260326082428.31660-1-jack@suse.cz>Base: vfs.base.ci
Lore: https://lore.kernel.org/linux-fsdevel/20260326082428.31660-1-jack@suse.cz
Automated by ml2pr