shrink_mmap bug in 2.2?

shrink_mmap bug in 2.2?

[Neil Schemenauer]

This code looks strange to me (possibly because I don't
understand it):

    /*
     * Is it a page swap page? If so, we want to
     * drop it if it is no longer used, even if it
     * were to be marked referenced..
     */
    if (PageSwapCache(page)) {
            if (referenced && swap_count(page->offset) != 1)
                    continue;
            delete_from_swap_cache(page);
            return 1;
    }       

Can pages be deleted from the swap cache if swap_count is not
one?  If not, then I think this code is wrong.  It should be:

    if (PageSwapCache(page)) {
            if (swap_count(page->offset) != 1)
                    continue;
            delete_from_swap_cache(page);
            return 1;
    }       
 
Thanks in advance for any help.

    Neil

-- 
"God, root, what is difference?" - Pitr
"God is more forgiving." - Dave Aronson
--
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.eu.org/Linux-MM/

Re: shrink_mmap bug in 2.2?

[Kanoj Sarcar]

> 
> This code looks strange to me (possibly because I don't
> understand it):
> 
>     /*
>      * Is it a page swap page? If so, we want to
>      * drop it if it is no longer used, even if it
>      * were to be marked referenced..
>      */
>     if (PageSwapCache(page)) {
>             if (referenced && swap_count(page->offset) != 1)
>                     continue;
>             delete_from_swap_cache(page);
>             return 1;
>     }       

Aren't you misreading the logic here? It is

	referenced && swap_count(page->offset) != 1)
	          ^^^^
and not

	referenced || swap_count(page->offset) != 1)
                 ^^^^^

So delete_from_swap_cache will only ever be called on a page
with swap_count(page->offset) == 1.

Kanoj

> 
> Can pages be deleted from the swap cache if swap_count is not
> one?  If not, then I think this code is wrong.  It should be:
> 
>     if (PageSwapCache(page)) {
>             if (swap_count(page->offset) != 1)
>                     continue;
>             delete_from_swap_cache(page);
>             return 1;
>     }       
>  
--
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.eu.org/Linux-MM/

Re: shrink_mmap bug in 2.2?

[volodya]

On Wed, 14 Jun 2000, Kanoj Sarcar wrote:

> > 
> > This code looks strange to me (possibly because I don't
> > understand it):
> > 
> >     /*
> >      * Is it a page swap page? If so, we want to
> >      * drop it if it is no longer used, even if it
> >      * were to be marked referenced..
> >      */
> >     if (PageSwapCache(page)) {
> >             if (referenced && swap_count(page->offset) != 1)
> >                     continue;
> >             delete_from_swap_cache(page);
> >             return 1;
> >     }       
> 
> Aren't you misreading the logic here? It is
> 
> 	referenced && swap_count(page->offset) != 1)
> 	          ^^^^
> and not
> 
> 	referenced || swap_count(page->offset) != 1)
>                  ^^^^^
> 
> So delete_from_swap_cache will only ever be called on a page
> with swap_count(page->offset) == 1.
> 

This evades me. We delete when the condition is false. So if referenced is
0  if will not happen and we delete the page..

                                 Vladimir Dergachev
> Kanoj
> 
> > 
> > Can pages be deleted from the swap cache if swap_count is not
> > one?  If not, then I think this code is wrong.  It should be:
> > 
> >     if (PageSwapCache(page)) {
> >             if (swap_count(page->offset) != 1)
> >                     continue;
> >             delete_from_swap_cache(page);
> >             return 1;
> >     }       
> >  
> --
> 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.eu.org/Linux-MM/
> 

--
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.eu.org/Linux-MM/

Re: shrink_mmap bug in 2.2?

[Kanoj Sarcar]

