From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wi0-f181.google.com (mail-wi0-f181.google.com [209.85.212.181]) by dpdk.org (Postfix) with ESMTP id 02FB67F18 for ; Fri, 7 Nov 2014 15:43:14 +0100 (CET) Received: by mail-wi0-f181.google.com with SMTP id n3so4815359wiv.8 for ; Fri, 07 Nov 2014 06:52:46 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=kBEgvsby922HQEeieYq96mHo28B9NyjgKDpmjidEVKI=; b=eKrMBd6+MtXzg5r+RW4JkIKtZUsIfSHn5bm9amspnxE/k3+OODbgmbvFG+6rAp9hDM bAO987+9KoukcMX9I8D80Aqy32uJ8JxVt0gjBfz/esFpvn1fALn9ChFR8GJi6NLBpsgk gKBZBw0mAjzrquREDI8ruIGhUTVb/M7Xcrw2TM9crtPkENpLWbzd27Z+9nkTuLJfrrRN X1hyoIIXvicJpyixLP4DqCwPRNLdwu3g7VLV0UPtYqTnYE0t0yM6NvRx5lfXrMcCn67C D75xissnKoaiu6KgptQDcbTLht/cDHD0BZGRnmqNlbRLQpNriK+OKFtm2MGou/HkAdGx 7LdA== MIME-Version: 1.0 X-Received: by 10.180.93.37 with SMTP id cr5mr5869620wib.76.1415371966089; Fri, 07 Nov 2014 06:52:46 -0800 (PST) Received: by 10.27.86.144 with HTTP; Fri, 7 Nov 2014 06:52:46 -0800 (PST) In-Reply-To: <20141107144410.GC12092@bricha3-MOBL3> References: <20141106092228.GA3056@bricha3-MOBL3> <9190772.1rnKUO3oNV@xps13> <545b6b74.a96db40a.26af.ffffe7fb@mx.google.com> <20141106135951.GB7252@bricha3-MOBL3> <20141107094521.GB4628@bricha3-MOBL3> <20141107135303.GB12092@bricha3-MOBL3> <20141107144410.GC12092@bricha3-MOBL3> Date: Fri, 7 Nov 2014 16:52:46 +0200 Message-ID: From: jigsaw To: Bruce Richardson Content-Type: text/plain; charset=UTF-8 X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Cc: "dev@dpdk.org" Subject: Re: [dpdk-dev] =?utf-8?b?562U5aSNOiAgW1BBVENIXSBBZGQgdXNlciBkZWZpbmVk?= =?utf-8?q?_tag_calculation_callback_tolibrte=5Fdistributor=2E?= X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 07 Nov 2014 14:43:14 -0000 Yeah that's better. As below, right? @@ -290,6 +294,7 @@ rte_distributor_process(struct rte_distributor *d, match |= (!(d->in_flight_tags[i] ^ new_tag) << i); + match &= d->in_flight_bitmask; if (match) { next_mb = NULL; unsigned worker = __builtin_ctz(match); On Fri, Nov 7, 2014 at 4:44 PM, Bruce Richardson wrote: > On Fri, Nov 07, 2014 at 04:31:18PM +0200, jigsaw wrote: > > Hi Bruce, > > > > Pls have a quick look at the diff to see if this is exactly what you mean > > about the bitmask. > > I just wrote it without even compiling, just to express the idea. So it > may > > leave some places unpatched. > > If this is agreed, I will make a decent test to verify it before sending > > the patch for RFC. > > > > diff --git a/lib/librte_distributor/rte_distributor.c > > b/lib/librte_distributor/rte_di > > index 585ff88..d606bcf 100644 > > --- a/lib/librte_distributor/rte_distributor.c > > +++ b/lib/librte_distributor/rte_distributor.c > > @@ -92,6 +92,8 @@ struct rte_distributor { > > unsigned num_workers; /**< Number of workers > > polling */ > > > > uint32_t in_flight_tags[RTE_MAX_LCORE]; > > + uint32_t in_flight_bitmask; > > + > > struct rte_distributor_backlog backlog[RTE_MAX_LCORE]; > > > > union rte_distributor_buffer bufs[RTE_MAX_LCORE]; > > @@ -188,6 +190,7 @@ static inline void > > handle_worker_shutdown(struct rte_distributor *d, unsigned wkr) > > { > > d->in_flight_tags[wkr] = 0; > > + d->in_flight_mask &= ~(1 << wkr); > > d->bufs[wkr].bufptr64 = 0; > > if (unlikely(d->backlog[wkr].count != 0)) { > > /* On return of a packet, we need to move the > > @@ -241,6 +244,7 @@ process_returns(struct rte_distributor *d) > > else { > > d->bufs[wkr].bufptr64 = > RTE_DISTRIB_GET_BUF; > > d->in_flight_tags[wkr] = 0; > > + d->in_flight_mask &= ~(1 << wkr); > > } > > oldbuf = data >> RTE_DISTRIB_FLAG_BITS; > > } else if (data & RTE_DISTRIB_RETURN_BUF) { > > @@ -282,12 +286,13 @@ rte_distributor_process(struct rte_distributor *d, > > next_mb = mbufs[next_idx++]; > > next_value = (((int64_t)(uintptr_t)next_mb) > > << RTE_DISTRIB_FLAG_BITS); > > - new_tag = (next_mb->hash.rss | 1); > > + new_tag = next_mb->hash.rss; > > > > uint32_t match = 0; > > unsigned i; > > for (i = 0; i < d->num_workers; i++) > > - match |= (!(d->in_flight_tags[i] ^ > new_tag) > > + match |= (((!(d->in_flight_tags[i] ^ > > new_tag)) & > > + (d->in_flight_bitmask >> > i)) > > I would not do the bitmask comparison here, as that's extra instruction in > the > loop. Instead, because its a bitmask, build up the match variable as it was > before, and then just do a single and operation afterwards, outside the > loop > body. > > /Bruce > > > << i); > > > > if (match) { > > @@ -309,6 +314,7 @@ rte_distributor_process(struct rte_distributor *d, > > else { > > d->bufs[wkr].bufptr64 = next_value; > > d->in_flight_tags[wkr] = new_tag; > > + d->in_flight_bitmask |= 1 << wkr; > > next_mb = NULL; > > } > > oldbuf = data >> RTE_DISTRIB_FLAG_BITS; > > > > > > >