Commit 24c19ef4 authored by Mark Fasheh's avatar Mark Fasheh
Browse files

ocfs2: Remove i_generation from inode lock names 



OCFS2 puts inode meta data in the "lock value block" provided by the DLM.
Typically, i_generation is encoded in the lock name so that a deleted inode
on and a new one in the same block don't share the same lvb.

Unfortunately, that scheme means that the read in ocfs2_read_locked_inode()
is potentially thrown away as soon as the meta data lock is taken - we
cannot encode the lock name without first knowing i_generation, which
requires a disk read.

This patch encodes i_generation in the inode meta data lvb, and removes the
value from the inode meta data lock name. This way, the read can be covered
by a lock, and at the same time we can distinguish between an up to date and
a stale LVB.

This will help cold-cache stat(2) performance in particular.

Since this patch changes the protocol version, we take the opportunity to do
a minor re-organization of two of the LVB fields.

Signed-off-by: default avatarMark Fasheh <mark.fasheh@oracle.com>
parent f9e2d82e

fs/ocfs2/cluster/tcp_internal.h

+4 −1
Original line number Diff line number Diff line
@@ -44,6 +44,9 @@ 
 * locking semantics of the file system using the protocol.  It should 

 * be somewhere else, I'm sure, but right now it isn't.

 *

 * New in version 4:

 * 	- Remove i_generation from lock names for better stat performance.

 *

 * New in version 3:

 * 	- Replace dentry votes with a cluster lock

 *
@@ -51,7 +54,7 @@ 
 * 	- full 64 bit i_size in the metadata lock lvbs

 * 	- introduction of "rw" lock and pushing meta/data locking down

 */

#define O2NET_PROTOCOL_VERSION 3ULL

#define O2NET_PROTOCOL_VERSION 4ULL

struct o2net_handshake {

	__be64	protocol_version;

	__be64	connector_id;

fs/ocfs2/dlmglue.c

+36 −6
Original line number Diff line number Diff line
@@ -320,6 +320,7 @@ void ocfs2_lock_res_init_once(struct ocfs2_lock_res *res) 


void ocfs2_inode_lock_res_init(struct ocfs2_lock_res *res,

			       enum ocfs2_lock_type type,

			       unsigned int generation,

			       struct inode *inode)

{

	struct ocfs2_lock_res_ops *ops;
@@ -341,7 +342,7 @@ void ocfs2_inode_lock_res_init(struct ocfs2_lock_res *res, 
	};



	ocfs2_build_lock_name(type, OCFS2_I(inode)->ip_blkno,

			      inode->i_generation, res->l_name);

			      generation, res->l_name);

	ocfs2_lock_res_init_common(OCFS2_SB(inode->i_sb), res, type, ops, inode);

}
@@ -1173,17 +1174,19 @@ static void ocfs2_cluster_unlock(struct ocfs2_super *osb, 


int ocfs2_create_new_lock(struct ocfs2_super *osb,

			  struct ocfs2_lock_res *lockres,

			  int ex)

			  int ex,

			  int local)

{

	int level =  ex ? LKM_EXMODE : LKM_PRMODE;

	unsigned long flags;

	int lkm_flags = local ? LKM_LOCAL : 0;



	spin_lock_irqsave(&lockres->l_lock, flags);

	BUG_ON(lockres->l_flags & OCFS2_LOCK_ATTACHED);

	lockres_or_flags(lockres, OCFS2_LOCK_LOCAL);

	spin_unlock_irqrestore(&lockres->l_lock, flags);



	return ocfs2_lock_create(osb, lockres, level, LKM_LOCAL);

	return ocfs2_lock_create(osb, lockres, level, lkm_flags);

}



