Messages in this thread | | | Date | Fri, 29 May 2020 15:36:41 +0200 | From | Peter Zijlstra <> | Subject | Re: [RFC][PATCH 5/7] irq_work, smp: Allow irq_work on call_single_queue |
| |
On Fri, May 29, 2020 at 01:40:32AM +0200, Frederic Weisbecker wrote: > On Tue, May 26, 2020 at 06:11:02PM +0200, Peter Zijlstra wrote:
> > +/* > > + * structure shares layout with single_call_data_t. > > + */ > > struct irq_work { > > - atomic_t flags; > > struct llist_node llnode; > > + atomic_t flags; > > > We should probably have: > > struct csd_node { > atomic_t flags; > struct llist_node; > } > > embed inside struct irq_work and struct __call_single_data. Relying on > structure layout for things to work doesn't really clarify things :-)
Yes I know, but changing those structures is going to cause an aweful lot of churn, and I didn't want to do that just now.. :-(
Also, there's more fun..
CSD_TYPE_SYNC/ASYNC:
struct { struct llist_node node; unsigned int flags; smp_call_func_t func; void *info; };
CSD_TYPE_IRQ_WORK:
struct { struct llist_node node; atomic_t flags; void (*func)(struct irq_work *); };
CSD_TYPE_TTWU:
struct { struct llist_node node; unsigned int flags; };
So while they all have a 'u32' sized @flags, irq_work wants it atomic. Also, if we were to actually have the struct csd_node {}, you get a 4 byte hole when you embed it in task_struct.
This is all entirely fugly. No doubt about it.
But I failed to find a 'sane' way to express it and needed to get these patches out because things were broken.
Maybe I can anonymous-union my way around it, dunno. I'll think about it. I'm certainly not proud of this. But at least the BUILD_BUG_ON()s should catch the more blatant breakage here.
| |