Hi Kieran,
On Mon, May 25, 2020 at 02:19:02PM +0100, Kieran Bingham wrote:
Hi Eugeniu,
Yeouch. Looks like I really missed a trick there!
Not a big deal. The good part is that it can be proactively fixed and shared across the community.
We should probably update the $SUBJECT to match what is performed in the patch, which is perhaps more like:
"media: vsp1: dl: Store VSP reference when creating cmd pools"
To be honest, I am not a big fan of WHAT summary lines. Rather, I prefer the WHY summary lines (and I think everyone should).
On 23/05/2020 09:13, Eugeniu Rosca wrote:
And then we can explain here:
In commit f3b98e3c4d2e16 ("media: vsp1: Provide support for extended command pools"), the vsp pointer used for referencing the VSP1 device structure from a command pool during vsp1_dl_ext_cmd_pool_destroy() was not populated.
Correctly assign the pointer to prevent the following null-pointer-dereference when removing the device:
That sounds good and I can push this improved description as v2.
Fixes: f3b98e3c4d2e16 ("media: vsp1: Provide support for extended command pools") Cc: stable@vger.kernel.org # v4.19+ Signed-off-by: Eugeniu Rosca erosca@de.adit-jv.com
Reviewed-by: Kieran Bingham kieran.bingham+renesas@ideasonboard.com
How about adding a new unit test perfoming unbind/rebind to http://git.ideasonboard.com/renesas/vsp-tests.git, to avoid such issues in future?
Yes, now I wish I had done so back at 4.19! I hope this wasn't too painful to diagnose and fix, and thank you for being so thorough in your report!
Locally, below command has been used to identify the problem:
for f in $(find /sys/bus/platform/devices/ -name "*vsp*" -o -name "*fdp*"); do \ b=$(basename $f); \ echo $b > $f/driver/unbind; \ done
I've created a test to add to vsp-tests, which I'll post next, thank you for the suggestion.
Before your patch is applied, I experience the same crash you have seen, and after your patch - I can successfully unbind/bind all of the VSP1 instances.
So I think you can have this too:
Tested-by: Kieran Bingham kieran.bingham+renesas@ideasonboard.com
Awesome. Thanks!