9 messages in org.freebsd.freebsd-fsRe: RFC: which patch is preferred for...| From | Sent On | Attachments |
|---|---|---|
| Rick Macklem | Apr 1, 2012 5:53 pm |  .patch, .patch |
| John Baldwin | Apr 2, 2012 5:27 am | |
| Rick Macklem | Apr 2, 2012 8:44 am | |
| John Baldwin | Apr 2, 2012 10:10 am | |
| Konstantin Belousov | Apr 2, 2012 12:12 pm | |
| John Baldwin | Apr 2, 2012 1:37 pm | |
| Rick Macklem | Apr 2, 2012 7:02 pm | |
| Joel Ray Holveck | Apr 3, 2012 2:28 am | |
| John Baldwin | Apr 3, 2012 5:53 am |
| Subject: | Re: RFC: which patch is preferred for fix to PR#165923 | |
|---|---|---|
| From: | John Baldwin (jh...@freebsd.org) | |
| Date: | Apr 3, 2012 5:53:11 am | |
| List: | org.freebsd.freebsd-fs | |
On Monday, April 02, 2012 10:02:55 pm Rick Macklem wrote:
John Baldwin wrote:
On Monday, April 02, 2012 11:45:10 am Rick Macklem wrote:
However, there seems to be some disagreement as to how a patch to use the mmap() credentials should be done.
putpages-a.patch - This one captures the credentials during the mmap() call by using the property that only mmap() calls vnode_pager_alloc() with "prot != 0" and references them in a new field in vm_object_t.
putpages-b.patch - This one adds a new VOP_PUTPAGES_NOTIFY() call, which is done by mmap(). For all file systems except NFS clients, it is a null op. For the NFS clients, they reference the credentials in a new field of the NFS specific part of the vnode.
- I don't have a patch for the third one, but I believe Joel was proposing something similar to putpages-a.patch, except that the credentials are passed as an extra argument to VOP_PUTPAGES(), instead of the NFS client's VOP_PUTPAGES() looking in the vnode's vm_object_t for the cred.
I think what I would prefer for the longterm is option 3) (and possibly doing the same for VOP_GETPAGES(). Also, I would call the new field in vm_object something like 'writecred'. For MFC purposes you could use putpages-a.patch instead and only make the change to the VOPs in HEAD.
However, I wonder if you can't manage this all at the vnode layer? If all you need is a credential that is allowed to write, can't you cache the credential used for a successful VOP_ACCESS() check with write support in the nfsnode and just use that for write RPCs? The same thing would work for reads, yes? I know for NFSv4 you already maintain a cache of credentials from open()'s that you use for other RPC's already, so this would seem to be similar to that case. This would also let you do the "list of credentials" idea if needed and even let you add new write-capable credentials to the list via VOP_ACCESS() calls after the file was mmap()'d or open()'d.
I think there is a question w.r.t. should you use credentials that weren't ones that mmap'd the file with PROT_WRITE.
But you already are potentially (in that you might mismatch permissions for a given WRITE since the page might have been dirtied by a different user than the credential we use). You have to open() the file with write permissions before you mmap() it, so all mmap()'s are going to do a VOP_ACCESS() prior, so if you just cache whatever credential last worked for write access to VOP_ACCESS() that would more or less work, and be about as reliable while not requiring any changes outside of the NFS client itself.
Taking a reference to the cred passed to the most recent VOP_OPEN() with FWRITE for the vnode and keeping that in the nfs specific vnode sounds ok to me.
Ok.
The only issue is that the NFS client will do this for many vnodes that never get mmap'd, but that just means more credentials may be allocated for longer. (As I understand it, the crfree() can't be done until the NFS VOP_INACTIVE()/VOP_RECLAIM(), since writing to mmap'd files can happen after the file descriptor is closed.)
In practice the credential is already referenced by the open file handle most of the time anyway, plus many processes and files may all share the same credential, so I think this would be fine in practice.
This would avoid any new/modified VOPs and any changes to vm_object_t.
Yes. If others are ok with this approach I think it is the simplest fix.
-- John Baldwin
_______________________________________________ free...@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-fs To unsubscribe, send any mail to "free...@freebsd.org"