/* Grants us an EX lock on the data and metadata resources, skipping
@@ -1212,19 +1215,23 @@ int ocfs2_create_new_inode_locks(struct inode *inode) 
	 * on a resource which has an invalid one -- we'll set it

	 * valid when we release the EX. */



	ret = ocfs2_create_new_lock(osb, &OCFS2_I(inode)->ip_rw_lockres, 1);

	ret = ocfs2_create_new_lock(osb, &OCFS2_I(inode)->ip_rw_lockres, 1, 1);

	if (ret) {

		mlog_errno(ret);

		goto bail;

	}



	ret = ocfs2_create_new_lock(osb, &OCFS2_I(inode)->ip_meta_lockres, 1);

	/*

	 * We don't want to use LKM_LOCAL on a meta data lock as they

	 * don't use a generation in their lock names.

	 */

	ret = ocfs2_create_new_lock(osb, &OCFS2_I(inode)->ip_meta_lockres, 1, 0);

	if (ret) {

		mlog_errno(ret);

		goto bail;

	}



	ret = ocfs2_create_new_lock(osb, &OCFS2_I(inode)->ip_data_lockres, 1);

	ret = ocfs2_create_new_lock(osb, &OCFS2_I(inode)->ip_data_lockres, 1, 1);

	if (ret) {

		mlog_errno(ret);

		goto bail;
@@ -1413,6 +1420,16 @@ static void __ocfs2_stuff_meta_lvb(struct inode *inode) 


	lvb = (struct ocfs2_meta_lvb *) lockres->l_lksb.lvb;



	/*

	 * Invalidate the LVB of a deleted inode - this way other

	 * nodes are forced to go to disk and discover the new inode

	 * status.

	 */

	if (oi->ip_flags & OCFS2_INODE_DELETED) {

		lvb->lvb_version = 0;

		goto out;

	}



	lvb->lvb_version   = OCFS2_LVB_VERSION;

	lvb->lvb_isize	   = cpu_to_be64(i_size_read(inode));

	lvb->lvb_iclusters = cpu_to_be32(oi->ip_clusters);
@@ -1429,6 +1446,7 @@ static void __ocfs2_stuff_meta_lvb(struct inode *inode) 
	lvb->lvb_iattr    = cpu_to_be32(oi->ip_attr);

	lvb->lvb_igeneration = cpu_to_be32(inode->i_generation);



out:

	mlog_meta_lvb(0, lockres);



	mlog_exit_void();
@@ -1727,6 +1745,18 @@ int ocfs2_meta_lock_full(struct inode *inode, 
		wait_event(osb->recovery_event,

			   ocfs2_node_map_is_empty(osb, &osb->recovery_map));



	/*

	 * We only see this flag if we're being called from

	 * ocfs2_read_locked_inode(). It means we're locking an inode

	 * which hasn't been populated yet, so clear the refresh flag

	 * and let the caller handle it.

	 */

	if (inode->i_state & I_NEW) {

		status = 0;

		ocfs2_complete_lock_res_refresh(lockres, 0);

		goto bail;

	}



	/* This is fun. The caller may want a bh back, or it may

	 * not. ocfs2_meta_lock_update definitely wants one in, but

	 * may or may not read one, depending on what's in the

fs/ocfs2/dlmglue.h

+4 −3
Original line number Diff line number Diff line
@@ -32,9 +32,9 @@ 
#define OCFS2_LVB_VERSION 4



struct ocfs2_meta_lvb {

	__be16       lvb_reserved0;

	__u8         lvb_reserved1;

	__u8         lvb_version;

	__u8         lvb_reserved0;

	__be16       lvb_reserved1;

	__be32       lvb_iclusters;

	__be32       lvb_iuid;

	__be32       lvb_igid;
@@ -62,13 +62,14 @@ void ocfs2_dlm_shutdown(struct ocfs2_super *osb); 
void ocfs2_lock_res_init_once(struct ocfs2_lock_res *res);

void ocfs2_inode_lock_res_init(struct ocfs2_lock_res *res,

			       enum ocfs2_lock_type type,

			       unsigned int generation,

			       struct inode *inode);

void ocfs2_dentry_lock_res_init(struct ocfs2_dentry_lock *dl,

				u64 parent, struct inode *inode);

void ocfs2_lock_res_free(struct ocfs2_lock_res *res);

int ocfs2_create_new_inode_locks(struct inode *inode);

int ocfs2_create_new_lock(struct ocfs2_super *osb,

			  struct ocfs2_lock_res *lockres, int ex);

			  struct ocfs2_lock_res *lockres, int ex, int local);

int ocfs2_drop_inode_locks(struct inode *inode);

int ocfs2_data_lock_full(struct inode *inode,

			 int write,

fs/ocfs2/export.c

+2 −2
Original line number Diff line number Diff line
@@ -58,7 +58,7 @@ static struct dentry *ocfs2_get_dentry(struct super_block *sb, void *vobjp) 
		return ERR_PTR(-ESTALE);

	}



	inode = ocfs2_iget(OCFS2_SB(sb), handle->ih_blkno);

	inode = ocfs2_iget(OCFS2_SB(sb), handle->ih_blkno, 0);



	if (IS_ERR(inode)) {

		mlog_errno(PTR_ERR(inode));
@@ -115,7 +115,7 @@ static struct dentry *ocfs2_get_parent(struct dentry *child) 
		goto bail_unlock;

	}



	inode = ocfs2_iget(OCFS2_SB(dir->i_sb), blkno);

	inode = ocfs2_iget(OCFS2_SB(dir->i_sb), blkno, 0);

	if (IS_ERR(inode)) {

		mlog(ML_ERROR, "Unable to create inode %llu\n",

		     (unsigned long long)blkno);

fs/ocfs2/inode.c

+111 −35
Original line number Diff line number Diff line
@@ -54,8 +54,6 @@ 


#include "buffer_head_io.h"



#define OCFS2_FI_FLAG_NOWAIT	0x1

#define OCFS2_FI_FLAG_DELETE	0x2

struct ocfs2_find_inode_args

{

	u64		fi_blkno;
@@ -109,7 +107,7 @@ struct inode *ocfs2_ilookup_for_vote(struct ocfs2_super *osb, 
	return ilookup5(osb->sb, args.fi_ino, ocfs2_find_actor, &args);

}



struct inode *ocfs2_iget(struct ocfs2_super *osb, u64 blkno)

struct inode *ocfs2_iget(struct ocfs2_super *osb, u64 blkno, int flags)

{

	struct inode *inode = NULL;

	struct super_block *sb = osb->sb;
@@ -127,7 +125,7 @@ struct inode *ocfs2_iget(struct ocfs2_super *osb, u64 blkno) 
	}



	args.fi_blkno = blkno;

	args.fi_flags = 0;

	args.fi_flags = flags;

	args.fi_ino = ino_from_blkno(sb, blkno);



	inode = iget5_locked(sb, args.fi_ino, ocfs2_find_actor,
@@ -297,15 +295,11 @@ int ocfs2_populate_inode(struct inode *inode, struct ocfs2_dinode *fe, 
	OCFS2_I(inode)->ip_orphaned_slot = OCFS2_INVALID_SLOT;

	OCFS2_I(inode)->ip_attr = le32_to_cpu(fe->i_attr);



	if (create_ino)

		inode->i_ino = ino_from_blkno(inode->i_sb,

			       le64_to_cpu(fe->i_blkno));



	mlog(0, "blkno = %llu, ino = %lu, create_ino = %s\n",

	     (unsigned long long)fe->i_blkno, inode->i_ino, create_ino ? "true" : "false");



	inode->i_nlink = le16_to_cpu(fe->i_links_count);



	if (fe->i_flags & cpu_to_le32(OCFS2_SYSTEM_FL))

		OCFS2_I(inode)->ip_flags |= OCFS2_INODE_SYSTEM_FILE;



	if (fe->i_flags & cpu_to_le32(OCFS2_LOCAL_ALLOC_FL)) {

		OCFS2_I(inode)->ip_flags |= OCFS2_INODE_BITMAP;

		mlog(0, "local alloc inode: i_ino=%lu\n", inode->i_ino);
@@ -343,12 +337,28 @@ int ocfs2_populate_inode(struct inode *inode, struct ocfs2_dinode *fe, 
		    break;

	}



	ocfs2_inode_lock_res_init(&OCFS2_I(inode)->ip_rw_lockres,

				  OCFS2_LOCK_TYPE_RW, inode);

	if (create_ino) {

		inode->i_ino = ino_from_blkno(inode->i_sb,

			       le64_to_cpu(fe->i_blkno));



		/*

		 * If we ever want to create system files from kernel,

		 * the generation argument to

		 * ocfs2_inode_lock_res_init() will have to change.

		 */

		BUG_ON(fe->i_flags & cpu_to_le32(OCFS2_SYSTEM_FL));



		ocfs2_inode_lock_res_init(&OCFS2_I(inode)->ip_meta_lockres,

				  OCFS2_LOCK_TYPE_META, inode);

					  OCFS2_LOCK_TYPE_META, 0, inode);

	}



	ocfs2_inode_lock_res_init(&OCFS2_I(inode)->ip_rw_lockres,

				  OCFS2_LOCK_TYPE_RW, inode->i_generation,

				  inode);



	ocfs2_inode_lock_res_init(&OCFS2_I(inode)->ip_data_lockres,

				  OCFS2_LOCK_TYPE_DATA, inode);

				  OCFS2_LOCK_TYPE_DATA, inode->i_generation,

				  inode);



	ocfs2_set_inode_flags(inode);

	inode->i_flags |= S_NOATIME;
