From: Xiubo Li xiubli@redhat.com
There is a race between capsnaps flush and removing the inode from 'mdsc->snap_flush_list' list:
== Thread A == == Thread B == ceph_queue_cap_snap() -> allocate 'capsnapA' ->ihold('&ci->vfs_inode') ->add 'capsnapA' to 'ci->i_cap_snaps' ->add 'ci' to 'mdsc->snap_flush_list' ... == Thread C == ceph_flush_snaps() ->__ceph_flush_snaps() ->__send_flush_snap() handle_cap_flushsnap_ack() ->iput('&ci->vfs_inode') this also will release 'ci' ... == Thread D == ceph_handle_snap() ->flush_snaps() ->iterate 'mdsc->snap_flush_list' ->get the stale 'ci' ->remove 'ci' from ->ihold(&ci->vfs_inode) this 'mdsc->snap_flush_list' will WARNING
To fix this we will increase the inode's i_count ref when adding 'ci' to the 'mdsc->snap_flush_list' list.
Cc: stable@vger.kernel.org URL: https://bugzilla.redhat.com/show_bug.cgi?id=2209299 Reviewed-by: Milind Changire mchangir@redhat.com Signed-off-by: Xiubo Li xiubli@redhat.com ---
V4: - s/put/need_put/
fs/ceph/caps.c | 6 ++++++ fs/ceph/snap.c | 4 +++- 2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c index 8c1fc64b290c..bb78d0fbfa62 100644 --- a/fs/ceph/caps.c +++ b/fs/ceph/caps.c @@ -1684,6 +1684,7 @@ void ceph_flush_snaps(struct ceph_inode_info *ci, struct inode *inode = &ci->netfs.inode; struct ceph_mds_client *mdsc = ceph_inode_to_client(inode)->mdsc; struct ceph_mds_session *session = NULL; + int need_put = 0; int mds;
dout("ceph_flush_snaps %p\n", inode); @@ -1728,8 +1729,13 @@ void ceph_flush_snaps(struct ceph_inode_info *ci, ceph_put_mds_session(session); /* we flushed them all; remove this inode from the queue */ spin_lock(&mdsc->snap_flush_lock); + if (!list_empty(&ci->i_snap_flush_item)) + need_put++; list_del_init(&ci->i_snap_flush_item); spin_unlock(&mdsc->snap_flush_lock); + + if (need_put) + iput(inode); }
/* diff --git a/fs/ceph/snap.c b/fs/ceph/snap.c index 7943981914cf..e03b020d87d7 100644 --- a/fs/ceph/snap.c +++ b/fs/ceph/snap.c @@ -697,8 +697,10 @@ int __ceph_finish_cap_snap(struct ceph_inode_info *ci, capsnap->size);
spin_lock(&mdsc->snap_flush_lock); - if (list_empty(&ci->i_snap_flush_item)) + if (list_empty(&ci->i_snap_flush_item)) { + ihold(inode); list_add_tail(&ci->i_snap_flush_item, &mdsc->snap_flush_list); + } spin_unlock(&mdsc->snap_flush_lock); return 1; /* caller may want to ceph_flush_snaps */ }
On Wed, Jun 7, 2023 at 4:57 AM xiubli@redhat.com wrote:
From: Xiubo Li xiubli@redhat.com
There is a race between capsnaps flush and removing the inode from 'mdsc->snap_flush_list' list:
== Thread A == == Thread B == ceph_queue_cap_snap() -> allocate 'capsnapA' ->ihold('&ci->vfs_inode') ->add 'capsnapA' to 'ci->i_cap_snaps' ->add 'ci' to 'mdsc->snap_flush_list' ... == Thread C == ceph_flush_snaps() ->__ceph_flush_snaps() ->__send_flush_snap() handle_cap_flushsnap_ack() ->iput('&ci->vfs_inode') this also will release 'ci' ... == Thread D == ceph_handle_snap() ->flush_snaps() ->iterate 'mdsc->snap_flush_list' ->get the stale 'ci' ->remove 'ci' from ->ihold(&ci->vfs_inode) this 'mdsc->snap_flush_list' will WARNING
To fix this we will increase the inode's i_count ref when adding 'ci' to the 'mdsc->snap_flush_list' list.
Cc: stable@vger.kernel.org URL: https://bugzilla.redhat.com/show_bug.cgi?id=2209299 Reviewed-by: Milind Changire mchangir@redhat.com Signed-off-by: Xiubo Li xiubli@redhat.com
V4:
- s/put/need_put/
Hi Xiubo,
The other part of the suggestion was to make it a bool. I made the adjustment and queued up this patch for 6.4-rc6.
Thanks,
Ilya
On 6/8/23 15:07, Ilya Dryomov wrote:
On Wed, Jun 7, 2023 at 4:57 AM xiubli@redhat.com wrote:
From: Xiubo Li xiubli@redhat.com
There is a race between capsnaps flush and removing the inode from 'mdsc->snap_flush_list' list:
== Thread A == == Thread B ==
ceph_queue_cap_snap() -> allocate 'capsnapA' ->ihold('&ci->vfs_inode') ->add 'capsnapA' to 'ci->i_cap_snaps' ->add 'ci' to 'mdsc->snap_flush_list' ... == Thread C == ceph_flush_snaps() ->__ceph_flush_snaps() ->__send_flush_snap() handle_cap_flushsnap_ack() ->iput('&ci->vfs_inode') this also will release 'ci' ... == Thread D == ceph_handle_snap() ->flush_snaps() ->iterate 'mdsc->snap_flush_list' ->get the stale 'ci' ->remove 'ci' from ->ihold(&ci->vfs_inode) this 'mdsc->snap_flush_list' will WARNING
To fix this we will increase the inode's i_count ref when adding 'ci' to the 'mdsc->snap_flush_list' list.
Cc: stable@vger.kernel.org URL: https://bugzilla.redhat.com/show_bug.cgi?id=2209299 Reviewed-by: Milind Changire mchangir@redhat.com Signed-off-by: Xiubo Li xiubli@redhat.com
V4:
- s/put/need_put/
Hi Xiubo,
The other part of the suggestion was to make it a bool. I made the adjustment and queued up this patch for 6.4-rc6.
Sure, thanks Ilya.
- Xiubo
Thanks,
Ilya
linux-stable-mirror@lists.linaro.org