This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH v2 2/2] manual/llio.texi: update manual to document file-private locks
- From: Jeff Layton <jlayton at redhat dot com>
- To: mtk dot manpages at gmail dot com
- Cc: mtk dot lists at gmail dot com, libc-alpha at sourceware dot org
- Date: Wed, 16 Apr 2014 09:48:27 -0400
- Subject: Re: [PATCH v2 2/2] manual/llio.texi: update manual to document file-private locks
- Authentication-results: sourceware.org; auth=none
- References: <1397220945-11926-1-git-send-email-jlayton at redhat dot com> <1397220945-11926-3-git-send-email-jlayton at redhat dot com> <534E8209 dot 3070705 at gmail dot com>
On Wed, 16 Apr 2014 15:13:45 +0200
"Michael Kerrisk (man-pages)" <mtk.lists@gmail.com> wrote:
> On 04/11/2014 02:55 PM, Jeff Layton wrote:
> > Signed-off-by: Jeff Layton <jlayton@redhat.com>
>
> [...]
>
You may want to look at the v3 patch I sent yesterday. It has some
changes, but I think your comments below still apply to it.
> > @@ -3817,6 +3834,168 @@ Remember that file locks are only a @emph{voluntary} protocol for
>
> s/voluntary/advisory/
>
Ok...
> > controlling access to a file. There is still potential for access to
> > the file by programs that don't use the lock protocol.
> >
> > +@node File-private Locks
> > +@section File-private Locks
> > +
> > +In contrast to classic record locks (@pxref{File Locks}), file-private
> > +locks are associated with an open file table entry rather than a
> > +process. File-private locks set on an open file descriptor will never
>
> s/File-private locks/New file-private locks/
>
Carlos suggested avoiding adjectives like "new" or "classic" because in
a year or two, these will no longer be "new". I've moved to using
"process-associated" instead of "classic" in the latest rev and
dropping the "new" moniker.
> > +conflict with existing file-private locks set on that file descriptor.
>
> Perhaps add a sentence here that a lock conversion may be involved?
>
> [...]
>
Sure, sounds reasonable.
> > +There might be more than one lock affecting the region specified by the
> > +@var{lockp} argument, but @code{fcntl} only returns information about
> > +one of them. The @code{l_whence} member of the @var{lockp} structure is
> > +set to @code{SEEK_SET} and the @code{l_start} and @code{l_len} fields
> > +set to identify the locked region.
> > +
> > +If no lock applies, the only change to the @var{lockp} structure is to
>
> s/If no lock applies/If no conflicting lock exists/
>
> > +update the @code{l_type} to a value of @code{F_UNLCK}.
>
> Slightly clumsy wording
>
> s/update the @code{l_type} to a value of/
> update the @code{l_type} field to the value/
>
> s/update the @code{l_type} to a value of/
> update @code{l_type} to the value/
>
>
> > +The normal return value from @code{fcntl} with this command is an
> > +unspecified value other than @math{-1}, which is reserved to indicate an
> > +error.
>
> This doesn't sound correct. At the glibc level isn't the return value
> either 0 for success or -1 on error?
>
> [...]
>
Good catch. I copied that verbatim from the older File lock section.
That may be the case on non-linux kernels, but on linux, you'll get
back 0 on success. I think we'd want to mandate that for other OS' that
implement this as well. Will fix.
> > +If the opened file already has a lock on any part of the
> > +region, the old lock on that part is replaced with the new lock. You
> > +can remove a lock by specifying a lock type of @code{F_UNLCK}.
> > +
> > +If the lock cannot be set, @code{fcntl} returns immediately with a value
> > +of @math{-1}. This function does not block waiting for other tasks
> > +to release locks. If @code{fcntl} succeeds, it returns a value other
> > +than @math{-1}.
>
> See comment above.
>
Thanks, will fix.
> [...]
>
> > +File-private locks are useful in the same sorts of situations as classic
> > +record locks. They can also be used to synchronize file access between
> > +threads within the same process by giving each thread its own open file
> > +instance.
>
> I think this could be explained more clearly. How about:
>
> They can also be used to synchronize file access between threads
> within the same process by giving having each thread perform its
> own @code{open} of the file, to obtain its own open file instance.
>
Much better. I'll use that.
> > +Because they are only released automatically when the last reference to
> > +an open file is destroyed, file-private locks allow more assurance that
>
> Suggest s/destroyed/closed/ (that gives the reader a clue what the
> user-space operation is).
>
> > +the locks will not be released due to a library routine opening and
> > +closing a file without the application being aware.
>
> I suggest rewording as:
>
> ..., file-private locks avoid the possibility that locks are
> released due to a library routine opening and closing a
> file without the application being aware.
>
Much better. I'll use that too.
> > +
> > +As with classic record locks, file-private locks are also voluntary.
>
> s/voluntary/advisory/
>
> Cheers,
>
> Michael
Thanks for the review. I'll plan to post v4 later today that
incorporates your suggestions once I give Carlos and others a chance to
comment. Once we get the manual sorted out here, I'll resubmit the fcntl
manpage patch.
--
Jeff Layton <jlayton@redhat.com>