Hello Nathan,
On Thursday 17 October 2002 00:48, Nathan Scott wrote:
> Mornin' all,
>
> On Wed, Oct 16, 2002 at 11:50:12AM -0400, Theodore Ts'o wrote:
> > On Wed, Oct 16, 2002 at 02:11:04PM +0100, Christoph Hellwig wrote:
> > > Ted, please either go _always_ through the {get,set}_posix_acl methods
> > > or never. Currently XFS doesn't know and doesn't want to know
> > > about the so called "egenric ACL representation" used by ext2/ext3.
> > > With theses methods we'd have to add it to XFS which is fine for me as
> > > long as it the representation generally used for working with ACLs.
> > > That would mean we'd have to add new syscall or at least VFS-level
> > > hooks to the xattr code.
> >
> > Fine. I'll just yank the {get,set}_posix_acl methods for now. The
>
> Please remove them permanently, IMO they are broken by design.
That's done, despite that I disagree with your line of argument (see below).
> They are an optimisization for the one special case (posix acls),
> and manage to pollute the VFS for that one special case - a much
> cleaner solution is to optimise the code sitting behind the xattr
> inode calls and preferably in ways that all of the ext2/3 EAs can
> benefit (users attrs, capabilities, MAC labels, etc), & not just
> ACLs.
Nathan, you are ignoring the design of the inode operations: They are on
purpose passing extended attributes by value. This is good as a kernel/user
space interface, but inappropriate for passing around references to kernel
internal structures. There is no further optimizing to be done behind that
interface.
In the current design no filesystem independent part of the kernel would
benefit from a faster interface except that one NFS hack. Since that appears
to be no serious performance bottleneck and the hack will eventually go away
using ->getxattr() seems acceptable.
As soon as any filesystem independent part of the kernel needs an interface
more efficient that pass-by-value we will again have exactly the same
problem.
> Yes, this means nfsd has to do some more work to alloc some space
> and convert from syscall format to native before doing its thing
> with the attributes, but thats in the noise compared to going to
> disk and fetching the EA (which is what we really want to optimise
> here) and can also be wrapped in Andreas' lib code so that the nfsd
> changes are minimal.
I'm not sure if you have looked at how the [gs]et_posix_acl() operations are
implemented for ext2/ext3/reiserfs/xfs. They are caching the ACLs per inode
instead of repeatedly copying around the xattr values; permission() calls
therefore are very efficient. XFS is the only exception; it always goes
through the xattr interface and does not cache ACLs. I think XFS should also
use caching, at least for ACL_TYPE_ACCESS ACLs.
Going to disk and fetching EAs only causes disk accesses once; afterwards the
block is cached.
> > However, the reality is that at this point, we probably won't have
> > time to get support in for the NFS server ACL before feature freeze,
> > and changing the interface to ACL's (never mind the headaches of
> > trying to agree to a new syscall interface at this late date), given
> > the deployed userspace tools, just doesn't seem to be realistic.
>
> No additional syscalls are needed. This would also be short sighted
> - we don't yet support any other system EAs (as I've said to Andreas
> on several occassions, capabilities look like a really good candidate
> since they also must be read in kernel space on exec and read/written
> from userspace), so any interface changes now based on a fairly weak
> optimisation for the single system attribute that the patches support
> today (posix acls) is premature at best.
File system capabilities are in a different league that ACLs. They only need
to be read once for each exec(), so going through the xattr interface is no
problem at all. (In comparison, all ACLs along the path to a file are
accessed whenever that file is accessed.)
Cheers,
Andreas.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
This archive was generated by hypermail 2b29 : Wed Oct 23 2002 - 22:00:33 EST