On Fri, May 23, 2008 at 7:16 PM, Matthew Dillon <[EMAIL PROTECTED]> wrote: > > :The issue is that FSCRED and NOCRED are checked in the kern_prot.c > :helper functions, but the vnops functions in the various file systems > :dereference the struct ucred pointers without checking for a > :NULL(NOCRED) or 0xFFFFFFFF(FSCRED) pointers. So what is the ideal > :solution? Should the ucred API be extended in kern_prot to do the uid > :check that the file systems do (while taking into account NOCRED and > :FSCRED)? > : > :And I thought this was such a simple patch ;) > : > :-- Dion > > No, I would not change the API. > > There are three 'real' creds available that can be used. First, there > is the current process cred. Second, there is the cred stored with > the file pointer (struct file -> f_cred), and third there is the > cred associated with the proc0 (proc0.p_ucred).
Ah. I suppose the proc0 cred would be the one to use when no process context is available, at least, that's how it is done when initializing f_cred. > > After looking at your patch I think we actually should avoid using > SETATTR to update the atime. That is a very active and expensive VOP > and kinda messes up the critical path. > > If we want to update the atime from exec*() and from mmap() we should > have access to the cred in the file pointer. What I would then do is > actually create a new VOP op and let the filesystem do whatever internal > actions it feels is reasonable to do the update. So, e.g. we would > implement a new VOP called, say, VOP_SIGNAL_ACCESS(vp, cred) (maybe you > can come up with a better name), with a default operation which is just > a NOP. UFS and HAMMER would then implement that VOP (the only two > filesystems we really care about insofar as atime goes), allowing them > to optimize the operation. > > Our exec and mmap code would then call the VOP. > > How does that sound for a plan? Sound good to me. Pretty much what FBSD does. I just wasn't sure if adding another VOP was the way to go -- I didn't realize the setattr was expensive. It would explain their choice. Also, I was thinking about how to add a helper function for chmod setattr code, but it seems you've already done it ;) -- Dion > > -Matt > Matthew Dillon > <[EMAIL PROTECTED]> >
