Commit 6ab26555 authored by Andreas Gruenbacher's avatar Andreas Gruenbacher
Browse files

gfs2: Add proper lockspace locking 



GFS2 has been calling functions like dlm_lock() even after the lockspace
that these functions operate on has been released with
dlm_release_lockspace().  It has always assumed that those functions
would return -EINVAL in that case, but that was never guaranteed, and it
certainly is no longer the case since commit 4db41bf4 ("dlm: remove
ls_local_handle from struct dlm_ls").

To fix that, add proper lockspace locking.

Fixes: 3e11e530 ("GFS2: ignore unlock failures after withdraw")
Signed-off-by: default avatarAndreas Gruenbacher <agruenba@redhat.com>
Reviewed-by: default avatarAndrew Price <anprice@redhat.com>
parent 47faf937

fs/gfs2/file.c

+15 −8
Original line number Diff line number Diff line
@@ -1442,6 +1442,7 @@ static int gfs2_lock(struct file *file, int cmd, struct file_lock *fl) 
	struct gfs2_inode *ip = GFS2_I(file->f_mapping->host);

	struct gfs2_sbd *sdp = GFS2_SB(file->f_mapping->host);

	struct lm_lockstruct *ls = &sdp->sd_lockstruct;

	int ret;



	if (!(fl->c.flc_flags & FL_POSIX))

		return -ENOLCK;
@@ -1450,14 +1451,20 @@ static int gfs2_lock(struct file *file, int cmd, struct file_lock *fl) 
			locks_lock_file_wait(file, fl);

		return -EIO;

	}

	down_read(&ls->ls_sem);

	ret = -ENODEV;

	if (likely(ls->ls_dlm != NULL)) {

		if (cmd == F_CANCELLK)

		return dlm_posix_cancel(ls->ls_dlm, ip->i_no_addr, file, fl);

			ret = dlm_posix_cancel(ls->ls_dlm, ip->i_no_addr, file, fl);

		else if (IS_GETLK(cmd))

		return dlm_posix_get(ls->ls_dlm, ip->i_no_addr, file, fl);

			ret = dlm_posix_get(ls->ls_dlm, ip->i_no_addr, file, fl);

		else if (lock_is_unlock(fl))

		return dlm_posix_unlock(ls->ls_dlm, ip->i_no_addr, file, fl);

			ret = dlm_posix_unlock(ls->ls_dlm, ip->i_no_addr, file, fl);

		else

		return dlm_posix_lock(ls->ls_dlm, ip->i_no_addr, file, cmd, fl);

			ret = dlm_posix_lock(ls->ls_dlm, ip->i_no_addr, file, cmd, fl);

	}

	up_read(&ls->ls_sem);

	return ret;

}



static void __flock_holder_uninit(struct file *file, struct gfs2_holder *fl_gh)

fs/gfs2/glock.c

