Hi pstore maintainers,
Recently we are trying to use pmsg ramoops to store relatively large
file. We found that even with enough space configured with pmsg-size,
it still failed because of kmalloc limitation.
We are thinking of using kvmalloc instead of kmalloc for ramoops pmsg
record buffer. It seems ok to us that the memory is not physically
contiguous. We want to consult with you whether it is safe to do so.
Below is the patch file, note that the change only touches the pmsg
ram path, we don't touch other paths like pstore zone. It is not yet a
solution for all the use case in pstore, just an idea of how we want
to address the issue. We test this on 5.15 and large file works fine.
-----------------------------------
From 5f7ac624d47b49ea827c1ad8e3b47534e0709ea3 Mon Sep 17 00:00:00 2001
From: Yuxiao Zhang <[email protected]>
Date: Mon, 26 Jun 2023 16:40:40 -0700
Subject: [PATCH] pstore: ramoops: support pmsg size larger than kmalloc
limitation
Current pmsg implementation is using kmalloc for pmsg record buffer,
which has max size limits based on page size. Currently even we
allocate enough space with pmsg-size, pmsg will still fail if the
file size is larger than what kmalloc allowed.
Since we don't need physical contiguous memory for pmsg buffer
, we can use kvmalloc to avoid such limitation.
Signed-off-by: Yuxiao Zhang <[email protected]>
---
fs/pstore/inode.c | 2 +-
fs/pstore/platform.c | 9 +++++----
fs/pstore/ram.c | 5 +++--
fs/pstore/ram_core.c | 3 ++-
4 files changed, 11 insertions(+), 8 deletions(-)
diff --git a/fs/pstore/inode.c b/fs/pstore/inode.c
index ffbadb8b3032..df7fb2ad4599 100644
--- a/fs/pstore/inode.c
+++ b/fs/pstore/inode.c
@@ -54,7 +54,7 @@ static void free_pstore_private(struct
pstore_private *private)
if (!private)
return;
if (private->record) {
- kfree(private->record->buf);
+ kvfree(private->record->buf);
kfree(private->record->priv);
kfree(private->record);
}
diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c
index cbc0b468c1ab..f51e9460ac9d 100644
--- a/fs/pstore/platform.c
+++ b/fs/pstore/platform.c
@@ -32,6 +32,7 @@
#include <linux/uaccess.h>
#include <linux/jiffies.h>
#include <linux/workqueue.h>
+#include <linux/mm.h>
#include "internal.h"
@@ -549,7 +550,7 @@ static int pstore_write_user_compat(struct
pstore_record *record,
if (record->buf)
return -EINVAL;
- record->buf = memdup_user(buf, record->size);
+ record->buf = vmemdup_user(buf, record->size);
if (IS_ERR(record->buf)) {
ret = PTR_ERR(record->buf);
goto out;
@@ -557,7 +558,7 @@ static int pstore_write_user_compat(struct
pstore_record *record,
ret = record->psi->write(record);
- kfree(record->buf);
+ kvfree(record->buf);
out:
record->buf = NULL;
@@ -730,7 +731,7 @@ static void decompress_record(struct pstore_record *record)
return;
/* Swap out compressed contents with decompressed contents. */
- kfree(record->buf);
+ kvfree(record->buf);
record->buf = unzipped;
record->size = unzipped_len;
record->compressed = false;
@@ -783,7 +784,7 @@ void pstore_get_backend_records(struct pstore_info *psi,
rc = pstore_mkfile(root, record);
if (rc) {
/* pstore_mkfile() did not take record, so free it. */
- kfree(record->buf);
+ kvfree(record->buf);
kfree(record->priv);
kfree(record);
if (rc != -EEXIST || !quiet)
diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
index ade66dbe5f39..296465b14fa9 100644
--- a/fs/pstore/ram.c
+++ b/fs/pstore/ram.c
@@ -20,6 +20,7 @@
#include <linux/compiler.h>
#include <linux/of.h>
#include <linux/of_address.h>
+#include <linux/mm.h>
#include "internal.h"
#include "ram_internal.h"
@@ -268,7 +269,7 @@ static ssize_t ramoops_pstore_read(struct
pstore_record *record)
/* ECC correction notice */
record->ecc_notice_size = persistent_ram_ecc_string(prz, NULL, 0);
- record->buf = kmalloc(size + record->ecc_notice_size + 1, GFP_KERNEL);
+ record->buf = kvmalloc(size + record->ecc_notice_size + 1, GFP_KERNEL);
if (record->buf == NULL) {
size = -ENOMEM;
goto out;
@@ -282,7 +283,7 @@ static ssize_t ramoops_pstore_read(struct
pstore_record *record)
out:
if (free_prz) {
- kfree(prz->old_log);
+ kvfree(prz->old_log);
kfree(prz);
}
diff --git a/fs/pstore/ram_core.c b/fs/pstore/ram_core.c
index 966191d3a5ba..3453d493ec27 100644
--- a/fs/pstore/ram_core.c
+++ b/fs/pstore/ram_core.c
@@ -17,6 +17,7 @@
#include <linux/slab.h>
#include <linux/uaccess.h>
#include <linux/vmalloc.h>
+#include <linux/mm.h>
#include <asm/page.h>
#include "ram_internal.h"
@@ -385,7 +386,7 @@ void *persistent_ram_old(struct persistent_ram_zone *prz)
void persistent_ram_free_old(struct persistent_ram_zone *prz)
{
- kfree(prz->old_log);
+ kvfree(prz->old_log);
prz->old_log = NULL;
prz->old_log_size = 0;
}
--
-------------------------------------------
Thanks in advance.
-Yuxiao
On Mon, Jun 26, 2023 at 06:20:29PM -0700, Yuxiao Zhang wrote:
> @@ -730,7 +731,7 @@ static void decompress_record(struct pstore_record *record)
> return;
>
> /* Swap out compressed contents with decompressed contents. */
> - kfree(record->buf);
> + kvfree(record->buf);
> record->buf = unzipped;
> record->size = unzipped_len;
> record->compressed = false;
> @@ -783,7 +784,7 @@ void pstore_get_backend_records(struct pstore_info *psi,
Patch is corrupted and can not be applied at all, please fix up your
email client to not do this.
thanks,
greg k-h
kfree(record->priv);
kfree(record);
if (rc != -EEXIST || !quiet)
diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
index ade66dbe5f39..296465b14fa9 100644
--- a/fs/pstore/ram.c
+++ b/fs/pstore/ram.c
@@ -20,6 +20,7 @@
#include <linux/compiler.h>
#include <linux/of.h>
#include <linux/of_address.h>
+#include <linux/mm.h>
#include "internal.h"
#include "ram_internal.h"
@@ -268,7 +269,7 @@ static ssize_t ramoops_pstore_read(struct
pstore_record *record)
/* ECC correction notice */
record->ecc_notice_size = persistent_ram_ecc_string(prz, NULL, 0);
- record->buf = kmalloc(size + record->ecc_notice_size + 1, GFP_KERNEL);
+ record->buf = kvmalloc(size + record->ecc_notice_size + 1, GFP_KERNEL);
if (record->buf == NULL) {
size = -ENOMEM;
goto out;
@@ -282,7 +283,7 @@ static ssize_t ramoops_pstore_read(struct
pstore_record *record)
out:
if (free_prz) {
- kfree(prz->old_log);
+ kvfree(prz->old_log);
kfree(prz);
}
diff --git a/fs/pstore/ram_core.c b/fs/pstore/ram_core.c
index 966191d3a5ba..3453d493ec27 100644
--- a/fs/pstore/ram_core.c
+++ b/fs/pstore/ram_core.c
@@ -17,6 +17,7 @@
#include <linux/slab.h>
#include <linux/uaccess.h>
#include <linux/vmalloc.h>
+#include <linux/mm.h>
#include <asm/page.h>
#include "ram_internal.h"
@@ -385,7 +386,7 @@ void *persistent_ram_old(struct persistent_ram_zone *prz)
void persistent_ram_free_old(struct persistent_ram_zone *prz)
{
- kfree(prz->old_log);
+ kvfree(prz->old_log);
prz->old_log = NULL;
prz->old_log_size = 0;
}
--
On Mon, Jun 26, 2023 at 11:20 PM Greg KH <[email protected]> wrote:
>
> On Mon, Jun 26, 2023 at 06:20:29PM -0700, Yuxiao Zhang wrote:
> > @@ -730,7 +731,7 @@ static void decompress_record(struct pstore_record *record)
> > return;
> >
> > /* Swap out compressed contents with decompressed contents. */
> > - kfree(record->buf);
> > + kvfree(record->buf);
> > record->buf = unzipped;
> > record->size = unzipped_len;
> > record->compressed = false;
> > @@ -783,7 +784,7 @@ void pstore_get_backend_records(struct pstore_info *psi,
>
> Patch is corrupted and can not be applied at all, please fix up your
> email client to not do this.
>
> thanks,
>
> greg k-h
On Tue, Jun 27, 2023 at 10:05:04AM -0700, Yuxiao Zhang wrote:
> kfree(record->priv);
> kfree(record);
> if (rc != -EEXIST || !quiet)
> diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
> index ade66dbe5f39..296465b14fa9 100644
> --- a/fs/pstore/ram.c
> +++ b/fs/pstore/ram.c
> @@ -20,6 +20,7 @@
> #include <linux/compiler.h>
> #include <linux/of.h>
> #include <linux/of_address.h>
> +#include <linux/mm.h>
>
> #include "internal.h"
> #include "ram_internal.h"
> @@ -268,7 +269,7 @@ static ssize_t ramoops_pstore_read(struct
> pstore_record *record)
> /* ECC correction notice */
> record->ecc_notice_size = persistent_ram_ecc_string(prz, NULL, 0);
>
> - record->buf = kmalloc(size + record->ecc_notice_size + 1, GFP_KERNEL);
> + record->buf = kvmalloc(size + record->ecc_notice_size + 1, GFP_KERNEL);
> if (record->buf == NULL) {
> size = -ENOMEM;
> goto out;
Please try emailing a patch to yourself and see if you can apply it
afterwards. The kernel documentation has a section for how to handle
email clients, perhaps you should read it?
thanks,
greg k-h
On Tue, Jun 27, 2023 at 10:31:23AM -0700, Yuxiao Zhang wrote:
> Sorry about that, seems gmail doesn’t play well with tabs. Resending the patch:
You sent this in html format, still impossible to apply and the mailing
lists reject :(
Thanks for the pointer, the document is very helpful. Seems that I cannot use gmail to send patches. I sent it again by git send-email. I also tested myself that the patch is well-formatted. Hopefully this time it works.
> On Jun 27, 2023, at 10:35 AM, Greg KH <[email protected]> wrote:
>
> On Tue, Jun 27, 2023 at 10:05:04AM -0700, Yuxiao Zhang wrote:
>> kfree(record->priv);
>> kfree(record);
>> if (rc != -EEXIST || !quiet)
>> diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
>> index ade66dbe5f39..296465b14fa9 100644
>> --- a/fs/pstore/ram.c
>> +++ b/fs/pstore/ram.c
>> @@ -20,6 +20,7 @@
>> #include <linux/compiler.h>
>> #include <linux/of.h>
>> #include <linux/of_address.h>
>> +#include <linux/mm.h>
>>
>> #include "internal.h"
>> #include "ram_internal.h"
>> @@ -268,7 +269,7 @@ static ssize_t ramoops_pstore_read(struct
>> pstore_record *record)
>> /* ECC correction notice */
>> record->ecc_notice_size = persistent_ram_ecc_string(prz, NULL, 0);
>>
>> - record->buf = kmalloc(size + record->ecc_notice_size + 1, GFP_KERNEL);
>> + record->buf = kvmalloc(size + record->ecc_notice_size + 1, GFP_KERNEL);
>> if (record->buf == NULL) {
>> size = -ENOMEM;
>> goto out;
>
> Please try emailing a patch to yourself and see if you can apply it
> afterwards. The kernel documentation has a section for how to handle
> email clients, perhaps you should read it?
>
> thanks,
>
> greg k-h