Re: [PATCH v9 1/5] driver core: Find an existing link between two devices

From: Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org>
To: "Rafael J. Wysocki" <rjw-LthD3rsA81gm4RdzfppkhA@public.gmane.org>,
	Tomasz Figa <tfiga-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Lukas Wunner <lukas-JFq808J9C/izQB+pC5nmwQ@public.gmane.org>,
	linux-arm-msm
	<linux-arm-msm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>,
	Linux Kernel Mailing List
	<linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	"open list:IOMMU DRIVERS"
	<iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org>,
	Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	Greg Kroah-Hartman
	<gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>
Subject: Re: [PATCH v9 1/5] driver core: Find an existing link between two devices
Date: Wed, 14 Mar 2018 12:14:15 +0000	[thread overview]
Message-ID: <11eae0bc-7921-e598-9c01-63498400c85b@arm.com> (raw)
In-Reply-To: <2705105.V6KYPvoJqj-yvgW3jdyMHm1GS7QM15AGw@public.gmane.org>

Hi Rafael,

On 14/03/18 11:57, Rafael J. Wysocki wrote:
> On Wednesday, March 14, 2018 12:50:54 PM CET Tomasz Figa wrote:
>> On Wed, Mar 14, 2018 at 8:12 PM, Rafael J. Wysocki <rjw-LthD3rsA81gm4RdzfppkhA@public.gmane.org> wrote:
>>> On Tuesday, March 13, 2018 12:23:34 PM CET Tomasz Figa wrote:
>>>> On Tue, Mar 13, 2018 at 7:34 PM, Vivek Gautam
>>>> <vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> wrote:
>>>>> Hi Tomasz,
>>>>>
>>>>> On Tue, Mar 13, 2018 at 3:45 PM, Tomasz Figa <tfiga-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
>>>>>> Hi Vivek,
>>>>>>
>>>>>> Thanks for the patch.
>>>>>>
>>>>>> On Tue, Mar 13, 2018 at 5:55 PM, Vivek Gautam
>>>>>> <vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> wrote:
>>>>>>> The lists managing the device-links can be traversed to
>>>>>>> find the link between two devices. The device_link_add() APIs
>>>>>>> does traverse these lists to check if there's already a link
>>>>>>> setup between the two devices.
>>>>>>> So, add a new APIs, device_link_find(), to find an existing
>>>>>>> device link between two devices - suppliers and consumers.
>>>>>>
>>>>>> I'm wondering if this API would be useful for anything else that the
>>>>>> problem we're trying to solve with deleting links without storing them
>>>>>> anywhere. Perhaps a device_link_del_dev(consumer, supplier) would be a
>>>>>> better alternative?
>>>>>
>>>>> Yea, that sounds simpler i think. Will add this API instead of
>>>>> find_link(). Thanks.
>>>>
>>>> Perhaps let's wait for a moment to see if there are other opinions. :)
>>>>
>>>> Rafael, Lucas, any thoughts?
>>>
>>> It is not clear to me what the device_link_del_dev(consumer, supplier) would do.
>>
>> It would delete a link between consumer and supplier.
> 
> If there's one I suppose.
> 
> I'm wondering if you are somehow trying to address the same problem as the
> device links reference counting patch from Lukas that has been queued up for 4.17
> already.

Not quite - the issue here is that we have one supplier with an 
arbitrarily large number of consumers, and would prefer that supplier 
not to have to spend a whole bunch of memory to store all the struct 
device_link pointers for the sole reason of having something to give to 
device_link_del() at the end, given that the device links code is 
already keeping track of everything internally anyway.

The current API would permit doing this:

	iommu_attach(dev) {
		...
		if (!device_link_add(dev, iommu, IOMMU_LINK_FLAGS))
			return -ENODEV;
		...
	}

	iommu_detach(dev) {
		...
		// Will return the existing link from earlier
		link = device_link_add(dev, iommu, IOMMU_LINK_FLAGS);
		device_link_del(link);
		// Needed once refcounting is in place
		//device_link_del(link);
		...
	}

but it looks so wacky and non-obvious that we'd like to encapsulate the 
same behaviour into a more formal interface (my personal naming 
preference would be device_link_remove(consumer, supplier)).

Robin.