From mboxrd@z Thu Jan  1 00:00:00 1970
From: "Jan Beulich" <JBeulich@novell.com>
Subject: RE: [Pv-ops][PATCH 0/3 v3] Netback multiple threads support
Date: Tue, 04 May 2010 13:10:52 +0100
Message-ID: <4BE02AEC0200007800001237@vpn.id2.novell.com>
References: <D5AB6E638E5A3E4B8F4406B113A5A19A1D94B446@shsmsx501.ccr.corp.intel.com>
	<4BE0073B02000078000011E7@vpn.id2.novell.com>
	<D5AB6E638E5A3E4B8F4406B113A5A19A1D94B535@shsmsx501.ccr.corp.intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: quoted-printable
Return-path: <xen-devel-bounces@lists.xensource.com>
In-Reply-To: <D5AB6E638E5A3E4B8F4406B113A5A19A1D94B535@shsmsx501.ccr.corp.intel.com>
Content-Disposition: inline
List-Unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xensource.com>
List-Help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-Subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
Sender: xen-devel-bounces@lists.xensource.com
Errors-To: xen-devel-bounces@lists.xensource.com
To: Dongxiao Xu <dongxiao.xu@intel.com>
Cc: Jeremy Fitzhardinge <jeremy@goop.org>, "xen-devel@lists.xensource.com" <xen-devel@lists.xensource.com>, Steven Smith <steven.smith@citrix.com>, KonradRzeszutek Wilk <konrad.wilk@oracle.com>
List-Id: xen-devel@lists.xenproject.org

>>> "Xu, Dongxiao" <dongxiao.xu@intel.com> 04.05.10 13:36 >>>
>Jan Beulich wrote:
>>>>> "Xu, Dongxiao" <dongxiao.xu@intel.com> 04.05.10 03:52 >>>
>>> 1. Merge "group" and "idx" into "netif->mapping", therefore
>>> page_ext is not used now.
>>=20
>> Open coding this seems very fragile (and even more using literal
>> constants in those code fragments).
>>=20
>> I'm also not convinced restricting either part to 16 bits is a good
>> thing (particularly on 64-bits, where you could easily have each part
>> use 32 bits).
>
>Do you have any suggestion on how to embed the data into
>netif->mapping?=20

Here is how I implemented this in our version of those patches:

/* extra field used in struct page */
union page_ext {
	struct {
#if BITS_PER_LONG < 64
#define GROUP_WIDTH (BITS_PER_LONG - CONFIG_XEN_NETDEV_TX_SHIFT)
#define MAX_GROUPS ((1U << GROUP_WIDTH) - 1)
		unsigned int grp:GROUP_WIDTH;
		unsigned int idx:CONFIG_XEN_NETDEV_TX_SHIFT;
#else
#define MAX_GROUPS UINT_MAX
		unsigned int grp, idx;
#endif
	} e;
	void *mapping;
};

static inline void netif_set_page_ext(struct page *pg, unsigned int group,
				      unsigned int idx)
{
	union page_ext ext =3D { .e =3D { .grp =3D group + 1, .idx =3D idx =
} };

	BUILD_BUG_ON(sizeof(ext) > sizeof(ext.mapping));
	pg->mapping =3D ext.mapping;
}

static inline unsigned int netif_page_group(const struct page *pg)
{
	union page_ext ext =3D { .mapping =3D pg->mapping };

	return ext.e.grp - 1;
}

static inline unsigned int netif_page_index(const struct page *pg)
{
	union page_ext ext =3D { .mapping =3D pg->mapping };

	return ext.e.idx;
}

CONFIG_XEN_NETDEV_TX_SHIFT is something an earlier patch of
ours introduces (range limited to 5...16; certainly pv-ops could
benefit from having such too), so you will need to replace this
with a literal constant 8 for the time being.

Jan