Re: [PATCH v2 2/2] manual/llio.texi: update manual to document file-private locks

["Michael Kerrisk (man-pages)" <mtk dot lists at gmail dot com>]

On 04/11/2014 02:55 PM, Jeff Layton wrote:
> Signed-off-by: Jeff Layton <jlayton@redhat.com>

[...]

> @@ -3817,6 +3834,168 @@ Remember that file locks are only a @emph{voluntary} protocol for

s/voluntary/advisory/

>  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/

> +conflict with existing file-private locks set on that file descriptor.

Perhaps add a sentence here that a lock conversion may be involved?

[...]

> +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?

[...]

> +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.

[...]

> +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.

> +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.

> +
> +As with classic record locks, file-private locks are also voluntary.

s/voluntary/advisory/

Cheers,

Michael