On 04/01/15 16:57, Stuart Haslam wrote:
On Wed, Apr 01, 2015 at 04:30:59PM +0300, Maxim Uvarov wrote:
Commit:
8fa64fb9fc4e43fbd9d3c226ed89229863bdb771
linux-generic: scheduler: restructured queue and pktio integration
Implement race with schedule termination and polling input queue.
This patch locks pktio while doing poll to prevent destroy linked
queue in the middle of this poll.
Admittedly I've not looked in detail at the terminate sequence after the
scheduler changes, so don't really understand what you're fixing, but
this feels like a workaround rather than a fix. Shouldn't the pktin
queue have been removed from the scheduler before it's closed? What's
the sequence that leads to the problem?
as I understand that right one thread goes to
schedule()->pktin_poll(sched_cmd->pe))
then successful do:
odp_pktio_recv(entry->s.handle, pkt_tbl, QUEUE_MULTI_MAX);
after that other thread calls terminate and bellow in the pktio_poll code:
queue_enq_multi(qentry, hdr_tbl, num_enq);
qentry here is corrupted due to it has been destroyed by other thread.
Because of qentry is linked to pktio entry we have to lock pktio entry
for that
queue to make sure that is was not modified while pktin_poll execution.
I did make check from the root to find that problem (it occurs about 1
of 10 run times).
I sent back trace some to mailing list some time ago:
ore was generated by `./test/validation/odp_pktio'.
Program terminated with signal SIGSEGV, Segmentation fault.
#0 0x0000000000411dc4 in odp_atomic_fetch_inc_u32 (atom=0x2baaaadfff00)
at ./include/odp/atomic.h:70
70 return __atomic_fetch_add(&atom->v, 1, __ATOMIC_RELAXED);
(gdb) bt
#0 0x0000000000411dc4 in odp_atomic_fetch_inc_u32 (atom=0x2baaaadfff00)
at ./include/odp/atomic.h:70
#1 0x0000000000411e8a in odp_ticketlock_lock
(ticketlock=0x2baaaadfff00) at odp_ticketlock.c:28
#2 0x000000000040f0f8 in queue_enq_multi (queue=0x2baaaadfff00,
buf_hdr=0x7fff1fccb0b0, num=1) at odp_queue.c:376
#3 0x000000000040987d in pktin_poll (entry=0x2aaaab200600) at
odp_packet_io.c:713
#4 0x0000000000410378 in schedule (out_queue=0x0,
out_ev=0x7fff1fccb1d8, max_num=1, max_deq=4) at odp_schedule.c:455
#5 0x000000000041050a in schedule_loop (out_queue=0x0, wait=1,
out_ev=0x7fff1fccb1d8, max_num=1, max_deq=4) at odp_schedule.c:518
#6 0x00000000004105a4 in odp_schedule (out_queue=0x0, wait=1) at
odp_schedule.c:551
#7 0x0000000000402b83 in destroy_inq (pktio=0x2) at odp_pktio.c:320
#8 0x00000000004032fa in pktio_test_txrx (q_type=1, num_pkts=4) at
odp_pktio.c:474
#9 0x000000000040337d in test_odp_pktio_poll_multi () at odp_pktio.c:487
#10 0x00007f3e96575482 in run_single_test () from
/usr/local/lib/libcunit.so.1
#11 0x00007f3e965750b2 in run_single_suite () from
/usr/local/lib/libcunit.so.1
#12 0x00007f3e96572d55 in CU_run_all_tests () from
/usr/local/lib/libcunit.so.1
#13 0x00007f3e96577245 in basic_run_all_tests () from
/usr/local/lib/libcunit.so.1
#14 0x00007f3e96576fe7 in CU_basic_run_tests () from
/usr/local/lib/libcunit.so.1
#15 0x0000000000403fe1 in main () at common/odp_cunit_common.c:77
(gdb) p atom
$1 = (odp_atomic_u32_t *) 0x2baaaadfff00
(gdb) p atom->v
Cannot access memory at address 0x2baaaadfff00
(gdb) up
#1 0x0000000000411e8a in odp_ticketlock_lock
(ticketlock=0x2baaaadfff00) at odp_ticketlock.c:28
28 ticket = odp_atomic_fetch_inc_u32(&ticketlock->next_ticket);
(gdb) up
#2 0x000000000040f0f8 in queue_enq_multi (queue=0x2baaaadfff00,
buf_hdr=0x7fff1fccb0b0, num=1) at odp_queue.c:376
376 LOCK(&queue->s.lock);
(gdb)
Another reason I'm not keen on this change is that those locks really
need to go as they serialise all send/recv calls (per pktio) and kill
scalability. As far as I can see they're only there to protect against
the application closing a pktio handle while another thread is using it,
but IMO we should consider that an application error. The only
linux-generic implementation that needs them is MMAP, and that can be
fixed. So it would be good to find a solution for this problem that
doesn't depend on those locks.
Yes, that is true. There might be sched lock should be somewhere or it
should not be possible
to terminate from other thread resources related to other thread. We
know that odp_schedule()
is 10-20 times slower than direct pktio, so probably we need to
reconsider locking scheme in
the scheduler.
Maxim.
--
Stuart.
_______________________________________________
lng-odp mailing list
[email protected]
https://lists.linaro.org/mailman/listinfo/lng-odp
