Messages in this thread | | | Date | Wed, 10 Apr 2024 06:52:56 -0700 | From | Breno Leitao <> | Subject | Re: [PATCH net-next v2 4/4] net: dqs: make struct dql more cache efficient |
| |
On Tue, Apr 09, 2024 at 05:21:49PM -0700, Jakub Kicinski wrote: > On Mon, 8 Apr 2024 10:25:56 -0700 Breno Leitao wrote: > > With the previous change, struct dqs->stall_thrs will be in the hot path > > (at queue side), even if DQS is disabled. > > > > The other fields accessed in this function (last_obj_cnt and num_queued) > > are in the first cache line, let's move this field (stall_thrs) to the > > very first cache line, since there is a hole there. > > > > This does not change the structure size, since it moves an short (2 > > bytes) to 4-bytes whole in the first cache line. > > Doesn't this move the cache line bouncing problem to the other side?
I think so. Looking at dql_check_stall(), it only uses fields in the second cache line, except now 'dql->stall_thrs' that is in the first cache line.
> Eric said "copy" I read that as "have two fields with the same value".
Sorry, I misunderstood it. I can create two fields, and update them together at the only place where they will be updated (bql_set_stall_thrs).
> I think it's single digit number of alu instructions we'd be saving > here, not super convinced patch 3 is the right trade off...
Right. I was more concerned about the write barriers (smp_wmb()) inside the loop which happen quite frequently.
But, if you think that the this is not the right approach, I can drop this whole patchset. Do you think a profiler will us here?
Thanks!
| |