On Wed, Nov 25, 2020 at 9:49 AM Steven Whitehouse <[email protected]> wrote: > On 24/11/2020 16:42, Andreas Gruenbacher wrote: > > Commit 20f829999c38 ("gfs2: Rework read and page fault locking") has lifted > > the > > glock lock taking from the low-level ->readpage and ->readahead address > > space > > operations to the higher-level ->read_iter file and ->fault vm operations. > > The > > glocks are still taken in LM_ST_SHARED mode only. On filesystems mounted > > without the noatime option, ->read_iter needs to update the atime as well > > though, so we currently run into a failed locking mode assertion in > > gfs2_dirty_inode. Fix that by taking the glock in LM_ST_EXCLUSIVE mode on > > filesystems mounted without the noatime mount option. > > > > Faulting in pages doesn't update the atime, so in the ->fault vm operation, > > taking the glock in LM_ST_SHARED mode is enough. > > I don't think this makes any sense to do. It is going to reduce the > scalibility quite a lot I suspect. Even if you have multiple nodes > reading a file, the atime updates would not be synchronous with the > reads, so why insist on an exclusive lock here?
Indeed, it's a bit slow, even though I'm not sure where you're going with that "not synchronous" argument. I've replaced this with a patch that upgrades a shared lock to an exclusive lock only when the atime actually needs to be updated; see "gfs2: Upgrade shared glocks for atime updates". (Note that the revised patch isn't easy to test without instrumentation because usually, the atime update will happen when we try to read from the page cache, in which case gfs2_update_time will be called without holding the glock.) Thanks, Andreas
