• Kirill Smelkov's avatar
    nodefs: Allow for several Lookup requests to be served simultaneously · 17c0c400
    Kirill Smelkov authored
    Commit d0fca860 introduced big lookup lock with the idea to "make sure
    we don't return forgotten nodes to the kernel". However this way it also
    started to prevent several Lookup handlers to be running simultaneously,
    which can deadlock if e.g. Lookup handler somehow synchronizes with
    other thread which caused the lookup:
    
    https://github.com/hanwen/go-fuse/commit/d0fca860#commitcomment-37772099
    
    On the surface the fix is easy: if we need to prevent lookups running in
    parallel to forget, we can turn lookupLock into shared one wrt Lookup,
    but exclusive wrt Forget.
    
    However a more correct fix would be to review nodefs locking completely:
    there is already per-filesystem treeLock RWMutex, which is _already_
    used to synchronize for forgets and lookups. From this point of view
    lookupLock is unneeded and the correct fix that d0fca860 should have been
    doing is to correct the scope of treeLock to be used more precisely.
    
    However that would be a more intrusive change, and given that nodefs is
    considered deprecated today, imho it is better to proceed with making
    lookupLock a shared one.
    
    The test that is added deadlocks without provided fix.
    
    I suggest not to reject this patch based on rationale that "nodefs is
    deprecated" as there still are real filesystems that use nodefs.
    
    I had not reviewed v2 go-fuse/fs locking yet.
    
    Change-Id: I18e01457f474dea31dc17186dfe6db582c2e6337
    17c0c400
fsconnector.go 13.4 KB