linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@transmeta.com>
To: Chris Mason <mason@suse.com>
Cc: linux-mm@kvack.org
Subject: Re: [PATCH] kill flush_dirty_buffers
Date: Mon, 6 Aug 2001 11:00:02 -0700 (PDT)	[thread overview]
Message-ID: <Pine.LNX.4.33.0108061048240.8972-100000@penguin.transmeta.com> (raw)
In-Reply-To: <663080000.997117789@tiny>

On Mon, 6 Aug 2001, Chris Mason wrote:
>
> Patch is lightly tested on ext2 and reiserfs, use at your own risk
> for now.  Linus, if this is what you were talking about in the
> vm suckage thread, I'll test/benchmark harder....

This is what I was talking about, but I'd rather have two separate
functions. Right now we have a simple "write_unlocked_buffers()" that is
very straightforward, and I hate having "flags" arguments to functions
that change their behaviour.

In general, the kind of code I like is

	static int generic_helper_fn(...)
	{
	...
	}

	static int another_helper_fn(..)
	{
	...
	}

	static int one_special_case(args)
	{
		if (generic_helper_fn(x) < args)
			another_helper_fn();
	}

	static int another_special_case(void)
	{
		while (another_helper_fn())
			wait_for_results();
	}


Rather than trying to have

	static int both_special_cases(args)
	{
		if (args) {
			if (generic_helper_fn(x) < args)
				another_helper_fn();
		} else {
			while (another_helper_fn())
				wait_for_results();
		}
	}

if you see what I mean?

I know that Computer Science is all about finding the generic solution to
a problem. But the fact is, that the specific solutions are usually
simpler and easier to understand, and if you name your functions
appropriately, it's MUCH easier to understand code that does

	age_page_up_locked(page);

than code that does

	age_page_up(page, 1);

(where "1" is the argument that tells the function that we've already
locked the page).

So I'd rather have a simple and straightforward function called

	write_unlocked_buffers(kdev_t dev)

that just writes all the buffers for that device, and then have _another_
function that does the flushtime checking etc, and is called something
appropriate.

And if they have code that is _unconditionally_ common, then that code can
be made a inline function or something, so that the two functions
themselves are smaller.

The other issue is that I suspect that "flushtime" is completely useless
these days, and should just be dropped. If we've decided to start flushing
stuff out, we shouldn't stop flushing just because some buffer hasn't
quite reached the proper age yet. We'd have been better off maybe deciding
not to even _start_ flushing at all, but once we've started, we might as
well do the dirty buffers we see (up to a maximum that is due to IO
_latency_, not due to "how long since this buffer was dirtied")

		Linus

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

  reply	other threads:[~2001-08-06 18:00 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2001-08-06 17:09 Chris Mason
2001-08-06 18:00 ` Linus Torvalds [this message]
2001-08-06 18:39   ` Rik van Riel
2001-08-06 19:53     ` Daniel Phillips
2001-08-07 21:19       ` Jens Axboe
2001-08-06 21:14   ` Chris Mason

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.LNX.4.33.0108061048240.8972-100000@penguin.transmeta.com \
    --to=torvalds@transmeta.com \
    --cc=linux-mm@kvack.org \
    --cc=mason@suse.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox