ixl(4)/ixlv(4): Revert m_collapse() in ixl_xmit() to m_defrag().
ClosedPublic
Actions

Authored by erj on Feb 5 2016, 11:05 PM.

Details

Reviewers

skozlov
jeffrey.e.pieper_intel.com
gnn
sbruno

Group Reviewers

Intel Networking

Summary

The m_collapse() failed when transmitting medium-sized packets when the
interface mtu was set to 9000, so revert back to the non-error-causing
m_defrag().

It's also possible there was no performance benefit to using m_collapse()
over m_defrag() there, but that would need to be investigated.

Test Plan

Test medium (960-1400 byte) message sizes with 9000 MTU.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 2406
Build 2422: arc lint + arc unit

Event Timeline

erj updated this revision to Diff 13073.Feb 5 2016, 11:05 PM

erj retitled this revision from to ixl(4)/ixlv(4): Revert m_collapse() in ixl_xmit() to m_defrag()..

erj updated this object.

Herald added a reviewer: Intel Networking. · View Herald TranscriptFeb 5 2016, 11:05 PM

Herald added a subscriber: imp. · View Herald Transcript

erj added reviewers: sbruno, gnn, jeffrey.e.pieper_intel.com.Feb 5 2016, 11:05 PM

erj added a reviewer: skozlov.

jeffrey.e.pieper_intel.com accepted this revision.Feb 11 2016, 11:04 PM

jeffrey.e.pieper_intel.com edited the test plan for this revision. (Show Details)

jeffrey.e.pieper_intel.com edited edge metadata.

This revision is now accepted and ready to land.Feb 11 2016, 11:05 PM

This was committed in rS295831, but this revision didn't update for some reason.

Revision Contents
Changeset List

Path

Size

sys/

dev/

ixl/

ixl_txrx.c

2 lines

Diff 13073

View Options

sys/dev/ixl/ixl_txrx.c

Context not available.
	* SWAPBLK_NONE on an allocation failure.	* SWAPBLK_NONE on an allocation failure.
	*	*
	* A radix tree is used to maintain the bitmap. Two radix constants are	* A radix tree is used to maintain the bitmap. Two radix constants are
	* involved: One for the bitmaps contained in the leaf nodes (typically	* involved: One for the bitmaps contained in the leaf nodes (typically
	* 64), and one for the meta nodes (typically 16). Both meta and leaf	* 64), and one for the meta nodes (typically 16). Each subtree is
	* nodes have a hint field. This field gives us a hint as to the largest	* associated with a range of blocks. The root of any subtree stores a
	* free contiguous range of blocks under the node. It may contain a	* hint field that defines an upper bound on the size of the largest
	* value that is too high, but will never contain a value that is too	* allocation that can begin in the associated block range. A hint is an
	* low. When the radix tree is searched, allocation failures in subtrees	* upper bound on a potential allocation, but not necessarily a tight upper
	* update the hint.	* bound.
	*	*
	* The radix tree also implements two collapsed states for meta nodes:	* The radix tree also implements two collapsed states for meta nodes:
	* the ALL-ALLOCATED state and the ALL-FREE state. If a meta node is	* the ALL-ALLOCATED state and the ALL-FREE state. If a meta node is
Context not available.
	#define bitcount64(x) __bitcount64((uint64_t)(x))	#define bitcount64(x) __bitcount64((uint64_t)(x))
	#define malloc(a,b,c) calloc(a, 1)	#define malloc(a,b,c) calloc(a, 1)
	#define free(a,b) free(a)	#define free(a,b) free(a)
		#define imax(a,b) (((a)>(b))?(a):(b))
	#define CTASSERT(expr)	#define CTASSERT(expr)

	#include <sys/blist.h>	#include <sys/blist.h>
Context not available.
	/*	/*
	* static support functions	* static support functions
	*/	*/
	static daddr_t blst_leaf_alloc(blmeta_t *scan, daddr_t blk, int count,	static daddr_t blst_leaf_alloc(blmeta_t *scan, daddr_t blk, int count);
	daddr_t cursor);
	static daddr_t blst_meta_alloc(blmeta_t *scan, daddr_t cursor, daddr_t count,	static daddr_t blst_meta_alloc(blmeta_t *scan, daddr_t cursor, daddr_t count,
	u_daddr_t radix);	u_daddr_t radix);
	static void blst_leaf_free(blmeta_t *scan, daddr_t relblk, int count);	static void blst_leaf_free(blmeta_t *scan, daddr_t relblk, int count);
		markjUnsubmitted Not Done Inline Actions Looks like radix_to_skip() could use this too. It might also be reasonable to have a BLIST_BMAP_MASK. markj: Looks like radix_to_skip() could use this too. It might also be reasonable to have a…
		dougmAuthorUnsubmitted Not Done Inline Actions Done. BLIST_META_MASK will come up later in future patches, I hope. dougm: Done. BLIST_META_MASK will come up later in future patches, I hope.
