On 2013/04/04 09:21 PM, H Hartley Sweeten wrote:
A comment about the reference counts issue might be a good idea so it isn't forgotten.
In brief, my plan is to add a struct kref to the struct comedi_device, and point comedi_class->dev_release to a function that does a kref_put() on that kref. We need to do a kref_get() for each of the class devices created for the subdevices, but not for the class device created for the main comedi device. The kref_get()s for the subdevices will be balanced out by the ->dev_release()/kref_put() calls when those subdevice class devices are destroyed. The extra ->dev_release/kref_put() resulting from destruction of the main class device will result in the struct comedi_device being freed.
Question. Are all the BUG_ON checks of the minor actually needed? comedi_alloc_subdevice() and comedi_alloc_subdevice_minor() both validate the minor before setting s->minor. The BUG_ON checks in comedi_free_subdevice_minor() should never happen and the others shouldn't be possible either.
Good question. Of course bugs should never happen. ;) How much checking for potential bugs is done is somewhat subjective. Should the code assume nothing but the core will change s->minor or play it safe and check for a potential bug?
Other than that... Reviewed-by: H Hartley Sweeten <hsweeten@xxxxxxxxxxxxxxxxxxx>
Thanks for the review. -- -=( Ian Abbott @ MEV Ltd. E-mail: <abbotti@xxxxxxxxx> )=- -=( Tel: +44 (0)161 477 1898 FAX: +44 (0)161 718 3587 )=- _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
