On 11/17/2015 01:27 AM, Mauro Carvalho Chehab wrote: > Em Mon, 16 Nov 2015 12:54:28 -0700 > Shuah Khan <[email protected]> escreveu: > >> On 11/16/2015 11:36 AM, Mauro Carvalho Chehab wrote: >>> Em Mon, 16 Nov 2015 01:02:56 +0200 >>> Sakari Ailus <[email protected]> escreveu: >>> >>>> Hi Shuah, >>>> >>>> On Thu, Nov 12, 2015 at 07:41:47AM -0700, Shuah Khan wrote: >>>>> Media core drivers (dvb, v4l2, bridge driver) unregister >>>>> their entities calling media_device_unregister_entity() >>>>> during device removal from their unregister paths. In >>>>> addition media_device_unregister() tries to unregister >>>>> entity calling media_device_unregister_entity() for each >>>>> one of them. This adds lot of contention on mdev->lock in >>>>> device removal sequence. Fix to not unregister entities >>>>> from media_device_unregister(), and let drivers take care >>>>> of it. Drivers need to unregister to cover the case of >>>>> module removal. This patch fixes the problem by deleting >>>>> the entity list walk to call media_device_unregister_entity() >>>>> for each entity. With this fix there is no kernel hang after >>>>> a sequence of device insertions followed by removal. >>>>> >>>>> Signed-off-by: Shuah Khan <[email protected]> >>>>> --- >>>>> drivers/media/media-device.c | 5 ----- >>>>> 1 file changed, 5 deletions(-) >>>>> >>>>> diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c >>>>> index 1312e93..c7ab7c9 100644 >>>>> --- a/drivers/media/media-device.c >>>>> +++ b/drivers/media/media-device.c >>>>> @@ -577,8 +577,6 @@ EXPORT_SYMBOL_GPL(__media_device_register); >>>>> */ >>>>> void media_device_unregister(struct media_device *mdev) >>>>> { >>>>> - struct media_entity *entity; >>>>> - struct media_entity *next; >>>>> struct media_link *link, *tmp_link; >>>>> struct media_interface *intf, *tmp_intf; >>>>> >>>>> @@ -596,9 +594,6 @@ void media_device_unregister(struct media_device >>>>> *mdev) >>>>> kfree(intf); >>>>> } >>>>> >>>>> - list_for_each_entry_safe(entity, next, &mdev->entities, graph_obj.list) >>>>> - media_device_unregister_entity(entity); >>>>> - >>>>> device_remove_file(&mdev->devnode.dev, &dev_attr_model); >>>>> media_devnode_unregister(&mdev->devnode); >>>>> >>>> >>>> media_device_unregister() is expected to clean up all the entities still >>>> registered with it (as it does to links and interfaces). Could you share >>>> details of the problems you saw in case you haven't found the actual >>>> underlying issue causing them? >>>> >>> >>> I was not able to reproduce here with au0828. Tried to register/unregister >>> 20 >>> times with: >>> $ i=1; while :; do echo $i; sudo modprobe au0828 && sleep 2 && sudo >>> rmmod au0828; i=$((i+1)); done >>> >>> The results are at: >>> http://pastebin.com/1B9c9nYm >> >> Mauro, >> >> Please remove the device physically to see the problem I am seeing. >> Also make sure you have the locking debug enabled. I just have to >> remove the device physically just once to see the hang. > > I did that too: no problem. I can remove/reinsert the device without > causing any hangs or any dmesg related to locking issues. > >> >>> >>> >>> PS.: I had to add the 2 seconds sleep there, as otherwise unregister may >>> fail, because udev is started every time the devnodes are created. Without >>> that, sometimes it returns -EBUSY, because udev didn't close the devnode >>> yet. Still no problem, as a later rmmod works: >>> >>> $ sudo modprobe au0828 && sudo rmmod au0828 >>> $ sudo modprobe au0828 && sudo rmmod au0828 >>> rmmod: ERROR: Module au0828 is in use >>> $ sudo rmmod au0828 >>> $ >>> >>> So, I don't think that the issues you're experiencing are related to the >>> MC next generation. >> >> On my system it is the case. I have run physical removal of the device >> on MC and didn't see the problem. > > Sorry. Do you mean that rmmod au0828 never works on your system?
No physical removal not rmmod that causes problems. > > Then indeed there's something broken with udev and some other module, > and the radeon patch you've applied didn't actually solve the bug. > Right - I will try without radeon to see if the problem persists. I think we are good based on your tests. This problem might indeed be an isolated one specific my setup. Thanks for testing on your system. -- Shuah -- Shuah Khan Sr. Linux Kernel Developer Open Source Innovation Group Samsung Research America (Silicon Valley) [email protected] | (970) 217-8978 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
