Messages in this thread |  | | From | Paul Mackerras <> | Date | Mon, 21 May 2001 09:48:17 +1000 (EST) | Subject | Re: add page argument to copy/clear_user_page |
| |
Linus Torvalds writes:
> If you add the page argument, why leave the old arguments lingering there > at all? They only create confusion, and add no information.
You mean the `to' pointer argument, or the `vaddr' argument? The `vaddr' argument isn't redundant, it's the user virtual address where the page is mapped, and sparc64 needs it in order to avoid D-cache aliasing issues I believe. (Dave?)
As for the `to' argument, yes it is redundant since it is just kmap(page). But copy/clear_user_page isn't the interface that gets called from the MM stuff, copy/clear_user_highpage is, defined in include/linux/highmem.h. These are two of a whole series of functions which all do kmap, do something, kunmap.
IMHO having the kmap/kunmap calls in copy/clear_user_highpage in include/linux/highmem.h is the best approach because it means that most architectures can just #define copy/clear_user_page as copy/clear_page in include/asm/page.h (as they do at the moment). It means that the kmap/kunmap calls are in one place only instead of being duplicated in every architecture.
But we could instead push the kmap/kunmap down into copy/clear_user_page. Then we might as well rename them into copy/clear_user_highpage. Here is how it might turn out in include/asm-i386/page.h (and asm-alpha, asm-arm, asm-crus, asm-s390, asm-sh, asm-s390x...):
extern void clear_page(void *page); extern void copy_page(void * _to, void * _from);
#define clear_user_highpage(page, vaddr) \ do { \ struct page *__page = page; \ clear_page(kmap(__page)); \ kunmap(__page); \ } while (0)
#define copy_user_highpage(to, from, vaddr) \ do { \ struct page *__to = to, *__from = from; \ copy_page(kmap(__to), kmap(__from)); \ kunmap(__from); \ kunmap(__to); \ } while (0)
Doing it with inline functions would be cleaner but would mean that we would need the declaration of kmap/kunmap in page.h. That would mean that we would need to #include <linux/highmem.h> in include/asm/page.h which is starting to get pretty messy and inviting circular inclusions. We could move these declarations to another file in include/asm - include/asm/highmem.h might seem the natural place but it is only used if CONFIG_HIGHMEM is defined and not all ports have it.
I assume nobody wants to do these functions out-of-line. :)
So on the whole the way I had it seems cleanest to me. But I can whip up a patch to do the kmap/kunmap in the architecture-specific files instead, if you prefer - if so, do you prefer the macro version or the inline function version?
Paul. - 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/
|  |