Re: [PATCH] evl: replace `remap_pfn_range` with `vm_insert_page` in control_mmap.

[Philippe Gerum <rpm@xenomai.org>]

Qichen Qiu <ruiqurm@gmail.com> writes:

> Hi, I used vm_insert_page to map the area to user space, and it 
> successfully resolved the issue.
>

Thanks for the patch. A few observations:

- for this to work, you definitely need to update the system heap
  allocation method accordingly in the same patch, i.e. using vzalloc
  instead of kzalloc. This patch is incomplete.

- Why eagerly clearing the VM_WRITE flag from the vma?

- the short log should refer to what is being fixed as perceived by
  $user, not how we do that. The latter should be part of the long log
  instead. In this case, that would be something like
  evl/memory: allow system heap inspection using GDB
  The long log could then give a few details about GDB, ptracing, the
  VM_IO issue, and the reasoning behind the change of interface from
  remap_pfn_range to vm_insert_page.

> Signed-off-by: Qichen Qiu <ruiqurm@gmail.com>
> ---
>  kernel/evl/control.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/evl/control.c b/kernel/evl/control.c
> index 2d1b44f3b0dc..eee4bcaad390 100644
> --- a/kernel/evl/control.c
> +++ b/kernel/evl/control.c
> @@ -338,13 +338,22 @@ static long control_ioctl(struct file *filp, unsigned int cmd,
>  static int control_mmap(struct file *filp, struct vm_area_struct *vma)
>  {
>  	void *p = evl_get_heap_base(&evl_shared_heap);
> -	unsigned long pfn = __pa(p) >> PAGE_SHIFT;
>  	size_t len = vma->vm_end - vma->vm_start;
> +	unsigned long addr = vma->vm_start;
> +	int err;
>  
>  	if (len != evl_shm_size)
>  		return -EINVAL;
> +
> +	vm_flags_clear(vma, VM_WRITE);
> +
> +	while (addr < vma->vm_end) {
> +		err = vm_insert_page(vma, addr, virt_to_page(p));
> +		if (err < 0)
> +			return err;
> +		addr += PAGE_SIZE;
> +		p += PAGE_SIZE;
> +	}
>  
> -	return remap_pfn_range(vma, vma->vm_start, pfn, len, PAGE_SHARED);
> +	return 0;
>  }
>  
>  static const struct file_operations control_fops = {

-- 
Philippe.