From 8b9105d8281f10b500d47a00458631a586c7f1d4 Mon Sep 17 00:00:00 2001 From: Patrick Farrell Date: Tue, 28 Aug 2018 09:28:29 -0500 Subject: [PATCH] LU-11199 mdt: Attempt lookup lock on open Commit 4f50273a (LU-10269 ldlm: fix the issues introduced by try bits) changed the locking behavior on open to not attempt to grant the LOOKUP lock bit. This causes a performance regression in open(), which is up to 75% in some benchmarks, such as mdsrate (from lustre/tests/mpi): First create the files: mpirun -n 4 /usr/lib64/lustre/tests/mdsrate / -d /mnt/lustre/mdsrate --create --nfile=30000 Then drop caches: echo 3 > /proc/sys/vm/drop_caches Then run the open benchmark: mpirun -n 4 /usr/lib64/lustre/tests/mdsrate / -d /mnt/lustre/mdsrate --open --iters 8000 --nfile=30000 [More details in LU-1199] This patch reverts that specific part of 4f50273a, which restores open() performance to prior levels. It may not be 100% correct to ask for open only when also asking for layout, but this is the earlier behavior. There is a further patch in flight to optimize this code: https://review.whamcloud.com/32156 And further changes are left for that patch. Cray-bug-id: LUS-6358 Change-Id: Iceca88807e99955f28eba6bbcb3585964f7df2f4 Signed-off-by: Patrick Farrell Reviewed-on: https://review.whamcloud.com/32929 Tested-by: Jenkins Reviewed-by: Andreas Dilger Reviewed-by: Mike Pershin Tested-by: Maloo Reviewed-by: Oleg Drokin --- lustre/mdt/mdt_open.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/lustre/mdt/mdt_open.c b/lustre/mdt/mdt_open.c index cc0faf9..28d0656 100644 --- a/lustre/mdt/mdt_open.c +++ b/lustre/mdt/mdt_open.c @@ -909,9 +909,15 @@ static int mdt_object_open_lock(struct mdt_thread_info *info, mdt_lock_reg_init(lhc, lm); - /* one problem to return layout lock on open is that it may result - * in too many layout locks cached on the client side. */ + /* Return lookup lock to validate inode at the client side. + * This is pretty important otherwise MDT will return layout + * lock for each open. + * However this is a double-edged sword because changing + * permission will revoke a huge number of LOOKUP locks. + */ if (!OBD_FAIL_CHECK(OBD_FAIL_MDS_NO_LL_OPEN) && try_layout) { + if (!(*ibits & MDS_INODELOCK_LOOKUP)) + trybits |= MDS_INODELOCK_LOOKUP; trybits |= MDS_INODELOCK_LAYOUT; } -- 1.8.3.1