Skip to contentSkip to conversation

ACPI / debugger: Add kernel flushing support

PATCH Superseded Lv Zheng 2 participants 27 messages Jul 14, 2016 – Aug 17, 2016

Subsystems (4)
ACPI Driver core ABI/API ACPI component architecture (acpica)
lore
Share & tools
lore

Lv Zheng

author
#0 lore raw

cover ACPI / debugger: Add kernel flushing support

AML debugger is implemented in the kernel as a character device located in the debugfs. Currently, when its batch mode is used, the userspace tool needs to flush the logs/prompts remained in the kernel output buffer, this is implemented in an inefficient way in the userspace by polling the IO and reading everything out.

This patch introduces a kernel space flushing support, so that userspace can invoke fsync() to put the driver into a state waiting for new commands, all kernel space logs/prompts will be automatically discarded by fsync().

Lv Zheng (3):
debugfs: Add .fsync() callback proxy support ACPI / debugger: Add kernel flushing support tools/power/acpi/acpidbg: Use new flushing mechanism

 drivers/acpi/acpi_dbg.c                  |   94 ++++++++++++++++++++++++++++--
 fs/debugfs/file.c                        |    6 ++
 tools/power/acpi/tools/acpidbg/acpidbg.c |   49 ++--------------
 3 files changed, 101 insertions(+), 48 deletions(-)
Signature · 1 line
1.7.10
author
#1 lore raw

PATCH 1/3 debugfs: Add .fsync() callback proxy support

This patch adds .fsync() callback for debugfs files. ACPI AML debugger needs to implement this to flush cmds/logs.

Signed-off-by Lv Zheng <[email protected]>
Cc Greg Kroah-Hartman <[email protected]>
---
 fs/debugfs/file.c |    6 ++++++
 1 file changed, 6 insertions(+)
fs/debugfs/file.c +6 -0
index 592059f..f863a0c 100644
@@ -172,6 +172,10 @@ FULL_PROXY_FUNC(unlocked_ioctl, long, filp,
 		PROTO(struct file *filp, unsigned int cmd, unsigned long arg),
 		ARGS(filp, cmd, arg));
 
