Re: [RFC/PATCH 2/5] mm/fs: execute in place (V2)

[Christoph Hellwig]

>  static inline void do_generic_file_read(struct file * filp, loff_t *ppos,
>  					read_descriptor_t * desc,
>  					read_actor_t actor)
>  {
> -	do_generic_mapping_read(filp->f_mapping,
> -				&filp->f_ra,
> -				filp,
> -				ppos,
> -				desc,
> -				actor);
> +	if (file_is_xip(filp))
> +		do_xip_mapping_read(filp->f_mapping,
> +					&filp->f_ra,
> +					filp,
> +					ppos,
> +					desc,
> +					actor);
> +	else
> +		do_generic_mapping_read(filp->f_mapping,
> +					&filp->f_ra,
> +					filp,
> +					ppos,
> +					desc,
> +					actor);
>  }

>  	size_t count;
> +	int xip = file_is_xip(filp) ? 1 : 0;
>  
>  	count = 0;
>  	for (seg = 0; seg < nr_segs; seg++) {
> @@ -990,7 +992,9 @@
>  	}
>  
>  	/* coalesce the iovecs and go direct-to-BIO for O_DIRECT */
> -	if (filp->f_flags & O_DIRECT) {
> +	/* do not use generic_file_direct_IO on xip files, xip IO is
> +	   implicitly direct as well */
> +	if (filp->f_flags & O_DIRECT && !xip) {
>  		loff_t pos = *ppos, size;
>  		struct address_space *mapping;
>  		struct inode *inode;

I don't like this read split at all.  Please just define a completely
separate entry point for read, xip_file_read except for verifying the
iovecs you don't share any code.

> @@ -1538,10 +1545,13 @@
>  {
>  	struct address_space *mapping = file->f_mapping;
>  
> -	if (!mapping->a_ops->readpage)
> +	if ((!mapping->a_ops->readpage) && (!mapping_is_xip(mapping)))
>  		return -ENOEXEC;
>  	file_accessed(file);
> -	vma->vm_ops = &generic_file_vm_ops;
> +	if (mapping_is_xip(mapping))
> +		vma->vm_ops = &xip_file_vm_ops;
> +	else
> +		vma->vm_ops = &generic_file_vm_ops;
>  	return 0;
>  }

Similar please add a separate xip_file_mmap.

> @@ -2123,6 +2062,13 @@
>  
>  	inode_update_time(inode, 1);
>  
> +	if (file_is_xip(file)) {
> +		/* use execute in place to copy directly to disk */
> +		written = generic_file_xip_write (iocb, iov,
> +			        nr_segs, pos, ppos, count);
> +		goto out;
> +	}
> +

Dito with xip_file_write.

> diff -ruN linux-git/mm/filemap.h linux-git-xip/mm/filemap.h
> --- linux-git/mm/filemap.h	1970-01-01 01:00:00.000000000 +0100
> +++ linux-git-xip/mm/filemap.h	2005-05-17 18:33:57.792451512 +0200
> @@ -0,0 +1,141 @@
> +/*
> + *	linux/mm/filemap.h
> + *
> + * Copyright (C) 2005 IBM Corporation

I think just adding an IBM copyright isn't fair.  Just copy it from
filemap.c

> +		/*
> +		 * We need the page_table_lock to protect us from page faults,
> +		 * munmap, fork, etc...
> +		 */
> +		spin_lock(&mm->page_table_lock);
> +		pgd = pgd_offset(mm, address);
> +		if (!pgd_present(*pgd))
> +			goto next_unlock;
> +		pud = pud_offset(pgd, address);
> +		if (!pud_present(*pud))
> +			goto next_unlock;
> +		pmd = pmd_offset(pud, address);
> +		if (!pmd_present(*pmd))
> +			goto next_unlock;
> +		
> +		pte = pte_offset_map(pmd, address);
> +		if (!pte_present(*pte))
> +			goto next_unmap;
> +		if ((page_to_pfn(virt_to_page(empty_zero_page)))
> +			!= pte_pfn(*pte))
> +			/* pte does already reference new xip block here */

You should probably use page_check_address().  Currently it's static in rmap.c,
but that could be changed.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/