Sam,

 From what I can tell the directory_version gets set to the mtime
after the first upcall.  If future upcalls changed the mtime, we used
to start over, and we didn't want to call filldir again on . and ..

Correct..

But we recently changed this code to not start over if the directory
version changed from one readdir call to the next (it was causing
duplicate entries, see:  http://www.beowulf-underground.org/pipermail/
pvfs2-developers/2006-October/002739.html), instead we just update
the directory version stored in the inode.  There's also the diff of
the Murali's changes:

http://www.pvfs.org/cgi-bin/pvfs2/viewcvs/viewcvs.cgi/pvfs2/src/
kernel/linux-2.6/dir.c.diff?r1=1.45&r2=1.46

Oh yeah. I forgot about that. :)

The directory_version gets set back to 0 once we hit the end of a
directory, so I could imagine that killing an ls while its listing a
directory with many entries would cause the directory version to be
set to the mtime of the last readdir call before the abort.  This
would give the behavior of the . and .. not showing up in the next
listing, but as long as that listing was allowed to complete, the
directory_version would be set back to 0 again.

Well, if an ls was killed the directory is closed. Next time you start
it up it will reopen
the directory and start from offset 0. So i don't think that should be
a problem.
We keep the version for every open directory, so there should not be
any state across
process lifetimes.

Given that we removed the code that restarts if the version changes
on us midway through a listing, can we assume that the position will
be set to 0 _only_ on the first readdir call?  This would allow us to
remove the directory_version == 0 check.  In fact we should be able
to remove the PVFS_READDIR_END check that sets the directory_version
back to 0 as well...

position is basically copied over from file->f_pos which is updated on
every succesful
filldir callback. So it should be set to 0 when we start out.
To me it now looks like we can get rid of the directory_version field
itself from the pvfs2_inode structure and remove all associated checks
as well.
Please don't remove the if (pos == PVFS_READDIR_END) check though, we
need to retain that to return 0, but we can discard setting the
directory_version back to 0.

If you guys are busy, I can cook up a patch later today or tomorrow..
let me know if you guys plan to take a stab at it.
thanks,
Murali

-sam

> thanks,
> Murali
>
> On 11/8/06, Phil Carns <[EMAIL PROTECTED]> wrote:
>> It's been a while since I've seen this bug first hand, but I am
>> just now
>> getting around to looking at it.
>>
>> Every once in a while we have seen cases where "ls -al" in a pvfs2
>> directory fails to show the "." and ".." entries.  I _think_ this has
>> mainly occurred after restarting pvfs2-client and/or pvfs2-server,
>> but I
>> am not certain.  I can't seem to reproduce it.
>>
>> At any rate, looking at the code in dir.c, it seems like filling
>> in the
>> "." and ".." entries should be pretty much automatic.  However,
>> there is
>> an if statement wrapped around the filldir() calls that looks like
>> this:
>>
>>          if (pvfs2_inode->directory_version == 0)
>>          {
>>
>> Anyone know what the purpose if this check is?  It seems to me
>> like "."
>> and ".." should be entries for position 0 and 1 regardless of the
>> directory version, but I may be missing something.
>>
>> -Phil
>> _______________________________________________
>> Pvfs2-developers mailing list
>> Pvfs2-developers@beowulf-underground.org
>> http://www.beowulf-underground.org/mailman/listinfo/pvfs2-developers
>>
> _______________________________________________
> Pvfs2-developers mailing list
> Pvfs2-developers@beowulf-underground.org
> http://www.beowulf-underground.org/mailman/listinfo/pvfs2-developers
>


_______________________________________________
Pvfs2-developers mailing list
Pvfs2-developers@beowulf-underground.org
http://www.beowulf-underground.org/mailman/listinfo/pvfs2-developers

Reply via email to