Context not available.
	#endif	#endif

	CTASSERT(BLIST_BMAP_RADIX % BLIST_META_RADIX == 0);	CTASSERT(BLIST_BMAP_RADIX % BLIST_META_RADIX == 0);
		#define BLIST_BMAP_MASK (BLIST_BMAP_RADIX - 1)
	#define BLIST_META_MASK (BLIST_META_RADIX - 1)	#define BLIST_META_MASK (BLIST_META_RADIX - 1)

	/*	/*
		kibUnsubmitted Not Done Inline Actions mask is signed and -mask can overflow. This code depends on the presence of the -fwrapv option on the compiler invocation (which we do for kernel). I do not propose to change the fragment, only note that compiler authors do not aim to make a useful tool. kib: mask is signed and -mask can overflow. This code depends on the presence of the -fwrapv option…
Context not available.
	* time is proportional to log2(count) + bitpos time.	* time is proportional to log2(count) + bitpos time.
	*/	*/
	static daddr_t	static daddr_t
	blst_leaf_alloc(blmeta_t *scan, daddr_t blk, int count, daddr_t cursor)	blst_leaf_alloc(blmeta_t *scan, daddr_t blk, int count)
	{	{
	u_daddr_t mask;	u_daddr_t mask;
	int count1, lo, num_shifts, range1, range_ext;	int count1, hi, lo, num_shifts, range1, range_ext;

	if (count == BLIST_BMAP_RADIX) {
	/*
	* Optimize allocation of BLIST_BMAP_RADIX bits. If this wasn't
	* a special case, then forming the final value of 'mask' below
	* would require special handling to avoid an invalid left shift
	* when count equals the number of bits in mask.
	*/
	if (~scan->u.bmu_bitmap != 0) {
	scan->bm_bighint = BLIST_BMAP_RADIX - 1;
	return (SWAPBLK_NONE);
	}
	if (cursor != blk)
	return (SWAPBLK_NONE);
	scan->u.bmu_bitmap = 0;
	scan->bm_bighint = 0;
	return (blk);
	}
	range1 = 0;	range1 = 0;
	count1 = count - 1;	count1 = count - 1;
	num_shifts = fls(count1);	num_shifts = fls(count1);
	mask = scan->u.bmu_bitmap;	mask = scan->u.bmu_bitmap;
	while (mask != 0 && num_shifts > 0) {	while ((-mask & ~mask) != 0 && num_shifts > 0) {
	/*	/*
	* If bit i is set in mask, then bits in [i, i+range1] are set	* If bit i is set in mask, then bits in [i, i+range1] are set
	* in scan->u.bmu_bitmap. The value of range1 is equal to	* in scan->u.bmu_bitmap. The value of range1 is equal to
Context not available.
	* while preserving these invariants. The updates to mask leave	* while preserving these invariants. The updates to mask leave
	* fewer bits set, but each bit that remains set represents a	* fewer bits set, but each bit that remains set represents a
	* longer string of consecutive bits set in scan->u.bmu_bitmap.	* longer string of consecutive bits set in scan->u.bmu_bitmap.
		* If more updates to mask cannot clear more bits, because mask
		* is partitioned with all 0 bits preceding all 1 bits, the loop
		* terminates immediately.
	*/	*/
	num_shifts--;	num_shifts--;
	range_ext = range1 + ((count1 >> num_shifts) & 1);	range_ext = range1 + ((count1 >> num_shifts) & 1);
	mask &= mask >> range_ext;	/*
		* mask is a signed quantity for the shift because when it is
		* shifted right, the sign bit should copied; when the last
		* block of the leaf is free, pretend, for a while, that all the
		* blocks that follow it are also free.
		*/
		mask &= (daddr_t)mask >> range_ext;
	range1 += range_ext;	range1 += range_ext;
	}	}
	if (mask == 0) {	if (mask == 0) {
	/*	/*
	* Update bighint. There is no allocation bigger than range1	* Update bighint. There is no allocation bigger than range1
	* available in this leaf.	* starting in this leaf.
	*/	*/
	scan->bm_bighint = range1;	scan->bm_bighint = range1;
	return (SWAPBLK_NONE);	return (SWAPBLK_NONE);
	}	}

	/*	/*
	* Discard any candidates that appear before the cursor.	* Discard any candidates that appear before blk.
	*/	*/
	lo = cursor - blk;	mask &= (daddr_t)-1 << (blk & BLIST_BMAP_RADIX);
	mask &= ~(u_daddr_t)0 << lo;

	if (mask == 0)	if (mask == 0)
	return (SWAPBLK_NONE);	return (SWAPBLK_NONE);

Context not available.
	*/	*/
	mask &= -mask;	mask &= -mask;
	lo = bitpos(mask);	lo = bitpos(mask);
		hi = lo + count;
		if (hi > BLIST_BMAP_RADIX) {
		/*
		* An allocation within this leaf is impossible, so a successful
		* allocation depends on the next leaf providing some of the blocks.
		*/
		if (((blk / BLIST_BMAP_RADIX + 1) & BLIST_META_MASK) == 0) {
		/*
		* The next leaf has a different meta-node parent, so it
		* is not necessarily initialized. Update bighint,
		* comparing the range found at the end of mask to the
		* largest earlier range that could have been made to
		* vanish in the initial processing of mask.
		*/
		scan->bm_bighint = imax(BLIST_BMAP_RADIX - lo, range1);
		return (SWAPBLK_NONE);
		}
		hi -= BLIST_BMAP_RADIX;
		if (((scan[1].u.bmu_bitmap + 1) & ~((daddr_t)-1 << hi)) != 0) {
		/*
		* The next leaf doesn't have enough free blocks at the
		* beginning to complete the spanning allocation. The
		* hint cannot be updated, because same allocation
		* request could be satisfied later, by this leaf, if
		* the state of the next leaf changes, and without any
		* changes to this leaf.
		*/
		return (SWAPBLK_NONE);
		}
		/*
		* Clear the first 'hi' bits in the next leaf, allocating them.
		*/
		scan[1].u.bmu_bitmap &= (daddr_t)-1 << hi;

		hi = BLIST_BMAP_RADIX;
		}

	/*	/*
	* Set in mask exactly the bits being allocated, and clear them from	* Set the bits of mask at position 'lo' and higher.
	* the set of available bits.	*/
		mask = -mask;

		if (hi == BLIST_BMAP_RADIX) {
		/*
		* Update bighint. There is no allocation bigger than range1
		* available in this leaf after this allocation completes.
		*/
		scan->bm_bighint = range1;
		} else {
		/*
		* Clear the bits of mask at position 'hi' and higher.
		*/
		mask &= (u_daddr_t)-1 >> (BLIST_BMAP_RADIX - hi);
		}

		/*
		* Clear the allocated bits from this leaf.
	*/	*/
	mask = (mask << count) - mask;
	scan->u.bmu_bitmap &= ~mask;	scan->u.bmu_bitmap &= ~mask;
	return (blk + lo);	return (blk + lo);
	}	}