+2 −3
Original line number Diff line number Diff line
@@ -795,9 +795,8 @@ __acquires(&gl->gl_lockref.lock) 
		}

		clear_bit(GLF_PENDING_REPLY, &gl->gl_flags);



		if (ret == -EINVAL && gl->gl_target == LM_ST_UNLOCKED &&

		    target == LM_ST_UNLOCKED &&

		    test_bit(DFL_UNMOUNT, &ls->ls_recover_flags)) {

		if (ret == -ENODEV && gl->gl_target == LM_ST_UNLOCKED &&

		    target == LM_ST_UNLOCKED) {

			/*

			 * The lockspace has been released and the lock has

			 * been unlocked implicitly.

fs/gfs2/incore.h

+2 −0
Original line number Diff line number Diff line
@@ -656,6 +656,8 @@ struct lm_lockstruct { 
	struct completion ls_sync_wait; /* {control,mounted}_{lock,unlock} */

	char *ls_lvb_bits;



	struct rw_semaphore ls_sem;



	spinlock_t ls_recover_spin; /* protects following fields */

	unsigned long ls_recover_flags; /* DFL_ */

	uint32_t ls_recover_mount; /* gen in first recover_done cb */

fs/gfs2/lock_dlm.c

+37 −9
Original line number Diff line number Diff line
@@ -312,8 +312,13 @@ static int gdlm_lock(struct gfs2_glock *gl, unsigned int req_state, 
	 */



again:

	down_read(&ls->ls_sem);

	error = -ENODEV;

	if (likely(ls->ls_dlm != NULL)) {

		error = dlm_lock(ls->ls_dlm, req, &gl->gl_lksb, lkf, strname,

				GDLM_STRNAME_BYTES - 1, 0, gdlm_ast, gl, gdlm_bast);

	}

	up_read(&ls->ls_sem);

	if (error == -EBUSY) {

		msleep(20);

		goto again;
@@ -362,8 +367,13 @@ static void gdlm_put_lock(struct gfs2_glock *gl) 
		flags |= DLM_LKF_VALBLK;



again:

	down_read(&ls->ls_sem);

	error = -ENODEV;

	if (likely(ls->ls_dlm != NULL)) {

		error = dlm_unlock(ls->ls_dlm, gl->gl_lksb.sb_lkid, flags,

				   NULL, gl);

	}

	up_read(&ls->ls_sem);

	if (error == -EBUSY) {

		msleep(20);

		goto again;
@@ -379,8 +389,13 @@ static void gdlm_put_lock(struct gfs2_glock *gl) 
static void gdlm_cancel(struct gfs2_glock *gl)

{

	struct lm_lockstruct *ls = &gl->gl_name.ln_sbd->sd_lockstruct;



	down_read(&ls->ls_sem);

	if (likely(ls->ls_dlm != NULL)) {

		dlm_unlock(ls->ls_dlm, gl->gl_lksb.sb_lkid, DLM_LKF_CANCEL, NULL, gl);

	}

	up_read(&ls->ls_sem);

}



/*

 * dlm/gfs2 recovery coordination using dlm_recover callbacks
@@ -560,7 +575,11 @@ static int sync_unlock(struct gfs2_sbd *sdp, struct dlm_lksb *lksb, char *name) 
	struct lm_lockstruct *ls = &sdp->sd_lockstruct;

	int error;



	down_read(&ls->ls_sem);

	error = -ENODEV;

	if (likely(ls->ls_dlm != NULL))

		error = dlm_unlock(ls->ls_dlm, lksb->sb_lkid, 0, lksb, ls);

	up_read(&ls->ls_sem);

	if (error) {

		fs_err(sdp, "%s lkid %x error %d\n",

		       name, lksb->sb_lkid, error);
@@ -587,9 +606,14 @@ static int sync_lock(struct gfs2_sbd *sdp, int mode, uint32_t flags, 
	memset(strname, 0, GDLM_STRNAME_BYTES);

	snprintf(strname, GDLM_STRNAME_BYTES, "%8x%16x", LM_TYPE_NONDISK, num);



	down_read(&ls->ls_sem);

	error = -ENODEV;

	if (likely(ls->ls_dlm != NULL)) {

		error = dlm_lock(ls->ls_dlm, mode, lksb, flags,

				 strname, GDLM_STRNAME_BYTES - 1,

				 0, sync_wait_cb, ls, NULL);

	}

	up_read(&ls->ls_sem);

	if (error) {

		fs_err(sdp, "%s lkid %x flags %x mode %d error %d\n",

		       name, lksb->sb_lkid, flags, mode, error);
@@ -1316,6 +1340,7 @@ static int gdlm_mount(struct gfs2_sbd *sdp, const char *table) 
	 */



	INIT_DELAYED_WORK(&sdp->sd_control_work, gfs2_control_func);

	ls->ls_dlm = NULL;

	spin_lock_init(&ls->ls_recover_spin);

	ls->ls_recover_flags = 0;

	ls->ls_recover_mount = 0;
@@ -1350,6 +1375,7 @@ static int gdlm_mount(struct gfs2_sbd *sdp, const char *table) 
	 * create/join lockspace

	 */



	init_rwsem(&ls->ls_sem);

	error = dlm_new_lockspace(fsname, cluster, flags, GDLM_LVB_SIZE,

				  &gdlm_lockspace_ops, sdp, &ops_result,

				  &ls->ls_dlm);
@@ -1429,10 +1455,12 @@ static void gdlm_unmount(struct gfs2_sbd *sdp) 


	/* mounted_lock and control_lock will be purged in dlm recovery */

release:

	down_write(&ls->ls_sem);

	if (ls->ls_dlm) {

		dlm_release_lockspace(ls->ls_dlm, 2);

		ls->ls_dlm = NULL;

	}

	up_write(&ls->ls_sem);



	free_recover_size(ls);

}