| Message ID | 20211122142212.1621-1-sehuww@mail.scut.edu.cn (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ceph: fix duplicate increment of opened_inodes metric | expand |
On Mon, 2021-11-22 at 22:22 +0800, Hu Weiwen wrote: > opened_inodes is incremented twice when the same inode is opened twice > with O_RDONLY and O_WRONLY respectively. > > To reproduce, run this python script, then check the metrics: > > import os > for _ in range(10000): > fd_r = os.open('a', os.O_RDONLY) > fd_w = os.open('a', os.O_WRONLY) > os.close(fd_r) > os.close(fd_w) > > Fixes: 1dd8d4708136 ("ceph: metrics for opened files, pinned caps and opened inodes") > Signed-off-by: Hu Weiwen <sehuww@mail.scut.edu.cn> > --- > fs/ceph/caps.c | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c > index b9460b6fb76f..c447fa2e2d1f 100644 > --- a/fs/ceph/caps.c > +++ b/fs/ceph/caps.c > @@ -4350,7 +4350,7 @@ void ceph_get_fmode(struct ceph_inode_info *ci, int fmode, int count) > { > struct ceph_mds_client *mdsc = ceph_sb_to_mdsc(ci->vfs_inode.i_sb); > int bits = (fmode << 1) | 1; > - bool is_opened = false; > + bool already_opened = false; > int i; > > if (count == 1) > @@ -4358,19 +4358,19 @@ void ceph_get_fmode(struct ceph_inode_info *ci, int fmode, int count) > > spin_lock(&ci->i_ceph_lock); > for (i = 0; i < CEPH_FILE_MODE_BITS; i++) { > - if (bits & (1 << i)) > - ci->i_nr_by_mode[i] += count; > - > /* > - * If any of the mode ref is larger than 1, > + * If any of the mode ref is larger than 0, > * that means it has been already opened by > * others. Just skip checking the PIN ref. > */ > - if (i && ci->i_nr_by_mode[i] > 1) > - is_opened = true; > + if (i && ci->i_nr_by_mode[i]) > + already_opened = true; > + > + if (bits & (1 << i)) > + ci->i_nr_by_mode[i] += count; > } > > - if (!is_opened) > + if (!already_opened) > percpu_counter_inc(&mdsc->metric.opened_inodes); > spin_unlock(&ci->i_ceph_lock); > } Thanks for the patch. I think it's correct, but I'd like Xiubo to confirm before we merge this.
On 11/22/21 10:22 PM, Hu Weiwen wrote: > opened_inodes is incremented twice when the same inode is opened twice > with O_RDONLY and O_WRONLY respectively. > > To reproduce, run this python script, then check the metrics: > > import os > for _ in range(10000): > fd_r = os.open('a', os.O_RDONLY) > fd_w = os.open('a', os.O_WRONLY) > os.close(fd_r) > os.close(fd_w) > > Fixes: 1dd8d4708136 ("ceph: metrics for opened files, pinned caps and opened inodes") > Signed-off-by: Hu Weiwen <sehuww@mail.scut.edu.cn> > --- > fs/ceph/caps.c | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c > index b9460b6fb76f..c447fa2e2d1f 100644 > --- a/fs/ceph/caps.c > +++ b/fs/ceph/caps.c > @@ -4350,7 +4350,7 @@ void ceph_get_fmode(struct ceph_inode_info *ci, int fmode, int count) > { > struct ceph_mds_client *mdsc = ceph_sb_to_mdsc(ci->vfs_inode.i_sb); > int bits = (fmode << 1) | 1; > - bool is_opened = false; > + bool already_opened = false; > int i; > > if (count == 1) > @@ -4358,19 +4358,19 @@ void ceph_get_fmode(struct ceph_inode_info *ci, int fmode, int count) > > spin_lock(&ci->i_ceph_lock); > for (i = 0; i < CEPH_FILE_MODE_BITS; i++) { > - if (bits & (1 << i)) > - ci->i_nr_by_mode[i] += count; > - > /* > - * If any of the mode ref is larger than 1, > + * If any of the mode ref is larger than 0, > * that means it has been already opened by > * others. Just skip checking the PIN ref. > */ > - if (i && ci->i_nr_by_mode[i] > 1) > - is_opened = true; > + if (i && ci->i_nr_by_mode[i]) > + already_opened = true; > + > + if (bits & (1 << i)) > + ci->i_nr_by_mode[i] += count; > } > > - if (!is_opened) > + if (!already_opened) > percpu_counter_inc(&mdsc->metric.opened_inodes); > spin_unlock(&ci->i_ceph_lock); > } LGTM. Reviewed-by: Xiubo Li <xiubli@redhat.com>
On Mon, 2021-11-22 at 22:22 +0800, Hu Weiwen wrote: > opened_inodes is incremented twice when the same inode is opened twice > with O_RDONLY and O_WRONLY respectively. > > To reproduce, run this python script, then check the metrics: > > import os > for _ in range(10000): > fd_r = os.open('a', os.O_RDONLY) > fd_w = os.open('a', os.O_WRONLY) > os.close(fd_r) > os.close(fd_w) > > Fixes: 1dd8d4708136 ("ceph: metrics for opened files, pinned caps and opened inodes") > Signed-off-by: Hu Weiwen <sehuww@mail.scut.edu.cn> > --- > fs/ceph/caps.c | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c > index b9460b6fb76f..c447fa2e2d1f 100644 > --- a/fs/ceph/caps.c > +++ b/fs/ceph/caps.c > @@ -4350,7 +4350,7 @@ void ceph_get_fmode(struct ceph_inode_info *ci, int fmode, int count) > { > struct ceph_mds_client *mdsc = ceph_sb_to_mdsc(ci->vfs_inode.i_sb); > int bits = (fmode << 1) | 1; > - bool is_opened = false; > + bool already_opened = false; > int i; > > if (count == 1) > @@ -4358,19 +4358,19 @@ void ceph_get_fmode(struct ceph_inode_info *ci, int fmode, int count) > > spin_lock(&ci->i_ceph_lock); > for (i = 0; i < CEPH_FILE_MODE_BITS; i++) { > - if (bits & (1 << i)) > - ci->i_nr_by_mode[i] += count; > - > /* > - * If any of the mode ref is larger than 1, > + * If any of the mode ref is larger than 0, > * that means it has been already opened by > * others. Just skip checking the PIN ref. > */ > - if (i && ci->i_nr_by_mode[i] > 1) > - is_opened = true; > + if (i && ci->i_nr_by_mode[i]) > + already_opened = true; > + > + if (bits & (1 << i)) > + ci->i_nr_by_mode[i] += count; > } > > - if (!is_opened) > + if (!already_opened) > percpu_counter_inc(&mdsc->metric.opened_inodes); > spin_unlock(&ci->i_ceph_lock); > } Merged into the ceph-client testing branch. Thanks for the patch!
diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c index b9460b6fb76f..c447fa2e2d1f 100644 --- a/fs/ceph/caps.c +++ b/fs/ceph/caps.c @@ -4350,7 +4350,7 @@ void ceph_get_fmode(struct ceph_inode_info *ci, int fmode, int count) { struct ceph_mds_client *mdsc = ceph_sb_to_mdsc(ci->vfs_inode.i_sb); int bits = (fmode << 1) | 1; - bool is_opened = false; + bool already_opened = false; int i; if (count == 1) @@ -4358,19 +4358,19 @@ void ceph_get_fmode(struct ceph_inode_info *ci, int fmode, int count) spin_lock(&ci->i_ceph_lock); for (i = 0; i < CEPH_FILE_MODE_BITS; i++) { - if (bits & (1 << i)) - ci->i_nr_by_mode[i] += count; - /* - * If any of the mode ref is larger than 1, + * If any of the mode ref is larger than 0, * that means it has been already opened by * others. Just skip checking the PIN ref. */ - if (i && ci->i_nr_by_mode[i] > 1) - is_opened = true; + if (i && ci->i_nr_by_mode[i]) + already_opened = true; + + if (bits & (1 << i)) + ci->i_nr_by_mode[i] += count; } - if (!is_opened) + if (!already_opened) percpu_counter_inc(&mdsc->metric.opened_inodes); spin_unlock(&ci->i_ceph_lock); }
opened_inodes is incremented twice when the same inode is opened twice with O_RDONLY and O_WRONLY respectively. To reproduce, run this python script, then check the metrics: import os for _ in range(10000): fd_r = os.open('a', os.O_RDONLY) fd_w = os.open('a', os.O_WRONLY) os.close(fd_r) os.close(fd_w) Fixes: 1dd8d4708136 ("ceph: metrics for opened files, pinned caps and opened inodes") Signed-off-by: Hu Weiwen <sehuww@mail.scut.edu.cn> --- fs/ceph/caps.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)