netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jarek Poplawski <jarkao2@gmail.com>
To: jamal <hadi@cyberus.ca>
Cc: David Miller <davem@davemloft.net>,
	netdev@vger.kernel.org, slavon@bigtelecom.ru, kaber@trash.net
Subject: Re: [PATCH 1/3 v2][NET] gen_estimator: faster gen_kill_estimator
Date: Tue, 22 Jan 2008 20:22:14 +0100	[thread overview]
Message-ID: <20080122192214.GA2629@ami.dom.local> (raw)
In-Reply-To: <1201010068.4443.66.camel@localhost>

On Tue, Jan 22, 2008 at 08:54:28AM -0500, jamal wrote:
> On Tue, 2008-22-01 at 13:29 +0100, Jarek Poplawski wrote:
> > On Tue, Jan 22, 2008 at 06:42:07AM -0500, jamal wrote:
> > ...
> > > Jarek,
> > > 
> > > That looks different from the suggestion from Dave.
> > 
> > Hmm..., I'm not sure you mean my or your suggestion here, but you
> > are right anyway...
> 
> Your idea to grab a pointer to the estimator so you can quickly find it
> upon destruction is a good one.

Let's say it's quite common in the kernel type of idea...

> The challenge was not to break the ABI to user space.
> Dave suggested to use a different struct for the kernel side and
> maintain the user one as is[1]. Your patch didnt do this, hence my
> statement;->

Sure! David's idea is very good, but before reading his message I
decided to try something 'safer'. I simply don't know these structures
like you or David, and after these mistakes at the beginning, I tried
to avoid the next. But now I see it shouldn't be so hard, and I'll do
this David's way, I hope!

> > Maybe I miss something, but there still could be a lot of this walking
> 
> Indeed, that is possible in the case of many estimators configured with
> the same interval - because they will all fall in the same table bucket
> and the idx is not that useful to begin with.
> 
> I was wrong given the nature of interval - the majority of the users
> will have an estimator interval of say 1 sec which will put everything
> in one bucket still. 
> We could introduce a proper index that will allow proper distribution
> and have that stored by the class. I am not sure i made sense.
> But you are coding - and your idea sounds better.

As a matter of fact, I've considered yesterday some additional hash
table too, but wasn't sure it's worth of the savings. It seems there
should be considered optimization of these estimator structures yet.

But since there are now a few similar reports about misterious
problems during deletion of large qdisc, it would be nice to remove
main suspects around this at any price...

Many thanks for your concern with this! (And, if miss something again,
don't be afraid to make it straight & clear; I really prefer to know
the truth about my mistakes, than polite silence.)

Regards,
Jarek P.

> [1] This is _not uncommon_ (note the usage of double negation here for
> emphasis;->) technique actually; ones that go further for example can be
> found all over the net/sched code (struct tcf_police vs tc_police) etc.

I think David's points were very clear and of course right!; it seems now
that only my 'usual' way of friendly joking about this could be more
challenge than any double-negation and turns friendly fire insted...

  reply	other threads:[~2008-01-22 19:19 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-01-20 23:46 [PATCH 1/3][NET] gen_estimator: faster gen_kill_estimator Jarek Poplawski
2008-01-21 10:35 ` David Miller
2008-01-21 10:56   ` Jarek Poplawski
2008-01-21 22:31 ` [PATCH 1/3 v2][NET] " Jarek Poplawski
2008-01-21 22:41   ` Jarek Poplawski
2008-01-22  0:29   ` David Miller
2008-01-22  7:21     ` Jarek Poplawski
2008-01-22  7:26       ` Jarek Poplawski
2008-01-22 11:42       ` jamal
2008-01-22 12:29         ` Jarek Poplawski
2008-01-22 13:54           ` jamal
2008-01-22 19:22             ` Jarek Poplawski [this message]
2008-01-25 22:00     ` [PATCH v3][NET] " Jarek Poplawski

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=20080122192214.GA2629@ami.dom.local \
    --to=jarkao2@gmail.com \
    --cc=davem@davemloft.net \
    --cc=hadi@cyberus.ca \
    --cc=kaber@trash.net \
    --cc=netdev@vger.kernel.org \
    --cc=slavon@bigtelecom.ru \
    /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 NNTP newsgroup(s).