On Mon, Mar 6, 2017 at 4:08 PM, Sergiy Kibrik <sergiy.kib...@globallogic.com > wrote:
> On 03/05/2017 11:08 AM, Nadav Har'El wrote: > > > On Fri, Mar 3, 2017 at 11:45 AM, 'Sergiy Kibrik' via OSv Development < > osv-dev@googlegroups.com> wrote: > >> Initialize Xen event interrupt separately from xenbus driver, so that >> events >> can be served before xenbus is up, or even in case when xenbus not being >> probed >> at all or not required, e.g. in case of aarch64 Xen console. >> > > Unfortunately I'm not familiar enough with this code to say much about the > order of doing these things. > > You created a function that runs as a "constructor" but did not specify an > init priority (see include/osv/prio.hh), so your constructor function may > run after all other constructor functions. So, is it possible that you > could call the initialization function explicitly in some existing function > instead of having it be a constructor that runs at an unknown time? It is > clearer when a reader can tell exactly when/where this function gets called. > That being said, I see that before your patch, setup_xen_irq was already a > priority-less constructor function - so I guess there is no harm if you use > one too. > > hi Nadav, > I agree that it would be much clearer (and should be more robust also) to > explicitly specify priority level. > As this code expects interrupt_table to be already up, I'll put it into > level idt + 1 like this: > > static __attribute__((init_priority((int)init_prio::idt + 1))) void > xen_init_irq() > I don't know if that would be clearer, because is idt the only thing you need? I think it's actually better not to specify it at all, as in your original code. The default is priority 65535, so it will come after all the other constructors with specified priorities (including after idt). What could have been better (but I'm not insisting on it) is not to use a "constructor" at all, but rather find the place in the code where you really need this function to run for the first time, and run it there. Since there is no C++ object attached to this "constructor", it might not have to be a constructor at all. However, if it's more convenient for you to keep it a constructor, then by all means, do. -- You received this message because you are subscribed to the Google Groups "OSv Development" group. To unsubscribe from this group and stop receiving emails from it, send an email to osv-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
