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

Dave Jiang
2021-03-25T15:27:08Z

On 3/24/2021 11:28 PM, Vinod Koul wrote:
> On 23-03-21, 08:44, Dave Jiang wrote:
>> On 3/23/2021 4:56 AM, Jason Gunthorpe wrote:
>>> On Tue, Mar 23, 2021 at 05:15:30PM +0530, Vinod Koul wrote:
>>>>> 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!
> Yes that does sound reasonable to me, when should I expect this move to
> show up on list?

We will try to do this in stages. So first we need to get this 'struct
device' lifetime fixes series into the kernel. Next I have the 'struct
device_driver' setup/shutdown series fix that Dan is reviewing
internally right now that I will post as soon as he okays it. I also
have the uacce conversion series ready and pending Dan's review. The
mdev series I need to rebase it to Jason's new VFIO code refactor. Some
of that refactor code is not yet posted public (I think?). So that will
take a little longer. For the kernel SVA support, I have internal code
but need to discuss with Dan on the implementation. Also we need to
measure performance on hardware to make our case for the new kernel
usage enablings. So that will come later.