Re: [PATCH v2 0/3] devlink region trigger support

[Jacob Keller <jacob.e.keller@intel.com>]

On 1/10/2020 10:47 AM, Jakub Kicinski wrote:
> On Fri, 10 Jan 2020 09:54:20 -0800, Jacob Keller wrote:
>> Mostly I wanted to make sure the direction was looking good earlier than
>> the time frame for completing that work.
> 
> As Jiri said, quite hard to tell without seeing the real user.
> 

Fair.

> I presume you take some lock to ensure the contents of the snapshot are
> atomic?  Otherwise I wonder if you wouldn't be better served by just
> allowing region read to operate directly on the data rather then going
> through the snapshot create -> read -> snapshot remove cycle. Not sure
> Jiri would agree, it require more code.
> 

Right. I saw the original devlink region commit messages indicated
possible plans to support writing to regions. There is also the question
of handling data that might want to be read sparsely, rather than
reading an entire snapshot at once. Hmm.

> For the patches themselves we may want to move the callbacks into some
> ops structure in the region.  And I don't think you need to make the
> trigger command NO_LOCK.
> 

So the reason it was made NO_LOCK right now is because the trigger ends
up calling devlink_region_snapshot_id_get and
devlink_region_snapshot_create which both take the devlink lock.

I suppose this could be simplified by having the snapshot callback take
similar parameters as the devlink_region_snapshot_create, and assume the
devlink core code does such things as generating an id and adding the
snapshot to the list all while holding the lock...

I will sort out these questions into what I think makes sense for the
use case I envision, and will submit the core devlink patches at the
same time as the driver changes.

Thanks,
Jake