Re: [PATCH] ceph: make sure all the files successfully put before unmounting

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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.

- Xiubo


(Based on the code, it might also be possible for evict_inodes() to also see
nonzero refcount inodes due to fsnotify.  However, fsnotify_sb_delete() runs
before fscrypt_destroy_keyring(), so likewise it seems irrelevant here.)

- Eric





[Index of Archives]     [CEPH Users]     [Ceph Large]     [Ceph Dev]     [Information on CEPH]     [Linux BTRFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux