Groups > linux.kernel > #1250106 > unrolled thread

[PATCH v2 3/3] pstore: add pstore unregister

Started by	Geliang Tang <geliangtang@163.com>
First post	2015-10-18 13:00 +0200
Last post	2015-10-21 18:40 +0200
Articles	11 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

  [PATCH v2 3/3] pstore: add pstore unregister Geliang Tang <geliangtang@163.com> - 2015-10-18 13:00 +0200
    RE: [PATCH v2 3/3] pstore: add pstore unregister "Luck, Tony" <tony.luck@intel.com> - 2015-10-20 01:00 +0200
      [PATCH v3 0/3] pstore: add pstore unregister Geliang Tang <geliangtang@163.com> - 2015-10-20 09:50 +0200
        [PATCH v3 2/3] pstore: add a helper function pstore_register_kmsg Geliang Tang <geliangtang@163.com> - 2015-10-20 09:50 +0200
        [PATCH v3 1/3] pstore: add vmalloc error check Geliang Tang <geliangtang@163.com> - 2015-10-20 09:50 +0200
        [PATCH v3 3/3] pstore: add pstore unregister Geliang Tang <geliangtang@163.com> - 2015-10-20 10:00 +0200
        Re: [PATCH v3 0/3] pstore: add pstore unregister Kees Cook <keescook@chromium.org> - 2015-10-20 19:20 +0200
          Re: [PATCH v3 0/3] pstore: add pstore unregister Geliang Tang <geliangtang@163.com> - 2015-10-21 05:00 +0200
            Re: [PATCH v3 0/3] pstore: add pstore unregister Kees Cook <keescook@chromium.org> - 2015-10-21 05:10 +0200
              Re: [PATCH v3 0/3] pstore: add pstore unregister Geliang Tang <geliangtang@163.com> - 2015-10-21 06:30 +0200
                RE: [PATCH v3 0/3] pstore: add pstore unregister "Luck, Tony" <tony.luck@intel.com> - 2015-10-21 18:40 +0200

#1250106 — [PATCH v2 3/3] pstore: add pstore unregister

From	Geliang Tang <geliangtang@163.com>
Date	2015-10-18 13:00 +0200
Subject	[PATCH v2 3/3] pstore: add pstore unregister
Message-ID	<qkTR0-2VE-3@gated-at.bofh.it>

pstore doesn't support unregistering yet. It was marked as TODO.
This patch adds some code to fix it:
 1) Add functions to unregister kmsg/console/ftrace/pmsg.
 2) Add a function to free compression buffer.
 3) Unmap the memory and free it.
 4) Add a function to unregister pstore filesystem.

Signed-off-by: Geliang Tang <geliangtang@163.com>
---
Changes in v2:
 - Add pstore filesystem unregister.
 - update commit log.
---
 fs/pstore/Kconfig      |  2 +-
 fs/pstore/Makefile     |  6 +++---
 fs/pstore/ftrace.c     | 23 ++++++++++++++++++-----
 fs/pstore/inode.c      |  7 +++++++
 fs/pstore/internal.h   |  4 ++++
 fs/pstore/platform.c   | 35 +++++++++++++++++++++++++++++++++++
 fs/pstore/pmsg.c       |  7 +++++++
 fs/pstore/ram.c        | 17 +++++++----------
 include/linux/pstore.h | 14 +-------------
 kernel/printk/printk.c |  1 +
 10 files changed, 84 insertions(+), 32 deletions(-)

diff --git a/fs/pstore/Kconfig b/fs/pstore/Kconfig
index 916b8e2..360ae43 100644
--- a/fs/pstore/Kconfig
+++ b/fs/pstore/Kconfig
@@ -1,5 +1,5 @@
 config PSTORE
-	bool "Persistent store support"
+	tristate "Persistent store support"
 	default n
 	select ZLIB_DEFLATE
 	select ZLIB_INFLATE
diff --git a/fs/pstore/Makefile b/fs/pstore/Makefile
index e647d8e..b8803cc 100644
--- a/fs/pstore/Makefile
+++ b/fs/pstore/Makefile
@@ -2,12 +2,12 @@
 # Makefile for the linux pstorefs routines.
 #
 
-obj-y += pstore.o
+obj-$(CONFIG_PSTORE) += pstore.o
 
 pstore-objs += inode.o platform.o
-obj-$(CONFIG_PSTORE_FTRACE)	+= ftrace.o
+pstore-$(CONFIG_PSTORE_FTRACE)	+= ftrace.o
 
-obj-$(CONFIG_PSTORE_PMSG)	+= pmsg.o
+pstore-$(CONFIG_PSTORE_PMSG)	+= pmsg.o
 
 ramoops-objs += ram.o ram_core.o
 obj-$(CONFIG_PSTORE_RAM)	+= ramoops.o
diff --git a/fs/pstore/ftrace.c b/fs/pstore/ftrace.c
index 76a4eeb..788600f 100644
--- a/fs/pstore/ftrace.c
+++ b/fs/pstore/ftrace.c
@@ -104,21 +104,22 @@ static const struct file_operations pstore_knob_fops = {
 	.write	= pstore_ftrace_knob_write,
 };
 
