mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: green@linuxhacker.ru
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	devel@driverdev.osuosl.org,
	Andreas Dilger <andreas.dilger@intel.com>
Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Lustre Development List <lustre-devel@lists.lustre.org>,
	Vitaly Fertman <vitaly.fertman@seagate.com>,
	Jinshan Xiong <jinshan.xiong@intel.com>,
	Vitaly Fertman <vitaly_fertman@xyratex.com>,
	Oleg Drokin <green@linuxhacker.ru>
Subject: [PATCH v2 45/46] staging/lustre/ldlm: Solve a race for LRU lock cancel
Date: Wed, 30 Mar 2016 19:49:06 -0400	[thread overview]
Message-ID: <1459381747-2886687-46-git-send-email-green@linuxhacker.ru> (raw)
In-Reply-To: <1459381747-2886687-1-git-send-email-green@linuxhacker.ru>

From: Vitaly Fertman <vitaly.fertman@seagate.com>

This patch solves a race condition that the lock may be used again
after LRU cancellation policy check. In that case, the lock may have
locked or dirty pages that makes the policy check totally useless.
The problem is solved by checking l_last_used at cancellation time
therefore it can make sure that the lock has not been used.

Signed-off-by: Jinshan Xiong <jinshan.xiong@intel.com>
Signed-off-by: Vitaly Fertman <vitaly_fertman@xyratex.com>
Reviewed-on: http://review.whamcloud.com/12603
Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-5781
Reviewed-by: James Simmons <uja.ornl@yahoo.com>
Signed-off-by: Oleg Drokin <green@linuxhacker.ru>
---
 drivers/staging/lustre/lustre/ldlm/ldlm_internal.h |  3 ++-
 drivers/staging/lustre/lustre/ldlm/ldlm_lock.c     | 16 +++++++++++++---
 drivers/staging/lustre/lustre/ldlm/ldlm_request.c  | 11 +++++++++--
 3 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_internal.h b/drivers/staging/lustre/lustre/ldlm/ldlm_internal.h
index e31d84a..351f8b4 100644
--- a/drivers/staging/lustre/lustre/ldlm/ldlm_internal.h
+++ b/drivers/staging/lustre/lustre/ldlm/ldlm_internal.h
@@ -146,7 +146,8 @@ void ldlm_lock_decref_internal(struct ldlm_lock *, __u32 mode);
 void ldlm_lock_decref_internal_nolock(struct ldlm_lock *, __u32 mode);
 int ldlm_run_ast_work(struct ldlm_namespace *ns, struct list_head *rpc_list,
 		      enum ldlm_desc_ast_t ast_type);
-int ldlm_lock_remove_from_lru(struct ldlm_lock *lock);
+int ldlm_lock_remove_from_lru_check(struct ldlm_lock *lock, time_t last_use);
+#define ldlm_lock_remove_from_lru(lock) ldlm_lock_remove_from_lru_check(lock, 0)
 int ldlm_lock_remove_from_lru_nolock(struct ldlm_lock *lock);
 void ldlm_lock_destroy_nolock(struct ldlm_lock *lock);
 
diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c b/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
index 27a051b..3f9b8526 100644
--- a/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
+++ b/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
@@ -229,15 +229,25 @@ int ldlm_lock_remove_from_lru_nolock(struct ldlm_lock *lock)
 
 /**
  * Removes LDLM lock \a lock from LRU. Obtains the LRU lock first.
+ *
+ * If \a last_use is non-zero, it will remove the lock from LRU only if
+ * it matches lock's l_last_used.
+ *
+ * \retval 0 if \a last_use is set, the lock is not in LRU list or \a last_use
+ *           doesn't match lock's l_last_used;
+ *           otherwise, the lock hasn't been in the LRU list.
+ * \retval 1 the lock was in LRU list and removed.
  */
-int ldlm_lock_remove_from_lru(struct ldlm_lock *lock)
+int ldlm_lock_remove_from_lru_check(struct ldlm_lock *lock, time_t last_use)
 {
 	struct ldlm_namespace *ns = ldlm_lock_to_ns(lock);
-	int rc;
+	int rc = 0;
 
 	spin_lock(&ns->ns_lock);
-	rc = ldlm_lock_remove_from_lru_nolock(lock);
+	if (last_use == 0 || last_use == lock->l_last_used)
+		rc = ldlm_lock_remove_from_lru_nolock(lock);
 	spin_unlock(&ns->ns_lock);
+
 	return rc;
 }
 
diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_request.c b/drivers/staging/lustre/lustre/ldlm/ldlm_request.c
index 9aa4c2d..5b0e396 100644
--- a/drivers/staging/lustre/lustre/ldlm/ldlm_request.c
+++ b/drivers/staging/lustre/lustre/ldlm/ldlm_request.c
@@ -1369,6 +1369,7 @@ static int ldlm_prepare_lru_list(struct ldlm_namespace *ns,
 
 	while (!list_empty(&ns->ns_unused_list)) {
 		ldlm_policy_res_t result;
+		time_t last_use = 0;
 
 		/* all unused locks */
 		if (remained-- <= 0)
@@ -1387,6 +1388,10 @@ static int ldlm_prepare_lru_list(struct ldlm_namespace *ns,
 				/* already processed */
 				continue;
 
+			last_use = lock->l_last_used;
+			if (last_use == cfs_time_current())
+				continue;
+
 			/* Somebody is already doing CANCEL. No need for this
 			 * lock in LRU, do not traverse it again.
 			 */
@@ -1434,11 +1439,13 @@ static int ldlm_prepare_lru_list(struct ldlm_namespace *ns,
 		lock_res_and_lock(lock);
 		/* Check flags again under the lock. */
 		if ((lock->l_flags & LDLM_FL_CANCELING) ||
-		    (ldlm_lock_remove_from_lru(lock) == 0)) {
+		    (ldlm_lock_remove_from_lru_check(lock, last_use) == 0)) {
 			/* Another thread is removing lock from LRU, or
 			 * somebody is already doing CANCEL, or there
 			 * is a blocking request which will send cancel
-			 * by itself, or the lock is no longer unused.
+			 * by itself, or the lock is no longer unused or
+			 * the lock has been used since the pf() call and
+			 * pages could be put under it.
 			 */
 			unlock_res_and_lock(lock);
 			lu_ref_del(&lock->l_reference,
-- 
2.1.0

  parent reply	other threads:[~2016-03-30 23:51 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-30 23:48 [PATCH v2 00/46] Lustre IO stack simplifications and cleanups green
2016-03-30 23:48 ` [PATCH v2 01/46] staging/lustre/obdclass: limit lu_site hash table size green
2016-03-30 23:48 ` [PATCH v2 02/46] staging/lustre: Get rid of CFS_PAGE_MASK green
2016-03-30 23:48 ` [PATCH v2 03/46] staging/lustre: merge lclient/*.c into llite/ green
2016-03-30 23:48 ` [PATCH v2 04/46] staging/lustre: Reintroduce global env list green
2016-03-30 23:48 ` [PATCH v2 05/46] staging/lustre/osc: Adjustment on osc LRU for performance green
2016-03-30 23:48 ` [PATCH v2 06/46] staging/lustre/osc: to drop LRU pages with cl_lru_work green
2016-03-30 23:48 ` [PATCH v2 07/46] staging/lustre/clio: collapse layer of cl_page green
2016-03-30 23:48 ` [PATCH v2 08/46] staging/lustre/obdclass: Add a preallocated percpu cl_env green
2016-03-30 23:48 ` [PATCH v2 09/46] staging/lustre/clio: add pages into writeback cache in batches green
2016-03-30 23:48 ` [PATCH v2 10/46] staging/lustre/osc: add weight function for DLM lock green
2016-03-30 23:48 ` [PATCH v2 11/46] staging/lustre/clio: remove stackable cl_page completely green
2016-03-30 23:48 ` [PATCH v2 12/46] staging/lustre/clio: optimize read ahead code green
2016-03-30 23:48 ` [PATCH v2 13/46] staging/lustre/llite: remove lli_lvb green
2016-03-30 23:48 ` [PATCH v2 14/46] staging/lustre/lmv: remove lmv_init_{lock,unlock}() green
2016-03-30 23:48 ` [PATCH v2 15/46] staging/lustre/obd: remove struct client_obd_lock green
2016-03-30 23:48 ` [PATCH v2 16/46] staging/lustre/llite: remove some cl wrappers green
2016-03-30 23:48 ` [PATCH v2 17/46] staging/lustre: Remove struct ll_iattr green
2016-03-30 23:48 ` [PATCH v2 18/46] staging/lustre/clio: generalize cl_sync_io green
2016-03-30 23:48 ` [PATCH v2 19/46] staging/lustre/clio: cl_lock simplification green
2016-03-30 23:48 ` [PATCH v2 20/46] staging/lustre: update comments after " green
2016-03-30 23:48 ` [PATCH v2 21/46] staging/lustre/llite: clip page correctly for vvp_io_commit_sync green
2016-03-30 23:48 ` [PATCH v2 22/46] staging/lustre/llite: deadlock for page write green
2016-03-30 23:48 ` [PATCH v2 23/46] staging/lustre/llite: make sure we do cl_page_clip on the last page green
2016-03-30 23:48 ` [PATCH v2 24/46] staging/lustre/llite: merge lclient.h into llite/vvp_internal.h green
2016-03-30 23:48 ` [PATCH v2 25/46] staging/lustre/llite: rename ccc_device to vvp_device green
2016-03-30 23:48 ` [PATCH v2 26/46] staging/lustre/llite: rename ccc_object to vvp_object green
2016-03-30 23:48 ` [PATCH v2 27/46] staging/lustre/llite: rename ccc_page to vvp_page green
2016-03-30 23:48 ` [PATCH v2 28/46] staging/lustre/llite: rename ccc_lock to vvp_lock green
2016-03-30 23:48 ` [PATCH v2 29/46] staging/lustre:llite: remove struct ll_ra_read green
2016-03-30 23:48 ` [PATCH v2 30/46] staging/lustre/llite: merge ccc_io and vvp_io green
2016-03-30 23:48 ` [PATCH v2 31/46] staging/lustre/llite: use vui prefix for struct vvp_io members green
2016-03-30 23:48 ` [PATCH v2 32/46] staging/lustre/llite: move vvp_io functions to vvp_io.c green
2016-03-30 23:48 ` [PATCH v2 33/46] staging/lustre/llite: rename ccc_req to vvp_req green
2016-03-30 23:48 ` [PATCH v2 34/46] staging/lustre/llite: Rename struct ccc_grouplock to ll_grouplock green
2016-03-30 23:48 ` [PATCH v2 35/46] staging/lustre/llite: Rename struct vvp_thread_info to ll_thread_info green
2016-03-30 23:48 ` [PATCH v2 36/46] staging/lustre/llite: rename struct ccc_thread_info to vvp_thread_info green
2016-03-30 23:48 ` [PATCH v2 37/46] staging/lustre/llite: Remove ccc_global_{init,fini}() green
2016-03-30 23:48 ` [PATCH v2 38/46] staging/lustre/llite: Move ll_dirent_type_get and make it static green
2016-03-30 23:49 ` [PATCH v2 39/46] staging/lustre/llite: Move several declarations to llite_internal.h green
2016-03-30 23:49 ` [PATCH v2 40/46] staging/lustre/llite: Remove unused vui_local_lock field green
2016-03-30 23:49 ` [PATCH v2 41/46] staging/lustre/ldlm: ELC picks locks in a safer policy green
2016-03-30 23:49 ` [PATCH v2 42/46] staging/lustre/ldlm: revert changes to ldlm_cancel_aged_policy() green
2016-03-30 23:49 ` [PATCH v2 43/46] staging/lustre/ldlm: restore the ELC for enqueue green
2016-03-30 23:49 ` [PATCH v2 44/46] staging/lustre: Fix spacing style before open parenthesis green
2016-03-30 23:49 ` green [this message]
2016-03-30 23:49 ` [PATCH v2 46/46] staging/lustre: lov_io_init() should return error code green

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1459381747-2886687-46-git-send-email-green@linuxhacker.ru \
    --to=green@linuxhacker.ru \
    --cc=andreas.dilger@intel.com \
    --cc=devel@driverdev.osuosl.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=jinshan.xiong@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lustre-devel@lists.lustre.org \
    --cc=vitaly.fertman@seagate.com \
    --cc=vitaly_fertman@xyratex.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome