Re: [patch][rfc] Memory Binding (1/1)

Christoph Hellwig (hch@infradead.org)
Wed, 2 Apr 2003 08:26:52 +0100


On Tue, Apr 01, 2003 at 02:39:22PM -0800, Matthew Dobson wrote:
> --- linux-2.5.66-pre_membind/arch/i386/kernel/entry.S Mon Mar 24 14:00:11 2003
> +++ linux-2.5.66-membind/arch/i386/kernel/entry.S Mon Mar 31 17:45:20 2003
> @@ -852,6 +852,7 @@
> .long sys_clock_gettime /* 265 */
> .long sys_clock_getres
> .long sys_clock_nanosleep
> + .long sys_membind

Syscall slot 223 is unused, please use it instead of adding at the end.

> +#ifndef CONFIG_NUMA
> +
> +asmlinkage unsigned long sys_membind(unsigned long start, unsigned long len,
> + unsigned long *mask_ptr, unsigned int mask_len, unsigned long policy)
> +{
> + return -ENOSYS;
> +}
> +
> +#else /* CONFIG_NUMA */

Yuck! Compile this file conditional on CONFIG_NUMA and add a cond_syscall
entry for sys_membind to kernel/sys.c

> +#include <linux/binding.h>
> +#include <asm/string.h>
> +#include <asm/topology.h>
> +#include <asm/uaccess.h>
> +
> +/* Translate a cpumask to a nodemask */
> +static inline void cpumask_to_nodemask(DECLARE_BITMAP(cpumask, NR_CPUS),
> + DECLARE_BITMAP(nodemask, MAX_NUMNODES))

Umm, this arguments looks strange. DECLARE_BITMAP always expands to
an unsigned long array, so just use unsigned long * arguments.

> +static inline int add_zones(pg_data_t *pgdat, struct zonelist *zonelist,
> + int zone_num, int zone_type)
> +{
> + switch (zone_type) {
> + struct zone *zone;
> + default:
> + BUG();
> + case ZONE_HIGHMEM:
> + zone = pgdat->node_zones + ZONE_HIGHMEM;
> + if (zone->present_pages)
> + zonelist->zones[zone_num++] = zone;
> + case ZONE_NORMAL:
> + zone = pgdat->node_zones + ZONE_NORMAL;
> + if (zone->present_pages)
> + zonelist->zones[zone_num++] = zone;
> + case ZONE_DMA:
> + zone = pgdat->node_zones + ZONE_DMA;
> + if (zone->present_pages)
> + zonelist->zones[zone_num++] = zone;
> + }

Hardconding of zones looks like a bad idea. Do you really need to
expose zones in the API? User memory should always be in ZONE_HIGHMEM
(or ZONE_NORMAL for 64bit arches).

> + vma = find_vma(current->mm, start);
> + if (!(vma && vma->vm_file && vma->vm_ops &&
> + vma->vm_ops->nopage == shmem_nopage)) {
> + /* This isn't a shm segment. For now, we bail. */
> + printk("%s: Can only bind shm(em) segments for now!\n", __FUNCTION__);
> + error = -EINVAL;
> + goto out;
> + }

This check sounds like a bad idea. Please make this API generic.

-
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/