From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757995AbcHCSRT (ORCPT ); Wed, 3 Aug 2016 14:17:19 -0400 Received: from mail-pa0-f53.google.com ([209.85.220.53]:34543 "EHLO mail-pa0-f53.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756575AbcHCSRQ (ORCPT ); Wed, 3 Aug 2016 14:17:16 -0400 Date: Wed, 3 Aug 2016 11:17:12 -0700 From: Bjorn Andersson To: loic pallardy Cc: linux-remoteproc@vger.kernel.org, Ohad Ben-Cohen , linux-kernel@vger.kernel.org, Lee Jones Subject: Re: [PATCH 3/4] remoteproc: Move vdev handling to boot/shutdown Message-ID: <20160803181712.GE13516@tuxbot> References: <1470077883-7419-1-git-send-email-bjorn.andersson@linaro.org> <1470077883-7419-3-git-send-email-bjorn.andersson@linaro.org> <57A0B988.50803@st.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <57A0B988.50803@st.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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