[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Minios-devel] [UNIKRAFT PATCH v4 3/9] plat/linuxu: Add linuxu (x86_64) timer support



Hi Sharan,

going through these comments at the moment:

On 08/27/2018 02:38 PM, Costin Lupu wrote:

Missing include of signal.h

Right, more than that the current patch should come *after* the patch
adding interrupts support in linuxu.

I'll reverse the order and include linuxu/signal.h

+void ukplat_time_init(void)
+{
+    struct uk_sigevent sigev;
+    struct itimerspec its;
+    int rc;
+
+    ukplat_irq_register(TIMER_SIGNUM, timer_handler, NULL);
+
+    memset(&sigev, 0, sizeof(sigev));
+    sigev.sigev_notify = 0;


A minor comment, prefer if we use SIGALRM inplace of TIMER_SIGNUM :)
since we are working with signal numbers here.

TIMER_SIGNUM is a define over the preferred signal number (in our case
SIGALRM). I don't see what's wrong with this approach.

I think this is fine and we can keep it like that?

     void ukplat_time_fini(void)
   {


Do we want to unregister the timer interrupt while cleaning up the
platform?

Yeah, why not?

+    sys_timer_delete(timerid);
   }


Also seems like the right thing to do to me. ANy reasons why that would be a bad idea?

Cheers,
Florian

--
Dr. Florian Schmidt
フローリアン・シュミット
Research Scientist,
Systems and Machine Learning Group
NEC Laboratories Europe
Kurfürsten-Anlage 36, D-69115 Heidelberg
Tel.     +49 (0)6221 4342-265
Fax:     +49 (0)6221 4342-155
e-mail:  florian.schmidt@xxxxxxxxx
============================================================
Registered at Amtsgericht Mannheim, Germany, HRB728558

_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.