Commit f2e70d8f authored by Bob Peterson's avatar Bob Peterson Committed by Andreas Gruenbacher
Browse files

gfs2: fix GL_SKIP node_scope problems 



Before this patch, when a glock was locked, the very first holder on the
queue would unlock the lockref and call the go_instantiate glops function
(if one existed), unless GL_SKIP was specified. When we introduced the new
node-scope concept, we allowed multiple holders to lock glocks in EX mode
and share the lock.

But node-scope introduced a new problem: if the first holder has GL_SKIP
and the next one does NOT, since it is not the first holder on the queue,
the go_instantiate op was not called. Eventually the GL_SKIP holder may
call the instantiate sub-function (e.g. gfs2_rgrp_bh_get) but there was
still a window of time in which another non-GL_SKIP holder assumes the
instantiate function had been called by the first holder. In the case of
rgrp glocks, this led to a NULL pointer dereference on the buffer_heads.

This patch tries to fix the problem by introducing two new glock flags:

GLF_INSTANTIATE_NEEDED, which keeps track of when the instantiate function
needs to be called to "fill in" or "read in" the object before it is
referenced.

GLF_INSTANTIATE_IN_PROG which is used to determine when a process is
in the process of reading in the object. Whenever a function needs to
reference the object, it checks the GLF_INSTANTIATE_NEEDED flag, and if
set, it sets GLF_INSTANTIATE_IN_PROG and calls the glops "go_instantiate"
function.

As before, the gl_lockref spin_lock is unlocked during the IO operation,
which may take a relatively long amount of time to complete. While
unlocked, if another process determines go_instantiate is still needed,
it sees GLF_INSTANTIATE_IN_PROG is set, and waits for the go_instantiate
glop operation to be completed. Once GLF_INSTANTIATE_IN_PROG is cleared,
it needs to check GLF_INSTANTIATE_NEEDED again because the other process's
go_instantiate operation may not have been successful.

Functions that previously called the instantiate sub-functions now call
directly into gfs2_instantiate so the new bits are managed properly.

Signed-off-by: default avatarBob Peterson <rpeterso@redhat.com>
Signed-off-by: default avatarAndreas Gruenbacher <agruenba@redhat.com>
parent e6f85600

fs/gfs2/glock.c

+38 −4
Original line number Diff line number Diff line
@@ -479,12 +479,42 @@ find_first_strong_holder(struct gfs2_glock *gl) 
 * Returns: 0 if instantiate was successful, 2 if type specific operation is

 * underway, or error.

 */

static int gfs2_instantiate(struct gfs2_holder *gh)

int gfs2_instantiate(struct gfs2_holder *gh)

{

	struct gfs2_glock *gl = gh->gh_gl;

	const struct gfs2_glock_operations *glops = gl->gl_ops;

	int ret;



	return glops->go_instantiate(gh);

again:

	if (!test_bit(GLF_INSTANTIATE_NEEDED, &gl->gl_flags))

		return 0;



	/*

	 * Since we unlock the lockref lock, we set a flag to indicate

	 * instantiate is in progress.

	 */

	if (test_bit(GLF_INSTANTIATE_IN_PROG, &gl->gl_flags)) {

		wait_on_bit(&gl->gl_flags, GLF_INSTANTIATE_IN_PROG,

			    TASK_UNINTERRUPTIBLE);

		/*

		 * Here we just waited for a different instantiate to finish.

		 * But that may not have been successful, as when a process

		 * locks an inode glock _before_ it has an actual inode to

		 * instantiate into. So we check again. This process might

		 * have an inode to instantiate, so might be successful.

		 */

		goto again;

	}



	set_bit(GLF_INSTANTIATE_IN_PROG, &gl->gl_flags);



	ret = glops->go_instantiate(gh);

	if (!ret)

		clear_bit(GLF_INSTANTIATE_NEEDED, &gl->gl_flags);

	clear_bit(GLF_INSTANTIATE_IN_PROG, &gl->gl_flags);

	smp_mb__after_atomic();

	wake_up_bit(&gl->gl_flags, GLF_INSTANTIATE_IN_PROG);

	return ret;

}



