[PATCH] Document kfree and vfree NULL usage (resend)

[PATCH] Document kfree and vfree NULL usage (resend)

[Pekka Enberg]

Hi,

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

Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>
---

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

Index: 2.6.10-rc2/mm/slab.c
===================================================================
--- 2.6.10-rc2.orig/mm/slab.c	2004-11-27 14:33:14.000000000 +0200
+++ 2.6.10-rc2/mm/slab.c	2004-11-27 16:12:54.573387384 +0200
@@ -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
===================================================================
--- 2.6.10-rc2.orig/mm/vmalloc.c	2004-11-27 16:13:48.026261312 +0200
+++ 2.6.10-rc2/mm/vmalloc.c	2004-11-27 16:14:04.875699808 +0200
@@ -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.
  */

Re: [PATCH] Document kfree and vfree NULL usage (resend)

[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.

Pekka wrote:
>
> This patch adds comments for kfree() and vfree() stating that both
> accept NULL pointers.
>
> Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>
>
Signed-Off-By: Manfred Spraul <manfred@colorfullife.com>

---

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

Index: 2.6.10-rc2/mm/slab.c
===================================================================
--- 2.6.10-rc2.orig/mm/slab.c	2004-11-27 14:33:14.000000000 +0200
+++ 2.6.10-rc2/mm/slab.c	2004-11-27 16:12:54.573387384 +0200
@@ -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
===================================================================
--- 2.6.10-rc2.orig/mm/vmalloc.c	2004-11-27 16:13:48.026261312 +0200
+++ 2.6.10-rc2/mm/vmalloc.c	2004-11-27 16:14:04.875699808 +0200
@@ -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.
  */

Re: [PATCH] Document kfree and vfree NULL usage (resend)

[Kernel Stuff]

>   *	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

On Sunday 05 December 2004 10:33 am, Manfred Spraul wrote:
> 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.
>
> Pekka wrote:
> > This patch adds comments for kfree() and vfree() stating that both
> > accept NULL pointers.
> >
> > Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>
>
> Signed-Off-By: Manfred Spraul <manfred@colorfullife.com>
>
> ---
>
>  slab.c    |    2 ++
>  vmalloc.c |    3 ++-
>  2 files changed, 4 insertions(+), 1 deletion(-)
>
> Index: 2.6.10-rc2/mm/slab.c
> ===================================================================
> --- 2.6.10-rc2.orig/mm/slab.c	2004-11-27 14:33:14.000000000 +0200
> +++ 2.6.10-rc2/mm/slab.c	2004-11-27 16:12:54.573387384 +0200
> @@ -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
> ===================================================================
> --- 2.6.10-rc2.orig/mm/vmalloc.c	2004-11-27 16:13:48.026261312 +0200
> +++ 2.6.10-rc2/mm/vmalloc.c	2004-11-27 16:14:04.875699808 +0200
> @@ -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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] Document kfree and vfree NULL usage (resend)

[Manfred Spraul]

Kernel Stuff wrote:

>>  *	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

Re: [PATCH] Document kfree and vfree NULL usage (resend)

[Kernel Stuff]

[-- Attachment #1: Type: text/plain, Size: 1119 bytes --]

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

On Sunday 05 December 2004 11:59 am, Manfred Spraul wrote:
> Kernel Stuff wrote:
> >>  *	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
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

[-- Attachment #2: patch --]
[-- Type: text/x-diff, Size: 443 bytes --]

--- linux-mod/mm/vmalloc.c.orig	2004-12-05 12:40:50.699631616 -0500
+++ linux-mod/mm/vmalloc.c	2004-12-05 12:37:17.279076472 -0500
@@ -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)
 {

Re: [PATCH] Document kfree and vfree NULL usage (resend)

[Manfred Spraul]

Kernel Stuff wrote:

>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

Re: [PATCH] Document kfree and vfree NULL usage (resend)

[Parag Warudkar]

[-- Attachment #1: Type: text/plain, Size: 1744 bytes --]

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 <kernel-stuff@comcast.net>

--- linux-mod/mm/vmalloc.c.orig 2004-12-05 12:40:50.699631616 -0500
+++ linux-mod/mm/vmalloc.c      2004-12-05 12:37:17.279076472 -0500
@@ -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)
 {

On Sunday 05 December 2004 12:57 pm, Manfred Spraul wrote:
> Kernel Stuff wrote:
> 
> >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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] Document kfree and vfree NULL usage (resend)

[Pekka Enberg]

Hi Parag,

On Sun, 2004-12-05 at 13:12 -0500, Parag Warudkar wrote:
> 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

Re: [PATCH] Document kfree and vfree NULL usage (resend)

[Pekka Enberg]

Hi Parag,

On Sun, 2004-12-05 at 16:48 -0500, Parag Warudkar wrote:
> 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

Re: [PATCH] Document kfree and vfree NULL usage (resend)

[Pekka Enberg]

On Sun, 2004-12-05 at 17:59 +0100, Manfred Spraul wrote:
> 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

Re: [PATCH] Document kfree and vfree NULL usage (resend)

[Parag Warudkar]

[-- Attachment #1: Type: text/plain, Size: 1264 bytes --]

On Sunday 05 December 2004 03:21 pm, Pekka Enberg wrote:
> On Sun, 2004-12-05 at 17:59 +0100, Manfred Spraul wrote:
> > 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 <kernel-stuff@comcast.net>

--- linux-mod/mm/vmalloc.c.orig 2004-12-05 12:40:50.699631616 -0500
+++ linux-mod/mm/vmalloc.c      2004-12-05 15:33:26.344334328 -0500
@@ -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)
 {

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]