Re: [PATCH v1 1/5] hostfs: Fix ephemeral inodes

[Mickaël Salaün]

On 21/05/2023 23:13, Richard Weinberger wrote:
> ----- Ursprüngliche Mail -----
> > Von: "Mickaël Salaün" <mic@xxxxxxxxxxx>
> > hostfs creates a new inode for each opened or created file, which created
> > useless inode allocations and forbade identifying a host file with a kernel
> > inode.
> >
> > Fix this uncommon filesystem behavior by tying kernel inodes to host
> > file's inode and device IDs.  Even if the host filesystem inodes may be
> > recycled, this cannot happen while a file referencing it is open, which
> > is the case with hostfs.  It should be noted that hostfs inode IDs may
> > not be unique for the same hostfs superblock because multiple host's
> > (backed) superblocks may be used.
> >
> > Delete inodes when dropping them to force backed host's file descriptors
> > closing.
> >
> > This enables to entirely remove ARCH_EPHEMERAL_INODES, and then makes
> > Landlock fully supported by UML.  This is very useful for testing
> > (ongoing and backported) changes.
>
> Removing ARCH_EPHEMERAL_INODES should be a patch on its own, IMHO.

OK, I'll do that in the next series.

> > These changes also factor out and simplify some helpers thanks to the
> > new hostfs_inode_update() and the hostfs_iget() revamp: read_name(),
> > hostfs_create(), hostfs_lookup(), hostfs_mknod(), and
> > hostfs_fill_sb_common().
> >
> > A following commit with new Landlock tests check this new hostfs inode
> > consistency.
> >
> > Cc: Anton Ivanov <anton.ivanov@xxxxxxxxxxxxxxxxxx>
> > Cc: Johannes Berg <johannes@xxxxxxxxxxxxxxxx>
> > Cc: Richard Weinberger <richard@xxxxxx>
> > Cc: <stable@xxxxxxxxxxxxxxx> # 5.15.x: ce72750f04d6: hostfs: Fix writeback of
> > dirty pages
> > Cc: <stable@xxxxxxxxxxxxxxx> # 5.15+
>
> I'm not sure whether this patch qualifies as stable material.
> While I fully agree that the current behavoir is odd, nothing user visible
> is really broken so far.
I added the ARCH_EPHEMERAL_INODES knob to avoid unexpected behavior. 
Thanks to that there is no regression for Landlock, but it's unfortunate 
that we could not use UML to test old kernel versions. According to this 
odd behavior, I guess some user space may not work with hostfs because 
of this issue, hence this Cc. I can remove it if you think it is not the 
case.


> > Signed-off-by: Mickaël Salaün <mic@xxxxxxxxxxx>
> > Link: https://lore.kernel.org/r/20230309165455.175131-2-mic@xxxxxxxxxxx
>
> Other than that, patch looks good to me.

Good, I'll send a new series with your suggestions.

> Thanks,
> //richard