linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@infradead.org>
To: dhowells@redhat.com, akpm@osdl.org
Cc: linux-fsdevel@vger.kernel.org
Subject: fscache review comments, part 1
Date: Thu, 28 Sep 2006 17:45:29 +0100	[thread overview]
Message-ID: <20060928164529.GA3497@infradead.org> (raw)

Just to be on the same boat, all these comments are against
http://people.redhat.com/~dhowells/nfs/nfs+fscache-16.tar.bz2

This mail is just the core-touching paches, the fscache and cachefiles
code will get a review of it's own later on.

01-ino64.diff:
	ACK

02-ino64-nfs.diff:
	Unfortunately there's a lot of broken userspace that can't deal
	with 64bit inode numbers, so you need to make the lod behaviour
	a mount option at least, probably even the default.  Given that
	we're going to run into problems like that it might make sense
	to make the option VFS-level instead of just in nfs.  (Note:
	XFS already has an option like that)

03-fsmisc.diff:
	ACK

05-release-page.diff:
	ACK, though I'd wish someone could come up with nicer names for
	read_cache_pages_invalidate_page(s).

06-block-afs.diff:
	ACK.

09-cachefiles-ia64.diff:
	ACK.  (Although I remember at some point I wanted to unify
	copy_page and copy_highpage)

11-autofs-dcache.diff:
	ACK.  Should probably go in ASAP.

12-reiserfs-dcache.diff:
	ACK.  Again should probably go in ASAP.

	Note that reiserfs_kill_sb could be written a lot nicer by
	using a local variable.

static void reiserfs_kill_sb(struct super_block *sb)
{
	struct reiserfs_sb_info *si = REISERFS_SB(s);

	if (si) {
		if (si->xattr_root) {
			d_invalidate(si->xattr_root);
			dput(si->xattr_root);
		}
		if (si->priv_root) {
			d_invalidate(si->priv_root);
			dput(si->priv_root);
		}
	}

	kill_block_super(sb);
}

13-dcache-crunch.diff:
	I know Jan has been rather unhappy with this, and I tend to agree
	to at least the code duplication part.  I wish I had time to
	look into a better way to implement the rather nice idea behind
	shrink_dcache_for_umount.
	Why is this patch required for fscache anyway?
	(and why is it last in the series then?)

             reply	other threads:[~2006-09-28 16:45 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-09-28 16:45 Christoph Hellwig [this message]
2006-09-28 17:30 ` fscache review comments, part 1 Andreas Dilger
2006-09-29  0:02   ` Christoph Hellwig
2006-09-29  1:51     ` Timothy Shimmin
2006-09-29  8:38 ` David Howells
2006-10-02 13:40 ` David Howells
2006-10-02 17:39   ` Andrew Morton
2006-10-02 21:41     ` Christoph Hellwig
2006-10-04 13:44 ` Al Viro
2006-10-04 14:18 ` 64-bit inode number issues David Howells
2006-10-07 21:01   ` Christoph Hellwig
2006-10-09  9:01     ` David Chinner
2006-10-09 11:32     ` David Howells
2006-10-09 14:12       ` Arjan van de Ven
2006-10-09 23:53       ` David Chinner
2006-10-12 18:32     ` Benjamin LaHaise
2006-10-17  6:02       ` Chris Wedgwood
2006-10-09  7:58   ` David Howells
2006-10-04 14:23 ` David Howells
2006-10-08  2:00   ` Theodore Tso
2006-10-08 16:02     ` Alexander Viro
2006-10-09 13:46 ` fscache review comments, part 1 David Howells
2006-10-10 13:31   ` Christoph Hellwig

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=20060928164529.GA3497@infradead.org \
    --to=hch@infradead.org \
    --cc=akpm@osdl.org \
    --cc=dhowells@redhat.com \
    --cc=linux-fsdevel@vger.kernel.org \
    /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;
as well as URLs for NNTP newsgroup(s).