RE: [Pv-ops][PATCH 0/3 v3] Netback multiple threads support

["Jan Beulich" <JBeulich@novell.com>]

>>> "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.
>> 
>> Open coding this seems very fragile (and even more using literal
>> constants in those code fragments).
>> 
>> 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? 

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 = { .e = { .grp = group + 1, .idx = idx } };

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

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

	return ext.e.grp - 1;
}

static inline unsigned int netif_page_index(const struct page *pg)
{
	union page_ext ext = { .mapping = 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