lkml.org 
[lkml]   [1998]   [Jan]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
Subjectpatch for 2.1.78 nfs client
The attached patch takes care of a few loose ends from the VFS
inode->dentry changes, and fixes a oops when using the NFS debugging
facility.

The patch makes a few additional changes to the VFS to notify the
filesystem when the inode is being released from a dentry. NFS needs to
be sure that the dentry remain valid for any pending writes, so when the
inode is being released it needs to flush any writes depending on the
dentry.

I've implemented this by always using a routine dentry_iput() in
dcache.c to free the inode, and this routine in turn checks for a d_iput
dentry_op. If defined, d_iput() is called with the dentry and inode as
arguments. (The dentry d_inode field is cleared in advance to forestall
any races.)

The nfs_dentry_iput() routine checks first for any pending writes for
the inode, and if necessary waits until all writes for the particular
dentry have been completed.

While testing using NFS debugging I found an oops caused by
dereferencing a NULL page->inode pointer when flushing dirty pages. This
was easily fixed in nfs/write.c by changing the debugging print to use
the dentry name instead of inode dev/ino.

However, it raises a question about how the page with the pending write
has been freed from the inode's page cache. There's no problem with the
page disappearing until the write completes (the wb_req holds a page
count), but I'm concerned that a subsequent write to the same file could
add another page back into the inode cache. This could create an
incoherency by having two dirty pages with inconsistent information.

Regards,
Bill--- linux-2.1.78/include/linux/nfs_fs.h.old Tue Jan 6 12:01:32 1998
+++ linux-2.1.78/include/linux/nfs_fs.h Tue Jan 6 12:27:23 1998
@@ -171,6 +171,7 @@
* linux/fs/nfs/write.c
*/
extern int nfs_writepage(struct dentry *, struct page *);
+extern int nfs_find_dentry_request(struct inode *, struct dentry *);
extern int nfs_check_failed_request(struct inode *);
extern int nfs_check_error(struct inode *);
extern int nfs_flush_dirty_pages(struct inode *, pid_t, off_t, off_t);
--- linux-2.1.78/fs/nfs/dir.c.old Tue Jan 6 11:39:07 1998
+++ linux-2.1.78/fs/nfs/dir.c Wed Jan 7 18:09:22 1998
@@ -35,6 +35,7 @@
extern void nfs_renew_times(struct dentry *);

#define NFS_PARANOIA 1
+/* #define NFS_DEBUG_VERBOSE 1 */

/*
* Head for a dircache entry. Currently still very simple; when
@@ -458,6 +459,28 @@
}

/*
+ * Called to free the inode from the dentry. We must flush
+ * any pending writes for this dentry before freeing the inode.
+ */
+static void nfs_dentry_iput(struct dentry *dentry, struct inode *inode)
+{
+ if (NFS_WRITEBACK(inode)) {
+#ifdef NFS_PARANOIA
+printk("nfs_dentry_iput: pending writes for %s/%s, i_count=%d\n",
+dentry->d_parent->d_name.name, dentry->d_name.name, inode->i_count);
+#endif
+ while (nfs_find_dentry_request(inode, dentry)) {
+#ifdef NFS_PARANOIA
+printk("nfs_dentry_iput: flushing %s/%s\n",
+dentry->d_parent->d_name.name, dentry->d_name.name);
+#endif
+ nfs_flush_dirty_pages(inode, 0, 0, 0);
+ }
+ }
+ iput(inode);
+}
+
+/*
* Called when the dentry is being freed to release private memory.
*/
static void nfs_dentry_release(struct dentry *dentry)
@@ -471,9 +494,53 @@
NULL, /* d_hash */
NULL, /* d_compare */
nfs_dentry_delete, /* d_delete(struct dentry *) */
+ nfs_dentry_iput, /* d_iput(struct dentry *, struct inode *) */
nfs_dentry_release /* d_release(struct dentry *) */
};

+#ifdef NFS_PARANOIA
+/*
+ * Display all dentries holding the specified inode.
+ */
+static void show_dentry(struct inode * inode)
+{
+ struct dentry *parent = inode->i_sb->s_root;
+ struct dentry *this_parent = parent;
+ struct list_head *next;
+
+repeat:
+ next = this_parent->d_subdirs.next;
+resume:
+ while (next != &this_parent->d_subdirs) {
+ struct list_head *tmp = next;
+ struct dentry *dentry = list_entry(tmp, struct dentry, d_child);
+ next = tmp->next;
+ if (dentry->d_inode == inode) {
+ int unhashed = list_empty(&dentry->d_hash);
+ printk("show_dentry: %s/%s, d_count=%d%s\n",
+ dentry->d_parent->d_name.name,
+ dentry->d_name.name, dentry->d_count,
+ unhashed ? "(unhashed)" : "");
+ }
+ /*
+ * Descend a level if the d_subdirs list is non-empty.
+ */
+ if (!list_empty(&dentry->d_subdirs)) {
+ this_parent = dentry;
+ goto repeat;
+ }
+ }
+ /*
+ * All done at this level ... ascend and resume the search.
+ */
+ if (this_parent != parent) {
+ next = this_parent->d_child.next;
+ this_parent = this_parent->d_parent;
+ goto resume;
+ }
+}
+#endif
+
/*
* Whenever a lookup succeeds, we know the parent directories
* are all valid, so we want to update the dentry timestamps.
@@ -531,10 +598,12 @@
inode = nfs_fhget(dentry->d_sb, &fhandle, &fattr);
if (inode) {
#ifdef NFS_PARANOIA
-if (inode->i_count > (S_ISDIR(inode->i_mode) ? 1 : inode->i_nlink))
+if (inode->i_count > (S_ISDIR(inode->i_mode) ? 1 : inode->i_nlink)) {
printk("nfs_lookup: %s/%s ino=%ld in use, count=%d, nlink=%d\n",
dentry->d_parent->d_name.name, dentry->d_name.name,
inode->i_ino, inode->i_count, inode->i_nlink);
+show_dentry(inode);
+}
#endif
no_entry:
d_add(dentry, inode);
@@ -564,10 +633,12 @@
inode = nfs_fhget(dentry->d_sb, fhandle, fattr);
if (inode) {
#ifdef NFS_PARANOIA
-if (inode->i_count > (S_ISDIR(inode->i_mode) ? 1 : inode->i_nlink))
+if (inode->i_count > (S_ISDIR(inode->i_mode) ? 1 : inode->i_nlink)) {
printk("nfs_instantiate: %s/%s ino=%ld in use, count=%d, nlink=%d\n",
dentry->d_parent->d_name.name, dentry->d_name.name,
inode->i_ino, inode->i_count, inode->i_nlink);
+show_dentry(inode);
+}
#endif
d_instantiate(dentry, inode);
nfs_renew_times(dentry);
@@ -682,10 +753,6 @@
sattr.uid = sattr.gid = sattr.size = (unsigned) -1;
sattr.atime.seconds = sattr.mtime.seconds = (unsigned) -1;

- nfs_invalidate_dircache(dir);
- error = nfs_proc_mkdir(NFS_SERVER(dir), NFS_FH(dentry->d_parent),
- dentry->d_name.name, &sattr, &fhandle, &fattr);
-
/*
* Always drop the dentry, we can't always depend on
* the fattr returned by the server (AIX seems to be
@@ -693,6 +760,9 @@
* depending on potentially bogus information.
*/
d_drop(dentry);
+ nfs_invalidate_dircache(dir);
+ error = nfs_proc_mkdir(NFS_DSERVER(dentry), NFS_FH(dentry->d_parent),
+ dentry->d_name.name, &sattr, &fhandle, &fattr);
out:
return error;
}
--- linux-2.1.78/fs/nfs/write.c.old Tue Jan 6 11:39:08 1998
+++ linux-2.1.78/fs/nfs/write.c Wed Jan 7 22:57:42 1998
@@ -66,6 +66,7 @@
*/
#define IS_SOFT 0

+#define NFS_PARANOIA 1
#define NFSDBG_FACILITY NFSDBG_PAGECACHE

static void nfs_wback_lock(struct rpc_task *task);
@@ -282,6 +283,27 @@
}

