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

Dave Jiang
2021-03-23T15:45:33Z

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!