From: Chris Wilson <chris@chris-wilson.co.uk>
To: "Christian König" <christian.koenig@amd.com>,
"Chris Mi" <chrism@mellanox.com>,
netdev@vger.kernel.org
Cc: lucho@ionkov.net, sergey.senozhatsky.work@gmail.com,
snitzer@redhat.com, wsa@the-dreams.de, markb@mellanox.com,
tom.leiming@gmail.com, stefanr@s5r6.in-berlin.de,
zhi.a.wang@intel.com, nsekhar@ti.com,
dri-devel@lists.freedesktop.org, bfields@fieldses.org,
linux-sctp@vger.kernel.org, paulus@samba.org,
jinpu.wang@profitbricks.com, pshelar@ovn.org,
sumit.semwal@linaro.org, AlexBin.Xie@amd.com,
david1.zhou@amd.com, linux-samsung-soc@vger.kernel.org,
maximlevitsky@gmail.com, sudarsana.kalluru@qlogic.com,
marek.vasut@gmail.com, linux-atm-general@lists.sourceforge.net,
dtwlin@gmail.com, michel.daenzer@amd.com, dledford@redhat.com,
tpmdd-devel@lists.sourceforge.net, stern@rowland.harvard.edu,
longman@redhat.com, niranjana.vishwanathapura@intel.com,
philipp.reisner@linbit.com, shli@kernel.org, linux@roeck-us.net,
ohad@wizery.com, pmladek@suse.com,
dick.kennedy@broadcom.comlinux-
Subject: Re: [patch net-next 0/3] net/sched: Improve getting objects by indexes
Date: Wed, 16 Aug 2017 10:19:53 +0100 [thread overview]
Message-ID: <150287519355.15499.3124883464555211520@mail.alporthouse.com> (raw)
In-Reply-To: <144b87a3-bbe4-a194-ed83-e54840d7c7c2@amd.com>
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
_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k
prev parent reply other threads:[~2017-08-16 9:19 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-08-16 2:12 [patch net-next 0/3] net/sched: Improve getting objects by indexes Chris Mi
[not found] ` <1502849538-14284-1-git-send-email-chrism-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
2017-08-16 2:12 ` [patch net-next 1/3] idr: Use unsigned long instead of int Chris Mi
2017-08-16 2:12 ` [patch net-next 2/3] net/sched: Change cls_flower to use IDR Chris Mi
2017-08-16 2:12 ` [patch net-next 3/3] net/sched: Change act_api and act_xxx modules " Chris Mi
2017-08-16 7:49 ` [patch net-next 0/3] net/sched: Improve getting objects by indexes Christian König
2017-08-16 9:19 ` Chris Wilson [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=150287519355.15499.3124883464555211520@mail.alporthouse.com \
--to=chris@chris-wilson.co.uk \
--cc=AlexBin.Xie@amd.com \
--cc=bfields@fieldses.org \
--cc=chrism@mellanox.com \
--cc=christian.koenig@amd.com \
--cc=david1.zhou@amd.com \
--cc=dick.kennedy@broadcom.comlinux- \
--cc=dledford@redhat.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=dtwlin@gmail.com \
--cc=jinpu.wang@profitbricks.com \
--cc=linux-atm-general@lists.sourceforge.net \
--cc=linux-samsung-soc@vger.kernel.org \
--cc=linux-sctp@vger.kernel.org \
--cc=linux@roeck-us.net \
--cc=longman@redhat.com \
--cc=lucho@ionkov.net \
--cc=marek.vasut@gmail.com \
--cc=markb@mellanox.com \
--cc=maximlevitsky@gmail.com \
--cc=michel.daenzer@amd.com \
--cc=netdev@vger.kernel.org \
--cc=niranjana.vishwanathapura@intel.com \
--cc=nsekhar@ti.com \
--cc=ohad@wizery.com \
--cc=paulus@samba.org \
--cc=philipp.reisner@linbit.com \
--cc=pmladek@suse.com \
--cc=pshelar@ovn.org \
--cc=sergey.senozhatsky.work@gmail.com \
--cc=shli@kernel.org \
--cc=snitzer@redhat.com \
--cc=stefanr@s5r6.in-berlin.de \
--cc=stern@rowland.harvard.edu \
--cc=sudarsana.kalluru@qlogic.com \
--cc=sumit.semwal@linaro.org \
--cc=tom.leiming@gmail.com \
--cc=tpmdd-devel@lists.sourceforge.net \
--cc=wsa@the-dreams.de \
--cc=zhi.a.wang@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).