phdeniel/nfs-ganesha — comment on commit df4d1a1ab0809769c7e125c65e7cfe6dd84ff6ae
File:  line 0
By phdeniel:

Hi Max,

I have time (at least) to check your commits. I do have a few questions:
- in cache_inode_get.c : why removing the unlock in the case cache_inode_valid fails ? The entry will remain locked which will lead to later deadlocks. The fact is that this function almost never returns an error (which hides many things) : your code will result in a segfault because you set pentry to NULL then deference pentry->lock
- in cache_inode_getattr.c : you need to keep the V_r. If not, the pentry will remain locked, which will result in deadlocks
- in cache_inode_link.c : same issue
- in cache_inode_rename.c : same issue too.
The key here, is that cache_inode-* calls makes all of the lock management. If they take lock, when should release it when exiting. The statements that you removed were set for that purpose.
- in cache_inode_rdwr.c : why removing the test on if(seek_descriptor->whence != FSAL_SEEK_SET) ? Was it causing bugs ?

In the current state, I can't integrate that commit (the change in cache_inode_get seems quite dangerous).

Regards

Philippe