[PATCH] radix-tree pagecache for 2.4.19-pre5-ac3

[PATCH] radix-tree pagecache for 2.4.19-pre5-ac3

[Art Haas]

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

Hi.

Once again the patch has been re-diffed for the latest -ac
kernel. It's running on my machine now. Enjoy!

I'm interested in hearing back from anyone using this
patch - I'd like to be sure that it applies cleanly (I
believe it does), and that the kernel built with the
patch runs well. The -ac kernels with rmap and the O(1)
scheduler are working well for me, and I'm hoping that
this patch adds to those kernels. At some point I'm hoping
the patch is included in to the -ac patchset, and the
eventual inclusion into the standard kernel release.

My thanks to everyone working on the kernel.

Art Haas

-- 
They that can give up essential liberty to obtain a little temporary
safety deserve neither liberty nor safety.
 -- Benjamin Franklin, Historical Review of Pennsylvania, 1759

[-- Attachment #2: radix-tree-2419p5ac3.patch.bz2 --]
[-- Type: application/octet-stream, Size: 12586 bytes --]

Re: [PATCH] radix-tree pagecache for 2.4.19-pre5-ac3

[William Lee Irwin III]

On Sun, Apr 07, 2002 at 11:44:39AM -0500, Art Haas wrote:
> Once again the patch has been re-diffed for the latest -ac
> kernel. It's running on my machine now. Enjoy!
> I'm interested in hearing back from anyone using this
> patch - I'd like to be sure that it applies cleanly (I
> believe it does), and that the kernel built with the
> patch runs well. The -ac kernels with rmap and the O(1)
> scheduler are working well for me, and I'm hoping that
> this patch adds to those kernels. At some point I'm hoping
> the patch is included in to the -ac patchset, and the
> eventual inclusion into the standard kernel release.
> My thanks to everyone working on the kernel.

Thank you! This has saved me some effort.

