Re: locking around LFS_{SET,CLR}_UINO

[Eduardo Horvath <eeh%NetBSD.org@localhost>]

On Sun, 19 Dec 2010, NAKAJIMA Yoshihiro wrote:

> Hello LFS developers,
> 
> 
> When vmlocking2 was merged, lock and unlock were moved out of
> LFS_{SET,CLR}_UINO.
> 
> : % diff -U4 lfs.h:1.{122,123} | sed -n 94,104p
> :  #define LFS_SET_UINO(ip, flags) do {                                       
> \
> : -   simple_lock(&(ip)->i_lfs->lfs_interlock);                       \
> :     if (((flags) & IN_ACCESSED) && !((ip)->i_flag & IN_ACCESSED))   \
> :             ++(ip)->i_lfs->lfs_uinodes;                             \
> :     if (((flags) & IN_CLEANING) && !((ip)->i_flag & IN_CLEANING))   \
> :             ++(ip)->i_lfs->lfs_uinodes;                             \
> :     if (((flags) & IN_MODIFIED) && !((ip)->i_flag & IN_MODIFIED))   \
> :             ++(ip)->i_lfs->lfs_uinodes;                             \
> :     (ip)->i_flag |= (flags);                                        \
> : -   simple_unlock(&(ip)->i_lfs->lfs_interlock);                     \
> :  } while (0)
> 
> I guess the lock protects lfs_uinodes, when IN_ACCESSED, IN_CLEANING
> or IN_MODIFIED are set.
> 
> If it is right, a lock is unnecessary and some locks are missing.  See
> attached patch.

Looks reasonable.  You should definitely add a comment somewhere 
indicating the uino is protected by the lfs_lock.  Locking protocols must 
be documented or they are guaranteed to be broken.  Have you tested it 
under load? 

Eduardo