[Cluster-devel] [PATCH dlm/next 1/2] fs: dlm: plock debugfs to check for pending operations

[Cluster-devel] [PATCH dlm/next 1/2] fs: dlm: plock debugfs to check for pending operations

[Alexander Aring]

In the past issues were found that there were still ongoing plock
operations in the kernel but it should cleanup routines should clear
them up because there were no plock activity by the user anymore. To
check that "dlm_tool plocks $LS" can be used, but this only shows
pending operations in dlm_controld daemon. To check the kernel part, if
the kernel waits for an answer of the user space, this patch introduces
a debugfs entry which reports if there are ongoing plock operations or
not.

Signed-off-by: Alexander Aring <aahringo@redhat.com>
---
 fs/dlm/debug_fs.c     | 26 ++++++++++++++++++++++++++
 fs/dlm/dlm_internal.h |  5 +++++
 fs/dlm/plock.c        | 15 +++++++++++++++
 3 files changed, 46 insertions(+)

diff --git a/fs/dlm/debug_fs.c b/fs/dlm/debug_fs.c
index a1aca41c49d0..494a6e73f8e8 100644
--- a/fs/dlm/debug_fs.c
+++ b/fs/dlm/debug_fs.c
@@ -25,6 +25,7 @@ static struct mutex debug_buf_lock;
 
 static struct dentry *dlm_root;
 static struct dentry *dlm_comms;
+static struct dentry *dlm_plock;
 
 static char *print_lockmode(int mode)
 {
@@ -883,6 +884,30 @@ void dlm_delete_debug_comms_file(void *ctx)
 	debugfs_remove(ctx);
 }
 
+static int dlm_plock_ops_pending_show(struct seq_file *file, void *offset)
+{
+	seq_printf(file, "%d\n", dlm_plock_ops_pending());
+	return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(dlm_plock_ops_pending);
+
+void dlm_create_debug_plock_file(void)
+{
+	/* TODO currently use case if only to look if everything got cleaned
+	 * up probably if user space dlm_tool plocks $LS shows no activity
+	 * anymore on all lockspaces.
+	 *
+	 * However in future a dump could be useful as well.
+	 */
+	debugfs_create_file("plock_ops_pending", 0444, dlm_plock, NULL,
+			    &dlm_plock_ops_pending_fops);
+}
+
+void dlm_remove_debug_plock_file(void)
+{
+	debugfs_remove(dlm_plock);
+}
+
 void dlm_create_debug_file(struct dlm_ls *ls)
 {
 	char name[DLM_LOCKSPACE_LEN + 8];
@@ -943,6 +968,7 @@ void __init dlm_register_debugfs(void)
 	mutex_init(&debug_buf_lock);
 	dlm_root = debugfs_create_dir("dlm", NULL);
 	dlm_comms = debugfs_create_dir("comms", dlm_root);
+	dlm_plock = debugfs_create_dir("plock", dlm_root);
 }
 
 void dlm_unregister_debugfs(void)
diff --git a/fs/dlm/dlm_internal.h b/fs/dlm/dlm_internal.h
index 986a9d7b1f33..f5f741ee527b 100644
--- a/fs/dlm/dlm_internal.h
+++ b/fs/dlm/dlm_internal.h
@@ -805,6 +805,7 @@ static inline void dlm_set_sbflags_val(struct dlm_lkb *lkb, uint32_t val)
 			  __DLM_SBF_MAX_BIT);
 }
 
+int dlm_plock_ops_pending(void);
 int dlm_plock_init(void);
 void dlm_plock_exit(void);
 
@@ -815,6 +816,8 @@ void dlm_create_debug_file(struct dlm_ls *ls);
 void dlm_delete_debug_file(struct dlm_ls *ls);
 void *dlm_create_debug_comms_file(int nodeid, void *data);
 void dlm_delete_debug_comms_file(void *ctx);
+void dlm_create_debug_plock_file(void);
+void dlm_remove_debug_plock_file(void);
 #else
 static inline void dlm_register_debugfs(void) { }
 static inline void dlm_unregister_debugfs(void) { }
