The portable-lib from opencoff

t_ringbuf.c: Suspicious useless variable j in upause() function

Lines 188 to 199 in 9b47975

    
           // Simple busywait. 
        
           // usleep() is not granular enough 
        
           static void 
        
           upause(int n) 
        
           { 
        
               int i; 
        
               volatile uint64_t j = sys_cpu_timestamp(); 
        
               for (i = 0; i < n; ++i) { 
        
                   j += i; 
        
                   rte_pause(); 
        
               } 
        
           }

Maybe simply asm("nop") is better:

     for (i = 0; i < n; ++i) { 
         asm("nop");
         rte_pause(); 
     }

Bug: loop counter in ringbuf.h function __store_ring() and __load_ring() is wrong

Both __store_ring() and __load_ring() have a same bug.

function __store_ring:

portable-lib/inc/fast/ringbuf.h

Lines 237 to 277 in 10c3286

    
           __store_ring(struct rte_ring *r, uint_fast32_t head, void * const* obj, unsigned n) 
        
           { 
        
               unsigned i; 
        
               unsigned loops = n & 0x3; 
        
               unsigned idx   = head & r->mask; 
        
               // If we know we won't wrap around, we unroll 4 times 
        
               if (likely((idx + n) <= r->mask)) { 
        
                   for (i = 0; i < loops; i += 4, idx += 4) { 
        
                           r->ring[idx+0] = obj[i+0]; 
        
                           r->ring[idx+1] = obj[i+1]; 
        
                           r->ring[idx+2] = obj[i+2]; 
        
                           r->ring[idx+3] = obj[i+3]; 
        
                   } 
        
                   // mop up remainder 
        
                   switch (n & 0x3) { 
        
                       case 3: 
        
                           r->ring[idx+0] = obj[i+0]; 
        
                           r->ring[idx+1] = obj[i+1]; 
        
                           r->ring[idx+2] = obj[i+2]; 
        
                           break; 
        
                       case 2: 
        
                           r->ring[idx+0] = obj[i+0]; 
        
                           r->ring[idx+1] = obj[i+1]; 
        
                           break; 
        
                       case 1: 
        
                           r->ring[idx+0] = obj[i+0]; 
        
                           break; 
        
                   } 
        
               } else { 
        
                   const uint32_t mask = r->mask; 
        
                   for (i = 0; i < n; i++, idx++) { 
        
                       r->ring[idx & mask] = obj[i]; 
        
                   } 
        
               } 
        
           }

Bug: When n=1, function __store_ring(ring, head, elements, n) actually writes 4 elements into the ring.
Reason: loop should = n>>2 not n&0x03
Buggy code is at line 240:

portable-lib/inc/fast/ringbuf.h

Line 240 in 10c3286

unsigned loops = n & 0x3;
Solution:
The correct logic should be:

unsigned loop = n & ((unsigned) ~0x3U);

opencoff / portable-lib Goto Github PK

portable-lib's People

Contributors

Stargazers

Watchers

Forkers

portable-lib's Issues

t_ringbuf.c: Suspicious useless variable j in upause() function

Bug: loop counter in ringbuf.h function __store_ring() and __load_ring() is wrong

Recommend Projects

React

Vue.js

Typescript

TensorFlow

Django

Laravel

D3

Recommend Topics

javascript

web

server

Machine learning

Visualization

Game

Recommend Org

Facebook

Microsoft

Google

Alibaba

D3

Tencent

	// Simple busywait.
	// usleep() is not granular enough
	static void
	upause(int n)
	{
	int i;
	volatile uint64_t j = sys_cpu_timestamp();
	for (i = 0; i < n; ++i) {
	j += i;
	rte_pause();
	}
	}

	__store_ring(struct rte_ring r, uint_fast32_t head, void const* obj, unsigned n)
	{
	unsigned i;
	unsigned loops = n & 0x3;
	unsigned idx = head & r->mask;


	// If we know we won't wrap around, we unroll 4 times
	if (likely((idx + n) <= r->mask)) {
	for (i = 0; i < loops; i += 4, idx += 4) {
	r->ring[idx+0] = obj[i+0];
	r->ring[idx+1] = obj[i+1];
	r->ring[idx+2] = obj[i+2];
	r->ring[idx+3] = obj[i+3];
	}

	// mop up remainder
	switch (n & 0x3) {
	case 3:
	r->ring[idx+0] = obj[i+0];
	r->ring[idx+1] = obj[i+1];
	r->ring[idx+2] = obj[i+2];
	break;

	case 2:
	r->ring[idx+0] = obj[i+0];
	r->ring[idx+1] = obj[i+1];
	break;

	case 1:
	r->ring[idx+0] = obj[i+0];
	break;
	}
	} else {
	const uint32_t mask = r->mask;

	for (i = 0; i < n; i++, idx++) {
	r->ring[idx & mask] = obj[i];
	}
	}
	}