PR: 253894
Details
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
share/man/man9/VOP_READ_PGCACHE.9 | ||
---|---|---|
52 | I would probably spell this IO. | |
59 | Is it worth specifically stating how this differs from VOP_READ(9) -- i.e., the caller will not acquire the vnode lock, and will retry once holding the lock using VOP_READ(9) if this doesn't work? (Assuming I understand correctly.) Should the implementation of VO_READ_PGCACHE(9) promise *not* to do expensive things, such as disk IO, or is this simply an opportunity for it to provide a read operation without requiring the caller to lock? | |
68 | Perhaps instead: "request; it also might choose..." | |
84 | Should we refer the reader to VOP_READ(9) for the possible flag values ..? | |
85 | Should be "cred" not "cnp"? | |
89 | Can drop the definite article, "The," here. | |
93 | Should we drop the argument? Or is the plan that we might support it at some point? (Can we assert this in a VOP pre-hook to avoid confusion arising..?) | |
97 | Should we say something about the vnode lock not being required for callers .. and about whether the callee should or shouldn't acquire it? Should the callee fail if it needs to acquire the vnode lock, and let a followup call to VOP_READ(9) handle it? | |
105 | In this case, the access is performed, and the uio is still updated to reflect data read ..? Or is this an error and VOP_READ should be tried? I guess it makes me slightly uncomfortable to have a non-zero success mode, as it's so easy for callers to get that horribly wrong (vis OpenSSL), but I suppose there's also only one caller of this. Regardless, I think the expected semantics should be documented more thoroughly here (and possibly above as well, rather than in the error code section). | |
106 | Possibly we should point at possible error returns from VOP_READ(9)? |
share/man/man9/VOP_READ_PGCACHE.9 | ||
---|---|---|
59 | It is up to the filesystem to do whatever it intents to. Typically fs would not want to do vn_lock(); VOP_READ(); VOP_UNLOCK(); if only because the vnode can be reclaimed before or during lock, in which case fs should not do read on it at all. WRT expensive ops, it is up to fs. The main difference in the call environment: lockless and the permit to delegate to later VOP_READ() by special error return, are stated. | |
93 | It is up to filesystem, doing it in pre is not better than ensure that callers do it right. I do not want to drop the flag arg, we might want to pass some info, and I want to have | |
97 | I added the note about vnode not being locked. I do not want to give even a hint that the implementation of the VOP could lock the vnode. | |
105 | I added a note that uio is updated as far as IO is done. I thought that this is obvious (how would the caller know it, otherwise?) There should be a way to distinguish partial read vs. short read. EJUSTRETURN is as good as other ways to notify about partial read, e.g. explicit EOF flag. EJUSTRETURN not returned to userspace is used by other parts of VFS, e.g. for successful namei(9) call that does not return resulting vnode (e.g. CREATE). Of course it is important to handle EJUSTRETURN, but it is VFS problem and not fs issue. Somebody calling the VOP must know much more than somebody implementing it. Man pages are to provide the first look into the interface. |
share/man/man9/VOP_READ_PGCACHE.9 | ||
---|---|---|
124 | I wonder if it's worth |
share/man/man9/VOP_READ_PGCACHE.9 | ||
---|---|---|
124 | Can you explain this note, please? |
share/man/man9/VOP_READ_PGCACHE.9 | ||
---|---|---|
124 | I started making a comment and then decided not to make it, and experienced a user error. You can disregard this note. (I was going to suggest giving an example of such a situation, but ... I think it's OK without it.) |