Re: [PATCH 09/32] Unionfs: cache-coherency - dentries

2007-09-03 Thread Jan Engelhardt

On Sep 3 2007 10:08, Josef 'Jeff' Sipek wrote:
>> >+int is_newer_lower(const struct dentry *dentry)
>> 
>> Could use bool and true/false as return value.
> 
>I remember that way back when there was a discussion about the bool type.
>What how did that end? Is bool preferred?

Well if there were objections, it would not be in the tree.


Jan
-- 
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 09/32] Unionfs: cache-coherency - dentries

2007-09-03 Thread Josef 'Jeff' Sipek
On Mon, Sep 03, 2007 at 08:52:17AM +0200, Jan Engelhardt wrote:
> 
> On Sep 2 2007 22:20, Josef 'Jeff' Sipek wrote:
> >@@ -184,10 +183,92 @@ out:
> > }
> > 
> > /*
> >+ * Determine if the lower inode objects have changed from below the unionfs
> >+ * inode.  Return 1 if changed, 0 otherwise.
> >+ */
> >+int is_newer_lower(const struct dentry *dentry)
> 
> Could use bool and true/false as return value.
 
I remember that way back when there was a discussion about the bool type.
What how did that end? Is bool preferred?

> >-int __unionfs_d_revalidate_chain(struct dentry *dentry, struct nameidata 
> >*nd)
> >+int __unionfs_d_revalidate_chain(struct dentry *dentry, struct nameidata 
> >*nd,
> >+ int willwrite)
> 
> also looks like a bool (willwrite)

Right.

> >-if (!__unionfs_d_revalidate_chain(dentry, NULL)) {
> >+if (!__unionfs_d_revalidate_chain(dentry, NULL, 0)) {
> 
> (Are there any callers with ,1?)

Indirectly yes. There are callers that pass a value they get. Very large
majority is 0.

Jeff.

-- 
Bad pun of the week: The formula 1 control computer suffered from a race
condition

-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 09/32] Unionfs: cache-coherency - dentries

2007-09-02 Thread Jan Engelhardt

On Sep 2 2007 22:20, Josef 'Jeff' Sipek wrote:
>@@ -184,10 +183,92 @@ out:
> }
> 
> /*
>+ * Determine if the lower inode objects have changed from below the unionfs
>+ * inode.  Return 1 if changed, 0 otherwise.
>+ */
>+int is_newer_lower(const struct dentry *dentry)

Could use bool and true/false as return value.

>-int __unionfs_d_revalidate_chain(struct dentry *dentry, struct nameidata *nd)
>+int __unionfs_d_revalidate_chain(struct dentry *dentry, struct nameidata *nd,
>+   int willwrite)

also looks like a bool (willwrite)

