Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1443035 > unrolled thread

[PATCH 0/3] ACPI / debugger: Add kernel flushing support

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/3] ACPI / debugger: Add kernel flushing support Lv Zheng <lv.zheng@intel.com> - 2016-07-14 05:00 +0200
    [PATCH 1/3] debugfs: Add .fsync() callback proxy support Lv Zheng <lv.zheng@intel.com> - 2016-07-14 05:00 +0200
      RE: [PATCH 1/3] debugfs: Add .fsync() callback proxy support "Zheng, Lv" <lv.zheng@intel.com> - 2016-07-19 10:20 +0200
    [PATCH v2 2/2] tools/power/acpi/acpidbg: Use new flushing mechanism Lv Zheng <lv.zheng@intel.com> - 2016-07-19 12:10 +0200
    [PATCH v2 0/2] ACPI / debugger: Add kernel flushing support Lv Zheng <lv.zheng@intel.com> - 2016-07-19 12:10 +0200
      [PATCH v2 1/2] ACPI / debugger: Add kernel flushing support Lv Zheng <lv.zheng@intel.com> - 2016-07-19 12:10 +0200
        Re: [PATCH v2 1/2] ACPI / debugger: Add kernel flushing support "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-07-21 15:40 +0200
          RE: [PATCH v2 1/2] ACPI / debugger: Add kernel flushing support "Zheng, Lv" <lv.zheng@intel.com> - 2016-07-22 02:40 +0200
    [PATCH] tools/power/acpi/tools/acpidbg: Add multi-commands support in batch mode Lv Zheng <lv.zheng@intel.com> - 2016-07-20 10:20 +0200
      Re: [PATCH] tools/power/acpi/tools/acpidbg: Add multi-commands support in batch mode "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-07-21 15:50 +0200
        RE: [PATCH] tools/power/acpi/tools/acpidbg: Add multi-commands  support in batch mode "Zheng, Lv" <lv.zheng@intel.com> - 2016-07-22 02:30 +0200
    [PATCH v3 2/3] tools/power/acpi/acpidbg: Use new flushing mechanism Lv Zheng <lv.zheng@intel.com> - 2016-07-22 06:20 +0200
    [PATCH v3 0/3] ACPI / debugger: Add kernel flushing support Lv Zheng <lv.zheng@intel.com> - 2016-07-22 06:20 +0200
      [PATCH v3 1/3] ACPI / debugger: Add kernel flushing support Lv Zheng <lv.zheng@intel.com> - 2016-07-22 06:20 +0200
      [PATCH v3 3/3] tools/power/acpi/acpidbg: Add multi-commands support in batch mode Lv Zheng <lv.zheng@intel.com> - 2016-07-22 06:20 +0200

#1443035 — [PATCH 0/3] 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(-)

-- 
1.7.10

[toc] | [next] | [standalone]

#1443036 — [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 <lv.zheng@intel.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 fs/debugfs/file.c |    6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
index 592059f..f863a0c 100644
--- a/fs/debugfs/file.c
+++ b/fs/debugfs/file.c
@@ -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)
-- 
1.7.10

[toc] | [prev] | [next] | [standalone]

#1446224 — 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

> 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 <lv.zheng@intel.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>  fs/debugfs/file.c |    6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
> index 592059f..f863a0c 100644
> --- a/fs/debugfs/file.c
> +++ b/fs/debugfs/file.c
> @@ -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)
> --
> 1.7.10

[toc] | [prev] | [next] | [standalone]

#1446305 — [PATCH v2 2/2] tools/power/acpi/acpidbg: Use new flushing mechanism

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

Signed-off-by: Lv Zheng <lv.zheng@intel.com>
---
 tools/power/acpi/tools/acpidbg/acpidbg.c |   51 ++++--------------------------
 1 file changed, 7 insertions(+), 44 deletions(-)

diff --git a/tools/power/acpi/tools/acpidbg/acpidbg.c b/tools/power/acpi/tools/acpidbg/acpidbg.c
index a88ac45..0aba129 100644
--- a/tools/power/acpi/tools/acpidbg/acpidbg.c
+++ b/tools/power/acpi/tools/acpidbg/acpidbg.c
@@ -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:
-- 
1.7.10

[toc] | [prev] | [next] | [standalone]

#1446306 — [PATCH v2 0/2] 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 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

-- 
1.7.10

[toc] | [prev] | [next] | [standalone]

#1446309 — [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 <lv.zheng@intel.com>
---
 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

diff --git a/drivers/acpi/acpi_dbg.c b/drivers/acpi/acpi_dbg.c
index dee8692..6ac1388 100644
--- a/drivers/acpi/acpi_dbg.c
+++ b/drivers/acpi/acpi_dbg.c
@@ -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,
diff --git a/include/linux/acpi-ioctls.h b/include/linux/acpi-ioctls.h
new file mode 100644
index 0000000..56b8170
--- /dev/null
+++ b/include/linux/acpi-ioctls.h
@@ -0,0 +1,21 @@
+/*
+ * ACPI IOCTL collections
+ *
+ * Copyright (C) 2016, Intel Corporation
+ * Authors: Lv Zheng <lv.zheng@intel.com>
+ *
+ * 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 */
-- 
1.7.10

[toc] | [prev] | [next] | [standalone]

#1447886 — Re: [PATCH v2 1/2] ACPI / debugger: Add kernel flushing support

On Tuesday, July 19, 2016 06:00:39 PM Lv Zheng wrote:
> 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 <lv.zheng@intel.com>

Overall, this is an optimization, right?

So is adding a new IOCTL really worth it?

> ---
>  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
> 
> diff --git a/drivers/acpi/acpi_dbg.c b/drivers/acpi/acpi_dbg.c
> index dee8692..6ac1388 100644
> --- a/drivers/acpi/acpi_dbg.c
> +++ b/drivers/acpi/acpi_dbg.c
> @@ -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.

>  #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;

Oh well.

What about

	return !!(acpi_aml_io.flags & ACPI_AML_WAITING_CMD);

> +}
> +
> +static inline bool __acpi_aml_flushing_log(void)
> +{
> +	if (acpi_aml_io.flags & ACPI_AML_FLUSHING_LOG)
> +		return true;
> +	return false;

And analogously here?

> +}
> +
>  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;

	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,
> diff --git a/include/linux/acpi-ioctls.h b/include/linux/acpi-ioctls.h
> new file mode 100644
> index 0000000..56b8170
> --- /dev/null
> +++ b/include/linux/acpi-ioctls.h

include/uapi/linux/

> @@ -0,0 +1,21 @@
> +/*
> + * ACPI IOCTL collections
> + *
> + * Copyright (C) 2016, Intel Corporation
> + * Authors: Lv Zheng <lv.zheng@intel.com>
> + *
> + * 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 */
> 

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

Thanks,
Rafael

[toc] | [prev] | [next] | [standalone]

#1448277 — RE: [PATCH v2 1/2] ACPI / debugger: Add kernel flushing support

Hi,

> From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-
> owner@vger.kernel.org] On Behalf Of Rafael J. Wysocki
> Subject: Re: [PATCH v2 1/2] ACPI / debugger: Add kernel flushing support
> 
> On Tuesday, July 19, 2016 06:00:39 PM Lv Zheng wrote:
> > 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 <lv.zheng@intel.com>
> 
> 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.

> 
> > ---
> >  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
> >
> > diff --git a/drivers/acpi/acpi_dbg.c b/drivers/acpi/acpi_dbg.c
> > index dee8692..6ac1388 100644
> > --- a/drivers/acpi/acpi_dbg.c
> > +++ b/drivers/acpi/acpi_dbg.c
> > @@ -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.

> 
> >  #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;
> 
> Oh well.
> 
> What about
[Lv Zheng] 
OK.

> 
> 	return !!(acpi_aml_io.flags & ACPI_AML_WAITING_CMD);
> 
> > +}
> > +
> > +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.

> 
> > +}
> > +
> >  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;
> 
> 	return cmd == ACPI_IOCTL_DEBUGGER_FLUSH ? acpi_aml_flush() :
> -EINVAL;
[Lv Zheng] 
OK.

> 
> > +}
> > +
> >  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,
> > diff --git a/include/linux/acpi-ioctls.h b/include/linux/acpi-ioctls.h
> > new file mode 100644
> > index 0000000..56b8170
> > --- /dev/null
> > +++ b/include/linux/acpi-ioctls.h
> 
> include/uapi/linux/
[Lv Zheng] 
OK.