/*
+ * Find any requests for the specified dentry.
+ */
+int
+nfs_find_dentry_request(struct inode *inode, struct dentry *dentry)
+{
+ struct nfs_wreq *head, *req;
+ int found = 0;
+
+ req = head = NFS_WRITEBACK(inode);
+ while (req != NULL) {
+ if (req->wb_dentry == dentry) {
+ found = 1;
+ break;
+ }
+ if ((req = WB_NEXT(req)) == head)
+ break;
+ }
+ return found;
+}
+
+/*
* Find a failed write request by pid
*/
static struct nfs_wreq *
@@ -612,14 +634,18 @@
/*
* Flush out a dirty page.
*/
-static inline void
+static void
nfs_flush_request(struct nfs_wreq *req)
{
struct page *page = req->wb_page;

- dprintk("NFS: nfs_flush_request(%x/%ld, @%ld)\n",
- page->inode->i_dev, page->inode->i_ino,
- page->offset);
+#ifdef NFS_DEBUG_VERBOSE
+if (req->wb_inode != page->inode)
+printk("NFS: inode %ld no longer has page %p\n", req->wb_inode->i_ino, page);
+#endif
+ dprintk("NFS: nfs_flush_request(%s/%s, @%ld)\n",
+ req->wb_dentry->d_parent->d_name.name,
+ req->wb_dentry->d_name.name, page->offset);

req->wb_flags |= NFS_WRITE_WANTLOCK;
if (!test_and_set_bit(PG_locked, &page->flags)) {
@@ -656,7 +682,7 @@
if (rqoffset < end && offset < rqend
&& (pid == 0 || req->wb_pid == pid)) {
if (!WB_HAVELOCK(req)) {
-#ifdef NFS_PARANOIA
+#ifdef NFS_DEBUG_VERBOSE
printk("nfs_flush: flushing inode=%ld, %d @ %lu\n",
req->wb_inode->i_ino, req->wb_bytes, rqoffset);
#endif
@@ -664,11 +690,6 @@
}
last = req;
}
- } else {
-#ifdef NFS_PARANOIA
-printk("nfs_flush_pages: in progress inode=%ld, %d @ %lu\n",
-req->wb_inode->i_ino, req->wb_bytes, rqoffset);
-#endif
}
if (invalidate)
req->wb_flags |= NFS_WRITE_INVALIDATE;
--- linux-2.1.78/include/linux/dcache.h.old Tue Jan 6 11:39:09 1998
+++ linux-2.1.78/include/linux/dcache.h Tue Jan 6 11:58:23 1998
@@ -71,9 +71,10 @@

struct dentry_operations {
int (*d_revalidate)(struct dentry *);
- int (*d_hash) (struct dentry *,struct qstr *);
- int (*d_compare) (struct dentry *,struct qstr *, struct qstr *);
+ int (*d_hash) (struct dentry *, struct qstr *);
+ int (*d_compare) (struct dentry *, struct qstr *, struct qstr *);
void (*d_delete)(struct dentry *);
+ void (*d_iput)(struct dentry *, struct inode *);
void (*d_release)(struct dentry *);
};

--- linux-2.1.78/fs/dcache.c.old Tue Jan 6 11:39:05 1998
+++ linux-2.1.78/fs/dcache.c Wed Jan 7 14:06:56 1998
@@ -59,6 +60,22 @@
}

/*
+ * Release the dentry's inode, using the fileystem
+ * d_iput() operation if defined.
+ */
+static inline void dentry_iput(struct dentry * dentry)
+{
+ struct inode *inode = dentry->d_inode;
+ if (inode) {
+ dentry->d_inode = NULL;
+ if (dentry->d_op && dentry->d_op->d_iput)
+ dentry->d_op->d_iput(dentry, inode);
+ else
+ iput(inode);
+ }
+}
+
+/*
* dput()
*
* This is complicated by the fact that we do not want to put
@@ -104,13 +121,10 @@
list_del(&dentry->d_lru);
}
if (list_empty(&dentry->d_hash)) {
- struct inode *inode = dentry->d_inode;
struct dentry * parent;
+
list_del(&dentry->d_child);
- if (inode) {
- dentry->d_inode = NULL;
- iput(inode);
- }
+ dentry_iput(dentry);
parent = dentry->d_parent;
d_free(dentry);
if (dentry == parent)
@@ -259,12 +273,7 @@

list_del(&dentry->d_hash);
list_del(&dentry->d_child);
- if (dentry->d_inode) {
- struct inode * inode = dentry->d_inode;
-
- dentry->d_inode = NULL;
- iput(inode);
- }
+ dentry_iput(dentry);
parent = dentry->d_parent;
d_free(dentry);
dput(parent);
@@ -636,11 +713,7 @@
* Are we the only user?
*/
if (dentry->d_count == 1) {
- struct inode * inode = dentry->d_inode;
- if (inode) {
- dentry->d_inode = NULL;
- iput(inode);
- }
+ dentry_iput(dentry);
return;
}
\
 
 \ /
  Last update: 2005-03-22 13:41    [W:0.075 / U:3.356 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site