> On Oct 3, 2022, at 6:07 PM, NeilBrown <neilb@xxxxxxx> wrote: > > On Tue, 04 Oct 2022, Chuck Lever III wrote: >> >>> On Oct 3, 2022, at 7:34 AM, Jeff Layton <jlayton@xxxxxxxxxx> wrote: >>> >>> nfsd_file is RCU-freed, so we need to hold the rcu_read_lock long enough >>> to get a reference after finding it in the hash. Take the >>> rcu_read_lock() and call rhashtable_lookup directly. >>> >>> Switch to using rhashtable_lookup_insert_key as well, and use the usual >>> retry mechanism if we hit an -EEXIST. Eliminiate the insert_err goto >>> target as well. >> >> The insert_err goto is there to remove a very rare case from >> the hot path. I'd kinda like to keep that feature of this code. > > ???? > The fast path in the new code looks quite clean - what concerns you? > Maybe a "likely()" annotation can be used to encourage the compiler to > optimise for the non-error path so the error-handling gets moved > out-of-line (assuming it isn't already), but don't think the new code > needs that goto. It's an instruction cache footprint issue. A CPU populates its instruction cache by reading instructions from memory in bulk (some multiple of the size of the cacheline). I would like to keep the instructions involved with very rare cases (like this one) out-of-line so they do not clutter the CPU's instruction cache. Unfortunately the compiler on my system has decided to place this snippet of code right before the out_status: label, which defeats my intention. -- Chuck Lever
