Re: [patch] mspec - special memory driver and do_no_pfn handler

[Andrew Morton <akpm@osdl.org>]

Jes Sorensen <jes@sgi.com> wrote:
>
> Hi,
> 
> This is an updated version of the mspec driver (special memory
> support), formerly known as fetchop.
> 
> With this version I have implemented a do_no_pfn() handler, similar to
> the do_no_page() handler but for pages which are not backed by a
> struct page.

hm.  Is that a superset of ->nopage?  Should we be looking at
migrating over to ->nopfn, retire ->nopage?

<looks at the ghastly stuff in do_no_page>

Maybe not...

> This avoids the trick used in earlier versions where the
> driver was allocating a dumy page returning it back to the
> do_no_page() handler which would the free it immediately. Hopefully
> this addresses the main concern there were with this driver in the
> past.
> 
> The reason for taking the do_no_pfn() approch rather than
> remap_pfn_range() is that it needs to benefit from node locality of
> the pages on NUMA systems.
> 
> While the driver is currently only used on SN2 hardware, it is placed
> in drivers/char as it should be possible and beneficial for other
> architectures to implement and use the uncached mode.
> 
> Please let me know if there are any objections or comments etc. to
> this approach. If preferred I can split out the do_no_pfn part into a
> seperate patch.

That would probably be best.

> +#include <linux/config.h>
> +#include <linux/types.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/errno.h>
> +#include <linux/miscdevice.h>
> +#include <linux/spinlock.h>
> +#include <linux/mm.h>
> +#include <linux/proc_fs.h>
> +#include <linux/vmalloc.h>
> +#include <linux/bitops.h>
> +#include <linux/string.h>
> +#include <linux/slab.h>
> +#include <linux/seq_file.h>
> +#include <linux/efi.h>
> +#include <linux/numa.h>
> +#include <asm/page.h>
> +#include <asm/pal.h>
> +#include <asm/system.h>
> +#include <asm/pgtable.h>
> +#include <asm/atomic.h>
> +#include <asm/tlbflush.h>
> +#include <asm/uncached.h>
> +#include <asm/sn/addrs.h>
> +#include <asm/sn/arch.h>
> +#include <asm/sn/mspec.h>
> +#include <asm/sn/sn_cpuid.h>
> +#include <asm/sn/io.h>
> +#include <asm/sn/bte.h>
> +#include <asm/sn/shubio.h>

Wow.

> +static inline int
> +mspec_zero_block(unsigned long addr, int len)
> +{
> +	int status;
> +
> +	if (ia64_platform_is("sn2")) {

ia64 uses strcmp() in hotpaths to work out what sort of platform it's
running on?  Surely someone has cached this info in a __read_mostly integer
somewhere?

> +static __inline__ pmd_t *

`inline', please.

> +mspec_get_pmd(struct mm_struct *mm, u64 address)

This function has no callers.

> +static int __init
> +mspec_init(void)
> +{
> +	int ret;
> +	int nid;
> +
> +	/*
> +	 * The fetchop device only works on SN2 hardware, uncached and cached
> +	 * memory drivers should both be valid on all ia64 hardware
> +	 */
> +	if (ia64_platform_is("sn2")) {
> +		if (is_shub2()) {
> +			ret = -ENOMEM;
> +			for_each_online_node(nid) {
> +				int actual_nid;
> +
> +				scratch_page[nid] = uncached_alloc_page(nid);
> +				if (scratch_page[nid] = 0)
> +					goto free_scratch_pages;
> +				actual_nid = nasid_to_cnodeid(get_node_number(__pa(scratch_page[nid])));
> +				if (actual_nid != nid)
> +					goto free_scratch_pages;
> +			}
> +		}
> +
> +		ret = misc_register(&fetchop_miscdev);
> +		if (ret) {
> +			printk(KERN_ERR
> +			       "%s: failed to register device %i\n",
> +			       FETCHOP_ID, ret);
> +			goto free_scratch_pages;
> +		}
> +	}
> +	ret = misc_register(&cached_miscdev);
> +	if (ret) {
> +		printk(KERN_ERR "%s: failed to register device %i\n",
> +		       CACHED_ID, ret);
> +		misc_deregister(&fetchop_miscdev);

You don't know that fetchop_miscdev was registered.