| Message ID | 20210825052212.19625-1-xiubli@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ceph: init the i_list/g_list for cap flush | expand |
On Wed, 2021-08-25 at 13:22 +0800, xiubli@redhat.com wrote: > From: Xiubo Li <xiubli@redhat.com> > > Always init the i_list/g_list in the begining to make sure it won't > crash the kernel if someone want to delete the cap_flush from the > lists. > > Cc: stable@vger.kernel.org > URL: https://tracker.ceph.com/issues/52401 > Signed-off-by: Xiubo Li <xiubli@redhat.com> > --- > fs/ceph/caps.c | 2 +- > fs/ceph/snap.c | 2 ++ > 2 files changed, 3 insertions(+), 1 deletion(-) > > diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c > index 4f0dbc640b0b..60f60260cf42 100644 > --- a/fs/ceph/caps.c > +++ b/fs/ceph/caps.c > @@ -3666,7 +3666,7 @@ static void handle_cap_flush_ack(struct inode *inode, u64 flush_tid, > while (!list_empty(&to_remove)) { > cf = list_first_entry(&to_remove, > struct ceph_cap_flush, i_list); > - list_del(&cf->i_list); > + list_del_init(&cf->i_list); > if (!cf->is_capsnap) > ceph_free_cap_flush(cf); > } > diff --git a/fs/ceph/snap.c b/fs/ceph/snap.c > index 62fab59bbf96..b41e6724c591 100644 > --- a/fs/ceph/snap.c > +++ b/fs/ceph/snap.c > @@ -488,6 +488,8 @@ static void ceph_queue_cap_snap(struct ceph_inode_info *ci) > return; > } > capsnap->cap_flush.is_capsnap = true; > + INIT_LIST_HEAD(&capsnap->cap_flush.i_list); > + INIT_LIST_HEAD(&capsnap->cap_flush.g_list); > > spin_lock(&ci->i_ceph_lock); > used = __ceph_caps_used(ci); I'm not certain the second hunk is strictly needed. These either end up on the list or they just get freed. That said, they shouldn't hurt anything and it is more consistent. Merged into testing. Ilya, since this is marked for stable, this probably ought to go to Linus in the last v5.14 pile. Thanks,
On Wed, Aug 25, 2021 at 12:50 PM Jeff Layton <jlayton@kernel.org> wrote: > > On Wed, 2021-08-25 at 13:22 +0800, xiubli@redhat.com wrote: > > From: Xiubo Li <xiubli@redhat.com> > > > > Always init the i_list/g_list in the begining to make sure it won't > > crash the kernel if someone want to delete the cap_flush from the > > lists. > > > > Cc: stable@vger.kernel.org > > URL: https://tracker.ceph.com/issues/52401 > > Signed-off-by: Xiubo Li <xiubli@redhat.com> > > --- > > fs/ceph/caps.c | 2 +- > > fs/ceph/snap.c | 2 ++ > > 2 files changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c > > index 4f0dbc640b0b..60f60260cf42 100644 > > --- a/fs/ceph/caps.c > > +++ b/fs/ceph/caps.c > > @@ -3666,7 +3666,7 @@ static void handle_cap_flush_ack(struct inode *inode, u64 flush_tid, > > while (!list_empty(&to_remove)) { > > cf = list_first_entry(&to_remove, > > struct ceph_cap_flush, i_list); > > - list_del(&cf->i_list); > > + list_del_init(&cf->i_list); > > if (!cf->is_capsnap) > > ceph_free_cap_flush(cf); > > } > > diff --git a/fs/ceph/snap.c b/fs/ceph/snap.c > > index 62fab59bbf96..b41e6724c591 100644 > > --- a/fs/ceph/snap.c > > +++ b/fs/ceph/snap.c > > @@ -488,6 +488,8 @@ static void ceph_queue_cap_snap(struct ceph_inode_info *ci) > > return; > > } > > capsnap->cap_flush.is_capsnap = true; > > + INIT_LIST_HEAD(&capsnap->cap_flush.i_list); > > + INIT_LIST_HEAD(&capsnap->cap_flush.g_list); > > > > spin_lock(&ci->i_ceph_lock); > > used = __ceph_caps_used(ci); > > I'm not certain the second hunk is strictly needed. These either end up > on the list or they just get freed. That said, they shouldn't hurt > anything and it is more consistent. Merged into testing. > > Ilya, since this is marked for stable, this probably ought to go to > Linus in the last v5.14 pile. I'm inclined to fold this into "ceph: correctly handle releasing an embedded cap flush" which is already queued for 5.14. Thanks, Ilya
On 8/25/21 10:31 PM, Ilya Dryomov wrote: > On Wed, Aug 25, 2021 at 12:50 PM Jeff Layton <jlayton@kernel.org> wrote: >> On Wed, 2021-08-25 at 13:22 +0800, xiubli@redhat.com wrote: >>> From: Xiubo Li <xiubli@redhat.com> >>> >>> Always init the i_list/g_list in the begining to make sure it won't >>> crash the kernel if someone want to delete the cap_flush from the >>> lists. >>> >>> Cc: stable@vger.kernel.org >>> URL: https://tracker.ceph.com/issues/52401 >>> Signed-off-by: Xiubo Li <xiubli@redhat.com> >>> --- >>> fs/ceph/caps.c | 2 +- >>> fs/ceph/snap.c | 2 ++ >>> 2 files changed, 3 insertions(+), 1 deletion(-) >>> >>> diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c >>> index 4f0dbc640b0b..60f60260cf42 100644 >>> --- a/fs/ceph/caps.c >>> +++ b/fs/ceph/caps.c >>> @@ -3666,7 +3666,7 @@ static void handle_cap_flush_ack(struct inode *inode, u64 flush_tid, >>> while (!list_empty(&to_remove)) { >>> cf = list_first_entry(&to_remove, >>> struct ceph_cap_flush, i_list); >>> - list_del(&cf->i_list); >>> + list_del_init(&cf->i_list); >>> if (!cf->is_capsnap) >>> ceph_free_cap_flush(cf); >>> } >>> diff --git a/fs/ceph/snap.c b/fs/ceph/snap.c >>> index 62fab59bbf96..b41e6724c591 100644 >>> --- a/fs/ceph/snap.c >>> +++ b/fs/ceph/snap.c >>> @@ -488,6 +488,8 @@ static void ceph_queue_cap_snap(struct ceph_inode_info *ci) >>> return; >>> } >>> capsnap->cap_flush.is_capsnap = true; >>> + INIT_LIST_HEAD(&capsnap->cap_flush.i_list); >>> + INIT_LIST_HEAD(&capsnap->cap_flush.g_list); >>> >>> spin_lock(&ci->i_ceph_lock); >>> used = __ceph_caps_used(ci); >> I'm not certain the second hunk is strictly needed. These either end up >> on the list or they just get freed. That said, they shouldn't hurt >> anything and it is more consistent. Merged into testing. >> >> Ilya, since this is marked for stable, this probably ought to go to >> Linus in the last v5.14 pile. > I'm inclined to fold this into "ceph: correctly handle releasing an > embedded cap flush" which is already queued for 5.14. Yeah, that's will be better. > > Thanks, > > Ilya >
diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c index 4f0dbc640b0b..60f60260cf42 100644 --- a/fs/ceph/caps.c +++ b/fs/ceph/caps.c @@ -3666,7 +3666,7 @@ static void handle_cap_flush_ack(struct inode *inode, u64 flush_tid, while (!list_empty(&to_remove)) { cf = list_first_entry(&to_remove, struct ceph_cap_flush, i_list); - list_del(&cf->i_list); + list_del_init(&cf->i_list); if (!cf->is_capsnap) ceph_free_cap_flush(cf); } diff --git a/fs/ceph/snap.c b/fs/ceph/snap.c index 62fab59bbf96..b41e6724c591 100644 --- a/fs/ceph/snap.c +++ b/fs/ceph/snap.c @@ -488,6 +488,8 @@ static void ceph_queue_cap_snap(struct ceph_inode_info *ci) return; } capsnap->cap_flush.is_capsnap = true; + INIT_LIST_HEAD(&capsnap->cap_flush.i_list); + INIT_LIST_HEAD(&capsnap->cap_flush.g_list); spin_lock(&ci->i_ceph_lock); used = __ceph_caps_used(ci);