Skip to contentSkip to conversation

Document kfree and vfree NULL usage (resend)

PATCH In review Pekka Enberg 3 participants 11 messages Dec 3, 2004 – Dec 6, 2004 1 message not in the archive

Subsystems (2)
Memory management vmalloc
lore
Share & tools
lore

Pekka Enberg

author
#0 lore raw

PATCH 1/1 Document kfree and vfree NULL usage (resend)

Hi,

This patch adds comments for kfree() and vfree() stating that both accept NULL pointers.

Signed-off-by Pekka Enberg <[email protected]>

---

 slab.c    |    2 ++
 vmalloc.c |    3 ++-
 2 files changed, 4 insertions(+), 1 deletion(-)
mm/slab.c +2 -0
Index: 2.6.10-rc2/mm/slab.c
===================================================================
@@ -2535,6 +2535,8 @@
  * kfree - free previously allocated memory
  * @objp: pointer returned by kmalloc.
  *
+ * If @objp is NULL, no operation is performed.
+ *
  * Don't free memory not originally allocated by kmalloc()
  * or you will run into trouble.
  */
mm/vmalloc.c +2 -1
Index: 2.6.10-rc2/mm/vmalloc.c
===================================================================
@@ -389,7 +389,8 @@
  *	@addr:		memory base address
  *
  *	Free the virtually contiguous memory area starting at @addr, as
- *	obtained from vmalloc(), vmalloc_32() or __vmalloc().
+ *	obtained from vmalloc(), vmalloc_32() or __vmalloc(). If @addr is
+ *	NULL, no operation is performed.
  *
  *	May not be called in interrupt context.
  */
#1 lore raw

Hi Andrew,

I think it's worth to explicitely mention that kfree(NULL) is valid - too many users have/had their own (unnecessary) if(ptr) checks.

Quoted 6 lines · Pekka

This patch adds comments for kfree() and vfree() stating that both accept NULL pointers.

Signed-off-by Pekka Enberg <[email protected]>
Signed-off-by Manfred Spraul <[email protected]>

---

 slab.c    |    2 ++
 vmalloc.c |    3 ++-
 2 files changed, 4 insertions(+), 1 deletion(-)
mm/slab.c +2 -0
Index: 2.6.10-rc2/mm/slab.c
===================================================================
@@ -2535,6 +2535,8 @@
  * kfree - free previously allocated memory
  * @objp: pointer returned by kmalloc.
  *
+ * If @objp is NULL, no operation is performed.
+ *
  * Don't free memory not originally allocated by kmalloc()
  * or you will run into trouble.
  */
mm/vmalloc.c +2 -1
Index: 2.6.10-rc2/mm/vmalloc.c
===================================================================
@@ -389,7 +389,8 @@
  *	@addr:		memory base address
  *
  *	Free the virtually contiguous memory area starting at @addr, as
- *	obtained from vmalloc(), vmalloc_32() or __vmalloc().
+ *	obtained from vmalloc(), vmalloc_32() or __vmalloc(). If @addr is
+ *	NULL, no operation is performed.
  *
  *	May not be called in interrupt context.
  */
#2 lore raw
Quoted 1 line

* May not be called in interrupt context

Does this need to change to
* Must not be called in interrupt context
?
Is there a case where it is guaranteed that kfree will not sleep? "May" sounds to me like "You can call it in interrupt ctx, but it is not recommended", whereas calling it in interrupt ctx is definitely not recommended.

Parag

Quoted 54 lines · Manfred Spraul

Hi Andrew,

I think it's worth to explicitely mention that kfree(NULL) is valid - too many users have/had their own (unnecessary) if(ptr) checks.

Quoted 4 lines · Pekka

This patch adds comments for kfree() and vfree() stating that both accept NULL pointers.

Signed-off-by Pekka Enberg <[email protected]>
Signed-off-by Manfred Spraul <[email protected]>

---

 slab.c    |    2 ++
 vmalloc.c |    3 ++-
 2 files changed, 4 insertions(+), 1 deletion(-)

Index: 2.6.10-rc2/mm/slab.c
===================================================================

mm/slab.c +2 -0
@@ -2535,6 +2535,8 @@
  * kfree - free previously allocated memory
  * @objp: pointer returned by kmalloc.
  *
+ * If @objp is NULL, no operation is performed.
+ *
  * Don't free memory not originally allocated by kmalloc()
  * or you will run into trouble.
  */

Index: 2.6.10-rc2/mm/vmalloc.c
===================================================================

mm/vmalloc.c +2 -2
@@ -389,7 +389,8 @@
  *	@addr:		memory base address
  *
  *	Free the virtually contiguous memory area starting at @addr, as
- *	obtained from vmalloc(), vmalloc_32() or __vmalloc().
+ *	obtained from vmalloc(), vmalloc_32() or __vmalloc(). If @addr is
+ *	NULL, no operation is performed.
  *
  *	May not be called in interrupt context.
  */
 
 
 
-

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

#3 lore raw
Quoted 8 lines · Kernel Stuff
Quoted 3 lines

* May not be called in interrupt context

Does this need to change to
* Must not be called in interrupt context
?
Is there a case where it is guaranteed that kfree will not sleep?

kfree never sleeps. The comment you mention is part of the vfree documentation.

And you are right: for vfree, it's "must not be called". I'll send a separate patch. Or Andrew could just change it directly.

--
Manfred

#4 lore raw

The attached patch changes the vfree() documentation to correct "May not be called in interrupt context" to "Must not be called in interrupt context". Latter is compliant to RFC2119 and matches the absolute requirement for vfree.

Is not the same requirement true for vmalloc() - or is it ok to call vmalloc() in interrupt?

