On Tue 02 Aug 08:17 PDT 2016, loic pallardy wrote: > Hi Bjorn, >
Hi Loic, Thanks for looking at the patches! [..] > >diff --git a/drivers/remoteproc/remoteproc_core.c > >b/drivers/remoteproc/remoteproc_core.c [..] > >@@ -984,23 +990,11 @@ int rproc_trigger_recovery(struct rproc *rproc) > > /* TODO: make sure this works with rproc->power > 1 */ > > rproc_shutdown(rproc); > > > >- /* clean up remote vdev entries */ > >- list_for_each_entry_safe(rvdev, rvtmp, &rproc->rvdevs, node) > >- rproc_remove_virtio_dev(rvdev); > >- > > /* wait until there is no more rproc users */ > > wait_for_completion(&rproc->crash_comp); > > > >- /* Free the copy of the resource table */ > >- kfree(rproc->cached_table); > I think this line should be part of patch 4 "Move handling of cached table > to boot/shutdown" > > Regards, > Loic > >- > >- ret = rproc_add_virtio_devices(rproc); > >- if (ret) > >- return ret; Before this patch this operation will trigger an async firmware load that will reallocate (kmemdup) cached_table. The rproc_boot() below would wait for this to finish and there would be a cached_table in place. > >- > > /* > >- * boot the remote processor up again, waiting for the async fw load to > >- * finish > >+ * boot the remote processor up again > > */ > > rproc_boot(rproc); > > Now that we instead directly handle the vdev resources in rproc_boot() the cached_table is not reallocated in this code path and as such has the life span of rproc_add() (or rather, the async fw callback) to rproc_del(). Therefor it should not be freed here anymore. Please do let me know if you see any concerns based on this life cycle change. Regards, Bjorn
