All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Toke Høiland-Jørgensen" <toke@redhat.com>
To: Gautam Ramakrishnan <gautamramk@gmail.com>
Cc: Jamal Hadi Salim <jhs@mojatatu.com>,
	davem@davemloft.net, netdev@vger.kernel.org,
	"Mohit P . Tahiliani" <tahiliani@nitk.edu.in>,
	"Sachin D . Patil" <sdp.sachin@gmail.com>,
	Mohit Bhasi <mohitbhasi1998@gmail.com>,
	"V . Saicharan" <vsaicharan1998@gmail.com>,
	Leslie Monis <lesliemonis@gmail.com>,
	Dave Taht <dave.taht@gmail.com>
Subject: Re: [RFC net-next 2/2] net: sched: fq_pie: Flow Queue PIE AQM
Date: Mon, 08 Apr 2019 10:54:37 +0200	[thread overview]
Message-ID: <87tvf8zxki.fsf@toke.dk> (raw)
In-Reply-To: <CADAms0xKCFQLOvUP85hox7DoBgS8v3MLwzQibm7FOrYR6xzQEA@mail.gmail.com>

Gautam Ramakrishnan <gautamramk@gmail.com> writes:

> I was trying to refactor the code and I ran into some issues.
> 1. I moved some of the parameters such as flows_cnt into a new struct
> called fq_pie_params, instead of keeping them in fq_pie_sched_data.
> Should I move those parameters back into fq_pie_sched_data?

Hmm, this is getting into taste here, but I think I would just put them
directly into fq_pie_sched_data; after all, you are not reusing them in
multiple places, and things like 'q->params.p_params.target' is a bit
much for my taste :)

> 2. fq_codel maintains the backlog variable as a list in the
> fq_codel_sched_data, whereas I maintain a backlog in the struct
> fq_pie_flow. What approach should I follow?

The reason fq_codel does this is to keep that memory contiguous, so that
when it loops through it all (in fq_codel_drop()), it is less memory
that has to be loaded into cache. Since you're not doing that in fq_pie,
it's not strictly necessary to mirror this (admittedly, somewhat
confusing) aspect of fq_codel :)

> 3. Would maintaining a per flow pie_stats be useful in the future? I
> do not use the per flow stats anywhere in the code.

Hard to know what is useful in the future. Is it useful now? It's easier
to add new features later on than to remove them once they are there...
So if you don't have an immediate use case for it, my suggestion would
be to remove it, and add it back if someone really ends up needing it :)

-Toke

  parent reply	other threads:[~2019-04-08  8:54 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-31 17:40 [RFC net-next 0/2] net: sched: add Flow Queue PIE AQM Gautam Ramakrishnan
2019-03-31 17:40 ` [RFC net-next 1/2] net: sched: pie: refactor PIE Gautam Ramakrishnan
2019-03-31 17:40 ` [RFC net-next 2/2] net: sched: fq_pie: Flow Queue PIE AQM Gautam Ramakrishnan
2019-04-02 10:49   ` Toke Høiland-Jørgensen
2019-04-02 15:57     ` Gautam Ramakrishnan
2019-04-02 17:25       ` Toke Høiland-Jørgensen
2019-04-08  5:31         ` Gautam Ramakrishnan
2019-04-08  5:37           ` Dave Taht
2019-04-08  5:52             ` Dave Taht
2019-04-08  8:54           ` Toke Høiland-Jørgensen [this message]
2019-04-01 12:04 ` [RFC net-next 0/2] net: sched: add " Toke Høiland-Jørgensen
2019-04-01 13:31   ` Gautam Ramakrishnan
2019-04-02 10:36     ` Toke Høiland-Jørgensen

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=87tvf8zxki.fsf@toke.dk \
    --to=toke@redhat.com \
    --cc=dave.taht@gmail.com \
    --cc=davem@davemloft.net \
    --cc=gautamramk@gmail.com \
    --cc=jhs@mojatatu.com \
    --cc=lesliemonis@gmail.com \
    --cc=mohitbhasi1998@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=sdp.sachin@gmail.com \
    --cc=tahiliani@nitk.edu.in \
    --cc=vsaicharan1998@gmail.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.