Re: [PATCH][3/6]Interface for copying the dump pages

[Dave Hansen <haveblue@us.ibm.com>]

On Tue, 2004-08-17 at 05:08, Hariprasad Nellitheertha wrote:
> Regards, Hari
> +/* This is the same as kmap_atomic but takes in a pfn instead of a
> + * struct page.
> + */

How about:

/* 
 * This is the same as kmap_atomic() but can map memory that doesn't 
 * necessarily have struct page associated with it.
 */

> +ssize_t copy_hmem_page(unsigned long pfn, char *buf, size_t csize,
> int userbuf)
...
> +       if (pfn == -1) {
> +               /* Give them a zeroed page */
> +               if (userbuf) {
> +                       if (clear_user(buf, csize))
> +                               return -EFAULT;
> +               } else
> +                       memset(buf, 0, csize);
> +       } else {

This is really hackish.  If you want to zero a userspace page, just zero
it in the caller's code, or call a different function.  Zeroing a
userspace page has nothing to do with copying a highmem page, and it
really shouldn't be here.  Just changing it from taking a 0 to a -1 from
the last time you posted it doesn't really make it any better.

Also, that function might belong in the same area as copy_highpage(),
not in some driver.  In any case, it certainly needs a big fat comment
explaining why it is special, and different from copy_highpage().

If it shouldn't be moved and really is isolated to the mem.c driver, it
needs to be declared static.

-- Dave