Re: [PATCH 09/26] IB/ocrdma: Improve another size determination in ocrdma_mq_cq_handler()

[Yuval Shaia]

On Wed, Mar 08, 2017 at 02:00:56PM +0100, SF Markus Elfring wrote:
> From: Markus Elfring 
> Date: Tue, 7 Mar 2017 20:20:45 +0100
> 
> Replace the specification of a data structure by a pointer dereference
> as the parameter for the operator "sizeof" to make the corresponding size
> determination a bit safer according to the Linux coding style convention.
> 
> Signed-off-by: Markus Elfring 
> ---
>  drivers/infiniband/hw/ocrdma/ocrdma_hw.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/infiniband/hw/ocrdma/ocrdma_hw.c 
> b/drivers/infiniband/hw/ocrdma/ocrdma_hw.c
> index 0628600bee56..7d1e1caa90de 100644
> --- a/drivers/infiniband/hw/ocrdma/ocrdma_hw.c
> +++ b/drivers/infiniband/hw/ocrdma/ocrdma_hw.c
> @@ -890,7 +890,7 @@ static int ocrdma_mq_cq_handler(struct ocrdma_dev *dev, 
> u16 cq_id)
>   ocrdma_process_acqe(dev, cqe);
>   else if (cqe->valid_ae_cmpl_cons & OCRDMA_MCQE_CMPL_MASK)
>   ocrdma_process_mcqe(dev, cqe);
> - memset(cqe, 0, sizeof(struct ocrdma_mcqe));
> + memset(cqe, 0, sizeof(*cqe));
>   ocrdma_mcq_inc_tail(dev);
>   }

Reviewed-by: Yuval Shaia 

>   ocrdma_ring_cq_db(dev, dev->mq.cq.id, true, false, cqe_popped);
> -- 
> 2.12.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 07/26] IB/ocrdma: Improve another size determination in ocrdma_mbx_get_ctrl_attribs()

[Yuval Shaia]

On Wed, Mar 08, 2017 at 01:55:59PM +0100, SF Markus Elfring wrote:
> From: Markus Elfring <elfr...@users.sourceforge.net>
> Date: Tue, 7 Mar 2017 20:07:25 +0100
> 
> Replace the specification of a data structure by a pointer dereference
> as the parameter for the operator "sizeof" to make the corresponding size
> determination a bit safer according to the Linux coding style convention.
> 
> Signed-off-by: Markus Elfring <elfr...@users.sourceforge.net>
> ---
>  drivers/infiniband/hw/ocrdma/ocrdma_hw.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/infiniband/hw/ocrdma/ocrdma_hw.c 
> b/drivers/infiniband/hw/ocrdma/ocrdma_hw.c
> index 56f29e06dfe4..1f7d88d7569e 100644
> --- a/drivers/infiniband/hw/ocrdma/ocrdma_hw.c
> +++ b/drivers/infiniband/hw/ocrdma/ocrdma_hw.c
> @@ -1331,7 +1331,7 @@ static int ocrdma_mbx_get_ctrl_attribs(struct 
> ocrdma_dev *dev)
>   struct ocrdma_get_ctrl_attribs_rsp *ctrl_attr_rsp;
>   struct mgmt_hba_attribs *hba_attribs;
>  
> - mqe = kzalloc(sizeof(struct ocrdma_mqe), GFP_KERNEL);
> + mqe = kzalloc(sizeof(*mqe), GFP_KERNEL);
>   if (!mqe)
>   return status;
>  

Reviewed-by: Yuval Shaia <yuval.sh...@oracle.com>

> -- 
> 2.12.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 07/26] IB/ocrdma: Improve another size determination in ocrdma_mbx_get_ctrl_attribs()

[Yuval Shaia]

On Wed, Mar 08, 2017 at 01:55:59PM +0100, SF Markus Elfring wrote:
> From: Markus Elfring 
> Date: Tue, 7 Mar 2017 20:07:25 +0100
> 
> Replace the specification of a data structure by a pointer dereference
> as the parameter for the operator "sizeof" to make the corresponding size
> determination a bit safer according to the Linux coding style convention.
> 
> Signed-off-by: Markus Elfring 
> ---
>  drivers/infiniband/hw/ocrdma/ocrdma_hw.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/infiniband/hw/ocrdma/ocrdma_hw.c 
> b/drivers/infiniband/hw/ocrdma/ocrdma_hw.c
> index 56f29e06dfe4..1f7d88d7569e 100644
> --- a/drivers/infiniband/hw/ocrdma/ocrdma_hw.c
> +++ b/drivers/infiniband/hw/ocrdma/ocrdma_hw.c
> @@ -1331,7 +1331,7 @@ static int ocrdma_mbx_get_ctrl_attribs(struct 
> ocrdma_dev *dev)
>   struct ocrdma_get_ctrl_attribs_rsp *ctrl_attr_rsp;
>   struct mgmt_hba_attribs *hba_attribs;
>  
> - mqe = kzalloc(sizeof(struct ocrdma_mqe), GFP_KERNEL);
> + mqe = kzalloc(sizeof(*mqe), GFP_KERNEL);
>   if (!mqe)
>   return status;
>  

Reviewed-by: Yuval Shaia 

> -- 
> 2.12.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 05/26] IB/ocrdma: Improve another size determination in ocrdma_mbx_query_qp()

[Yuval Shaia]

On Wed, Mar 08, 2017 at 01:50:59PM +0100, SF Markus Elfring wrote:
> From: Markus Elfring <elfr...@users.sourceforge.net>
> Date: Tue, 7 Mar 2017 19:40:31 +0100
> 
> Replace the specification of a data structure by a pointer dereference
> as the parameter for the operator "sizeof" to make the corresponding size
> determination a bit safer according to the Linux coding style convention.
> 
> Signed-off-by: Markus Elfring <elfr...@users.sourceforge.net>
> ---
>  drivers/infiniband/hw/ocrdma/ocrdma_hw.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/infiniband/hw/ocrdma/ocrdma_hw.c 
> b/drivers/infiniband/hw/ocrdma/ocrdma_hw.c
> index 517a18e9351a..8122a1c7d2ae 100644
> --- a/drivers/infiniband/hw/ocrdma/ocrdma_hw.c
> +++ b/drivers/infiniband/hw/ocrdma/ocrdma_hw.c
> @@ -2487,7 +2487,7 @@ int ocrdma_mbx_query_qp(struct ocrdma_dev *dev, struct 
> ocrdma_qp *qp,
>   if (status)
>   goto mbx_err;
>   rsp = (struct ocrdma_query_qp_rsp *)cmd;
> - memcpy(param, >params, sizeof(struct ocrdma_qp_params));
> + memcpy(param, >params, sizeof(*param));
>  mbx_err:
>   kfree(cmd);
>   return status;
> -- 

Reviewed-by: Yuval Shaia <yuval.sh...@oracle.com>

> 2.12.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 05/26] IB/ocrdma: Improve another size determination in ocrdma_mbx_query_qp()

[Yuval Shaia]

