Re: Atomic file locking update

[Justus Winter]

Hi :)

Quoting Samuel Thibault (2014-11-23 17:38:32)
> Hello,
> 
> While Svante is working on the record locking, I have worked on at
> least fixing whole file locking: there was one bug in our current
> implementation: flock(LOCK_SH); flock(LOCK_EX);, as per POSIX, does not
> guarantee an atomic upgrade from LOCK_SH to LOCK_EX. But
> fcntl(SETLK,F_RDLCK); fcntl(SETLK,F_WRLCK); is supposed to guarantee an
> atomic upgrade from F_RDLCK to F_WRLCK.
> 
> I have thus added a __LOCK_ATOMIC flag, to be used along LOCK_SH
> and LOCK_EX, to guarantee atomic upgrades and downgrades, and then

Just to be sure I understand the change correctly, the downgrade is
somewhat trivial, right?  Because your patch has an explicit 'tell
others we are upgrading' and my symmetry-sense is tingling.

> implemented the support in libfshelp. To avoid starvation, my patch
> makes the thread which wants to upgrade to LOCK_EX set the lock type
> to LOCK_SH|LOCK_EX, and others manage that, to prevent newer LOCK_SH
> lockers.
> 
> I have successfully tested it with tdb's tdbtorture up to 5 processes
> for now, and it works fine, but perhaps somebody could proofread the
> changes again, to make sure we aren't forgetting any scenario?

From what I can see it looks good.  Also, I've included your patch in
my packages and from what I can see it at least doesn't break any
existing functionality ;)

> +      /* If there is another exclusive lock or a pending upgrade, wait for 
> it to
> +         end. */

Nitpick: eight spaces instead of a tab.

Cheers,
Justus