>-  if (!__unionfs_d_revalidate_chain(dentry, NULL)) {
>+  if (!__unionfs_d_revalidate_chain(dentry, NULL, 0)) {

(Are there any callers with ,1?)



Jan
-- 
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 09/32] Unionfs: cache-coherency - dentries

2007-09-02 Thread Josef 'Jeff' Sipek
From: Erez Zadok <[EMAIL PROTECTED]>

Utility functions to check if lower dentries/inodes are newer than upper
ones, and purging cached data if lower objects are newer.  Also passed flag
to our d_revalidate_chain, to tell it if the caller may be writing data or
just reading it.

[jsipek: changed purge_inode_data to take a struct inode]
Signed-off-by: Erez Zadok <[EMAIL PROTECTED]>
Signed-off-by: Josef 'Jeff' Sipek <[EMAIL PROTECTED]>
---
 fs/unionfs/commonfops.c |2 +-
 fs/unionfs/dentry.c |  127 --
 fs/unionfs/inode.c  |   24 ++---
 fs/unionfs/rename.c |4 +-
 fs/unionfs/super.c  |2 +-
 fs/unionfs/union.h  |3 +-
 fs/unionfs/unlink.c |   12 +++-
 fs/unionfs/xattr.c  |8 ++--
 8 files changed, 155 insertions(+), 27 deletions(-)

diff --git a/fs/unionfs/commonfops.c b/fs/unionfs/commonfops.c
index 28cb4e9..0dc7492 100644
--- a/fs/unionfs/commonfops.c
+++ b/fs/unionfs/commonfops.c
@@ -287,7 +287,7 @@ int unionfs_file_revalidate(struct file *file, int 
willwrite)
 * but not unhashed dentries.
 */
if (!d_deleted(dentry) &&
-   !__unionfs_d_revalidate_chain(dentry, NULL)) {
+   !__unionfs_d_revalidate_chain(dentry, NULL, willwrite)) {
err = -ESTALE;
goto out_nofree;
}
diff --git a/fs/unionfs/dentry.c b/fs/unionfs/dentry.c
index 4a3c479..c7bbeca 100644
--- a/fs/unionfs/dentry.c
+++ b/fs/unionfs/dentry.c
@@ -35,7 +35,6 @@ static int __unionfs_d_revalidate_one(struct dentry *dentry,
int positive = 0;
int locked = 0;
int interpose_flag;
-
struct nameidata lowernd; /* TODO: be gentler to the stack */
 
if (nd)
@@ -158,7 +157,7 @@ static int __unionfs_d_revalidate_one(struct dentry *dentry,
|| !lower_dentry->d_op->d_revalidate)
continue;
if (!lower_dentry->d_op->d_revalidate(lower_dentry,
-  &lowernd))
+ &lowernd))
valid = 0;
}
 
@@ -184,10 +183,92 @@ out:
 }
 
 /*
+ * Determine if the lower inode objects have changed from below the unionfs
+ * inode.  Return 1 if changed, 0 otherwise.
+ */
+int is_newer_lower(const struct dentry *dentry)
+{
+   int bindex;
+   struct inode *inode;
+   struct inode *lower_inode;
+
+   /* ignore if we're called on semi-initialized dentries/inodes */
+   if (!dentry || !UNIONFS_D(dentry))
+   return 0;
+   inode = dentry->d_inode;
+   if (!inode || !UNIONFS_I(inode) ||
+   ibstart(inode) < 0 || ibend(inode) < 0)
+   return 0;
+
+   for (bindex = ibstart(inode); bindex <= ibend(inode); bindex++) {
+   lower_inode = unionfs_lower_inode_idx(inode, bindex);
+   if (!lower_inode)
+   continue;
+   /*
+* We may want to apply other tests to determine if the
+* lower inode's data has changed, but checking for changed
+* ctime and mtime on the lower inode should be enough.
+*/
+   if (timespec_compare(&inode->i_mtime,
+&lower_inode->i_mtime) < 0) {
+   printk("unionfs: new lower inode mtime "
+  "(bindex=%d, name=%s)\n", bindex,
+  dentry->d_name.name);
+   return 1; /* mtime changed! */
+   }
+   if (timespec_compare(&inode->i_ctime,
+&lower_inode->i_ctime) < 0) {
+   printk("unionfs: new lower inode ctime "
+  "(bindex=%d, name=%s)\n", bindex,
+  dentry->d_name.name);
+   return 1; /* ctime changed! */
+   }
+   }
+   return 0;   /* default: lower is not newer */
+}
+
+/*
+ * Purge/remove/unmap all date pages of a unionfs inode.  This is called
+ * when the lower inode has changed, and we have to force processes to get
+ * the new data.
+ *
+ * XXX: Our implementation works in that as long as a user process will have
+ * caused Unionfs to be called, directly or indirectly, even to just do
+ * ->d_revalidate; then we will have purged the current Unionfs data and the
+ * process will see the new data.  For example, a process that continually
+ * re-reads the same file's data will see the NEW data as soon as the lower
+ * file had changed, upon the next read(2) syscall (even if the file is
+ * still open!)  However, this doesn't work when the process re-reads the
+ * open file's data via mmap(2) (unless the user unmaps/closes the file and
+ * remaps/reopens it).  Once we respond to ->readpage(s), then the kernel
+ * maps the page into the process's address space and there doesn't appear
+