Parag

Quoted 14 lines · Manfred Spraul
Quoted 6 lines · Kernel Stuff
Quoted 1 line

* May not be called in interrupt context

Does this need to change to
* Must not be called in interrupt context
?
Is there a case where it is guaranteed that kfree will not sleep?

kfree never sleeps. The comment you mention is part of the vfree documentation.

And you are right: for vfree, it's "must not be called". I'll send a separate patch. Or Andrew could just change it directly.

Quoted 7 lines

--
Manfred
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

mm/vmalloc.c renamed from mm/vmalloc.c.orig +1 -1
@@ -340,7 +340,7 @@ void __vunmap(void *addr, int deallocate
  *	Free the virtually contiguous memory area starting at @addr, as
  *	obtained from vmalloc(), vmalloc_32() or __vmalloc().
  *
- *	May not be called in interrupt context.
+ *	Must not be called in interrupt context.
  */
 void vfree(void *addr)
 {
#5 lore raw
Quoted 6 lines · Kernel Stuff

The attached patch changes the vfree() documentation to correct "May not be called in interrupt context" to "Must not be called in interrupt context". Latter is compliant to RFC2119 and matches the absolute requirement for vfree.

Is not the same requirement true for vmalloc() - or is it ok to call vmalloc() in interrupt?

No, it's not ok.
But that's not something worth mentioning: only a few functions are permitted from interrupt context. The special thing about vfree is the asymmetry: kfree from irq context is ok, vfree is forbidden. That's why it's documented.

--
Manfred

#6 lore raw

Ok. So need for changing vmalloc() documentation then.

Here is the same previous patch inline instead of the attachment, with Signed-off-by -

(Sorry this is my first patch !)

Parag

Signed-off-by Parag Warudkar <[email protected]>
mm/vmalloc.c 2004-12-05 12:37:17.279076472 -0500 renamed from mm/vmalloc.c.orig 2004-12-05 12:40:50.699631616 -0500 +1 -1
@@ -340,7 +340,7 @@ void __vunmap(void *addr, int deallocate
  *     Free the virtually contiguous memory area starting at @addr, as
  *     obtained from vmalloc(), vmalloc_32() or __vmalloc().
  *
- *     May not be called in interrupt context.
+ *     Must not be called in interrupt context.
  */
 void vfree(void *addr)
 {
Quoted 26 lines · Manfred Spraul
Quoted 10 lines · Kernel Stuff

The attached patch changes the vfree() documentation to correct "May not be called in interrupt context" to "Must not be called in interrupt context". Latter is compliant to RFC2119 and matches the absolute requirement for vfree.

Is not the same requirement true for vmalloc() - or is it ok to call vmalloc() in interrupt?

No, it's not ok.
But that's not something worth mentioning: only a few functions are permitted from interrupt context. The special thing about vfree is the
asymmetry: kfree from irq context is ok, vfree is forbidden. That's why it's documented.

--
Manfred
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

author
#10 lore raw
In reply to Manfred Spraul
Quoted 2 lines · Manfred Spraul

And you are right: for vfree, it's "must not be called". I'll send a separate patch. Or Andrew could just change it directly.

Please do it for vunmap() also.

Pekka

author
#7 lore raw
In reply to Parag Warudkar

Hi Parag,

Quoted 3 lines · Parag Warudkar

Ok. So need for changing vmalloc() documentation then.

Here is the same previous patch inline instead of the attachment, with Signed-off-by -

Your patch conflicts with mine so it will not apply cleanly. Furthermore, your mailer seems to throw in some control characters in the mix and GNU patch does not like that.

Pekka

#11 lore raw
In reply to Pekka Enberg
Quoted 9 lines · Pekka Enberg
Quoted 2 lines · Manfred Spraul

And you are right: for vfree, it's "must not be called". I'll send a separate patch. Or Andrew could just change it directly.

Please do it for vunmap() also.

Pekka

Ok. Patch for vfree and vunmap below. Please discard previous patch for kfree().

Parag

Signed-off-by Parag Warudkar <[email protected]>
mm/vmalloc.c 2004-12-05 15:33:26.344334328 -0500 renamed from mm/vmalloc.c.orig 2004-12-05 12:40:50.699631616 -0500 +2 -2
@@ -340,7 +340,7 @@ void __vunmap(void *addr, int deallocate
  *     Free the virtually contiguous memory area starting at @addr, as
  *     obtained from vmalloc(), vmalloc_32() or __vmalloc().
  *
- *     May not be called in interrupt context.
+ *     Must not be called in interrupt context.
  */
 void vfree(void *addr)
 {

@@ -358,7 +358,7 @@ EXPORT_SYMBOL(vfree);
  *     Free the virtually contiguous memory area starting at @addr,
  *     which was created from the page array passed to vmap().
  *
- *     May not be called in interrupt context.
+ *     Must not be called in interrupt context.
  */
 void vunmap(void *addr)
 {
author
#9 lore raw
In reply to a message not in the archive

Hi Parag,

Quoted 8 lines · Parag Warudkar

I was not able to apply your patch which I copied from KMail - so looks like KMail is inserting some control chars.

So here is what I did - I applied your patch to vmalloc.c manually and then modified vmalloc.c to include my changes. Attached is the new patch.
If this doesn't work would you mind manually applying vfree() and vunmap() doc changes to your patch and submit?

I appreciate the help but please don't do this. You're creating more work for me and confusion to the maintainers now. If Andrew already picked up my patch, your new patch will not apply. If he didn't, he cannot use your patch either because you left out the kfree() part completely. Manfred said he would send a separate patch so please respect that.

Andrew, please apply the original patch I sent and ignore the follow up patches to this thread.

Pekka