Re: [PATCH] some optimizations for Virtual Machines

[renzo@xxxxxxxxxxx (Renzo Davoli)]

> > > int utrace_access_process_vm(struct task_struct *tsk, unsigned long addr, char __user *ubuf, int len, int write, int string);
> 
> "string" smells like a hack, someone will come up with his favourite
> structure and ask for flag for copying, say, single-linked lists. :(

There are no system calls asking for linked lists as parameters.
Instead there are many having string! All those having a pathname.

Look at this:
char *s=strdup(x);
fd=open (s,O_RDONLY);

THis is a quiet, safe chunk of user code.
What do you do to grab the value of s (from a virtual machine monitor)?
With ptrace you can do a loop of PEEK_DATA one for each word of memory,
when there is a NULL byte you leave the loop.
IF you are designing a Virtual Machine this is just a performance
suicide.
You could open /proc/<pid>/mem, in this case either you keep 
one descriptor opened for each controlled process or you  open
/read/close the proc file per each access.
Either a scalability or a performance nightmare.
if you need to go fast something like:
access_process_vm(....,address_of_s, PATH_MAX ...)
can fail because the s could be in the unluky position at the end of
an allocated partition.

> > - it copies a memory area directly from a process vm to the user space
> >   of current and viceversa (it uses an internal one page buffer,
> 
> check for allocation failure
You mean:
actual>  mm = get_task_mm(tsk);
actual>  if (!mm)
actual>    return 0;
actual>
actual>  buf=kmalloc(PAGE_SIZE, GFP_KERNEL);
must be changed as:
updated>  mm = get_task_mm(tsk);
updated>  buf=kmalloc(PAGE_SIZE, GFP_KERNEL);
updated>  if (!mm || !buf)
updated>    return 0;

Okay, you're right, I'll update the patch.

> 
> > > 3- In the patch I have also implemented the support for PTRACE_MULTI 
> > > and PTRACE_SYSVM.
> 
> PTRACE_MULTI is horrible, it is asking for pain with compat version.

I cannot understand. If you do not use PTRACE_MULTI, ptrace works as
usual.
Old ptrace do not use PTRACE_MULTI so you do not need to support
PTRACE_MULTI for backward compatibility with old versions.

> 
> Mode switches are fast. This is a reason why read(2) doesn't have
> batched version, and read is called waaay more often than ptrace.

readv do exist.
Mode switches are fast, but having less mode switches is even faster.
> 
> I also wonder if this was tested with list debugging on: iterating over
> RCU protected list backwards when prev pointers are poisoned shouldn't
> work.
I do not know if the reverse scan of the list can be done better
or maybe my implementation is buggy.

I say that we do need that reverse traversal for SYSCALL_ENTRY otherwise
it is not possible to implement nested services based on utrace.

Regarding the 3 points of my original message:
1- order of calls: the patch (or a different patch implementing the same
idea) is needed, otherwise the support for nested engines 
become meaningless when dealing with system calls virtualization.
2- access to process vm: some solution is needed for a fast access to a
utraced process vm.
3- I need this patch for my project, I feel that it could speed up some
other programs, but this is not so crucial as 1 and 2.

renzo