From: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>
To: "Yigit, Ferruh" <ferruh.yigit@intel.com>,
Stephen Hemminger <stephen@networkplumber.org>
Cc: Sergey Vyazmitinov <s.vyazmitinov@brain4net.com>,
"olivier.matz@6wind.com" <olivier.matz@6wind.com>,
"dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH] kni: use bulk functions to allocate and free mbufs
Date: Wed, 11 Jan 2017 18:25:07 +0000 [thread overview]
Message-ID: <2601191342CEEE43887BDE71AB9772583F104048@irsmsx105.ger.corp.intel.com> (raw)
In-Reply-To: <63819ae1-f056-0ad7-b7dd-041fe1fe08fa@intel.com>
> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Wednesday, January 11, 2017 5:48 PM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Stephen Hemminger <stephen@networkplumber.org>
> Cc: Sergey Vyazmitinov <s.vyazmitinov@brain4net.com>; olivier.matz@6wind.com; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] kni: use bulk functions to allocate and free mbufs
>
> On 1/11/2017 5:43 PM, Ananyev, Konstantin wrote:
> >
> >
> >> -----Original Message-----
> >> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> >> Sent: Wednesday, January 11, 2017 5:36 PM
> >> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> >> Cc: Sergey Vyazmitinov <s.vyazmitinov@brain4net.com>; olivier.matz@6wind.com; Yigit, Ferruh <ferruh.yigit@intel.com>;
> dev@dpdk.org
> >> Subject: Re: [dpdk-dev] [PATCH] kni: use bulk functions to allocate and free mbufs
> >>
> >> On Wed, 11 Jan 2017 17:28:21 +0000
> >> "Ananyev, Konstantin" <konstantin.ananyev@intel.com> wrote:
> >>
> >>>> -----Original Message-----
> >>>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Stephen Hemminger
> >>>> Sent: Wednesday, January 11, 2017 4:18 PM
> >>>> To: Sergey Vyazmitinov <s.vyazmitinov@brain4net.com>
> >>>> Cc: olivier.matz@6wind.com; Yigit, Ferruh <ferruh.yigit@intel.com>; dev@dpdk.org
> >>>> Subject: Re: [dpdk-dev] [PATCH] kni: use bulk functions to allocate and free mbufs
> >>>>
> >>>> On Fri, 30 Dec 2016 04:50:16 +0700
> >>>> Sergey Vyazmitinov <s.vyazmitinov@brain4net.com> wrote:
> >>>>
> >>>>> /**
> >>>>> + * Free n packets mbuf back into its original mempool.
> >>>>> + *
> >>>>> + * Free each mbuf, and all its segments in case of chained buffers. Each
> >>>>> + * segment is added back into its original mempool.
> >>>>> + *
> >>>>> + * @param mp
> >>>>> + * The packets mempool.
> >>>>> + * @param mbufs
> >>>>> + * The packets mbufs array to be freed.
> >>>>> + * @param n
> >>>>> + * Number of packets.
> >>>>> + */
> >>>>> +static inline void rte_pktmbuf_free_bulk(struct rte_mempool *mp,
> >>>>> + struct rte_mbuf **mbufs, unsigned n)
> >>>>> +{
> >>>>> + struct rte_mbuf *mbuf, *m_next;
> >>>>> + unsigned i;
> >>>>> + for (i = 0; i < n; ++i) {
> >>>>> + mbuf = mbufs[i];
> >>>>> + __rte_mbuf_sanity_check(mbuf, 1);
> >>>>> +
> >>>>> + mbuf = mbuf->next;
> >>>>> + while (mbuf != NULL) {
> >>>>> + m_next = mbuf->next;
> >>>>> + rte_pktmbuf_free_seg(mbuf);
> >>>>> + mbuf = m_next;
> >>>>> + }
> >>>>> + }
> >>>>> + rte_mempool_put_bulk(mp, (void * const *)mbufs, n);
> >>>>> +}
> >>>>
> >>>> The mbufs may come from different pools. You need to handle that.
> >>>
> >>> I suppose both stituations are possible:
> >>> 1) user knows off-hand that all mbufs in the group are from the same mempool
> >>> 2) user can't guarantee that all mbufs in the group are from same mempool.
> >>>
> >>> As I understand that patch is for case 1) only.
> >>> For 2) it could be a separate function and separate patch.
> >>>
> >>> Konstantin
> >>>
> >>>
> >>
> >> Please don't make unnecessary assumptions in pursuit of minor optimizations.
> >
> > I don't suggest to make *any* assumptions.
> > What I am saying we can have 2 functions for two different cases.
>
> kni_free_mbufs() is static function. Even user knows if all mbufs are
> some same pool or not, can't pass this information to the free function.
>
> Of course this information can be passed via new API, or as an update to
> exiting API, but I think it is better to update free function to cover
> both cases instead of getting this information from user.
I suppose misunderstanding came from the fact that kni_free_mbufs()
is modified to use rte_pktmbuf_free_bulk(mp, mbufs, n).
I am not talking about kni part of the patch
(to be honest I didn't pay much attention to it).
What I am saying there are many situations when user knows off-hand
that all mbufs in that group are from the same mempool and such
function will be useful too.
BTW, for my own curiosity, how it could happen with KNI that
kni_fifo_get() would return mbufs not from kni->pktmbuf_pool
(I am not really familiar with KNI and its use-cases)?
Konstantin
>
> > Obviously we'll have to document it properly.
> > Konstantin
> >
> >> It is trivial to write a correct free bulk that handles pool changing.
> >> Also the free_seg could be bulked as well.
next prev parent reply other threads:[~2017-01-11 18:25 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-12-29 21:50 Sergey Vyazmitinov
2017-01-11 10:39 ` Ananyev, Konstantin
2017-01-11 16:17 ` Stephen Hemminger
2017-01-11 16:38 ` Olivier MATZ
2017-01-11 17:00 ` Ferruh Yigit
2017-01-11 17:28 ` Ananyev, Konstantin
2017-01-11 17:35 ` Stephen Hemminger
2017-01-11 17:43 ` Ananyev, Konstantin
2017-01-11 17:47 ` Ferruh Yigit
2017-01-11 18:25 ` Ananyev, Konstantin [this message]
2017-01-11 18:41 ` Ferruh Yigit
2017-01-11 18:56 ` Stephen Hemminger
2017-01-16 7:39 ` Yuanhan Liu
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=2601191342CEEE43887BDE71AB9772583F104048@irsmsx105.ger.corp.intel.com \
--to=konstantin.ananyev@intel.com \
--cc=dev@dpdk.org \
--cc=ferruh.yigit@intel.com \
--cc=olivier.matz@6wind.com \
--cc=s.vyazmitinov@brain4net.com \
--cc=stephen@networkplumber.org \
/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).