[v2,1/4] eal: add API to check if its interrupt context
Checks
Commit Message
Added an API to check if current execution is in interrupt
context. This will be helpful to handle nested interrupt cases.
Signed-off-by: Harman Kalra <hkalra@marvell.com>
Signed-off-by: Sunil Kumar Kori <skori@marvell.com>
---
v2:
- Rebased patch on 19.11-rc4
lib/librte_eal/common/include/rte_interrupts.h | 15 +++++++++++++++
lib/librte_eal/freebsd/eal/eal_interrupts.c | 5 +++++
lib/librte_eal/linux/eal/eal_interrupts.c | 5 +++++
lib/librte_eal/rte_eal_version.map | 1 +
4 files changed, 26 insertions(+)
Comments
On Wed, 27 Nov 2019 15:52:19 +0530
Sunil Kumar Kori <skori@marvell.com> wrote:
>
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice
> + *
> + * Check if currently executing in interrupt context
> + *
> + * @return
> + * - positive in case of interrupt context
> + * - zero in case of process context
> + * - negative if unsuccessful
> + */
> +__rte_experimental
> +int
> +rte_thread_is_intr(void);
If you only need this in drivers, it should be internal not exposed
as part of API
On Wed, Nov 27, 2019 at 04:03:19PM -0800, Stephen Hemminger wrote:
> External Email
>
> ----------------------------------------------------------------------
> On Wed, 27 Nov 2019 15:52:19 +0530
> Sunil Kumar Kori <skori@marvell.com> wrote:
>
> >
> > +/**
> > + * @warning
> > + * @b EXPERIMENTAL: this API may change without prior notice
> > + *
> > + * Check if currently executing in interrupt context
> > + *
> > + * @return
> > + * - positive in case of interrupt context
> > + * - zero in case of process context
> > + * - negative if unsuccessful
> > + */
> > +__rte_experimental
> > +int
> > +rte_thread_is_intr(void);
>
> If you only need this in drivers, it should be internal not exposed
> as part of API
Sorry, but can you please help me understand the query. Do you mean:
* Since "rte_thread_is_intr" would be used only used by
libraries/drivers and not by any external application, rather having
it in "rte_interrupt.h", we should have it in some internal header
file like "eal_private.h" ??
ANS - Yes we can do that but since all other related APIs like
"rte_intr_ack", "rte_intr_enable/disable" which are also used by
drivers/lib and not application, are prototyped in "rte_interupt.h".
OR do u mean
* Since only octeontx2 driver is using this, why it is exposed as an API
rather it should be defined as some driver internal function ??
ANS - "rte_thread_is_intr" is an counter part of "in_interrupt()" in
DPDK, which will return whether the current execution is in interrupt
context. This helps in dealing with nested interrupts case. We faced a
similar case while handling hotplug probing initiated via secondary
process.
We believe this API could be very useful for many drivers which might
end up in handling nested interrupts case.
Thanks
Harman
On Thu, 28 Nov 2019 08:10:23 +0000
Harman Kalra <hkalra@marvell.com> wrote:
> On Wed, Nov 27, 2019 at 04:03:19PM -0800, Stephen Hemminger wrote:
> > External Email
> >
> > ----------------------------------------------------------------------
> > On Wed, 27 Nov 2019 15:52:19 +0530
> > Sunil Kumar Kori <skori@marvell.com> wrote:
> >
> > >
> > > +/**
> > > + * @warning
> > > + * @b EXPERIMENTAL: this API may change without prior notice
> > > + *
> > > + * Check if currently executing in interrupt context
> > > + *
> > > + * @return
> > > + * - positive in case of interrupt context
> > > + * - zero in case of process context
> > > + * - negative if unsuccessful
> > > + */
> > > +__rte_experimental
> > > +int
> > > +rte_thread_is_intr(void);
> >
> > If you only need this in drivers, it should be internal not exposed
> > as part of API
>
> Sorry, but can you please help me understand the query. Do you mean:
> * Since "rte_thread_is_intr" would be used only used by
> libraries/drivers and not by any external application, rather having
> it in "rte_interrupt.h", we should have it in some internal header
> file like "eal_private.h" ??
>
> ANS - Yes we can do that but since all other related APIs like
> "rte_intr_ack", "rte_intr_enable/disable" which are also used by
> drivers/lib and not application, are prototyped in "rte_interupt.h".
>
> OR do u mean
> * Since only octeontx2 driver is using this, why it is exposed as an API
> rather it should be defined as some driver internal function ??
>
> ANS - "rte_thread_is_intr" is an counter part of "in_interrupt()" in
> DPDK, which will return whether the current execution is in interrupt
> context. This helps in dealing with nested interrupts case. We faced a
> similar case while handling hotplug probing initiated via secondary
> process.
> We believe this API could be very useful for many drivers which might
> end up in handling nested interrupts case.
>
> Thanks
> Harman
What I meant was that some functions in EAL are documented as
being internal (flagged with @internal) and don't show up in
the documented API.
My concern is that if we make this part of the public API, it makes
it harder to change structural things like the interrupt thread
later. The interrupt thread is already problematic for several
other usages.
On Thu, Nov 28, 2019 at 08:45:42AM -0800, Stephen Hemminger wrote:
> On Thu, 28 Nov 2019 08:10:23 +0000
> Harman Kalra <hkalra@marvell.com> wrote:
>
> > On Wed, Nov 27, 2019 at 04:03:19PM -0800, Stephen Hemminger wrote:
> > > External Email
> > >
> > > ----------------------------------------------------------------------
> > > On Wed, 27 Nov 2019 15:52:19 +0530
> > > Sunil Kumar Kori <skori@marvell.com> wrote:
> > >
> > > >
> > > > +/**
> > > > + * @warning
> > > > + * @b EXPERIMENTAL: this API may change without prior notice
> > > > + *
> > > > + * Check if currently executing in interrupt context
> > > > + *
> > > > + * @return
> > > > + * - positive in case of interrupt context
> > > > + * - zero in case of process context
> > > > + * - negative if unsuccessful
> > > > + */
> > > > +__rte_experimental
> > > > +int
> > > > +rte_thread_is_intr(void);
> > >
> > > If you only need this in drivers, it should be internal not exposed
> > > as part of API
> >
> > Sorry, but can you please help me understand the query. Do you mean:
> > * Since "rte_thread_is_intr" would be used only used by
> > libraries/drivers and not by any external application, rather having
> > it in "rte_interrupt.h", we should have it in some internal header
> > file like "eal_private.h" ??
> >
> > ANS - Yes we can do that but since all other related APIs like
> > "rte_intr_ack", "rte_intr_enable/disable" which are also used by
> > drivers/lib and not application, are prototyped in "rte_interupt.h".
> >
> > OR do u mean
> > * Since only octeontx2 driver is using this, why it is exposed as an API
> > rather it should be defined as some driver internal function ??
> >
> > ANS - "rte_thread_is_intr" is an counter part of "in_interrupt()" in
> > DPDK, which will return whether the current execution is in interrupt
> > context. This helps in dealing with nested interrupts case. We faced a
> > similar case while handling hotplug probing initiated via secondary
> > process.
> > We believe this API could be very useful for many drivers which might
> > end up in handling nested interrupts case.
> >
> > Thanks
> > Harman
>
> What I meant was that some functions in EAL are documented as
> being internal (flagged with @internal) and don't show up in
> the documented API.
>
> My concern is that if we make this part of the public API, it makes
> it harder to change structural things like the interrupt thread
> later. The interrupt thread is already problematic for several
> other usages.
>
I think now I understood your point, I will move prototype of this API to
"common/include/rte_eal_interrupts.h", flag it as @internal
and resend next version. I think this file was created for such
APIs only as its description says.
/**
* @file rte_eal_interrupts.h
* @internal
*
* Contains function prototypes exposed by the EAL for interrupt handling by
* drivers and other DPDK internal consumers.
*/
Thanks
Harman
@@ -138,6 +138,21 @@ int rte_intr_disable(const struct rte_intr_handle *intr_handle);
__rte_experimental
int rte_intr_ack(const struct rte_intr_handle *intr_handle);
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Check if currently executing in interrupt context
+ *
+ * @return
+ * - positive in case of interrupt context
+ * - zero in case of process context
+ * - negative if unsuccessful
+ */
+__rte_experimental
+int
+rte_thread_is_intr(void);
+
#ifdef __cplusplus
}
#endif
@@ -671,3 +671,8 @@ rte_intr_free_epoll_fd(struct rte_intr_handle *intr_handle)
{
RTE_SET_USED(intr_handle);
}
+
+int rte_thread_is_intr(void)
+{
+ return pthread_equal(intr_thread, pthread_self());
+}
@@ -1487,3 +1487,8 @@ rte_intr_cap_multiple(struct rte_intr_handle *intr_handle)
return 0;
}
+
+int rte_thread_is_intr(void)
+{
+ return pthread_equal(intr_thread, pthread_self());
+}
@@ -332,4 +332,5 @@ EXPERIMENTAL {
# added in 19.11
rte_log_get_stream;
rte_mcfg_get_single_file_segments;
+ rte_thread_is_intr;
};