[ sorry, corrected cc list ] On Mon, Aug 16, 2010 at 03:38:12PM -0400, Chris Mason wrote: > Hi everyone, > > I'm looking into a 2.6.35 btrfs performance regression, and perf tells > me that I'm spending a lot of time hammering on xattrs inside > remove_suid. This is pretty surprising because I'm running as root, and > my files are not suid. Looking back to this commit: > > http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=b53767719b6cd8789392ea3e7e2eb7b8906898f0 > > We've changed remove_suid's semantics from > > if (file_is_suid) > try to remove it > > To something that always checks to see if we have removal permissions. > > Was this intentional? It didn't cause my 2.6.35 regression (that's all > my fault) but it does look wrong to me: > > diff --git a/mm/filemap.c b/mm/filemap.c > index 4fb1546..79f24a9 100644 > --- a/mm/filemap.c > +++ b/mm/filemap.c > @@ -1627,12 +1627,18 @@ int __remove_suid(struct dentry *dentry, int kill) > > int remove_suid(struct dentry *dentry) > { > - int kill = should_remove_suid(dentry); > + int killsuid = should_remove_suid(dentry); > + int killpriv = security_inode_need_killpriv(dentry); > + int error = 0; > > - if (unlikely(kill)) > - return __remove_suid(dentry, kill); > + if (killpriv < 0) > + return killpriv; > + if (killpriv) > + error = security_inode_killpriv(dentry); > + if (!error && killsuid) > + error = __remove_suid(dentry, killsuid); > > - return 0; > + return error; > } > EXPORT_SYMBOL(remove_suid); > > -chris > -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
