Looks good to me. Reviewed-by: Milind Changire <mchangir@xxxxxxxxxx> On Mon, Jul 31, 2023 at 5:29 PM Xiubo Li <xiubli@xxxxxxxxxx> wrote: > > > On 7/31/23 19:47, Milind Changire wrote: > > On Tue, Jul 25, 2023 at 9:36 AM <xiubli@xxxxxxxxxx> wrote: > >> From: Xiubo Li <xiubli@xxxxxxxxxx> > >> > >> Flushing the dirty buffer may take a long time if the Rados is > >> overloaded or if there is network issue. So we should ping the > >> MDSs periodically to keep alive, else the MDS will blocklist > >> the kclient. > >> > >> Cc: stable@xxxxxxxxxxxxxxx > >> Cc: Venky Shankar <vshankar@xxxxxxxxxx> > >> URL: https://tracker.ceph.com/issues/61843 > >> Reviewed-by: Milind Changire <mchangir@xxxxxxxxxx> > >> Signed-off-by: Xiubo Li <xiubli@xxxxxxxxxx> > >> --- > >> > >> V3: > >> - Rebased to the master branch > >> > >> > >> fs/ceph/mds_client.c | 4 ++-- > >> fs/ceph/mds_client.h | 5 +++++ > >> fs/ceph/super.c | 10 ++++++++++ > >> 3 files changed, 17 insertions(+), 2 deletions(-) > >> > >> diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c > >> index 66048a86c480..5fb367b1d4b0 100644 > >> --- a/fs/ceph/mds_client.c > >> +++ b/fs/ceph/mds_client.c > >> @@ -4764,7 +4764,7 @@ static void delayed_work(struct work_struct *work) > >> > >> dout("mdsc delayed_work\n"); > >> > >> - if (mdsc->stopping) > >> + if (mdsc->stopping >= CEPH_MDSC_STOPPING_FLUSHED) > >> return; > > Do we want to continue to accept/perform delayed work when > > mdsc->stopping is set to CEPH_MDSC_STOPPING_BEGIN ? > > > > I thought setting the STOPPING_BEGIN flag would immediately bar any > > further new activity and STOPPING_FLUSHED would mark safe deletion of > > the superblock. > > Yes, we need. > > Locally I can reproduce this very easy with fsstress.sh script, please > see https://tracker.ceph.com/issues/61843#note-1. > > That's because when umounting and flushing the dirty buffer it could be > blocked by the Rados dues to the lower disk space or MM reasons. And > during this we need to ping MDS to keep alive to make sure the MDS won't > evict us before we close the sessions later. > > Thanks > > - Xiubo > > > > > > >> mutex_lock(&mdsc->mutex); > >> @@ -4943,7 +4943,7 @@ void send_flush_mdlog(struct ceph_mds_session *s) > >> void ceph_mdsc_pre_umount(struct ceph_mds_client *mdsc) > >> { > >> dout("pre_umount\n"); > >> - mdsc->stopping = 1; > >> + mdsc->stopping = CEPH_MDSC_STOPPING_BEGIN; > >> > >> ceph_mdsc_iterate_sessions(mdsc, send_flush_mdlog, true); > >> ceph_mdsc_iterate_sessions(mdsc, lock_unlock_session, false); > >> diff --git a/fs/ceph/mds_client.h b/fs/ceph/mds_client.h > >> index 724307ff89cd..86d2965e68a1 100644 > >> --- a/fs/ceph/mds_client.h > >> +++ b/fs/ceph/mds_client.h > >> @@ -380,6 +380,11 @@ struct cap_wait { > >> int want; > >> }; > >> > >> +enum { > >> + CEPH_MDSC_STOPPING_BEGIN = 1, > >> + CEPH_MDSC_STOPPING_FLUSHED = 2, > >> +}; > >> + > >> /* > >> * mds client state > >> */ > >> diff --git a/fs/ceph/super.c b/fs/ceph/super.c > >> index 3fc48b43cab0..a5f52013314d 100644 > >> --- a/fs/ceph/super.c > >> +++ b/fs/ceph/super.c > >> @@ -1374,6 +1374,16 @@ static void ceph_kill_sb(struct super_block *s) > >> ceph_mdsc_pre_umount(fsc->mdsc); > >> flush_fs_workqueues(fsc); > >> > >> + /* > >> + * Though the kill_anon_super() will finally trigger the > >> + * sync_filesystem() anyway, we still need to do it here > >> + * and then bump the stage of shutdown to stop the work > >> + * queue as earlier as possible. > >> + */ > >> + sync_filesystem(s); > >> + > >> + fsc->mdsc->stopping = CEPH_MDSC_STOPPING_FLUSHED; > >> + > >> kill_anon_super(s); > >> > >> fsc->client->extra_mon_dispatch = NULL; > >> -- > >> 2.39.1 > >> > > > -- Milind