Message ID | 20250513-revert-rproc-table-sz-v1-1-a8c6b5d6f8a7@kernel.org |
---|---|
State | New |
Headers | show |
Series | Revert "remoteproc: core: Clear table_sz when rproc_shutdown" | expand |
On 5/13/25 10:52 AM, Bjorn Andersson wrote: > Clearing the table_sz on cleanup seemed reasonable, but further > discussions concluded that this merely working around the issue > and that the fix is incomplete. > > As such, revert commit efdde3d73ab2 ("remoteproc: core: Clear table_sz > when rproc_shutdown") to avoid carrying a partial fix. > Setting table_sz to 0 still seems like a good idea from a defensive programming perspective. Both table_ptr and table_sz should be set and cleared together in all spots. In addition to this, another fix would be to also update both table_ptr and table_sz to 0 when loading firmware without a resource table. Both should be done, no need to revert this. Andrew > Signed-off-by: Bjorn Andersson <andersson@kernel.org> > --- > drivers/remoteproc/remoteproc_core.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c > index 48d146e1fa560397c11eeb8f824ae0fb844a022b..81b2ccf988e852ac79cee375c7e3f118c2a4b41a 100644 > --- a/drivers/remoteproc/remoteproc_core.c > +++ b/drivers/remoteproc/remoteproc_core.c > @@ -2025,7 +2025,6 @@ int rproc_shutdown(struct rproc *rproc) > kfree(rproc->cached_table); > rproc->cached_table = NULL; > rproc->table_ptr = NULL; > - rproc->table_sz = 0; > out: > mutex_unlock(&rproc->lock); > return ret; > > --- > base-commit: aa94665adc28f3fdc3de2979ac1e98bae961d6ca > change-id: 20250513-revert-rproc-table-sz-53ecf24726ae > > Best regards,
On Thu, May 15, 2025 at 12:21:14PM -0500, Andrew Davis wrote: > On 5/13/25 10:52 AM, Bjorn Andersson wrote: > > Clearing the table_sz on cleanup seemed reasonable, but further > > discussions concluded that this merely working around the issue > > and that the fix is incomplete. > > > > As such, revert commit efdde3d73ab2 ("remoteproc: core: Clear table_sz > > when rproc_shutdown") to avoid carrying a partial fix. > > > > Setting table_sz to 0 still seems like a good idea from a defensive > programming perspective. Both table_ptr and table_sz should be set > and cleared together in all spots. > > In addition to this, another fix would be to also update > both table_ptr and table_sz to 0 when loading firmware without > a resource table. Both should be done, no need to revert this. > As mentioned by Bjorn, this is a partial fix. I'm all good with setting table_sz to 0, but as long as the real solution to the problem is part of the same work. Once the patch is reverted, which I'm about to apply, work can continue. > Andrew > > > Signed-off-by: Bjorn Andersson <andersson@kernel.org> > > --- > > drivers/remoteproc/remoteproc_core.c | 1 - > > 1 file changed, 1 deletion(-) > > > > diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c > > index 48d146e1fa560397c11eeb8f824ae0fb844a022b..81b2ccf988e852ac79cee375c7e3f118c2a4b41a 100644 > > --- a/drivers/remoteproc/remoteproc_core.c > > +++ b/drivers/remoteproc/remoteproc_core.c > > @@ -2025,7 +2025,6 @@ int rproc_shutdown(struct rproc *rproc) > > kfree(rproc->cached_table); > > rproc->cached_table = NULL; > > rproc->table_ptr = NULL; > > - rproc->table_sz = 0; > > out: > > mutex_unlock(&rproc->lock); > > return ret; > > > > --- > > base-commit: aa94665adc28f3fdc3de2979ac1e98bae961d6ca > > change-id: 20250513-revert-rproc-table-sz-53ecf24726ae > > > > Best regards,
On Tue, May 13, 2025 at 04:52:46PM +0100, Bjorn Andersson wrote: > Clearing the table_sz on cleanup seemed reasonable, but further > discussions concluded that this merely working around the issue > and that the fix is incomplete. > > As such, revert commit efdde3d73ab2 ("remoteproc: core: Clear table_sz > when rproc_shutdown") to avoid carrying a partial fix. > > Signed-off-by: Bjorn Andersson <andersson@kernel.org> > --- > drivers/remoteproc/remoteproc_core.c | 1 - > 1 file changed, 1 deletion(-) > I have applied this patch. Thanks, Mathieu > diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c > index 48d146e1fa560397c11eeb8f824ae0fb844a022b..81b2ccf988e852ac79cee375c7e3f118c2a4b41a 100644 > --- a/drivers/remoteproc/remoteproc_core.c > +++ b/drivers/remoteproc/remoteproc_core.c > @@ -2025,7 +2025,6 @@ int rproc_shutdown(struct rproc *rproc) > kfree(rproc->cached_table); > rproc->cached_table = NULL; > rproc->table_ptr = NULL; > - rproc->table_sz = 0; > out: > mutex_unlock(&rproc->lock); > return ret; > > --- > base-commit: aa94665adc28f3fdc3de2979ac1e98bae961d6ca > change-id: 20250513-revert-rproc-table-sz-53ecf24726ae > > Best regards, > -- > Bjorn Andersson <andersson@kernel.org> >
diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c index 48d146e1fa560397c11eeb8f824ae0fb844a022b..81b2ccf988e852ac79cee375c7e3f118c2a4b41a 100644 --- a/drivers/remoteproc/remoteproc_core.c +++ b/drivers/remoteproc/remoteproc_core.c @@ -2025,7 +2025,6 @@ int rproc_shutdown(struct rproc *rproc) kfree(rproc->cached_table); rproc->cached_table = NULL; rproc->table_ptr = NULL; - rproc->table_sz = 0; out: mutex_unlock(&rproc->lock); return ret;
Clearing the table_sz on cleanup seemed reasonable, but further discussions concluded that this merely working around the issue and that the fix is incomplete. As such, revert commit efdde3d73ab2 ("remoteproc: core: Clear table_sz when rproc_shutdown") to avoid carrying a partial fix. Signed-off-by: Bjorn Andersson <andersson@kernel.org> --- drivers/remoteproc/remoteproc_core.c | 1 - 1 file changed, 1 deletion(-) --- base-commit: aa94665adc28f3fdc3de2979ac1e98bae961d6ca change-id: 20250513-revert-rproc-table-sz-53ecf24726ae Best regards,