Re: [PATCH v4 2/6] introduce UFFD-WP low-level interface helpers

[Peter Xu <peterx@redhat.com>]

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