Al Viro <viro@xxxxxxxxxxxxxxxxxx> writes: > On Wed, Aug 21, 2024 at 07:22:39PM -0400, Gabriel Krisman Bertazi wrote: > >> Would it be acceptable to just change the dentry->d_name here in a new >> flavor of d_add_ci used only by these filesystems? We are inside the >> creation path, so the dentry has never been hashed. Concurrent lookups >> will be stuck in d_wait_lookup() until we are done and will never become >> invalid after the change because the lookup was already done >> case-insensitively, so they all match the same dentry, per-definition, >> and we know there is no other matching dentries in the directory. We'd >> only need to be careful not to expose partial names to concurrent >> parallel lookups. > > *Ow* > > ->d_name stability rules are already convoluted as hell; that would make > them even more painful. > > What locking are you going to use there? Since we are in the ->d_lookup() during the rename, and we use the dcache-insensitively for the filesystems that will do the rename, we know there is nothing in the dcache and the dentry is still in the parallel lookup table. So we are not racing with a creation of the same name in the same directory. A parallel lookup will either find that dentry (old or new name, doesn't matter) or not find anything, in case it sees a partial ->d_name. Therefore, the only possible problem is a false negative/positive in parent->d_in_lookup_hash. Can we extend the rename_lock seqlock protection that already exists in d_alloc_parallel to include the d_in_lookup_hash walk? d_add_ci then acquires the rename_lock before writing ->d_name and d_alloc_parallel will see it changed after iterating over d_in_lookup_hash, in case it didn't find anything, and retry the entire sequence. Case-inexact lookups are not supposed to be frequent. Most lookups should be done in a case-exact way, so the extra acquisition of rename_lock shouldn't create more contention on the rename_lock for the regular path or for non-case-insensitive filesystems. The overhead in d_alloc_parallel is another read_seqretry() that is done only in the case where the dentry is not found anywhere and should be created. -- Gabriel Krisman Bertazi
