Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
To:     Johannes Berg <johannes@sipsolutions.net>,
        linux-wireless@vger.kernel.org
References: <20201216204316.44498-1-nbd@nbd.name>
 <20201216204316.44498-4-nbd@nbd.name>
 <3980921a9276b9d38b12651e40590118f1d65609.camel@sipsolutions.net>
From:   Felix Fietkau <nbd@nbd.name>
Subject: Re: [PATCH 4/7] net/fq_impl: do not maintain a backlog-sorted list of
 flows
Message-ID: <59a58091-9fbc-4150-b839-bbc8e5e63f00@nbd.name>
Date:   Thu, 17 Dec 2020 13:40:13 +0100
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.14; rv:78.0)
 Gecko/20100101 Thunderbird/78.5.0
MIME-Version: 1.0
In-Reply-To: <3980921a9276b9d38b12651e40590118f1d65609.camel@sipsolutions.net>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 2020-12-16 21:59, Johannes Berg wrote:
> On Wed, 2020-12-16 at 21:43 +0100, Felix Fietkau wrote:
>> 
>> +	u32 *flows_bitmap;
> 
>> +	fq->flows_bitmap[idx / 32] &= ~BIT(idx % 32);
> 
>> +	for (base = 0; base < fq->flows_cnt; base += 32) {
>> +		u32 mask = fq->flows_bitmap[base / 32];
> 
> This all seems a little awkward, why not use unsigned long * and
> <linux/bitops.h>?
> 
> The &=~ BIT() thing above is basically __clear_bit() then, the loops
> later are basically for_each_set_bit() if I'm reading things right.
I guess I can simplify this some more. I think I avoided
for_each_set_bit for performance reasons to keep things inline in the
loop, but I'm not sure how much that matters in practice.

- Felix