Quoting Christian König (2017-08-16 08:49:07) > 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. ret is now purely the error code, so it doesn't look that suspicious. > 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? And more to the point, arbitrarily changing the maximum to ULONG_MAX where the ABI only supports U32_MAX is dangerous. Unless you do the analysis otherwise, you have to replace all the end=0 with end=INT_MAX to maintain existing behaviour. -Chris _______________________________________________ greybus-dev mailing list greybus-dev@xxxxxxxxxxxxxxxx https://lists.linaro.org/mailman/listinfo/greybus-dev
![]() |