On Wed, Mar 08, 2017 at 01:50:59PM +0100, SF Markus Elfring wrote:
> From: Markus Elfring 
> Date: Tue, 7 Mar 2017 19:40:31 +0100
> 
> Replace the specification of a data structure by a pointer dereference
> as the parameter for the operator "sizeof" to make the corresponding size
> determination a bit safer according to the Linux coding style convention.
> 
> Signed-off-by: Markus Elfring 
> ---
>  drivers/infiniband/hw/ocrdma/ocrdma_hw.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/infiniband/hw/ocrdma/ocrdma_hw.c 
> b/drivers/infiniband/hw/ocrdma/ocrdma_hw.c
> index 517a18e9351a..8122a1c7d2ae 100644
> --- a/drivers/infiniband/hw/ocrdma/ocrdma_hw.c
> +++ b/drivers/infiniband/hw/ocrdma/ocrdma_hw.c
> @@ -2487,7 +2487,7 @@ int ocrdma_mbx_query_qp(struct ocrdma_dev *dev, struct 
> ocrdma_qp *qp,
>   if (status)
>   goto mbx_err;
>   rsp = (struct ocrdma_query_qp_rsp *)cmd;
> - memcpy(param, >params, sizeof(struct ocrdma_qp_params));
> + memcpy(param, >params, sizeof(*param));
>  mbx_err:
>   kfree(cmd);
>   return status;
> -- 

Reviewed-by: Yuval Shaia 

> 2.12.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 04/26] IB/ocrdma: Delete unnecessary variable initialisations in ocrdma_mbx_get_dcbx_config()

[Yuval Shaia]

On Wed, Mar 08, 2017 at 01:48:46PM +0100, SF Markus Elfring wrote:
> From: Markus Elfring <elfr...@users.sourceforge.net>
> Date: Tue, 7 Mar 2017 19:25:55 +0100
> 
> The local variables "req" and "rsp" will be set to appropriate pointers
> a bit later. Thus omit the explicit initialisations at the beginning.
> 
> Signed-off-by: Markus Elfring <elfr...@users.sourceforge.net>
> ---
>  drivers/infiniband/hw/ocrdma/ocrdma_hw.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/infiniband/hw/ocrdma/ocrdma_hw.c 
> b/drivers/infiniband/hw/ocrdma/ocrdma_hw.c
> index 85bebb887c5c..517a18e9351a 100644
> --- a/drivers/infiniband/hw/ocrdma/ocrdma_hw.c
> +++ b/drivers/infiniband/hw/ocrdma/ocrdma_hw.c
> @@ -2895,9 +2895,8 @@ static int ocrdma_mbx_get_dcbx_config(struct ocrdma_dev 
> *dev, u32 ptype,
>   int status;
>   dma_addr_t pa;
>   struct ocrdma_mqe cmd;
> -
> - struct ocrdma_get_dcbx_cfg_req *req = NULL;
> - struct ocrdma_get_dcbx_cfg_rsp *rsp = NULL;
> + struct ocrdma_get_dcbx_cfg_req *req;
> + struct ocrdma_get_dcbx_cfg_rsp *rsp;

Reviewed-by: Yuval Shaia <yuval.sh...@oracle.com>

>   struct pci_dev *pdev = dev->nic_info.pdev;
>   struct ocrdma_mqe_sge *mqe_sge = cmd.u.nonemb_req.sge;
>  
> -- 
> 2.12.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 04/26] IB/ocrdma: Delete unnecessary variable initialisations in ocrdma_mbx_get_dcbx_config()

[Yuval Shaia]

On Wed, Mar 08, 2017 at 01:48:46PM +0100, SF Markus Elfring wrote:
> From: Markus Elfring 
> Date: Tue, 7 Mar 2017 19:25:55 +0100
> 
> The local variables "req" and "rsp" will be set to appropriate pointers
> a bit later. Thus omit the explicit initialisations at the beginning.
> 
> Signed-off-by: Markus Elfring 
> ---
>  drivers/infiniband/hw/ocrdma/ocrdma_hw.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/infiniband/hw/ocrdma/ocrdma_hw.c 
> b/drivers/infiniband/hw/ocrdma/ocrdma_hw.c
> index 85bebb887c5c..517a18e9351a 100644
> --- a/drivers/infiniband/hw/ocrdma/ocrdma_hw.c
> +++ b/drivers/infiniband/hw/ocrdma/ocrdma_hw.c
> @@ -2895,9 +2895,8 @@ static int ocrdma_mbx_get_dcbx_config(struct ocrdma_dev 
> *dev, u32 ptype,
>   int status;
>   dma_addr_t pa;
>   struct ocrdma_mqe cmd;
> -
> - struct ocrdma_get_dcbx_cfg_req *req = NULL;
> - struct ocrdma_get_dcbx_cfg_rsp *rsp = NULL;
> + struct ocrdma_get_dcbx_cfg_req *req;
> + struct ocrdma_get_dcbx_cfg_rsp *rsp;

Reviewed-by: Yuval Shaia 

>   struct pci_dev *pdev = dev->nic_info.pdev;
>   struct ocrdma_mqe_sge *mqe_sge = cmd.u.nonemb_req.sge;
>  
> -- 
> 2.12.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 02/26] IB/ocrdma: Use kcalloc() in ocrdma_mbx_alloc_pd_range()

[Yuval Shaia]

On Wed, Mar 08, 2017 at 01:41:00PM +0100, SF Markus Elfring wrote:
> From: Markus Elfring 
> Date: Tue, 7 Mar 2017 18:23:54 +0100
> 
> * Multiplications for the size determination of memory allocations
>   indicated that array data structures should be processed.
>   Thus reuse the corresponding function "kcalloc".
> 
>   This issue was detected by using the Coccinelle software.
> 
> * Delete the local variable "pd_bitmap_size" which became unnecessary with
>   this refactoring.
> 
> Signed-off-by: Markus Elfring 
> ---
>  drivers/infiniband/hw/ocrdma/ocrdma_hw.c | 16 
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/infiniband/hw/ocrdma/ocrdma_hw.c 
> b/drivers/infiniband/hw/ocrdma/ocrdma_hw.c
> index cbac41b15d94..d5a3127b6df8 100644
> --- a/drivers/infiniband/hw/ocrdma/ocrdma_hw.c
> +++ b/drivers/infiniband/hw/ocrdma/ocrdma_hw.c
> @@ -1505,7 +1505,6 @@ int ocrdma_mbx_dealloc_pd(struct ocrdma_dev *dev, 
> struct ocrdma_pd *pd)
>  static int ocrdma_mbx_alloc_pd_range(struct ocrdma_dev *dev)
>  {
>   int status = -ENOMEM;
> - size_t pd_bitmap_size;
>   struct ocrdma_alloc_pd_range *cmd;
>   struct ocrdma_alloc_pd_range_rsp *rsp;
>  
> @@ -1527,10 +1526,10 @@ static int ocrdma_mbx_alloc_pd_range(struct 
> ocrdma_dev *dev)
>   dev->pd_mgr->pd_dpp_start = rsp->dpp_page_pdid &
>   OCRDMA_ALLOC_PD_RNG_RSP_START_PDID_MASK;
>   dev->pd_mgr->max_dpp_pd = rsp->pd_count;
> - pd_bitmap_size =
> - BITS_TO_LONGS(rsp->pd_count) * sizeof(long);
> - dev->pd_mgr->pd_dpp_bitmap = kzalloc(pd_bitmap_size,
> -  GFP_KERNEL);
> + dev->pd_mgr->pd_dpp_bitmap
> + = kcalloc(BITS_TO_LONGS(rsp->pd_count),
> + sizeof(long),
> + GFP_KERNEL);

kzalloc gives "clean" buffer.
Just making sure that it is fine with you if the array is not "clean".
(looking at _ocrdma_pd_mgr_get_bitmap i think it should).

>   }
>   kfree(cmd);
>   }
> @@ -1546,9 +1545,10 @@ static int ocrdma_mbx_alloc_pd_range(struct ocrdma_dev 
> *dev)
>   dev->pd_mgr->pd_norm_start = rsp->dpp_page_pdid &
>   OCRDMA_ALLOC_PD_RNG_RSP_START_PDID_MASK;
>   dev->pd_mgr->max_normal_pd = rsp->pd_count;
> - pd_bitmap_size = BITS_TO_LONGS(rsp->pd_count) * sizeof(long);
> - dev->pd_mgr->pd_norm_bitmap = kzalloc(pd_bitmap_size,
> -   GFP_KERNEL);
> + dev->pd_mgr->pd_norm_bitmap
> + = kcalloc(BITS_TO_LONGS(rsp->pd_count),
> + sizeof(long),
> + GFP_KERNEL);
>   }
>   kfree(cmd);
>  
> -- 
> 2.12.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 02/26] IB/ocrdma: Use kcalloc() in ocrdma_mbx_alloc_pd_range()

