From mboxrd@z Thu Jan 1 00:00:00 1970 From: Bob Peterson Date: Tue, 9 Jun 2020 10:14:53 -0400 (EDT) Subject: [Cluster-devel] [GFS2 PATCH] gfs2: fix trans slab error when withdraw occurs inside log_flush In-Reply-To: References: <181861383.32913711.1591710911880.JavaMail.zimbra@redhat.com> Message-ID: <439170501.32917657.1591712093863.JavaMail.zimbra@redhat.com> List-Id: To: cluster-devel.redhat.com MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit ----- Original Message ----- > I'm not sure quite what the aim is here... are you sure that it is ok to > move something to the AIL list if there has been a withdrawal? If the > log flush has not completed correctly then we should not be moving > anything to the AIL lists I think, > > Steve. Yes, I'm sure it's okay in this case. We only add it temporarily to the ail1 list so that function ail_drain() finds and removes it like the rest. I only coded it this way because Andreas didn't like my somewhat longer previous implementation, which follows. Regards, Bob diff --git a/fs/gfs2/log.c b/fs/gfs2/log.c index 3e4734431783..0c45851b88d5 100644 --- a/fs/gfs2/log.c +++ b/fs/gfs2/log.c @@ -845,8 +845,15 @@ static void log_write_header(struct gfs2_sbd *sdp, u32 flags) /** * ail_drain - drain the ail lists after a withdraw * @sdp: Pointer to GFS2 superblock + * @flush_tr: If non-null, transaction that was being flushed + * + * We're draining the ail lists after a withdraw, during a log flush op. + * If the log flush was targeting a specific transaction, it will never + * go through "after" lops processing, so we need to free it here. + * If we find it on one of the ail lists, we free it as we dequeue it. + * If we don't find it, we free it at the end. */ -static void ail_drain(struct gfs2_sbd *sdp) +static void ail_drain(struct gfs2_sbd *sdp, struct gfs2_trans *flush_tr) { struct gfs2_trans *tr; @@ -865,6 +872,8 @@ static void ail_drain(struct gfs2_sbd *sdp) gfs2_ail_empty_tr(sdp, tr, &tr->tr_ail2_list); list_del(&tr->tr_list); gfs2_trans_free(sdp, tr); + if (tr == flush_tr) + flush_tr = NULL; } while (!list_empty(&sdp->sd_ail2_list)) { tr = list_first_entry(&sdp->sd_ail2_list, struct gfs2_trans, @@ -872,7 +881,11 @@ static void ail_drain(struct gfs2_sbd *sdp) gfs2_ail_empty_tr(sdp, tr, &tr->tr_ail2_list); list_del(&tr->tr_list); gfs2_trans_free(sdp, tr); + if (tr == flush_tr) + flush_tr = NULL; } + if (flush_tr) + gfs2_trans_free(sdp, flush_tr); spin_unlock(&sdp->sd_ail_lock); } @@ -1002,7 +1015,7 @@ void gfs2_log_flush(struct gfs2_sbd *sdp, struct gfs2_glock *gl, u32 flags) out: if (gfs2_withdrawn(sdp)) { - ail_drain(sdp); /* frees all transactions */ + ail_drain(sdp, tr); /* frees all transactions */ tr = NULL; }