[Cluster-devel] [GFS2 PATCH] GFS2: Use local iopen glock holder in gfs2_evict_inode

[Andreas Gruenbacher]

On Tue, Jun 14, 2016 at 6:53 PM, Bob Peterson <rpeterso at redhat.com> wrote:
> Hi,
>
> Before this patch, function gfs2_evict_inode unlocked the iopen
> glock (from SH), waited for completion, then locked it again in
> EXclusive mode. That's all well and good except that other processes
> (not in gfs2_evict_inode) can try to do lookups, and function
> gfs2_inode_lookup tries to lock the iopen glock in SH again. This
> second lookup can and does wipe out the holder's pid with getpid().
> The first putpid (from glock_holder_uninit) will be successful, but
> the second one will crash the kernel with:
> BUG: unable to handle kernel paging request
> This patch introduces a holder variable, io_gh, local to function
> gfs2_evict_inode, which will keep its own getpid() and subsequent
> putpid() from interfering with one another. So simultaneous inode
> lookups won't change the value out from under gfs2_evict_inode.
>
> Signed-off-by: Bob Peterson <rpeterso at redhat.com>
> ---
> diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c
> index 9b2ff353..6dc5f2ac 100644
> --- a/fs/gfs2/super.c
> +++ b/fs/gfs2/super.c
> @@ -1518,7 +1518,7 @@ static void gfs2_evict_inode(struct inode *inode)
>         struct super_block *sb = inode->i_sb;
>         struct gfs2_sbd *sdp = sb->s_fs_info;
>         struct gfs2_inode *ip = GFS2_I(inode);
> -       struct gfs2_holder gh;
> +       struct gfs2_holder gh, io_gh;
>         struct address_space *metamapping;
>         int error;
>
> @@ -1527,6 +1527,7 @@ static void gfs2_evict_inode(struct inode *inode)
>                 return;
>         }
>
> +       memset(&io_gh, 0, sizeof(io_gh));
>         if (inode->i_nlink || (sb->s_flags & MS_RDONLY))
>                 goto out;
>
> @@ -1555,9 +1556,9 @@ static void gfs2_evict_inode(struct inode *inode)
>             test_bit(HIF_HOLDER, &ip->i_iopen_gh.gh_iflags)) {
>                 ip->i_iopen_gh.gh_flags |= GL_NOCACHE;
>                 gfs2_glock_dq_wait(&ip->i_iopen_gh);
> -               gfs2_holder_reinit(LM_ST_EXCLUSIVE, LM_FLAG_TRY_1CB | GL_NOCACHE,
> -                                  &ip->i_iopen_gh);
> -               error = gfs2_glock_nq(&ip->i_iopen_gh);
> +               error = gfs2_glock_nq_init(ip->i_iopen_gh.gh_gl,
> +                                          LM_ST_EXCLUSIVE, LM_FLAG_TRY_1CB |
> +                                          GL_NOCACHE, &io_gh);
>                 if (error)
>                         goto out_truncate;
>         }
> @@ -1610,12 +1611,12 @@ out_unlock:
>         if (gfs2_rs_active(&ip->i_res))
>                 gfs2_rs_deltree(&ip->i_res);
>
> -       if (ip->i_iopen_gh.gh_gl) {
> -               if (test_bit(HIF_HOLDER, &ip->i_iopen_gh.gh_iflags)) {
> -                       ip->i_iopen_gh.gh_flags |= GL_NOCACHE;
> -                       gfs2_glock_dq_wait(&ip->i_iopen_gh);
> +       if (io_gh.gh_gl) {
> +               if (test_bit(HIF_HOLDER, &io_gh.gh_iflags)) {
> +                       io_gh.gh_flags |= GL_NOCACHE;
> +                       gfs2_glock_dq_wait(&io_gh);
>                 }
> -               gfs2_holder_uninit(&ip->i_iopen_gh);
> +               gfs2_holder_uninit(&io_gh);
>         }
>         gfs2_glock_dq_uninit(&gh);
>         if (error && error != GLR_TRYFAILED && error != -EROFS)
>

Acked-by: Andreas Gruenbacher <agruenba at redhat.com>