Messages in this thread | | | Date | Fri, 4 Apr 2003 14:40:03 +0100 | From | Christoph Hellwig <> | Subject | Re: [rfc][patch] Memory Binding Take 2 (1/1) |
| |
> +#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/
| |