On Tue, Feb 13, 2024 at 6:42 AM Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote:
>
> On Mon, Feb 12, 2024 at 08:09:26AM +0000, Al Viro wrote:
>
> > Bloody bad idea, IMO. Note that straight use of kill_anon_super()
> > pretty much forces you into doing everything from ->put_super().
> > And that leads to rather clumsy failure exits in foo_fill_super(),
> > since you *won't* get ->put_super() called unless you've got to
> > setting ->s_root.
> >
> > Considering how easily the failure exits rot, I'd rather discourage
> > that variant.
>
> BTW, take a look at fs/ext2/super.c and compare the mess in failure
> exits in ext2_fill_super() with ext2_put_super(). See the amount of
> duplication?
>
> In case of ext2_fill_super() success eventual ->kill_sb() will call
> ->put_super() (from generic_shutdown_super(), from kill_block_super()).
>
> What happens in case of ext2_fill_super() failure? ->kill_sb() is called,
> but ->put_super() is only called if ->s_root is non-NULL (and at the very
> least it requires ->s_op to have been set). So in that case we have
> ext2_fill_super() manually undo the allocations, etc. it had managed to do,
> same as ext2_put_super() would've done.
>
> If that stuff gets lifted into ->kill_sb(), we get the bulk of ext2_put_super()
> moved into ext2_kill_super() (I wouldn't be surprised if ext2_put_super()
> completely disappeared, actually), with all those goto failed_mount<something>
> in ext2_fill_super() turning into plain return -E...
That sounds good, but I am not sure what you are advocating for?
What I wrote is that if kill_litter_super() becomes an alias of
kill_anon_super(),
then spraying kill_{anon,block}_super() automatically for new fs does not make
much sense from API POV.
It sounds like you are suggesting that the use of
kill_{anon,block}_super() should
be discouraged and that ext2 could be used to set an example.
I did not understand if you are suggesting API changes to encourage customized
->kill_sb()?
Thanks,
Amir.