[Yuval Shaia]

On Wed, Mar 08, 2017 at 01:41:00PM +0100, SF Markus Elfring wrote:
> From: Markus Elfring 
> Date: Tue, 7 Mar 2017 18:23:54 +0100
> 
> * Multiplications for the size determination of memory allocations
>   indicated that array data structures should be processed.
>   Thus reuse the corresponding function "kcalloc".
> 
>   This issue was detected by using the Coccinelle software.
> 
> * Delete the local variable "pd_bitmap_size" which became unnecessary with
>   this refactoring.
> 
> Signed-off-by: Markus Elfring 
> ---
>  drivers/infiniband/hw/ocrdma/ocrdma_hw.c | 16 
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/infiniband/hw/ocrdma/ocrdma_hw.c 
> b/drivers/infiniband/hw/ocrdma/ocrdma_hw.c
> index cbac41b15d94..d5a3127b6df8 100644
> --- a/drivers/infiniband/hw/ocrdma/ocrdma_hw.c
> +++ b/drivers/infiniband/hw/ocrdma/ocrdma_hw.c
> @@ -1505,7 +1505,6 @@ int ocrdma_mbx_dealloc_pd(struct ocrdma_dev *dev, 
> struct ocrdma_pd *pd)
>  static int ocrdma_mbx_alloc_pd_range(struct ocrdma_dev *dev)
>  {
>   int status = -ENOMEM;
> - size_t pd_bitmap_size;
>   struct ocrdma_alloc_pd_range *cmd;
>   struct ocrdma_alloc_pd_range_rsp *rsp;
>  
> @@ -1527,10 +1526,10 @@ static int ocrdma_mbx_alloc_pd_range(struct 
> ocrdma_dev *dev)
>   dev->pd_mgr->pd_dpp_start = rsp->dpp_page_pdid &
>   OCRDMA_ALLOC_PD_RNG_RSP_START_PDID_MASK;
>   dev->pd_mgr->max_dpp_pd = rsp->pd_count;
> - pd_bitmap_size =
> - BITS_TO_LONGS(rsp->pd_count) * sizeof(long);
> - dev->pd_mgr->pd_dpp_bitmap = kzalloc(pd_bitmap_size,
> -  GFP_KERNEL);
> + dev->pd_mgr->pd_dpp_bitmap
> + = kcalloc(BITS_TO_LONGS(rsp->pd_count),
> + sizeof(long),
> + GFP_KERNEL);

kzalloc gives "clean" buffer.
Just making sure that it is fine with you if the array is not "clean".
(looking at _ocrdma_pd_mgr_get_bitmap i think it should).

>   }
>   kfree(cmd);
>   }
> @@ -1546,9 +1545,10 @@ static int ocrdma_mbx_alloc_pd_range(struct ocrdma_dev 
> *dev)
>   dev->pd_mgr->pd_norm_start = rsp->dpp_page_pdid &
>   OCRDMA_ALLOC_PD_RNG_RSP_START_PDID_MASK;
>   dev->pd_mgr->max_normal_pd = rsp->pd_count;
> - pd_bitmap_size = BITS_TO_LONGS(rsp->pd_count) * sizeof(long);
> - dev->pd_mgr->pd_norm_bitmap = kzalloc(pd_bitmap_size,
> -   GFP_KERNEL);
> + dev->pd_mgr->pd_norm_bitmap
> + = kcalloc(BITS_TO_LONGS(rsp->pd_count),
> + sizeof(long),
> + GFP_KERNEL);
>   }
>   kfree(cmd);
>  
> -- 
> 2.12.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 03/26] IB/ocrdma: Improve size determinations in ocrdma_mbx_get_dcbx_config()

[Yuval Shaia]

