OpenAFS Master Repository branch, master, updated. openafs-devel-1_9_2-612-gaaa3676

[Gerrit Code Review]

The following commit has been merged in the master branch:
commit aaa367660f7da349582666544d732377e89a4280
Author: Andrew Deason <adeason@sinenomine.net>
Date:   Thu Aug 20 23:03:52 2026 -0500

    ubik: Drop cache_lock before commit flush
    
    Currently in ubik_EndTrans(), we drop dbase->cache_lock right after
    calling ubik_Flush(). But there is no reason to hold cache_lock across
    the ubik_Flush() call; ubik_Flush() just writes our buffered dirty data
    to the other ubik sites, and does not interact with the application
    cache in any way.
    
    This is significant if ubik_BeginTransReadAnyWrite() is in use, because
    if ubik_Flush() experiences network delays while communicating with the
    other sites, we are unnecessarily holding cache_lock while we are
    waiting on the network. This can prevent other transactions from
    running, if another transaction has ubik_Abort()'d, and is trying to
    acquire cache_lock to clear the application cache.
    
    This situation is currently not possible, since no applications in the
    tree use ubik_BeginTransReadAnyWrite(), and so no other transactions can
    be running while we are writing to the database. But this may change in
    future commits.
    
    To avoid this, drop cache_lock before calling ubik_Flush() in
    ubik_EndTrans().
    
    It is, of course, still possible for ubik_Flush() to be called in the
    middle of a transaction if we ubik_Write() more data than our buffer can
    hold. But ubik transactions for all OpenAFS applications tend to be very
    small (and fit in a single DISK_WriteV()), so by dropping cache_lock
    earlier in ubik_EndTrans(), we avoid the problem for practically all
    situations.
    
    With this commit, dbase->cache_lock is now dropped for a much longer
    length of time during ubik_EndTrans() for write transactions.
    If another thread acquires dbase->cache_lock and modifies the
    cache-related fields while ubik_EndTrans() is running, that sounds a bit
    dangerous, but shouldn't actually cause problems, because:
    
    No other transactions can be fully running while a write transaction is
    running, since no applications currently use
    ubik_BeginTransReadAnyWrite(). But if a read transaction managed to
    ubik_AbortTrans() (say, before calling ubik_SetLock(), or if
    ubik_SetLock() returned an error), it could grab dbase->cache_lock and
    clear dbase->cachedVersion. But even if it did clear
    dbase->cachedVersion, the write transaction would later set
    dbase->cachedVersion to a valid version later on anyway, after it
    write-locks dbase->cache_lock.
    
    Even if such a situation could cause a problem, any such issues would
    still be possible before this commit, because ubik_EndTrans() already
    dropped and reacquired dbase->cache_lock. But it is noteworthy that this
    commit greatly increases the amount of time that dbase->cache_lock is
    dropped during ubik_EndTrans().
    
    Change-Id: Idd7b22372ab296567c6a6a84fa83d139be0eafa9
    Reviewed-on: https://gerrit.openafs.org/16980
    Tested-by: BuildBot <buildbot@rampaginggeek.com>
    Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
    Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
    Reviewed-by: Marcio Brito Barbosa <mbarbosa@sinenomine.net>
    Reviewed-by: Andrew Deason <adeason@sinenomine.net>

 src/ubik/ubik.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

-- 
OpenAFS Master Repository