@@ -822,6 +825,8 @@ static inline void dlm_create_debug_file(struct dlm_ls *ls) { }
 static inline void dlm_delete_debug_file(struct dlm_ls *ls) { }
 static inline void *dlm_create_debug_comms_file(int nodeid, void *data) { return NULL; }
 static inline void dlm_delete_debug_comms_file(void *ctx) { }
+static inline void dlm_create_debug_plock_file(void) { };
+static inline void dlm_remove_debug_plock_file(void) { };
 #endif
 
 #endif				/* __DLM_INTERNAL_DOT_H__ */
diff --git a/fs/dlm/plock.c b/fs/dlm/plock.c
index 53d17dbbb716..d6ec70547b77 100644
--- a/fs/dlm/plock.c
+++ b/fs/dlm/plock.c
@@ -35,6 +35,18 @@ struct plock_op {
 	struct plock_async_data *data;
 };
 
+int dlm_plock_ops_pending(void)
+{
+	int rv;
+
+	spin_lock(&ops_lock);
+	rv = !list_empty(&send_list);
+	rv |= !list_empty(&recv_list);
+	spin_unlock(&ops_lock);
+
+	return rv;
+}
+
 static inline void set_version(struct dlm_plock_info *info)
 {
 	info->version[0] = DLM_PLOCK_VERSION_MAJOR;
@@ -509,11 +521,14 @@ int dlm_plock_init(void)
 	rv = misc_register(&plock_dev_misc);
 	if (rv)
 		log_print("dlm_plock_init: misc_register failed %d", rv);
+
+	dlm_create_debug_plock_file();
 	return rv;
 }
 
 void dlm_plock_exit(void)
 {
+	dlm_remove_debug_plock_file();
 	misc_deregister(&plock_dev_misc);
 }
 
-- 
2.31.1

[Cluster-devel] [PATCH dlm/next 2/2] fs: dlm: cleanup plock op lookup functionality

[Alexander Aring]

This patch will introduce plock_op_lookup() to lookup a plock op when a
result needs to lookup the plock op to find the original request.
Besides that we add additional sanity check to confirm the lookup is
still working in case of non F_SETLKW request as it requires a specific
order in recv_list.

Signed-off-by: Alexander Aring <aahringo@redhat.com>
---
 fs/dlm/plock.c | 117 ++++++++++++++++++++++++++++++++-----------------
 1 file changed, 78 insertions(+), 39 deletions(-)

diff --git a/fs/dlm/plock.c b/fs/dlm/plock.c
index d6ec70547b77..b2b6635fcb28 100644
--- a/fs/dlm/plock.c
+++ b/fs/dlm/plock.c
@@ -423,66 +423,105 @@ static ssize_t dev_read(struct file *file, char __user *u, size_t count,
 	return sizeof(info);
 }
 
-/* a write copies in one plock result that should match a plock_op
-   on the recv list */
-static ssize_t dev_write(struct file *file, const char __user *u, size_t count,
-			 loff_t *ppos)
+static struct plock_op *plock_op_lookup(const struct dlm_plock_info *info)
 {
 	struct plock_op *op = NULL, *iter;
-	struct dlm_plock_info info;
-	int do_callback = 0;
-
-	if (count != sizeof(info))
-		return -EINVAL;
-
-	if (copy_from_user(&info, u, sizeof(info)))
-		return -EFAULT;
-
-	if (check_version(&info))
-		return -EINVAL;
 
-	spin_lock(&ops_lock);
-	if (info.wait) {
+	/* F_SETLKW does not have an order in recv_list */
+	if (info->wait) {
 		list_for_each_entry(iter, &recv_list, list) {
-			if (iter->info.fsid == info.fsid &&
-			    iter->info.number == info.number &&
-			    iter->info.owner == info.owner &&
-			    iter->info.pid == info.pid &&
-			    iter->info.start == info.start &&
-			    iter->info.end == info.end &&
-			    iter->info.ex == info.ex &&
+			if (iter->info.fsid == info->fsid &&
+			    iter->info.number == info->number &&
+			    iter->info.owner == info->owner &&
+			    iter->info.pid == info->pid &&
+			    iter->info.start == info->start &&
+			    iter->info.end == info->end &&
+			    iter->info.ex == info->ex &&
 			    iter->info.wait) {
 				op = iter;
+
+				/* sanity check should be DLM_PLOCK_OP_LOCK */
+				WARN_ON(op->info.optype != DLM_PLOCK_OP_LOCK);
 				break;
 			}
 		}
 	} else {
+		/* others than F_SETLKW have an order in recv_list we
+		 * need to jump over all F_SETLKW ops (info->wait) and match
+		 * the first one.
+		 */
 		list_for_each_entry(iter, &recv_list, list) {
 			if (!iter->info.wait) {
 				op = iter;
+
+				/* sanity check to check we got the right one */
+				WARN_ON(op->info.fsid != info->fsid ||
+					op->info.number != info->number ||
+					op->info.owner != info->owner ||
+					op->info.optype != info->optype);
+
+				switch (op->info.optype) {
+				case DLM_PLOCK_OP_GET:
+					break;
+				default:
+					WARN_ON(op->info.pid != info->pid ||
+						op->info.start != info->start ||
+						op->info.end != info->end ||
+						op->info.ex != info->ex);
+					break;
+				}
 				break;
 			}
 		}
 	}
 
-	if (op) {
-		list_del_init(&op->list);
-		memcpy(&op->info, &info, sizeof(info));
-		if (op->data)
-			do_callback = 1;
-		else
-			op->done = 1;
-	}
-	spin_unlock(&ops_lock);
+	return op;
+}
 
-	if (op) {
-		if (do_callback)
-			dlm_plock_callback(op);
-		else
-			wake_up(&recv_wq);
-	} else
+/* a write copies in one plock result that should match a plock_op
+ * on the recv list
+ */
+static ssize_t dev_write(struct file *file, const char __user *u, size_t count,
+			 loff_t *ppos)
+{
+	struct dlm_plock_info info;
+	struct plock_op *op;
+
+	if (count != sizeof(info))
+		return -EINVAL;
+
+	if (copy_from_user(&info, u, sizeof(info)))
+		return -EFAULT;
+
+	if (check_version(&info))
+		return -EINVAL;
+
+	spin_lock(&ops_lock);
+	op = plock_op_lookup(&info);
+	if (!op) {
+		spin_unlock(&ops_lock);
 		pr_debug("%s: no op %x %llx", __func__,
 			 info.fsid, (unsigned long long)info.number);
+		return count;
+	}
+
+	list_del_init(&op->list);
+	/* update set new fields by user space e.g. F_GETLK */
+	memcpy(&op->info, &info, sizeof(info));
+
+	/* check for async handling */
+	if (op->data) {
+		spin_unlock(&ops_lock);
+		dlm_plock_callback(op);
+	} else {
+		/* must be set under ops_lock, because retry in setlkw_wait()
+		 * if -ERESTARTSYS.
+		 */
+		op->done = 1;
+		spin_unlock(&ops_lock);
+		wake_up(&recv_wq);
+	}
+
 	return count;
 }
 
-- 
2.31.1

[Cluster-devel] [PATCH dlm/next 2/2] fs: dlm: cleanup plock op lookup functionality

[Alexander Aring]

This patch will introduce plock_op_lookup() to lookup a plock op when a
result needs to lookup the plock op to find the original request.
Besides that we add additional sanity check to confirm the lookup is
still working in case of non F_SETLKW request as it requires a specific
order in recv_list.

Signed-off-by: Alexander Aring <aahringo@redhat.com>
---
 fs/dlm/plock.c | 107 +++++++++++++++++++++++++++++++------------------
 1 file changed, 68 insertions(+), 39 deletions(-)

diff --git a/fs/dlm/plock.c b/fs/dlm/plock.c
index 578bf8a1325f..933fb575e83a 100644
--- a/fs/dlm/plock.c
+++ b/fs/dlm/plock.c
@@ -433,14 +433,58 @@ static ssize_t dev_read(struct file *file, char __user *u, size_t count,
 	return sizeof(info);
 }
 
+static struct plock_op *plock_op_lookup(const struct dlm_plock_info *info)
+{
+	struct plock_op *op = NULL, *iter;
+
+	if (info->wait) {
+		list_for_each_entry(iter, &recv_setlkw_list, list) {
+			if (iter->info.fsid == info->fsid &&
+			    iter->info.number == info->number &&
+			    iter->info.owner == info->owner &&
+			    iter->info.pid == info->pid &&
+			    iter->info.start == info->start &&
+			    iter->info.end == info->end) {
+				op = iter;
+				break;
+			}
+		}
+	} else {
+		op = list_first_entry_or_null(&recv_list, struct plock_op,
+					      list);
+		if (op) {
+			/* sanity check to check we got the right one */
+			switch (op->info.optype) {
+			case DLM_PLOCK_OP_GET:
+				/* we can't check on some fields on get */
+				WARN_ON(op->info.fsid != info->fsid ||
+					op->info.number != info->number ||
+					op->info.owner != info->owner ||
+					op->info.optype != info->optype);
+				break;
+			default:
+				WARN_ON(op->info.fsid != info->fsid ||
+					op->info.number != info->number ||
+					op->info.owner != info->owner ||
+					op->info.pid != info->pid ||
+					op->info.start != info->start ||
+					op->info.end != info->end ||
+					op->info.optype != info->optype);
+				break;
+			}
+		}
+	}
+
+	return op;
+}
+
 /* a write copies in one plock result that should match a plock_op
    on the recv list */
 static ssize_t dev_write(struct file *file, const char __user *u, size_t count,
 			 loff_t *ppos)
 {
-	struct plock_op *op = NULL, *iter;
 	struct dlm_plock_info info;
-	int do_callback = 0;
+	struct plock_op *op;
 
 	if (count != sizeof(info))
 		return -EINVAL;
@@ -452,46 +496,31 @@ static ssize_t dev_write(struct file *file, const char __user *u, size_t count,
 		return -EINVAL;
 
 	spin_lock(&ops_lock);
-	if (info.wait) {
-		list_for_each_entry(iter, &recv_setlkw_list, list) {
-			if (iter->info.fsid == info.fsid &&
-			    iter->info.number == info.number &&
-			    iter->info.owner == info.owner &&
-			    iter->info.pid == info.pid &&
-			    iter->info.start == info.start &&
-			    iter->info.end == info.end) {
-				list_del_init(&iter->list);
-				memcpy(&iter->info, &info, sizeof(info));
-				if (iter->data)
-					do_callback = 1;
-				else
-					iter->done = 1;
-				op = iter;
-				break;
-			}
-		}
+	op = plock_op_lookup(&info);
+	if (!op) {
+		spin_unlock(&ops_lock);
+		pr_debug("%s: no op %x %llx", __func__,
+			 info.fsid, (unsigned long long)info.number);
+		return count;
+	}
+
+	list_del_init(&op->list);
+	/* update set new fields by user space e.g. F_GETLK */
+	memcpy(&op->info, &info, sizeof(info));
+
+	/* check for async handling */
+	if (op->data) {
+		spin_unlock(&ops_lock);
+		dlm_plock_callback(op);
 	} else {
-		op = list_first_entry_or_null(&recv_list, struct plock_op,
-					      list);
-		if (op) {
-			list_del_init(&op->list);
-			memcpy(&op->info, &info, sizeof(info));
-			if (op->data)
-				do_callback = 1;
-			else
-				op->done = 1;
-		}
+		/* must be set under ops_lock, because retry in setlkw_wait()
+		 * if -ERESTARTSYS.
+		 */
+		op->done = 1;
+		spin_unlock(&ops_lock);
+		wake_up(&recv_wq);
 	}
-	spin_unlock(&ops_lock);
 
-	if (op) {
-		if (do_callback)
-			dlm_plock_callback(op);
-		else
-			wake_up(&recv_wq);
-	} else
-		pr_debug("%s: no op %x %llx", __func__,
-			 info.fsid, (unsigned long long)info.number);
 	return count;
 }
 
-- 
2.31.1