[CFR] Kevent timer improvements

[Baptiste Daroussin]

On Sun, Apr 13, 2014 at 11:20:38PM +0300, Konstantin Belousov wrote:
> On Sun, Apr 13, 2014 at 09:19:28PM +0200, Baptiste Daroussin wrote:
> > On Sun, Apr 13, 2014 at 05:20:28PM +0300, Konstantin Belousov wrote:
> > > On Sun, Apr 13, 2014 at 03:15:51PM +0200, Baptiste Daroussin wrote:
> > > > I have splitted my patch in multiple parts, let start with the first one:
> > > > adding NOTE_NSECONDS, NOTE_USECONDS, NOTE_NSECONDS
> > > > 
> > > > http://people.freebsd.org/~bapt/kevent_timer.diff
> > > 
> > [...]
> > > > +	else if (flags & NOTE_NSECONDS)
> > > > +		modifier = SBT_1NS;
> > > It is better to put the 'modifier = SBT_1MS;' statement as the else part.
> > > 
> > > That said, IMO it would be sometimes beneficial to have real flag to
> > > specify milliseconds precision, in addition to milliseconds be the
> > > default.
> > 
> > Done in the new patch
> > > > +
> > > > +		    timer2sbintime(kn->kn_sdata, kn->kn_sfflags), 0 /* 1ms? */,
> > > There, at least the comment about precision should be updated.
> > > But, it seems that for the seconds precision, it makes sense to
> > > specify e.g. 1/2 sec as precision; or add an API flag to allow imprecise
> > > callout scheduling.
> > > 
> > While I do agree this might be useful I do not have time to work on that right
> > now so I just removed the comment which wasn't really accurate anyway.
> > 
> > Is that ok?
> Leave XXX comment right near the zero argument, at least ?
> 
> > +	if (flags & NOTE_SECONDS)
> > +		modifier = SBT_1S;
> > +	else if (flags & NOTE_USECONDS)
> > +		modifier = SBT_1US;
> > +	else if (flags & NOTE_NSECONDS)
> > +		modifier = SBT_1NS;
> > +	else
> > +		modifier = SBT_1MS;
> There should be a case for NOTE_MSECONDS.
> 
ok I'll do
> Hm, I think the checks should be made stronger. System should ensure
> that only one flag is passed, and no invalid flags are supplied, right ?

I can check that only one flags is passed and no invalid flagrs are supplied but
from what I see not a single part of kqueue(2) is checking that, so won't that
be inconsistent with the rest of the API?

I can probably check that only ona or 0 of the NOTE_*SECONDS is passed and
return EINVAL in that case but no more.

(note that more NOTE_* are to be added later)

regards,
Bapt
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://lists.freebsd.org/pipermail/freebsd-hackers/attachments/20140414/878a4813/attachment.sig>