[S23,v2,14/15] ice: Change type for queue counts
diff mbox series

Message ID 20190628150348.59202-1-anthony.l.nguyen@intel.com
State Superseded
Delegated to: Jeff Kirsher
Headers show
Series
  • Untitled series #116826
Related show

Commit Message

Tony Nguyen June 28, 2019, 3:03 p.m. UTC
From: Pawel Kaminski <pawel.kaminski@intel.com>

A negative queue count should never exist, change the type to unsigned
to convey that.

Signed-off-by: Pawel Kaminski <pawel.kaminski@intel.com>
---
v2: Change type from u16 to unsigned int
    Reword commit message

 drivers/net/ethernet/intel/ice/ice_virtchnl_pf.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

Comments

Bowers, AndrewX July 3, 2019, 6:20 p.m. UTC | #1
> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@osuosl.org] On
> Behalf Of Tony Nguyen
> Sent: Friday, June 28, 2019 8:04 AM
> To: intel-wired-lan@lists.osuosl.org
> Cc: pmenzel@molgen.mpg.de
> Subject: [Intel-wired-lan] [PATCH S23 v2 14/15] ice: Change type for queue
> counts
> 
> From: Pawel Kaminski <pawel.kaminski@intel.com>
> 
> A negative queue count should never exist, change the type to unsigned to
> convey that.
> 
> Signed-off-by: Pawel Kaminski <pawel.kaminski@intel.com>
> ---
> v2: Change type from u16 to unsigned int
>     Reword commit message
> 
>  drivers/net/ethernet/intel/ice/ice_virtchnl_pf.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Paul Menzel July 4, 2019, 11:28 a.m. UTC | #2
Dear Tony, dear Pawel,


On 06/28/19 17:03, Tony Nguyen wrote:
> From: Pawel Kaminski <pawel.kaminski@intel.com>
> 
> A negative queue count should never exist, change the type to unsigned
> to convey that.
> 
> Signed-off-by: Pawel Kaminski <pawel.kaminski@intel.com>
> ---
> v2: Change type from u16 to unsigned int
>     Reword commit message

Thank you.

Also suggestion for a more specific commit message summary.

> ice: Use unsigned types for queue counts

>  drivers/net/ethernet/intel/ice/ice_virtchnl_pf.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_virtchnl_pf.c b/drivers/net/ethernet/intel/ice/ice_virtchnl_pf.c
> index f2ebbe83ae4c..b8fc08c1d365 100644
> --- a/drivers/net/ethernet/intel/ice/ice_virtchnl_pf.c
> +++ b/drivers/net/ethernet/intel/ice/ice_virtchnl_pf.c
> @@ -2339,11 +2339,11 @@ static int ice_vc_request_qs_msg(struct ice_vf *vf, u8 *msg)
>  	enum virtchnl_status_code v_ret = VIRTCHNL_STATUS_SUCCESS;
>  	struct virtchnl_vf_res_request *vfres =
>  		(struct virtchnl_vf_res_request *)msg;
> -	int req_queues = vfres->num_queue_pairs;
> +	unsigned int req_queues = vfres->num_queue_pairs;

Looking at the struct definition again, there actually u16 is used.

    /* VF resource request */
    struct virtchnl_vf_res_request {
            u16 num_queue_pairs;
    };

So only for `req_queues` it might be indeed good to match the type.
Sorry for not checking that before, and I do not know if it is
important, as I am not a C expert.

> +	unsigned int max_allowed_vf_queues;
> +	unsigned int tx_rx_queue_left;
>  	struct ice_pf *pf = vf->pf;
> -	int max_allowed_vf_queues;
> -	int tx_rx_queue_left;
> -	int cur_queues;
> +	unsigned int cur_queues;
>  
>  	if (!test_bit(ICE_VF_STATE_ACTIVE, vf->vf_states)) {
>  		v_ret = VIRTCHNL_STATUS_ERR_PARAM;
> @@ -2353,10 +2353,10 @@ static int ice_vc_request_qs_msg(struct ice_vf *vf, u8 *msg)
>  	cur_queues = vf->num_vf_qs;
>  	tx_rx_queue_left = min_t(int, pf->q_left_tx, pf->q_left_rx);
>  	max_allowed_vf_queues = tx_rx_queue_left + cur_queues;
> -	if (req_queues <= 0) {
> +	if (!req_queues) {
>  		dev_err(&pf->pdev->dev,
> -			"VF %d tried to request %d queues. Ignoring.\n",
> -			vf->vf_id, req_queues);
> +			"VF %d tried to request 0 queues. Ignoring.\n",
> +			vf->vf_id);
>  	} else if (req_queues > ICE_MAX_BASE_QS_PER_VF) {
>  		dev_err(&pf->pdev->dev,
>  			"VF %d tried to request more than %d queues.\n",

There are more messages where the value of `req_queues` is printed.
Do the format specifiers need to be changed from %d to %u?


Kind regards,

Paul

>

Patch
diff mbox series

diff --git a/drivers/net/ethernet/intel/ice/ice_virtchnl_pf.c b/drivers/net/ethernet/intel/ice/ice_virtchnl_pf.c
index f2ebbe83ae4c..b8fc08c1d365 100644
--- a/drivers/net/ethernet/intel/ice/ice_virtchnl_pf.c
+++ b/drivers/net/ethernet/intel/ice/ice_virtchnl_pf.c
@@ -2339,11 +2339,11 @@  static int ice_vc_request_qs_msg(struct ice_vf *vf, u8 *msg)
 	enum virtchnl_status_code v_ret = VIRTCHNL_STATUS_SUCCESS;
 	struct virtchnl_vf_res_request *vfres =
 		(struct virtchnl_vf_res_request *)msg;
-	int req_queues = vfres->num_queue_pairs;
+	unsigned int req_queues = vfres->num_queue_pairs;
+	unsigned int max_allowed_vf_queues;
+	unsigned int tx_rx_queue_left;
 	struct ice_pf *pf = vf->pf;
-	int max_allowed_vf_queues;
-	int tx_rx_queue_left;
-	int cur_queues;
+	unsigned int cur_queues;
 
 	if (!test_bit(ICE_VF_STATE_ACTIVE, vf->vf_states)) {
 		v_ret = VIRTCHNL_STATUS_ERR_PARAM;
@@ -2353,10 +2353,10 @@  static int ice_vc_request_qs_msg(struct ice_vf *vf, u8 *msg)
 	cur_queues = vf->num_vf_qs;
 	tx_rx_queue_left = min_t(int, pf->q_left_tx, pf->q_left_rx);
 	max_allowed_vf_queues = tx_rx_queue_left + cur_queues;
-	if (req_queues <= 0) {
+	if (!req_queues) {
 		dev_err(&pf->pdev->dev,
-			"VF %d tried to request %d queues. Ignoring.\n",
-			vf->vf_id, req_queues);
+			"VF %d tried to request 0 queues. Ignoring.\n",
+			vf->vf_id);
 	} else if (req_queues > ICE_MAX_BASE_QS_PER_VF) {
 		dev_err(&pf->pdev->dev,
 			"VF %d tried to request more than %d queues.\n",