Messages in this thread | | | Date | Tue, 31 Mar 2009 23:44:48 +0300 | From | Avi Kivity <> | Subject | Re: [RFC PATCH 01/17] shm-signal: shared-memory signals |
| |
Gregory Haskins wrote: > This interface provides a bidirectional shared-memory based signaling > mechanism. It can be used by any entities which desire efficient > communication via shared memory. The implementation details of the > signaling are abstracted so that they may transcend a wide variety > of locale boundaries (e.g. userspace/kernel, guest/host, etc). > > The shm_signal mechanism supports event masking as well as spurious > event delivery mitigation. > + > +/* > + *--------- > + * The following structures represent data that is shared across boundaries > + * which may be quite disparate from one another (e.g. Windows vs Linux, > + * 32 vs 64 bit, etc). Therefore, care has been taken to make sure they > + * present data in a manner that is independent of the environment. > + *----------- > + */ > + > +#define SHM_SIGNAL_MAGIC 0x58fa39df > +#define SHM_SIGNAL_VER 1 > + > +struct shm_signal_irq { > + __u8 enabled; > + __u8 pending; > + __u8 dirty; > +}; >
Some ABIs may choose to pad this, suggest explicit padding.
> + > +enum shm_signal_locality { > + shm_locality_north, > + shm_locality_south, > +}; > + > +struct shm_signal_desc { > + __u32 magic; > + __u32 ver; > + struct shm_signal_irq irq[2]; > +}; >
Similarly, this should be padded to 0 (mod 8).
Instead of versions, I prefer feature flags which can be independently enabled or disabled.
> + > +/* --- END SHARED STRUCTURES --- */ > + > +#ifdef __KERNEL__ > + > +#include <linux/interrupt.h> > + > +struct shm_signal_notifier { > + void (*signal)(struct shm_signal_notifier *); > +}; >
This means "->inject() has been called from the other side"?
(reading below I see this is so. not used to reading well commented code... :)
> + > +struct shm_signal; > + > +struct shm_signal_ops { > + int (*inject)(struct shm_signal *s); > + void (*fault)(struct shm_signal *s, const char *fmt, ...); >
Eww. Must we involve strings and printf formats?
> + void (*release)(struct shm_signal *s); > +}; > + > +/* > + * signaling protocol: > + * > + * each side of the shm_signal has an "irq" structure with the following > + * fields: > + * > + * - enabled: controlled by shm_signal_enable/disable() to mask/unmask > + * the notification locally > + * - dirty: indicates if the shared-memory is dirty or clean. This > + * is updated regardless of the enabled/pending state so that > + * the state is always accurately tracked. > + * - pending: indicates if a signal is pending to the remote locale. > + * This allows us to determine if a remote-notification is > + * already in flight to optimize spurious notifications away. > + */ >
When you overlay a ring on top of this, won't the ring indexes convey the same information as ->dirty?
-- I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain.
| |