On Sun, 19 Jan 2025 at 15:57, James Bottomley <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx> wrote: > > On Sun, 2025-01-19 at 15:50 +0100, Ard Biesheuvel wrote: > > On Thu, 16 Jan 2025 at 23:13, James Bottomley > > <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx> wrote: > > > > > > On Thu, 2025-01-16 at 14:05 -0500, James Bottomley wrote: > > > > On Thu, 2025-01-16 at 18:36 +0000, Al Viro wrote: > > > > > On Mon, Jan 06, 2025 at 06:35:23PM -0800, James Bottomley > > > > > wrote: > > > > > > Make the inodes the default management vehicle for struct > > > > > > efivar_entry, so they are now all freed automatically if the > > > > > > file is removed and on unmount in kill_litter_super(). > > > > > > Remove the now superfluous iterator to free the entries after > > > > > > kill_litter_super(). > > > > > > > > > > > > Also fixes a bug where some entry freeing was missing causing > > > > > > efivarfs to leak memory. > > > > > > > > > > Umm... I'd rather coallocate struct inode and struct > > > > > efivar_entry; that way once you get rid of the list you don't > > > > > need - evict_inode() anymore. > > > > > > > > > > It's pretty easy - see e.g. > > > > > https://lore.kernel.org/all/20250112080705.141166-1-viro@xxxxxxxxxxxxxxxxxx/ > > > > > for recent example of such conversion. > > > > > > > > OK, I can do that. Although I think since the number of > > > > variables is usually around 150, it would probably be overkill to > > > > give it its own inode cache allocator. > > > > > > OK, this is what I've got. As you can see from the diffstat it's > > > about 10 lines more than the previous; mostly because of the new > > > allocation routine, the fact that the root inode has to be special > > > cased for the list and the guid has to be parsed in efivarfs_create > > > before we have the inode. > > > > > > > That looks straight-forward enough. > > > > Can you send this as a proper patch? Or would you prefer me to squash > > this into the one that is already queued up? > > Sure; I've got a slightly different version because after talking with > Al he thinks it's OK still to put a pointer to the efivar_entry in > i_private, which means less disruption. But there is enough disruption > that the whole of the series needs redoing to avoid conflicts. > > > I'm fine with either, but note that I'd still like to target v6.14 > > with this. > > Great, but I'm afraid the fix for the zero size problem also could do > with being a precursor which is making the timing pretty tight. > OK, I'll queue up your v3 about I won't send it out with the initial pull request, so we can make up our minds later. I take it the setattr series is intended for merging straight away?
