| Message ID | 1654888985-3846-2-git-send-email-quic_clew@quicinc.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | Introduce helpers for carveout management | expand |
Hi, On 6/10/22 21:23, Chris Lew wrote: > To mirror the exported rproc_add_carveout(), add a rproc_del_carveout() > so memory carveout resources added by devices outside of remoteproc can > manage the resource lifetime more accurately. > > Signed-off-by: Chris Lew <quic_clew@quicinc.com> > --- > drivers/remoteproc/remoteproc_core.c | 20 ++++++++++++++++++++ > include/linux/remoteproc.h | 1 + > 2 files changed, 21 insertions(+) > > diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c > index 02a04ab34a23..ee71fccae970 100644 > --- a/drivers/remoteproc/remoteproc_core.c > +++ b/drivers/remoteproc/remoteproc_core.c > @@ -1001,6 +1001,26 @@ void rproc_add_carveout(struct rproc *rproc, struct rproc_mem_entry *mem) > EXPORT_SYMBOL(rproc_add_carveout); > > /** > + * rproc_del_carveout() - remove an allocated carveout region > + * @rproc: rproc handle > + * @mem: memory entry to register > + * > + * This function removes specified memory entry in @rproc carveouts list. > + */ > +void rproc_del_carveout(struct rproc *rproc, struct rproc_mem_entry *mem) > +{ > + struct rproc_mem_entry *entry, *tmp; > + > + list_for_each_entry_safe(entry, tmp, &rproc->carveouts, node) { > + if (entry == mem) { > + list_del(&mem->node); > + return; > + } > + } > +} > +EXPORT_SYMBOL(rproc_del_carveout); This API seems to me quite dangerous because it can be called while carveouts are in use. At least some checks should be added... What about using rproc_resource_cleanup instead? Regards, Arnaud > + > +/** > * rproc_mem_entry_init() - allocate and initialize rproc_mem_entry struct > * @dev: pointer on device struct > * @va: virtual address > diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h > index 7c943f0a2fc4..43112aa78ffe 100644 > --- a/include/linux/remoteproc.h > +++ b/include/linux/remoteproc.h > @@ -658,6 +658,7 @@ struct rproc *devm_rproc_alloc(struct device *dev, const char *name, > int devm_rproc_add(struct device *dev, struct rproc *rproc); > > void rproc_add_carveout(struct rproc *rproc, struct rproc_mem_entry *mem); > +void rproc_del_carveout(struct rproc *rproc, struct rproc_mem_entry *mem); > > struct rproc_mem_entry * > rproc_mem_entry_init(struct device *dev,
On 6/30/2022 9:36 AM, Arnaud POULIQUEN wrote: > Hi, > > On 6/10/22 21:23, Chris Lew wrote: >> To mirror the exported rproc_add_carveout(), add a rproc_del_carveout() >> so memory carveout resources added by devices outside of remoteproc can >> manage the resource lifetime more accurately. >> >> Signed-off-by: Chris Lew <quic_clew@quicinc.com> >> --- >> drivers/remoteproc/remoteproc_core.c | 20 ++++++++++++++++++++ >> include/linux/remoteproc.h | 1 + >> 2 files changed, 21 insertions(+) >> >> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c >> index 02a04ab34a23..ee71fccae970 100644 >> --- a/drivers/remoteproc/remoteproc_core.c >> +++ b/drivers/remoteproc/remoteproc_core.c >> @@ -1001,6 +1001,26 @@ void rproc_add_carveout(struct rproc *rproc, struct rproc_mem_entry *mem) >> EXPORT_SYMBOL(rproc_add_carveout); >> >> /** >> + * rproc_del_carveout() - remove an allocated carveout region >> + * @rproc: rproc handle >> + * @mem: memory entry to register >> + * >> + * This function removes specified memory entry in @rproc carveouts list. >> + */ >> +void rproc_del_carveout(struct rproc *rproc, struct rproc_mem_entry *mem) >> +{ >> + struct rproc_mem_entry *entry, *tmp; >> + >> + list_for_each_entry_safe(entry, tmp, &rproc->carveouts, node) { >> + if (entry == mem) { >> + list_del(&mem->node); >> + return; >> + } >> + } >> +} >> +EXPORT_SYMBOL(rproc_del_carveout); > > This API seems to me quite dangerous because it can be called while carveouts are in use. > At least some checks should be added... > > What about using rproc_resource_cleanup instead? > > Regards, > Arnaud > The intended users of this API would be devices who negotiate with the device and know when the carveouts it has added are in use or can be reclaimed. I had looked at rproc_resource_cleanup() and that seemed more like a blanket cleanup rather than being able to cleanup a specific carveout. I agree the API seems dangerous, but I wasn't quite sure what kind of checks could be put here since remoteproc itself doesn't have any mechanisms to monitor the carveout state itself. We're relying on the fact that the device who added the carveout shouldn't make any mistakes which is fairly fragile. Thanks! Chris >> + >> +/** >> * rproc_mem_entry_init() - allocate and initialize rproc_mem_entry struct >> * @dev: pointer on device struct >> * @va: virtual address >> diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h >> index 7c943f0a2fc4..43112aa78ffe 100644 >> --- a/include/linux/remoteproc.h >> +++ b/include/linux/remoteproc.h >> @@ -658,6 +658,7 @@ struct rproc *devm_rproc_alloc(struct device *dev, const char *name, >> int devm_rproc_add(struct device *dev, struct rproc *rproc); >> >> void rproc_add_carveout(struct rproc *rproc, struct rproc_mem_entry *mem); >> +void rproc_del_carveout(struct rproc *rproc, struct rproc_mem_entry *mem); >> >> struct rproc_mem_entry * >> rproc_mem_entry_init(struct device *dev,
diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c index 02a04ab34a23..ee71fccae970 100644 --- a/drivers/remoteproc/remoteproc_core.c +++ b/drivers/remoteproc/remoteproc_core.c @@ -1001,6 +1001,26 @@ void rproc_add_carveout(struct rproc *rproc, struct rproc_mem_entry *mem) EXPORT_SYMBOL(rproc_add_carveout); /** + * rproc_del_carveout() - remove an allocated carveout region + * @rproc: rproc handle + * @mem: memory entry to register + * + * This function removes specified memory entry in @rproc carveouts list. + */ +void rproc_del_carveout(struct rproc *rproc, struct rproc_mem_entry *mem) +{ + struct rproc_mem_entry *entry, *tmp; + + list_for_each_entry_safe(entry, tmp, &rproc->carveouts, node) { + if (entry == mem) { + list_del(&mem->node); + return; + } + } +} +EXPORT_SYMBOL(rproc_del_carveout); + +/** * rproc_mem_entry_init() - allocate and initialize rproc_mem_entry struct * @dev: pointer on device struct * @va: virtual address diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h index 7c943f0a2fc4..43112aa78ffe 100644 --- a/include/linux/remoteproc.h +++ b/include/linux/remoteproc.h @@ -658,6 +658,7 @@ struct rproc *devm_rproc_alloc(struct device *dev, const char *name, int devm_rproc_add(struct device *dev, struct rproc *rproc); void rproc_add_carveout(struct rproc *rproc, struct rproc_mem_entry *mem); +void rproc_del_carveout(struct rproc *rproc, struct rproc_mem_entry *mem); struct rproc_mem_entry * rproc_mem_entry_init(struct device *dev,
To mirror the exported rproc_add_carveout(), add a rproc_del_carveout() so memory carveout resources added by devices outside of remoteproc can manage the resource lifetime more accurately. Signed-off-by: Chris Lew <quic_clew@quicinc.com> --- drivers/remoteproc/remoteproc_core.c | 20 ++++++++++++++++++++ include/linux/remoteproc.h | 1 + 2 files changed, 21 insertions(+)