Skip to content

L
LHC Instability Trigger Distribution LIST
Commit 0a972764 authored by Federico Vaga's avatar Federico Vaga
Browse files
Options

wrtd:[incompat] remove hash table from RT application 

    "premature optimization is the root of all evil"

The hash implementation was buggy. Difficult to understand because it
includes also a memory allocator.

My humble opinion is that we do not need such optimization for a very
limited entries set (128). I replaced the hash table with an ordered by
trigger-id array. Ordered because with an optimization you can use
a binary search to retrieve in few step the correct trigger.

NOTE: a draft of the binary search algorithm is there but not tested,
that's way is inactive code.
Signed-off-by: Federico Vaga's avatarFederico Vaga <federico.vaga@cern.ch>


NOTE
This commit has been created by `git subtree` on the Mock Turtle repository
on tag mock-turtle-2.0

This commit will not compile
parent 59e38928
Expand all Hide whitespace changes
Inline Side-by-side
Showing with 295 additions and 448 deletions
include/wrtd-common.h
View file @ 0a972764
......@@ -87,7 +87,7 @@
#define TDC_TRIGGER_COALESCE_LIMIT 5
#define FD_NUM_CHANNELS 4
#define FD_HASH_ENTRIES 128
#define FD_HASH_ENTRIES 64
#define FD_MAX_QUEUE_PULSES 16
......
lib/libwrtd-output.c
View file @ 0a972764
......@@ -63,7 +63,7 @@ static int __wrtd_out_trig_get(struct wrtd_desc *wrtd, uint32_t output,
struct wrnc_msg *msg,
struct wrtd_output_trigger_state *trigger)
{
uint32_t id = 0, seq = 0, is_conditional, entry_ok, state;
uint32_t id = 0, seq = 0, entry_ok, state, next_trig;
uint32_t latency_worst, latency_avg_nsamples, latency_avg_sum;
int err;
......@@ -82,13 +82,12 @@ static int __wrtd_out_trig_get(struct wrtd_desc *wrtd, uint32_t output,
}
wrnc_msg_uint32(msg, &entry_ok);
wrnc_msg_uint32(msg, &trigger->is_conditional);
if (!entry_ok) {
errno = EWRTD_INVALID_ANSWER_HASH;
return -1;
}
wrnc_msg_int32(msg, &trigger->is_conditional);
trigger->handle.channel = output;
wrnc_msg_uint32(msg, (uint32_t *) &trigger->handle.ptr_trig);
wrnc_msg_uint32(msg, (uint32_t *) &trigger->handle.ptr_cond);
......@@ -104,6 +103,10 @@ static int __wrtd_out_trig_get(struct wrtd_desc *wrtd, uint32_t output,
if (trigger->is_conditional)
wrtd_msg_trig_id(msg, &trigger->condition);
/* Get pointer to the next trigger declared in the RT application */
wrnc_msg_uint32(msg, &next_trig);
trigger->private_data = (void *) next_trig;
if (latency_avg_nsamples == 0)
trigger->latency_average_us = 0;
else
......@@ -123,49 +126,6 @@ static int __wrtd_out_trig_get(struct wrtd_desc *wrtd, uint32_t output,
}
/**
* Retreives a trigger entry specified by a position in the hash table
* (bucket/index number) or by its handle (if handle parameter is not null)
* @param[in] dev device token
* @param[in] output index (0-based) of output channel
* @param[in] bucket bucket in the hash table to read
* @param[in] index specifies which entry in the selected bucket will be read
* @param[in] handle handle of the trigger to read (if null, takes bucket/index)
* @param[out] trigger retrieved trigger entry
*/
static int wrtd_out_trig_get(struct wrtd_node *dev, unsigned int output,
unsigned int bucket, unsigned int index,
struct wrtd_trigger_handle *handle,
struct wrtd_output_trigger_state *trigger)
{
struct wrtd_desc *wrtd = (struct wrtd_desc *)dev;
struct wrnc_msg msg = wrnc_msg_init (16);
uint32_t seq = 0, id ;
uint32_t ptr = 0;
if (output > FD_NUM_CHANNELS) {
errno = EWRTD_INVALID_CHANNEL;
return -1;
}
/* Build the message */
id = WRTD_CMD_FD_READ_HASH;
wrnc_msg_header (&msg, &id, &seq);
wrnc_msg_uint32 (&msg, &bucket);
wrnc_msg_uint32 (&msg, &index);
wrnc_msg_uint32 (&msg, &output);
if(handle)
ptr = handle->ptr_cond ? handle->ptr_cond : handle->ptr_trig;
wrnc_msg_uint32 (&msg, &ptr);
/* Send the message and get answer */
return __wrtd_out_trig_get(wrtd, output, &msg, trigger);
}
/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */
/* * * * * * * * * * PROTOTYPEs IMPLEMENTATION * * * * * * * * * */
/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */
......@@ -403,30 +363,26 @@ int wrtd_out_trig_get_all(struct wrtd_node *dev, unsigned int output,
struct wrtd_output_trigger_state *triggers,
int max_count)
{
int err, bucket, count = 0, index;
int err, count = 0;
struct wrtd_trigger_handle handle = {0, 0, output};
if (output > FD_NUM_CHANNELS) {
errno = EWRTD_INVALID_CHANNEL;
return -1;
}
#define WRTD_HASH_ENTRY_OK 0
#define WRTD_HASH_ENTRY_CONDITIONAL 1
#define WRTD_HASH_ENTRY_LAST 2
for (bucket = 0; bucket < FD_HASH_ENTRIES; bucket++) {
index = 0;
while (count < max_count) {
err = wrtd_out_trig_get(dev, output, bucket, index,
NULL, &triggers[count]);
if (err && errno == EWRTD_INVALID_ANSWER_HASH)
break;
else if (err)
return -1;
count++;
index++;
}
}
do {
triggers[count].private_data = NULL;
err = wrtd_out_trig_state_get_by_handle(dev, &handle,
&triggers[count]);
/* Save pointer to the next trigger in the output application */
handle.ptr_trig = (uint32_t) triggers[count].private_data;
count++;
} while(!err && handle.ptr_trig && count < max_count);
/* Do not count trigger with error */
if (err)
count--;
return count;
}
......@@ -436,20 +392,22 @@ int wrtd_out_trig_get_all(struct wrtd_node *dev, unsigned int output,
* It returns a trigger state from a given handle.
* @param[in] dev pointer to open node device.
* @param[in] handle trigger where act on
* @param[out] state trigger status
* @param[out] trigger trigger status
* @return 0 on success, -1 on error and errno is set appropriately
*/
int wrtd_out_trig_state_get_by_handle(struct wrtd_node *dev,
struct wrtd_trigger_handle *handle,
struct wrtd_output_trigger_state *state)
struct wrtd_output_trigger_state *trigger)
{
int err;
struct wrtd_desc *wrtd = (struct wrtd_desc *)dev;
struct wrnc_msg msg = wrnc_msg_init(6);
uint32_t seq = 0, id = WRTD_CMD_FD_READ_HASH;
err = wrtd_out_trig_get(dev, handle->channel, 0, 0, handle, state);
if (err)
return -1;
wrnc_msg_header(&msg, &id, &seq);
wrnc_msg_int32(&msg, &handle->channel);
wrnc_msg_uint32(&msg, &handle->ptr_trig);
return 0;
return __wrtd_out_trig_get(wrtd, handle->channel, &msg, trigger);
}
......@@ -477,7 +435,7 @@ int wrtd_out_trig_state_get_by_id(struct wrtd_node *dev,
wrtd_msg_trig_id(&msg, tid);
ret = __wrtd_out_trig_get(wrtd, i, &msg, trigger);
if (ret == WRTD_HASH_ENTRY_OK)
if (!ret)
return 0;
}
......@@ -689,7 +647,7 @@ int wrtd_out_trigger_mode_set(struct wrtd_node *dev,
enum wrtd_trigger_mode mode)
{
struct wrnc_msg msg = wrnc_msg_init(4);
uint32_t seq = 0, id;
uint32_t seq = 0, id, m = mode;
if (output > FD_NUM_CHANNELS) {
errno = EWRTD_INVALID_CHANNEL;
......@@ -700,7 +658,7 @@ int wrtd_out_trigger_mode_set(struct wrtd_node *dev,
id = WRTD_CMD_FD_CHAN_SET_MODE;
wrnc_msg_header(&msg, &id, &seq);
wrnc_msg_uint32(&msg, &output);
wrnc_msg_uint32(&msg, &mode);
wrnc_msg_uint32(&msg, &m);
return wrtd_out_trivial_request (dev, &msg);
}
......
lib/libwrtd.h
View file @ 0a972764
......@@ -111,6 +111,7 @@ struct wrtd_output_trigger_state {
int latency_average_us; /**< Average latency in micro-seconds */
uint32_t executed_pulses; /**< Number of executed pulses */
uint32_t missed_pulses; /**< Number of missed pulses */
void *private_data; /**< private pointer used by the library */
};
/**
......
rt/fd/Makefile
View file @ 0a972764
OBJS = rt-fd.o hash.o
OBJS = rt-fd.o
OUTPUT = rt-fd
include ../common/wrnode.mk
rt/fd/hash.c deleted 100644 → 0
View file @ 59e38928
/*
* This work is part of the White Rabbit Node Core project.
*
* Copyright (C) 2013-2014 CERN (www.cern.ch)
* Author: Tomasz Wlostowski <tomasz.wlostowski@cern.ch>
*
* Released according to the GNU GPL, version 2 or any later version.
*/
/*.
* LHC Instability Trigger Distribution (LIST) Firmware.
*
* hash.c: Trigger Output hash table implementation
*/
#include <string.h>
#include "hash.h"
/* constant sized block memory pool, providing constant time allocation and freeing */
struct blockpool {
int blk_size;
int blk_count;
uint16_t *fq;
int fq_head, fq_tail, fq_count;
void *pool;
};
/* a helper macro to declare a blockpool of given data type and reserve the memory
for storage */
#define DECLARE_BLOCK_POOL(name, entry_type, entries_count ) \
static uint32_t name##_pool_mem [ (entries_count) * (sizeof (entry_type) + 3) / 4 ]; \
static uint16_t name##_pool_queue [ (entries_count ) + 1]; \
static struct blockpool name##_blockpool;
/* Hash table entry block pool */
DECLARE_BLOCK_POOL ( hash, struct lrt_hash_entry, FD_HASH_ENTRIES )
/* Separate pool for output rule allocation (to save memory) */
DECLARE_BLOCK_POOL ( rules, struct lrt_output_rule, FD_HASH_ENTRIES )
/* The hash table itself */
struct lrt_hash_entry* htab [ FD_HASH_ENTRIES ];
/* Initializes an empty blockpool structure */
void blockpool_init (struct blockpool *bp, int blk_size, int blk_count, void *data, void *queue)
{
int i;
bp->pool = data;
bp->fq = queue;
bp->blk_count = blk_count;
bp->blk_size = blk_size;
bp->fq_head = blk_count;
bp->fq_tail = 0;
bp->fq_count = blk_count;
/* Fill in the free queue with all available blocks */
for(i=0;i<bp->blk_count;i++)
bp->fq[i] = i;
}
/* Returns a new block from the blockpool bp (or null if none available) */
void *blockpool_alloc(struct blockpool *bp)
{
if (bp->fq_head == bp->fq_tail)
return NULL;
void *blk = bp->pool + bp->blk_size * (int)bp->fq [bp->fq_tail];
if(bp->fq_tail == bp->blk_count)
bp->fq_tail = 0;
else
bp->fq_tail++;
bp->fq_count--;
return blk;
}
/* Releases a block back into the blockpool */
void blockpool_free( struct blockpool *bp, void *ptr )
{
int blk_id = (ptr - bp->pool) / bp->blk_size;
bp->fq [bp->fq_head] = blk_id;
if(bp->fq_head == bp->blk_count)
bp->fq_head = 0;
else
bp->fq_head++;
bp->fq_count++;
}
/* Initializes the hash table & blockpools */
void hash_init()
{
blockpool_init (&hash_blockpool, sizeof(struct lrt_hash_entry), FD_HASH_ENTRIES, hash_pool_mem, hash_pool_queue);
blockpool_init (&rules_blockpool, sizeof(struct lrt_output_rule), FD_HASH_ENTRIES, rules_pool_mem, rules_pool_queue);
memset (&htab, 0, sizeof (htab));
}
/* Creates an empty entry in the hash bucket (pos) */
struct lrt_hash_entry *hash_alloc( int pos )
{
struct lrt_hash_entry *prev = NULL, *current = htab[pos];
/* Is there already something @ pos? Iterate to the end
of the linked list if so */
while (current)
{
prev = current;
current = current->next;
}
/* Allocate memory for new entry */
current = blockpool_alloc(&hash_blockpool);
memset(current->ocfg, 0, FD_NUM_CHANNELS * sizeof (struct lrt_output_rule *));
/* And link it to the bucket/list */
if(!prev)
htab[pos] = current;
else
prev->next = current;
current->next = NULL;
return current;
}
/* Adds a new output rule for given trigger ID and output pair or overwrites an existing one. */
struct lrt_hash_entry *hash_add ( struct wrtd_trig_id *id, int output, struct lrt_output_rule *rule )
{
int pos;
struct lrt_hash_entry *ent = hash_search( id, &pos );
if(!ent)
ent = hash_alloc( pos );
if(!ent)
return NULL;
rule->latency_worst = 0;
rule->latency_avg_sum = 0 ;
rule->latency_avg_nsamples = 0 ;
rule->hits = 0;
rule->misses = 0;
ent->id = *id;
struct lrt_output_rule *orule = ent->ocfg[output];
if(orule == NULL) /* no memory allocated yet */
{
orule = blockpool_alloc(&rules_blockpool);
if(!orule)
return NULL; // should never happen as hashes are of same size
}
memcpy(orule, rule, sizeof(struct lrt_output_rule));
ent->ocfg[output] = orule;
return ent;
}
/* Removes an output rule from the hash. */
int hash_remove ( struct lrt_hash_entry *ent, int output )
{
int bucket, i;
struct lrt_hash_entry *e;
/* release the rule */
blockpool_free(&rules_blockpool, ent->ocfg[output]);
ent->ocfg[output] = NULL;
for(i = 0; i < FD_NUM_CHANNELS; i++)
if(ent->ocfg[i]) /* Same ID is assigned to another output? */
return 0;
/* Identify the bucket where the entity is supposed to be stored */
bucket = hash_func(&ent->id);
/* Removing the given entity from the hash table */
e = htab[bucket];
if (e == ent)
goto out;
for (; e != NULL; e = e->next) {
if (e->next == ent)
goto out;
}
/* If we reach this point something is wrong in the htab */
pp_printf("%s: entity is not in the hash table\n");
return -1;
out:
blockpool_free(&hash_blockpool, ent);
e->next = ent->next;
return 0;
}
/* Returns a pos-th hash entry in given hash bucket. */
struct lrt_hash_entry *hash_get_entry (int bucket, int pos)
{
int i;
if (bucket < 0 || bucket >= FD_HASH_ENTRIES)
return NULL;
struct lrt_hash_entry *l = htab[bucket];
for (i = 0; l != NULL && i < pos; i++, l = l->next)
if (!l)
return NULL;
return l;
}
/* It counts the number of triggers assigned to a given channel */
int hash_count_rules(int ch)
{
struct lrt_hash_entry *l, *ln;
int count, i, k;
for (i = 0; i < FD_HASH_ENTRIES; i++) {
l = htab[i];
for (k = 0; ln; ln = ln->next) {
if (ln->ocfg[ch])
count++;
}
}
return count;
}
/* Returns the number of free hash entries */
int hash_free_count()
{
return hash_blockpool.fq_count ;
}
rt/fd/hash.h deleted 100644 → 0
View file @ 59e38928
/*
* This work is part of the White Rabbit Node Core project.
*
* Copyright (C) 2013-2014 CERN (www.cern.ch)
* Author: Tomasz Wlostowski <tomasz.wlostowski@cern.ch>
*
* Released according to the GNU GPL, version 2 or any later version.
*/
/*
* LHC Instability Trigger Distribution (LIST) Firmware
*
* hash.h: Trigger Output hash table
*/
#ifndef __LIST_HASH_H
#define __LIST_HASH_H
#include "rt.h"
#include "wrtd-common.h"
/* The hash is a fast search structure O(1) for matching incoming trigger messages against their IDs
with triggering rules associated with each output of the node. It consists of FD_NUM_CHANNEL buckets.
Each bucket may contain a single ID entry or a single-linked list in case of a collision. */
/* Rule defining the behaviour of a trigger output upon reception of a trigger message
with matching ID */
struct lrt_output_rule {
/* Delay to add to the timestamp enclosed within the trigger message */
uint32_t delay_cycles;
uint16_t delay_frac;
/* State of the rule (empty, disabled, conditional action, condition, etc.) */
uint16_t state;
/* Pointer to conditional action. Used for rules that define triggering conditions. */
struct lrt_output_rule *cond_ptr;
/* Worst-case latency (in 8ns ticks)*/
uint32_t latency_worst;
/* Average latency accumulator and number of samples */
uint32_t latency_avg_sum;
uint32_t latency_avg_nsamples;
/* Number of times the rule has successfully produced a pulse */
int hits;
/* Number of times the rule has missed a pulse (for any reason) */
int misses;
};
/* Bucket of the hash table */
struct lrt_hash_entry {
/* Trigger ID to match against */
struct wrtd_trig_id id;
/* Rules for each trigger output */
struct lrt_output_rule *ocfg [FD_NUM_CHANNELS];
/* Linked list pointer in case of hash collision */
struct lrt_hash_entry *next;
};
extern struct lrt_hash_entry* htab [FD_HASH_ENTRIES];
void hash_init ();
struct lrt_hash_entry *hash_add (struct wrtd_trig_id *id, int output, struct lrt_output_rule *rule);
int hash_remove (struct lrt_hash_entry *ent, int output);
int hash_free_count ();
int hash_count_rules(int ch);
struct lrt_hash_entry *hash_get_entry (int bucket, int pos);
/* Hash function, returing the hash table index corresponding to a given trigger ID */
static inline int hash_func( struct wrtd_trig_id *id )
{
int h = 0;
h += id->system * 10291;
h += id->source_port * 10017;
h += id->trigger * 3111;
return h & (FD_HASH_ENTRIES - 1); // hash table size must be a power of 2
}
/* Searches for hash entry matching given trigger id and returns a non-NULL pointer if found.
If (pos) is not null, the index in the hash table entry is stored there */
static inline struct lrt_hash_entry *hash_search( struct wrtd_trig_id *id, int *pos )
{
int p = hash_func( id );
struct lrt_hash_entry *ent = htab [p];
if(pos)
*pos = p;
while (ent)
{
if( ent->id.system == id->system &&
ent->id.source_port == id->source_port &&
ent->id.trigger == id->trigger)
return ent;
ent = ent->next;
}
return NULL;
};
#endif
rt/fd/rt-fd.c
View file @ 0a972764
This diff is collapsed.
tools/wrtd-out-config.c
View file @ 0a972764
......@@ -417,16 +417,20 @@ static int wrtd_cmd_trig_unassign(struct wrtd_node *wrtd, int output,
int argc, char *argv[])
{
struct wrtd_output_trigger_state trig;
int index, err;
struct wrtd_trig_id tid;
int err;
if (argc != 1 || argv[0] == NULL) {
fprintf(stderr, "Missing arguments: unassign <trigger index>\n");
fprintf(stderr, "Missing arguments: unassign <trigger ID>\n");
return -1;
}
index = atoi(argv[0]);
err = parse_trigger_id(argv[0], &tid);
if (err)
return err;
/* Get a trigger */
err = wrtd_out_trig_state_get_by_index(wrtd, index, output, &trig);
err = wrtd_out_trig_state_get_by_id(wrtd, &tid, &trig);
if (err)
return err;
......@@ -525,8 +529,6 @@ static int wrtd_cmd_trig_find(struct wrtd_node *wrtd, int output,
static void help()
{
int i;
fprintf(stderr, "\n\n");
fprintf(stderr, "wrtd-out-config -D 0x<hex-number> -C <string> -c <number> [cmd-options]\n\n");
fprintf(stderr, "It configures an output channel on a White Rabbit Trigger-Distribution node\n\n");
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment