| Message ID | 20230921121945.4701-1-jack@suse.cz (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RESEND] btrfs: use the super_block as holder when mounting file systems | expand |
On Thu, Sep 21, 2023 at 02:19:45PM +0200, Jan Kara wrote: > From: Christoph Hellwig <hch@lst.de> > > The file system type is not a very useful holder as it doesn't allow us > to go back to the actual file system instance. Pass the super_block > instead which is useful when passed back to the file system driver. > > This matches what is done for all other block device based file systems and it > also fixes an issue that block device freezing (as used e.g. by LVM when > performing device snapshots) starts working for btrfs. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > Acked-by: Christian Brauner <brauner@kernel.org> > Reviewed-by: Josef Bacik <josef@toxicpanda.com> > Message-Id: <20230811100828.1897174-7-hch@lst.de> > Signed-off-by: Jan Kara <jack@suse.cz> > --- > fs/btrfs/super.c | 7 ++----- > 1 file changed, 2 insertions(+), 5 deletions(-) > > Hello, > > I'm resending this btrfs fix. Can you please merge it David? It's the only bit > remaining from the original Christoph's block device opening patches and is > blocking me in pushing out the opening of block devices using bdev_handle. I'll add the patch to the rest of the series, thanks. I don't have a timeframe when this will be moved to the main patch queue, until then it'll be in linux-next. The series has been sitting in the review backlog for a few weeks but no takers. I try to have a look from time to time but the kind of changes done there are not trivial so I'm still not confident enough to put to the main patch queue.
On Thu, Sep 21, 2023 at 02:50:07PM +0200, Christian Brauner wrote: > On Thu, Sep 21, 2023 at 02:19:45PM +0200, Jan Kara wrote: > > From: Christoph Hellwig <hch@lst.de> > > > > The file system type is not a very useful holder as it doesn't allow us > > to go back to the actual file system instance. Pass the super_block > > instead which is useful when passed back to the file system driver. > > > > This matches what is done for all other block device based file systems and it > > also fixes an issue that block device freezing (as used e.g. by LVM when > > performing device snapshots) starts working for btrfs. > > > > Signed-off-by: Christoph Hellwig <hch@lst.de> > > Acked-by: Christian Brauner <brauner@kernel.org> > > Reviewed-by: Josef Bacik <josef@toxicpanda.com> > > Message-Id: <20230811100828.1897174-7-hch@lst.de> > > Signed-off-by: Jan Kara <jack@suse.cz> > > --- > > fs/btrfs/super.c | 7 ++----- > > 1 file changed, 2 insertions(+), 5 deletions(-) > > > > Hello, > > > > I'm resending this btrfs fix. Can you please merge it David? It's the only bit > > remaining from the original Christoph's block device opening patches and is > > blocking me in pushing out the opening of block devices using bdev_handle. > > Thanks! > > Thanks for resending. > > Next time we will ensure that a vfs triggered conversion must go through > a vfs tree as this half converted state with forgotten patches is not > something that we should repeat. Taking this as an example, I really don't want to let such patches go through VFS git. I understand there are API-level cleanups done in VFS that require reorganizing code in the filesystems but IMO the conversions must be done in the filesystems first and the VFS cleanups as a followup. We have conflicting goals, you want better API and filesystems stand in the way so you/somebody fix it in a seemingly correct way and you want to merge it because it's ok for you. I view it as change from the outside potentially introducing bugs that we'll have to deal and unless we have somebody who's familiar with the changes to recognize the problems and fix bugs eventually it's risky. What I can do right now is that I'll keep the patches in linux-next, with the additional fix so the series is complete. I'll let you know if there's a change regarding the reviews or merge.
On Mon, Sep 25, 2023 at 05:54:09PM +0200, David Sterba wrote: > > Reviewed-by: Josef Bacik <josef@toxicpanda.com> > it'll be in linux-next. The series has been sitting in the review > backlog for a few weeks but no takers. I try to have a look from time I don't understand isn't Josef one of your reviewers/maintainers? Who is the person that must review it?
> > Next time we will ensure that a vfs triggered conversion must go through > > a vfs tree as this half converted state with forgotten patches is not > > something that we should repeat. > > Taking this as an example, I really don't want to let such patches go > through VFS git. I understand there are API-level cleanups done in > VFS that require reorganizing code in the filesystems but IMO the > conversions must be done in the filesystems first and the VFS cleanups > as a followup. Whether or not this order is possible depends on the patchset. If we do VFS level conversions that impact all filesystems we will do changes to all filesystems in one go if the changes need to be done all at the same time as we have done many times in the past. There's really nothing new. > there's a change regarding the reviews or merge. We'll likely end pulling in the required btrfs changes from you anyway so Jan's series isn't blocked indefinitely from getting testing in -next.
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c index 4577cd64da2e..94ca2236e57f 100644 --- a/fs/btrfs/super.c +++ b/fs/btrfs/super.c @@ -70,8 +70,6 @@ static const struct super_operations btrfs_super_ops; * requested by subvol=/path. That way the callchain is straightforward and we * don't have to play tricks with the mount options and recursive calls to * btrfs_mount. - * - * The new btrfs_root_fs_type also servers as a tag for the bdev_holder. */ static struct file_system_type btrfs_fs_type; static struct file_system_type btrfs_root_fs_type; @@ -1462,8 +1460,7 @@ static struct dentry *btrfs_mount_root(struct file_system_type *fs_type, struct btrfs_fs_devices *fs_devices = fs_info->fs_devices; mutex_lock(&uuid_mutex); - error = btrfs_open_devices(fs_devices, sb_open_mode(flags), - fs_type); + error = btrfs_open_devices(fs_devices, sb_open_mode(flags), s); mutex_unlock(&uuid_mutex); if (error) goto error_deactivate; @@ -1477,7 +1474,7 @@ static struct dentry *btrfs_mount_root(struct file_system_type *fs_type, fs_devices->latest_dev->bdev); shrinker_debugfs_rename(&s->s_shrink, "sb-%s:%s", fs_type->name, s->s_id); - btrfs_sb(s)->bdev_holder = fs_type; + btrfs_sb(s)->bdev_holder = s; error = btrfs_fill_super(s, fs_devices, data); } if (!error)