Re: Question on ptr_ring linux header

["Michael S. Tsirkin" <mst@redhat.com>]

On Fri, Feb 01, 2019 at 04:12:46PM +0800, fei phung wrote:
> > I am not sure what does assignment of pointers mean in this context.
> > ptr_ring is designed for a single producer and a single consumer.  For
> > why it works see explanation about data dependencies in
> > Documentation/memory-barriers.txt.  You will have to be more specific
> > about the data race that you see if you expect more specific answers.
> 
> Hi,
> 
> ptr_ring_produce_any(sc->recv[chnl]->msgs,
> &item_recv_push[item_recv_push_index])  needs to have
> non-NULL pointer assigned for item_recv_push[item_recv_push_index] , right ?

No that's irrelevant. If your item_recv_push_index isn't getting
out of bounds then &item_recv_push[item_recv_push_index]
won't be NULL and that's all ptr_ring cares about.


> Note:
> ptr_ring_produce_any() occurs in interrupt handler, while
> ptr_ring_consume_any() occurs in thread.
> 
> https://gist.github.com/promach/7716ee8addcaa33fda140d74d1ad94d6/cdc6599b8313e265bdfb073a65a124e1ba3303a2#file-riffa_driver_ptr_ring-c-L306-L320
> 
>         // TX (PC receive) scatter gather buffer is read.
>         if (vect & (1<<((5*i)+1))) {
>             recv = 1;
> 
>             item_recv_push[item_recv_push_index].val1 = EVENT_SG_BUF_READ;
>             item_recv_push[item_recv_push_index].val2 = 0;
> 
>             // Keep track so the thread can handle this.
>             if (ptr_ring_produce_any(sc->recv[chnl]->msgs,
> &item_recv_push[item_recv_push_index])) {
>                 printk(KERN_ERR "riffa: fpga:%d chnl:%d, recv sg buf
> read msg queue full\n", sc->id, chnl);
>             }
>             DEBUG_MSG(KERN_INFO "riffa: fpga:%d chnl:%d, recv sg buf
> read\n", sc->id, chnl);
> 
>             item_recv_push_index++;
>         }
> 
> 
> The kernel log points me to ptr_ring_consume_any(). So, this is
> definitely data race issue
> with my own ptr_ring interfacing code.
> 
> besides, I am also getting a reference to zero-length ring for the
> kernel dmesg log.
> I am not sure how this is related to the data race though.
> 
> The kernel log points to
> https://github.com/torvalds/linux/blame/master/include/linux/ptr_ring.h#L175
> (click open git blame on this line)
> 
> Regards,
> Phung


Sorry I'm not sure what you are trying to say here. If I had to
guess I'd say the way you play with indices is probably racy
so you are producing an invalid index.


-- 
MST