On Thu, 2020-12-17 at 10:14 -0500, Tejun Heo wrote: > Hello, > > On Thu, Dec 17, 2020 at 07:48:49PM +0800, Ian Kent wrote: > > > What could be done is to make the kernfs node attr_mutex > > > a pointer and dynamically allocate it but even that is too > > > costly a size addition to the kernfs node structure as > > > Tejun has said. > > > > I guess the question to ask is, is there really a need to > > call kernfs_refresh_inode() from functions that are usually > > reading/checking functions. > > > > Would it be sufficient to refresh the inode in the write/set > > operations in (if there's any) places where things like > > setattr_copy() is not already called? > > > > Perhaps GKH or Tejun could comment on this? > > My memory is a bit hazy but invalidations on reads is how sysfs > namespace is > implemented, so I don't think there's an easy around that. The only > thing I > can think of is embedding the lock into attrs and doing xchg dance > when > attaching it. Sounds like your saying it would be ok to add a lock to the attrs structure, am I correct? Assuming it is then, to keep things simple, use two locks. One global lock for the allocation and an attrs lock for all the attrs field updates including the kernfs_refresh_inode() update. The critical section for the global lock could be reduced and it changed to a spin lock. In __kernfs_iattrs() we would have something like: take the allocation lock do the allocated checks assign if existing attrs release the allocation lock return existing if found othewise release the allocation lock allocate and initialize attrs take the allocation lock check if someone beat us to it free and grab exiting attrs otherwise assign the new attrs release the allocation lock return attrs Add a spinlock to the attrs struct and use it everywhere for field updates. Am I on the right track or can you see problems with this? Ian
