From: Gang He <ghe@suse.com>
To: ocfs2-devel@oss.oracle.com
Subject: [Ocfs2-devel] [PATCH] ocfs2: re-queue AST or BAST if sending is failed to improve the reliability
Date: Mon, 07 Aug 2017 01:43:17 -0600 [thread overview]
Message-ID: <59888A95020000F90008611E@prv-mh.provo.novell.com> (raw)
In-Reply-To: <63ADC13FD55D6546B7DECE290D39E373AC2CB721@H3CMLB14-EX.srv.huawei-3com.com>
Base on your description, this case should be a corner case, NOT a fatal error.
Should we use mlog(ML_NOTICE, ...) to print these logs?
Thanks
Gang
>>>
> Hi,
>
> In current code, while flushing AST, we don't handle an exception that
> sending AST or BAST is failed.
> But it is indeed possible that AST or BAST is lost due to some kind of
> networks fault.
>
> If above exception happens, the requesting node will never obtain an AST
> back, hence, it will never acquire the lock or abort current locking.
>
> With this patch, I'd like to fix this issue by re-queuing the AST or
> BAST if sending is failed due to networks fault.
>
> And the re-queuing AST or BAST will be dropped if the requesting node is
> dead!
>
> It will improve the reliability a lot.
>
>
> Thanks.
>
> Changwei.
>
> Signed-off-by: Changwei Ge <ge.changwei@h3c.com>
> ---
> fs/ocfs2/dlm/dlmrecovery.c | 51
> ++++++++++++++++++++++++++++++++++++++++++--
> fs/ocfs2/dlm/dlmthread.c | 39 +++++++++++++++++++++++++++------
> 2 files changed, 81 insertions(+), 9 deletions(-)
>
> diff --git a/fs/ocfs2/dlm/dlmrecovery.c b/fs/ocfs2/dlm/dlmrecovery.c
> index 74407c6..ddfaf74 100644
> --- a/fs/ocfs2/dlm/dlmrecovery.c
> +++ b/fs/ocfs2/dlm/dlmrecovery.c
> @@ -2263,11 +2263,45 @@ static void dlm_revalidate_lvb(struct dlm_ctxt *dlm,
> }
> }
>
> +static int dlm_drop_pending_ast_bast(struct dlm_ctxt *dlm,
> + struct dlm_lock *lock)
> +{
> + int reserved = 0;
> +
> + spin_lock(&dlm->ast_lock);
> + if (!list_empty(&lock->ast_list)) {
> + mlog(0, "%s: drop pending AST for lock(cookie=%u:%llu).\n",
> + dlm->name,
> + dlm_get_lock_cookie_node(be64_to_cpu(lock->ml.cookie)),
> + dlm_get_lock_cookie_seq(be64_to_cpu(lock->ml.cookie)));
> + list_del_init(&lock->ast_list);
> + lock->ast_pending = 0;
> + dlm_lock_put(lock);
> + reserved++;
> + }
> +
> + if (!list_empty(&lock->bast_list)) {
> + mlog(0, "%s: drop pending BAST for lock(cookie=%u:%llu).\n",
> + dlm->name,
> + dlm_get_lock_cookie_node(be64_to_cpu(lock->ml.cookie)),
> + dlm_get_lock_cookie_seq(be64_to_cpu(lock->ml.cookie)));
> + list_del_init(&lock->bast_list);
> + lock->bast_pending = 0;
> + dlm_lock_put(lock);
> + reserved++;
> + }
> + spin_unlock(&dlm->ast_lock);
> +
> + return reserved;
> +}
> +
> static void dlm_free_dead_locks(struct dlm_ctxt *dlm,
> - struct dlm_lock_resource *res, u8 dead_node)
> + struct dlm_lock_resource *res, u8 dead_node,
> + int *reserved)
> {
> struct dlm_lock *lock, *next;
> unsigned int freed = 0;
> + int reserved_tmp = 0;
>
> /* this node is the lockres master:
> * 1) remove any stale locks for the dead node
> @@ -2284,6 +2318,9 @@ static void dlm_free_dead_locks(struct dlm_ctxt *dlm,
> if (lock->ml.node == dead_node) {
> list_del_init(&lock->list);
> dlm_lock_put(lock);
> +
> + reserved_tmp += dlm_drop_pending_ast_bast(dlm, lock);
> +
> /* Can't schedule DLM_UNLOCK_FREE_LOCK - do manually */
> dlm_lock_put(lock);
> freed++;
> @@ -2293,6 +2330,9 @@ static void dlm_free_dead_locks(struct dlm_ctxt *dlm,
> if (lock->ml.node == dead_node) {
> list_del_init(&lock->list);
> dlm_lock_put(lock);
> +
> + reserved_tmp += dlm_drop_pending_ast_bast(dlm, lock);
> +
> /* Can't schedule DLM_UNLOCK_FREE_LOCK - do manually */
> dlm_lock_put(lock);
> freed++;
> @@ -2308,6 +2348,8 @@ static void dlm_free_dead_locks(struct dlm_ctxt *dlm,
> }
> }
>
> + *reserved = reserved_tmp;
> +
> if (freed) {
> mlog(0, "%s:%.*s: freed %u locks for dead node %u, "
> "dropping ref from lockres\n", dlm->name,
> @@ -2367,6 +2409,7 @@ static void dlm_do_local_recovery_cleanup(struct
> dlm_ctxt *dlm, u8 dead_node)
> for (i = 0; i < DLM_HASH_BUCKETS; i++) {
> bucket = dlm_lockres_hash(dlm, i);
> hlist_for_each_entry_safe(res, tmp, bucket, hash_node) {
> + int reserved = 0;
> /* always prune any $RECOVERY entries for dead nodes,
> * otherwise hangs can occur during later recovery */
> if (dlm_is_recovery_lock(res->lockname.name,
> @@ -2420,7 +2463,7 @@ static void dlm_do_local_recovery_cleanup(struct
> dlm_ctxt *dlm, u8 dead_node)
> continue;
> }
> } else if (res->owner == dlm->node_num) {
> - dlm_free_dead_locks(dlm, res, dead_node);
> + dlm_free_dead_locks(dlm, res, dead_node, &reserved);
> __dlm_lockres_calc_usage(dlm, res);
> } else if (res->owner == DLM_LOCK_RES_OWNER_UNKNOWN) {
> if (test_bit(dead_node, res->refmap)) {
> @@ -2432,6 +2475,10 @@ static void dlm_do_local_recovery_cleanup(struct
> dlm_ctxt *dlm, u8 dead_node)
> }
> }
> spin_unlock(&res->spinlock);
> + while (reserved) {
> + dlm_lockres_release_ast(dlm, res);
> + reserved--;
> + }
> }
> }
>
> diff --git a/fs/ocfs2/dlm/dlmthread.c b/fs/ocfs2/dlm/dlmthread.c
> index 838a06d..c34a619 100644
> --- a/fs/ocfs2/dlm/dlmthread.c
> +++ b/fs/ocfs2/dlm/dlmthread.c
> @@ -587,13 +587,13 @@ static int dlm_dirty_list_empty(struct dlm_ctxt *dlm)
>
> static void dlm_flush_asts(struct dlm_ctxt *dlm)
> {
> - int ret;
> + int ret = 0;
> struct dlm_lock *lock;
> struct dlm_lock_resource *res;
> u8 hi;
>
> spin_lock(&dlm->ast_lock);
> - while (!list_empty(&dlm->pending_asts)) {
> + while (!list_empty(&dlm->pending_asts) && !ret) {
> lock = list_entry(dlm->pending_asts.next,
> struct dlm_lock, ast_list);
> /* get an extra ref on lock */
> @@ -628,8 +628,20 @@ static void dlm_flush_asts(struct dlm_ctxt *dlm)
> mlog(0, "%s: res %.*s, AST queued while flushing last "
> "one\n", dlm->name, res->lockname.len,
> res->lockname.name);
> - } else
> - lock->ast_pending = 0;
> + } else {
> + if (unlikely(ret < 0)) {
> + /* If this AST is not sent back successfully,
> + * there is no chance that the second lock
> + * request comes.
> + */
> + spin_lock(&res->spinlock);
> + __dlm_lockres_reserve_ast(res);
> + spin_unlock(&res->spinlock);
> + __dlm_queue_ast(dlm, lock);
> + } else {
> + lock->ast_pending = 0;
> + }
> + }
>
> /* drop the extra ref.
> * this may drop it completely. */
> @@ -637,7 +649,9 @@ static void dlm_flush_asts(struct dlm_ctxt *dlm)
> dlm_lockres_release_ast(dlm, res);
> }
>
> - while (!list_empty(&dlm->pending_basts)) {
> + ret = 0;
> +
> + while (!list_empty(&dlm->pending_basts) && !ret) {
> lock = list_entry(dlm->pending_basts.next,
> struct dlm_lock, bast_list);
> /* get an extra ref on lock */
> @@ -650,7 +664,6 @@ static void dlm_flush_asts(struct dlm_ctxt *dlm)
> spin_lock(&lock->spinlock);
> BUG_ON(lock->ml.highest_blocked <= LKM_IVMODE);
> hi = lock->ml.highest_blocked;
> - lock->ml.highest_blocked = LKM_IVMODE;
> spin_unlock(&lock->spinlock);
>
> /* remove from list (including ref) */
> @@ -681,7 +694,19 @@ static void dlm_flush_asts(struct dlm_ctxt *dlm)
> "one\n", dlm->name, res->lockname.len,
> res->lockname.name);
> } else
> - lock->bast_pending = 0;
> + if (unlikely(ret)) {
> + spin_lock(&res->spinlock);
> + __dlm_lockres_reserve_ast(res);
> + spin_unlock(&res->spinlock);
> + __dlm_queue_bast(dlm, lock);
> + } else {
> + lock->bast_pending = 0;
> + /* Set ::highest_blocked to invalid after
> + * sending BAST successfully so that
> + * no more BAST would be queued.
> + */
> + lock->ml.highest_blocked = LKM_IVMODE;
> + }
>
> /* drop the extra ref.
> * this may drop it completely. */
> --
> 1.7.9.5
>
>
> _______________________________________________
> Ocfs2-devel mailing list
> Ocfs2-devel at oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/ocfs2-devel
next prev parent reply other threads:[~2017-08-07 7:43 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-08-07 7:13 [Ocfs2-devel] [PATCH] ocfs2: re-queue AST or BAST if sending is failed to improve the reliability Changwei Ge
2017-08-07 7:43 ` Gang He [this message]
2017-08-07 7:55 ` Changwei Ge
2017-08-07 20:19 ` Mark Fasheh
2017-08-08 10:56 ` Changwei Ge
2017-08-22 20:49 ` Mark Fasheh
2017-08-23 1:06 ` Joseph Qi
2017-08-09 11:32 ` Joseph Qi
2017-08-09 15:24 ` ge changwei
2017-08-10 9:34 ` Joseph Qi
2017-08-10 10:49 ` Changwei Ge
2017-08-23 2:23 ` Junxiao Bi
2017-08-23 3:34 ` Joseph Qi
2017-08-23 4:47 ` Gang He
2017-08-23 5:56 ` Changwei Ge
[not found] ` <63ADC13FD55D6546B7DECE290D39E373CED4F4ED@H3CMLB14-EX.srv.huawei-3com.com>
2017-09-13 7:03 ` Changwei Ge
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=59888A95020000F90008611E@prv-mh.provo.novell.com \
--to=ghe@suse.com \
--cc=ocfs2-devel@oss.oracle.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.