Burning question:
What are the hunks changing arch/i386/kernel/setup.c there for?
Also, there appears to be a livelock in add_to_swap(), (yes, this
has killed my boxen dead) something to help with this follows.
(It at least turned up a different problem I'm still working on.)


Cheers,
Bill


diff -urN linux-virgin/mm/swap_state.c linux/mm/swap_state.c
--- linux-virgin/mm/swap_state.c	Tue Apr  9 18:50:48 2002
+++ linux/mm/swap_state.c	Tue Apr  9 21:28:15 2002
@@ -104,6 +104,7 @@
  */
 int add_to_swap(struct page * page)
 {
+	int error;
 	swp_entry_t entry;
 
 	if (!PageLocked(page))
@@ -118,11 +119,15 @@
 		 * (adding to the page cache will clear the dirty
 		 * and uptodate bits, so we need to do it again)
 		 */
-		if (add_to_swap_cache(page, entry) == 0) {
+		error = add_to_swap_cache(page, entry);
+		if (!error) {
 			SetPageUptodate(page);
 			set_page_dirty(page);
 			swap_free(entry);
 			return 1;
+		} else if (error = -ENOMEM) {
+			swap_free(entry);
+			return 0;
 		}
 		/* Raced with "speculative" read_swap_cache_async */
 		swap_free(entry);
--
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: [PATCH] radix-tree pagecache for 2.4.19-pre5-ac3

[Art Haas]

On Wed, Apr 10, 2002 at 01:59:47PM -0700, William Lee Irwin III wrote:
> On Sun, Apr 07, 2002 at 11:44:39AM -0500, Art Haas wrote:
> [ ... snip ... ]
> 
> Thank you! This has saved me some effort.
> 
> Burning question:
> What are the hunks changing arch/i386/kernel/setup.c there for?

The initial patch I downloaded had those changes in there. Looking
at them now there doesn't seem to be any reason to include them. Perhaps
they were inadvertently added originally, and I've been continuing
that tradition ...

> Also, there appears to be a livelock in add_to_swap(), (yes, this
> has killed my boxen dead) something to help with this follows.
> (It at least turned up a different problem I'm still working on.)

Sorry to hear that. I haven't had any trouble on my machine, but
it's an old machine (200MHz Pentium), and I run desktop stuff, so
the load the patch is exposed to on this machine must not be enough
to trip things up. 

Thanks for the feedback and your additional patch! I have
one question about it though ...

> diff -urN linux-virgin/mm/swap_state.c linux/mm/swap_state.c
> --- linux-virgin/mm/swap_state.c	Tue Apr  9 18:50:48 2002
> +++ linux/mm/swap_state.c	Tue Apr  9 21:28:15 2002
> @@ -104,6 +104,7 @@
>   */
>  int add_to_swap(struct page * page)
>  {
> +	int error;
>  	swp_entry_t entry;
>  
>  	if (!PageLocked(page))
> @@ -118,11 +119,15 @@
>  		 * (adding to the page cache will clear the dirty
>  		 * and uptodate bits, so we need to do it again)
>  		 */
> -		if (add_to_swap_cache(page, entry) == 0) {
> +		error = add_to_swap_cache(page, entry);
> +		if (!error) {
>  			SetPageUptodate(page);
>  			set_page_dirty(page);
>  			swap_free(entry);
>  			return 1;
> +		} else if (error = -ENOMEM) {
> +			swap_free(entry);
> +			return 0;
>  		}
>  		/* Raced with "speculative" read_swap_cache_async */
>  		swap_free(entry);
> 

Should the new "else" clause be ...

		} else if (error == -ENOMEM) {

I think you've dropped an "=". Maybe this is the cause of the
other trouble you were seeing?

I hadn't posted it yet, but I've made a newer version of the
patch that incorporates the latest changes from Christoph Hellwig
and Andrew Morton - the changes of spin_lock() and spin_unlock()
to (read|write)_lock() and (read|write)_unlock, plus a few cosmetic
changes. I'm running a kernel with those changes right now (and things
work for me, but we've seen how that goes ...) . I'll rebuild my kernel
with your fix to the swap_state.c file and see how that kernel
performs. I can cook up a script to repeatedly build something and
let things run for a while. If it works I'll post my modified patch tomorrow.

Thanks again for the feedback!
-- 
They that can give up essential liberty to obtain a little temporary
safety deserve neither liberty nor safety.
 -- Benjamin Franklin, Historical Review of Pennsylvania, 1759
--
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: [PATCH] radix-tree pagecache for 2.4.19-pre5-ac3

[William Lee Irwin III]

On Wed, Apr 10, 2002 at 05:08:42PM -0500, Art Haas wrote:
> I think you've dropped an "=". Maybe this is the cause of the
> other trouble you were seeing?
> I hadn't posted it yet, but I've made a newer version of the
> patch that incorporates the latest changes from Christoph Hellwig
> and Andrew Morton - the changes of spin_lock() and spin_unlock()
> to (read|write)_lock() and (read|write)_unlock, plus a few cosmetic
> changes. I'm running a kernel with those changes right now (and things
> work for me, but we've seen how that goes ...) . I'll rebuild my kernel
> with your fix to the swap_state.c file and see how that kernel
> performs. I can cook up a script to repeatedly build something and
> let things run for a while. If it works I'll post my modified patch tomorrow.
> Thanks again for the feedback!

I did, you will need the == sorry.

I've been running Cerberus with this on a logical 8-way machine
(physical 4-way), it seems to hit about 2-5 minutes into the tests.
I'll back out my own changes and see what I turn up.


Cheers,
Bill
--
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: [PATCH] radix-tree pagecache for 2.4.19-pre5-ac3

[William Lee Irwin III]

On Wed, Apr 10, 2002 at 05:08:42PM -0500, Art Haas wrote:
> I think you've dropped an "=". Maybe this is the cause of the
> other trouble you were seeing?

I don't believe so. Just a vanilla 2.4.19-pre5-ac3 + your posted patch
+ corrected livelock fix (without it it livelocks instead) oopses at
fs/inode.c:515 within approximately 2 minutes:

    510 void clear_inode(struct inode *inode)
    511 {
    512         invalidate_inode_buffers(inode);
    513
    514         if (inode->i_data.nrpages)
    515                 BUG();
    516         if (!(inode->i_state & I_FREEING))
    517                 BUG();
    518         if (inode->i_state & I_CLEAR)
    519                 BUG();


Cheers,
Bill
--
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: [PATCH] radix-tree pagecache for 2.4.19-pre5-ac3

[William Lee Irwin III]

On Wed, Apr 10, 2002 at 05:08:42PM -0500, Art Haas wrote:
> Sorry to hear that. I haven't had any trouble on my machine, but
> it's an old machine (200MHz Pentium), and I run desktop stuff, so
> the load the patch is exposed to on this machine must not be enough
> to trip things up. 
> I think you've dropped an "=". Maybe this is the cause of the
> other trouble you were seeing?

No, it appears to be because all pagecache locking was removed from vmscan.c
Acquisitions and releases of pagecache_lock must be converted to the
analogous acquisitions and releases of the mapping->page_lock, with proper
movement of the points it's acquired and released for the per-mapping lock.
Testing with Cerberus on SMP machines helps find these issues.

The following hunks might need a bit more critical examination.


Cheers,
Bill


--- linux-2.4.19-pre5-ac3/mm/vmscan.c.ajh	2002-04-06 15:33:00.000000000 -0600
+++ linux-2.4.19-pre5-ac3/mm/vmscan.c	2002-04-06 15:33:45.000000000 -0600
@@ -84,11 +84,10 @@
 	int maxscan;
 
 	/*
-	 * We need to hold the pagecache_lock around all tests to make sure
-	 * reclaim_page() cannot race with find_get_page() and friends.
+	 * The pagecache_lock was removed with the addition of
+	 * the radix-tree patch.
 	 */
 	spin_lock(&pagemap_lru_lock);
-	spin_lock(&pagecache_lock);
 	maxscan = zone->inactive_clean_pages;
 	while (maxscan-- && !list_empty(&zone->inactive_clean_list)) {
 		page_lru = zone->inactive_clean_list.prev;
@@ -136,13 +135,11 @@
 		zone->inactive_clean_pages--;
 		UnlockPage(page);
 	}
-	spin_unlock(&pagecache_lock);
 	spin_unlock(&pagemap_lru_lock);
 	return NULL;
 
 found_page:
 	del_page_from_inactive_clean_list(page);
-	spin_unlock(&pagecache_lock);
 	spin_unlock(&pagemap_lru_lock);
 	if (entry.val)
 		swap_free(entry);
--
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: [PATCH] radix-tree pagecache for 2.4.19-pre5-ac3

[Art Haas]

On Thu, Apr 11, 2002 at 11:39:59AM -0700, William Lee Irwin III wrote:
> On Wed, Apr 10, 2002 at 05:08:42PM -0500, Art Haas wrote:
> > Sorry to hear that. I haven't had any trouble on my machine, but
> > it's an old machine (200MHz Pentium), and I run desktop stuff, so
> > the load the patch is exposed to on this machine must not be enough
> > to trip things up. 
> > I think you've dropped an "=". Maybe this is the cause of the
> > other trouble you were seeing?
> 
> No, it appears to be because all pagecache locking was removed from vmscan.c
> Acquisitions and releases of pagecache_lock must be converted to the
> analogous acquisitions and releases of the mapping->page_lock, with proper
> movement of the points it's acquired and released for the per-mapping lock.
> Testing with Cerberus on SMP machines helps find these issues.
> 
> The following hunks might need a bit more critical examination.
> 
> [ ... snip part of patch ... ]

I'll take any and all blame for the changes in vmscan.c that cause
problems.  I'm running on a machine with a single processor, and don't
have access to an SMP machine to test things on, so problems
arising on such machines would likely slip past me. :-(

I did go download Cerberus today, and started looking at what it
does, how to run it, etc.

I'll go dig into vmscan.c again. Thank you for your continued
efforts on reviewing and fixing the radix-tree patch.

-- 
They that can give up essential liberty to obtain a little temporary
safety deserve neither liberty nor safety.
 -- Benjamin Franklin, Historical Review of Pennsylvania, 1759
--
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/