On Thu, Nov 17, 2005 at 02:20:23PM -0800, Andrew Morton wrote:
> Vivek Goyal <[email protected]> wrote:
> >
> > +ssize_t copy_oldmem_page(unsigned long pfn, char *buf,
> > + size_t csize, unsigned long offset, int userbuf)
> > +{
> > + void *vaddr;
> > +
> > + if (!csize)
> > + return 0;
> > +
> > + vaddr = kmap_atomic_pfn(pfn, KM_PTE0);
> > +
> > + if (userbuf) {
> > + if (copy_to_user(buf, (vaddr + offset), csize)) {
> > + kunmap_atomic(vaddr, KM_PTE0);
> > + return -EFAULT;
>
> The copy_*_user() inside kmap_atomic() is problematic.
>
> On some configs (eg, x86, highmem) the process is running atomically, hence
> the copy_*_user() will *refuse* to fault in the user's page if it's not
> present. Because pagefaulting involves doing things which sleep.
>
> So
>
> a) This code will generate might_sleep() warnings at runtime and
>
> b) It'll return -EFAULT for user pages which haven't been faulted in yet.
>
>
> We do all sorts of gruesome tricks in mm/filemap.c to get around all this.
> I don't think your code is as performance-sensitive, so a suitable fix
> might be to double-copy the data. Make sure that the same physical page is
> used as a bounce page for each copy (ie: get the caller to pass it in) and
> that page will be cache-hot and the performance should be acceptable.
>
> If it really is performance-sensitive then you'll need to play filemap.c
> games. It'd be better to use a sleeping kmap instead, if poss. That's
> kmap().
>
> Please send an incremental patch when it's sorted.
Hi Andrew
Sending along the incremental patch as suggested.
In this patch, a global buffer page is introduced, where the page
from the previous kernel's memory is copied, before copying it
out to a user buffer. This will take care of the issue of
copy_to_user() page faulting in an atomic context.
This patch has been generated against 2.6.15-rc2-mm1.
Kindly review.
Thanks
Rachita
o This patch allocates a page to copy the previous kernel's memory
before we copy it onto a user buffer using copy_to_user(), thereby
taking care of the scenario of a possible page fault in an atomic
context.
Signed-off-by: Rachita Kothiyal <[email protected]>
---
arch/i386/kernel/crash_dump.c | 39 +++++++++++++++++++++++++++++++++------
1 files changed, 33 insertions(+), 6 deletions(-)
diff -puN arch/i386/kernel/crash_dump.c~double_copy_read_oldmem arch/i386/kernel/crash_dump.c
--- linux-2.6.15-rc2-mm1/arch/i386/kernel/crash_dump.c~double_copy_read_oldmem 2005-11-23 17:50:07.258543864 +0530
+++ linux-2.6.15-rc2-mm1-rachita/arch/i386/kernel/crash_dump.c 2005-11-23 17:50:36.779056064 +0530
@@ -11,6 +11,8 @@
#include <asm/uaccess.h>
+static void *kdump_buf_page;
+
/**
* copy_oldmem_page - copy one page from "oldmem"
* @pfn: page frame number to be copied
@@ -23,6 +25,10 @@
*
* Copy a page from "oldmem". For this page, there is no pte mapped
* in the current kernel. We stitch up a pte, similar to kmap_atomic.
+ *
+ * Calling copy_to_user() in atomic context is not desirable. Hence first
+ * copying the data to a pre-allocated kernel page and then copying to user
+ * space in non-atomic context.
*/
ssize_t copy_oldmem_page(unsigned long pfn, char *buf,
size_t csize, unsigned long offset, int userbuf)
@@ -34,14 +40,35 @@ ssize_t copy_oldmem_page(unsigned long p
vaddr = kmap_atomic_pfn(pfn, KM_PTE0);
- if (userbuf) {
- if (copy_to_user(buf, (vaddr + offset), csize)) {
- kunmap_atomic(vaddr, KM_PTE0);
+ if (!userbuf) {
+ memcpy(buf, (vaddr + offset), csize);
+ kunmap_atomic(vaddr, KM_PTE0);
+ } else {
+ if (!kdump_buf_page) {
+ printk(KERN_WARNING "Kdump: Kdump buffer page not"
+ " allocated\n");
return -EFAULT;
}
- } else
- memcpy(buf, (vaddr + offset), csize);
+ copy_page(kdump_buf_page, vaddr);
+ kunmap_atomic(vaddr, KM_PTE0);
+ if (copy_to_user(buf, (kdump_buf_page + offset), csize))
+ return -EFAULT;
+ }
- kunmap_atomic(vaddr, KM_PTE0);
return csize;
}
+
+static int __init kdump_buf_page_init(void)
+{
+ int ret = 0;
+
+ kdump_buf_page = kmalloc(PAGE_SIZE, GFP_KERNEL);
+ if (!kdump_buf_page) {
+ printk(KERN_WARNING "Kdump: Failed to allocate kdump buffer"
+ " page\n");
+ ret = -ENOMEM;
+ }
+
+ return ret;
+}
+arch_initcall(kdump_buf_page_init);
_
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
