| Message ID | 20230310103210.22372-8-vbabka@suse.cz (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | remove SLOB and allow kfree() with kmem_cache_alloc() | expand |
On Fri, Mar 10, 2023 at 11:32:09AM +0100, Vlastimil Babka wrote: > This will make it easier to free objects in situations when they can > come from either kmalloc() or kmem_cache_alloc(), and also allow > kfree_rcu() for freeing objects from kmem_cache_alloc(). > > For the SLAB and SLUB allocators this was always possible so with SLOB > gone, we can document it as supported. > > Signed-off-by: Vlastimil Babka <vbabka@suse.cz> > Cc: Mike Rapoport <rppt@kernel.org> > Cc: Jonathan Corbet <corbet@lwn.net> > Cc: "Paul E. McKenney" <paulmck@kernel.org> > Cc: Frederic Weisbecker <frederic@kernel.org> > Cc: Neeraj Upadhyay <quic_neeraju@quicinc.com> > Cc: Josh Triplett <josh@joshtriplett.org> > Cc: Steven Rostedt <rostedt@goodmis.org> > Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> > Cc: Lai Jiangshan <jiangshanlai@gmail.com> > Cc: Joel Fernandes <joel@joelfernandes.org> > --- > Documentation/core-api/memory-allocation.rst | 15 +++++++++++---- > include/linux/rcupdate.h | 6 ++++-- > mm/slab_common.c | 5 +---- > 3 files changed, 16 insertions(+), 10 deletions(-) > > diff --git a/Documentation/core-api/memory-allocation.rst b/Documentation/core-api/memory-allocation.rst > index 5954ddf6ee13..f9e8d352ed67 100644 > --- a/Documentation/core-api/memory-allocation.rst > +++ b/Documentation/core-api/memory-allocation.rst > @@ -170,7 +170,14 @@ should be used if a part of the cache might be copied to the userspace. > After the cache is created kmem_cache_alloc() and its convenience > wrappers can allocate memory from that cache. > > -When the allocated memory is no longer needed it must be freed. You can > -use kvfree() for the memory allocated with `kmalloc`, `vmalloc` and > -`kvmalloc`. The slab caches should be freed with kmem_cache_free(). And > -don't forget to destroy the cache with kmem_cache_destroy(). > +When the allocated memory is no longer needed it must be freed. Objects I'd add a line break before Objects ^ > +allocated by `kmalloc` can be freed by `kfree` or `kvfree`. > +Objects allocated by `kmem_cache_alloc` can be freed with `kmem_cache_free` > +or also by `kfree` or `kvfree`, which can be more convenient as it does Maybe replace 'or also by' with a coma: Objects allocated by `kmem_cache_alloc` can be freed with `kmem_cache_free`, `kfree` or `kvfree`, which can be more convenient as it does > +not require the kmem_cache pointed. ^ pointer. > +The rules for _bulk and _rcu flavors of freeing functions are analogical. Maybe The same rules apply to _bulk and _rcu flavors of freeing functions. > + > +Memory allocated by `vmalloc` can be freed with `vfree` or `kvfree`. > +Memory allocated by `kvmalloc` can be freed with `kvfree`. > +Caches created by `kmem_cache_create` should be freed with > +`kmem_cache_destroy` only after freeing all the allocated objects first. > diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h > index 094321c17e48..dcd2cf1e8326 100644 > --- a/include/linux/rcupdate.h > +++ b/include/linux/rcupdate.h > @@ -976,8 +976,10 @@ static inline notrace void rcu_read_unlock_sched_notrace(void) > * either fall back to use of call_rcu() or rearrange the structure to > * position the rcu_head structure into the first 4096 bytes. > * > - * Note that the allowable offset might decrease in the future, for example, > - * to allow something like kmem_cache_free_rcu(). > + * The object to be freed can be allocated either by kmalloc() or > + * kmem_cache_alloc(). > + * > + * Note that the allowable offset might decrease in the future. > * > * The BUILD_BUG_ON check must not involve any function calls, hence the > * checks are done in macros here. > diff --git a/mm/slab_common.c b/mm/slab_common.c > index 1522693295f5..607249785c07 100644 > --- a/mm/slab_common.c > +++ b/mm/slab_common.c > @@ -989,12 +989,9 @@ EXPORT_SYMBOL(__kmalloc_node_track_caller); > > /** > * kfree - free previously allocated memory > - * @object: pointer returned by kmalloc. > + * @object: pointer returned by kmalloc() or kmem_cache_alloc() > * > * If @object is NULL, no operation is performed. > - * > - * Don't free memory not originally allocated by kmalloc() > - * or you will run into trouble. > */ > void kfree(const void *object) > { > -- > 2.39.2 >
On 3/12/23 10:59, Mike Rapoport wrote: > On Fri, Mar 10, 2023 at 11:32:09AM +0100, Vlastimil Babka wrote: >> This will make it easier to free objects in situations when they can >> come from either kmalloc() or kmem_cache_alloc(), and also allow >> kfree_rcu() for freeing objects from kmem_cache_alloc(). >> >> For the SLAB and SLUB allocators this was always possible so with SLOB >> gone, we can document it as supported. >> >> Signed-off-by: Vlastimil Babka <vbabka@suse.cz> >> Cc: Mike Rapoport <rppt@kernel.org> >> Cc: Jonathan Corbet <corbet@lwn.net> >> Cc: "Paul E. McKenney" <paulmck@kernel.org> >> Cc: Frederic Weisbecker <frederic@kernel.org> >> Cc: Neeraj Upadhyay <quic_neeraju@quicinc.com> >> Cc: Josh Triplett <josh@joshtriplett.org> >> Cc: Steven Rostedt <rostedt@goodmis.org> >> Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> >> Cc: Lai Jiangshan <jiangshanlai@gmail.com> >> Cc: Joel Fernandes <joel@joelfernandes.org> >> --- >> Documentation/core-api/memory-allocation.rst | 15 +++++++++++---- >> include/linux/rcupdate.h | 6 ++++-- >> mm/slab_common.c | 5 +---- >> 3 files changed, 16 insertions(+), 10 deletions(-) >> >> diff --git a/Documentation/core-api/memory-allocation.rst b/Documentation/core-api/memory-allocation.rst >> index 5954ddf6ee13..f9e8d352ed67 100644 >> --- a/Documentation/core-api/memory-allocation.rst >> +++ b/Documentation/core-api/memory-allocation.rst >> @@ -170,7 +170,14 @@ should be used if a part of the cache might be copied to the userspace. >> After the cache is created kmem_cache_alloc() and its convenience >> wrappers can allocate memory from that cache. >> >> -When the allocated memory is no longer needed it must be freed. You can >> -use kvfree() for the memory allocated with `kmalloc`, `vmalloc` and >> -`kvmalloc`. The slab caches should be freed with kmem_cache_free(). And >> -don't forget to destroy the cache with kmem_cache_destroy(). >> +When the allocated memory is no longer needed it must be freed. Objects > > I'd add a line break before Objects ^ > >> +allocated by `kmalloc` can be freed by `kfree` or `kvfree`. >> +Objects allocated by `kmem_cache_alloc` can be freed with `kmem_cache_free` >> +or also by `kfree` or `kvfree`, which can be more convenient as it does > > Maybe replace 'or also by' with a coma: > > Objects allocated by `kmem_cache_alloc` can be freed with `kmem_cache_free`, > `kfree` or `kvfree`, which can be more convenient as it does But then I need to clarify what the "which" applies to? > >> +not require the kmem_cache pointed. > > ^ pointer. > >> +The rules for _bulk and _rcu flavors of freeing functions are analogical. > > Maybe > > The same rules apply to _bulk and _rcu flavors of freeing functions. So like this incremental diff? diff --git a/Documentation/core-api/memory-allocation.rst b/Documentation/core-api/memory-allocation.rst index f9e8d352ed67..1c58d883b273 100644 --- a/Documentation/core-api/memory-allocation.rst +++ b/Documentation/core-api/memory-allocation.rst @@ -170,12 +170,14 @@ should be used if a part of the cache might be copied to the userspace. After the cache is created kmem_cache_alloc() and its convenience wrappers can allocate memory from that cache. -When the allocated memory is no longer needed it must be freed. Objects -allocated by `kmalloc` can be freed by `kfree` or `kvfree`. -Objects allocated by `kmem_cache_alloc` can be freed with `kmem_cache_free` -or also by `kfree` or `kvfree`, which can be more convenient as it does -not require the kmem_cache pointed. -The rules for _bulk and _rcu flavors of freeing functions are analogical. +When the allocated memory is no longer needed it must be freed. + +Objects allocated by `kmalloc` can be freed by `kfree` or `kvfree`. Objects +allocated by `kmem_cache_alloc` can be freed with `kmem_cache_free`, `kfree` +or `kvfree`, where the latter two might be more convenient thanks to not +needing the kmem_cache pointer. + +The same rules apply to _bulk and _rcu flavors of freeing functions. Memory allocated by `vmalloc` can be freed with `vfree` or `kvfree`. Memory allocated by `kvmalloc` can be freed with `kvfree`.
On Wed, Mar 15, 2023 at 02:38:47PM +0100, Vlastimil Babka wrote: > On 3/12/23 10:59, Mike Rapoport wrote: > > On Fri, Mar 10, 2023 at 11:32:09AM +0100, Vlastimil Babka wrote: > >> This will make it easier to free objects in situations when they can > >> come from either kmalloc() or kmem_cache_alloc(), and also allow > >> kfree_rcu() for freeing objects from kmem_cache_alloc(). > >> > >> For the SLAB and SLUB allocators this was always possible so with SLOB > >> gone, we can document it as supported. > >> > >> Signed-off-by: Vlastimil Babka <vbabka@suse.cz> > >> Cc: Mike Rapoport <rppt@kernel.org> > >> Cc: Jonathan Corbet <corbet@lwn.net> > >> Cc: "Paul E. McKenney" <paulmck@kernel.org> > >> Cc: Frederic Weisbecker <frederic@kernel.org> > >> Cc: Neeraj Upadhyay <quic_neeraju@quicinc.com> > >> Cc: Josh Triplett <josh@joshtriplett.org> > >> Cc: Steven Rostedt <rostedt@goodmis.org> > >> Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> > >> Cc: Lai Jiangshan <jiangshanlai@gmail.com> > >> Cc: Joel Fernandes <joel@joelfernandes.org> > >> --- > >> Documentation/core-api/memory-allocation.rst | 15 +++++++++++---- > >> include/linux/rcupdate.h | 6 ++++-- > >> mm/slab_common.c | 5 +---- > >> 3 files changed, 16 insertions(+), 10 deletions(-) > >> > >> diff --git a/Documentation/core-api/memory-allocation.rst b/Documentation/core-api/memory-allocation.rst > >> index 5954ddf6ee13..f9e8d352ed67 100644 > >> --- a/Documentation/core-api/memory-allocation.rst > >> +++ b/Documentation/core-api/memory-allocation.rst > >> @@ -170,7 +170,14 @@ should be used if a part of the cache might be copied to the userspace. > >> After the cache is created kmem_cache_alloc() and its convenience > >> wrappers can allocate memory from that cache. > >> > >> -When the allocated memory is no longer needed it must be freed. You can > >> -use kvfree() for the memory allocated with `kmalloc`, `vmalloc` and > >> -`kvmalloc`. The slab caches should be freed with kmem_cache_free(). And > >> -don't forget to destroy the cache with kmem_cache_destroy(). > >> +When the allocated memory is no longer needed it must be freed. Objects > > > > I'd add a line break before Objects ^ > > > >> +allocated by `kmalloc` can be freed by `kfree` or `kvfree`. > >> +Objects allocated by `kmem_cache_alloc` can be freed with `kmem_cache_free` > >> +or also by `kfree` or `kvfree`, which can be more convenient as it does > > > > Maybe replace 'or also by' with a coma: > > > > Objects allocated by `kmem_cache_alloc` can be freed with `kmem_cache_free`, > > `kfree` or `kvfree`, which can be more convenient as it does > > But then I need to clarify what the "which" applies to? Yeah, I kinda missed that... > > > >> +not require the kmem_cache pointed. > > > > ^ pointer. > > > >> +The rules for _bulk and _rcu flavors of freeing functions are analogical. > > > > Maybe > > > > The same rules apply to _bulk and _rcu flavors of freeing functions. > > So like this incremental diff? > diff --git a/Documentation/core-api/memory-allocation.rst b/Documentation/core-api/memory-allocation.rst > index f9e8d352ed67..1c58d883b273 100644 > --- a/Documentation/core-api/memory-allocation.rst > +++ b/Documentation/core-api/memory-allocation.rst > @@ -170,12 +170,14 @@ should be used if a part of the cache might be copied to the userspace. > After the cache is created kmem_cache_alloc() and its convenience > wrappers can allocate memory from that cache. > > -When the allocated memory is no longer needed it must be freed. Objects > -allocated by `kmalloc` can be freed by `kfree` or `kvfree`. > -Objects allocated by `kmem_cache_alloc` can be freed with `kmem_cache_free` > -or also by `kfree` or `kvfree`, which can be more convenient as it does > -not require the kmem_cache pointed. > -The rules for _bulk and _rcu flavors of freeing functions are analogical. > +When the allocated memory is no longer needed it must be freed. > + > +Objects allocated by `kmalloc` can be freed by `kfree` or `kvfree`. Objects > +allocated by `kmem_cache_alloc` can be freed with `kmem_cache_free`, `kfree` > +or `kvfree`, where the latter two might be more convenient thanks to not > +needing the kmem_cache pointer. ... but this way it's more explicit that kfree and kvfree don't need kmem_cache pointer. > + > +The same rules apply to _bulk and _rcu flavors of freeing functions. > > Memory allocated by `vmalloc` can be freed with `vfree` or `kvfree`. > Memory allocated by `kvmalloc` can be freed with `kvfree`. >
diff --git a/Documentation/core-api/memory-allocation.rst b/Documentation/core-api/memory-allocation.rst index 5954ddf6ee13..f9e8d352ed67 100644 --- a/Documentation/core-api/memory-allocation.rst +++ b/Documentation/core-api/memory-allocation.rst @@ -170,7 +170,14 @@ should be used if a part of the cache might be copied to the userspace. After the cache is created kmem_cache_alloc() and its convenience wrappers can allocate memory from that cache. -When the allocated memory is no longer needed it must be freed. You can -use kvfree() for the memory allocated with `kmalloc`, `vmalloc` and -`kvmalloc`. The slab caches should be freed with kmem_cache_free(). And -don't forget to destroy the cache with kmem_cache_destroy(). +When the allocated memory is no longer needed it must be freed. Objects +allocated by `kmalloc` can be freed by `kfree` or `kvfree`. +Objects allocated by `kmem_cache_alloc` can be freed with `kmem_cache_free` +or also by `kfree` or `kvfree`, which can be more convenient as it does +not require the kmem_cache pointed. +The rules for _bulk and _rcu flavors of freeing functions are analogical. + +Memory allocated by `vmalloc` can be freed with `vfree` or `kvfree`. +Memory allocated by `kvmalloc` can be freed with `kvfree`. +Caches created by `kmem_cache_create` should be freed with +`kmem_cache_destroy` only after freeing all the allocated objects first. diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h index 094321c17e48..dcd2cf1e8326 100644 --- a/include/linux/rcupdate.h +++ b/include/linux/rcupdate.h @@ -976,8 +976,10 @@ static inline notrace void rcu_read_unlock_sched_notrace(void) * either fall back to use of call_rcu() or rearrange the structure to * position the rcu_head structure into the first 4096 bytes. * - * Note that the allowable offset might decrease in the future, for example, - * to allow something like kmem_cache_free_rcu(). + * The object to be freed can be allocated either by kmalloc() or + * kmem_cache_alloc(). + * + * Note that the allowable offset might decrease in the future. * * The BUILD_BUG_ON check must not involve any function calls, hence the * checks are done in macros here. diff --git a/mm/slab_common.c b/mm/slab_common.c index 1522693295f5..607249785c07 100644 --- a/mm/slab_common.c +++ b/mm/slab_common.c @@ -989,12 +989,9 @@ EXPORT_SYMBOL(__kmalloc_node_track_caller); /** * kfree - free previously allocated memory - * @object: pointer returned by kmalloc. + * @object: pointer returned by kmalloc() or kmem_cache_alloc() * * If @object is NULL, no operation is performed. - * - * Don't free memory not originally allocated by kmalloc() - * or you will run into trouble. */ void kfree(const void *object) {
This will make it easier to free objects in situations when they can come from either kmalloc() or kmem_cache_alloc(), and also allow kfree_rcu() for freeing objects from kmem_cache_alloc(). For the SLAB and SLUB allocators this was always possible so with SLOB gone, we can document it as supported. Signed-off-by: Vlastimil Babka <vbabka@suse.cz> Cc: Mike Rapoport <rppt@kernel.org> Cc: Jonathan Corbet <corbet@lwn.net> Cc: "Paul E. McKenney" <paulmck@kernel.org> Cc: Frederic Weisbecker <frederic@kernel.org> Cc: Neeraj Upadhyay <quic_neeraju@quicinc.com> Cc: Josh Triplett <josh@joshtriplett.org> Cc: Steven Rostedt <rostedt@goodmis.org> Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> Cc: Lai Jiangshan <jiangshanlai@gmail.com> Cc: Joel Fernandes <joel@joelfernandes.org> --- Documentation/core-api/memory-allocation.rst | 15 +++++++++++---- include/linux/rcupdate.h | 6 ++++-- mm/slab_common.c | 5 +---- 3 files changed, 16 insertions(+), 10 deletions(-)