On Wed, Jul 06, 2016 at 10:18:32AM +0100, Tvrtko Ursulin wrote:
>
> On 06/07/16 08:45, Chris Wilson wrote:
> >As we inspect both the tasklet (to check for an active bottom-half) and
> >set the irq-posted flag at the same time (both in the interrupt handler
> >and then in the bottom-halt), group those two together into the same
> >cacheline. (Not having total control over placement of the struct means
> >we can't guarantee the cacheline boundary, we need to align the kmalloc
> >and then each struct, but the grouping should help.)
>
> Any actual difference or just tidy?
Just motivated by tidying. I expect this to be in the noise (but since I
have the tools, I should check just in case).
> >@@ -167,16 +167,20 @@ struct intel_engine_cs {
> > * the overhead of waking that client is much preferred.
> > */
> > struct intel_breadcrumbs {
> >+ struct task_struct *irq_tasklet; /* bh for user interrupts */
>
> Tasklet was kind of passable, irq_tasklet is imho worse. I think
> anyone who see that name would thing this handles interrupts of some
> sort. :)
My thinking was to give a similar name to the three variables used in
the irq handler (and bottom-half) and move them aside from the spinlock.
> How about first_wait_task ?
>
> I know it may feel like pointless bike-shedding and maybe it is so I
> am leaving it with you.
Similarity argument still holds imo.
irq_seqno_bh ?
> >+ unsigned long irq_count;
> >+ bool irq_posted;
> >+
> > spinlock_t lock; /* protects the lists of requests */
> > struct rb_root waiters; /* sorted by retirement, priority */
> > struct rb_root signals; /* sorted by retirement */
> > struct intel_wait *first_wait; /* oldest waiter by retirement */
> >- struct task_struct *tasklet; /* bh for user interrupts */
> > struct task_struct *signaler; /* used for fence signalling */
> > struct drm_i915_gem_request *first_signal;
> > struct timer_list fake_irq; /* used after a missed interrupt */
> >- bool irq_enabled;
> >- bool rpm_wakelock;
> >+
> >+ bool irq_enabled : 1;
> >+ bool rpm_wakelock : 1;
>
> Is there much point in having bitfields? To me two plain bools would
> be just fine and smaller code.
In this case a fractionally smaller struct (-4 bytes)
The code size in this case is identical
text data bss dec hex
1067277 4565 416 1072258 105c82 as bool
1067277 4565 416 1072258 105c82 as bool : 1
since we only do very simple test and sets.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