> 
> > @@ -0,0 +1,21 @@
> > +/*
> > + * ACPI IOCTL collections
> > + *
> > + * Copyright (C) 2016, Intel Corporation
> > + * Authors: Lv Zheng <lv.zheng@intel.com>
> > + *
> > + * 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 */
> >
> 
> Plus patches that change the ABI should be CCed to the ABI review list for,
> well, review.
[Lv Zheng] 
OK.

Thanks,
-Lv

[toc] | [prev] | [next] | [standalone]

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

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 <lv.zheng@intel.com>
---
 tools/power/acpi/tools/acpidbg/acpidbg.c |   77 +++++++++++++++++++++++++-----
 1 file changed, 64 insertions(+), 13 deletions(-)

diff --git a/tools/power/acpi/tools/acpidbg/acpidbg.c b/tools/power/acpi/tools/acpidbg/acpidbg.c
index 0aba129..330e5fe 100644
--- a/tools/power/acpi/tools/acpidbg/acpidbg.c
+++ b/tools/power/acpi/tools/acpidbg/acpidbg.c
@@ -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)
-- 
1.7.10

[toc] | [prev] | [next] | [standalone]

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

On Wednesday, July 20, 2016 04:12:08 PM Lv Zheng wrote:
> 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 <lv.zheng@intel.com>

Does this depend on the kernel flushing support series?

Thanks,
Rafael

[toc] | [prev] | [next] | [standalone]

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

Hi, Rafael

> From: Rafael J. Wysocki [mailto:rjw@rjwysocki.net]
> Subject: Re: [PATCH] tools/power/acpi/tools/acpidbg: Add multi-
> commands support in batch mode
> 
> On Wednesday, July 20, 2016 04:12:08 PM Lv Zheng wrote:
> > 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 <lv.zheng@intel.com>
> 
> 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

[toc] | [prev] | [next] | [standalone]

#1448381 — [PATCH v3 2/3] tools/power/acpi/acpidbg: Use new flushing mechanism

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

Signed-off-by: Lv Zheng <lv.zheng@intel.com>
---
 tools/power/acpi/tools/acpidbg/acpidbg.c |   51 ++++--------------------------
 1 file changed, 7 insertions(+), 44 deletions(-)

diff --git a/tools/power/acpi/tools/acpidbg/acpidbg.c b/tools/power/acpi/tools/acpidbg/acpidbg.c
index a88ac45..f5542b9 100644
--- a/tools/power/acpi/tools/acpidbg/acpidbg.c
+++ b/tools/power/acpi/tools/acpidbg/acpidbg.c
@@ -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:
-- 
1.7.10

[toc] | [prev] | [next] | [standalone]

#1448382 — [PATCH v3 0/3] 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 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

-- 
1.7.10

[toc] | [prev] | [next] | [standalone]

#1448384 — [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 <lv.zheng@intel.com>
Cc: linux-api@vger.kernel.org
---
 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

diff --git a/drivers/acpi/acpi_dbg.c b/drivers/acpi/acpi_dbg.c
index dee8692..a5f4457 100644
--- a/drivers/acpi/acpi_dbg.c
+++ b/drivers/acpi/acpi_dbg.c
@@ -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,
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 08235a6..9354fb8 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -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
diff --git a/include/uapi/linux/acpi-ioctls.h b/include/uapi/linux/acpi-ioctls.h
new file mode 100644
index 0000000..71b891a
--- /dev/null
+++ b/include/uapi/linux/acpi-ioctls.h
@@ -0,0 +1,21 @@
+/*
+ * ACPI IOCTL collections
+ *
+ * Copyright (C) 2016, Intel Corporation
+ * Authors: Lv Zheng <lv.zheng@intel.com>
+ *
+ * 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 */
-- 
1.7.10

[toc] | [prev] | [next] | [standalone]

#1448389 — [PATCH v3 3/3] tools/power/acpi/acpidbg: Add multi-commands support in batch mode

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 <lv.zheng@intel.com>
---
 tools/power/acpi/tools/acpidbg/acpidbg.c |   77 +++++++++++++++++++++++++-----
 1 file changed, 64 insertions(+), 13 deletions(-)

diff --git a/tools/power/acpi/tools/acpidbg/acpidbg.c b/tools/power/acpi/tools/acpidbg/acpidbg.c
index f5542b9..99a8f766 100644
--- a/tools/power/acpi/tools/acpidbg/acpidbg.c
+++ b/tools/power/acpi/tools/acpidbg/acpidbg.c
@@ -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)
-- 
1.7.10

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web