On Sun, Nov 29, 2020 at 11:12:10PM +0300, Andrey Gruzdev wrote:
> > > +void ram_write_tracking_stop(void)
> > > +{
> > > +#ifdef CONFIG_LINUX
> > > + RAMState *rs = ram_state;
> > > + RAMBlock *bs;
> > > + assert(rs->uffdio_fd >= 0);
> >
> > Maybe too harsh - we can return if it's invalid.
> >
> > Meanwhile, better rcu_read_lock(), as well?
> >
>
> Yep, RCU lock, I'll add. Why too harsh? Just a debug assertion.
I was afraid some special path could trigger ram_write_tracking_stop() being
called before ram_write_tracking_start(), then vm could crash. If we can
guarantee that not happening, then it's also ok with assert().
[...]
> > > +/**
> > > + * uffd_poll_events: poll UFFD file descriptor for read
> > > + *
> > > + * Returns true if events are available for read, false otherwise
> > > + *
> > > + * @uffd: UFFD file descriptor
> > > + * @tmo: timeout in milliseconds, 0 for non-blocking operation,
> > > + * negative value for infinite wait
> > > + */
> > > +bool uffd_poll_events(int uffd, int tmo)
> >
> > Shall we spell "tmo" out?
> In the comment? I think it's ok.
I'd suggest to spell it out everywhere, especially in the codes. But feel free
to take your own preference. Thanks,
--
Peter Xu
