Re: [PATCH 1/2] afs: use d_time instead of d_fsdata

[David Howells <dhowells@redhat.com>]

NeilBrown <neilb@ownmail.net> wrote:

> afs uses ->d_fsdata to store version information for the parent
> directory.  ->d_time is arguably a better field to store this
> information as the version is like a time stamp, and ->d_time is an
> unsigned long, while ->d_fsdata is a void *.

It wasn't clear that ->d_time was available for private use by the filesystem.

> This will leave ->d_fsdata free for a different use ...  which
> admittedly is also not a void*, but is certainly not at all a time.

Are you going to use ->d_fsdata for something generic?

> Interestingly the value stored in ->d_time or d_fsdata is u64 which does
> not fit in "unsigned long" or "void *" on 32 bit hosts.  Maybe that
> doesn't matter.

I know.  I just have to hope that someone triggers revalidation on a dirent
before 4 billion changes have been made to the directory on a 32-bit machine.
I could use both d_time and d_fsdata on 32-bit, I suppose.  I really don't
want to allocate an 8-byte blob for each dentry.

Maybe I should switch to a single-bit flag and "invalidate" all the dentries
attached to a directory if I detect a jump in the directory's version number
due to a third-party modification (the version is monotonically incremented
for each modification committed).

I'm not sure how to do a mass invalidation without risking deadlock, though,
because I'd quite likely be starting from a random dirent in a directory and
then have to walk through all the other dirents of the directory.  I suppose I
could lock the directory (probably using dvnode->validate_lock) and then walk
through d_children.

David