Messages in this thread | | | Subject | Re: [PATCH 9/10] kdump: read previous kernel's memory | From | (Eric W. Biederman) | Date | Thu, 17 Nov 2005 16:14:43 -0700 |
| |
Andrew Morton <akpm@osdl.org> writes:
> Vivek Goyal <vgoyal@in.ibm.com> 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.
I'm going send my standard grumble that what is really needed is to track why we can't use /dev/mem.
We could solve this with a normal kmap except that we quite possibly don't have a struct page for this page of memory.
/dev/mem does not allow reads in this situation because of how problematic getting I/O reads correct is. But not having a good interface for mapping it into user space is similar.
Could we simply make this into a file that you can mmap but you can't read? I think that would be cleaner, and simpler.
Eric
- 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/
| |