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

[Andrey Gruzdev]

On 30.11.2020 18:34, Peter Xu wrote:
> 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().
>
> [...]

It can't really happen, but I agree that assert() is a bit excessive here.

> > > > +/**
> > > > + * 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,

Thanks,

--
Andrey Gruzdev, Principal Engineer
Virtuozzo GmbH  +7-903-247-6397
                virtuzzo.com