Rainer Toebbicke wrote:
> I'm slightly worried about afs/LINUX/osi_vfsops.c, function afs_put_inode.
>
>
> The code in 1.4.1-rc7 looks a bit odd:
>
> if (VREFCOUNT(vcp) == 2) {
> if (VREFCOUNT(vcp) == 2)
> afs_InactiveVCache(vcp, NULL);
>
> The double-making-sure that VREFCOUNT(vcp) == 2 looks like an editing bug.
>
> More seriously however, afs_InactiveVCache() through the subsequently
> called afs_InvalidateAllSegments() needs the vnode to be locked. I'm
> worried that given that this is an upcall from the kernel this might not
> always be the case, e.g. when invoked through afs_linux_lookup().
>
> (Besides I still believe this function can be called with the global
> lock already held and thus deadlock, but this is another story.)Searching through CVS you can find that the removal of the lock took place in: http://www.openafs.org/cgi-bin/cvsweb.cgi/openafs/src/afs/LINUX/osi_vfsops.c.diff?r1=1.40&r2=1.41 This was to fix ticket 20820. At which point there was a VREFCOUNT test followed by the acquisition of AFS_GLOCK() followed by another VREFCOUNT test. Then in http://www.openafs.org/cgi-bin/cvsweb.cgi/openafs/src/afs/LINUX/osi_vfsops.c.diff?r1=1.41&r2=1.42 The AFS_GLOCK() was moved outside the initial VREFCOUNT test which resulted in two identical tests being performed in a row. This was to fix 23318. Chas will have to explain why the combination of these changes is what is desired. Jeffrey Altman
smime.p7s
Description: S/MIME Cryptographic Signature
