[CRIU] [PATCH] cr-service: require non-cooperative userfaultfd for lazy-pages

[Mike Rapoport]

On Thu, Aug 10, 2017 at 06:35:36PM +0300, Pavel Emelyanov wrote:
> On 08/10/2017 05:02 PM, Mike Rapoport wrote:
> > On Thu, Aug 10, 2017 at 04:23:57PM +0300, Pavel Emelyanov wrote:
> >> On 08/10/2017 03:05 PM, Mike Rapoport wrote:
> >>> On Thu, Aug 10, 2017 at 02:40:23PM +0300, Pavel Emelyanov wrote:
> >>>> On 08/10/2017 12:59 PM, Mike Rapoport wrote:
> >>>>> Pavel,
> >>>>>
> >>>>> Any comments?
> >>>>
> >>>> Oops, sorry :) Too many e-mails after vacation. Yes, I agree with that. The
> >>>> only thing that I'd like to see as incremental fix is saving the uffd and
> >>>> uffd-non-coop on kdat cache and using these values everywhere.
> >>>
> >>> The kdat fields haven't changed. The only change is how cr-check and
> >>> cr-service treat them.
> >>
> >> I mean -- handle_feature_check calls check_uffd_noncoop which does testing
> >> of the feature, while instead it should just read the kdat.something field :)
> > 
> > Maybe that's because cr-check was not updated when kdat cache got merged? ;-)
> 
> Maybe, but the handle_feature_check is (should be) called with kdat
> cache loaded.

If I'm not much mistaken, cr-service does not init kdat at all:

~/git/criu$ git grep kerndat_init
criu/cr-dump.c: if (kerndat_init())
criu/cr-dump.c: if (kerndat_init())
criu/cr-restore.c:      if (kerndat_init())
criu/include/kerndat.h:extern int kerndat_init(void);
criu/kerndat.c:int kerndat_init(void)

 
> >>>> Said that
> >>>>
> >>>> Acked-by: Pavel Emelyanov <xemul at virtuozzo.com>
> >>>>
> >>>> -- Pavel
> >>>>
> >>>>> On Wed, Aug 09, 2017 at 09:21:31AM +0200, Adrian Reber wrote:
> >>>>>> On Wed, Aug 09, 2017 at 08:41:52AM +0300, Mike Rapoport wrote:
> >>>>>>> I've noticed that we don't really have RPC support for lazy-pages. I've
> >>>>>>> rechecked Adrian's pull request to runc and there is a requirement to run
> >>>>>>> lazy-pages daemon manually.
> >>>>>>> Do we want to add an RPC interface for lazy-pages?
> >>>>>>> Another possibility is to fork() lazy-pages from restore...
> >>>>>>
> >>>>>> I think fork()-ing makes sense. Except for debug situations there is no
> >>>>>> real reason to start the lazy-pages daemon manually.
> >>>>>>
> >>>>>> It would still be nice to have a way to start it manually, though. But
> >>>>>> the default should be that it fork()s.
> >>>
> >>> And what about fork()ing lazy-pages when restore is run with --lazy-pages?
> >>
> >> Sorry, missed that :) I think fork-ing is OK, but we should somehow decide
> >> on when restore fork()-s the lazy daemon and when connects to whatever is
> >> there in the work dir.
> >>
> >> Say, if there's a socket -- connect. Otherwise -- fork. If this is the way to
> >> go what to do if there's a stale socket left from previous lazy daemon run?
> >> Bail out or still fork()?
> > 
> > What about:
> > 
> > * If there's a socket -- connect
> > * If Connect failed:
> > 	- re-create the socket
> > 	- fork and let lazy-pages inherit the socket
> 
> O_o  If connect failed ... for any reason? I don't know, this sounds dangerous.
> I'd make it simple -- socket -> connect and fail if fail, no socket -> fork.
> 
> -- Pavel
> 

-- 
Sincerely yours,
Mike.