On 02/27/14 08:13, Kohji Okuno wrote:
Hi John-Mark,
Thank you for you comment.
From: John-Mark Gurney <j...@funkthat.com>
Kohji Okuno wrote this message on Thu, Feb 27, 2014 at 14:26 +0900:
I tried add kqueue I/F to usb_dev.c. I attached my patch.
What do you think about my patch?
A few comments...
1) You should just drop the use of flag_iskevent and just
unconditionally call KNOTE... since you have the lock already held,
the cost is minimal (and w/ modern branch prediction, may be cheaper)...
Should we set the use of flag_iskevent, when usb_filter_read() and
usb_filter_write() return `0'?
2) Why do you try to start read/write transfers in the _filter? You
should just check to see if data is available and not do work.. This
is also important since kqueue calls the filter just before delivering
the knote to userland to verify that there is still data, and it will
call your _event function for each knote on the fd... The work should
be started through other mechanisms, like read/write syscall or
interrupt or timeout/callout... If it's required to get results from
USB_IF_POLL, then it's fine..
I copied from usb_poll().
Should we try to start read/write transfers in usb_kqfilter()?
Or should not we try to start read/write transfers in poll and kqueue?
3) I don't see any calls to knlist_destroy... These calls are needed
to clean up the knlist...
I understood.
Obviously the #if 1's will need to go...
Also, I don't think your change is against HEAD.. The line numbers
in my version of usb_dev.c are different...
I'm sorry.
Hi,
I've found two bugs:
1)
+#if 1
+ knlist_init_mtx(&f_tx->selinfo.si_note, f_rx->priv_mtx);
+#endif
Should be:
+#if 1
+ knlist_init_mtx(&f_tx->selinfo.si_note, f_tx->priv_mtx);
+#endif
2)
Event filters need to lock the FIFO's mutex.
BTW: I'm working on getting the code into -HEAD. I'll run some test
before it goes in.
--HPS
_______________________________________________
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
