[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v2 1/3] tools/libs/evtchn: decouple more from mini-os
On 11/01/2022 15:03, Juergen Gross wrote: > diff --git a/tools/libs/evtchn/minios.c b/tools/libs/evtchn/minios.c > index e5dfdc5ef5..c3a5ce3b98 100644 > --- a/tools/libs/evtchn/minios.c > +++ b/tools/libs/evtchn/minios.c > @@ -38,29 +38,40 @@ > > #include "private.h" > > -extern void minios_evtchn_close_fd(int fd); > +LIST_HEAD(port_list, port_info); > + > +struct port_info { > + LIST_ENTRY(port_info) list; > + evtchn_port_t port; > + bool pending; > + bool bound; > +}; > > extern struct wait_queue_head event_queue; Yuck. This should come from minios's evtchn header, rather than being extern'd like this, but lets consider that future cleanup work. > +int minios_evtchn_close_fd(int fd); You don't need this forward declaration, because nothing in this file calls minios_evtchn_close_fd(). The extern should simply be deleted, and it removes a hunk from your later xen.git series. > @@ -69,18 +80,54 @@ static void port_dealloc(struct evtchn_port_info > *port_info) > free(port_info); > } > > +int minios_evtchn_close_fd(int fd) > +{ > + struct port_info *port_info, *tmp; > + struct file *file = get_file_from_fd(fd); > + struct port_list *port_list = file->dev; Looking at this, the file_ops don't need to have the C ABI. The single caller already needs access to the file structure, so could pass both file and fd in to the ops->close() function. Thoughts? > + > + LIST_FOREACH_SAFE(port_info, port_list, list, tmp) > + port_dealloc(port_info); > + free(port_list); > + > + return 0; > +} > + > +static struct file_ops evtchn_ops = { This wants to become const, when alloc_file_type() has been appropriately const'd. > + .name = "evtchn", > + .close = minios_evtchn_close_fd, > + .select_rd = select_read_flag, > +}; > + > /* > * XENEVTCHN_NO_CLOEXEC is being ignored, as there is no exec() call > supported > * in Mini-OS. > */ > int osdep_evtchn_open(xenevtchn_handle *xce, unsigned int flags) > { > - int fd = alloc_fd(FTYPE_EVTCHN); > + int fd; > + struct file *file; > + struct port_list *list; > + static unsigned int ftype_evtchn; > > - if ( fd == -1 ) > + if ( !ftype_evtchn ) > + ftype_evtchn = alloc_file_type(&evtchn_ops); Hmm. MiniOS doesn't appear to support __attribute__((constructor)) but this would be an ideal candidate. It would remove a non-threadsafe singleton from a (largely unrelated) codepath. Should be very simple to add to MiniOS. See Xen's init_constructors(), and add CONSTRUCTORS to the linker file. > @@ -134,42 +171,43 @@ int xenevtchn_notify(xenevtchn_handle *xce, > evtchn_port_t port) > > static void evtchn_handler(evtchn_port_t port, struct pt_regs *regs, void > *data) > { > - int fd = (int)(intptr_t)data; > - struct evtchn_port_info *port_info; > + xenevtchn_handle *xce = data; > + struct file *file = get_file_from_fd(xce->fd); > + struct port_info *port_info; > + struct port_list *port_list; > > - assert(files[fd].type == FTYPE_EVTCHN); > + assert(file); > + port_list = file->dev; > mask_evtchn(port); > - LIST_FOREACH(port_info, &files[fd].evtchn.ports, list) > + LIST_FOREACH(port_info, port_list, list) > { > if ( port_info->port == port ) > goto found; > } > > - printk("Unknown port for handle %d\n", fd); > + printk("Unknown port for handle %d\n", xce->fd); As you're editing this line anyway, it really wants to become "Unknown port %d for handle %d\n". ~Andrew
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |