[BUG] in do_mmap_pgoff() (2.4.19-preX)

[BUG] in do_mmap_pgoff() (2.4.19-preX)

[Abhishek Nayani]

Hi,

	While documenting the do_mmap_pgoff() function, i found this
snippet of code very suspicious:

        /* Private writable mapping? Check memory availability.. */
	        if ((vm_flags & (VM_SHARED | VM_WRITE)) == VM_WRITE &&
                                           !(flags & MAP_NORESERVE) &&
			          !vm_enough_memory(len >> PAGE_SHIFT))
			return -ENOMEM; 
											 
	Here we need to quit if *any* one of the condition is true. So I
think it should be "||" instead of "&&". As according to the present
code, it quits only if all the 3 conditions is true, which is wrong.




					Bye,
						Abhi.
	
	
Linux Kernel Documentation Project
http://freesoftware.fsf.org/lkdp
--------------------------------------------------------------------------------
Those who cannot remember the past are condemned to repeat it - George Santayana
--------------------------------------------------------------------------------
                          Home Page: http://www.abhi.tk
-----BEGIN GEEK CODE BLOCK------------------------------------------------------
GCS d+ s:- a-- C+++ UL P+ L+++ E- W++ N+ o K- w--- O-- M- V- PS PE Y PGP 
t+ 5 X+ R- tv+ b+++ DI+ D G e++ h! !r y- 
------END GEEK CODE BLOCK-------------------------------------------------------

[BUG] in do_mmap_pgoff() (2.4.19-preX)

[Abhishek Nayani]

Hi,

	While documenting the do_mmap_pgoff() function, i found this
snippet of code very suspicious:

        /* Private writable mapping? Check memory availability.. */
	        if ((vm_flags & (VM_SHARED | VM_WRITE)) == VM_WRITE &&
                                           !(flags & MAP_NORESERVE) &&
			          !vm_enough_memory(len >> PAGE_SHIFT))
			return -ENOMEM; 
											 
	Here we need to quit if *any* one of the condition is true. So I
think it should be "||" instead of "&&". As according to the present
code, it quits only if all the 3 conditions is true, which is wrong.




					Bye,
						Abhi.
	
	
Linux Kernel Documentation Project
http://freesoftware.fsf.org/lkdp
--------------------------------------------------------------------------------
Those who cannot remember the past are condemned to repeat it - George Santayana
--------------------------------------------------------------------------------
                          Home Page: http://www.abhi.tk
-----BEGIN GEEK CODE BLOCK------------------------------------------------------
GCS d+ s:- a-- C+++ UL P+ L+++ E- W++ N+ o K- w--- O-- M- V- PS PE Y PGP 
t+ 5 X+ R- tv+ b+++ DI+ D G e++ h! !r y- 
------END GEEK CODE BLOCK-------------------------------------------------------
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/

Re: [BUG] in do_mmap_pgoff() (2.4.19-preX)

[Rik van Riel]

On Mon, 17 Jun 2002, Abhishek Nayani wrote:

> 	While documenting the do_mmap_pgoff() function, i found this
> snippet of code very suspicious:
>
>         /* Private writable mapping? Check memory availability.. */
> 	        if ((vm_flags & (VM_SHARED | VM_WRITE)) == VM_WRITE &&
>                                            !(flags & MAP_NORESERVE) &&
> 			          !vm_enough_memory(len >> PAGE_SHIFT))
> 			return -ENOMEM;
>
> 	Here we need to quit if *any* one of the condition is true. So I
> think it should be "||" instead of "&&". As according to the present
> code, it quits only if all the 3 conditions is true, which is wrong.

No, the code is correct.

The only case where we end up allocating new pages for
this mapping is when the mapping is both writable and
private.

Read-only mappings and shared mappings always have backing
store, the file from which the mapping comes and empty_zero_page.

regards,

Rik
-- 
Bravely reimplemented by the knights who say "NIH".

http://www.surriel.com/		http://distro.conectiva.com/

Re: [BUG] in do_mmap_pgoff() (2.4.19-preX)

[Rik van Riel]

On Mon, 17 Jun 2002, Abhishek Nayani wrote:

> 	While documenting the do_mmap_pgoff() function, i found this
> snippet of code very suspicious:
>
>         /* Private writable mapping? Check memory availability.. */
> 	        if ((vm_flags & (VM_SHARED | VM_WRITE)) == VM_WRITE &&
>                                            !(flags & MAP_NORESERVE) &&
> 			          !vm_enough_memory(len >> PAGE_SHIFT))
> 			return -ENOMEM;
>
> 	Here we need to quit if *any* one of the condition is true. So I
> think it should be "||" instead of "&&". As according to the present
> code, it quits only if all the 3 conditions is true, which is wrong.

No, the code is correct.

The only case where we end up allocating new pages for
this mapping is when the mapping is both writable and
private.

Read-only mappings and shared mappings always have backing
store, the file from which the mapping comes and empty_zero_page.

regards,

Rik
-- 
Bravely reimplemented by the knights who say "NIH".

http://www.surriel.com/		http://distro.conectiva.com/

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/