On 9/23/24 9:07 AM, Al Viro wrote: > On Mon, Sep 23, 2024 at 12:30:48AM -0600, Jens Axboe wrote: > >> 1) Just don't reuse the entry. Then we can drop the struct >> filename->aname completely as well. Yes that might incur an extra >> alloc for the odd case of audit_enabled and being deep enough that >> the preallocated names have been used, but doesn't anyone really >> care? It'll be noise in the overhead anyway. Side note - that would >> unalign struct filename again. Would be nice to drop audit_names from >> a core fs struct... > > You'll get different output in logs, though. Whether that breaks userland > setups/invalidates certifications/etc.... fuck knows. No idea about that... But I'd say without strong evidence that this breaks userland for something as odd as audit, well... And honestly really a layering problem that struct filename has an audit link in there. > If anything, a loop through the list, searching for matching entry would > be safer in that respect. Order of the items... might or might not be > an issue - see above. > >> 2) Add a ref to struct audit_names, RCU kfree it when it drops to zero. >> This would mean dropping struct audit_context->preallocated_names, as > > Costly, that. For sure. And you could keep preallocated_names if you rcu free the context too. But I strongly believe that approach #1 is, by far, the cheaper alternative. If we can tolerate the ordering potentially changing. >> otherwise we'd run into trouble there if a context gets blown away >> while someone else has a ref to that audit_names struct. We could do >> this without a ref as well, as long as we can store an audit_context >> pointer in struct audit_names and be able to validate it under RCU. >> If ctx doesn't match, don't use it. > > That's one of the variants I mentioned upthread... Sorry, still away on travels and conferences, so haven't been keeping up on replies. -- Jens Axboe
