All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Monjalon <thomas@monjalon.net>
To: Qi Zhang <qi.z.zhang@intel.com>
Cc: dev@dpdk.org, declan.doherty@intel.com, ferruh.yigit@intel.com,
	ktraynor@redhat.com, benjamin.h.shelton@intel.com,
	narender.vangati@intel.com
Subject: Re: [PATCH v2] doc: update API deprecation for device reset
Date: Mon, 28 Jan 2019 01:20:24 +0100	[thread overview]
Message-ID: <1648146.NeMYav50z5@xps> (raw)
In-Reply-To: <20180920045959.106252-1-qi.z.zhang@intel.com>

Hi,

I don't remember why this deprecation notice did not get any feedback
in the 18.11 cycle. It looks to be outdated now.
In case you still plan some changes, I give my opinion below.

20/09/2018 06:59, Qi Zhang:
> Device reset may have the dependency, for example, a VF reset expects
> PF ready, or a NIC function as a part of a SOC need to wait for other
> parts of the system be ready, these are time-consuming tasks and will
> block current thread.
> 
> So we rename rte_eth_dev_reset to rte_eth_dev_reset_async as an async
> API, that makes things easy for an application that what to reset the
> device from the interrupt thread since typically a
> RTE_ETH_EVENT_INTR_RESET handler is invoked in interrupt thread.
> 
> RFC patch:
> http://patchwork.dpdk.org/patch/44989/
> 
> Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>

I remmember I did not like this function when it was introduced.
I think the cases addressed by rte_eth_dev_reset() should be handled
with hotplug mechanism.

> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> +* ethdev: In v19.02 ``rte_eth_dev_reset`` is renamed to

Is it still planned for 19.05?

> +  ``rte_eth_dev_reset_async``. As the name, it is an async API.

Why removing the old function?
We could avoid breaking applications by adding an async flavor
of the function.

> +  Application should not assume device reset is finished after
> +  ``rte_eth_dev_reset_async`` return, it should always wait for a
> +  RTE_ETH_EVENT_RESET_COMPLETE event and check the reset result.

This could be done in the synchronous flavor with sleeps in a loop.

      reply	other threads:[~2019-01-28  0:20 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-20  4:59 [PATCH v2] doc: update API deprecation for device reset Qi Zhang
2019-01-28  0:20 ` Thomas Monjalon [this message]

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=1648146.NeMYav50z5@xps \
    --to=thomas@monjalon.net \
    --cc=benjamin.h.shelton@intel.com \
    --cc=declan.doherty@intel.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=ktraynor@redhat.com \
    --cc=narender.vangati@intel.com \
    --cc=qi.z.zhang@intel.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.