Am 16.08.2017 um 10:16 schrieb Jiri Pirko:
Wed, Aug 16, 2017 at 09:49:07AM CEST, christian.koe...@amd.com wrote:
Am 16.08.2017 um 04:12 schrieb Chris Mi:
Using current TC code, it is very slow to insert a lot of rules.
In order to improve the rules update rate in TC,
we introduced the following two changes:
1) changed cls_flower to use IDR to manage the filters.
2) changed all act_xxx modules to use IDR instead of
a small hash table
But IDR has a limitation that it uses int. TC handle uses u32.
To make sure there is no regression, we also changed IDR to use
unsigned long. All clients of IDR are changed to use new IDR API.
WOW, wait a second. The idr change is touching a lot of drivers and to be
honest doesn't looks correct at all.
Just look at the first chunk of your modification:
@@ -998,8 +999,9 @@ int bsg_register_queue(struct request_queue *q, struct
device *parent,
mutex_lock(&bsg_mutex);
- ret = idr_alloc(&bsg_minor_idr, bcd, 0, BSG_MAX_DEVS, GFP_KERNEL);
- if (ret < 0) {
+ ret = idr_alloc(&bsg_minor_idr, bcd, &idr_index, 0, BSG_MAX_DEVS,
+ GFP_KERNEL);
+ if (ret) {
if (ret == -ENOSPC) {
printk(KERN_ERR "bsg: too many bsg devices\n");
ret = -EINVAL;
The condition "if (ret)" will now always be true after the first allocation
and so we always run into the error handling after that.
On success, idr_alloc returns 0.
Ah, I see. You change the idr_alloc to return the resulting index as
separate parameter.
You should explicit note that in the commit message, cause that is
something easily overlooked.
In general I strongly suggest to add a separate interface for allocating
unsigned long handles, use that for the while being and then move the
existing drivers over bit by bit.
A single patch which touches so many different driver is practically
impossible to review consequently.
I've never read the bsg code before, but that's certainly not correct. And
that incorrect pattern repeats over and over again in this code.
Apart from that why the heck do you want to allocate more than 1<<31 handles?
tc action indexes for example. That is part of this patchset.
Well, let me refine the question: Why does tc action indexes need more
than 31 bits? From an outside view that looks like pure overkill.
Regards,
Christian.
