Open njbourdo opened 4 years ago
Hi Nick!
Sorry, I don't really know much about this stuff, I just stopped when an example seemed to work!
So the things you mentioned, they appear to be bugs on what I had, is that correct? Did you manage to get it working in the end? If you can share you patch, that would be amazing.
Hi Ciro,
no problem! I found that I had to call put_page for the first page only in order for everything to be tracked properly. I read in several places online that freeing the pages should do that automatically, but I consistently got error saying that my group of pages still had a count of 1 even though nothing was using them anymore. After printing out the count value for each page, I found that only the first still had a count value of 1 so I added a call to put_page() in the release function and that fixed everything. After allocating and freeing 64 pages over 230000 times, I still got no errors so I think it's good.
My code has changed quite a bit from your original version so I just went back and made the appropriate changes to your code. I'd suggest testing it before committing! Thanks again! Let me know if you have any other questions.
Nick
P.S. github doesn't like .diff files so I changed it to a .txt mmap.txt
Awesome, thanks for sharing it.
Sorry I couldn't help you :-) but it is good to have that info in public which might help others.
If you find any further info, please to share in further comments.
Gonna reproduce the patch here:
--- mmap-orig.c 2020-08-06 10:27:00.844340199 -0700
+++ mmap-mod.c 2020-08-06 10:33:36.733107851 -0700
@@ -11,7 +11,12 @@
static const char *filename = "lkmc_mmap";
-enum { BUFFER_SIZE = 4 };
+#ifndef PAGE_SIZE //should be defined in kernel
+#define PAGE_SIZE 4096
+#endif
+#define BUFFER_PAGES 64
+#define PAGES_ORDER 6 //2^6 = 64 pages
+#define BUFFER_SIZE (PAGE_SIZE * BUFFER_PAGES)
struct mmap_info {
char *data;
@@ -32,7 +37,7 @@
pr_info("vm_fault\n");
info = (struct mmap_info *)vmf->vma->vm_private_data;
if (info->data) {
- page = virt_to_page(info->data);
+ page = virt_to_page(info->data + (PAGE_SIZE * vmf->pgoff));
get_page(page);
vmf->page = page;
}
@@ -69,8 +74,14 @@
pr_info("open\n");
info = kmalloc(sizeof(struct mmap_info), GFP_KERNEL);
pr_info("virt_to_phys = 0x%llx\n", (unsigned long long)virt_to_phys((void *)info));
- info->data = (char *)get_zeroed_page(GFP_KERNEL);
- memcpy(info->data, "asdf", BUFFER_SIZE);
+ info->data = (char *)__get_free_pages(GFP_KERNEL, PAGES_ORDER);
+ if(info->data == NULL)
+ {
+ pr_err("Unable to allocate framebuffer!\n");
+ return -ENOMEM;
+ }
+ memset(info->data, 0, PAGE_SIZE << PAGES_ORDER); //zero the buffer
+ memcpy(info->data, "asdf", 4);
filp->private_data = info;
return 0;
}
@@ -105,10 +116,13 @@
static int release(struct inode *inode, struct file *filp)
{
struct mmap_info *info;
+ struct page *page;
pr_info("release\n");
info = filp->private_data;
- free_page((unsigned long)info->data);
+ page = virt_to_page(info->data);
+ put_page(page);
+ free_pages((unsigned long)info->data, PAGES_ORDER);
kfree(info);
filp->private_data = NULL;
return 0;
Oh don't worry, you definitely did help! I'll be sure to follow up if I learn anything new.
Hi Ciro,
First, thank you so much for your mmap example, it saved me a TON of time. I've modified it to use multiple pages (64 pages for my purposes) and it works, but periodically crashes when allocating pages, since some of the pages it's trying to allocate have a count value of 1. I assume this is because the pages aren't properly being freed on previous uses.
I've changed open() to use __get_free_pages() instead of get_zeroed_page(), vm_fault() to find the address of the correct page using vmf->pgoff, and release() to use free_pages() instead of free_page(). Do you know if there is anything else I need to do in order to properly manage the pages? Thank you,
Nick