On Fri, Aug 16, 2024 at 1:53 PM Roberto Sassu <roberto.sassu@xxxxxxxxxxxxxxx> wrote: > > On Tue, 2024-08-06 at 15:36 +0200, Mateusz Guzik wrote: > > The EVM_NEW_FILE flag is unset if the file already existed at the time > > of open and this can be checked without looking at i_writecount. > > Agreed. EVM_NEW_FILE is not going to be set during the open(), only > before, in evm_post_path_mknod(). > > Looks good to me. > > Reviewed-by: Roberto Sassu <roberto.sassu@xxxxxxxxxx> > > Thanks thanks for the review are there plans to pick this up for this merge window? > > Roberto > > > Not accessing it reduces traffic on the cacheline during parallel open > > of the same file and drop the evm_file_release routine from second place > > to bottom of the profile. > > > > Signed-off-by: Mateusz Guzik <mjguzik@xxxxxxxxx> > > --- > > > > The context is that I'm writing a patch which removes one lockref > > get/put cycle on parallel open. An operational WIP reduces ping-pong in > > that area and made do_dentry_open skyrocket along with evm_file_release, > > due to i_writecount access. With the patch they go down again and > > apparmor takes the rightful first place. > > > > The patch accounts for about 5% speed up at 20 cores running open3 from > > will-it-scale on top of the above wip. (the apparmor + lockref thing > > really don't scale, that's next) > > > > I would provide better measurements, but the wip is not ready (as the > > description suggests) and I need evm out of the way for the actual > > patch. > > > > security/integrity/evm/evm_main.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c > > index 62fe66dd53ce..309630f319e2 100644 > > --- a/security/integrity/evm/evm_main.c > > +++ b/security/integrity/evm/evm_main.c > > @@ -1084,7 +1084,8 @@ static void evm_file_release(struct file *file) > > if (!S_ISREG(inode->i_mode) || !(mode & FMODE_WRITE)) > > return; > > > > - if (iint && atomic_read(&inode->i_writecount) == 1) > > + if (iint && iint->flags & EVM_NEW_FILE && > > + atomic_read(&inode->i_writecount) == 1) > > iint->flags &= ~EVM_NEW_FILE; > > } > > > -- Mateusz Guzik <mjguzik gmail.com>
