Hi Xiubo,
On Fri, Dec 02, 2022 at 03:04:58PM +0800, Xiubo Li wrote:
>
> On 02/12/2022 12:19, Eric Biggers wrote:
> > On Fri, Dec 02, 2022 at 09:49:49AM +0800, Xiubo Li wrote:
> > > On 02/12/2022 05:10, Eric Biggers wrote:
> > > > On Thu, Dec 01, 2022 at 11:18:33AM -0800, Eric Biggers wrote:
> > > > > On Thu, Dec 01, 2022 at 02:58:00PM +0800, xiubli@xxxxxxxxxx wrote:
> > > > > > From: Xiubo Li <xiubli@xxxxxxxxxx>
> > > > > >
> > > > > > When close a file it will be deferred to call the fput(), which
> > > > > > will hold the inode's i_count. And when unmounting the mountpoint
> > > > > > the evict_inodes() may skip evicting some inodes.
> > > > > >
> > > > > > If encrypt is enabled the kernel generate a warning when removing
> > > > > > the encrypt keys when the skipped inodes still hold the keyring:
> > > > > This does not make sense. Unmounting is only possible once all the files on the
> > > > > filesystem have been closed.
> > > > >
> > > > Specifically, __fput() puts the reference to the dentry (and thus the inode)
> > > > *before* it puts the reference to the mount. And an unmount cannot be done
> > > > while the mount still has references. So there should not be any issue here.
> > > Eric,
> > >
> > > When I unmounting I can see the following logs, which I added a debug log in
> > > the evcit_inodes():
> > >
> > > diff --git a/fs/inode.c b/fs/inode.c
> > > index b608528efd3a..f6e69b778d9c 100644
> > > --- a/fs/inode.c
> > > +++ b/fs/inode.c
> > > @@ -716,8 +716,11 @@ void evict_inodes(struct super_block *sb)
> > > again:
> > > spin_lock(&sb->s_inode_list_lock);
> > > list_for_each_entry_safe(inode, next, &sb->s_inodes, i_sb_list) {
> > > - if (atomic_read(&inode->i_count))
> > > + if (atomic_read(&inode->i_count)) {
> > > + printk("evict_inodes inode %p, i_count = %d, was
> > > skipped!\n",
> > > + inode, atomic_read(&inode->i_count));
> > > continue;
> > > + }
> > >
> > > spin_lock(&inode->i_lock);
> > > if (inode->i_state & (I_NEW | I_FREEING | I_WILL_FREE)) {
> > >
> > > The logs:
> > >
> > > <4>[ 95.977395] evict_inodes inode 00000000f90aab7b, i_count = 1, was
> > > skipped!
> > >
> > > Any reason could cause this ? Since the inode couldn't be evicted in time
> > > and then when removing the master keys it will print this warning.
> > >
> > It is expected for evict_inodes() to see some inodes with nonzero refcount, but
> > they should only be filesystem internal inodes. For example, with ext4 this
> > happens with the journal inode.
> >
> > However, filesystem internal inodes cannot be encrypted, so they are irrelevant
> > here.
> >
> > I'd guess that CephFS has a bug where it is leaking a reference to a user inode
> > somewhere.
>
> I also added some debug logs to tracker all the inodes in ceph, and all the
> requests has been finished.
>
> I will debug it more to see whether it's leaking a reference here.
>
> Thanks Eric.
>
Any progress on tracking this down?
- Eric