> 
> 
> 
> On Wed, 14 Jun 2000, Kanoj Sarcar wrote:
> 
> > > 
> > > This code looks strange to me (possibly because I don't
> > > understand it):
> > > 
> > >     /*
> > >      * Is it a page swap page? If so, we want to
> > >      * drop it if it is no longer used, even if it
> > >      * were to be marked referenced..
> > >      */
> > >     if (PageSwapCache(page)) {
> > >             if (referenced && swap_count(page->offset) != 1)
> > >                     continue;
> > >             delete_from_swap_cache(page);
> > >             return 1;
> > >     }       
> > 
> > Aren't you misreading the logic here? It is
> > 
> > 	referenced && swap_count(page->offset) != 1)
> > 	          ^^^^
> > and not
> > 
> > 	referenced || swap_count(page->offset) != 1)
> >                  ^^^^^
> > 
> > So delete_from_swap_cache will only ever be called on a page
> > with swap_count(page->offset) == 1.
> > 
> 
> This evades me. We delete when the condition is false. So if referenced is
> 0  if will not happen and we delete the page..

Oh okay, I was confused myself. 

Looking at 2.4.0-test1, I think the code is now,

                /*
                 * Is it a page swap page? If so, we want to
                 * drop it if it is no longer used, even if it
                 * were to be marked referenced..
                 */
                if (PageSwapCache(page)) {
                        spin_unlock(&pagecache_lock);
                        __delete_from_swap_cache(page);
                        goto made_inode_progress;
                }


So, the code that Neil posted is a little older. In any case, the
logic there is this: if the page has been referenced, it is evidently
not a good decision to steal it, since due to time-locality of reference,
it is going to be accessed soon again, and we do not want to incur a 
swapin at that point (if we steal the page and reuse it for another
purpose, thereby loosing the current contents). 

Even if the page has been referenced, the contents of the page are
already on the swap, so if the swap_count of the page is 1, it is 
due to being in the swapcache itself (no user process has a reference 
to the page or the corresponding swap handle). Good candidate to be
stolen.

If the page has not been referenced, it is a good candidate for stealing.
Since the contents are already on swap, it does not matter how many
user processes have references to the swap handle, they will incur a
swapin cost when they access the corresponding user virtual address.
At this point, the page must go out of the swapcache, since it is
potentially going to be used for another purpose.

Another thing to note is that this code is executed on a page which
has no user reference count to the page, since all such references
have been converted to references to the corresponding swap handle.

Kanoj

> 
>                                  Vladimir Dergachev
> > Kanoj
> > 
> > > 
> > > Can pages be deleted from the swap cache if swap_count is not
> > > one?  If not, then I think this code is wrong.  It should be:
> > > 
> > >     if (PageSwapCache(page)) {
> > >             if (swap_count(page->offset) != 1)
> > >                     continue;
> > >             delete_from_swap_cache(page);
> > >             return 1;
> > >     }       
> > >  
> > --
> > 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.eu.org/Linux-MM/
> > 
> 

--
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.eu.org/Linux-MM/

Re: shrink_mmap bug in 2.2?

[Neil Schemenauer]

On Wed, Jun 14, 2000 at 06:16:10PM -0700, Kanoj Sarcar wrote:
> Aren't you misreading the logic here? It is
> 
> 	referenced && swap_count(page->offset) != 1)
> 	          ^^^^
[...]
> So delete_from_swap_cache will only ever be called on a page
> with swap_count(page->offset) == 1.

If the page is not referenced it will be removed regardless of
the swap_count.  My question is "does swap_count have to equal
one before calling delete_from_swap_cache"?  If it does then I
think the code is wrong.

I wouldn't consider this code old as it is in the current stable
kernel (2.2.16).  

  Neil

-- 
145 = 1! + 4! + 5!
--
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.eu.org/Linux-MM/

Re: shrink_mmap bug in 2.2?

[Stephen C. Tweedie]

Hi,

On Thu, Jun 15, 2000 at 01:12:41PM -0600, Neil Schemenauer wrote:
> 
> If the page is not referenced it will be removed regardless of
> the swap_count.  My question is "does swap_count have to equal
> one before calling delete_from_swap_cache"?  If it does then I
> think the code is wrong.

No.  The swap cache is only an in-memory cached copy of the on-disk
swap page.  It is quite safe to delete the swap cache page in memory
no matter how many references to the on-disk copy there are.  If
this were not true, we'd be unable to delete any swapped-out pages
from ram!

Cheers,
 Stephen
--
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.eu.org/Linux-MM/