/**
@@ -526,7 +556,7 @@ __acquires(&gl->gl_lockref.lock) 
			incompat_holders_demoted = true;

			first_gh = gh;

		}

		if (gh->gh_list.prev == &gl->gl_holders &&

		if (test_bit(GLF_INSTANTIATE_NEEDED, &gl->gl_flags) &&

		    !(gh->gh_flags & GL_SKIP) && gl->gl_ops->go_instantiate) {

			lock_released = true;

			spin_unlock(&gl->gl_lockref.lock);
@@ -1162,7 +1192,7 @@ int gfs2_glock_get(struct gfs2_sbd *sdp, u64 number, 


	atomic_inc(&sdp->sd_glock_disposal);

	gl->gl_node.next = NULL;

	gl->gl_flags = 0;

	gl->gl_flags = glops->go_instantiate ? BIT(GLF_INSTANTIATE_NEEDED) : 0;

	gl->gl_name = name;

	lockdep_set_subclass(&gl->gl_lockref.lock, glops->go_subclass);

	gl->gl_lockref.count = 1;
@@ -2326,6 +2356,10 @@ static const char *gflags2str(char *buf, const struct gfs2_glock *gl) 
		*p++ = 'P';

	if (test_bit(GLF_FREEING, gflags))

		*p++ = 'x';

	if (test_bit(GLF_INSTANTIATE_NEEDED, gflags))

		*p++ = 'n';

	if (test_bit(GLF_INSTANTIATE_IN_PROG, gflags))

		*p++ = 'N';

	*p = 0;

	return buf;

}

fs/gfs2/glock.h

+1 −0
Original line number Diff line number Diff line
@@ -204,6 +204,7 @@ extern void gfs2_holder_reinit(unsigned int state, u16 flags, 
extern void gfs2_holder_uninit(struct gfs2_holder *gh);

extern int gfs2_glock_nq(struct gfs2_holder *gh);

extern int gfs2_glock_poll(struct gfs2_holder *gh);

extern int gfs2_instantiate(struct gfs2_holder *gh);

extern int gfs2_glock_wait(struct gfs2_holder *gh);

extern int gfs2_glock_async_wait(unsigned int num_gh, struct gfs2_holder *ghs);

extern void gfs2_glock_dq(struct gfs2_holder *gh);

fs/gfs2/glops.c

+6 −4
Original line number Diff line number Diff line
@@ -357,6 +357,7 @@ static void inode_go_inval(struct gfs2_glock *gl, int flags) 
		truncate_inode_pages(mapping, 0);

		if (ip) {

			set_bit(GIF_INVALID, &ip->i_flags);

			set_bit(GLF_INSTANTIATE_NEEDED, &gl->gl_flags);

			forget_all_cached_acls(&ip->i_inode);

			security_inode_invalidate_secctx(&ip->i_inode);

			gfs2_dir_hash_inval(ip);
@@ -495,13 +496,13 @@ static int inode_go_instantiate(struct gfs2_holder *gh) 
	struct gfs2_inode *ip = gl->gl_object;

	int error = 0;



	if (!ip)

		return 0;

	if (!ip) /* no inode to populate - read it in later */

		goto out;



	if (test_bit(GIF_INVALID, &ip->i_flags)) {

		error = gfs2_inode_refresh(ip);

		if (error)

			return error;

			goto out;

	}



	if (gh->gh_state != LM_ST_DEFERRED)
@@ -515,9 +516,10 @@ static int inode_go_instantiate(struct gfs2_holder *gh) 
			list_add(&ip->i_trunc_list, &sdp->sd_trunc_list);

		spin_unlock(&sdp->sd_trunc_lock);

		wake_up(&sdp->sd_quota_wait);

		return 1;

		error = 1;

	}



out:

	return error;

}

fs/gfs2/incore.h

+2 −0
Original line number Diff line number Diff line
@@ -316,6 +316,7 @@ struct gfs2_alloc_parms { 


enum {

	GLF_LOCK			= 1,

	GLF_INSTANTIATE_NEEDED		= 2, /* needs instantiate */

	GLF_DEMOTE			= 3,

	GLF_PENDING_DEMOTE		= 4,

	GLF_DEMOTE_IN_PROGRESS		= 5,
@@ -325,6 +326,7 @@ enum { 
	GLF_REPLY_PENDING		= 9,

	GLF_INITIAL			= 10,

	GLF_FROZEN			= 11,

	GLF_INSTANTIATE_IN_PROG		= 12, /* instantiate happening now */

	GLF_LRU				= 13,

	GLF_OBJECT			= 14, /* Used only for tracing */

	GLF_BLOCKING			= 15,

fs/gfs2/inode.c

+2 −1
Original line number Diff line number Diff line
@@ -183,6 +183,7 @@ struct inode *gfs2_inode_lookup(struct super_block *sb, unsigned int type, 


		glock_set_object(ip->i_gl, ip);

		set_bit(GIF_INVALID, &ip->i_flags);

		set_bit(GLF_INSTANTIATE_NEEDED, &ip->i_gl->gl_flags);

		error = gfs2_glock_nq_init(io_gl, LM_ST_SHARED, GL_EXACT, &ip->i_iopen_gh);

		if (unlikely(error))

			goto fail;
@@ -196,7 +197,7 @@ struct inode *gfs2_inode_lookup(struct super_block *sb, unsigned int type, 


		if (type == DT_UNKNOWN) {

			/* Inode glock must be locked already */

			error = gfs2_inode_refresh(GFS2_I(inode));

			error = gfs2_instantiate(&i_gh);

			if (error)

				goto fail;

		} else {