Re: [PATCH 9/10] kdump: read previous kernel's memory

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.  
-
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]
  Powered by Linux