Re: [PATCH 03/19] fs: release anon dev_t in deactivate_locked_super

2023-09-15 Thread Christian Brauner
> Lifetime rules for fs-private parts of superblock are really private to Fine, I'll drop that. It's still correct that a filesystem needs to take care when it frees sb->s_fs_info. See the RCU fun you just encountered.

Re: [PATCH 03/19] fs: release anon dev_t in deactivate_locked_super

2023-09-15 Thread Al Viro
On Fri, Sep 15, 2023 at 03:28:14PM +0100, Al Viro wrote: > On Fri, Sep 15, 2023 at 04:12:07PM +0200, Christian Brauner wrote: > > + static void some_fs_kill_sb(struct super_block *sb) > > + { > > + struct some_fs_info *info = sb->s_fs_info; > > + > > + kill_*_super(sb); > >

Re: [PATCH 03/19] fs: release anon dev_t in deactivate_locked_super

2023-09-15 Thread Al Viro
On Fri, Sep 15, 2023 at 04:12:07PM +0200, Christian Brauner wrote: > + static void some_fs_kill_sb(struct super_block *sb) > + { > + struct some_fs_info *info = sb->s_fs_info; > + > + kill_*_super(sb); > + kfree(info); > + } > + > +It's best practice

Re: [PATCH 03/19] fs: release anon dev_t in deactivate_locked_super

2023-09-15 Thread Christian Brauner
> > tree of any filesystem (in-tree one or not) will have to go through the > > changes and figure out WTF to do with their existing code. We are > > going to play whack-a-mole for at least several years as development > > branches get rebased and merged. > > Let me write something up. So here

Re: [PATCH 03/19] fs: release anon dev_t in deactivate_locked_super

2023-09-15 Thread Christian Brauner
On Thu, Sep 14, 2023 at 05:58:05PM +0100, Al Viro wrote: > On Thu, Sep 14, 2023 at 04:02:25PM +0200, Christian Brauner wrote: > > > Yes, you're right that making the superblock and not the filesytem type > > the bd_holder changes the logic and we are aware of that of course. And > > it requires

Re: [PATCH 03/19] fs: release anon dev_t in deactivate_locked_super

2023-09-15 Thread Christian Brauner
On Thu, Sep 14, 2023 at 08:23:31PM +0100, Al Viro wrote: > On Thu, Sep 14, 2023 at 05:58:05PM +0100, Al Viro wrote: > > > Incidentally, I'm going to add a (belated by 10 years) chunk in porting.rst > > re making sure that anything in superblock that might be needed by methods > > called in RCU

Re: [PATCH 03/19] fs: release anon dev_t in deactivate_locked_super

2023-09-14 Thread Al Viro
On Thu, Sep 14, 2023 at 05:58:05PM +0100, Al Viro wrote: > Incidentally, I'm going to add a (belated by 10 years) chunk in porting.rst > re making sure that anything in superblock that might be needed by methods > called in RCU mode should *not* be freed without an RCU delay... Should've > done

Re: [PATCH 03/19] fs: release anon dev_t in deactivate_locked_super

2023-09-14 Thread Al Viro
On Thu, Sep 14, 2023 at 04:02:25PM +0200, Christian Brauner wrote: > Yes, you're right that making the superblock and not the filesytem type > the bd_holder changes the logic and we are aware of that of course. And > it requires changes such as moving additional block device closing from > where

Re: [PATCH 03/19] fs: release anon dev_t in deactivate_locked_super

2023-09-14 Thread Christian Brauner
> Christoph, could you explain what the hell do we need that for? It does > create the race in question and AFAICS 2c18a63b760a (and followups trying > to plug holes in it) had been nothing but headache. > > Old logics: if mount attempt with a different fs type happens, -EBUSY > is precisely

Re: [PATCH 03/19] fs: release anon dev_t in deactivate_locked_super

2023-09-14 Thread Christian Brauner
> BTW, this part of commit message in 2c18a63b760a is rather confused: > Recent rework moved block device closing out of sb->put_super() and into > sb->kill_sb() to avoid deadlocks as s_umount is held in put_super() and > blkdev_put() can end up taking s_umount again. > > That was

Re: [PATCH 03/19] fs: release anon dev_t in deactivate_locked_super

2023-09-13 Thread Al Viro
On Thu, Sep 14, 2023 at 03:37:05AM +0100, Al Viro wrote: > On Thu, Sep 14, 2023 at 12:27:12AM +0100, Al Viro wrote: > > On Wed, Sep 13, 2023 at 08:09:57AM -0300, Christoph Hellwig wrote: > > > Releasing an anon dev_t is a very common thing when freeing a > > > super_block, as that's done for

Re: [PATCH 03/19] fs: release anon dev_t in deactivate_locked_super

2023-09-13 Thread Al Viro
On Thu, Sep 14, 2023 at 12:27:12AM +0100, Al Viro wrote: > On Wed, Sep 13, 2023 at 08:09:57AM -0300, Christoph Hellwig wrote: > > Releasing an anon dev_t is a very common thing when freeing a > > super_block, as that's done for basically any not block based file > > system (modulo the odd mtd

Re: [PATCH 03/19] fs: release anon dev_t in deactivate_locked_super

2023-09-13 Thread Al Viro
On Wed, Sep 13, 2023 at 08:09:57AM -0300, Christoph Hellwig wrote: > Releasing an anon dev_t is a very common thing when freeing a > super_block, as that's done for basically any not block based file > system (modulo the odd mtd special case). So instead of requiring > a special ->kill_sb helper

[PATCH 03/19] fs: release anon dev_t in deactivate_locked_super

2023-09-13 Thread Christoph Hellwig
Releasing an anon dev_t is a very common thing when freeing a super_block, as that's done for basically any not block based file system (modulo the odd mtd special case). So instead of requiring a special ->kill_sb helper and a lot of boilerplate in more complicated file systems, just release the