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

[Chautru, Nicolas]

Hi Akhil, 
Can this serie below be applied now?
Thanks and regards,
Nic

> -----Original Message-----
> From: Tom Rix <trix at redhat.com>
> Sent: Monday, September 13, 2021 1:02 PM
> To: Chautru, Nicolas <nicolas.chautru at intel.com>; dev at dpdk.org;
> gakhil at marvell.com
> Cc: thomas at monjalon.net; hemant.agrawal at nxp.com; Zhang, Mingshan
> <mingshan.zhang at intel.com>; Joshi, Arun <arun.joshi at intel.com>
> Subject: Re: [PATCH v3 6/6] bbdev: reduce warning level for one scenario
> 
> 
> On 9/13/21 10:03 AM, Chautru, Nicolas wrote:
> >> -----Original Message-----
> >> From: Tom Rix <trix at redhat.com>
> >> Sent: Sunday, September 12, 2021 5:55 AM
> >> To: Chautru, Nicolas <nicolas.chautru at intel.com>; dev at dpdk.org;
> >> gakhil at marvell.com
> >> Cc: thomas at monjalon.net; hemant.agrawal at nxp.com; Zhang, Mingshan
> >> <mingshan.zhang at intel.com>; Joshi, Arun <arun.joshi at 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 at 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 at redhat.com>
> 
> Tom
> 
> >
> > Thanks
> > Nic
> >
> >> Tom
> >>
> >>>    		return ret;
> >>>    	}
> >>>