On Wed, Jul 27, 2022 at 6:32 PM Bob Peterson <rpete...@redhat.com> wrote:
> When a gfs2 file system is withdrawn, it requests recovery from another
> cluster node. To do that, it evicts its journal from memory, but it
> keeps the journal entry queued to the journals queue, jindex_list. After
> recovery it tries to grab a new inode for its (recovered) journal. If it
> cannot, it skips further recovery but its evicted journal is still on
> the jindex list, which means unmount will try to iput it a second time
> after it's been evicted. This second iput causes vfs to complain and BUG
> out:
>
> kernel BUG at fs/inode.c:1680!
>
> To prevent this, this patch takes steps to dequeue the journal
> descriptor from the list when it cannot get a replacement inode. So
> unmount won't find it on the list and try to iput it again.
>
> Signed-off-by: Bob Peterson <rpete...@redhat.com>
> ---
> fs/gfs2/util.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/fs/gfs2/util.c b/fs/gfs2/util.c
> index 8241029a2a5d..78cb12d0fba1 100644
> --- a/fs/gfs2/util.c
> +++ b/fs/gfs2/util.c
> @@ -275,6 +275,17 @@ static void signal_our_withdraw(struct gfs2_sbd *sdp)
> if (IS_ERR(inode)) {
> fs_warn(sdp, "Reprocessing of jid %d failed with %ld.\n",
> sdp->sd_lockstruct.ls_jid, PTR_ERR(inode));
> + /*
> + * We couldn't get a replacement inode for our journal but we
> + * evicted the old one. So dequeue it from the journals queue,
> + * jindex_list, so that unmount doesn't do iput on it twice.
> + */
> + spin_lock(&sdp->sd_jindex_spin);
> + list_del(&sdp->sd_jdesc->jd_list);
> + sdp->sd_journals--;
> + spin_unlock(&sdp->sd_jindex_spin);
> + kfree(sdp->sd_jdesc);
> + sdp->sd_jdesc = NULL;
Wouldn't it make more sense to set sdp->sd_jdesc->jd_inode to NULL
where we call iput() on that inode? An iput(NULL) is a no-op, so we'd
not need to change gfs2_jindex_free() to make that work.
Thanks,
Andreas
> goto skip_recovery;
> }
> sdp->sd_jdesc->jd_inode = inode;
> --
> 2.36.1
>
