Re: [dpdk-dev] [PATCH v3 6/6] bbdev: reduce warning level for one scenario

[Tom Rix <trix@redhat.com>]

On 9/13/21 10:03 AM, Chautru, Nicolas wrote:
>> -----Original Message-----
>> From: Tom Rix <trix@redhat.com>
>> Sent: Sunday, September 12, 2021 5:55 AM
>> To: Chautru, Nicolas <nicolas.chautru@intel.com>; dev@dpdk.org;
>> gakhil@marvell.com
>> Cc: thomas@monjalon.net; hemant.agrawal@nxp.com; Zhang, Mingshan
>> <mingshan.zhang@intel.com>; Joshi, Arun <arun.joshi@intel.com>
>> Subject: Re: [PATCH v3 6/6] bbdev: reduce warning level for one scenario
>>
>>
>> On 9/7/21 6:15 PM, Nicolas Chautru wrote:
>>> Queue setup may genuinely fail when adding incremental queues for a
>>> given priority level. In that case application would attempt to
>>> configure a queue at a different priority level.
>>> Not an actual error.
>>>
>>> Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
>>> ---
>>>    lib/bbdev/rte_bbdev.c | 7 ++++---
>>>    1 file changed, 4 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/lib/bbdev/rte_bbdev.c b/lib/bbdev/rte_bbdev.c index
>>> fc37236..defddcf 100644
>>> --- a/lib/bbdev/rte_bbdev.c
>>> +++ b/lib/bbdev/rte_bbdev.c
>>> @@ -528,9 +528,10 @@ struct rte_bbdev *
>>>    	ret = dev->dev_ops->queue_setup(dev, queue_id, (conf != NULL) ?
>>>    			conf : &dev_info.default_queue_conf);
>>>    	if (ret < 0) {
>>> -		rte_bbdev_log(ERR,
>>> -				"Device %u queue %u setup failed", dev_id,
>>> -				queue_id);
>>> +		/* This may happen when trying different priority levels */
>>> +		rte_bbdev_log(INFO,
>>> +				"Device %u queue %u setup failed",
>>> +				dev_id, queue_id);
>> This change is just changing the log level, which is fine.
>>
>> I am looking at how the error handling is done for the function.
>>
>> It seems like the bailing is done in the middle of change the queue state.
>>
>> ex/ the block above this one
>>
>> /* Release existing queue ... */
>>
>> Does this leave the queue in a bad state ?
> Hi Tom,
> That would not be related to that change indeed.
>
> The queue would end up in a not configured when rte_bbdev_queue_configure() fails but then can still  be configured again without limitation (worst thing than can happen is that queue_release is called, hence leaves the queue in a deterministic state, unconfigured but ready to be configured).
> Note that queue_release() just removes the configuration of the queue, but the queue is still there and can be configured again (actual total number of queues unchanged, based on number previously set with rte_bbdev_setup_queues()).

So its in a bad state, but outside the scope of this commit.

Reviewed-by: Tom Rix <trix@redhat.com>

Tom

>
> Thanks
> Nic
>
>> Tom
>>
>>>    		return ret;
>>>    	}
>>>