From: sebastien dugue <sebastien.dugue-6ktuUTfB/bM@public.gmane.org>
To: Roland Dreier <rdreier-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>
Cc: Eli Cohen <eli-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>,
linux-rdma <linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
Subject: Re: [PATCH] mlx4: Fix chunk sg list overflow in mlx4_alloc_icm()
Date: Thu, 6 May 2010 08:04:53 +0200 [thread overview]
Message-ID: <20100506080453.43594a4b@frecb007965> (raw)
In-Reply-To: <adatyqmmej0.fsf-BjVyx320WGW9gfZ95n9DRSW4+XlvGpQz@public.gmane.org>
Hi Roland,
On Wed, 05 May 2010 10:42:11 -0700
Roland Dreier <rdreier-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org> wrote:
> good catch, applied. Did this hit you in practice? I guess it would
> take a big coherent ICM allocation, were you getting those?
Yes, some customer got hit by this, which ended up corrupting memory.
>
> Also what do you think of this independent cleanup on top of your patch?
> It handles the error case for allocation and then avoids having the
> common case inside a deeper nested block:
No problem, it indeed makes the code more readable.
Thanks,
Sebastien.
>
> drivers/net/mlx4/icm.c | 37 +++++++++++++++++++------------------
> 1 files changed, 19 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/net/mlx4/icm.c b/drivers/net/mlx4/icm.c
> index ef62f17..b07e4de 100644
> --- a/drivers/net/mlx4/icm.c
> +++ b/drivers/net/mlx4/icm.c
> @@ -163,29 +163,30 @@ struct mlx4_icm *mlx4_alloc_icm(struct mlx4_dev *dev, int npages,
> ret = mlx4_alloc_icm_pages(&chunk->mem[chunk->npages],
> cur_order, gfp_mask);
>
> - if (!ret) {
> - ++chunk->npages;
> -
> - if (coherent)
> - ++chunk->nsg;
> - else if (chunk->npages == MLX4_ICM_CHUNK_LEN) {
> - chunk->nsg = pci_map_sg(dev->pdev, chunk->mem,
> - chunk->npages,
> - PCI_DMA_BIDIRECTIONAL);
> -
> - if (chunk->nsg <= 0)
> - goto fail;
> - }
> + if (ret) {
> + if (--cur_order < 0)
> + goto fail;
> + else
> + continue;
> + }
>
> - if (chunk->npages == MLX4_ICM_CHUNK_LEN)
> - chunk = NULL;
> + ++chunk->npages;
>
> - npages -= 1 << cur_order;
> - } else {
> - --cur_order;
> - if (cur_order < 0)
> + if (coherent)
> + ++chunk->nsg;
> + else if (chunk->npages == MLX4_ICM_CHUNK_LEN) {
> + chunk->nsg = pci_map_sg(dev->pdev, chunk->mem,
> + chunk->npages,
> + PCI_DMA_BIDIRECTIONAL);
> +
> + if (chunk->nsg <= 0)
> goto fail;
> }
> +
> + if (chunk->npages == MLX4_ICM_CHUNK_LEN)
> + chunk = NULL;
> +
> + npages -= 1 << cur_order;
> }
>
> if (!coherent && chunk) {
>
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
next prev parent reply other threads:[~2010-05-06 6:04 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-04-30 11:21 [PATCH] mlx4: Fix chunk sg list overflow in mlx4_alloc_icm() sebastien dugue
2010-05-03 6:27 ` Eli Cohen
2010-05-05 17:42 ` Roland Dreier
[not found] ` <adatyqmmej0.fsf-BjVyx320WGW9gfZ95n9DRSW4+XlvGpQz@public.gmane.org>
2010-05-06 6:04 ` sebastien dugue [this message]
2010-05-06 16:08 ` Roland Dreier
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20100506080453.43594a4b@frecb007965 \
--to=sebastien.dugue-6ktuutfb/bm@public.gmane.org \
--cc=eli-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org \
--cc=linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=rdreier-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox