lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <1265726692.2126.80.camel@achroite.uk.solarflarecom.com>
Date:	Tue, 09 Feb 2010 14:44:52 +0000
From:	Ben Hutchings <bhutchings@...arflare.com>
To:	Yevgeny Petrilin <yevgenyp@...lanox.co.il>
Cc:	Roland Dreier <rdreier@...co.com>, general@...ts.openfabrics.org,
	netdev@...r.kernel.org, liranl@...lanox.co.il,
	tziporet@...lanox.co.il
Subject: Re: [PATCH 04/23 v3] mlx4_core: add slave resource allocation

On Thu, 2010-02-04 at 17:54 +0200, Yevgeny Petrilin wrote:
> All QPs/CQs/SRQs/MPTs/MTTs are allocated from shared pools, which are owned by
> the master. In addition, all backing ICM memory for these objects is managed by
> the master.
> To allow slaves to allocate resources, ICM allocation is separated from the rest
> of the object state, which is held at the slave.
> Slaves can then reserve resource ranges and allocate ICM over the comm channel.
> 
> Signed-off-by: Liran Liss <liranl@...lanox.co.il>
> Signed-off-by: Yevgeny Petrilin <yevgenyp@...lanox.co.il>
> ---
>  drivers/net/mlx4/cmd.c   |  110 +++++++++++++++++++++++++++++++++
>  drivers/net/mlx4/cq.c    |   91 +++++++++++++++++++++-------
>  drivers/net/mlx4/mlx4.h  |   27 ++++++++
>  drivers/net/mlx4/mr.c    |  125 ++++++++++++++++++++++++++++++++++----
>  drivers/net/mlx4/qp.c    |  151 +++++++++++++++++++++++++++++++++-------------
>  drivers/net/mlx4/srq.c   |   88 ++++++++++++++++++++-------
>  include/linux/mlx4/cmd.h |    2 +
>  7 files changed, 496 insertions(+), 98 deletions(-)
> 
> diff --git a/drivers/net/mlx4/cmd.c b/drivers/net/mlx4/cmd.c
> index 9e85330..533ce77 100644
> --- a/drivers/net/mlx4/cmd.c
> +++ b/drivers/net/mlx4/cmd.c
> @@ -417,6 +417,100 @@ static int mlx4_ACCESS_MEM(struct mlx4_dev *dev, u64 master_addr,
>  					   MLX4_CMD_TIME_CLASS_A);
>  }
>  
> +static int mlx4_RESOURCE_wrapper(struct mlx4_dev *dev, int slave, struct mlx4_vhcr *vhcr,
> +						       struct mlx4_cmd_mailbox *inbox,
> +						       struct mlx4_cmd_mailbox *outbox)
> +{
> +	u32 param1 = *((u32 *) &vhcr->in_param);
> +	u32 param2 = *(((u32 *) &vhcr->in_param) + 1);
> +	int ret;
> +
> +#if 0
> +	char *res[] = {"QP", "CQ", "SRQ", "MPT", "MTT"};
> +	mlx4_warn(dev, "resource wrapper - %s (mode: %s) type:%s param1:%d param2:%d\n",
> +			vhcr->op == MLX4_CMD_ALLOC_RES ? "allocate" : "free",
> +			vhcr->op_modifier == ICM_RESERVE ? "reserve" :
> +				(vhcr->op_modifier == ICM_ALLOC ? "alloc" : "reserve+alloc"),
> +			res[vhcr->in_modifier], param1, param2);
> +#endif

Either make this really conditional or remove it.  '#if 0' is not
useful.

[...]
> diff --git a/drivers/net/mlx4/cq.c b/drivers/net/mlx4/cq.c
> index ccfe276..3fb9f7f 100644
> --- a/drivers/net/mlx4/cq.c
> +++ b/drivers/net/mlx4/cq.c
[...]
> +void mlx4_cq_free_icm(struct mlx4_dev *dev, int cqn)
> +{
> +	struct mlx4_priv *priv = mlx4_priv(dev);
> +	struct mlx4_cq_table *cq_table = &priv->cq_table;
> +	u64 in_param;
> +	int err;
> +
> +	if (mlx4_is_slave(dev)) {
> +		*((u32 *) &in_param) = cqn;
> +		*(((u32 *) &in_param) + 1) = 0;
> +		err = mlx4_cmd(dev, in_param, RES_CQ, ICM_RESERVE_AND_ALLOC,
> +						      MLX4_CMD_FREE_RES,
> +						      MLX4_CMD_TIME_CLASS_A);

This is fairly disgusting, and surely only works for one byte order.  I
think you really want:

	in_param = cqn;

[...]
> diff --git a/drivers/net/mlx4/mr.c b/drivers/net/mlx4/mr.c
> index 9e4ab0f..0db7f88 100644
> --- a/drivers/net/mlx4/mr.c
> +++ b/drivers/net/mlx4/mr.c
> @@ -178,10 +178,26 @@ static void mlx4_buddy_cleanup(struct mlx4_buddy *buddy)
>  	kfree(buddy->num_free);
>  }
>  
> -static u32 mlx4_alloc_mtt_range(struct mlx4_dev *dev, int order)
> +u32 mlx4_alloc_mtt_range(struct mlx4_dev *dev, int order)
>  {
>  	struct mlx4_mr_table *mr_table = &mlx4_priv(dev)->mr_table;
> +	u64 in_param;
> +	u64 out_param;
>  	u32 seg;
> +	int err;
> +
> +	if (mlx4_is_slave(dev)) {
> +		*((u32 *) &in_param) = order;
> +		*(((u32 *) &in_param) + 1) = 0;
> +		err = mlx4_cmd_imm(dev, in_param, &out_param, RES_MTT,
> +						       ICM_RESERVE_AND_ALLOC,
> +						       MLX4_CMD_ALLOC_RES,
> +						       MLX4_CMD_TIME_CLASS_A);

Again with the byte-ordering...

> +		if (err)
> +			return -1;
> +		else
> +			return out_param;
> +	}
>  
>  	seg = mlx4_buddy_alloc(&mr_table->mtt_buddy, order);
>  	if (seg == -1)
> @@ -219,16 +235,33 @@ int mlx4_mtt_init(struct mlx4_dev *dev, int npages, int page_shift,
>  }
>  EXPORT_SYMBOL_GPL(mlx4_mtt_init);
>  
> -void mlx4_mtt_cleanup(struct mlx4_dev *dev, struct mlx4_mtt *mtt)
> +void mlx4_free_mtt_range(struct mlx4_dev *dev, u32 first_seg, int order)
>  {
>  	struct mlx4_mr_table *mr_table = &mlx4_priv(dev)->mr_table;
> +	u64 in_param;
> +	int err;
> +
> +	if (mlx4_is_slave(dev)) {
> +		*((u32 *) &in_param) = first_seg;
> +		*(((u32 *) &in_param) + 1) = order;
> +		err = mlx4_cmd(dev, in_param, RES_MTT, ICM_RESERVE_AND_ALLOC,
> +						       MLX4_CMD_FREE_RES,
> +						       MLX4_CMD_TIME_CLASS_A);

and here:

		in_param = first_seg | (u64)order << 32;

[...]
> +void mlx4_mr_release(struct mlx4_dev *dev, u32 index)
> +{
> +	struct mlx4_priv *priv = mlx4_priv(dev);
> +	u64 in_param;
> +	int err;
> +
> +	if (mlx4_is_slave(dev)) {
> +		*((u32 *) &in_param) = index;
> +		*(((u32 *) &in_param) + 1) = 0;

etc. etc.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