On Wed, Mar 08, 2017 at 01:45:43PM +0100, SF Markus Elfring wrote:
> From: Markus Elfring <elfr...@users.sourceforge.net>
> Date: Tue, 7 Mar 2017 19:09:20 +0100
> 
> Replace the specification of data structures by references for variables
> as the parameter for the operator "sizeof" to make the corresponding size
> determinations a bit safer according to the Linux coding style convention.
> 
> Signed-off-by: Markus Elfring <elfr...@users.sourceforge.net>
> ---
>  drivers/infiniband/hw/ocrdma/ocrdma_hw.c | 13 +
>  1 file changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/infiniband/hw/ocrdma/ocrdma_hw.c 
> b/drivers/infiniband/hw/ocrdma/ocrdma_hw.c
> index d5a3127b6df8..85bebb887c5c 100644
> --- a/drivers/infiniband/hw/ocrdma/ocrdma_hw.c
> +++ b/drivers/infiniband/hw/ocrdma/ocrdma_hw.c
> @@ -2901,9 +2901,8 @@ static int ocrdma_mbx_get_dcbx_config(struct ocrdma_dev 
> *dev, u32 ptype,
>   struct pci_dev *pdev = dev->nic_info.pdev;
>   struct ocrdma_mqe_sge *mqe_sge = cmd.u.nonemb_req.sge;
>  
> - memset(, 0, sizeof(struct ocrdma_mqe));
> - cmd.hdr.pyld_len = max_t (u32, sizeof(struct ocrdma_get_dcbx_cfg_rsp),
> - sizeof(struct ocrdma_get_dcbx_cfg_req));
> + memset(, 0, sizeof(cmd));
> + cmd.hdr.pyld_len = max_t(u32, sizeof(*rsp), sizeof(*req));
>   req = dma_alloc_coherent(>dev, cmd.hdr.pyld_len, , GFP_KERNEL);
>   if (!req) {
>   status = -ENOMEM;
> @@ -2915,8 +2914,7 @@ static int ocrdma_mbx_get_dcbx_config(struct ocrdma_dev 
> *dev, u32 ptype,
>   mqe_sge->pa_lo = (u32) (pa & 0xUL);
>   mqe_sge->pa_hi = (u32) upper_32_bits(pa);
>   mqe_sge->len = cmd.hdr.pyld_len;
> -
> - memset(req, 0, sizeof(struct ocrdma_get_dcbx_cfg_req));
> + memset(req, 0, sizeof(*req));
>   ocrdma_init_mch(>hdr, OCRDMA_CMD_GET_DCBX_CONFIG,
>   OCRDMA_SUBSYS_DCBX, cmd.hdr.pyld_len);
>   req->param_type = ptype;
> @@ -2926,9 +2924,8 @@ static int ocrdma_mbx_get_dcbx_config(struct ocrdma_dev 
> *dev, u32 ptype,
>   goto mbx_err;
>  
>   rsp = (struct ocrdma_get_dcbx_cfg_rsp *)req;
> - ocrdma_le32_to_cpu(rsp, sizeof(struct ocrdma_get_dcbx_cfg_rsp));
> - memcpy(dcbxcfg, >cfg, sizeof(struct ocrdma_dcbx_cfg));
> -
> +     ocrdma_le32_to_cpu(rsp, sizeof(*rsp));
> + memcpy(dcbxcfg, >cfg, sizeof(*dcbxcfg));
>  mbx_err:
>   dma_free_coherent(>dev, cmd.hdr.pyld_len, req, pa);
>  mem_err:

Reviewed-by: Yuval Shaia <yuval.sh...@oracle.com>

> -- 
> 2.12.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 03/26] IB/ocrdma: Improve size determinations in ocrdma_mbx_get_dcbx_config()

[Yuval Shaia]

On Wed, Mar 08, 2017 at 01:45:43PM +0100, SF Markus Elfring wrote:
> From: Markus Elfring 
> Date: Tue, 7 Mar 2017 19:09:20 +0100
> 
> Replace the specification of data structures by references for variables
> as the parameter for the operator "sizeof" to make the corresponding size
> determinations a bit safer according to the Linux coding style convention.
> 
> Signed-off-by: Markus Elfring 
> ---
>  drivers/infiniband/hw/ocrdma/ocrdma_hw.c | 13 +
>  1 file changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/infiniband/hw/ocrdma/ocrdma_hw.c 
> b/drivers/infiniband/hw/ocrdma/ocrdma_hw.c
> index d5a3127b6df8..85bebb887c5c 100644
> --- a/drivers/infiniband/hw/ocrdma/ocrdma_hw.c
> +++ b/drivers/infiniband/hw/ocrdma/ocrdma_hw.c
> @@ -2901,9 +2901,8 @@ static int ocrdma_mbx_get_dcbx_config(struct ocrdma_dev 
> *dev, u32 ptype,
>   struct pci_dev *pdev = dev->nic_info.pdev;
>   struct ocrdma_mqe_sge *mqe_sge = cmd.u.nonemb_req.sge;
>  
> - memset(, 0, sizeof(struct ocrdma_mqe));
> - cmd.hdr.pyld_len = max_t (u32, sizeof(struct ocrdma_get_dcbx_cfg_rsp),
> - sizeof(struct ocrdma_get_dcbx_cfg_req));
> + memset(, 0, sizeof(cmd));
> + cmd.hdr.pyld_len = max_t(u32, sizeof(*rsp), sizeof(*req));
>   req = dma_alloc_coherent(>dev, cmd.hdr.pyld_len, , GFP_KERNEL);
>   if (!req) {
>   status = -ENOMEM;
> @@ -2915,8 +2914,7 @@ static int ocrdma_mbx_get_dcbx_config(struct ocrdma_dev 
> *dev, u32 ptype,
>   mqe_sge->pa_lo = (u32) (pa & 0xUL);
>   mqe_sge->pa_hi = (u32) upper_32_bits(pa);
>   mqe_sge->len = cmd.hdr.pyld_len;
> -
> - memset(req, 0, sizeof(struct ocrdma_get_dcbx_cfg_req));
> + memset(req, 0, sizeof(*req));
>   ocrdma_init_mch(>hdr, OCRDMA_CMD_GET_DCBX_CONFIG,
>   OCRDMA_SUBSYS_DCBX, cmd.hdr.pyld_len);
>   req->param_type = ptype;
> @@ -2926,9 +2924,8 @@ static int ocrdma_mbx_get_dcbx_config(struct ocrdma_dev 
> *dev, u32 ptype,
>   goto mbx_err;
>  
>   rsp = (struct ocrdma_get_dcbx_cfg_rsp *)req;
> - ocrdma_le32_to_cpu(rsp, sizeof(struct ocrdma_get_dcbx_cfg_rsp));
> - memcpy(dcbxcfg, >cfg, sizeof(struct ocrdma_dcbx_cfg));
> -
> +     ocrdma_le32_to_cpu(rsp, sizeof(*rsp));
> + memcpy(dcbxcfg, >cfg, sizeof(*dcbxcfg));
>  mbx_err:
>   dma_free_coherent(>dev, cmd.hdr.pyld_len, req, pa);
>  mem_err:

Reviewed-by: Yuval Shaia 

> -- 
> 2.12.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 16/26] IB/IPoIB: Inline ib_dma_map_*() functions

[Yuval Shaia]

>   skb = small_skb;
>   goto copied;
> diff --git a/drivers/infiniband/ulp/ipoib/ipoib_ib.c 
> b/drivers/infiniband/ulp/ipoib/ipoib_ib.c
> index 5038f9d2d753..79204bd966bd 100644
> --- a/drivers/infiniband/ulp/ipoib/ipoib_ib.c
> +++ b/drivers/infiniband/ulp/ipoib/ipoib_ib.c
> @@ -92,9 +92,8 @@ void ipoib_free_ah(struct kref *kref)
>  static void ipoib_ud_dma_unmap_rx(struct ipoib_dev_priv *priv,
> u64 mapping[IPOIB_UD_RX_SG])
>  {
> - ib_dma_unmap_single(priv->ca, mapping[0],
> - IPOIB_UD_BUF_SIZE(priv->max_ib_mtu),
> - DMA_FROM_DEVICE);
> + dma_unmap_single(priv->ca->dma_device, mapping[0],
> +  IPOIB_UD_BUF_SIZE(priv->max_ib_mtu), DMA_FROM_DEVICE);
>  }
>  
>  static int ipoib_ib_post_receive(struct net_device *dev, int id)
> @@ -139,9 +138,9 @@ static struct sk_buff *ipoib_alloc_rx_skb(struct 
> net_device *dev, int id)
>   skb_reserve(skb, sizeof(struct ipoib_pseudo_header));
>  
>   mapping = priv->rx_ring[id].mapping;
> - mapping[0] = ib_dma_map_single(priv->ca, skb->data, buf_size,
> -DMA_FROM_DEVICE);
> - if (unlikely(ib_dma_mapping_error(priv->ca, mapping[0])))
> + mapping[0] = dma_map_single(priv->ca->dma_device, skb->data, buf_size,
> + DMA_FROM_DEVICE);
> + if (unlikely(dma_mapping_error(priv->ca->dma_device, mapping[0])))
>   goto error;
>  
>   priv->rx_ring[id].skb = skb;
> @@ -278,9 +277,9 @@ int ipoib_dma_map_tx(struct ib_device *ca, struct 
> ipoib_tx_buf *tx_req)
>   int off;
>  
>   if (skb_headlen(skb)) {
> - mapping[0] = ib_dma_map_single(ca, skb->data, skb_headlen(skb),
> -DMA_TO_DEVICE);
> - if (unlikely(ib_dma_mapping_error(ca, mapping[0])))
> + mapping[0] = dma_map_single(ca->dma_device, skb->data,
> + skb_headlen(skb), DMA_TO_DEVICE);
> + if (unlikely(dma_mapping_error(ca->dma_device, mapping[0])))
>   return -EIO;
>  
>   off = 1;
> @@ -289,11 +288,12 @@ int ipoib_dma_map_tx(struct ib_device *ca, struct 
> ipoib_tx_buf *tx_req)
>  
>   for (i = 0; i < skb_shinfo(skb)->nr_frags; ++i) {
>   const skb_frag_t *frag = _shinfo(skb)->frags[i];
> - mapping[i + off] = ib_dma_map_page(ca,
> -  skb_frag_page(frag),
> -  frag->page_offset, 
> skb_frag_size(frag),
> -  DMA_TO_DEVICE);
> - if (unlikely(ib_dma_mapping_error(ca, mapping[i + off])))
> + mapping[i + off] = dma_map_page(ca->dma_device,
> + skb_frag_page(frag),
> + frag->page_offset,
> + skb_frag_size(frag),
> + DMA_TO_DEVICE);
> + if (unlikely(dma_mapping_error(ca->dma_device, mapping[i + 
> off])))
>   goto partial_error;
>   }
>   return 0;
> @@ -302,11 +302,13 @@ int ipoib_dma_map_tx(struct ib_device *ca, struct 
> ipoib_tx_buf *tx_req)
>   for (; i > 0; --i) {
>   const skb_frag_t *frag = _shinfo(skb)->frags[i - 1];
>  
> - ib_dma_unmap_page(ca, mapping[i - !off], skb_frag_size(frag), 
> DMA_TO_DEVICE);
> + dma_unmap_page(ca->dma_device, mapping[i - !off],
> +skb_frag_size(frag), DMA_TO_DEVICE);
>   }
>  
>   if (off)
> - ib_dma_unmap_single(ca, mapping[0], skb_headlen(skb), 
> DMA_TO_DEVICE);
> + dma_unmap_single(ca->dma_device, mapping[0], skb_headlen(skb),
> +  DMA_TO_DEVICE);
>  
>   return -EIO;
>  }
> @@ -320,8 +322,8 @@ void ipoib_dma_unmap_tx(struct ipoib_dev_priv *priv,
>   int off;
>  
>   if (skb_headlen(skb)) {
> - ib_dma_unmap_single(priv->ca, mapping[0], skb_headlen(skb),
> - DMA_TO_DEVICE);
> + dma_unmap_single(priv->ca->dma_device, mapping[0],
> +  skb_headlen(skb), DMA_TO_DEVICE);
>   off = 1;
>   } else
>   off = 0;
> @@ -329,8 +331,8 @@ void ipoib_dma_unmap_tx(struct ipoib_dev_priv *priv,
>   for (i = 0; i < skb_shinfo(skb)->nr_frags; ++i) {
>   const skb_frag_t *frag = _shinfo(skb)->frags[i];
>  
> - ib_dma_unmap_page(priv->ca, mapping[i + off],
> -   skb_frag_size(frag), DMA_TO_DEVICE);
> + dma_unmap_page(priv->ca->dma_device, mapping[i + off],
> +skb_frag_size(frag), DMA_TO_DEVICE);
>   }
>  }
>  

Reviewed-by: Yuval Shaia <yuval.sh...@oracle.com>

> -- 
> 2.11.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 16/26] IB/IPoIB: Inline ib_dma_map_*() functions

[Yuval Shaia]

t; diff --git a/drivers/infiniband/ulp/ipoib/ipoib_ib.c 
> b/drivers/infiniband/ulp/ipoib/ipoib_ib.c
> index 5038f9d2d753..79204bd966bd 100644
> --- a/drivers/infiniband/ulp/ipoib/ipoib_ib.c
> +++ b/drivers/infiniband/ulp/ipoib/ipoib_ib.c
> @@ -92,9 +92,8 @@ void ipoib_free_ah(struct kref *kref)
>  static void ipoib_ud_dma_unmap_rx(struct ipoib_dev_priv *priv,
> u64 mapping[IPOIB_UD_RX_SG])
>  {
> - ib_dma_unmap_single(priv->ca, mapping[0],
> - IPOIB_UD_BUF_SIZE(priv->max_ib_mtu),
> - DMA_FROM_DEVICE);
> + dma_unmap_single(priv->ca->dma_device, mapping[0],
> +  IPOIB_UD_BUF_SIZE(priv->max_ib_mtu), DMA_FROM_DEVICE);
>  }
>  
>  static int ipoib_ib_post_receive(struct net_device *dev, int id)
> @@ -139,9 +138,9 @@ static struct sk_buff *ipoib_alloc_rx_skb(struct 
> net_device *dev, int id)
>   skb_reserve(skb, sizeof(struct ipoib_pseudo_header));
>  
>   mapping = priv->rx_ring[id].mapping;
> - mapping[0] = ib_dma_map_single(priv->ca, skb->data, buf_size,
> -DMA_FROM_DEVICE);
> - if (unlikely(ib_dma_mapping_error(priv->ca, mapping[0])))
> + mapping[0] = dma_map_single(priv->ca->dma_device, skb->data, buf_size,
> + DMA_FROM_DEVICE);
> + if (unlikely(dma_mapping_error(priv->ca->dma_device, mapping[0])))
>   goto error;
>  
>   priv->rx_ring[id].skb = skb;
> @@ -278,9 +277,9 @@ int ipoib_dma_map_tx(struct ib_device *ca, struct 
> ipoib_tx_buf *tx_req)
>   int off;
>  
>   if (skb_headlen(skb)) {
> - mapping[0] = ib_dma_map_single(ca, skb->data, skb_headlen(skb),
> -DMA_TO_DEVICE);
> - if (unlikely(ib_dma_mapping_error(ca, mapping[0])))
> + mapping[0] = dma_map_single(ca->dma_device, skb->data,
> + skb_headlen(skb), DMA_TO_DEVICE);
> + if (unlikely(dma_mapping_error(ca->dma_device, mapping[0])))
>   return -EIO;
>  
>   off = 1;
> @@ -289,11 +288,12 @@ int ipoib_dma_map_tx(struct ib_device *ca, struct 
> ipoib_tx_buf *tx_req)
>  
>   for (i = 0; i < skb_shinfo(skb)->nr_frags; ++i) {
>   const skb_frag_t *frag = _shinfo(skb)->frags[i];
> - mapping[i + off] = ib_dma_map_page(ca,
> -  skb_frag_page(frag),
> -  frag->page_offset, 
> skb_frag_size(frag),
> -  DMA_TO_DEVICE);
> - if (unlikely(ib_dma_mapping_error(ca, mapping[i + off])))
> + mapping[i + off] = dma_map_page(ca->dma_device,
> + skb_frag_page(frag),
> + frag->page_offset,
> + skb_frag_size(frag),
> + DMA_TO_DEVICE);
> + if (unlikely(dma_mapping_error(ca->dma_device, mapping[i + 
> off])))
>   goto partial_error;
>   }
>   return 0;
> @@ -302,11 +302,13 @@ int ipoib_dma_map_tx(struct ib_device *ca, struct 
> ipoib_tx_buf *tx_req)
>   for (; i > 0; --i) {
>   const skb_frag_t *frag = _shinfo(skb)->frags[i - 1];
>  
> - ib_dma_unmap_page(ca, mapping[i - !off], skb_frag_size(frag), 
> DMA_TO_DEVICE);
> + dma_unmap_page(ca->dma_device, mapping[i - !off],
> +skb_frag_size(frag), DMA_TO_DEVICE);
>   }
>  
>   if (off)
> - ib_dma_unmap_single(ca, mapping[0], skb_headlen(skb), 
> DMA_TO_DEVICE);
> + dma_unmap_single(ca->dma_device, mapping[0], skb_headlen(skb),
> +  DMA_TO_DEVICE);
>  
>   return -EIO;
>  }
> @@ -320,8 +322,8 @@ void ipoib_dma_unmap_tx(struct ipoib_dev_priv *priv,
>   int off;
>  
>   if (skb_headlen(skb)) {
> - ib_dma_unmap_single(priv->ca, mapping[0], skb_headlen(skb),
> - DMA_TO_DEVICE);
> + dma_unmap_single(priv->ca->dma_device, mapping[0],
> +  skb_headlen(skb), DMA_TO_DEVICE);
>   off = 1;
>   } else
>   off = 0;
> @@ -329,8 +331,8 @@ void ipoib_dma_unmap_tx(struct ipoib_dev_priv *priv,
>   for (i = 0; i < skb_shinfo(skb)->nr_frags; ++i) {
>   const skb_frag_t *frag = _shinfo(skb)->frags[i];
>  
> - ib_dma_unmap_page(priv->ca, mapping[i + off],
> -   skb_frag_size(frag), DMA_TO_DEVICE);
> + dma_unmap_page(priv->ca->dma_device, mapping[i + off],
> +skb_frag_size(frag), DMA_TO_DEVICE);
>   }
>  }
>  

Reviewed-by: Yuval Shaia 

> -- 
> 2.11.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] net: return value of skb_linearize should be handled in Linux kernel

[Yuval Shaia]

On Tue, Dec 06, 2016 at 03:10:33PM +0800, Zhouyi Zhou wrote:
> kmalloc_reserve may fail to allocate memory inside skb_linearize, 
> which means skb_linearize's return value should not be ignored. 
> Following patch correct the uses of skb_linearize.
> 
> Compiled in x86_64

FWIW compiled also on SPARC

Reviewed-by: Yuval Shaia <yuval.sh...@oracle.com>

> 
> Signed-off-by: Zhouyi Zhou <zhouzho...@gmail.com>
> ---
>  drivers/infiniband/hw/nes/nes_nic.c   | 5 +++--
>  drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c | 6 +-
>  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 3 +--
>  drivers/scsi/bnx2fc/bnx2fc_fcoe.c | 7 +--
>  drivers/scsi/fcoe/fcoe.c  | 5 -
>  net/tipc/link.c   | 3 ++-
>  net/tipc/name_distr.c | 5 -
>  7 files changed, 24 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/infiniband/hw/nes/nes_nic.c 
> b/drivers/infiniband/hw/nes/nes_nic.c
> index 2b27d13..69372ea 100644
> --- a/drivers/infiniband/hw/nes/nes_nic.c
> +++ b/drivers/infiniband/hw/nes/nes_nic.c
> @@ -662,10 +662,11 @@ static int nes_netdev_start_xmit(struct sk_buff *skb, 
> struct net_device *netdev)
>   nesnic->sq_head &= nesnic->sq_size-1;
>   }
>   } else {
> - nesvnic->linearized_skbs++;
>   hoffset = skb_transport_header(skb) - skb->data;
>   nhoffset = skb_network_header(skb) - skb->data;
> - skb_linearize(skb);
> + if (skb_linearize(skb))
> + return NETDEV_TX_BUSY;
> + nesvnic->linearized_skbs++;
>   skb_set_transport_header(skb, hoffset);
>   skb_set_network_header(skb, nhoffset);
>   if (!nes_nic_send(skb, netdev))
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c 
> b/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
> index 2a653ec..ab787cb 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
> @@ -490,7 +490,11 @@ int ixgbe_fcoe_ddp(struct ixgbe_adapter *adapter,
>*/
>   if ((fh->fh_r_ctl == FC_RCTL_DD_SOL_DATA) &&
>   (fctl & FC_FC_END_SEQ)) {
> - skb_linearize(skb);
> + int err = 0;
> +
> + err = skb_linearize(skb);
> + if (err)
> + return err;
>   crc = (struct fcoe_crc_eof *)skb_put(skb, sizeof(*crc));
>   crc->fcoe_eof = FC_EOF_T;
>   }
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c 
> b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index fee1f29..4926d48 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -2173,8 +2173,7 @@ static int ixgbe_clean_rx_irq(struct ixgbe_q_vector 
> *q_vector,
>   total_rx_bytes += ddp_bytes;
>   total_rx_packets += DIV_ROUND_UP(ddp_bytes,
>mss);
> - }
> - if (!ddp_bytes) {
> + } else {
>   dev_kfree_skb_any(skb);
>   continue;
>   }
> diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c 
> b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
> index f9ddb61..197d02e 100644
> --- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
> +++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
> @@ -542,8 +542,11 @@ static void bnx2fc_recv_frame(struct sk_buff *skb)
>   return;
>   }
>  
> - if (skb_is_nonlinear(skb))
> - skb_linearize(skb);
> + if (skb_linearize(skb)) {
> + kfree_skb(skb);
> + return;
> + }
> +
>   mac = eth_hdr(skb)->h_source;
>   dest_mac = eth_hdr(skb)->h_dest;
>  
> diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
> index 9bd41a3..f691b97 100644
> --- a/drivers/scsi/fcoe/fcoe.c
> +++ b/drivers/scsi/fcoe/fcoe.c
> @@ -1685,7 +1685,10 @@ static void fcoe_recv_frame(struct sk_buff *skb)
>   skb->dev ? skb->dev->name : "");
>  
>   port = lport_priv(lport);
> - skb_linearize(skb); /* check for skb_is_nonlinear is within 
> skb_linearize */
> + if (skb_linearize(skb)) {
> + kfree_skb(skb);
> + return;
> + }
>  
>   /*
>* Frame length checks and setting up the header pointer

Re: [PATCH] net: return value of skb_linearize should be handled in Linux kernel

[Yuval Shaia]

On Tue, Dec 06, 2016 at 03:10:33PM +0800, Zhouyi Zhou wrote:
> kmalloc_reserve may fail to allocate memory inside skb_linearize, 
> which means skb_linearize's return value should not be ignored. 
> Following patch correct the uses of skb_linearize.
> 
> Compiled in x86_64

FWIW compiled also on SPARC

Reviewed-by: Yuval Shaia 

> 
> Signed-off-by: Zhouyi Zhou 
> ---
>  drivers/infiniband/hw/nes/nes_nic.c   | 5 +++--
>  drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c | 6 +-
>  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 3 +--
>  drivers/scsi/bnx2fc/bnx2fc_fcoe.c | 7 +--
>  drivers/scsi/fcoe/fcoe.c  | 5 -
>  net/tipc/link.c   | 3 ++-
>  net/tipc/name_distr.c | 5 -
>  7 files changed, 24 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/infiniband/hw/nes/nes_nic.c 
> b/drivers/infiniband/hw/nes/nes_nic.c
> index 2b27d13..69372ea 100644
> --- a/drivers/infiniband/hw/nes/nes_nic.c
> +++ b/drivers/infiniband/hw/nes/nes_nic.c
> @@ -662,10 +662,11 @@ static int nes_netdev_start_xmit(struct sk_buff *skb, 
> struct net_device *netdev)
>   nesnic->sq_head &= nesnic->sq_size-1;
>   }
>   } else {
> - nesvnic->linearized_skbs++;
>   hoffset = skb_transport_header(skb) - skb->data;
>   nhoffset = skb_network_header(skb) - skb->data;
> - skb_linearize(skb);
> + if (skb_linearize(skb))
> + return NETDEV_TX_BUSY;
> + nesvnic->linearized_skbs++;
>   skb_set_transport_header(skb, hoffset);
>   skb_set_network_header(skb, nhoffset);
>   if (!nes_nic_send(skb, netdev))
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c 
> b/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
> index 2a653ec..ab787cb 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
> @@ -490,7 +490,11 @@ int ixgbe_fcoe_ddp(struct ixgbe_adapter *adapter,
>*/
>   if ((fh->fh_r_ctl == FC_RCTL_DD_SOL_DATA) &&
>   (fctl & FC_FC_END_SEQ)) {
> - skb_linearize(skb);
> + int err = 0;
> +
> + err = skb_linearize(skb);
> + if (err)
> + return err;
>   crc = (struct fcoe_crc_eof *)skb_put(skb, sizeof(*crc));
>   crc->fcoe_eof = FC_EOF_T;
>   }
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c 
> b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index fee1f29..4926d48 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -2173,8 +2173,7 @@ static int ixgbe_clean_rx_irq(struct ixgbe_q_vector 
> *q_vector,
>   total_rx_bytes += ddp_bytes;
>   total_rx_packets += DIV_ROUND_UP(ddp_bytes,
>mss);
> - }
> - if (!ddp_bytes) {
> + } else {
>   dev_kfree_skb_any(skb);
>   continue;
>   }
> diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c 
> b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
> index f9ddb61..197d02e 100644
> --- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
> +++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
> @@ -542,8 +542,11 @@ static void bnx2fc_recv_frame(struct sk_buff *skb)
>   return;
>   }
>  
> - if (skb_is_nonlinear(skb))
> - skb_linearize(skb);
> + if (skb_linearize(skb)) {
> + kfree_skb(skb);
> + return;
> + }
> +
>   mac = eth_hdr(skb)->h_source;
>   dest_mac = eth_hdr(skb)->h_dest;
>  
> diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
> index 9bd41a3..f691b97 100644
> --- a/drivers/scsi/fcoe/fcoe.c
> +++ b/drivers/scsi/fcoe/fcoe.c
> @@ -1685,7 +1685,10 @@ static void fcoe_recv_frame(struct sk_buff *skb)
>   skb->dev ? skb->dev->name : "");
>  
>   port = lport_priv(lport);
> - skb_linearize(skb); /* check for skb_is_nonlinear is within 
> skb_linearize */
> + if (skb_linearize(skb)) {
> + kfree_skb(skb);
> + return;
> + }
>  
>   /*
>* Frame length checks and setting up the header pointers
> diff --git a/net/tipc/link.c b/net/tipc/link.c
> index bd

Re: [PATCH] fix:infiniband:hw:cxgb4:qp:mark symbols static where possible

[Yuval Shaia]

One minor comment inline.
Besides that:
Reviewed-by: Yuval Shaia <yuval.sh...@oracle.com>

On Sun, Aug 28, 2016 at 12:00:18AM +0800, Baoyou Xie wrote:
> We get 1 warning when biuld kernel with W=1:

s/biuld/build

> drivers/infiniband/hw/cxgb4/qp.c:686:6: warning: no previous prototype for 
> '_free_qp' [-Wmissing-prototypes]
> 
> In fact, this function is only used in the file in which it is declared
> and don't need a declaration, but can be made static.
> so this patch marks it 'static'.
> 
> Signed-off-by: Baoyou Xie <baoyou@linaro.org>
> ---
>  drivers/infiniband/hw/cxgb4/qp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/infiniband/hw/cxgb4/qp.c 
> b/drivers/infiniband/hw/cxgb4/qp.c
> index edb1172..6904352 100644
> --- a/drivers/infiniband/hw/cxgb4/qp.c
> +++ b/drivers/infiniband/hw/cxgb4/qp.c
> @@ -683,7 +683,7 @@ static int build_inv_stag(union t4_wr *wqe, struct 
> ib_send_wr *wr,
>   return 0;
>  }
>  
> -void _free_qp(struct kref *kref)
> +static void _free_qp(struct kref *kref)
>  {
>   struct c4iw_qp *qhp;
>  
> -- 
> 2.7.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] fix:infiniband:hw:cxgb4:qp:mark symbols static where possible

[Yuval Shaia]

One minor comment inline.
Besides that:
Reviewed-by: Yuval Shaia 

On Sun, Aug 28, 2016 at 12:00:18AM +0800, Baoyou Xie wrote:
> We get 1 warning when biuld kernel with W=1:

s/biuld/build

> drivers/infiniband/hw/cxgb4/qp.c:686:6: warning: no previous prototype for 
> '_free_qp' [-Wmissing-prototypes]
> 
> In fact, this function is only used in the file in which it is declared
> and don't need a declaration, but can be made static.
> so this patch marks it 'static'.
> 
> Signed-off-by: Baoyou Xie 
> ---
>  drivers/infiniband/hw/cxgb4/qp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/infiniband/hw/cxgb4/qp.c 
> b/drivers/infiniband/hw/cxgb4/qp.c
> index edb1172..6904352 100644
> --- a/drivers/infiniband/hw/cxgb4/qp.c
> +++ b/drivers/infiniband/hw/cxgb4/qp.c
> @@ -683,7 +683,7 @@ static int build_inv_stag(union t4_wr *wqe, struct 
> ib_send_wr *wr,
>   return 0;
>  }
>  
> -void _free_qp(struct kref *kref)
> +static void _free_qp(struct kref *kref)
>  {
>   struct c4iw_qp *qhp;
>  
> -- 
> 2.7.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 18/22] IB/fmr_pool: Convert the cleanup thread into kthread worker API

[Yuval Shaia]

On Wed, Nov 18, 2015 at 02:25:23PM +0100, Petr Mladek wrote:
> Kthreads are currently implemented as an infinite loop. Each
> has its own variant of checks for terminating, freezing,
> awakening. In many cases it is unclear to say in which state
> it is and sometimes it is done a wrong way.
> 
> The plan is to convert kthreads into kthread_worker or workqueues
> API. It allows to split the functionality into separate operations.
> It helps to make a better structure. Also it defines a clean state
> where no locks are taken, IRQs blocked, the kthread might sleep
> or even be safely migrated.
> 
> The kthread worker API is useful when we want to have a dedicated
> single thread for the work. It helps to make sure that it is
> available when needed. Also it allows a better control, e.g.
> define a scheduling priority.
> 
> This patch converts the frm_pool kthread into the kthread worker
s/frm/fmr
> API because I am not sure how busy the thread is. It is well
> possible that it does not need a dedicated kthread and workqueues
> would be perfectly fine. Well, the conversion between kthread
> worker API and workqueues is pretty trivial.
> 
> The patch moves one iteration from the kthread into the work function.
> It preserves the check for a spurious queuing (wake up). Then it
> processes one request. Finally, it re-queues itself if more requests
> are pending.
> 
> Otherwise, wake_up_process() is replaced by queuing the work.
> 
> Important: The change is only compile tested. I did not find an easy
> way how to check it in a real life.
What are the expectations?
> 
> Signed-off-by: Petr Mladek 
> CC: Doug Ledford 
> CC: Sean Hefty 
> CC: Hal Rosenstock 
> CC: linux-r...@vger.kernel.org
> ---
>  drivers/infiniband/core/fmr_pool.c | 54 
> ++
>  1 file changed, 25 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/infiniband/core/fmr_pool.c 
> b/drivers/infiniband/core/fmr_pool.c
> index 9f5ad7cc33c8..5f2b06bd14da 100644
> --- a/drivers/infiniband/core/fmr_pool.c
> +++ b/drivers/infiniband/core/fmr_pool.c
> @@ -96,7 +96,8 @@ struct ib_fmr_pool {
>  void *  arg);
>   void *flush_arg;
>  
> - struct task_struct   *thread;
> + struct kthread_worker *worker;
> + struct kthread_work   work;
>  
>   atomic_t  req_ser;
>   atomic_t  flush_ser;
> @@ -174,29 +175,26 @@ static void ib_fmr_batch_release(struct ib_fmr_pool 
> *pool)
>   spin_unlock_irq(>pool_lock);
>  }
>  
> -static int ib_fmr_cleanup_thread(void *pool_ptr)
> +static void ib_fmr_cleanup_func(struct kthread_work *work)
>  {
> - struct ib_fmr_pool *pool = pool_ptr;
> + struct ib_fmr_pool *pool = container_of(work, struct ib_fmr_pool, work);
>  
> - do {
> - if (atomic_read(>flush_ser) - atomic_read(>req_ser) 
> < 0) {
> - ib_fmr_batch_release(pool);
> -
> - atomic_inc(>flush_ser);
> - wake_up_interruptible(>force_wait);
> + /*
> +  * The same request might be queued twice when it appears and
> +  * by re-queuing from this work.
> +  */
> + if (atomic_read(>flush_ser) - atomic_read(>req_ser) >= 0)
> + return;
>  
> - if (pool->flush_function)
> - pool->flush_function(pool, pool->flush_arg);
> - }
> + ib_fmr_batch_release(pool);
> + atomic_inc(>flush_ser);
> + wake_up_interruptible(>force_wait);
>  
> - set_current_state(TASK_INTERRUPTIBLE);
> - if (atomic_read(>flush_ser) - atomic_read(>req_ser) 
> >= 0 &&
> - !kthread_should_stop())
> - schedule();
> - __set_current_state(TASK_RUNNING);
> - } while (!kthread_should_stop());
> + if (pool->flush_function)
> + pool->flush_function(pool, pool->flush_arg);
>  
> - return 0;
> + if (atomic_read(>flush_ser) - atomic_read(>req_ser) < 0)
> + queue_kthread_work(pool->worker, >work);
>  }
>  
>  /**
> @@ -286,15 +284,13 @@ struct ib_fmr_pool *ib_create_fmr_pool(struct ib_pd 
> *pd,
>   atomic_set(>flush_ser, 0);
>   init_waitqueue_head(>force_wait);
>  
> - pool->thread = kthread_run(ib_fmr_cleanup_thread,
> -pool,
> -"ib_fmr(%s)",
> -device->name);
> - if (IS_ERR(pool->thread)) {
> - printk(KERN_WARNING PFX "couldn't start cleanup thread\n");
> - ret = PTR_ERR(pool->thread);
> + pool->worker = create_kthread_worker(0, "ib_fmr(%s)", device->name);
Is this patch depends on some other patch?
> + if (IS_ERR(pool->worker)) {
> + pr_warn(PFX "couldn't start cleanup kthread worker\n");
> + ret = PTR_ERR(pool->worker);
>   goto out_free_pool;
>   }
> 

Re: [PATCH v3 18/22] IB/fmr_pool: Convert the cleanup thread into kthread worker API

[Yuval Shaia]

On Wed, Nov 18, 2015 at 02:25:23PM +0100, Petr Mladek wrote:
> Kthreads are currently implemented as an infinite loop. Each
> has its own variant of checks for terminating, freezing,
> awakening. In many cases it is unclear to say in which state
> it is and sometimes it is done a wrong way.
> 
> The plan is to convert kthreads into kthread_worker or workqueues
> API. It allows to split the functionality into separate operations.
> It helps to make a better structure. Also it defines a clean state
> where no locks are taken, IRQs blocked, the kthread might sleep
> or even be safely migrated.
> 
> The kthread worker API is useful when we want to have a dedicated
> single thread for the work. It helps to make sure that it is
> available when needed. Also it allows a better control, e.g.
> define a scheduling priority.
> 
> This patch converts the frm_pool kthread into the kthread worker
s/frm/fmr
> API because I am not sure how busy the thread is. It is well
> possible that it does not need a dedicated kthread and workqueues
> would be perfectly fine. Well, the conversion between kthread
> worker API and workqueues is pretty trivial.
> 
> The patch moves one iteration from the kthread into the work function.
> It preserves the check for a spurious queuing (wake up). Then it
> processes one request. Finally, it re-queues itself if more requests
> are pending.
> 
> Otherwise, wake_up_process() is replaced by queuing the work.
> 
> Important: The change is only compile tested. I did not find an easy
> way how to check it in a real life.
What are the expectations?
> 
> Signed-off-by: Petr Mladek 
> CC: Doug Ledford 
> CC: Sean Hefty 
> CC: Hal Rosenstock 
> CC: linux-r...@vger.kernel.org
> ---
>  drivers/infiniband/core/fmr_pool.c | 54 
> ++
>  1 file changed, 25 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/infiniband/core/fmr_pool.c 
> b/drivers/infiniband/core/fmr_pool.c
> index 9f5ad7cc33c8..5f2b06bd14da 100644
> --- a/drivers/infiniband/core/fmr_pool.c
> +++ b/drivers/infiniband/core/fmr_pool.c
> @@ -96,7 +96,8 @@ struct ib_fmr_pool {
>  void *  arg);
>   void *flush_arg;
>  
> - struct task_struct   *thread;
> + struct kthread_worker *worker;
> + struct kthread_work   work;
>  
>   atomic_t  req_ser;
>   atomic_t  flush_ser;
> @@ -174,29 +175,26 @@ static void ib_fmr_batch_release(struct ib_fmr_pool 
> *pool)
>   spin_unlock_irq(>pool_lock);
>  }
>  
> -static int ib_fmr_cleanup_thread(void *pool_ptr)
> +static void ib_fmr_cleanup_func(struct kthread_work *work)
>  {
> - struct ib_fmr_pool *pool = pool_ptr;
> + struct ib_fmr_pool *pool = container_of(work, struct ib_fmr_pool, work);
>  
> - do {
> - if (atomic_read(>flush_ser) - atomic_read(>req_ser) 
> < 0) {
> - ib_fmr_batch_release(pool);
> -
> - atomic_inc(>flush_ser);
> - wake_up_interruptible(>force_wait);
> + /*
> +  * The same request might be queued twice when it appears and
> +  * by re-queuing from this work.
> +  */
> + if (atomic_read(>flush_ser) - atomic_read(>req_ser) >= 0)
> + return;
>  
> - if (pool->flush_function)
> - pool->flush_function(pool, pool->flush_arg);
> - }
> + ib_fmr_batch_release(pool);
> + atomic_inc(>flush_ser);
> + wake_up_interruptible(>force_wait);
>  
> - set_current_state(TASK_INTERRUPTIBLE);
> - if (atomic_read(>flush_ser) - atomic_read(>req_ser) 
> >= 0 &&
> - !kthread_should_stop())
> - schedule();
> - __set_current_state(TASK_RUNNING);
> - } while (!kthread_should_stop());
> + if (pool->flush_function)
> + pool->flush_function(pool, pool->flush_arg);
>  
> - return 0;
> + if (atomic_read(>flush_ser) - atomic_read(>req_ser) < 0)
> + queue_kthread_work(pool->worker, >work);
>  }
>  
>  /**
> @@ -286,15 +284,13 @@ struct ib_fmr_pool *ib_create_fmr_pool(struct ib_pd 
> *pd,
>   atomic_set(>flush_ser, 0);
>   init_waitqueue_head(>force_wait);
>  
> - pool->thread = kthread_run(ib_fmr_cleanup_thread,
> -pool,
> -"ib_fmr(%s)",
> -device->name);
> - if (IS_ERR(pool->thread)) {
> - printk(KERN_WARNING PFX "couldn't start cleanup thread\n");
> - ret = PTR_ERR(pool->thread);
> + pool->worker = create_kthread_worker(0, "ib_fmr(%s)", device->name);
Is this patch depends on some other patch?
> + if (IS_ERR(pool->worker)) {
> + pr_warn(PFX "couldn't start cleanup kthread worker\n");
> +