Re: Change soft-dirty interface?

[Minchan Kim <minchan@kernel.org>]

Hi Pavel,

Sorry for the delaying.
Maybe our timezone difference and my boys's interrupt.

On Fri, Jun 14, 2013 at 03:37:09PM +0400, Pavel Emelyanov wrote:
> On 06/14/2013 03:22 PM, Minchan Kim wrote:
> > Hello Pavel,
> > 
> > On Fri, Jun 14, 2013 at 02:01:23PM +0400, Pavel Emelyanov wrote:
> >>>>>>> If it's not allowed, another approach should be new system call.
> >>>>>>>
> >>>>>>>         int sys_softdirty(pid_t pid, void *addr, size_t len);
> >>>>>>
> >>>>>> This looks like existing sys_madvise() one.
> >>>>>
> >>>>> Except pid part. It is added by your purpose, which external task
> >>>>> can control any process.
> >>
> >> In CRIU we can work with pid-less syscalls just fine :) So extending regular
> >> madvise would work.
> > 
> > I didn't know that.
> > Just out of curiosity. How can CRIU control other tasks without pid?
> 
> We use the parasite-injection technique [1]. Briefly -- we put a code into
> other task's address space using ptrace() and /proc/PID/map_files/ and make
> this code run and do what we need. Thus we can call madvise() "on" another
> task.

Interesting.

> 
> [1] http://lwn.net/Articles/454304/


> 
> >>
> >>>>>>
> >>>>>>> If we approach new system call, we don't need to maintain current
> >>>>>>> proc interface and it would be very handy to get a information
> >>>>>>> without pagemap (open/read/close) so we can add a parameter to
> >>>>>>> get a dirty information easily.
> >>>>>>>
> >>>>>>>         int sys_softdirty(pid_t pid, void *addr, size_t len, unsigned char *vec)
> >>>>>>>
> >>>>>>> What do you think about it?
> >>>>>>>
> >>>>>>
> >>>>>> This is OK for me, though there's another issue with this API I'd like
> >>>>>> to mention -- consider your app is doing these tricks with soft-dirty
> >>>>>> and at the same time CRIU tools live-migrate it using the soft-dirty bits
> >>>>>> to optimize the freeze time.
> >>>>>>
> >>>>>> In that case soft-dirty bits would be in wrong state for both -- you app
> >>>>>> and CRIU, but with the proc API we could compare the ctime-s of the 
> >>>>>> clear_refs file and find out, that someone spoiled the soft-dirty state
> >>>>>> from last time we messed with it and handle it somehow (copy all the memory
> >>>>>> in the worst case). Can we somehow handle this with your proposal?
> >>>>>
> >>>>> Good point I didn't think over that.
> >>>>> A simple idea popped from my mind is we can use read/write lock
> >>>>> so if pid is equal to calling process's one or pid is NULL,
> >>>>> we use read side lock, which can allow marking soft-dirty 
> >>>>> several vmas with parallel. And pid is not equal to calling
> >>>>> process's one, the API should try to hold write-side lock
> >>>>> then, if it's fail, the API should return EAGAIN so that CRIU
> >>>>> can progress other processes and retry it after a while.
> >>>>> Of course, it would make live-lock so that sys_softdirty might
> >>>>> need another argument like "int block".
> >>>>
> >>>> And we need a flag to show SELF_SOFT_DIRTY or EXTERNAL_SOFT_DIRTY
> >>>> and the flag will be protected by above lock. It could prevent mixed
> >>>> case by self and external.
> >>>
> >>> I realized it's not enough. Another idea is here.
> >>> The intenion is followin as,
> >>>
> >>> self softdirty VS self softdirty -> NOT exclusive
> >>> self softdirty VS external softdirty -> exclusive
> >>> external softdirty VS external softdirty-> excluisve
> >>
> >> I think it might work for us. However, I have two comments to the
> >> implementation, please see below.
> >>
> >>> struct softdirty token {
> >>>         u64 external;
> >>>         u64 internal;
> >>> };
> >>>
> >>>        int sys_set_softdirty(pid_t pid, unsigned long start, size_t len,
> >>>                                 struct softdirty *token); 
> > 
> > I should have mentioned that start and len are ignored if pid is not eqaul
> > to caller's pid.
> 
> OK
> 
> >>>        int sys_get_softdirty(pid_t pid, unsigned long start, size_t len, 
> >>>                                 struct softdirty token, char *vec);
> >>
> >> Can you please show an example how to use these two, I don't quite get how
> >> can I do external soft-dirty tracking in atomic manner.
> > 
> > Hmm, I don't know how CRIU works but ...
> > 
> > 	while(1) {
> > 
> > 		struct softdirty token;
> > 		
> > 		sys_set_softdirty(tracked_pid, 0, 0, &token);
> > 		...
> > 		...
> > 		...
> > 		if (!sys_get_softdirty(tacked_pid, 0, 0, token, NULL))
> > 			break;
> > 	}
> > 
> > Maybe do you have a concern about live-lock?
> 
> No, I worry about potential races with which we or application can skip
> dirty page. Let me describe how CRIU uses existing soft-dirty implementation.
> 
> 1. stop the task we want to work on
> 2. read the /proc/pid/pagemap file to find out which pages to
>    read. Those with soft-dirty _cleared_ should be _skipped_
> 3. read task's memory at calculated bitmap
> 4. reset soft dirty bits on task
> 5. resume task execution

Let me try to parse as my term.

1. admin does "echo 4 > /proc/<target>/clear_refs"
2. admin stop the target
3. admin reads the /proc/<target>/pagemap and make bitmap
   with only soft-dirty marked pages so we can avoid unnecessary
   migration
4. admin reads target's dirtied pages via bitmap from 3
5. admin does "echo 4 > /proc/<target>/clear_refs" again to find
   future diry pages of the target.
6. admin resumes the target

Right?
If so, my interface is following as

1. admin does set_softdirty(target, 0, 0, &token);
   (set_softdirty clears all soft-dirty bit from target process's
   page table.
2. admin stop the target
3. admin reads the /proc/target/pagemap and make bitmap
   with only soft-dirty marked pages so we can avoid unnecessary
   migration. 
4. admins does get_softdirty(target, 0, 0, token) to confirm
   someone else spoiled since 1
4-1. If it is reports error, then admins discard the bitmap got
     from 3 and have to read all memory.
5. admin does set_softdirty(target, 0, 0, &token) again to find
   future dirty pages of the target  
5. admin resumes the target.

> 
> With the interface you propose the sequence presumably should look like
> 
> 1. stop the task we want to work on
> 2. call set_softdirty + get_softdirty to get the soft-dirty bitmap and
>    reset one. If it reports error, then the soft-dirty we did before is
>    spoiled and all memory should be read (iow -- bitmap should be filled
>    with 1-s)
> 3. read task's memory at calculated bitmap
> 4. resume task execution

> 
> Am I right with this? If yes, why do we need two calls, wouldn't it be better

I failed to parse your terms so I wrote scnario as my understanding
so please see my above sequence and if you have a comment, please ask
again.

> to merge them into one?

It's not hard part but I wanted to show my intention clearly.
If we all agree on, let's think over interface again.

Thanks!

> 
> Thanks,
> Pavel

-- 
Kind regards,
Minchan Kim

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>