On Tue, Jan 15, 2019 at 12:33:28PM +0800, fei phung wrote: > Hi netdev mailing list and Michael, > > I am having problem getting proper ptr_ring operation where one > ptr_ring entry (val1=2 for item_recv_pop) is missing from the void ** > queue > > Did I initialize the ring pointers (ptr_ring_init()) correctly ? > > See the following for more context: > > https://i.imgur.com/xWJOH1G.png > https://gist.github.com/promach/7716ee8addcaa33fda140d74d1ad94d6#file-riffa_driver-c-L663
Didn't read in depth but this one seems to poke at the internal producer index for its own purposes. That's not something I expected when I built ptr_ring so I don't know how well that will work. You might want to try and come up with a reasonable API for that instead. > https://gist.github.com/promach/65e9331d55a43a2815239430a28e29c6#file-circ_ring-c-L44 > Hard to say for sure but things like that inline int push_circ_queue(struct ptr_ring * buffer, struct item * item_push) { if (!ptr_ring_full_any(buffer)) // if (not full) { DEBUG_MSG(KERN_INFO "Pushing %u and %u\n", item_push->val1, item_push->val2); /* insert one item into the buffer */ ptr_ring_produce_any(buffer, item_push); are a waste, and they can be racy with multiple producers. Just call ptr_ring_produce_any and it will tell you whether it could produce. same here: inline int pop_circ_queue(struct ptr_ring * buffer, struct item * item_pop) { if (!ptr_ring_empty_any(buffer)) // if (not empty) { DEBUG_MSG(KERN_INFO "Before pop, head = %u , tail = %u\n", buffer->consumer_head, buffer->consumer_tail); /* extract one item struct containing two unsigned integers from the buffer */ *item_pop = *((struct item *)ptr_ring_consume_any(buffer)); DEBUG_MSG(KERN_INFO "val1 = %u , val2 = %u\n", item_pop->val1, item_pop->val2); DEBUG_MSG(KERN_INFO "After pop, head = %u , tail = %u\n", buffer->consumer_head, buffer->consumer_tail); return 0; } else return 1; // empty, nothing to pop from the ring } racy if there are multiple consumers. just call ptr_ring_consume_any. And it seems to leak the memory the pointer to which you have consumed - although it's possible it's freed elsewhere - I wouldn't know. > struct ptr_ring * init_circ_queue(int len) > { > struct ptr_ring * q; > > q = kzalloc(sizeof(struct ptr_ring), GFP_KERNEL); > if (q == NULL) { > DEBUG_MSG(KERN_ERR "Not enough memory to allocate ptr_ring"); > return NULL; > } > > // creates an array of length 'len' where each array location > can store a struct * item > if(ptr_ring_init(q, len, GFP_KERNEL) != 0) { > DEBUG_MSG(KERN_ERR "Not enough memory to allocate > ptr_ring array"); > return NULL; > } > > return q; > } > This part looks good. > > while ((nomsg = pop_circ_queue(sc->recv[chnl]->msgs, > &item_recv_pop))) { > prepare_to_wait(&sc->recv[chnl]->waitq, &wait, > TASK_INTERRUPTIBLE); > // Another check before we schedule. > if ((nomsg = > pop_circ_queue(sc->recv[chnl]->msgs, &item_recv_pop))) > tymeout = schedule_timeout(tymeout); > finish_wait(&sc->recv[chnl]->waitq, &wait); > if (signal_pending(current)) { > free_sg_buf(sc, sc->recv[chnl]->sg_map_0); > free_sg_buf(sc, sc->recv[chnl]->sg_map_1); > return -ERESTARTSYS; > } > if (!nomsg) > break; > if (tymeout == 0) { > printk(KERN_ERR "riffa: fpga:%d > chnl:%d, recv timed out\n", sc->id, chnl); > /*free_sg_buf(sc, sc->recv[chnl]->sg_map_0); > free_sg_buf(sc, sc->recv[chnl]->sg_map_1); > return (unsigned int)(recvd>>2);*/ > } > } > tymeout = tymeouto; > msg_type = item_recv_pop.val1; > msg = item_recv_pop.val2; > DEBUG_MSG(KERN_INFO "recv msg_type: %u\n", msg_type); > > > Regards, > Phung
