Alexander Bluhm <alexander.bl...@gmx.net> wrote: > On Wed, May 11, 2022 at 11:20:15AM +0300, Vitaliy Makkoveev wrote: > > sys_umask() only modifies `fd_cmask', which modification is already > > protected by `fd_lock' rwlock(9). > > I found this in sys/filedesc.h > > u_short fd_cmask; /* [f/w] mask for file creation */ > u_short fd_refcnt; /* [K] reference count */ > > We have two short variables that are protected by different locks. > I think 16 bit values are not MP independent on all architectures. > > When one CPU modifies the lower 16 bit and another CPU writes to > the higher 16 bit the result in the full 32 bit is not defined. > This is at least my understanding.
Such as the oldest alpha cpus, before the BWX extensions were added. > I have seen problems in real live with two shorts when one 16 bit > part was changed without spl protection and the other 16 bits were > written by interrupt. > > Should we convert them to u_int? I think fd_cmask should be changed to the proper type: mode_t, which is __mode_t, which is __uint32_t Once that expands to a 32-bit value, there is no need to try to be space-optimal with fd_refcnt, it may as well be an int. Funny thing. On 64-bit architectures the alignment and padding rules are placing these fields at these offsets: fd_cmask 64 fd_refcnt 66 fd_lock 72 So 68-71 are 4-pad bytes to align fd_lock. Changing both types to int will not grow this structure on 64-bit architectures.
