Ping?
On 2023/01/04 22:47, Tetsuo Handa wrote:
> I suspect that cleanup is not done appropriately when gfs2_find_jhead()
> failed.
> Looking into gfs2_make_fs_rw(), I see there are two worrisome things.
>
> One is that gfs2_make_fs_rw() returns an error without calling
> gfs2_consist(sdp) when
> gfs2_find_jhead() returned an error whereas gfs2_make_fs_rw() returns -EIO
> after calling
> gfs2_consist(sdp) when head.lh_flags does not contain GFS2_LOG_HEAD_UNMOUNT
> flag.
>
> Since head.lh_flags is assigned by gfs2_find_jhead(), we might want to call
> gfs2_consist(sdp)
> when gfs2_find_jhead() returned an error. And actually
>
> --- a/fs/gfs2/super.c
> +++ b/fs/gfs2/super.c
> @@ -138,7 +138,11 @@ int gfs2_make_fs_rw(struct gfs2_sbd *sdp)
> return -EIO;
>
> error = gfs2_find_jhead(sdp->sd_jdesc, &head, false);
> - if (error || gfs2_withdrawn(sdp))
> + if (error) {
> + gfs2_consist(sdp);
> + return error;
> + }
> + if (gfs2_withdrawn(sdp))
> return error;
>
> if (!(head.lh_flags & GFS2_LOG_HEAD_UNMOUNT)) {
>
> avoids this deadlock. But I don't know when/how to use gfs2_withdraw().
> Please check if this change is appropriate.
>
> The other worrisome thing is that gfs2_make_fs_rw() is returning 0 (and
> mount operation will continue) when gfs2_withdrawn() == true. Can the caller
> of gfs2_make_fs_rw() survive when callgfs2_make_fs_rw() returned 0 without
> processing
>
> /* Initialize some head of the log stuff */
> sdp->sd_log_sequence = head.lh_sequence + 1;
> gfs2_log_pointers_init(sdp, head.lh_blkno);
>
> lines? Shouldn't the caller of gfs2_make_fs_rw() observe an error when
> gfs2_make_fs_rw() did not execute the
>
> set_bit(SDF_JOURNAL_LIVE, &sdp->sd_flags);
>
> line due to gfs2_withdrawn() == true?
>
