Message ID | 5e4e7c68ef710c23034d6a7a180e8912d6ebbc7d.1603460665.git.josef@toxicpanda.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | Block group caching fixes | expand |
On Fri, Oct 23, 2020 at 5:14 PM Josef Bacik <josef@toxicpanda.com> wrote: > > Historically we've allowed recursive locking specifically for the free > space inode. This is because we are only doing reads and know that it's > safe. However we don't actually need this feature, we can get away with > reading the commit root for the extents. In fact if we want to allow > asynchronous loading of the free space cache we have to use the commit > root, otherwise we will deadlock. > > Switch to using the commit root for the file extents. These are only > read at load time, and are replaced as soon as we start writing the > cache out to disk. The cache is never read again, so this is > legitimate. This matches what we do for the inode itself, as we read > that from the commit root as well. > > Signed-off-by: Josef Bacik <josef@toxicpanda.com> Reviewed-by: Filipe Manana <fdmanana@suse.com> Looks good, thanks. > --- > fs/btrfs/inode.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c > index 1dcccd212809..53d6a66670d3 100644 > --- a/fs/btrfs/inode.c > +++ b/fs/btrfs/inode.c > @@ -6577,7 +6577,15 @@ struct extent_map *btrfs_get_extent(struct btrfs_inode *inode, > */ > path->leave_spinning = 1; > > - path->recurse = btrfs_is_free_space_inode(inode); > + /* > + * The same explanation in load_free_space_cache applies here as well, > + * we only read when we're loading the free space cache, and at that > + * point the commit_root has everything we need. > + */ > + if (btrfs_is_free_space_inode(inode)) { > + path->search_commit_root = 1; > + path->skip_locking = 1; > + } > > ret = btrfs_lookup_file_extent(NULL, root, path, objectid, start, 0); > if (ret < 0) { > -- > 2.26.2 >
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c index 1dcccd212809..53d6a66670d3 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -6577,7 +6577,15 @@ struct extent_map *btrfs_get_extent(struct btrfs_inode *inode, */ path->leave_spinning = 1; - path->recurse = btrfs_is_free_space_inode(inode); + /* + * The same explanation in load_free_space_cache applies here as well, + * we only read when we're loading the free space cache, and at that + * point the commit_root has everything we need. + */ + if (btrfs_is_free_space_inode(inode)) { + path->search_commit_root = 1; + path->skip_locking = 1; + } ret = btrfs_lookup_file_extent(NULL, root, path, objectid, start, 0); if (ret < 0) {
Historically we've allowed recursive locking specifically for the free space inode. This is because we are only doing reads and know that it's safe. However we don't actually need this feature, we can get away with reading the commit root for the extents. In fact if we want to allow asynchronous loading of the free space cache we have to use the commit root, otherwise we will deadlock. Switch to using the commit root for the file extents. These are only read at load time, and are replaced as soon as we start writing the cache out to disk. The cache is never read again, so this is legitimate. This matches what we do for the inode itself, as we read that from the commit root as well. Signed-off-by: Josef Bacik <josef@toxicpanda.com> --- fs/btrfs/inode.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)