Context not available.
	int child;	int child;
	bool scan_from_start;	bool scan_from_start;

	blk = cursor & -radix;
	if (radix == BLIST_BMAP_RADIX)	if (radix == BLIST_BMAP_RADIX)
	return (blst_leaf_alloc(scan, blk, count, cursor));	return (blst_leaf_alloc(scan, cursor, count));
	if (scan->u.bmu_avail < count) {	if (scan->u.bmu_avail < count) {
	/*	/*
	* The meta node's hint must be too large if the allocation	* The meta node's hint must be too large if the allocation
Context not available.
	scan->bm_bighint = scan->u.bmu_avail;	scan->bm_bighint = scan->u.bmu_avail;
	return (SWAPBLK_NONE);	return (SWAPBLK_NONE);
	}	}
		blk = cursor & -radix;
	skip = radix_to_skip(radix);	skip = radix_to_skip(radix);
	next_skip = skip / BLIST_META_RADIX;	next_skip = skip / BLIST_META_RADIX;

Context not available.
	* \_________/\__/	* \_________/\__/
	* v n	* v n
	*/	*/
	int n = blk & (BLIST_BMAP_RADIX - 1);	int n = blk & BLIST_BMAP_MASK;
	u_daddr_t mask;	u_daddr_t mask;

	mask = ((u_daddr_t)-1 << n) &	mask = ((u_daddr_t)-1 << n) &
Context not available.
	static daddr_t	static daddr_t
	blst_leaf_fill(blmeta_t *scan, daddr_t blk, int count)	blst_leaf_fill(blmeta_t *scan, daddr_t blk, int count)
	{	{
	int n = blk & (BLIST_BMAP_RADIX - 1);	int n = blk & BLIST_BMAP_MASK;
	daddr_t nblks;	daddr_t nblks;
	u_daddr_t mask;	u_daddr_t mask;

Context not available.

ixl(4)/ixlv(4): Revert m_collapse() in ixl_xmit() to m_defrag().ClosedPublicActions