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?)
next 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).