+static struct dentry *pstore_ftrace_dir;
+
 void pstore_register_ftrace(void)
 {
-	struct dentry *dir;
 	struct dentry *file;
 
 	if (!psinfo->write_buf)
 		return;
 
-	dir = debugfs_create_dir("pstore", NULL);
-	if (!dir) {
+	pstore_ftrace_dir = debugfs_create_dir("pstore", NULL);
+	if (!pstore_ftrace_dir) {
 		pr_err("%s: unable to create pstore directory\n", __func__);
 		return;
 	}
 
-	file = debugfs_create_file("record_ftrace", 0600, dir, NULL,
+	file = debugfs_create_file("record_ftrace", 0600, pstore_ftrace_dir, NULL,
 				   &pstore_knob_fops);
 	if (!file) {
 		pr_err("%s: unable to create record_ftrace file\n", __func__);
@@ -127,5 +128,17 @@ void pstore_register_ftrace(void)
 
 	return;
 err_file:
-	debugfs_remove(dir);
+	debugfs_remove(pstore_ftrace_dir);
+}
+
+void pstore_unregister_ftrace(void)
+{
+	mutex_lock(&pstore_ftrace_lock);
+	if (pstore_ftrace_enabled) {
+		unregister_ftrace_function(&pstore_ftrace_ops);
+		pstore_ftrace_enabled = 0;
+	}
+	mutex_unlock(&pstore_ftrace_lock);
+
+	debugfs_remove_recursive(pstore_ftrace_dir);
 }
diff --git a/fs/pstore/inode.c b/fs/pstore/inode.c
index 3adcc46..598b52a 100644
--- a/fs/pstore/inode.c
+++ b/fs/pstore/inode.c
@@ -479,5 +479,12 @@ out:
 }
 module_init(init_pstore_fs)
 
+static void __exit exit_pstore_fs(void)
+{
+	unregister_filesystem(&pstore_fs_type);
+	sysfs_remove_mount_point(fs_kobj, "pstore");
+}
+module_exit(exit_pstore_fs)
+
 MODULE_AUTHOR("Tony Luck <tony.luck@intel.com>");
 MODULE_LICENSE("GPL");
diff --git a/fs/pstore/internal.h b/fs/pstore/internal.h
index c36ba2c..96253c4 100644
--- a/fs/pstore/internal.h
+++ b/fs/pstore/internal.h
@@ -41,14 +41,18 @@ pstore_ftrace_decode_cpu(struct pstore_ftrace_record *rec)
 
 #ifdef CONFIG_PSTORE_FTRACE
 extern void pstore_register_ftrace(void);
+extern void pstore_unregister_ftrace(void);
 #else
 static inline void pstore_register_ftrace(void) {}
+static inline void pstore_unregister_ftrace(void) {}
 #endif
 
 #ifdef CONFIG_PSTORE_PMSG
 extern void pstore_register_pmsg(void);
+extern void pstore_unregister_pmsg(void);
 #else
 static inline void pstore_register_pmsg(void) {}
+static inline void pstore_unregister_pmsg(void) {}
 #endif
 
 extern struct pstore_info *psinfo;
diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c
index 1b11249..0aab920 100644
--- a/fs/pstore/platform.c
+++ b/fs/pstore/platform.c
@@ -237,6 +237,14 @@ static void allocate_buf_for_compression(void)
 
 }
 
+static void free_buf_for_compression(void)
+{
+	kfree(stream.workspace);
+	stream.workspace = NULL;
+	kfree(big_oops_buf);
+	big_oops_buf = NULL;
+}
+
 /*
  * Called when compression fails, since the printk buffer
  * would be fetched for compression calling it again when
@@ -358,6 +366,11 @@ static void pstore_register_kmsg(void)
 	kmsg_dump_register(&pstore_dumper);
 }
 
+static void pstore_unregister_kmsg(void)
+{
+	kmsg_dump_unregister(&pstore_dumper);
+}
+
 #ifdef CONFIG_PSTORE_CONSOLE
 static void pstore_console_write(struct console *con, const char *s, unsigned c)
 {
@@ -395,8 +408,14 @@ static void pstore_register_console(void)
 {
 	register_console(&pstore_console);
 }
+
+static void pstore_unregister_console(void)
+{
+	unregister_console(&pstore_console);
+}
 #else
 static void pstore_register_console(void) {}
+static void pstore_unregister_console(void) {}
 #endif
 
 static int pstore_write_compat(enum pstore_type_id type,
@@ -467,12 +486,28 @@ int pstore_register(struct pstore_info *psi)
 	 */
 	backend = psi->name;
 
+	module_put(owner);
+
 	pr_info("Registered %s as persistent store backend\n", psi->name);
 
 	return 0;
 }
 EXPORT_SYMBOL_GPL(pstore_register);
 
+void pstore_unregister(struct pstore_info *psi)
+{
+	pstore_unregister_pmsg();
+	pstore_unregister_ftrace();
+	pstore_unregister_console();
+	pstore_unregister_kmsg();
+
+	free_buf_for_compression();
+
+	psinfo = NULL;
+	backend = NULL;
+}
+EXPORT_SYMBOL_GPL(pstore_unregister);
+
 /*
  * Read all the records from the persistent store. Create
  * files in our filesystem.  Don't warn about -EEXIST errors
diff --git a/fs/pstore/pmsg.c b/fs/pstore/pmsg.c
index 5a2f05a..7de20cd 100644
--- a/fs/pstore/pmsg.c
+++ b/fs/pstore/pmsg.c
@@ -114,3 +114,10 @@ err_class:
 err:
 	return;
 }
+
+void pstore_unregister_pmsg(void)
+{
+	device_destroy(pmsg_class, MKDEV(pmsg_major, 0));
+	class_destroy(pmsg_class);
+	unregister_chrdev(pmsg_major, PMSG_NAME);
+}
diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
index 6c26c4d..68889a7 100644
--- a/fs/pstore/ram.c
+++ b/fs/pstore/ram.c
@@ -580,28 +580,25 @@ fail_out:
 
 static int __exit ramoops_remove(struct platform_device *pdev)
 {
-#if 0
-	/* TODO(kees): We cannot unload ramoops since pstore doesn't support
-	 * unregistering yet.
-	 */
 	struct ramoops_context *cxt = &oops_cxt;
 
-	iounmap(cxt->virt_addr);
-	release_mem_region(cxt->phys_addr, cxt->size);
+	pstore_unregister(&cxt->pstore);
 	cxt->max_dump_cnt = 0;
 
-	/* TODO(kees): When pstore supports unregistering, call it here. */
 	kfree(cxt->pstore.buf);
 	cxt->pstore.bufsize = 0;
 
+	persistent_ram_free(cxt->mprz);
+	persistent_ram_free(cxt->fprz);
+	persistent_ram_free(cxt->cprz);
+	ramoops_free_przs(cxt);
+
 	return 0;
-#endif
-	return -EBUSY;
 }
 
 static struct platform_driver ramoops_driver = {
 	.probe		= ramoops_probe,
-	.remove		= __exit_p(ramoops_remove),
+	.remove		= ramoops_remove,
 	.driver		= {
 		.name	= "ramoops",
 	},
diff --git a/include/linux/pstore.h b/include/linux/pstore.h
index 8e7a25b..831479f 100644
--- a/include/linux/pstore.h
+++ b/include/linux/pstore.h
@@ -75,20 +75,8 @@ struct pstore_info {
 
 #define	PSTORE_FLAGS_FRAGILE	1
 
-#ifdef CONFIG_PSTORE
 extern int pstore_register(struct pstore_info *);
+extern void pstore_unregister(struct pstore_info *);
 extern bool pstore_cannot_block_path(enum kmsg_dump_reason reason);
-#else
-static inline int
-pstore_register(struct pstore_info *psi)
-{
-	return -ENODEV;
-}
-static inline bool
-pstore_cannot_block_path(enum kmsg_dump_reason reason)
-{
-	return false;
-}
-#endif
 
 #endif /*_LINUX_PSTORE_H*/
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 8f0324e..b16f354 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -517,6 +517,7 @@ int check_syslog_permissions(int type, int source)
 ok:
 	return security_syslog(type);
 }
+EXPORT_SYMBOL_GPL(check_syslog_permissions);
 
 static void append_char(char **pp, char *e, char c)
 {
-- 
2.5.0


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1251142

From	"Luck, Tony" <tony.luck@intel.com>
Date	2015-10-20 01:00 +0200
Message-ID	<qlrzk-1Mv-9@gated-at.bofh.it>
In reply to	#1250106

> pstore doesn't support unregistering yet. It was marked as TODO.

Thanks for looking to close out this TODO item.

The thing that scared me about unloading pstore was what happens to
a process that is in the middle of reading some /sys/fs/pstore/file-name-here

Do we have all the right reference counts to make sure that process doesn't do
weird things if you rmmod pstore in the middle of a read? Or for a subsequent
read from the still-open file descriptor?

-Tony


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1251366 — [PATCH v3 0/3] pstore: add pstore unregister

From	Geliang Tang <geliangtang@163.com>
Date	2015-10-20 09:50 +0200
Subject	[PATCH v3 0/3] pstore: add pstore unregister
Message-ID	<qlzQd-5Bb-9@gated-at.bofh.it>
In reply to	#1251142

On Mon, Oct 19, 2015 at 10:56:54PM +0000, Luck, Tony wrote:
> Thanks for looking to close out this TODO item.
> 
> The thing that scared me about unloading pstore was what happens to
> a process that is in the middle of reading some /sys/fs/pstore/file-name-here
> 
> Do we have all the right reference counts to make sure that process doesn't do
> weird things if you rmmod pstore in the middle of a read? Or for a subsequent
> read from the still-open file descriptor?
> 
> -Tony

Thanks for your review. I updated the patches as you suggested.

// Increase a reference count when pstore file is read.
 static const struct file_operations pstore_file_operations = {
+       .owner          = THIS_MODULE,
        .open           = pstore_file_open,
        .read           = pstore_file_read,
        .llseek         = pstore_file_llseek,

// Increase a reference count when pstore is mounted.
 static struct file_system_type pstore_fs_type = {
+       .owner          = THIS_MODULE,
        .name           = "pstore",
        .mount          = pstore_mount,
        .kill_sb        = pstore_kill_sb,

---
Changes in v3:
 - Increase a reference count when pstore is used.
Changes in v2:
 - Add pstore filesystem unregister.
 - update commit log.
---

Geliang Tang (3):
  pstore: add vmalloc error check
  pstore: add a helper function pstore_register_kmsg
  pstore: add pstore unregister

 fs/pstore/Kconfig      |  2 +-
 fs/pstore/Makefile     |  6 +++---
 fs/pstore/ftrace.c     | 23 ++++++++++++++++++-----
 fs/pstore/inode.c      |  9 +++++++++
 fs/pstore/internal.h   |  4 ++++
 fs/pstore/platform.c   | 42 +++++++++++++++++++++++++++++++++++++++++-
 fs/pstore/pmsg.c       |  9 +++++++++
 fs/pstore/ram.c        | 17 +++++++----------
 include/linux/pstore.h | 14 +-------------
 kernel/printk/printk.c |  1 +
 10 files changed, 94 insertions(+), 33 deletions(-)

-- 
1.9.1


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1251370 — [PATCH v3 2/3] pstore: add a helper function pstore_register_kmsg

From	Geliang Tang <geliangtang@163.com>
Date	2015-10-20 09:50 +0200
Subject	[PATCH v3 2/3] pstore: add a helper function pstore_register_kmsg
Message-ID	<qlzQe-5Bb-21@gated-at.bofh.it>
In reply to	#1251366

Add a new wraper function pstore_register_kmsg to keep the
consistency with other similar pstore_register_* functions.

Signed-off-by: Geliang Tang <geliangtang@163.com>
---
 fs/pstore/platform.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c
index 791743d..1b11249 100644
--- a/fs/pstore/platform.c
+++ b/fs/pstore/platform.c
@@ -353,6 +353,11 @@ static struct kmsg_dumper pstore_dumper = {
 	.dump = pstore_dump,
 };
 
+static void pstore_register_kmsg(void)
+{
+	kmsg_dump_register(&pstore_dumper);
+}
+
 #ifdef CONFIG_PSTORE_CONSOLE
 static void pstore_console_write(struct console *con, const char *s, unsigned c)
 {
@@ -442,7 +447,7 @@ int pstore_register(struct pstore_info *psi)
 	if (pstore_is_mounted())
 		pstore_get_records(0);
 
-	kmsg_dump_register(&pstore_dumper);
+	pstore_register_kmsg();
 
 	if ((psi->flags & PSTORE_FLAGS_FRAGILE) == 0) {
 		pstore_register_console();
-- 
1.9.1


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1251373 — [PATCH v3 1/3] pstore: add vmalloc error check

From	Geliang Tang <geliangtang@163.com>
Date	2015-10-20 09:50 +0200
Subject	[PATCH v3 1/3] pstore: add vmalloc error check
Message-ID	<qlzQe-5Bb-27@gated-at.bofh.it>
In reply to	#1251366

When vmalloc is failed, let write_pmsg return -ENOMEM.

Signed-off-by: Geliang Tang <geliangtang@163.com>
Acked-by: Kees Cook <keescook@chromium.org>
---
 fs/pstore/pmsg.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/pstore/pmsg.c b/fs/pstore/pmsg.c
index feb5dd2..5a2f05a 100644
--- a/fs/pstore/pmsg.c
+++ b/fs/pstore/pmsg.c
@@ -37,6 +37,8 @@ static ssize_t write_pmsg(struct file *file, const char __user *buf,
 	if (buffer_size > PMSG_MAX_BOUNCE_BUFFER_SIZE)
 		buffer_size = PMSG_MAX_BOUNCE_BUFFER_SIZE;
 	buffer = vmalloc(buffer_size);
+	if (!buffer)
+		return -ENOMEM;
 
 	mutex_lock(&pmsg_lock);
 	for (i = 0; i < count; ) {
-- 
1.9.1


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1251405 — [PATCH v3 3/3] pstore: add pstore unregister

From	Geliang Tang <geliangtang@163.com>
Date	2015-10-20 10:00 +0200
Subject	[PATCH v3 3/3] pstore: add pstore unregister
Message-ID	<qlzZV-5Mu-25@gated-at.bofh.it>
In reply to	#1251366

pstore doesn't support unregistering yet. It was marked as TODO.
This patch adds some code to fix it:
 1) Add functions to unregister kmsg/console/ftrace/pmsg.
 2) Add a function to free compression buffer.
 3) Unmap the memory and free it.
 4) Add a function to unregister pstore filesystem.

Signed-off-by: Geliang Tang <geliangtang@163.com>
---
 fs/pstore/Kconfig      |  2 +-
 fs/pstore/Makefile     |  6 +++---
 fs/pstore/ftrace.c     | 23 ++++++++++++++++++-----
 fs/pstore/inode.c      |  9 +++++++++
 fs/pstore/internal.h   |  4 ++++
 fs/pstore/platform.c   | 35 +++++++++++++++++++++++++++++++++++
 fs/pstore/pmsg.c       |  7 +++++++
 fs/pstore/ram.c        | 17 +++++++----------
 include/linux/pstore.h | 14 +-------------
 kernel/printk/printk.c |  1 +
 10 files changed, 86 insertions(+), 32 deletions(-)

diff --git a/fs/pstore/Kconfig b/fs/pstore/Kconfig
index 916b8e2..360ae43 100644
--- a/fs/pstore/Kconfig
+++ b/fs/pstore/Kconfig
@@ -1,5 +1,5 @@
 config PSTORE
-	bool "Persistent store support"
+	tristate "Persistent store support"
 	default n
 	select ZLIB_DEFLATE
 	select ZLIB_INFLATE
diff --git a/fs/pstore/Makefile b/fs/pstore/Makefile
index e647d8e..b8803cc 100644
--- a/fs/pstore/Makefile
+++ b/fs/pstore/Makefile
@@ -2,12 +2,12 @@
 # Makefile for the linux pstorefs routines.
 #
 
-obj-y += pstore.o
+obj-$(CONFIG_PSTORE) += pstore.o
 
 pstore-objs += inode.o platform.o
-obj-$(CONFIG_PSTORE_FTRACE)	+= ftrace.o
+pstore-$(CONFIG_PSTORE_FTRACE)	+= ftrace.o
 
-obj-$(CONFIG_PSTORE_PMSG)	+= pmsg.o
+pstore-$(CONFIG_PSTORE_PMSG)	+= pmsg.o
 
 ramoops-objs += ram.o ram_core.o
 obj-$(CONFIG_PSTORE_RAM)	+= ramoops.o
diff --git a/fs/pstore/ftrace.c b/fs/pstore/ftrace.c
index 76a4eeb..788600f 100644
--- a/fs/pstore/ftrace.c
+++ b/fs/pstore/ftrace.c
@@ -104,21 +104,22 @@ static const struct file_operations pstore_knob_fops = {
 	.write	= pstore_ftrace_knob_write,
 };
 
+static struct dentry *pstore_ftrace_dir;
+
 void pstore_register_ftrace(void)
 {
-	struct dentry *dir;
 	struct dentry *file;
 
 	if (!psinfo->write_buf)
 		return;
 
-	dir = debugfs_create_dir("pstore", NULL);
-	if (!dir) {
+	pstore_ftrace_dir = debugfs_create_dir("pstore", NULL);
+	if (!pstore_ftrace_dir) {
 		pr_err("%s: unable to create pstore directory\n", __func__);
 		return;
 	}
 
-	file = debugfs_create_file("record_ftrace", 0600, dir, NULL,
+	file = debugfs_create_file("record_ftrace", 0600, pstore_ftrace_dir, NULL,
 				   &pstore_knob_fops);
 	if (!file) {
 		pr_err("%s: unable to create record_ftrace file\n", __func__);
@@ -127,5 +128,17 @@ void pstore_register_ftrace(void)
 
 	return;
 err_file:
-	debugfs_remove(dir);
+	debugfs_remove(pstore_ftrace_dir);
+}
+
+void pstore_unregister_ftrace(void)
+{
+	mutex_lock(&pstore_ftrace_lock);
+	if (pstore_ftrace_enabled) {
+		unregister_ftrace_function(&pstore_ftrace_ops);
+		pstore_ftrace_enabled = 0;
+	}
+	mutex_unlock(&pstore_ftrace_lock);
+
+	debugfs_remove_recursive(pstore_ftrace_dir);
 }
diff --git a/fs/pstore/inode.c b/fs/pstore/inode.c
index 3adcc46..3586491 100644
--- a/fs/pstore/inode.c
+++ b/fs/pstore/inode.c
@@ -178,6 +178,7 @@ static loff_t pstore_file_llseek(struct file *file, loff_t off, int whence)
 }
 
 static const struct file_operations pstore_file_operations = {
+	.owner		= THIS_MODULE,
 	.open		= pstore_file_open,
 	.read		= pstore_file_read,
 	.llseek		= pstore_file_llseek,
@@ -456,6 +457,7 @@ static void pstore_kill_sb(struct super_block *sb)
 }
 
 static struct file_system_type pstore_fs_type = {
+	.owner          = THIS_MODULE,
 	.name		= "pstore",
 	.mount		= pstore_mount,
 	.kill_sb	= pstore_kill_sb,
@@ -479,5 +481,12 @@ out:
 }
 module_init(init_pstore_fs)
 
+static void __exit exit_pstore_fs(void)
+{
+	unregister_filesystem(&pstore_fs_type);
+	sysfs_remove_mount_point(fs_kobj, "pstore");
+}
+module_exit(exit_pstore_fs)
+
 MODULE_AUTHOR("Tony Luck <tony.luck@intel.com>");
 MODULE_LICENSE("GPL");
diff --git a/fs/pstore/internal.h b/fs/pstore/internal.h
index c36ba2c..96253c4 100644
--- a/fs/pstore/internal.h
+++ b/fs/pstore/internal.h
@@ -41,14 +41,18 @@ pstore_ftrace_decode_cpu(struct pstore_ftrace_record *rec)
 
 #ifdef CONFIG_PSTORE_FTRACE
 extern void pstore_register_ftrace(void);
+extern void pstore_unregister_ftrace(void);
 #else
 static inline void pstore_register_ftrace(void) {}
+static inline void pstore_unregister_ftrace(void) {}
 #endif
 
 #ifdef CONFIG_PSTORE_PMSG
 extern void pstore_register_pmsg(void);
+extern void pstore_unregister_pmsg(void);
 #else
 static inline void pstore_register_pmsg(void) {}
+static inline void pstore_unregister_pmsg(void) {}
 #endif
 
 extern struct pstore_info *psinfo;
diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c
index 1b11249..0aab920 100644
--- a/fs/pstore/platform.c
+++ b/fs/pstore/platform.c
@@ -237,6 +237,14 @@ static void allocate_buf_for_compression(void)
 
 }
 
+static void free_buf_for_compression(void)
+{
+	kfree(stream.workspace);
+	stream.workspace = NULL;
+	kfree(big_oops_buf);
+	big_oops_buf = NULL;
+}
+
 /*
  * Called when compression fails, since the printk buffer
  * would be fetched for compression calling it again when
@@ -358,6 +366,11 @@ static void pstore_register_kmsg(void)
 	kmsg_dump_register(&pstore_dumper);
 }
 
+static void pstore_unregister_kmsg(void)
+{
+	kmsg_dump_unregister(&pstore_dumper);
+}
+
 #ifdef CONFIG_PSTORE_CONSOLE
 static void pstore_console_write(struct console *con, const char *s, unsigned c)
 {
@@ -395,8 +408,14 @@ static void pstore_register_console(void)
 {
 	register_console(&pstore_console);
 }
+
+static void pstore_unregister_console(void)
+{
+	unregister_console(&pstore_console);
+}
 #else
 static void pstore_register_console(void) {}
+static void pstore_unregister_console(void) {}
 #endif
 
 static int pstore_write_compat(enum pstore_type_id type,
@@ -467,12 +486,28 @@ int pstore_register(struct pstore_info *psi)
 	 */
 	backend = psi->name;
 
+	module_put(owner);
+
 	pr_info("Registered %s as persistent store backend\n", psi->name);
 
 	return 0;
 }
 EXPORT_SYMBOL_GPL(pstore_register);
 
+void pstore_unregister(struct pstore_info *psi)
+{
+	pstore_unregister_pmsg();
+	pstore_unregister_ftrace();
+	pstore_unregister_console();
+	pstore_unregister_kmsg();
+
+	free_buf_for_compression();
+
+	psinfo = NULL;
+	backend = NULL;
+}
+EXPORT_SYMBOL_GPL(pstore_unregister);
+
 /*
  * Read all the records from the persistent store. Create
  * files in our filesystem.  Don't warn about -EEXIST errors
diff --git a/fs/pstore/pmsg.c b/fs/pstore/pmsg.c
index 5a2f05a..7de20cd 100644
--- a/fs/pstore/pmsg.c
+++ b/fs/pstore/pmsg.c
@@ -114,3 +114,10 @@ err_class:
 err:
 	return;
 }
+
+void pstore_unregister_pmsg(void)
+{
+	device_destroy(pmsg_class, MKDEV(pmsg_major, 0));
+	class_destroy(pmsg_class);
+	unregister_chrdev(pmsg_major, PMSG_NAME);
+}
diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
index 6c26c4d..68889a7 100644
--- a/fs/pstore/ram.c
+++ b/fs/pstore/ram.c
@@ -580,28 +580,25 @@ fail_out:
 
 static int __exit ramoops_remove(struct platform_device *pdev)
 {
-#if 0
-	/* TODO(kees): We cannot unload ramoops since pstore doesn't support
-	 * unregistering yet.
-	 */
 	struct ramoops_context *cxt = &oops_cxt;
 
-	iounmap(cxt->virt_addr);
-	release_mem_region(cxt->phys_addr, cxt->size);
+	pstore_unregister(&cxt->pstore);
 	cxt->max_dump_cnt = 0;
 
-	/* TODO(kees): When pstore supports unregistering, call it here. */
 	kfree(cxt->pstore.buf);
 	cxt->pstore.bufsize = 0;
 
+	persistent_ram_free(cxt->mprz);
+	persistent_ram_free(cxt->fprz);
+	persistent_ram_free(cxt->cprz);
+	ramoops_free_przs(cxt);
+
 	return 0;
-#endif
-	return -EBUSY;
 }
 
 static struct platform_driver ramoops_driver = {
 	.probe		= ramoops_probe,
-	.remove		= __exit_p(ramoops_remove),
+	.remove		= ramoops_remove,
 	.driver		= {
 		.name	= "ramoops",
 	},
diff --git a/include/linux/pstore.h b/include/linux/pstore.h
index 8e7a25b..831479f 100644
--- a/include/linux/pstore.h
+++ b/include/linux/pstore.h
@@ -75,20 +75,8 @@ struct pstore_info {
 
 #define	PSTORE_FLAGS_FRAGILE	1
 
-#ifdef CONFIG_PSTORE
 extern int pstore_register(struct pstore_info *);
+extern void pstore_unregister(struct pstore_info *);
 extern bool pstore_cannot_block_path(enum kmsg_dump_reason reason);
-#else
-static inline int
-pstore_register(struct pstore_info *psi)
-{
-	return -ENODEV;
-}
-static inline bool
-pstore_cannot_block_path(enum kmsg_dump_reason reason)
-{
-	return false;
-}
-#endif
 
 #endif /*_LINUX_PSTORE_H*/
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 8f0324e..b16f354 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -517,6 +517,7 @@ int check_syslog_permissions(int type, int source)
 ok:
 	return security_syslog(type);
 }
+EXPORT_SYMBOL_GPL(check_syslog_permissions);
 
 static void append_char(char **pp, char *e, char c)
 {
-- 
1.9.1


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1251899 — Re: [PATCH v3 0/3] pstore: add pstore unregister

From	Kees Cook <keescook@chromium.org>
Date	2015-10-20 19:20 +0200
Subject	Re: [PATCH v3 0/3] pstore: add pstore unregister
Message-ID	<qlIJQ-1Ol-3@gated-at.bofh.it>
In reply to	#1251366

On Tue, Oct 20, 2015 at 12:39 AM, Geliang Tang <geliangtang@163.com> wrote:
> On Mon, Oct 19, 2015 at 10:56:54PM +0000, Luck, Tony wrote:
>> Thanks for looking to close out this TODO item.
>>
>> The thing that scared me about unloading pstore was what happens to
>> a process that is in the middle of reading some /sys/fs/pstore/file-name-here

Were you able to verify that this reading-while-rmmod case works correctly?

-Kees

>>
>> Do we have all the right reference counts to make sure that process doesn't do
>> weird things if you rmmod pstore in the middle of a read? Or for a subsequent
>> read from the still-open file descriptor?
>>
>> -Tony
>
> Thanks for your review. I updated the patches as you suggested.
>
> // Increase a reference count when pstore file is read.
>  static const struct file_operations pstore_file_operations = {
> +       .owner          = THIS_MODULE,
>         .open           = pstore_file_open,
>         .read           = pstore_file_read,
>         .llseek         = pstore_file_llseek,
>
> // Increase a reference count when pstore is mounted.
>  static struct file_system_type pstore_fs_type = {
> +       .owner          = THIS_MODULE,
>         .name           = "pstore",
>         .mount          = pstore_mount,
>         .kill_sb        = pstore_kill_sb,
>
> ---
> Changes in v3:
>  - Increase a reference count when pstore is used.
> Changes in v2:
>  - Add pstore filesystem unregister.
>  - update commit log.
> ---
>
> Geliang Tang (3):
>   pstore: add vmalloc error check
>   pstore: add a helper function pstore_register_kmsg
>   pstore: add pstore unregister
>
>  fs/pstore/Kconfig      |  2 +-
>  fs/pstore/Makefile     |  6 +++---
>  fs/pstore/ftrace.c     | 23 ++++++++++++++++++-----
>  fs/pstore/inode.c      |  9 +++++++++
>  fs/pstore/internal.h   |  4 ++++
>  fs/pstore/platform.c   | 42 +++++++++++++++++++++++++++++++++++++++++-
>  fs/pstore/pmsg.c       |  9 +++++++++
>  fs/pstore/ram.c        | 17 +++++++----------
>  include/linux/pstore.h | 14 +-------------
>  kernel/printk/printk.c |  1 +
>  10 files changed, 94 insertions(+), 33 deletions(-)
>
> --
> 1.9.1
>
>



-- 
Kees Cook
Chrome OS Security
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1252439 — Re: [PATCH v3 0/3] pstore: add pstore unregister

From	Geliang Tang <geliangtang@163.com>
Date	2015-10-21 05:00 +0200
Subject	Re: [PATCH v3 0/3] pstore: add pstore unregister
Message-ID	<qlRN7-6zC-5@gated-at.bofh.it>
In reply to	#1251899

On Tue, Oct 20, 2015 at 10:19:09AM -0700, Kees Cook wrote:
> On Tue, Oct 20, 2015 at 12:39 AM, Geliang Tang <geliangtang@163.com> wrote:
> > On Mon, Oct 19, 2015 at 10:56:54PM +0000, Luck, Tony wrote:
> >> Thanks for looking to close out this TODO item.
> >>
> >> The thing that scared me about unloading pstore was what happens to
> >> a process that is in the middle of reading some /sys/fs/pstore/file-name-here
> 
> Were you able to verify that this reading-while-rmmod case works correctly?
> 
> -Kees

$ sudo insmod zlib_deflate.ko
$ sudo insmod pstore.ko
$ lsmod
Module                  Size  Used by
pstore                 11225  0
zlib_deflate           18292  1 pstore

$ sudo mount -t pstore pstore /sys/fs/pstore
$ lsmod
Module                  Size  Used by
pstore                 11225  1
zlib_deflate           18292  1 pstore

$ sudo insmod reed_solomon.ko
$ sudo insmod ramoops.ko mem_address=0x40000000 mem_size=0x400000
$ lsmod
Module                  Size  Used by
ramoops                 9638  0
reed_solomon            5150  1 ramoops
pstore                 11225  2 ramoops
zlib_deflate           18292  1 pstore

$ tail -f /sys/fs/pstore/console-ramoops-0 &
[1] 3483
$ lsmod
Module                  Size  Used by
ramoops                 9638  0
reed_solomon            5150  1 ramoops
pstore                 11225  3 ramoops
zlib_deflate           18292  1 pstore

$ kill -9 3483
$ lsmod
Module                  Size  Used by
ramoops                 9638  0
reed_solomon            5150  1 ramoops
pstore                 11225  2 ramoops
zlib_deflate           18292  1 pstore

$ sudo rmmod ramoops
$ lsmod
Module                  Size  Used by
reed_solomon            5150  0
pstore                 11225  1
zlib_deflate           18292  1 pstore

$ sudo umount /sys/fs/pstore/
$ lsmod
Module                  Size  Used by
reed_solomon            5150  0 
pstore                 11225  0 
zlib_deflate           18292  1 pstore

$ sudo rmmod pstore
$ lsmod
Module                  Size  Used by
reed_solomon            5150  0 
zlib_deflate           18292  0

Thanks.
Geliang Tang

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1252444 — Re: [PATCH v3 0/3] pstore: add pstore unregister

From	Kees Cook <keescook@chromium.org>
Date	2015-10-21 05:10 +0200
Subject	Re: [PATCH v3 0/3] pstore: add pstore unregister
Message-ID	<qlRWO-706-15@gated-at.bofh.it>
In reply to	#1252439

On Tue, Oct 20, 2015 at 7:52 PM, Geliang Tang <geliangtang@163.com> wrote:
> On Tue, Oct 20, 2015 at 10:19:09AM -0700, Kees Cook wrote:
>> On Tue, Oct 20, 2015 at 12:39 AM, Geliang Tang <geliangtang@163.com> wrote:
>> > On Mon, Oct 19, 2015 at 10:56:54PM +0000, Luck, Tony wrote:
>> >> Thanks for looking to close out this TODO item.
>> >>
>> >> The thing that scared me about unloading pstore was what happens to
>> >> a process that is in the middle of reading some /sys/fs/pstore/file-name-here
>>
>> Were you able to verify that this reading-while-rmmod case works correctly?
>>
>> -Kees
>
> $ sudo insmod zlib_deflate.ko
> $ sudo insmod pstore.ko
> $ lsmod
> Module                  Size  Used by
> pstore                 11225  0
> zlib_deflate           18292  1 pstore
>
> $ sudo mount -t pstore pstore /sys/fs/pstore
> $ lsmod
> Module                  Size  Used by
> pstore                 11225  1
> zlib_deflate           18292  1 pstore
>
> $ sudo insmod reed_solomon.ko
> $ sudo insmod ramoops.ko mem_address=0x40000000 mem_size=0x400000
> $ lsmod
> Module                  Size  Used by
> ramoops                 9638  0
> reed_solomon            5150  1 ramoops
> pstore                 11225  2 ramoops
> zlib_deflate           18292  1 pstore
>
> $ tail -f /sys/fs/pstore/console-ramoops-0 &
> [1] 3483
> $ lsmod
> Module                  Size  Used by
> ramoops                 9638  0
> reed_solomon            5150  1 ramoops
> pstore                 11225  3 ramoops
> zlib_deflate           18292  1 pstore
>
> $ kill -9 3483
> $ lsmod
> Module                  Size  Used by
> ramoops                 9638  0
> reed_solomon            5150  1 ramoops
> pstore                 11225  2 ramoops
> zlib_deflate           18292  1 pstore
>
> $ sudo rmmod ramoops
> $ lsmod
> Module                  Size  Used by
> reed_solomon            5150  0
> pstore                 11225  1
> zlib_deflate           18292  1 pstore

What happens if you leave the tail running and try to rmmod ramoops?
(I assume it'll just refuse.)

Nice!

Acked-by: Kees Cook <keescook@chromium.org>

-Kees

>
> $ sudo umount /sys/fs/pstore/
> $ lsmod
> Module                  Size  Used by
> reed_solomon            5150  0
> pstore                 11225  0
> zlib_deflate           18292  1 pstore
>
> $ sudo rmmod pstore
> $ lsmod
> Module                  Size  Used by
> reed_solomon            5150  0
> zlib_deflate           18292  0
>
> Thanks.
> Geliang Tang
>



-- 
Kees Cook
Chrome OS Security
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1252460 — Re: [PATCH v3 0/3] pstore: add pstore unregister

From	Geliang Tang <geliangtang@163.com>
Date	2015-10-21 06:30 +0200
Subject	Re: [PATCH v3 0/3] pstore: add pstore unregister
Message-ID	<qlTce-ll-7@gated-at.bofh.it>
In reply to	#1252444

On Tue, Oct 20, 2015 at 08:01:20PM -0700, Kees Cook wrote:
> On Tue, Oct 20, 2015 at 7:52 PM, Geliang Tang <geliangtang@163.com> wrote:
> > On Tue, Oct 20, 2015 at 10:19:09AM -0700, Kees Cook wrote:
> >> On Tue, Oct 20, 2015 at 12:39 AM, Geliang Tang <geliangtang@163.com> wrote:
> >> > On Mon, Oct 19, 2015 at 10:56:54PM +0000, Luck, Tony wrote:
> >> >> Thanks for looking to close out this TODO item.
> >> >>
> >> >> The thing that scared me about unloading pstore was what happens to
> >> >> a process that is in the middle of reading some /sys/fs/pstore/file-name-here
> >>
> >> Were you able to verify that this reading-while-rmmod case works correctly?
> >>
> >> -Kees
> >
> > $ sudo insmod zlib_deflate.ko
> > $ sudo insmod pstore.ko
> > $ lsmod
> > Module                  Size  Used by
> > pstore                 11225  0
> > zlib_deflate           18292  1 pstore
> >
> > $ sudo mount -t pstore pstore /sys/fs/pstore
> > $ lsmod
> > Module                  Size  Used by
> > pstore                 11225  1
> > zlib_deflate           18292  1 pstore
> >
> > $ sudo insmod reed_solomon.ko
> > $ sudo insmod ramoops.ko mem_address=0x40000000 mem_size=0x400000
> > $ lsmod
> > Module                  Size  Used by
> > ramoops                 9638  0
> > reed_solomon            5150  1 ramoops
> > pstore                 11225  2 ramoops
> > zlib_deflate           18292  1 pstore
> >
> > $ tail -f /sys/fs/pstore/console-ramoops-0 &
> > [1] 3483
> > $ lsmod
> > Module                  Size  Used by
> > ramoops                 9638  0
> > reed_solomon            5150  1 ramoops
> > pstore                 11225  3 ramoops
> > zlib_deflate           18292  1 pstore
> >
> > $ kill -9 3483
> > $ lsmod
> > Module                  Size  Used by
> > ramoops                 9638  0
> > reed_solomon            5150  1 ramoops
> > pstore                 11225  2 ramoops
> > zlib_deflate           18292  1 pstore
> >
> > $ sudo rmmod ramoops
> > $ lsmod
> > Module                  Size  Used by
> > reed_solomon            5150  0
> > pstore                 11225  1
> > zlib_deflate           18292  1 pstore
> 
> What happens if you leave the tail running and try to rmmod ramoops?
> (I assume it'll just refuse.)
> 

It'll refuse if we try to unload pstore module. But we can unload
ramoops module. Because we only increase a reference count of pstore
module when the file is opened. We didn't increase ramoops module's
reference count.

Thanks.
Geliang Tang

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1253043 — RE: [PATCH v3 0/3] pstore: add pstore unregister

From	"Luck, Tony" <tony.luck@intel.com>
Date	2015-10-21 18:40 +0200
Subject	RE: [PATCH v3 0/3] pstore: add pstore unregister
Message-ID	<qm4AG-at-5@gated-at.bofh.it>
In reply to	#1252460

> It'll refuse if we try to unload pstore module. But we can unload
> ramoops module. Because we only increase a reference count of pstore
> module when the file is opened. We didn't increase ramoops module's
> reference count.

Thanks.  Series applied.

I changed:

part1: re-worded commit a little
part2: spelling fix s/wraper/wrapper/
part3: fixed one "line over 80 character" checkpatch warning

Assuming the 0-day robots don't find any additional problems this should
show up in linux-next in a day or two. I'll ask Linus to pull in the 4.4 merge
window.

-Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

csiph-web

[PATCH v2 3/3] pstore: add pstore unregister

Contents

#1250106 — [PATCH v2 3/3] pstore: add pstore unregister

#1251142

#1251366 — [PATCH v3 0/3] pstore: add pstore unregister

#1251370 — [PATCH v3 2/3] pstore: add a helper function pstore_register_kmsg

#1251373 — [PATCH v3 1/3] pstore: add vmalloc error check

#1251405 — [PATCH v3 3/3] pstore: add pstore unregister

#1251899 — Re: [PATCH v3 0/3] pstore: add pstore unregister

#1252439 — Re: [PATCH v3 0/3] pstore: add pstore unregister

#1252444 — Re: [PATCH v3 0/3] pstore: add pstore unregister

#1252460 — Re: [PATCH v3 0/3] pstore: add pstore unregister

#1253043 — RE: [PATCH v3 0/3] pstore: add pstore unregister