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

Christoph Hellwig (hch@infradead.org)
Fri, 4 Apr 2003 14:40:03 +0100


> +#ifndef _LINUX_MBIND_H
> +#define _LINUX_MBIND_H
> +
> +#ifdef CONFIG_NUMA
> +
> +#include <linux/mmzone.h>
> +
> +/* Structure to keep track of memory segment (VMA) bindings */
> +struct binding {
> + struct zonelist zonelist;
> +};
> +
> +#endif /* CONFIG_NUMA */
> +#endif /* _LINUX_MBIND_H */

Using CONFIG_ without explicitly including config.h is a bad idea.
But the ifdef is unessecary anyway, no one is hurt by having this
struct defintion in the non-numa case. Also I wonder how you can have
a copyright on a single trivial struct defintion :) What about just
moving it to mmzone.h, btw?

> +#include <linux/errno.h>
> +#include <linux/mm.h>
> +#include <linux/mbind.h>
> +#include <asm/string.h>

Use <linux/string.h> instead.

> + binding = (struct binding *)kmalloc(sizeof(struct binding), GFP_KERNEL);

The cast is superflous.

> + if (!(vma && vma->vm_file && vma->vm_ops &&
> + vma->vm_ops->nopage == shmem_nopage)) {
> + /* This isn't a shm segment. For now, we bail. */
> + error = -EINVAL;
> + goto out;
> + }

This check is just horrible. Please describe what kind of mappings
this doesn't work for and why and add a VM_ flag for those that
support memory binding.

> .private_list = LIST_HEAD_INIT(swapper_space.private_list),
> +#ifdef CONFIG_NUMA
> + .binding = NULL,
> +#endif

You don't need to initialize this at all.

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