Re: [PATCH v7 0/8] idxd 'struct device' lifetime handling fixes

[Dave Jiang <dave.jiang@intel.com>]

On 3/23/2021 4:56 AM, Jason Gunthorpe wrote:
> On Tue, Mar 23, 2021 at 05:15:30PM +0530, Vinod Koul wrote:
>> Hi Dave,
>>
>> On 22-03-21, 16:31, Dave Jiang wrote:
>>> v7:
>>> - Fix up the 'struct device' setup in char device code (Jason)
>>> - Split out the char dev fixes (Jason)
>>> - Split out the DMA dev fixes (Dan)
>>> - Split out the each of the conf_dev fixes
>>> - Split out removal of the pcim_* calls
>>> - Split out removal of the devm_* calls
>>> - Split out the fixes for interrupt config calls
>>> - Reviewed by Dan.
>>>
>>> v6:
>>> - Fix char dev initialization issues (Jason)
>>> - Fix other 'struct device' initialization issues.
>>>
>>> v5:
>>> - Rebased against 5.12-rc dmaengine/fixes
>>> v4:
>>> - fix up the life time of cdev creation/destruction (Jason)
>>> - Tested with KASAN and other memory allocation leak detections. (Jason)
>>>
>>> v3:
>>> - Remove devm_* for irq request and cleanup related bits (Jason)
>>> v2:
>>> - Remove all devm_* alloc for idxd_device (Jason)
>>> - Add kref dep for dma_dev (Jason)
>>>
>>> Vinod,
>>> The series fixes the various 'struct device' lifetime handling in the
>>> idxd driver. The devm managed lifetime is incompatible with 'struct device'
>>> objects that resides in the idxd context. Tested with
>>> CONFIG_DEBUG_KOBJECT_RELEASE and address all issues from that.
>> Sorry for not looking into this earlier.. But I would like to check on
>> the direction idxd is taking. Somehow I get the feel the dmaengine is
>> not the right place for this. Considering that now we have auxdev merged
>> in, should the idxd be spilt into smaller function and no dmaengine
>> parts moved away from dmaengine... I think we do lack a subsystem for
>> all things accelerator in kernel atm...
> auxdev shouldn't be over-used IMHO.
>
> If the main purpose of the driver is dma engine then it is OK if the
> "core" part lives there too.

Hi Vinod,

So this patch series serves as the basis of getting the idxd 
dsa_bus_type related bits fixed up so that auxdev is not necessary. When 
Jason reviewed previous iterations of the mdev series, he noted that the 
mdev driver needs to go under VFIO. After the auxdev conversion of the 
mdev series, Jason and Dan after further review suggested that given 
there's an internal bus in idxd driver already (dsa_bus_type), that can 
be used to load drivers rather than needing to rely on auxiliary bus. 
But the implementation of the dsa_bus_type needs some fixes. After this 
series, I have another series that's going through internal review right 
now that will fixup the driver setup and initialization of dsa_bus_type 
and allow us to load external drivers for the wq. The in kernel use 
cases for DSA is still valid under dmaengine so the core parts remains 
valid for dmaengine. The plan going forward is after getting all the 
fixups in we are planning to:

1. Introduce UACCE framework support for idxd and have a wq driver 
resides under drivers/misc/uacce/idxd to support the char device 
operations and deprecate the current custom char dev in idxd. This 
should remove the burden on you to deal with the char device.

2. Resubmit the mdev driver under drivers/vfio/mdev/idxd after Jason's 
latest VFIO refactoring is done.

3. Introduce new kernel use cases with dmanegine API support for SVA 
operations.

Let me know if this sounds ok to you. Thanks!