@@ -366,15 +376,15 @@ static int ocfs2_read_locked_inode(struct inode *inode, 
	struct ocfs2_super *osb;

	struct ocfs2_dinode *fe;

	struct buffer_head *bh = NULL;

	int status;

	int sysfile = 0;

	int status, can_lock;

	u32 generation = 0;



	mlog_entry("(0x%p, 0x%p)\n", inode, args);



	status = -EINVAL;

	if (inode == NULL || inode->i_sb == NULL) {

		mlog(ML_ERROR, "bad inode\n");

		goto bail;

		return status;

	}

	sb = inode->i_sb;

	osb = OCFS2_SB(sb);
@@ -382,50 +392,110 @@ static int ocfs2_read_locked_inode(struct inode *inode, 
	if (!args) {

		mlog(ML_ERROR, "bad inode args\n");

		make_bad_inode(inode);

		goto bail;

		return status;

	}



	/*

	 * To improve performance of cold-cache inode stats, we take

	 * the cluster lock here if possible.

	 *

	 * Generally, OCFS2 never trusts the contents of an inode

	 * unless it's holding a cluster lock, so taking it here isn't

	 * a correctness issue as much as it is a performance

	 * improvement.

	 *

	 * There are three times when taking the lock is not a good idea:

	 *

	 * 1) During startup, before we have initialized the DLM.

	 *

	 * 2) If we are reading certain system files which never get

	 *    cluster locks (local alloc, truncate log).

	 *

	 * 3) If the process doing the iget() is responsible for

	 *    orphan dir recovery. We're holding the orphan dir lock and

	 *    can get into a deadlock with another process on another

	 *    node in ->delete_inode().

	 *

	 * #1 and #2 can be simply solved by never taking the lock

	 * here for system files (which are the only type we read

	 * during mount). It's a heavier approach, but our main

	 * concern is user-accesible files anyway.

	 *

	 * #3 works itself out because we'll eventually take the

	 * cluster lock before trusting anything anyway.

	 */

	can_lock = !(args->fi_flags & OCFS2_FI_FLAG_SYSFILE)

		&& !(args->fi_flags & OCFS2_FI_FLAG_NOLOCK);



	/*

	 * To maintain backwards compatibility with older versions of

	 * ocfs2-tools, we still store the generation value for system

	 * files. The only ones that actually matter to userspace are

	 * the journals, but it's easier and inexpensive to just flag

	 * all system files similarly.

	 */

	if (args->fi_flags & OCFS2_FI_FLAG_SYSFILE)

		generation = osb->fs_generation;



	ocfs2_inode_lock_res_init(&OCFS2_I(inode)->ip_meta_lockres,

				  OCFS2_LOCK_TYPE_META,

				  generation, inode);



	if (can_lock) {

		status = ocfs2_meta_lock(inode, NULL, NULL, 0);

		if (status) {

			make_bad_inode(inode);

			mlog_errno(status);

			return status;

		}

	}



	/* Read the FE off disk. This is safe because the kernel only

	 * does one read_inode2 for a new inode, and if it doesn't

	 * exist yet then nobody can be working on it! */

	status = ocfs2_read_block(osb, args->fi_blkno, &bh, 0, NULL);

	status = ocfs2_read_block(osb, args->fi_blkno, &bh, 0,

				  can_lock ? inode : NULL);

	if (status < 0) {

		mlog_errno(status);

		make_bad_inode(inode);

		goto bail;

	}



	status = -EINVAL;

	fe = (struct ocfs2_dinode *) bh->b_data;

	if (!OCFS2_IS_VALID_DINODE(fe)) {

		mlog(ML_ERROR, "Invalid dinode #%llu: signature = %.*s\n",

		     (unsigned long long)fe->i_blkno, 7, fe->i_signature);

		make_bad_inode(inode);

		goto bail;

	}



	if (fe->i_flags & cpu_to_le32(OCFS2_SYSTEM_FL))

		sysfile = 1;

	/*

	 * This is a code bug. Right now the caller needs to

	 * understand whether it is asking for a system file inode or

	 * not so the proper lock names can be built.

	 */

	mlog_bug_on_msg(!!(fe->i_flags & cpu_to_le32(OCFS2_SYSTEM_FL)) !=

			!!(args->fi_flags & OCFS2_FI_FLAG_SYSFILE),

			"Inode %llu: system file state is ambigous\n",

			(unsigned long long)args->fi_blkno);



	if (S_ISCHR(le16_to_cpu(fe->i_mode)) ||

	    S_ISBLK(le16_to_cpu(fe->i_mode)))

    		inode->i_rdev = huge_decode_dev(le64_to_cpu(fe->id1.dev1.i_rdev));



	status = -EINVAL;

	if (ocfs2_populate_inode(inode, fe, 0) < 0) {

		mlog(ML_ERROR, "populate failed! i_blkno=%llu, i_ino=%lu\n",

		     (unsigned long long)fe->i_blkno, inode->i_ino);

		make_bad_inode(inode);

		goto bail;

	}



	BUG_ON(args->fi_blkno != le64_to_cpu(fe->i_blkno));



	if (sysfile)

	       OCFS2_I(inode)->ip_flags |= OCFS2_INODE_SYSTEM_FILE;



	status = 0;



bail:

	if (can_lock)

		ocfs2_meta_unlock(inode, 0);



	if (status < 0)

		make_bad_inode(inode);



	if (args && bh)

		brelse(bh);
@@ -898,9 +968,15 @@ void ocfs2_delete_inode(struct inode *inode) 
		goto bail_unlock_inode;

	}



	/* Mark the inode as successfully deleted. This is important

	 * for ocfs2_clear_inode as it will check this flag and skip

	 * any checkpointing work */

	/*

	 * Mark the inode as successfully deleted.

	 *

	 * This is important for ocfs2_clear_inode() as it will check

	 * this flag and skip any checkpointing work

	 *

	 * ocfs2_stuff_meta_lvb() also uses this flag to invalidate

	 * the LVB for other nodes.

	 */

	OCFS2_I(inode)->ip_flags |= OCFS2_INODE_DELETED;



bail_unlock_inode:

CRA Git | Maintained and supported by SUSTech CRA and CCSE