All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Chuck Lever <cel@monkey.org>
To: "Benjamin C.R. LaHaise" <blah@kvack.org>
Cc: Linus Torvalds <torvalds@transmeta.com>, linux-mm@kvack.org
Subject: Re: [patch] mmap<->write deadlock fix, plus bug in block_write_zero_range
Date: Wed, 22 Dec 1999 23:00:40 -0500 (EST)	[thread overview]
Message-ID: <Pine.BSO.4.10.9912222254540.25860-100000@funky.monkey.org> (raw)
In-Reply-To: <Pine.LNX.3.96.991222103000.22064A-100000@kanga.kvack.org>

On Wed, 22 Dec 1999, Benjamin C.R. LaHaise wrote:
> On Wed, 22 Dec 1999, Chuck Lever wrote:
> > On Wed, 22 Dec 1999, Benjamin C.R. LaHaise wrote:
> > i've tried this before several times.  i could never get the system to
> > perform as well under benchmark load using find_page_nolock as when using
> > find_get_page. the throughput difference was about 5%, if i recall.  i
> > haven't explained this to myself yet.
>
> Here's my hypothesis about why find_page_nolock vs find_get_page makes a
> difference: using find_page_nolock means that we'll never do a
> run_task_queue(&tq_disk); to get our async readahead requests run.  So, in
> theory, doing that in filemap_nopage will restore performance.  Isn't
> there a way that the choice of when to run tq_disk could be made a bit
> less arbitrary?

this patch appears to have negligible effect on benchmark throughput
measurements, whereas, without the run_task_queue, throughput drops.

btw, i notice that a "read_cache_page" function has appeared that looks
similar to "page_cache_read" -- is there necessity for both?

--- linux-2.3.34-ref/mm/filemap.c	Wed Dec 22 21:23:03 1999
+++ linux/mm/filemap.c	Wed Dec 22 22:53:19 1999
@@ -1325,9 +1325,13 @@
 	 */
 	hash = page_hash(&inode->i_data, pgoff);
 retry_find:
-	page = __find_get_page(&inode->i_data, pgoff, hash);
+	spin_lock(&pagecache_lock);
+	page = __find_page_nolock(&inode->i_data, pgoff, *hash);
 	if (!page)
 		goto no_cached_page;
+	get_page(page);
+	spin_unlock(&pagecache_lock);
+	run_task_queue(&tq_disk);
 
 	/*
 	 * Ok, found a page in the page cache, now we need to check
@@ -1358,6 +1362,8 @@
 	return old_page;
 
 no_cached_page:
+	spin_unlock(&pagecache_lock);
+
 	/*
 	 * If the requested offset is within our file, try to read a whole 
 	 * cluster of pages at once.

	- Chuck Lever
--
corporate:	<chuckl@netscape.com>
personal:	<chucklever@netscape.net> or <cel@monkey.org>

The Linux Scalability project:
	http://www.citi.umich.edu/projects/linux-scalability/

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

      parent reply	other threads:[~1999-12-23  4:00 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
1999-12-22  5:58 [patch] mmap<->write deadlock fix, plus bug in block_write_zero_range Benjamin C.R. LaHaise
1999-12-22 15:08 ` Chuck Lever
1999-12-22 15:43   ` Benjamin C.R. LaHaise
1999-12-22 15:58     ` Chuck Lever
1999-12-23  4:00     ` Chuck Lever [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Pine.BSO.4.10.9912222254540.25860-100000@funky.monkey.org \
    --to=cel@monkey.org \
    --cc=blah@kvack.org \
    --cc=linux-mm@kvack.org \
    --cc=torvalds@transmeta.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.