Hi Stephen:

-----Original Message-----
From: Stephen Hemminger [mailto:stephen@networkplumber.org]
Sent: Saturday, June 16, 2018 12:09 AM
To: Zhang, Qi Z <redacted>
Cc: thomas@monjalon.net; Burakov, Anatoly <redacted>;
Ananyev, Konstantin [off-list ref]; dev@dpdk.org;
Richardson, Bruce [off-list ref]; Yigit, Ferruh
[off-list ref]; Shelton, Benjamin H
[off-list ref]; Vangati, Narender
[off-list ref]
Subject: Re: [dpdk-dev] [PATCH 05/22] ethdev: introduce device lock

On Thu,  7 Jun 2018 20:38:32 +0800
Qi Zhang [off-list ref] wrote:

quoted

+/**
+ * Lock an Ethernet Device directly or register a callback function
+ * for condition check at runtime, this help application to prevent
+ * a device be detached unexpectly.
+ * NOTE: Lock a device mutliple times with same parmeter will increase
+ * a ref_count, and coresponding unlock decrease the ref_count, the
+ * device will be unlocked when ref_count reach 0.
+ *
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ * @param callback
+ *   !NULL the callback function will be added into a pre-detach list,
+ *         it will be invoked when a device is going to be detached. The
+ *         return value will decide if continue detach the device or not.
+ *   NULL  lock the device directly, basically this just regiter a empty
+ *         callback function(dev_is_busy) that return -EBUSY, so we can
+ *         handle the pre-detach check in unified way.
+ * @param user_args
+ *   parameter will be parsed to callback function, only valid when
+ *   callback != NULL.
+ * @return
+ *   0 on success, negative on error.
+ */
+int rte_eth_dev_lock(uint16_t port_id, rte_eth_dev_lock_callback_t

callback,

quoted

+		     void *user_args);

I prefer API's that do one thing with one function.

Agree

Why not
	rte_eth_dev_lock(uint16_t port_id);
	rte_eth_dev_ondetach(uint16_t port_id, rte_eth_dev_lock_callback_t
callback,
           		     void *user_args);

Rte_eth_dev_ondetach looks like a callback function, 
but this is the function to register some condition check.
How about rte_eth_dev_lock and rte_eth_dev_lock_with_cond?

Thanks
Qi