[Pvfs2-developers] kernel readdir question

[Murali Vilayannur]

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 <pcarns at wastedcycles.org> 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 at beowulf-underground.org
> >> http://www.beowulf-underground.org/mailman/listinfo/pvfs2-developers
> >>
> > _______________________________________________
> > Pvfs2-developers mailing list
> > Pvfs2-developers at beowulf-underground.org
> > http://www.beowulf-underground.org/mailman/listinfo/pvfs2-developers
> >
>
>