DeviceQueue: keeping track of buffer ids should work
authorRoni Häcki <roni.haecki@inf.ethz.ch>
Fri, 12 Aug 2016 11:17:39 +0000 (13:17 +0200)
committerRoni Häcki <roni.haecki@inf.ethz.ch>
Fri, 12 Aug 2016 11:17:39 +0000 (13:17 +0200)
Signed-off-by: Roni Häcki <roni.haecki@inf.ethz.ch>

include/devif/queue_interface.h
lib/devif/queue_interface.c
lib/devif/region.c
lib/devif/region.h
lib/devif/region_pool.c

index fd129f7..6ad664d 100644 (file)
@@ -42,7 +42,8 @@ typedef errval_t (*devq_enqueue_t)(struct devq *q, regionid_t region_id,
                                    uint64_t misc_flags, bufferid_t* buffer_id);
 typedef errval_t (*devq_dequeue_t)(struct devq *q, regionid_t* region_id,
                                    lpaddr_t* base, size_t* length,
-                                   bufferid_t buffer_id, uint64_t misc_flags);
+                                   bufferid_t* buffer_id, uint64_t* misc_flags);
+
 typedef errval_t (*devq_notify_t) (struct devq *q);
 typedef errval_t (*devq_register_t)(struct devq *q, struct capref cap,
                                    regionid_t* region_id);
index e34fcef..e589ab2 100644 (file)
@@ -115,6 +115,8 @@ errval_t devq_create(struct devq **q,
             USER_PANIC("Devq: unknown device type \n");
 
     }
+    
+    *q = tmp;
     // TODO initalize device 
     // TODO initalize device state
     return SYS_ERR_OK;