+FULL_PROXY_FUNC(fsync, int, filp,
+		PROTO(struct file *filp, loff_t start, loff_t end, int datasync),
+		ARGS(filp, start, end, datasync));
+
 static unsigned int full_proxy_poll(struct file *filp,
 				struct poll_table_struct *wait)
 {

@@ -226,6 +230,8 @@ static void __full_proxy_fops_init(struct file_operations *proxy_fops,
 		proxy_fops->poll = full_proxy_poll;
 	if (real_fops->unlocked_ioctl)
 		proxy_fops->unlocked_ioctl = full_proxy_unlocked_ioctl;
+	if (real_fops->fsync)
+		proxy_fops->fsync = full_proxy_fsync;
 }
 
 static int full_proxy_open(struct inode *inode, struct file *filp)
Signature · 1 line
1.7.10
author
#2 lore raw

RE: [PATCH 1/3] debugfs: Add .fsync() callback proxy support

Hi, Greg

Sorry for the noise.
It's better to use ioctl to implement this functionality. Please ignore this series.
It's already possible to implement ioctl for a debugfs file.

Thanks and best regards
-Lv

Quoted 40 lines

From: Zheng, Lv
Subject: [PATCH 1/3] debugfs: Add .fsync() callback proxy support

This patch adds .fsync() callback for debugfs files. ACPI AML debugger needs to implement this to flush cmds/logs.

Signed-off-by Lv Zheng <[email protected]>
Cc Greg Kroah-Hartman <[email protected]>
---
 fs/debugfs/file.c |    6 ++++++
 1 file changed, 6 insertions(+)
fs/debugfs/file.c +4 -0
index 592059f..f863a0c 100644
@@ -172,6 +172,10 @@ FULL_PROXY_FUNC(unlocked_ioctl, long, filp,
 		PROTO(struct file *filp, unsigned int cmd, unsigned long
 arg),
 		ARGS(filp, cmd, arg));
 
+FULL_PROXY_FUNC(fsync, int, filp,
+		PROTO(struct file *filp, loff_t start, loff_t end, int datasync),
+		ARGS(filp, start, end, datasync));
+
 static unsigned int full_proxy_poll(struct file *filp,
 				struct poll_table_struct *wait)

{

fs/debugfs/file.c +2 -0
@@ -226,6 +230,8 @@ static void __full_proxy_fops_init(struct
 file_operations *proxy_fops,
 		proxy_fops->poll = full_proxy_poll;
 	if (real_fops->unlocked_ioctl)
 		proxy_fops->unlocked_ioctl = full_proxy_unlocked_ioctl;
+	if (real_fops->fsync)
+		proxy_fops->fsync = full_proxy_fsync;
 }

static int full_proxy_open(struct inode *inode, struct file *filp)
--
1.7.10

author
#3 lore raw

PATCH 2/3 ACPI / debugger: Add kernel flushing support

In reply to Lv Zheng

This patch adds debugger log flushing support in kernel via .fsync() callback. The in-kernel flushing is more efficient, because it reduces useless log IOs by bypassing log user_read/kern_write during the flush period.

Signed-off-by Lv Zheng <[email protected]>
---
 drivers/acpi/acpi_dbg.c |   94 +++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 90 insertions(+), 4 deletions(-)
drivers/acpi/acpi_dbg.c +90 -4
index dee8692..90d6922 100644
@@ -46,6 +46,8 @@
 #define ACPI_AML_KERN		(ACPI_AML_IN_KERN | ACPI_AML_OUT_KERN)
 #define ACPI_AML_BUSY		(ACPI_AML_USER | ACPI_AML_KERN)
 #define ACPI_AML_OPEN		(ACPI_AML_OPENED | ACPI_AML_CLOSED)
+#define ACPI_AML_FLUSHING_LOG	0x0040 /* flushing log output */
+#define ACPI_AML_WAITING_CMD	0x0080 /* waiting for cmd input */
 
 struct acpi_aml_io {
 	wait_queue_head_t wait;

@@ -120,6 +122,20 @@ static inline bool __acpi_aml_busy(void)
 	return false;
 }
 
+static inline bool __acpi_aml_waiting_cmd(void)
+{
+	if (acpi_aml_io.flags & ACPI_AML_WAITING_CMD)
+		return true;
+	return false;
+}
+
+static inline bool __acpi_aml_flushing_log(void)
+{
+	if (acpi_aml_io.flags & ACPI_AML_FLUSHING_LOG)
+		return true;
+	return false;
+}
+
 static inline bool __acpi_aml_opened(void)
 {
 	if (acpi_aml_io.flags & ACPI_AML_OPEN)

@@ -152,6 +168,26 @@ static bool acpi_aml_busy(void)
 	return ret;
 }
 
+static inline bool acpi_aml_waiting_cmd(void)
+{
+	bool ret;
+
+	mutex_lock(&acpi_aml_io.lock);
+	ret = __acpi_aml_waiting_cmd();
+	mutex_unlock(&acpi_aml_io.lock);
+	return ret;
+}
+
+static inline bool acpi_aml_flushing_log(void)
+{
+	bool ret;
+
+	mutex_lock(&acpi_aml_io.lock);
+	ret = __acpi_aml_flushing_log();
+	mutex_unlock(&acpi_aml_io.lock);
+	return ret;
+}
+
 static bool acpi_aml_used(void)
 {
 	bool ret;

@@ -183,7 +219,8 @@ static bool acpi_aml_kern_writable(void)
 
 	mutex_lock(&acpi_aml_io.lock);
 	ret = !__acpi_aml_access_ok(ACPI_AML_OUT_KERN) ||
-	      __acpi_aml_writable(&acpi_aml_io.out_crc, ACPI_AML_OUT_KERN);
+	      __acpi_aml_writable(&acpi_aml_io.out_crc, ACPI_AML_OUT_KERN) ||
+	      __acpi_aml_flushing_log();
 	mutex_unlock(&acpi_aml_io.lock);
 	return ret;
 }

@@ -264,6 +301,9 @@ static int acpi_aml_write_kern(const char *buf, int len)
 	int n;
 	char *p;
 
+	if (acpi_aml_flushing_log())
+		return len;
+
 	ret = acpi_aml_lock_write(crc, ACPI_AML_OUT_KERN);
 	if (ret < 0)
 		return ret;

@@ -458,9 +498,18 @@ static int acpi_aml_wait_command_ready(bool single_step,
 	else
 		acpi_os_printf("\n%1c ", ACPI_DEBUGGER_COMMAND_PROMPT);
 
+	mutex_lock(&acpi_aml_io.lock);
+	acpi_aml_io.flags |= ACPI_AML_WAITING_CMD;
+	wake_up_interruptible(&acpi_aml_io.wait);
+	mutex_unlock(&acpi_aml_io.lock);
+
 	status = acpi_os_get_line(buffer, length, NULL);
 	if (ACPI_FAILURE(status))
 		return -EINVAL;
+
+	mutex_lock(&acpi_aml_io.lock);
+	acpi_aml_io.flags &= ~ACPI_AML_WAITING_CMD;
+	mutex_unlock(&acpi_aml_io.lock);
 	return 0;
 }
 

@@ -593,9 +642,11 @@ static int acpi_aml_read_user(char __user *buf, int len)
 	smp_rmb();
 	p = &crc->buf[crc->tail];
 	n = min(len, circ_count_to_end(crc));
-	if (copy_to_user(buf, p, n)) {
-		ret = -EFAULT;
-		goto out;
+	if (!acpi_aml_flushing_log()) {
+		if (copy_to_user(buf, p, n)) {
+			ret = -EFAULT;
+			goto out;
+		}
 	}
 	/* sync tail after removing logs */
 	smp_mb();

@@ -731,10 +782,45 @@ static unsigned int acpi_aml_poll(struct file *file, poll_table *wait)
 	return masks;
 }
 
+static int acpi_aml_flush(void)
+{
+	int ret;
+
+	/*
+	 * Discard output buffer and put the driver into a state waiting
+	 * for the new user input.
+	 */
+	mutex_lock(&acpi_aml_io.lock);
+	acpi_aml_io.flags |= ACPI_AML_FLUSHING_LOG;
+	mutex_unlock(&acpi_aml_io.lock);
+
+	ret = wait_event_interruptible(acpi_aml_io.wait,
+		acpi_aml_waiting_cmd());
+	(void)acpi_aml_read_user(NULL, ACPI_AML_BUF_SIZE);
+
+	mutex_lock(&acpi_aml_io.lock);
+	acpi_aml_io.flags &= ~ACPI_AML_FLUSHING_LOG;
+	mutex_unlock(&acpi_aml_io.lock);
+	return ret;
+}
+
+static int acpi_aml_fsync(struct file *file,
+			  loff_t start, loff_t end, int datasync)
+{
+	struct inode *inode = file_inode(file);
+	int ret;
+
+	inode_lock(inode);
+	ret = acpi_aml_flush();
+	inode_unlock(inode);
+	return ret;
+}
+
 static const struct file_operations acpi_aml_operations = {
 	.read		= acpi_aml_read,
 	.write		= acpi_aml_write,
 	.poll		= acpi_aml_poll,
+	.fsync		= acpi_aml_fsync,
 	.open		= acpi_aml_open,
 	.release	= acpi_aml_release,
 	.llseek		= generic_file_llseek,
Signature · 1 line
1.7.10
author
#4 lore raw

PATCH 3/3 tools/power/acpi/acpidbg: Use new flushing mechanism

In reply to Lv Zheng

This patch converts tools/power/acpi/tools/acpidbg/acpidbg to use the new flushing mechanism.

Signed-off-by Lv Zheng <[email protected]>
---
 tools/power/acpi/tools/acpidbg/acpidbg.c |   49 +++---------------------------
 1 file changed, 5 insertions(+), 44 deletions(-)
tools/power/acpi/tools/acpidbg/acpidbg.c +5 -44
index a88ac45..770b556 100644
@@ -83,7 +83,6 @@ static const char *acpi_aml_file_path = ACPI_AML_FILE;
 static unsigned long acpi_aml_mode = ACPI_AML_INTERACTIVE;
 static bool acpi_aml_exit;
 
-static bool acpi_aml_batch_drain;
 static unsigned long acpi_aml_batch_state;
 static char acpi_aml_batch_prompt;
 static char acpi_aml_batch_roll;

@@ -239,11 +238,9 @@ static int acpi_aml_write_batch_log(int fd, struct circ_buf *crc)
 
 	p = &crc->buf[crc->tail];
 	len = circ_count_to_end(crc);
-	if (!acpi_aml_batch_drain) {
-		len = write(fd, p, len);
-		if (len < 0)
-			perror("write");
-	}
+	len = write(fd, p, len);
+	if (len < 0)
+		perror("write");
 	if (len > 0)
 		crc->tail = (crc->tail + len) & (ACPI_AML_BUF_SIZE - 1);
 	return len;

@@ -270,10 +267,7 @@ static void acpi_aml_loop(int fd)
 	if (acpi_aml_mode == ACPI_AML_BATCH) {
 		acpi_aml_log_state = ACPI_AML_LOG_START;
 		acpi_aml_batch_pos = acpi_aml_batch_cmd;
-		if (acpi_aml_batch_drain)
-			acpi_aml_batch_state = ACPI_AML_BATCH_READ_LOG;
-		else
-			acpi_aml_batch_state = ACPI_AML_BATCH_WRITE_CMD;
+		acpi_aml_batch_state = ACPI_AML_BATCH_WRITE_CMD;
 	}
 	acpi_aml_exit = false;
 	while (!acpi_aml_exit) {

@@ -330,39 +324,6 @@ static void acpi_aml_loop(int fd)
 	}
 }
 
-static bool acpi_aml_readable(int fd)
-{
-	fd_set rfds;
-	struct timeval tv;
-	int ret;
-	int maxfd = 0;
-
-	tv.tv_sec = 0;
-	tv.tv_usec = ACPI_AML_USEC_PEEK;
-	FD_ZERO(&rfds);
-	maxfd = acpi_aml_set_fd(fd, maxfd, &rfds);
-	ret = select(maxfd+1, &rfds, NULL, NULL, &tv);
-	if (ret < 0)
-		perror("select");
-	if (ret > 0 && FD_ISSET(fd, &rfds))
-		return true;
-	return false;
-}
-
-/*
- * This is a userspace IO flush implementation, replying on the prompt
- * characters and can be turned into a flush() call after kernel implements
- * .flush() filesystem operation.
- */
-static void acpi_aml_flush(int fd)
-{
-	while (acpi_aml_readable(fd)) {
-		acpi_aml_batch_drain = true;
-		acpi_aml_loop(fd);
-		acpi_aml_batch_drain = false;
-	}
-}
-
 void usage(FILE *file, char *progname)
 {
 	fprintf(file, "usage: %s [-b cmd] [-f file] [-h]\n", progname);

@@ -426,7 +387,7 @@ int main(int argc, char **argv)
 	acpi_aml_set_fl(STDOUT_FILENO, O_NONBLOCK);
 
 	if (acpi_aml_mode == ACPI_AML_BATCH)
-		acpi_aml_flush(fd);
+		fsync(fd);
 	acpi_aml_loop(fd);
 
 exit:
Signature · 1 line
1.7.10
author
#5 lore raw

v2 cover ACPI / debugger: Add kernel flushing support

In reply to Lv Zheng

AML debugger is implemented in the kernel as a character device located in the debugfs. Currently, when its batch mode is used, the userspace tool needs to flush the logs/prompts remained in the kernel output buffer, this is implemented in an inefficient way in the userspace by polling the IO and reading everything out.

This patch introduces a kernel space flushing support, so that userspace can invoke ioctl() to put the driver into a state waiting for new commands, all kernel space logs/prompts will be automatically discarded by ioctl().

Lv Zheng (2):
ACPI / debugger: Add kernel flushing support tools/power/acpi/acpidbg: Use new flushing mechanism

 drivers/acpi/acpi_dbg.c                  |   98 ++++++++++++++++++++++++++++--
 include/linux/acpi-ioctls.h              |   21 +++++++
 tools/power/acpi/tools/acpidbg/acpidbg.c |   51 +++-------------
 3 files changed, 122 insertions(+), 48 deletions(-)
 create mode 100644 include/linux/acpi-ioctls.h
Signature · 1 line
1.7.10
author
#6 lore raw

PATCH v2 1/2 ACPI / debugger: Add kernel flushing support

This patch adds debugger log flushing support in kernel via .ioctl() callback. The in-kernel flushing is more efficient, because it reduces useless log IOs by bypassing log user_read/kern_write during the flush period.

Signed-off-by Lv Zheng <[email protected]>
---
 drivers/acpi/acpi_dbg.c     |   98 +++++++++++++++++++++++++++++++++++++++++--
 include/linux/acpi-ioctls.h |   21 ++++++++++
 2 files changed, 115 insertions(+), 4 deletions(-)
 create mode 100644 include/linux/acpi-ioctls.h
drivers/acpi/acpi_dbg.c +94 -4
index dee8692..6ac1388 100644
@@ -22,6 +22,7 @@
 #include <linux/debugfs.h>
 #include <linux/circ_buf.h>
 #include <linux/acpi.h>
+#include <linux/acpi-ioctls.h>
 #include "internal.h"
 
 #define ACPI_AML_BUF_ALIGN	(sizeof (acpi_size))

@@ -46,6 +47,8 @@
 #define ACPI_AML_KERN		(ACPI_AML_IN_KERN | ACPI_AML_OUT_KERN)
 #define ACPI_AML_BUSY		(ACPI_AML_USER | ACPI_AML_KERN)
 #define ACPI_AML_OPEN		(ACPI_AML_OPENED | ACPI_AML_CLOSED)
+#define ACPI_AML_FLUSHING_LOG	0x0040 /* flushing log output */
+#define ACPI_AML_WAITING_CMD	0x0080 /* waiting for cmd input */
 
 struct acpi_aml_io {
 	wait_queue_head_t wait;

@@ -120,6 +123,20 @@ static inline bool __acpi_aml_busy(void)
 	return false;
 }
 
+static inline bool __acpi_aml_waiting_cmd(void)
+{
+	if (acpi_aml_io.flags & ACPI_AML_WAITING_CMD)
+		return true;
+	return false;
+}
+
+static inline bool __acpi_aml_flushing_log(void)
+{
+	if (acpi_aml_io.flags & ACPI_AML_FLUSHING_LOG)
+		return true;
+	return false;
+}
+
 static inline bool __acpi_aml_opened(void)
 {
 	if (acpi_aml_io.flags & ACPI_AML_OPEN)

@@ -152,6 +169,26 @@ static bool acpi_aml_busy(void)
 	return ret;
 }
 
+static inline bool acpi_aml_waiting_cmd(void)
+{
+	bool ret;
+
+	mutex_lock(&acpi_aml_io.lock);
+	ret = __acpi_aml_waiting_cmd();
+	mutex_unlock(&acpi_aml_io.lock);
+	return ret;
+}
+
+static inline bool acpi_aml_flushing_log(void)
+{
+	bool ret;
+
+	mutex_lock(&acpi_aml_io.lock);
+	ret = __acpi_aml_flushing_log();
+	mutex_unlock(&acpi_aml_io.lock);
+	return ret;
+}
+
 static bool acpi_aml_used(void)
 {
 	bool ret;

@@ -183,7 +220,8 @@ static bool acpi_aml_kern_writable(void)
 
 	mutex_lock(&acpi_aml_io.lock);
 	ret = !__acpi_aml_access_ok(ACPI_AML_OUT_KERN) ||
-	      __acpi_aml_writable(&acpi_aml_io.out_crc, ACPI_AML_OUT_KERN);
+	      __acpi_aml_writable(&acpi_aml_io.out_crc, ACPI_AML_OUT_KERN) ||
+	      __acpi_aml_flushing_log();
 	mutex_unlock(&acpi_aml_io.lock);
 	return ret;
 }

@@ -264,6 +302,9 @@ static int acpi_aml_write_kern(const char *buf, int len)
 	int n;
 	char *p;
 
+	if (acpi_aml_flushing_log())
+		return len;
+
 	ret = acpi_aml_lock_write(crc, ACPI_AML_OUT_KERN);
 	if (ret < 0)
 		return ret;

@@ -458,9 +499,18 @@ static int acpi_aml_wait_command_ready(bool single_step,
 	else
 		acpi_os_printf("\n%1c ", ACPI_DEBUGGER_COMMAND_PROMPT);
 
+	mutex_lock(&acpi_aml_io.lock);
+	acpi_aml_io.flags |= ACPI_AML_WAITING_CMD;
+	wake_up_interruptible(&acpi_aml_io.wait);
+	mutex_unlock(&acpi_aml_io.lock);
+
 	status = acpi_os_get_line(buffer, length, NULL);
 	if (ACPI_FAILURE(status))
 		return -EINVAL;
+
+	mutex_lock(&acpi_aml_io.lock);
+	acpi_aml_io.flags &= ~ACPI_AML_WAITING_CMD;
+	mutex_unlock(&acpi_aml_io.lock);
 	return 0;
 }
 

@@ -593,9 +643,11 @@ static int acpi_aml_read_user(char __user *buf, int len)
 	smp_rmb();
 	p = &crc->buf[crc->tail];
 	n = min(len, circ_count_to_end(crc));
-	if (copy_to_user(buf, p, n)) {
-		ret = -EFAULT;
-		goto out;
+	if (!acpi_aml_flushing_log()) {
+		if (copy_to_user(buf, p, n)) {
+			ret = -EFAULT;
+			goto out;
+		}
 	}
 	/* sync tail after removing logs */
 	smp_mb();

@@ -731,10 +783,48 @@ static unsigned int acpi_aml_poll(struct file *file, poll_table *wait)
 	return masks;
 }
 
+static int acpi_aml_flush(void)
+{
+	int ret;
+
+	/*
+	 * Discard output buffer and put the driver into a state waiting
+	 * for the new user input.
+	 */
+	mutex_lock(&acpi_aml_io.lock);
+	acpi_aml_io.flags |= ACPI_AML_FLUSHING_LOG;
+	mutex_unlock(&acpi_aml_io.lock);
+
+	ret = wait_event_interruptible(acpi_aml_io.wait,
+		acpi_aml_waiting_cmd());
+	(void)acpi_aml_read_user(NULL, ACPI_AML_BUF_SIZE);
+
+	mutex_lock(&acpi_aml_io.lock);
+	acpi_aml_io.flags &= ~ACPI_AML_FLUSHING_LOG;
+	mutex_unlock(&acpi_aml_io.lock);
+	return ret;
+}
+
+static long acpi_aml_ioctl(struct file *file,
+			   unsigned int cmd, unsigned long arg)
+{
+	long ret = -EINVAL;
+
+	switch (cmd) {
+	case ACPI_IOCTL_DEBUGGER_FLUSH:
+		ret = acpi_aml_flush();
+		break;
+	default:
+		break;
+	}
+	return ret;
+}
+
 static const struct file_operations acpi_aml_operations = {
 	.read		= acpi_aml_read,
 	.write		= acpi_aml_write,
 	.poll		= acpi_aml_poll,
+	.unlocked_ioctl	= acpi_aml_ioctl,
 	.open		= acpi_aml_open,
 	.release	= acpi_aml_release,
 	.llseek		= generic_file_llseek,
include/linux/acpi-ioctls.h new file +21 -0
new file mode 100644
index 0000000..56b8170
@@ -0,0 +1,21 @@
+/*
+ * ACPI IOCTL collections
+ *
+ * Copyright (C) 2016, Intel Corporation
+ * Authors: Lv Zheng <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef _LINUX_ACPI_IOCTLS_H
+#define _LINUX_ACPI_IOCTLS_H
+
+#include <linux/ioctl.h>
+
+#define ACPI_IOCTL_IDENT		'a'
+
+#define ACPI_IOCTL_DEBUGGER_FLUSH	_IO(ACPI_IOCTL_IDENT, 0x80)
+
+#endif /* _LINUX_ACPI_IOCTLS_H */
Signature · 1 line
1.7.10
#7 lore raw
Quoted 6 lines · Lv Zheng

This patch adds debugger log flushing support in kernel via .ioctl() callback. The in-kernel flushing is more efficient, because it reduces useless log IOs by bypassing log user_read/kern_write during the flush period.

Signed-off-by Lv Zheng <[email protected]>

Overall, this is an optimization, right?

So is adding a new IOCTL really worth it?

Quoted 11 lines
---
 drivers/acpi/acpi_dbg.c     |   98 +++++++++++++++++++++++++++++++++++++++++--
 include/linux/acpi-ioctls.h |   21 ++++++++++
 2 files changed, 115 insertions(+), 4 deletions(-)
 create mode 100644 include/linux/acpi-ioctls.h
drivers/acpi/acpi_dbg.c +0 -0
index dee8692..6ac1388 100644

drivers/acpi/acpi_dbg.c line 25

Quoted 4 lines
drivers/acpi/acpi_dbg.c +1 -0
@@ -22,6 +22,7 @@
 #include <linux/debugfs.h>
 #include <linux/circ_buf.h>
 #include <linux/acpi.h>
+#include <linux/acpi-ioctls.h>

That should go into the uapi directory at least.

Quoted 13 lines
drivers/acpi/acpi_dbg.c +2 -0
 #include "internal.h"
 
 #define ACPI_AML_BUF_ALIGN	(sizeof (acpi_size))

@@ -46,6 +47,8 @@
 #define ACPI_AML_KERN		(ACPI_AML_IN_KERN | ACPI_AML_OUT_KERN)
 #define ACPI_AML_BUSY		(ACPI_AML_USER | ACPI_AML_KERN)
 #define ACPI_AML_OPEN		(ACPI_AML_OPENED | ACPI_AML_CLOSED)
+#define ACPI_AML_FLUSHING_LOG	0x0040 /* flushing log output */
+#define ACPI_AML_WAITING_CMD	0x0080 /* waiting for cmd input */
 
 struct acpi_aml_io {
 	wait_queue_head_t wait;

drivers/acpi/acpi_dbg.c static inline bool __acpi_aml_busy(void) · line 130

Quoted 8 lines
drivers/acpi/acpi_dbg.c +5 -0
@@ -120,6 +123,20 @@ static inline bool __acpi_aml_busy(void)
 	return false;
 }
 
+static inline bool __acpi_aml_waiting_cmd(void)
+{
+	if (acpi_aml_io.flags & ACPI_AML_WAITING_CMD)
+		return true;
+	return false;

Oh well.

What about

	return !!(acpi_aml_io.flags & ACPI_AML_WAITING_CMD);

drivers/acpi/acpi_dbg.c static inline bool __acpi_aml_busy(void) · line 137

Quoted 7 lines
drivers/acpi/acpi_dbg.c +7 -0
+}
+
+static inline bool __acpi_aml_flushing_log(void)
+{
+	if (acpi_aml_io.flags & ACPI_AML_FLUSHING_LOG)
+		return true;
+	return false;

And analogously here?

Quoted 115 lines
drivers/acpi/acpi_dbg.c +66 -4
+}
+
 static inline bool __acpi_aml_opened(void)
 {
 	if (acpi_aml_io.flags & ACPI_AML_OPEN)

@@ -152,6 +169,26 @@ static bool acpi_aml_busy(void)
 	return ret;
 }
 
+static inline bool acpi_aml_waiting_cmd(void)
+{
+	bool ret;
+
+	mutex_lock(&acpi_aml_io.lock);
+	ret = __acpi_aml_waiting_cmd();
+	mutex_unlock(&acpi_aml_io.lock);
+	return ret;
+}
+
+static inline bool acpi_aml_flushing_log(void)
+{
+	bool ret;
+
+	mutex_lock(&acpi_aml_io.lock);
+	ret = __acpi_aml_flushing_log();
+	mutex_unlock(&acpi_aml_io.lock);
+	return ret;
+}
+
 static bool acpi_aml_used(void)
 {
 	bool ret;

@@ -183,7 +220,8 @@ static bool acpi_aml_kern_writable(void)
 
 	mutex_lock(&acpi_aml_io.lock);
 	ret = !__acpi_aml_access_ok(ACPI_AML_OUT_KERN) ||
-	      __acpi_aml_writable(&acpi_aml_io.out_crc, ACPI_AML_OUT_KERN);
+	      __acpi_aml_writable(&acpi_aml_io.out_crc, ACPI_AML_OUT_KERN) ||
+	      __acpi_aml_flushing_log();
 	mutex_unlock(&acpi_aml_io.lock);
 	return ret;
 }

@@ -264,6 +302,9 @@ static int acpi_aml_write_kern(const char *buf, int len)
 	int n;
 	char *p;
 
+	if (acpi_aml_flushing_log())
+		return len;
+
 	ret = acpi_aml_lock_write(crc, ACPI_AML_OUT_KERN);
 	if (ret < 0)
 		return ret;

@@ -458,9 +499,18 @@ static int acpi_aml_wait_command_ready(bool single_step,
 	else
 		acpi_os_printf("\n%1c ", ACPI_DEBUGGER_COMMAND_PROMPT);
 
+	mutex_lock(&acpi_aml_io.lock);
+	acpi_aml_io.flags |= ACPI_AML_WAITING_CMD;
+	wake_up_interruptible(&acpi_aml_io.wait);
+	mutex_unlock(&acpi_aml_io.lock);
+
 	status = acpi_os_get_line(buffer, length, NULL);
 	if (ACPI_FAILURE(status))
 		return -EINVAL;
+
+	mutex_lock(&acpi_aml_io.lock);
+	acpi_aml_io.flags &= ~ACPI_AML_WAITING_CMD;
+	mutex_unlock(&acpi_aml_io.lock);
 	return 0;
 }
 

@@ -593,9 +643,11 @@ static int acpi_aml_read_user(char __user *buf, int len)
 	smp_rmb();
 	p = &crc->buf[crc->tail];
 	n = min(len, circ_count_to_end(crc));
-	if (copy_to_user(buf, p, n)) {
-		ret = -EFAULT;
-		goto out;
+	if (!acpi_aml_flushing_log()) {
+		if (copy_to_user(buf, p, n)) {
+			ret = -EFAULT;
+			goto out;
+		}
 	}
 	/* sync tail after removing logs */
 	smp_mb();

@@ -731,10 +783,48 @@ static unsigned int acpi_aml_poll(struct file *file, poll_table *wait)
 	return masks;
 }
 
+static int acpi_aml_flush(void)
+{
+	int ret;
+
+	/*
+	 * Discard output buffer and put the driver into a state waiting
+	 * for the new user input.
+	 */
+	mutex_lock(&acpi_aml_io.lock);
+	acpi_aml_io.flags |= ACPI_AML_FLUSHING_LOG;
+	mutex_unlock(&acpi_aml_io.lock);
+
+	ret = wait_event_interruptible(acpi_aml_io.wait,
+		acpi_aml_waiting_cmd());
+	(void)acpi_aml_read_user(NULL, ACPI_AML_BUF_SIZE);
+
+	mutex_lock(&acpi_aml_io.lock);
+	acpi_aml_io.flags &= ~ACPI_AML_FLUSHING_LOG;
+	mutex_unlock(&acpi_aml_io.lock);
+	return ret;
+}
+
+static long acpi_aml_ioctl(struct file *file,
+			   unsigned int cmd, unsigned long arg)
+{

drivers/acpi/acpi_dbg.c static unsigned int acpi_aml_poll(struct file *file, poll_table *wait) · line 820

Quoted 10 lines
drivers/acpi/acpi_dbg.c +10 -0
+	long ret = -EINVAL;
+
+	switch (cmd) {
+	case ACPI_IOCTL_DEBUGGER_FLUSH:
+		ret = acpi_aml_flush();
+		break;
+	default:
+		break;
+	}
+	return ret;

	return cmd == ACPI_IOCTL_DEBUGGER_FLUSH ? acpi_aml_flush() : -EINVAL;

include/linux/acpi-ioctls.h

Quoted 15 lines
drivers/acpi/acpi_dbg.c +3 -0
+}
+
 static const struct file_operations acpi_aml_operations = {
 	.read		= acpi_aml_read,
 	.write		= acpi_aml_write,
 	.poll		= acpi_aml_poll,
+	.unlocked_ioctl	= acpi_aml_ioctl,
 	.open		= acpi_aml_open,
 	.release	= acpi_aml_release,
 	.llseek		= generic_file_llseek,
include/linux/acpi-ioctls.h new file +0 -0
new file mode 100644
index 0000000..56b8170

include/uapi/linux/

Quoted 13 lines
include/linux/acpi-ioctls.h new file +11 -0
@@ -0,0 +1,21 @@
+/*
+ * ACPI IOCTL collections
+ *
+ * Copyright (C) 2016, Intel Corporation
+ * Authors: Lv Zheng <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+

include/linux/acpi-ioctls.h line 21

Quoted 10 lines
include/linux/acpi-ioctls.h new file +10 -0
+#ifndef _LINUX_ACPI_IOCTLS_H
+#define _LINUX_ACPI_IOCTLS_H
+
+#include <linux/ioctl.h>
+
+#define ACPI_IOCTL_IDENT		'a'
+
+#define ACPI_IOCTL_DEBUGGER_FLUSH	_IO(ACPI_IOCTL_IDENT, 0x80)
+
+#endif /* _LINUX_ACPI_IOCTLS_H */

Plus patches that change the ABI should be CCed to the ABI review list for, well, review.

Thanks,
Rafael

author
#8 lore raw

Hi,

Quoted 10 lines

From: [email protected] [mailto:linux-acpi- [email protected]] On Behalf Of Rafael J. Wysocki
Subject: Re: [PATCH v2 1/2] ACPI / debugger: Add kernel flushing support

Quoted 5 lines · Lv Zheng

This patch adds debugger log flushing support in kernel via .ioctl() callback. The in-kernel flushing is more efficient, because it reduces useless log IOs by bypassing log user_read/kern_write during the flush period.

Quoted 5 lines
Quoted 1 line
Signed-off-by Lv Zheng <[email protected]>

Overall, this is an optimization, right?

So is adding a new IOCTL really worth it?

[Lv Zheng]
I was thinking I could use fsync.
But most of the kernel drivers implement fsync to flush data to the storage device. And tty uses IOCTL to flush the input/output streams.
So I changed my mind to use ioctl.

Quoted 8 lines
Quoted 3 lines
---
 drivers/acpi/acpi_dbg.c     |   98

+++++++++++++++++++++++++++++++++++++++++--

Quoted 4 lines
 include/linux/acpi-ioctls.h |   21 ++++++++++
 2 files changed, 115 insertions(+), 4 deletions(-)
 create mode 100644 include/linux/acpi-ioctls.h
Quoted 11 lines

drivers/acpi/acpi_dbg.c line 25

Quoted 9 lines
drivers/acpi/acpi_dbg.c +1 -0
index dee8692..6ac1388 100644
@@ -22,6 +22,7 @@
 #include <linux/debugfs.h>
 #include <linux/circ_buf.h>
 #include <linux/acpi.h>
+#include <linux/acpi-ioctls.h>

That should go into the uapi directory at least.

[Lv Zheng]
OK.
I'll check it.

Quoted 29 lines

drivers/acpi/acpi_dbg.c line 47

Quoted 6 lines
drivers/acpi/acpi_dbg.c +0 -0
 #include "internal.h"
 
 #define ACPI_AML_BUF_ALIGN	(sizeof (acpi_size))

@@ -46,6 +47,8 @@
 #define ACPI_AML_KERN		(ACPI_AML_IN_KERN |

ACPI_AML_OUT_KERN)

drivers/acpi/acpi_dbg.c line 48

Quoted 1 line
drivers/acpi/acpi_dbg.c +0 -0
 #define ACPI_AML_BUSY		(ACPI_AML_USER |

ACPI_AML_KERN)

drivers/acpi/acpi_dbg.c line 49

Quoted 1 line
drivers/acpi/acpi_dbg.c +0 -0
 #define ACPI_AML_OPEN		(ACPI_AML_OPENED |

ACPI_AML_CLOSED)

drivers/acpi/acpi_dbg.c line 50

Quoted 1 line
drivers/acpi/acpi_dbg.c +1 -0
+#define ACPI_AML_FLUSHING_LOG	0x0040 /* flushing log

output */

drivers/acpi/acpi_dbg.c static inline bool __acpi_aml_busy(void) · line 130

Quoted 13 lines
drivers/acpi/acpi_dbg.c +6 -0
+#define ACPI_AML_WAITING_CMD	0x0080 /* waiting for cmd input */
 
 struct acpi_aml_io {
 	wait_queue_head_t wait;

@@ -120,6 +123,20 @@ static inline bool __acpi_aml_busy(void)
 	return false;
 }
 
+static inline bool __acpi_aml_waiting_cmd(void)
+{
+	if (acpi_aml_io.flags & ACPI_AML_WAITING_CMD)
+		return true;
+	return false;

Oh well.

Quoted 1 line

What about

[Lv Zheng]
OK.

Quoted 12 lines
	return !!(acpi_aml_io.flags & ACPI_AML_WAITING_CMD);

drivers/acpi/acpi_dbg.c static inline bool __acpi_aml_busy(void) · line 137

Quoted 7 lines
drivers/acpi/acpi_dbg.c +7 -0
+}
+
+static inline bool __acpi_aml_flushing_log(void)
+{
+	if (acpi_aml_io.flags & ACPI_AML_FLUSHING_LOG)
+		return true;
+	return false;

And analogously here?

[Lv Zheng]
OK.

Quoted 135 lines
Quoted 34 lines
drivers/acpi/acpi_dbg.c +22 -0
+}
+
 static inline bool __acpi_aml_opened(void)
 {
 	if (acpi_aml_io.flags & ACPI_AML_OPEN)

@@ -152,6 +169,26 @@ static bool acpi_aml_busy(void)
 	return ret;
 }
 
+static inline bool acpi_aml_waiting_cmd(void)
+{
+	bool ret;
+
+	mutex_lock(&acpi_aml_io.lock);
+	ret = __acpi_aml_waiting_cmd();
+	mutex_unlock(&acpi_aml_io.lock);
+	return ret;
+}
+
+static inline bool acpi_aml_flushing_log(void)
+{
+	bool ret;
+
+	mutex_lock(&acpi_aml_io.lock);
+	ret = __acpi_aml_flushing_log();
+	mutex_unlock(&acpi_aml_io.lock);
+	return ret;
+}
+
 static bool acpi_aml_used(void)
 {
 	bool ret;

drivers/acpi/acpi_dbg.c static bool acpi_aml_kern_writable(void) · line 186

Quoted 4 lines
drivers/acpi/acpi_dbg.c +0 -1
@@ -183,7 +220,8 @@ static bool acpi_aml_kern_writable(void)
 
 	mutex_lock(&acpi_aml_io.lock);
 	ret = !__acpi_aml_access_ok(ACPI_AML_OUT_KERN) ||
-	      __acpi_aml_writable(&acpi_aml_io.out_crc,

ACPI_AML_OUT_KERN);

drivers/acpi/acpi_dbg.c static bool acpi_aml_kern_writable(void) · line 223

Quoted 1 line
drivers/acpi/acpi_dbg.c +1 -0
+	      __acpi_aml_writable(&acpi_aml_io.out_crc,

ACPI_AML_OUT_KERN) ||

drivers/acpi/acpi_dbg.c static int acpi_aml_write_kern(const char *buf,

Quoted 5 lines
drivers/acpi/acpi_dbg.c +1 -0
+	      __acpi_aml_flushing_log();
 	mutex_unlock(&acpi_aml_io.lock);
 	return ret;
 }

@@ -264,6 +302,9 @@ static int acpi_aml_write_kern(const char *buf,

int len)

drivers/acpi/acpi_dbg.c static int acpi_aml_wait_command_ready(bool

Quoted 10 lines
drivers/acpi/acpi_dbg.c +3 -0
 	int n;
 	char *p;
 
+	if (acpi_aml_flushing_log())
+		return len;
+
 	ret = acpi_aml_lock_write(crc, ACPI_AML_OUT_KERN);
 	if (ret < 0)
 		return ret;

@@ -458,9 +499,18 @@ static int acpi_aml_wait_command_ready(bool

single_step,

drivers/acpi/acpi_dbg.c static int acpi_aml_wait_command_ready(bool · line 500

Quoted 2 lines
drivers/acpi/acpi_dbg.c +0 -0
 	else
 		acpi_os_printf("\n%1c ",

ACPI_DEBUGGER_COMMAND_PROMPT);

drivers/acpi/acpi_dbg.c static int acpi_aml_read_user(char __user *buf,

Quoted 17 lines
drivers/acpi/acpi_dbg.c +9 -0
+	mutex_lock(&acpi_aml_io.lock);
+	acpi_aml_io.flags |= ACPI_AML_WAITING_CMD;
+	wake_up_interruptible(&acpi_aml_io.wait);
+	mutex_unlock(&acpi_aml_io.lock);
+
 	status = acpi_os_get_line(buffer, length, NULL);
 	if (ACPI_FAILURE(status))
 		return -EINVAL;
+
+	mutex_lock(&acpi_aml_io.lock);
+	acpi_aml_io.flags &= ~ACPI_AML_WAITING_CMD;
+	mutex_unlock(&acpi_aml_io.lock);
 	return 0;
 }
 

@@ -593,9 +643,11 @@ static int acpi_aml_read_user(char __user *buf,

int len)

drivers/acpi/acpi_dbg.c static unsigned int acpi_aml_poll(struct file

Quoted 15 lines
drivers/acpi/acpi_dbg.c +5 -3
 	smp_rmb();
 	p = &crc->buf[crc->tail];
 	n = min(len, circ_count_to_end(crc));
-	if (copy_to_user(buf, p, n)) {
-		ret = -EFAULT;
-		goto out;
+	if (!acpi_aml_flushing_log()) {
+		if (copy_to_user(buf, p, n)) {
+			ret = -EFAULT;
+			goto out;
+		}
 	}
 	/* sync tail after removing logs */
 	smp_mb();

@@ -731,10 +783,48 @@ static unsigned int acpi_aml_poll(struct file

*file, poll_table *wait)

Quoted 28 lines
drivers/acpi/acpi_dbg.c +25 -0
 	return masks;
 }
 
+static int acpi_aml_flush(void)
+{
+	int ret;
+
+	/*
+	 * Discard output buffer and put the driver into a state waiting
+	 * for the new user input.
+	 */
+	mutex_lock(&acpi_aml_io.lock);
+	acpi_aml_io.flags |= ACPI_AML_FLUSHING_LOG;
+	mutex_unlock(&acpi_aml_io.lock);
+
+	ret = wait_event_interruptible(acpi_aml_io.wait,
+		acpi_aml_waiting_cmd());
+	(void)acpi_aml_read_user(NULL, ACPI_AML_BUF_SIZE);
+
+	mutex_lock(&acpi_aml_io.lock);
+	acpi_aml_io.flags &= ~ACPI_AML_FLUSHING_LOG;
+	mutex_unlock(&acpi_aml_io.lock);
+	return ret;
+}
+
+static long acpi_aml_ioctl(struct file *file,
+			   unsigned int cmd, unsigned long arg)
+{

drivers/acpi/acpi_dbg.c static unsigned int acpi_aml_poll(struct file *file, poll_table *wait) · line 820

Quoted 10 lines
drivers/acpi/acpi_dbg.c +10 -0
+	long ret = -EINVAL;
+
+	switch (cmd) {
+	case ACPI_IOCTL_DEBUGGER_FLUSH:
+		ret = acpi_aml_flush();
+		break;
+	default:
+		break;
+	}
+	return ret;

	return cmd == ACPI_IOCTL_DEBUGGER_FLUSH ? acpi_aml_flush() :
Quoted 1 line

-EINVAL;

[Lv Zheng]
OK.

Quoted 10 lines
Quoted 10 lines
drivers/acpi/acpi_dbg.c +3 -0
+}
+
 static const struct file_operations acpi_aml_operations = {
 	.read		= acpi_aml_read,
 	.write		= acpi_aml_write,
 	.poll		= acpi_aml_poll,
+	.unlocked_ioctl	= acpi_aml_ioctl,
 	.open		= acpi_aml_open,
 	.release	= acpi_aml_release,
Quoted 8 lines

include/linux/acpi-ioctls.h

Quoted 6 lines
drivers/acpi/acpi_dbg.c +0 -0
 	.llseek		= generic_file_llseek,
include/linux/acpi-ioctls.h new file +0 -0
new file mode 100644
index 0000000..56b8170

include/uapi/linux/

[Lv Zheng]
OK.

Quoted 22 lines
Quoted 11 lines
include/linux/acpi-ioctls.h new file +9 -0
@@ -0,0 +1,21 @@
+/*
+ * ACPI IOCTL collections
+ *
+ * Copyright (C) 2016, Intel Corporation
+ * Authors: Lv Zheng <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.

include/linux/acpi-ioctls.h line 19

Quoted 10 lines
include/linux/acpi-ioctls.h new file +10 -0
+ */
+
+#ifndef _LINUX_ACPI_IOCTLS_H
+#define _LINUX_ACPI_IOCTLS_H
+
+#include <linux/ioctl.h>
+
+#define ACPI_IOCTL_IDENT		'a'
+
+#define ACPI_IOCTL_DEBUGGER_FLUSH	_IO(ACPI_IOCTL_IDENT,

0x80)

Quoted 6 lines

include/linux/acpi-ioctls.h line 21

Quoted 3 lines
include/linux/acpi-ioctls.h new file +2 -0
+
+#endif /* _LINUX_ACPI_IOCTLS_H */

Plus patches that change the ABI should be CCed to the ABI review list for, well, review.

[Lv Zheng]
OK.

Thanks,
-Lv

author
#9 lore raw

PATCH v2 2/2 tools/power/acpi/acpidbg: Use new flushing mechanism

In reply to Lv Zheng

This patch converts tools/power/acpi/tools/acpidbg/acpidbg to use the new flushing mechanism.

Signed-off-by Lv Zheng <[email protected]>
---
 tools/power/acpi/tools/acpidbg/acpidbg.c |   51 ++++--------------------------
 1 file changed, 7 insertions(+), 44 deletions(-)
tools/power/acpi/tools/acpidbg/acpidbg.c +7 -44
index a88ac45..0aba129 100644
@@ -15,7 +15,9 @@
 #include <stdbool.h>
 #include <fcntl.h>
 #include <assert.h>
+#include <sys/ioctl.h>
 #include <linux/circ_buf.h>
+#include <linux/acpi-ioctls.h>
 
 #define ACPI_AML_FILE		"/sys/kernel/debug/acpi/acpidbg"
 #define ACPI_AML_SEC_TICK	1

@@ -83,7 +85,6 @@ static const char *acpi_aml_file_path = ACPI_AML_FILE;
 static unsigned long acpi_aml_mode = ACPI_AML_INTERACTIVE;
 static bool acpi_aml_exit;
 
-static bool acpi_aml_batch_drain;
 static unsigned long acpi_aml_batch_state;
 static char acpi_aml_batch_prompt;
 static char acpi_aml_batch_roll;

@@ -239,11 +240,9 @@ static int acpi_aml_write_batch_log(int fd, struct circ_buf *crc)
 
 	p = &crc->buf[crc->tail];
 	len = circ_count_to_end(crc);
-	if (!acpi_aml_batch_drain) {
-		len = write(fd, p, len);
-		if (len < 0)
-			perror("write");
-	}
+	len = write(fd, p, len);
+	if (len < 0)
+		perror("write");
 	if (len > 0)
 		crc->tail = (crc->tail + len) & (ACPI_AML_BUF_SIZE - 1);
 	return len;

@@ -270,10 +269,7 @@ static void acpi_aml_loop(int fd)
 	if (acpi_aml_mode == ACPI_AML_BATCH) {
 		acpi_aml_log_state = ACPI_AML_LOG_START;
 		acpi_aml_batch_pos = acpi_aml_batch_cmd;
-		if (acpi_aml_batch_drain)
-			acpi_aml_batch_state = ACPI_AML_BATCH_READ_LOG;
-		else
-			acpi_aml_batch_state = ACPI_AML_BATCH_WRITE_CMD;
+		acpi_aml_batch_state = ACPI_AML_BATCH_WRITE_CMD;
 	}
 	acpi_aml_exit = false;
 	while (!acpi_aml_exit) {

@@ -330,39 +326,6 @@ static void acpi_aml_loop(int fd)
 	}
 }
 
-static bool acpi_aml_readable(int fd)
-{
-	fd_set rfds;
-	struct timeval tv;
-	int ret;
-	int maxfd = 0;
-
-	tv.tv_sec = 0;
-	tv.tv_usec = ACPI_AML_USEC_PEEK;
-	FD_ZERO(&rfds);
-	maxfd = acpi_aml_set_fd(fd, maxfd, &rfds);
-	ret = select(maxfd+1, &rfds, NULL, NULL, &tv);
-	if (ret < 0)
-		perror("select");
-	if (ret > 0 && FD_ISSET(fd, &rfds))
-		return true;
-	return false;
-}
-
-/*
- * This is a userspace IO flush implementation, replying on the prompt
- * characters and can be turned into a flush() call after kernel implements
- * .flush() filesystem operation.
- */
-static void acpi_aml_flush(int fd)
-{
-	while (acpi_aml_readable(fd)) {
-		acpi_aml_batch_drain = true;
-		acpi_aml_loop(fd);
-		acpi_aml_batch_drain = false;
-	}
-}
-
 void usage(FILE *file, char *progname)
 {
 	fprintf(file, "usage: %s [-b cmd] [-f file] [-h]\n", progname);

@@ -426,7 +389,7 @@ int main(int argc, char **argv)
 	acpi_aml_set_fl(STDOUT_FILENO, O_NONBLOCK);
 
 	if (acpi_aml_mode == ACPI_AML_BATCH)
-		acpi_aml_flush(fd);
+		ioctl(fd, ACPI_IOCTL_DEBUGGER_FLUSH);
 	acpi_aml_loop(fd);
 
 exit:
Signature · 1 line
1.7.10
author
#10 lore raw

PATCH 1/1 tools/power/acpi/tools/acpidbg: Add multi-commands support in batch mode

In reply to Lv Zheng

This patch adds multi-commands support for the batch mode. The same mode can be seen in acpiexec.

However people may think this is not useful for an in-kernel debugger, because the in-kernel debugger is always running, never exits. So we can run another command by running another acpidbg batch mode instance.

But this mode should still be useful for acpidbg. The reason is: when the in-kernel debugger has entered the single-stepping mode, ending acpidbg (which closes the debugger IO interface) will lead to the end of the single-stepping mode.

So we need the acpidbg multi-commands batch mode in order to execute multiple single-stepping mode commands in the batch mode.

Signed-off-by Lv Zheng <[email protected]>
---
 tools/power/acpi/tools/acpidbg/acpidbg.c |   77 +++++++++++++++++++++++++-----
 1 file changed, 64 insertions(+), 13 deletions(-)
tools/power/acpi/tools/acpidbg/acpidbg.c +64 -13
index 0aba129..330e5fe 100644
@@ -90,6 +90,7 @@ static char acpi_aml_batch_prompt;
 static char acpi_aml_batch_roll;
 static unsigned long acpi_aml_log_state;
 static char *acpi_aml_batch_cmd = NULL;
+static char *acpi_aml_batch_cmds = NULL;
 static char *acpi_aml_batch_pos = NULL;
 
 static int acpi_aml_set_fl(int fd, int flags)

@@ -326,12 +327,65 @@ static void acpi_aml_loop(int fd)
 	}
 }
 
+static void acpi_aml_delete_batch(void)
+{
+	if (acpi_aml_batch_cmd) {
+		free(acpi_aml_batch_cmd);
+		acpi_aml_batch_cmd = NULL;
+	}
+}
+
+static bool acpi_aml_create_batch(char *cmd)
+{
+	int len;
+
+	acpi_aml_delete_batch();
+	len = strlen(cmd);
+	acpi_aml_batch_cmd = calloc(len + 2, 1);
+	if (!acpi_aml_batch_cmd) {
+		perror("calloc");
+		return false;
+	}
+	memcpy(acpi_aml_batch_cmd, cmd, len);
+	acpi_aml_batch_cmd[len] = '\n';
+	return true;
+}
+
+static void acpi_aml_batch(int fd)
+{
+	char *ptr, *cmd;
+	bool run = false;
+
+	cmd = ptr = acpi_aml_batch_cmds;
+	while (*ptr) {
+		if (*ptr == ',') {
+			/* Convert commas to spaces */
+			*ptr = ' ';
+		} else if (*ptr == ';') {
+			*ptr = '\0';
+			run = true;
+		}
+		ptr++;
+		if (run || (*ptr == '\0')) {
+			if (!acpi_aml_create_batch(cmd))
+				return;
+			ioctl(fd, ACPI_IOCTL_DEBUGGER_FLUSH);
+			acpi_aml_loop(fd);
+			run = 0;
+			cmd = ptr;
+			acpi_aml_delete_batch();
+		}
+	}
+}
+
 void usage(FILE *file, char *progname)
 {
 	fprintf(file, "usage: %s [-b cmd] [-f file] [-h]\n", progname);
 	fprintf(file, "\nOptions:\n");
-	fprintf(file, "  -b     Specify command to be executed in batch mode\n");
-	fprintf(file, "  -f     Specify interface file other than");
+	fprintf(file, "  -b     Specify commands to be executed in batch mode\n");
+	fprintf(file, "         Use ';' as command delimiters\n");
+	fprintf(file, "         Use ',' as spaces\n");
+	fprintf(file, "  -f     Specify interface file other than\n");
 	fprintf(file, "         /sys/kernel/debug/acpi/acpidbg\n");
 	fprintf(file, "  -h     Print this help message\n");
 }

@@ -340,27 +394,23 @@ int main(int argc, char **argv)
 {
 	int fd = -1;
 	int ch;
-	int len;
 	int ret = EXIT_SUCCESS;
 
 	while ((ch = getopt(argc, argv, "b:f:h")) != -1) {
 		switch (ch) {
 		case 'b':
-			if (acpi_aml_batch_cmd) {
+			if (acpi_aml_batch_cmds) {
 				fprintf(stderr, "Already specify %s\n",
-					acpi_aml_batch_cmd);
+					acpi_aml_batch_cmds);
 				ret = EXIT_FAILURE;
 				goto exit;
 			}
-			len = strlen(optarg);
-			acpi_aml_batch_cmd = calloc(len + 2, 1);
-			if (!acpi_aml_batch_cmd) {
-				perror("calloc");
+			acpi_aml_batch_cmds = strdup(optarg);
+			if (!acpi_aml_batch_cmds) {
+				perror("strdup");
 				ret = EXIT_FAILURE;
 				goto exit;
 			}
-			memcpy(acpi_aml_batch_cmd, optarg, len);
-			acpi_aml_batch_cmd[len] = '\n';
 			acpi_aml_mode = ACPI_AML_BATCH;
 			break;
 		case 'f':

@@ -389,8 +439,9 @@ int main(int argc, char **argv)
 	acpi_aml_set_fl(STDOUT_FILENO, O_NONBLOCK);
 
 	if (acpi_aml_mode == ACPI_AML_BATCH)
-		ioctl(fd, ACPI_IOCTL_DEBUGGER_FLUSH);
-	acpi_aml_loop(fd);
+		acpi_aml_batch(fd);
+	else
+		acpi_aml_loop(fd);
 
 exit:
 	if (fd >= 0)
Signature · 1 line
1.7.10
#11 lore raw

Re: [PATCH] tools/power/acpi/tools/acpidbg: Add multi-commands support in batch mode

Quoted 12 lines · Lv Zheng

This patch adds multi-commands support for the batch mode. The same mode can be seen in acpiexec.

However people may think this is not useful for an in-kernel debugger, because the in-kernel debugger is always running, never exits. So we can run another command by running another acpidbg batch mode instance.

But this mode should still be useful for acpidbg. The reason is: when the in-kernel debugger has entered the single-stepping mode, ending acpidbg (which closes the debugger IO interface) will lead to the end of the single-stepping mode.

Quoted 4 lines

So we need the acpidbg multi-commands batch mode in order to execute multiple single-stepping mode commands in the batch mode.

Signed-off-by Lv Zheng <[email protected]>

Does this depend on the kernel flushing support series?

Thanks,
Rafael

author
#12 lore raw

RE: [PATCH] tools/power/acpi/tools/acpidbg: Add multi-commands support in batch mode

Hi, Rafael

Quoted 18 lines

From: Rafael J. Wysocki [mailto:[email protected]]
Subject: Re: [PATCH] tools/power/acpi/tools/acpidbg: Add multi- commands support in batch mode

Quoted 1 line · Lv Zheng

This patch adds multi-commands support for the batch mode. The same

mode

Quoted 11 lines

can be seen in acpiexec.

However people may think this is not useful for an in-kernel debugger, because the in-kernel debugger is always running, never exits. So we can run another command by running another acpidbg batch mode instance.

But this mode should still be useful for acpidbg. The reason is: when the in-kernel debugger has entered the single-stepping mode, ending acpidbg (which closes the debugger IO interface) will lead to the end of the single-stepping mode.

Quoted 6 lines
Quoted 4 lines

So we need the acpidbg multi-commands batch mode in order to execute multiple single-stepping mode commands in the batch mode.

Signed-off-by Lv Zheng <[email protected]>

Does this depend on the kernel flushing support series?

[Lv Zheng]
This depends on the "flush" stuff.
That's why I sent it to the same mailing thread.

Thanks
-Lv

author
#13 lore raw

v3 cover ACPI / debugger: Add kernel flushing support

In reply to Lv Zheng

AML debugger is implemented in the kernel as a character device located in the debugfs. Currently, when its batch mode is used, the userspace tool needs to flush the logs/prompts remained in the kernel output buffer, this is implemented in an inefficient way in the userspace by polling the IO and reading everything out.

This patch introduces a kernel space flushing support, so that userspace can invoke ioctl() to put the driver into a state waiting for new commands, all kernel space logs/prompts will be automatically discarded by ioctl().

Lv Zheng (3):
ACPI / debugger: Add kernel flushing support tools/power/acpi/acpidbg: Use new flushing mechanism tools/power/acpi/acpidbg: Add multi-commands support in batch mode

 drivers/acpi/acpi_dbg.c                  |   85 +++++++++++++++++++++--
 include/linux/acpi.h                     |    1 +
 include/uapi/linux/acpi-ioctls.h         |   21 ++++++
 tools/power/acpi/tools/acpidbg/acpidbg.c |  110 +++++++++++++++++-------------
 4 files changed, 165 insertions(+), 52 deletions(-)
 create mode 100644 include/uapi/linux/acpi-ioctls.h
Signature · 1 line
1.7.10
author
#14 lore raw

PATCH v3 1/3 ACPI / debugger: Add kernel flushing support

This patch adds debugger log flushing support in kernel via .ioctl() callback. The in-kernel flushing is more efficient, because it reduces useless log IOs by bypassing log user_read/kern_write during the flush period.

Signed-off-by Lv Zheng <[email protected]>
Cc [email protected]
---
 drivers/acpi/acpi_dbg.c          |   85 ++++++++++++++++++++++++++++++++++++--
 include/linux/acpi.h             |    1 +
 include/uapi/linux/acpi-ioctls.h |   21 ++++++++++
 3 files changed, 103 insertions(+), 4 deletions(-)
 create mode 100644 include/uapi/linux/acpi-ioctls.h
drivers/acpi/acpi_dbg.c +81 -4
index dee8692..a5f4457 100644
@@ -46,6 +46,8 @@
 #define ACPI_AML_KERN		(ACPI_AML_IN_KERN | ACPI_AML_OUT_KERN)
 #define ACPI_AML_BUSY		(ACPI_AML_USER | ACPI_AML_KERN)
 #define ACPI_AML_OPEN		(ACPI_AML_OPENED | ACPI_AML_CLOSED)
+#define ACPI_AML_FLUSHING_LOG	0x0040 /* flushing log output */
+#define ACPI_AML_WAITING_CMD	0x0080 /* waiting for cmd input */
 
 struct acpi_aml_io {
 	wait_queue_head_t wait;

@@ -120,6 +122,16 @@ static inline bool __acpi_aml_busy(void)
 	return false;
 }
 
+static inline bool __acpi_aml_waiting_cmd(void)
+{
+	return !!(acpi_aml_io.flags & ACPI_AML_WAITING_CMD);
+}
+
+static inline bool __acpi_aml_flushing_log(void)
+{
+	return !!(acpi_aml_io.flags & ACPI_AML_FLUSHING_LOG);
+}
+
 static inline bool __acpi_aml_opened(void)
 {
 	if (acpi_aml_io.flags & ACPI_AML_OPEN)

@@ -152,6 +164,26 @@ static bool acpi_aml_busy(void)
 	return ret;
 }
 
+static inline bool acpi_aml_waiting_cmd(void)
+{
+	bool ret;
+
+	mutex_lock(&acpi_aml_io.lock);
+	ret = __acpi_aml_waiting_cmd();
+	mutex_unlock(&acpi_aml_io.lock);
+	return ret;
+}
+
+static inline bool acpi_aml_flushing_log(void)
+{
+	bool ret;
+
+	mutex_lock(&acpi_aml_io.lock);
+	ret = __acpi_aml_flushing_log();
+	mutex_unlock(&acpi_aml_io.lock);
+	return ret;
+}
+
 static bool acpi_aml_used(void)
 {
 	bool ret;

@@ -183,7 +215,8 @@ static bool acpi_aml_kern_writable(void)
 
 	mutex_lock(&acpi_aml_io.lock);
 	ret = !__acpi_aml_access_ok(ACPI_AML_OUT_KERN) ||
-	      __acpi_aml_writable(&acpi_aml_io.out_crc, ACPI_AML_OUT_KERN);
+	      __acpi_aml_writable(&acpi_aml_io.out_crc, ACPI_AML_OUT_KERN) ||
+	      __acpi_aml_flushing_log();
 	mutex_unlock(&acpi_aml_io.lock);
 	return ret;
 }

@@ -264,6 +297,9 @@ static int acpi_aml_write_kern(const char *buf, int len)
 	int n;
 	char *p;
 
+	if (acpi_aml_flushing_log())
+		return len;
+
 	ret = acpi_aml_lock_write(crc, ACPI_AML_OUT_KERN);
 	if (ret < 0)
 		return ret;

@@ -458,9 +494,18 @@ static int acpi_aml_wait_command_ready(bool single_step,
 	else
 		acpi_os_printf("\n%1c ", ACPI_DEBUGGER_COMMAND_PROMPT);
 
+	mutex_lock(&acpi_aml_io.lock);
+	acpi_aml_io.flags |= ACPI_AML_WAITING_CMD;
+	wake_up_interruptible(&acpi_aml_io.wait);
+	mutex_unlock(&acpi_aml_io.lock);
+
 	status = acpi_os_get_line(buffer, length, NULL);
 	if (ACPI_FAILURE(status))
 		return -EINVAL;
+
+	mutex_lock(&acpi_aml_io.lock);
+	acpi_aml_io.flags &= ~ACPI_AML_WAITING_CMD;
+	mutex_unlock(&acpi_aml_io.lock);
 	return 0;
 }
 

@@ -593,9 +638,11 @@ static int acpi_aml_read_user(char __user *buf, int len)
 	smp_rmb();
 	p = &crc->buf[crc->tail];
 	n = min(len, circ_count_to_end(crc));
-	if (copy_to_user(buf, p, n)) {
-		ret = -EFAULT;
-		goto out;
+	if (!acpi_aml_flushing_log()) {
+		if (copy_to_user(buf, p, n)) {
+			ret = -EFAULT;
+			goto out;
+		}
 	}
 	/* sync tail after removing logs */
 	smp_mb();

@@ -731,10 +778,40 @@ static unsigned int acpi_aml_poll(struct file *file, poll_table *wait)
 	return masks;
 }
 
+static int acpi_aml_flush(void)
+{
+	int ret;
+
+	/*
+	 * Discard output buffer and put the driver into a state waiting
+	 * for the new user input.
+	 */
+	mutex_lock(&acpi_aml_io.lock);
+	acpi_aml_io.flags |= ACPI_AML_FLUSHING_LOG;
+	mutex_unlock(&acpi_aml_io.lock);
+
+	ret = wait_event_interruptible(acpi_aml_io.wait,
+		acpi_aml_waiting_cmd());
+	(void)acpi_aml_read_user(NULL, ACPI_AML_BUF_SIZE);
+
+	mutex_lock(&acpi_aml_io.lock);
+	acpi_aml_io.flags &= ~ACPI_AML_FLUSHING_LOG;
+	mutex_unlock(&acpi_aml_io.lock);
+	return ret;
+}
+
+static long acpi_aml_ioctl(struct file *file,
+			   unsigned int cmd, unsigned long arg)
+{
+	return cmd == ACPI_IOCTL_DEBUGGER_FLUSH ?
+	       acpi_aml_flush() : -EINVAL;
+}
+
 static const struct file_operations acpi_aml_operations = {
 	.read		= acpi_aml_read,
 	.write		= acpi_aml_write,
 	.poll		= acpi_aml_poll,
+	.unlocked_ioctl	= acpi_aml_ioctl,
 	.open		= acpi_aml_open,
 	.release	= acpi_aml_release,
 	.llseek		= generic_file_llseek,
include/linux/acpi.h +1 -0
index 08235a6..9354fb8 100644
@@ -26,6 +26,7 @@
 #include <linux/resource_ext.h>
 #include <linux/device.h>
 #include <linux/property.h>
+#include <uapi/linux/acpi-ioctls.h>
 
 #ifndef _LINUX
 #define _LINUX
include/uapi/linux/acpi-ioctls.h new file +21 -0
new file mode 100644
index 0000000..71b891a
@@ -0,0 +1,21 @@
+/*
+ * ACPI IOCTL collections
+ *
+ * Copyright (C) 2016, Intel Corporation
+ * Authors: Lv Zheng <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef _UAPI_LINUX_ACPI_IOCTLS_H
+#define _UAPI_LINUX_ACPI_IOCTLS_H
+
+#include <linux/ioctl.h>
+
+#define ACPI_IOCTL_IDENT		'a'
+
+#define ACPI_IOCTL_DEBUGGER_FLUSH	_IO(ACPI_IOCTL_IDENT, 0x80)
+
+#endif /* _UAPI_LINUX_ACPI_IOCTLS_H */
Signature · 1 line
1.7.10
author
#15 lore raw

PATCH v3 2/3 tools/power/acpi/acpidbg: Use new flushing mechanism

In reply to Lv Zheng

This patch converts tools/power/acpi/tools/acpidbg/acpidbg to use the new flushing mechanism.

Signed-off-by Lv Zheng <[email protected]>
---
 tools/power/acpi/tools/acpidbg/acpidbg.c |   51 ++++--------------------------
 1 file changed, 7 insertions(+), 44 deletions(-)
tools/power/acpi/tools/acpidbg/acpidbg.c +7 -44
index a88ac45..f5542b9 100644
@@ -15,7 +15,9 @@
 #include <stdbool.h>
 #include <fcntl.h>
 #include <assert.h>
+#include <sys/ioctl.h>
 #include <linux/circ_buf.h>
+#include <uapi/linux/acpi-ioctls.h>
 
 #define ACPI_AML_FILE		"/sys/kernel/debug/acpi/acpidbg"
 #define ACPI_AML_SEC_TICK	1

@@ -83,7 +85,6 @@ static const char *acpi_aml_file_path = ACPI_AML_FILE;
 static unsigned long acpi_aml_mode = ACPI_AML_INTERACTIVE;
 static bool acpi_aml_exit;
 
-static bool acpi_aml_batch_drain;
 static unsigned long acpi_aml_batch_state;
 static char acpi_aml_batch_prompt;
 static char acpi_aml_batch_roll;

@@ -239,11 +240,9 @@ static int acpi_aml_write_batch_log(int fd, struct circ_buf *crc)
 
 	p = &crc->buf[crc->tail];
 	len = circ_count_to_end(crc);
-	if (!acpi_aml_batch_drain) {
-		len = write(fd, p, len);
-		if (len < 0)
-			perror("write");
-	}
+	len = write(fd, p, len);
+	if (len < 0)
+		perror("write");
 	if (len > 0)
 		crc->tail = (crc->tail + len) & (ACPI_AML_BUF_SIZE - 1);
 	return len;

@@ -270,10 +269,7 @@ static void acpi_aml_loop(int fd)
 	if (acpi_aml_mode == ACPI_AML_BATCH) {
 		acpi_aml_log_state = ACPI_AML_LOG_START;
 		acpi_aml_batch_pos = acpi_aml_batch_cmd;
-		if (acpi_aml_batch_drain)
-			acpi_aml_batch_state = ACPI_AML_BATCH_READ_LOG;
-		else
-			acpi_aml_batch_state = ACPI_AML_BATCH_WRITE_CMD;
+		acpi_aml_batch_state = ACPI_AML_BATCH_WRITE_CMD;
 	}
 	acpi_aml_exit = false;
 	while (!acpi_aml_exit) {

@@ -330,39 +326,6 @@ static void acpi_aml_loop(int fd)
 	}
 }
 
-static bool acpi_aml_readable(int fd)
-{
-	fd_set rfds;
-	struct timeval tv;
-	int ret;
-	int maxfd = 0;
-
-	tv.tv_sec = 0;
-	tv.tv_usec = ACPI_AML_USEC_PEEK;
-	FD_ZERO(&rfds);
-	maxfd = acpi_aml_set_fd(fd, maxfd, &rfds);
-	ret = select(maxfd+1, &rfds, NULL, NULL, &tv);
-	if (ret < 0)
-		perror("select");
-	if (ret > 0 && FD_ISSET(fd, &rfds))
-		return true;
-	return false;
-}
-
-/*
- * This is a userspace IO flush implementation, replying on the prompt
- * characters and can be turned into a flush() call after kernel implements
- * .flush() filesystem operation.
- */
-static void acpi_aml_flush(int fd)
-{
-	while (acpi_aml_readable(fd)) {
-		acpi_aml_batch_drain = true;
-		acpi_aml_loop(fd);
-		acpi_aml_batch_drain = false;
-	}
-}
-
 void usage(FILE *file, char *progname)
 {
 	fprintf(file, "usage: %s [-b cmd] [-f file] [-h]\n", progname);

@@ -426,7 +389,7 @@ int main(int argc, char **argv)
 	acpi_aml_set_fl(STDOUT_FILENO, O_NONBLOCK);
 
 	if (acpi_aml_mode == ACPI_AML_BATCH)
-		acpi_aml_flush(fd);
+		ioctl(fd, ACPI_IOCTL_DEBUGGER_FLUSH);
 	acpi_aml_loop(fd);
 
 exit:
Signature · 1 line
1.7.10
author
#16 lore raw

PATCH v3 3/3 tools/power/acpi/acpidbg: Add multi-commands support in batch mode

In reply to Lv Zheng

This patch adds multi-commands support for the batch mode. The same mode can be seen in acpiexec.

However people may think this is not useful for an in-kernel debugger, because the in-kernel debugger is always running, never exits. So we can run another command by running another acpidbg batch mode instance.

But this mode should still be useful for acpidbg. The reason is: when the in-kernel debugger has entered the single-stepping mode, ending acpidbg (which closes the debugger IO interface) will lead to the end of the single-stepping mode.

So we need the acpidbg multi-commands batch mode in order to execute multiple single-stepping mode commands in the batch mode.

Signed-off-by Lv Zheng <[email protected]>
---
 tools/power/acpi/tools/acpidbg/acpidbg.c |   77 +++++++++++++++++++++++++-----
 1 file changed, 64 insertions(+), 13 deletions(-)
tools/power/acpi/tools/acpidbg/acpidbg.c +64 -13
index f5542b9..99a8f766 100644
@@ -90,6 +90,7 @@ static char acpi_aml_batch_prompt;
 static char acpi_aml_batch_roll;
 static unsigned long acpi_aml_log_state;
 static char *acpi_aml_batch_cmd = NULL;
+static char *acpi_aml_batch_cmds = NULL;
 static char *acpi_aml_batch_pos = NULL;
 
 static int acpi_aml_set_fl(int fd, int flags)

@@ -326,12 +327,65 @@ static void acpi_aml_loop(int fd)
 	}
 }
 
+static void acpi_aml_delete_batch(void)
+{
+	if (acpi_aml_batch_cmd) {
+		free(acpi_aml_batch_cmd);
+		acpi_aml_batch_cmd = NULL;
+	}
+}
+
+static bool acpi_aml_create_batch(char *cmd)
+{
+	int len;
+
+	acpi_aml_delete_batch();
+	len = strlen(cmd);
+	acpi_aml_batch_cmd = calloc(len + 2, 1);
+	if (!acpi_aml_batch_cmd) {
+		perror("calloc");
+		return false;
+	}
+	memcpy(acpi_aml_batch_cmd, cmd, len);
+	acpi_aml_batch_cmd[len] = '\n';
+	return true;
+}
+
+static void acpi_aml_batch(int fd)
+{
+	char *ptr, *cmd;
+	bool run = false;
+
+	cmd = ptr = acpi_aml_batch_cmds;
+	while (*ptr) {
+		if (*ptr == ',') {
+			/* Convert commas to spaces */
+			*ptr = ' ';
+		} else if (*ptr == ';') {
+			*ptr = '\0';
+			run = true;
+		}
+		ptr++;
+		if (run || (*ptr == '\0')) {
+			if (!acpi_aml_create_batch(cmd))
+				return;
+			ioctl(fd, ACPI_IOCTL_DEBUGGER_FLUSH);
+			acpi_aml_loop(fd);
+			run = 0;
+			cmd = ptr;
+			acpi_aml_delete_batch();
+		}
+	}
+}
+
 void usage(FILE *file, char *progname)
 {
 	fprintf(file, "usage: %s [-b cmd] [-f file] [-h]\n", progname);
 	fprintf(file, "\nOptions:\n");
-	fprintf(file, "  -b     Specify command to be executed in batch mode\n");
-	fprintf(file, "  -f     Specify interface file other than");
+	fprintf(file, "  -b     Specify commands to be executed in batch mode\n");
+	fprintf(file, "         Use ';' as command delimiters\n");
+	fprintf(file, "         Use ',' as spaces\n");
+	fprintf(file, "  -f     Specify interface file other than\n");
 	fprintf(file, "         /sys/kernel/debug/acpi/acpidbg\n");
 	fprintf(file, "  -h     Print this help message\n");
 }

@@ -340,27 +394,23 @@ int main(int argc, char **argv)
 {
 	int fd = -1;
 	int ch;
-	int len;
 	int ret = EXIT_SUCCESS;
 
 	while ((ch = getopt(argc, argv, "b:f:h")) != -1) {
 		switch (ch) {
 		case 'b':
-			if (acpi_aml_batch_cmd) {
+			if (acpi_aml_batch_cmds) {
 				fprintf(stderr, "Already specify %s\n",
-					acpi_aml_batch_cmd);
+					acpi_aml_batch_cmds);
 				ret = EXIT_FAILURE;
 				goto exit;
 			}
-			len = strlen(optarg);
-			acpi_aml_batch_cmd = calloc(len + 2, 1);
-			if (!acpi_aml_batch_cmd) {
-				perror("calloc");
+			acpi_aml_batch_cmds = strdup(optarg);
+			if (!acpi_aml_batch_cmds) {
+				perror("strdup");
 				ret = EXIT_FAILURE;
 				goto exit;
 			}
-			memcpy(acpi_aml_batch_cmd, optarg, len);
-			acpi_aml_batch_cmd[len] = '\n';
 			acpi_aml_mode = ACPI_AML_BATCH;
 			break;
 		case 'f':

@@ -389,8 +439,9 @@ int main(int argc, char **argv)
 	acpi_aml_set_fl(STDOUT_FILENO, O_NONBLOCK);
 
 	if (acpi_aml_mode == ACPI_AML_BATCH)
-		ioctl(fd, ACPI_IOCTL_DEBUGGER_FLUSH);
-	acpi_aml_loop(fd);
+		acpi_aml_batch(fd);
+	else
+		acpi_aml_loop(fd);
 
 exit:
 	if (fd >= 0)
Signature · 1 line
1.7.10
author
#17 lore raw

v4 cover ACPI / debugger: Add kernel flushing support

In reply to Lv Zheng

The AML debugger can be used to dump the runtime value of a named object, which is useful for remote debugging issues. The batch mode of the AML debugger allows the debugger functionalities to be integrated into the scripts.

Currently, when the batch mode is used, the userspace tool needs to flush the output (logs/prompts) remained in the kernel output buffer in order not to mess up the result of the executed batch mode command, this is implemented in an inefficient way in the userspace by polling the IO and reading everything out.
However, the input needn't be flushed as the command input should have already been flushed by the signals and won't be passed to acpi_os_get_line() due to an error return value.

This patch introduces a kernel space flushing support, so that userspace can invoke ioctl() to request the driver to drop old outputs.

Lv Zheng (3):
ACPI / debugger: Add kernel flushing support tools/power/acpi/acpidbg: Use new flushing mechanism tools/power/acpi/acpidbg: Add multi-commands support in batch mode

 drivers/acpi/acpi_dbg.c                  |   85 +++++++++++++++++++++--
 include/linux/acpi.h                     |    1 +
 include/uapi/linux/acpi-ioctls.h         |   21 ++++++
 tools/power/acpi/tools/acpidbg/acpidbg.c |  110 +++++++++++++++++-------------
 4 files changed, 165 insertions(+), 52 deletions(-)
 create mode 100644 include/uapi/linux/acpi-ioctls.h
Signature · 1 line
1.7.10
author
#18 lore raw

PATCH v4 1/3 ACPI / debugger: Add kernel flushing support

This patch adds debugger output flushing support in kernel via .ioctl() callback. The in-kernel flushing is more efficient, because it reduces useless log IOs by bypassing log user_read/kern_write during the flush period.

This mechanism is useful for the batch mode. Scripts can integrate a batch mode acpidbg instance to perform AML debugger functionalities.

As the batch mode always starts from a new command write, it thus requires the kernel debugger driver to drop the old input/output first. The old input is automatically dropped by acpi_os_get_line() via an error returning value, but the output are remained in acpi_dbg output buffers and should be dropped prior than reading the new command, otherwise, the old output can be read out by the batch mode instance and the result of the batch mode command will be messed up.

Signed-off-by Lv Zheng <[email protected]>
Cc [email protected]
---
 drivers/acpi/acpi_dbg.c          |   85 ++++++++++++++++++++++++++++++++++++--
 include/linux/acpi.h             |    1 +
 include/uapi/linux/acpi-ioctls.h |   21 ++++++++++
 3 files changed, 103 insertions(+), 4 deletions(-)
 create mode 100644 include/uapi/linux/acpi-ioctls.h
drivers/acpi/acpi_dbg.c +81 -4
index dee8692..a5f4457 100644
@@ -46,6 +46,8 @@
 #define ACPI_AML_KERN		(ACPI_AML_IN_KERN | ACPI_AML_OUT_KERN)
 #define ACPI_AML_BUSY		(ACPI_AML_USER | ACPI_AML_KERN)
 #define ACPI_AML_OPEN		(ACPI_AML_OPENED | ACPI_AML_CLOSED)
+#define ACPI_AML_FLUSHING_LOG	0x0040 /* flushing log output */
+#define ACPI_AML_WAITING_CMD	0x0080 /* waiting for cmd input */
 
 struct acpi_aml_io {
 	wait_queue_head_t wait;

@@ -120,6 +122,16 @@ static inline bool __acpi_aml_busy(void)
 	return false;
 }
 
+static inline bool __acpi_aml_waiting_cmd(void)
+{
+	return !!(acpi_aml_io.flags & ACPI_AML_WAITING_CMD);
+}
+
+static inline bool __acpi_aml_flushing_log(void)
+{
+	return !!(acpi_aml_io.flags & ACPI_AML_FLUSHING_LOG);
+}
+
 static inline bool __acpi_aml_opened(void)
 {
 	if (acpi_aml_io.flags & ACPI_AML_OPEN)

@@ -152,6 +164,26 @@ static bool acpi_aml_busy(void)
 	return ret;
 }
 
+static inline bool acpi_aml_waiting_cmd(void)
+{
+	bool ret;
+
+	mutex_lock(&acpi_aml_io.lock);
+	ret = __acpi_aml_waiting_cmd();
+	mutex_unlock(&acpi_aml_io.lock);
+	return ret;
+}
+
+static inline bool acpi_aml_flushing_log(void)
+{
+	bool ret;
+
+	mutex_lock(&acpi_aml_io.lock);
+	ret = __acpi_aml_flushing_log();
+	mutex_unlock(&acpi_aml_io.lock);
+	return ret;
+}
+
 static bool acpi_aml_used(void)
 {
 	bool ret;

@@ -183,7 +215,8 @@ static bool acpi_aml_kern_writable(void)
 
 	mutex_lock(&acpi_aml_io.lock);
 	ret = !__acpi_aml_access_ok(ACPI_AML_OUT_KERN) ||
-	      __acpi_aml_writable(&acpi_aml_io.out_crc, ACPI_AML_OUT_KERN);
+	      __acpi_aml_writable(&acpi_aml_io.out_crc, ACPI_AML_OUT_KERN) ||
+	      __acpi_aml_flushing_log();
 	mutex_unlock(&acpi_aml_io.lock);
 	return ret;
 }

@@ -264,6 +297,9 @@ static int acpi_aml_write_kern(const char *buf, int len)
 	int n;
 	char *p;
 
+	if (acpi_aml_flushing_log())
+		return len;
+
 	ret = acpi_aml_lock_write(crc, ACPI_AML_OUT_KERN);
 	if (ret < 0)
 		return ret;

@@ -458,9 +494,18 @@ static int acpi_aml_wait_command_ready(bool single_step,
 	else
 		acpi_os_printf("\n%1c ", ACPI_DEBUGGER_COMMAND_PROMPT);
 
+	mutex_lock(&acpi_aml_io.lock);
+	acpi_aml_io.flags |= ACPI_AML_WAITING_CMD;
+	wake_up_interruptible(&acpi_aml_io.wait);
+	mutex_unlock(&acpi_aml_io.lock);
+
 	status = acpi_os_get_line(buffer, length, NULL);
 	if (ACPI_FAILURE(status))
 		return -EINVAL;
+
+	mutex_lock(&acpi_aml_io.lock);
+	acpi_aml_io.flags &= ~ACPI_AML_WAITING_CMD;
+	mutex_unlock(&acpi_aml_io.lock);
 	return 0;
 }
 

@@ -593,9 +638,11 @@ static int acpi_aml_read_user(char __user *buf, int len)
 	smp_rmb();
 	p = &crc->buf[crc->tail];
 	n = min(len, circ_count_to_end(crc));
-	if (copy_to_user(buf, p, n)) {
-		ret = -EFAULT;
-		goto out;
+	if (!acpi_aml_flushing_log()) {
+		if (copy_to_user(buf, p, n)) {
+			ret = -EFAULT;
+			goto out;
+		}
 	}
 	/* sync tail after removing logs */
 	smp_mb();

@@ -731,10 +778,40 @@ static unsigned int acpi_aml_poll(struct file *file, poll_table *wait)
 	return masks;
 }
 
+static int acpi_aml_flush(void)
+{
+	int ret;
+
+	/*
+	 * Discard output buffer and put the driver into a state waiting
+	 * for the new user input.
+	 */
+	mutex_lock(&acpi_aml_io.lock);
+	acpi_aml_io.flags |= ACPI_AML_FLUSHING_LOG;
+	mutex_unlock(&acpi_aml_io.lock);
+
+	ret = wait_event_interruptible(acpi_aml_io.wait,
+		acpi_aml_waiting_cmd());
+	(void)acpi_aml_read_user(NULL, ACPI_AML_BUF_SIZE);
+
+	mutex_lock(&acpi_aml_io.lock);
+	acpi_aml_io.flags &= ~ACPI_AML_FLUSHING_LOG;
+	mutex_unlock(&acpi_aml_io.lock);
+	return ret;
+}
+
+static long acpi_aml_ioctl(struct file *file,
+			   unsigned int cmd, unsigned long arg)
+{
+	return cmd == ACPI_IOCTL_DEBUGGER_FLUSH ?
+	       acpi_aml_flush() : -EINVAL;
+}
+
 static const struct file_operations acpi_aml_operations = {
 	.read		= acpi_aml_read,
 	.write		= acpi_aml_write,
 	.poll		= acpi_aml_poll,
+	.unlocked_ioctl	= acpi_aml_ioctl,
 	.open		= acpi_aml_open,
 	.release	= acpi_aml_release,
 	.llseek		= generic_file_llseek,
include/linux/acpi.h +1 -0
index 08235a6..9354fb8 100644
@@ -26,6 +26,7 @@
 #include <linux/resource_ext.h>
 #include <linux/device.h>
 #include <linux/property.h>
+#include <uapi/linux/acpi-ioctls.h>
 
 #ifndef _LINUX
 #define _LINUX
include/uapi/linux/acpi-ioctls.h new file +21 -0
new file mode 100644
index 0000000..71b891a
@@ -0,0 +1,21 @@
+/*
+ * ACPI IOCTL collections
+ *
+ * Copyright (C) 2016, Intel Corporation
+ * Authors: Lv Zheng <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef _UAPI_LINUX_ACPI_IOCTLS_H
+#define _UAPI_LINUX_ACPI_IOCTLS_H
+
+#include <linux/ioctl.h>
+
+#define ACPI_IOCTL_IDENT		'a'
+
+#define ACPI_IOCTL_DEBUGGER_FLUSH	_IO(ACPI_IOCTL_IDENT, 0x80)
+
+#endif /* _UAPI_LINUX_ACPI_IOCTLS_H */
Signature · 1 line
1.7.10
#19 lore raw
Quoted 6 lines · Lv Zheng

This patch adds debugger output flushing support in kernel via .ioctl() callback. The in-kernel flushing is more efficient, because it reduces useless log IOs by bypassing log user_read/kern_write during the flush period.

This mechanism is useful for the batch mode.

Is it only useful or is it required?

Also the batch mode is introduced by the remaining patches in the series, isn't it?

Quoted 2 lines

Scripts can integrate a batch mode acpidbg instance to perform AML debugger functionalities.

This sentence is not parsable for me. What does it mean, really?

Quoted 2 lines

As the batch mode always starts from a new command write, it thus requires the kernel debugger driver to drop the old input/output first.

What does "the old" mean here?

Quoted 2 lines

The old input is automatically dropped by acpi_os_get_line() via an error returning value,

I can't parse this too, sorry.

Quoted 4 lines

but the output are remained in acpi_dbg output buffers and should be dropped prior than reading the new command, otherwise, the old output can be read out by the batch mode instance and the result of the batch mode command will be messed up.

Can you give an example here for clarity, please?

Thanks,
Rafael

author
#20 lore raw

Hi, Rafael

Quoted 4 lines

From: Rafael J. Wysocki [mailto:[email protected]]
Subject: Re: [PATCH v4 1/3] ACPI / debugger: Add kernel flushing support

On Tuesday, July 26, 2016 07:01:33 PM Lv Zheng wrote:

Quoted 8 lines
Quoted 6 lines

This patch adds debugger output flushing support in kernel via .ioctl() callback. The in-kernel flushing is more efficient, because it reduces useless log IOs by bypassing log user_read/kern_write during the flush period.

This mechanism is useful for the batch mode.

Is it only useful or is it required?

Should be required.

Quoted 3 lines

Also the batch mode is introduced by the remaining patches in the series, isn't it?

No, the batch mode is already in the upstream.
It's acpidbg -b "debugger commands".
For example:
acpidbg "-b namespace"
It's currently working.

Quoted 5 lines
Quoted 3 lines

Scripts can integrate a batch mode acpidbg instance to perform AML debugger functionalities.

This sentence is not parsable for me. What does it mean, really?

By default, just like acpiexec, acpidbg is a shell like interactive utility. Running acpidbg enters an interactive acpidbg shell.
So it cannot be used by the shell scripts.

With acpidbg -b option, acpidbg executes 1 command and exits. So it can be used in a script like:

#!/bin/sh

acpidbg -b "find _LID"
acpidbg -b "namespace"

So that validators can use "acpidbg -b" to create recursive test cases.

Quoted 5 lines
Quoted 3 lines

As the batch mode always starts from a new command write, it thus requires the kernel debugger driver to drop the old input/output first.

What does "the old" mean here?

There are 2 cases, requiring the "flush" operation to be performed before an "acpidbg -b" execution.

A. The first case requiring flush is "prompt string":
After the kernel is booted, for the first acpidbg execution, user may choose to use it in either batch mode or in interactive mode.

A.1. First use is batch mode, for example, "acpidbg -b namespace"

1. acpi_dbg kernel driver outputs prompt strings into the output buffer.
2. acpidbg user program writes "namespace" command to the acpi_dbg kernel driver's input buffer.
3. acpi_dbg kernel driver reads the input buffer.
It then obtains the "namespace" command and starts to execute it.
4. acpi_dbg kernel driver put the command result into the output buffer.
And wait the userspace acpidbg to read the output.
5. acpidbg user program reads the acpi_dbg kernel driver's output buffer.
It then obtains both the unexpected "prompt string" and the expected command result.

If there is a "flush" between 1 and 2, acpidbg user program won't obtain the unexpected "prompt string".

A.2. First use is interactive mode

However, it is not ensured that the "prompt string" can always appear before the command result. For example:
1. User runs acpidbg with the interactive mode
2. acpi_dbg kernel driver outputs prompt strings into the output buffer.
3. acpidbg user program reads the output buffer and get the "prompt string".
4. User exits acpidbg.
5. User runs acpidbg with the batch mode.
In this case, "prompt string" has already been read by the 1st "acpidbg interactive execution" Thus the 2nd "acpidbg batch execution" won't read unexpected "prompt string".

There is no harm to have a "flush" between 4 and 5.

Conclusion:
Whether there is "prompt string" in the output buffer depends on whether the "acpidbg -b" is the first use. While running “flush" operation before batch mode execution can always drain the output buffer.

B. The second case requiring "flush" is the old output:

The old output can be remained in the output buffer because we allow "asynchronous termination" of acpi_dbg IO.

The use case is:

1. User runs acpidbg by executing a command in either interactive mode or in batch mode.
2. acpi_dbg kernel driver put the command result into the output buffer.
3. When the output buffer is full, acpi_dbg kernel driver blocks, to wait for the userspace to read the result.
4. Instead of reading the result, userspace can close the acpi_dbg IO.
User can achieve this by typing "ctrl+C" in acpidbg user program.
5. User runs acpidbg with the batch mode.
It then obtains both the unexpected "old output", "prompt string" and the expected command result.

If there is a "flush" between 4 and 5, acpidbg user program won't obtain the unexpected "old output" and "prompt string".

Quoted 5 lines
Quoted 3 lines

The old input is automatically dropped by acpi_os_get_line() via an error returning value,

I can't parse this too, sorry.

OK, so we don't need this statement.
The flush is mainly used to drain the output buffer. Because it can wrong output for the batch mode.

Quoted 7 lines
Quoted 5 lines

but the output are remained in acpi_dbg output buffers and should be dropped prior than reading the new command, otherwise, the old output can be read out by the batch mode instance and the result of the batch mode command will be messed up.

Can you give an example here for clarity, please?

Please find the 2 cases in A.1. and B.

Thanks and best regards
Lv

author
#21 lore raw

PATCH v4 2/3 tools/power/acpi/acpidbg: Use new flushing mechanism

In reply to Lv Zheng

This patch converts tools/power/acpi/tools/acpidbg/acpidbg to use the new flushing mechanism.

Signed-off-by Lv Zheng <[email protected]>
---
 tools/power/acpi/tools/acpidbg/acpidbg.c |   51 ++++--------------------------
 1 file changed, 7 insertions(+), 44 deletions(-)
tools/power/acpi/tools/acpidbg/acpidbg.c +7 -44
index a88ac45..f5542b9 100644
@@ -15,7 +15,9 @@
 #include <stdbool.h>
 #include <fcntl.h>
 #include <assert.h>
+#include <sys/ioctl.h>
 #include <linux/circ_buf.h>
+#include <uapi/linux/acpi-ioctls.h>
 
 #define ACPI_AML_FILE		"/sys/kernel/debug/acpi/acpidbg"
 #define ACPI_AML_SEC_TICK	1

@@ -83,7 +85,6 @@ static const char *acpi_aml_file_path = ACPI_AML_FILE;
 static unsigned long acpi_aml_mode = ACPI_AML_INTERACTIVE;
 static bool acpi_aml_exit;
 
-static bool acpi_aml_batch_drain;
 static unsigned long acpi_aml_batch_state;
 static char acpi_aml_batch_prompt;
 static char acpi_aml_batch_roll;

@@ -239,11 +240,9 @@ static int acpi_aml_write_batch_log(int fd, struct circ_buf *crc)
 
 	p = &crc->buf[crc->tail];
 	len = circ_count_to_end(crc);
-	if (!acpi_aml_batch_drain) {
-		len = write(fd, p, len);
-		if (len < 0)
-			perror("write");
-	}
+	len = write(fd, p, len);
+	if (len < 0)
+		perror("write");
 	if (len > 0)
 		crc->tail = (crc->tail + len) & (ACPI_AML_BUF_SIZE - 1);
 	return len;

@@ -270,10 +269,7 @@ static void acpi_aml_loop(int fd)
 	if (acpi_aml_mode == ACPI_AML_BATCH) {
 		acpi_aml_log_state = ACPI_AML_LOG_START;
 		acpi_aml_batch_pos = acpi_aml_batch_cmd;
-		if (acpi_aml_batch_drain)
-			acpi_aml_batch_state = ACPI_AML_BATCH_READ_LOG;
-		else
-			acpi_aml_batch_state = ACPI_AML_BATCH_WRITE_CMD;
+		acpi_aml_batch_state = ACPI_AML_BATCH_WRITE_CMD;
 	}
 	acpi_aml_exit = false;
 	while (!acpi_aml_exit) {

@@ -330,39 +326,6 @@ static void acpi_aml_loop(int fd)
 	}
 }
 
-static bool acpi_aml_readable(int fd)
-{
-	fd_set rfds;
-	struct timeval tv;
-	int ret;
-	int maxfd = 0;
-
-	tv.tv_sec = 0;
-	tv.tv_usec = ACPI_AML_USEC_PEEK;
-	FD_ZERO(&rfds);
-	maxfd = acpi_aml_set_fd(fd, maxfd, &rfds);
-	ret = select(maxfd+1, &rfds, NULL, NULL, &tv);
-	if (ret < 0)
-		perror("select");
-	if (ret > 0 && FD_ISSET(fd, &rfds))
-		return true;
-	return false;
-}
-
-/*
- * This is a userspace IO flush implementation, replying on the prompt
- * characters and can be turned into a flush() call after kernel implements
- * .flush() filesystem operation.
- */
-static void acpi_aml_flush(int fd)
-{
-	while (acpi_aml_readable(fd)) {
-		acpi_aml_batch_drain = true;
-		acpi_aml_loop(fd);
-		acpi_aml_batch_drain = false;
-	}
-}
-
 void usage(FILE *file, char *progname)
 {
 	fprintf(file, "usage: %s [-b cmd] [-f file] [-h]\n", progname);

@@ -426,7 +389,7 @@ int main(int argc, char **argv)
 	acpi_aml_set_fl(STDOUT_FILENO, O_NONBLOCK);
 
 	if (acpi_aml_mode == ACPI_AML_BATCH)
-		acpi_aml_flush(fd);
+		ioctl(fd, ACPI_IOCTL_DEBUGGER_FLUSH);
 	acpi_aml_loop(fd);
 
 exit:
Signature · 1 line
1.7.10
#22 lore raw

Re: [PATCH v4 2/3] tools/power/acpi/acpidbg: Use new flushing mechanism

Quoted 2 lines · Lv Zheng

This patch converts tools/power/acpi/tools/acpidbg/acpidbg to use the new flushing mechanism.

I guess it will use the flush interface provided by the kernel instead of the previously existing flush implementation in user space?

Thanks,
Rafael

author
#23 lore raw

RE: [PATCH v4 2/3] tools/power/acpi/acpidbg: Use new flushing mechanism

Hi, Rafael

Quoted 9 lines

From: Rafael J. Wysocki [mailto:[email protected]]
Subject: Re: [PATCH v4 2/3] tools/power/acpi/acpidbg: Use new flushing mechanism

Quoted 2 lines · Lv Zheng

This patch converts tools/power/acpi/tools/acpidbg/acpidbg to use the new flushing mechanism.

I guess it will use the flush interface provided by the kernel instead of the previously existing flush implementation in user space?

Yes.
The existing user space flush implementation is just a compromise during the period the kernel flush is not ready.

Thanks
Lv

author
#24 lore raw

PATCH v4 3/3 tools/power/acpi/acpidbg: Add multi-commands support in batch mode

In reply to Lv Zheng

This patch adds multi-commands support for the batch mode. The same mode can be seen in acpiexec.

However people may think this is not useful for an in-kernel debugger, because the in-kernel debugger is always running, never exits. So we can run another command by running another acpidbg batch mode instance.

But this mode should still be useful for acpidbg. The reason is: when the in-kernel debugger has entered the single-stepping mode, ending acpidbg (which closes the debugger IO interface) will lead to the end of the single-stepping mode.

So we need the acpidbg multi-commands batch mode in order to execute multiple single-stepping mode commands in scripts.

Signed-off-by Lv Zheng <[email protected]>
---
 tools/power/acpi/tools/acpidbg/acpidbg.c |   77 +++++++++++++++++++++++++-----
 1 file changed, 64 insertions(+), 13 deletions(-)
tools/power/acpi/tools/acpidbg/acpidbg.c +64 -13
index f5542b9..99a8f766 100644
@@ -90,6 +90,7 @@ static char acpi_aml_batch_prompt;
 static char acpi_aml_batch_roll;
 static unsigned long acpi_aml_log_state;
 static char *acpi_aml_batch_cmd = NULL;
+static char *acpi_aml_batch_cmds = NULL;
 static char *acpi_aml_batch_pos = NULL;
 
 static int acpi_aml_set_fl(int fd, int flags)

@@ -326,12 +327,65 @@ static void acpi_aml_loop(int fd)
 	}
 }
 
+static void acpi_aml_delete_batch(void)
+{
+	if (acpi_aml_batch_cmd) {
+		free(acpi_aml_batch_cmd);
+		acpi_aml_batch_cmd = NULL;
+	}
+}
+
+static bool acpi_aml_create_batch(char *cmd)
+{
+	int len;
+
+	acpi_aml_delete_batch();
+	len = strlen(cmd);
+	acpi_aml_batch_cmd = calloc(len + 2, 1);
+	if (!acpi_aml_batch_cmd) {
+		perror("calloc");
+		return false;
+	}
+	memcpy(acpi_aml_batch_cmd, cmd, len);
+	acpi_aml_batch_cmd[len] = '\n';
+	return true;
+}
+
+static void acpi_aml_batch(int fd)
+{
+	char *ptr, *cmd;
+	bool run = false;
+
+	cmd = ptr = acpi_aml_batch_cmds;
+	while (*ptr) {
+		if (*ptr == ',') {
+			/* Convert commas to spaces */
+			*ptr = ' ';
+		} else if (*ptr == ';') {
+			*ptr = '\0';
+			run = true;
+		}
+		ptr++;
+		if (run || (*ptr == '\0')) {
+			if (!acpi_aml_create_batch(cmd))
+				return;
+			ioctl(fd, ACPI_IOCTL_DEBUGGER_FLUSH);
+			acpi_aml_loop(fd);
+			run = 0;
+			cmd = ptr;
+			acpi_aml_delete_batch();
+		}
+	}
+}
+
 void usage(FILE *file, char *progname)
 {
 	fprintf(file, "usage: %s [-b cmd] [-f file] [-h]\n", progname);
 	fprintf(file, "\nOptions:\n");
-	fprintf(file, "  -b     Specify command to be executed in batch mode\n");
-	fprintf(file, "  -f     Specify interface file other than");
+	fprintf(file, "  -b     Specify commands to be executed in batch mode\n");
+	fprintf(file, "         Use ';' as command delimiters\n");
+	fprintf(file, "         Use ',' as spaces\n");
+	fprintf(file, "  -f     Specify interface file other than\n");
 	fprintf(file, "         /sys/kernel/debug/acpi/acpidbg\n");
 	fprintf(file, "  -h     Print this help message\n");
 }

@@ -340,27 +394,23 @@ int main(int argc, char **argv)
 {
 	int fd = -1;
 	int ch;
-	int len;
 	int ret = EXIT_SUCCESS;
 
 	while ((ch = getopt(argc, argv, "b:f:h")) != -1) {
 		switch (ch) {
 		case 'b':
-			if (acpi_aml_batch_cmd) {
+			if (acpi_aml_batch_cmds) {
 				fprintf(stderr, "Already specify %s\n",
-					acpi_aml_batch_cmd);
+					acpi_aml_batch_cmds);
 				ret = EXIT_FAILURE;
 				goto exit;
 			}
-			len = strlen(optarg);
-			acpi_aml_batch_cmd = calloc(len + 2, 1);
-			if (!acpi_aml_batch_cmd) {
-				perror("calloc");
+			acpi_aml_batch_cmds = strdup(optarg);
+			if (!acpi_aml_batch_cmds) {
+				perror("strdup");
 				ret = EXIT_FAILURE;
 				goto exit;
 			}
-			memcpy(acpi_aml_batch_cmd, optarg, len);
-			acpi_aml_batch_cmd[len] = '\n';
 			acpi_aml_mode = ACPI_AML_BATCH;
 			break;
 		case 'f':

@@ -389,8 +439,9 @@ int main(int argc, char **argv)
 	acpi_aml_set_fl(STDOUT_FILENO, O_NONBLOCK);
 
 	if (acpi_aml_mode == ACPI_AML_BATCH)
-		ioctl(fd, ACPI_IOCTL_DEBUGGER_FLUSH);
-	acpi_aml_loop(fd);
+		acpi_aml_batch(fd);
+	else
+		acpi_aml_loop(fd);
 
 exit:
 	if (fd >= 0)
Signature · 1 line
1.7.10
#25 lore raw

Re: [PATCH v4 3/3] tools/power/acpi/acpidbg: Add multi-commands support in batch mode

Quoted 7 lines · Lv Zheng

This patch adds multi-commands support for the batch mode. The same mode can be seen in acpiexec.

However people may think this is not useful for an in-kernel debugger, because the in-kernel debugger is always running, never exits. So we can run another command by running another acpidbg batch mode instance.

Quoted 7 lines

But this mode should still be useful for acpidbg. The reason is: when the in-kernel debugger has entered the single-stepping mode, ending acpidbg (which closes the debugger IO interface) will lead to the end of the single-stepping mode.

So we need the acpidbg multi-commands batch mode in order to execute multiple single-stepping mode commands in scripts.

An example would be really useful here IMO.

Thanks,
Rafael

author
#26 lore raw

RE: [PATCH v4 3/3] tools/power/acpi/acpidbg: Add multi-commands support in batch mode

Hi, Rafael

Quoted 16 lines

From: Rafael J. Wysocki [mailto:[email protected]]
Subject: Re: [PATCH v4 3/3] tools/power/acpi/acpidbg: Add multi-commands support in batch mode

Quoted 12 lines · Lv Zheng

This patch adds multi-commands support for the batch mode. The same mode can be seen in acpiexec.

However people may think this is not useful for an in-kernel debugger, because the in-kernel debugger is always running, never exits. So we can run another command by running another acpidbg batch mode instance.

But this mode should still be useful for acpidbg. The reason is: when the in-kernel debugger has entered the single-stepping mode, ending acpidbg (which closes the debugger IO interface) will lead to the end of the single-stepping mode.

Quoted 4 lines
Quoted 2 lines

So we need the acpidbg multi-commands batch mode in order to execute multiple single-stepping mode commands in scripts.

An example would be really useful here IMO.

Considering the following control method:

Name (TVAL, Zero)
Method (TMTD)
{
Inrement (TVAL)
}

When it is executed in acpiexec:
---
#!/bin/sh
acpiexec -b "ex \TMTD" -es dsdt.aml
acpiexec -b "ex \TVAL" -es dsdt.aml
---
The result is "0" for TVAL, because each acpiexec instance re-initializes the namespace. Thus acpiexec provides a multi-command batch mode:
---
#!/bin/sh
acpiexec -b "ex \TMTD; ex \TVAL" -es dsdt.aml
---
The result is "1" now.

Then for this case (I'll use it to compare acpidbg behavior):
---
#!/bin/sh
acpiexec -b "ex \TMTD" -es dsdt.aml
acpiexec -b "ex \TMTD; ex \TVAL" -es dsdt.aml
---
The result is "1".

Unlike acpiexec, whatever the AML debugger is initialized/terminated. The namespace won't be re-initialized.

So for the above cases:
---
#!/bin/sh
acpidbg -b "ex \TMTD"
acpidbg -b "ex \TVAL"
---
The result is "1".
---
#!/bin/sh
acpidbg -b "ex \TMTD"
acpidbg -b "ex \TMTD; ex \TVAL"
---
The result is "2", it should be no different than:
---
#!/bin/sh
acpidbg -b "ex \TMTD"
acpidbg -b "ex \TMTD"
acpidbg -b "ex \TVAL"
---
Thus I said:
People may think this (the multi-command support) is not useful for an in-kernel debugger.

But there is a special case for the single stepping mode.
---
#!/bin/sh
acpidbg -b "debug \TMTD"
acpidbg -b "locals"
---
The "debug" command is special, it puts AML debugger into single stepping mode. In this mode, user can use single stepping mode commands to debug the evaluation of the TMTD. The result of this script is:
There is no method currently executing.

Because for the kernel AML debugger, if we leave a \TMTD unfinished.
Then mutex held in this method could block normal kernel evaluations of the methods requiring same mutexes.

Thus if acpidbg exits (closing acpi_dbg IO), single stepping mode will also run into an end. See:
acpi_terminate_debugger() in acpi_aml_release().

So we need the multi-command batch mode for this case:
---
#!/bin/sh
acpidbg -b "debug \TMTD; locals"
---
The result of this script is:
No Local Variables are initialized for method (TMTD)

Best regards
Lv