On Thu, Sep 3, 2020 at 4:22 PM Xiubo Li <xiubli@xxxxxxxxxx> wrote:
>
> On 2020/9/3 22:18, Jeff Layton wrote:
> > On Thu, 2020-09-03 at 09:01 -0400, xiubli@xxxxxxxxxx wrote:
> >> From: Xiubo Li <xiubli@xxxxxxxxxx>
> >>
> >> Changed in V5:
> >> - Remove mdsc parsing helpers except the ceph_sb_to_mdsc()
> >> - Remove the is_opened member.
> >>
> >> Changed in V4:
> >> - A small fix about the total_inodes.
> >>
> >> Changed in V3:
> >> - Resend for V2 just forgot one patch, which is adding some helpers
> >> support to simplify the code.
> >>
> >> Changed in V2:
> >> - Add number of inodes that have opened files.
> >> - Remove the dir metrics and fold into files.
> >>
> >>
> >>
> >> Xiubo Li (2):
> >> ceph: add ceph_sb_to_mdsc helper support to parse the mdsc
> >> ceph: metrics for opened files, pinned caps and opened inodes
> >>
> >> fs/ceph/caps.c | 41 +++++++++++++++++++++++++++++++++++++----
> >> fs/ceph/debugfs.c | 11 +++++++++++
> >> fs/ceph/dir.c | 20 +++++++-------------
> >> fs/ceph/file.c | 13 ++++++-------
> >> fs/ceph/inode.c | 11 ++++++++---
> >> fs/ceph/locks.c | 2 +-
> >> fs/ceph/metric.c | 14 ++++++++++++++
> >> fs/ceph/metric.h | 7 +++++++
> >> fs/ceph/quota.c | 10 +++++-----
> >> fs/ceph/snap.c | 2 +-
> >> fs/ceph/super.h | 6 ++++++
> >> 11 files changed, 103 insertions(+), 34 deletions(-)
> >>
> > Looks good. I went ahead and merge this into testing.
> >
> > Small merge conflict in quota.c, which I guess is probably due to not
> > basing this on testing branch. I also dropped what looks like an
> > unrelated hunk in the second patch.
> >
> > In the future, if you can be sure that patches you post apply cleanly to
> > testing branch then that would make things easier.
>
> Okay, will do it.
Hi Xiubo,
There is a problem with lifetimes here. mdsc isn't guaranteed to exist
when ->free_inode() is called. This can lead to crashes on a NULL mdsc
in ceph_free_inode() in case of e.g. "umount -f". I know it was Jeff's
suggestion to move the decrement of total_inodes into ceph_free_inode(),
but it doesn't look like it can be easily deferred past ->evict_inode().
Thanks,
Ilya
