On 18/07/17 16:47, Niklas Söderlund wrote:
>>> void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier)
>>> {
>>> - struct v4l2_subdev *sd, *tmp;
>>> + struct v4l2_subdev *sd, *tmp, **subdev;
>>> unsigned int notif_n_subdev = notifier->num_subdevs;
>>> unsigned int n_subdev = min(notif_n_subdev, V4L2_MAX_SUBDEVS);
>>> struct device **dev;
>>> @@ -217,6 +293,12 @@ void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier)
>>> "Failed to allocate device cache!\n");
>>> }
>>>
>>> + subdev = kvmalloc_array(n_subdev, sizeof(*subdev), GFP_KERNEL);
>>> + if (!dev) {
>>> + dev_err(notifier->v4l2_dev->dev,
>>> + "Failed to allocate subdevice cache!\n");
>>> + }
>>> +
>>
>> How about making a little struct:
>>
>> struct whatever {
>> struct device *dev;
>> struct v4l2_subdev *sd;
>> };
>>
>> and allocate an array of that. Only need to call kvmalloc_array once.
>
> Neat idea, will do so for next version.
>
>>
>> Some comments after the dev_err of why you ignore the failed memory allocation
>> and what the consequences of that are would be helpful. It is unexpected code,
>> and that needs documentation.
>
> I agree that it's unexpected and I don't know the reason for it, I was
> just mimic the existing behavior. If you are OK with it I be more then
> happy to add patch to this series returning -ENOMEM if the allocation
> failed as Geert pointed out if this allocation fails I think we are in a
> lot of trouble anyhow...
>
> Let me know what you think, but I don't think I can add a comment
> explaining why the function don't simply abort on failure since I don't
> understand it myself.
So you don't understand the device_release_driver/device_attach reprobing bit either?
I did some digging and found this thread:
http://lkml.iu.edu/hypermail/linux/kernel/1210.2/00713.html
It explains the reason for this.
I'm pretty sure Greg K-H never saw this code :-)
Looking in drivers/base/bus.c I see this function: device_reprobe().
I think we need to use that instead.
Regards,
Hans
