From: "Carrillo, Erik G" <erik.g.carrillo@intel.com>
To: Pavan Nikhilesh Bhagavatula <pbhagavatula@caviumnetworks.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [RFC PATCH v4 2/4] eventtimer: add common code
Date: Fri, 1 Dec 2017 20:19:43 +0000 [thread overview]
Message-ID: <BE54F058557D9A4FAC1D84E2FC6D87570ED7EF89@fmsmsx115.amr.corp.intel.com> (raw)
In-Reply-To: <20171201051314.osskzpohzike7r3h@Pavan-LT>
> -----Original Message-----
> From: Pavan Nikhilesh Bhagavatula
> [mailto:pbhagavatula@caviumnetworks.com]
> Sent: Thursday, November 30, 2017 11:13 PM
> To: Carrillo, Erik G <erik.g.carrillo@intel.com>
> Cc: dev@dpdk.org
> Subject: Re: [RFC PATCH v4 2/4] eventtimer: add common code
>
> On Thu, Nov 30, 2017 at 08:59:19PM +0000, Carrillo, Erik G wrote:
> > Hi Pavan,
> >
> > Thanks for the review; I'm working on addressing the comments and have
> the following question (inline):
> >
> > <... snipped ...>
> >
> > > > + adapter->data->mz = mz;
> > > > + adapter->data->event_dev_id = conf->event_dev_id;
> > > > + adapter->data->id = adapter_id;
> > > > + adapter->data->socket_id = conf->socket_id;
> > > > + adapter->data->conf = *conf; /* copy conf structure */
> > > > +
> > > > + /* Query eventdev PMD for timer adapter capabilities and ops */
> > > > + ret = dev->dev_ops->timer_adapter_caps_get(dev,
> > > > + &adapter->data->caps,
> > > > + &adapter->ops);
> > >
> > > The underlying driver needs info about the adapter flags i.e.
> > > RTE_EVENT_TIMER_ADAPTER_F_ADJUST_RES and
> > > RTE_EVENT_TIMER_ADAPTER_F_SP_PUT so we need to pass those too
> conf-
> > > >flags.
> >
> > By "underlying driver", are you referring to the eventdev PMD, or the
> event timer adapter "driver" (i.e., the set of ops functions)?
> >
> > If the latter, the adapter "driver" will have a chance to inspect the flags
> when adapter->ops->init() is called below, since it can look at the flags
> through the adapter arg.
> >
>
> I was refering to the timer driver, the presence of flag
> RTE_EVENT_TIMER_ADAPTER_F_SP_PUT would suggest the driver to use a
> multi thread unsafe arm/cancel data path API and it would set a different
> function pointers to adapter->arm_burst etc.
>
> I dont think in the current scheme this is possible. Currently, if we see
> mempool it inspects flags before setting ops.
>
> Hope this clears things up.
>
> -Pavan
Yes, I see your point now. I agree that it would be useful to allow different ops structures to be selected based on the flags that are set in addition to being able to inspect the flags within the ops functions themselves. I have made the change in the follow-up patch series.
Thanks,
Gabriel
>
> > If the former, will the eventdev PMD consider the flags when deciding
> whether or not to provide an ops definition in the timer_adapter_caps_get()
> call?
> >
> > >
> > > > + if (ret < 0) {
> > > > + rte_errno = -EINVAL;
> > > > + return NULL;
> > > > + }
> > > > +
> > > > + if (!(adapter->data->caps &
> > > > + RTE_EVENT_TIMER_ADAPTER_CAP_INTERNAL_PORT)) {
> > > > + FUNC_PTR_OR_NULL_RET_WITH_ERRNO(conf_cb, -EINVAL);
> > > > + ret = conf_cb(adapter->data->id, adapter->data-
> > > >event_dev_id,
> > > > + &adapter->data->event_port_id, conf_arg);
> > > > + if (ret < 0) {
> > > > + rte_errno = -EINVAL;
> > > > + return NULL;
> > > > + }
> > > > + }
> > > > +
> > > > + /* Allow driver to do some setup */
> > > > + FUNC_PTR_OR_NULL_RET_WITH_ERRNO(adapter->ops->init, -
> > > ENOTSUP);
> > > > + ret = adapter->ops->init(adapter);
> > > > + if (ret < 0) {
> > > > + rte_errno = -EINVAL;
> > > > + return NULL;
> > > > + }
> > > > +
> > > > + /* Set fast-path function pointers */
> > > > + adapter->arm_burst = adapter->ops->arm_burst;
> > > > + adapter->arm_tmo_tick_burst = adapter->ops-
> > > >arm_tmo_tick_burst;
> > > > + adapter->cancel_burst = adapter->ops->cancel_burst;
> > > > +
> > > > + adapter->allocated = 1;
> > > > +
> > > > + return adapter;
> > > > +}
> >
> > <... snipped ...>
next prev parent reply other threads:[~2017-12-01 20:19 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-08-17 16:11 [dpdk-dev] [RFC PATCH 0/1] eventtimer: introduce event timer wheel Jerin Jacob
2017-08-17 16:11 ` [dpdk-dev] [RFC PATCH 1/1] " Jerin Jacob
2017-08-23 22:57 ` [dpdk-dev] [RFC PATCH 0/1] " Carrillo, Erik G
2017-08-25 10:25 ` Jerin Jacob
2017-08-29 15:02 ` Thomas Monjalon
2017-08-29 15:41 ` Jerin Jacob
2017-08-29 15:48 ` Thomas Monjalon
2017-08-29 16:07 ` Jerin Jacob
2017-09-22 15:17 ` [dpdk-dev] [RFC PATCH v2 0/1] eventtimer: introduce event timer adapter Erik Gabriel Carrillo
2017-09-22 15:17 ` [dpdk-dev] [RFC PATCH v2 1/1] " Erik Gabriel Carrillo
2017-10-03 14:37 ` [dpdk-dev] [RFC PATCH v2 0/1] " Jerin Jacob
2017-10-09 20:30 ` Carrillo, Erik G
2017-10-16 12:04 ` Pavan Nikhilesh Bhagavatula
2017-10-16 12:37 ` Pavan Nikhilesh Bhagavatula
2017-10-18 21:48 ` Carrillo, Erik G
2017-10-26 15:45 ` Pavan Nikhilesh Bhagavatula
2017-11-20 22:35 ` [dpdk-dev] [RFC PATCH v3 " Erik Gabriel Carrillo
2017-11-20 22:35 ` [dpdk-dev] [RFC PATCH v3 1/1] " Erik Gabriel Carrillo
2017-11-23 4:37 ` Pavan Nikhilesh Bhagavatula
2017-11-27 14:47 ` Carrillo, Erik G
2017-11-28 17:40 ` [dpdk-dev] [RFC PATCH v4 0/4] " Erik Gabriel Carrillo
2017-11-28 17:40 ` [dpdk-dev] [RFC PATCH v4 1/4] " Erik Gabriel Carrillo
2017-11-29 10:29 ` Pavan Nikhilesh Bhagavatula
2017-11-28 17:40 ` [dpdk-dev] [RFC PATCH v4 2/4] eventtimer: add common code Erik Gabriel Carrillo
2017-11-29 5:19 ` Pavan Nikhilesh Bhagavatula
2017-11-30 20:59 ` Carrillo, Erik G
2017-12-01 5:13 ` Pavan Nikhilesh Bhagavatula
2017-12-01 20:19 ` Carrillo, Erik G [this message]
2017-11-28 17:40 ` [dpdk-dev] [RFC PATCH v4 3/4] eventtimer: add default software implementation stub Erik Gabriel Carrillo
2017-11-29 10:34 ` Pavan Nikhilesh Bhagavatula
2017-11-30 23:56 ` Carrillo, Erik G
2017-12-01 5:15 ` Pavan Nikhilesh Bhagavatula
2017-11-28 17:40 ` [dpdk-dev] [RFC PATCH v4 4/4] test: add event timer adapter auto-test Erik Gabriel Carrillo
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=BE54F058557D9A4FAC1D84E2FC6D87570ED7EF89@fmsmsx115.amr.corp.intel.com \
--to=erik.g.carrillo@intel.com \
--cc=dev@dpdk.org \
--cc=pbhagavatula@caviumnetworks.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 NNTP newsgroup(s).