@@ -199,7 +201,9 @@ errval_t devq_enqueue(struct devq *q,
                       uint64_t misc_flags,
                       bufferid_t* buffer_id)
 {
+    errval_t err;
     size_t num_free = 0;
+
     if (q->tx_head >= q->tx_tail) {
        num_free = DESCQ_SIZE - (q->tx_head - q->tx_tail);
     } else {
@@ -207,11 +211,16 @@ errval_t devq_enqueue(struct devq *q,
     }
 
     if (num_free == 0) {
-        // TODO reasonable error
-        return -1;
+        return DEVQ_ERR_TX_FULL;
+    }
+
+    // Add buffer to used ones
+    err = region_pool_get_buffer_id_from_region(q->pool, region_id, base,
+                                                buffer_id);
+    if (err_is_fail(err)) {
+        return DEVQ_ERR_BUFFER_ID;
     }
 
-    *buffer_id = 0;
     q->tx[q->tx_head].region_id = region_id;
     q->tx[q->tx_head].base = base;
     q->tx[q->tx_head].length = length;
@@ -245,6 +254,7 @@ errval_t devq_dequeue(struct devq *q,
                       bufferid_t* buffer_id,
                       uint64_t* misc_flags)
 {
+    errval_t err;
     size_t num_used = 0;
     if (q->rx_head >= q->rx_tail) {
        num_used = (q->rx_head - q->rx_tail);
@@ -253,8 +263,7 @@ errval_t devq_dequeue(struct devq *q,
     }
 
     if (num_used == 0) {
-        // TODO reasonable error
-        return -1;
+        return DEVQ_ERR_RX_FULL;
     }
 
     *region_id = q->rx[q->rx_head].region_id;
@@ -264,6 +273,32 @@ errval_t devq_dequeue(struct devq *q,
     *misc_flags = q->rx[q->rx_head].misc_flags;
 
     q->rx_head = q->rx_head + 1 % DESCQ_SIZE;
+
+/*a
+    // Only uncomment for testing
+    if (q->tx_head >= q->tx_tail) {
+       num_used = (q->tx_head - q->tx_tail);
+    } else {
+       num_used = (q->tx_head + DESCQ_SIZE - q->tx_tail);
+    }
+    if (num_used == 0) {
+        return DEVQ_ERR_RX_FULL;
+    }
+    *region_id = q->tx[q->tx_tail].region_id;
+    *base = q->tx[q->tx_tail].base;
+    *length = q->tx[q->tx_tail].length;
+    *buffer_id = q->tx[q->tx_tail].buffer_id;
+    *misc_flags = q->tx[q->tx_tail].misc_flags;
+
+    q->tx_tail = q->tx_tail + 1 % DESCQ_SIZE;
+*/
+    // Add buffer to free ones
+    err = region_pool_return_buffer_id_to_region(q->pool, *region_id,
+                                                 *buffer_id);
+    if (err_is_fail(err)) {
+        return DEVQ_ERR_BUFFER_ID;
+    }
+
     return SYS_ERR_OK;
 }
 
@@ -290,6 +325,8 @@ errval_t devq_register(struct devq *q,
                        regionid_t* region_id)
 {
     errval_t err;
+    DQI_DEBUG("register q=%p, cap=%p, regionid=%d \n", (void*) q, 
+              (void*) &cap, *region_id);
     err = region_pool_add_region(q->pool, cap, region_id); 
     return err;
 }
@@ -311,6 +348,8 @@ errval_t devq_deregister(struct devq *q,
 {
     errval_t err;
     err = region_pool_remove_region(q->pool, region_id, cap); 
+    DQI_DEBUG("deregister q=%p, cap=%p, regionid=%d \n", (void*) q, 
+              (void*) cap, region_id);
     return err;
 }
 
index aebcdfa..3876388 100644 (file)
@@ -46,7 +46,7 @@ errval_t region_init(struct region** region,
         return LIB_ERR_MALLOC_FAIL;
     }
 
-    tmp->region_id = region_id;
+    tmp->id = region_id;
     tmp->cap = cap;
 
     err = invoke_frame_identify(*cap, &id);
@@ -65,8 +65,8 @@ errval_t region_init(struct region** region,
 
     *region = tmp;   
     
-    DQI_DEBUG("Initialize Region size=%ld addr=%16lx num_bufs=%ld \n",
-              tmp->len, tmp->base_addr, tmp->num_buf);
+    DQI_DEBUG("Initialize Region size=%ld addr=%16lx\n",
+              tmp->len, tmp->base_addr);
 
     return SYS_ERR_OK;
 }
@@ -81,6 +81,13 @@ errval_t region_init(struct region** region,
  */
 errval_t region_destroy(struct region* region)
 {
+    for (int i = 0; i < region->max_page_id; i++) {
+        if (region->used_bufs[i] != NULL) {
+            return DEVQ_ERR_REGION_DESTROY;
+        }
+    }
+
+    free(region->used_bufs);
     free(region);
     return SYS_ERR_OK;
 }
@@ -99,18 +106,16 @@ errval_t region_get_buffer_id(struct region* region,
 {
     uint32_t page_id;
     *buffer_id = (addr - region->base_addr);
-    page_id = *buffer_id/BASE_PAGE_SIZE;
+    page_id = (*buffer_id)/BASE_PAGE_SIZE;
 
     // Test if buffer can not be in region
     if (page_id > region->max_page_id) {
-        // TODO reasonable error
-        return -1;
+        return DEVQ_ERR_BUFFER_NOT_IN_REGION;
     }
 
     // Test if buffer is already used
     if (region_buffer_id_in_use(region, *buffer_id)) {
-        // TODO reasonable error
-        return -1;
+        return DEVQ_ERR_BUFFER_ALREADY_IN_USE;
     }
 
     struct buffer* tmp = slab_alloc(&region->alloc);
@@ -121,6 +126,10 @@ errval_t region_get_buffer_id(struct region* region,
     // Empty list
     if (ele == NULL) {
         region->used_bufs[page_id] = tmp;
+
+        DQI_DEBUG("buffer region=%d bucket=%d, id=%d, addr=%16lx\n", 
+                  region->id, page_id, *buffer_id, addr);
+        return SYS_ERR_OK;
     }    
 
     // Iterate through list
@@ -130,7 +139,8 @@ errval_t region_get_buffer_id(struct region* region,
 
     ele->next = tmp;
     
-    DQI_DEBUG("buffer addr=%16lx got assigned id=%d\n", addr, *buffer_id);
+    DQI_DEBUG("buffer region=%d bucket=%d, id=%d, addr=%16lx\n", 
+               region->id, page_id, *buffer_id, addr);
     return SYS_ERR_OK;
 }
 
@@ -150,14 +160,12 @@ errval_t region_free_buffer_id(struct region* region,
     page_id = buffer_id/BASE_PAGE_SIZE;
     // Test if buffer can not be in region
     if (page_id > region->max_page_id) {
-        // TODO reasonable error
-        return -1;
+        return DEVQ_ERR_BUFFER_NOT_IN_REGION;
     }
 
     // Test if buffer is used
     if (!region_buffer_id_in_use(region, buffer_id)) {
-        // TODO reasonable error
-        return -1;
+        return DEVQ_ERR_BUFFER_NOT_IN_USE;
     }
     
     struct buffer* ele = region->used_bufs[page_id];
@@ -165,7 +173,7 @@ errval_t region_free_buffer_id(struct region* region,
     if (ele->id == buffer_id) {
         region->used_bufs[page_id] = ele->next;
         slab_free(&region->alloc, ele);
-        DQI_DEBUG("Returned buffer id=%d \n", buffer_id);
+        DQI_DEBUG("Returned buffer id=%d (first entry)\n", buffer_id);
         return SYS_ERR_OK;
     }
     
@@ -173,7 +181,8 @@ errval_t region_free_buffer_id(struct region* region,
         if (ele->next->id == buffer_id) {
             ele->next = ele->next->next;
             slab_free(&region->alloc, ele);
-            DQI_DEBUG("Returned buffer id=%d \n", buffer_id);
+            DQI_DEBUG("Returned buffer id=%d (second or higher entry) \n", 
+                      buffer_id);
             return SYS_ERR_OK;
         }
         ele = ele->next;
@@ -211,7 +220,6 @@ bool region_buffer_id_in_use(struct region* region,
         ele = ele->next;
     }
 
-    // TODO check
     DQI_DEBUG("Returned buffer id=%d \n", buffer_id);
     return false;
 }
index 76d5631..bac2b16 100644 (file)
@@ -22,7 +22,7 @@ struct buffer {
 
 struct region {
     // ID of the region
-    regionid_t region_id;
+    regionid_t id;
     // Base address of the region
     lpaddr_t base_addr;
     // Capability of the region
index 954dea7..8719efd 100644 (file)
@@ -87,7 +87,7 @@ errval_t region_pool_destroy(struct region_pool* pool)
         // There are regions left -> remove them
         for (int i = 0; i < pool->size; i++) {
             if ((void*) pool->pool[i] != NULL) {
-                err = region_pool_remove_region(pool, pool->pool[i]->region_id,
+                err = region_pool_remove_region(pool, pool->pool[i]->id,
                                                 &cap);
                 if (err_is_fail(err)){
                     printf("Region pool has regions that are still used,"
@@ -132,7 +132,7 @@ static errval_t region_pool_grow(struct region_pool* pool)
     struct region* region;
     for (int i = 0; i < pool->size; i++) {
         region = pool->pool[i];
-        uint16_t index =  region->region_id % new_size;
+        uint16_t index =  region->id % new_size;
         tmp[index] = pool->pool[i];
     }
 
@@ -187,15 +187,14 @@ errval_t region_pool_add_region(struct region_pool* pool,
     }
 
     pool->last_offset = offset;
-    // TODO size 
     err = region_init(&region,
                       pool->region_offset + pool->num_regions + offset,
                       &cap);
 
     // insert into pool
-    pool->pool[region->region_id % pool->size] = region;
-    *region_id = region->region_id;
-    DQI_DEBUG("Inserting region into pool at %d \n", region->region_id % pool->size);
+    pool->pool[region->id % pool->size] = region;
+    *region_id = region->id;
+    DQI_DEBUG("Inserting region into pool at %d \n", region->id % pool->size);
     return SYS_ERR_OK;
 }
 
@@ -212,17 +211,22 @@ errval_t region_pool_remove_region(struct region_pool* pool,
                                    regionid_t region_id,
                                    struct capref* cap)
 {
+    errval_t err;
     struct region* region;
     region = pool->pool[region_id % pool->size]; 
     if (region == NULL) {
-        // TODO reasonable error;
-        return -1;
+        return DEVQ_ERR_INVALID_REGION_ID;
     }
 
-    DQI_DEBUG("Removing slot %d \n", region_id % pool->size);
     cap = region->cap;
   
-    region_destroy(region);
+    err = region_destroy(region);
+    if (err_is_fail(err)) {
+        DQI_DEBUG("Failed to destroy region, some buffers might still be in use \n");
+        return err;
+    }
+
+    DQI_DEBUG("Removing slot %d \n", region_id % pool->size);
     pool->pool[region_id % pool->size] = NULL;
 
     pool->num_regions--;
@@ -245,8 +249,7 @@ static errval_t region_pool_get_region(struct region_pool* pool,
 {
     *region = pool->pool[region_id % pool->size];
     if (region == NULL) {
-        // TODO reasonable error;
-        return -1;
+        return DEVQ_ERR_INVALID_REGION_ID;
     }
 
     return SYS_ERR_OK;
@@ -306,7 +309,6 @@ errval_t region_pool_return_buffer_id_to_region(struct region_pool* pool,
         return err;
     }
     
-    // TODO size
     err = region_free_buffer_id(region, buffer_id);
     if (err_is_fail(err)) {
         return err;