This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


Hi Jeff,

On Wed, Apr 16, 2014 at 6:00 PM, Jeff Layton <jlayton@redhat.com> wrote:
> On Wed, 16 Apr 2014 17:37:53 +0200
> Michael Kerrisk <mtk.manpages@gmail.com> wrote:
>
>> Jeff, (and Carlos)
>>
>> > On Sat, 12 Apr 2014 12:52:33 -0400
>> > "Carlos O'Donell" <carlos@redhat.com> wrote:
>>
>> [...]
>>
>>
>> >> OT: Why is it F_*LKP? What does the "P" part of "LKP" mean?
>>
>> That does not strike me as OT at all. The existing byte-range locking
>> system has persisted (despite egregious faults) for well over two
>> decades. One supposes that Jeff's new improved version might be around
>> at least as long. With that in mind, and before setting in stone (and
>> pushing into POSIX) a model of thinking that thousands of programmers
>> will live with for a long time, it's worth thinking about names.
>>
>> > Sorry, forgot to answer this:
>> >
>> > "P" is for "Private". What can I say...I suck at naming things, and no
>> > one else proposed anything better. It's not too late to change that if
>> > you can come up with a better moniker, but we'd need to move fast
>> > before v3.15 ships.
>>
>> I agree on the "suck" bit ;-). Carlos nailed it when he referred to
>> traditional byte-range locks as being "process-associated". That is
>> the distinction between these two two locking APIs: one associates
>> locks with the process, the other associates them with a file handle.
>> The terminology should reflect that difference.
>>
>> From that perspective, the word "private" here makes no sense at all.
>> What does "private" mean in this context? (By their very definition,
>> file locks are not private--they're something that is visible between
>> processes.) Some thought should be given to fixing that name, now, so
>> that we have better terminology for the coming decades ;-).
>>
>> I would at the least suggest renaming these locks to something like:
>>
>>     file-associated locks
>>
>
> That might be ok.
>
>> or
>>
>>    file-handle locks
>>
>
> I'd avoid this. "File handle" has certain connotations in context of
> NFS, and other (completely different) connotations in other situations.

Fair enough. It's the simplest of the alternatives, but I take your point.

>> or (using POSIX terminology)
>>
>>     file-description locks
>>
>> but that last might be a bit confusing.
>>
>
> Again, somewhat confusing since they aren't tied to the file-descriptor
> per-se. Consider interactions with dup(), for instance.

I think you've kind of shown my point about it being confusing. Note
that I wrote "file description", not "file descriptor". In POSIX a
"file description" is the thing that a file descriptor" refers to--in
other words, what is variously also called a "file handle" or an "open
file table entry". I like the term (and by extension, I like "file
description lock"), because POSIX has a clearly defined, unambiguous
meaning for it. But, the problem is that people too easily misread
"file description" as "file descriptor".

>> With some thought, you might also come up with an even better name.
>> But, I think *all* of the above alternatives are better than
>> "file-private locks".
>>
>
> Another possibility is file-owned locks. That's really
> closer to how the model works inside the kernel anyway. We have an
> opaque "owner" of the file (an unsigned long basically). Process
> associated locks use a pointer to the file descriptor table there.
> File-private locks (or whatever we want to call them) use a pointer to
> the "struct file".

Yes, but it seems to me here that you're getting into an implementer's
term, not a *users* term. When I see "file-owned file lock" it feels
pretty vacuous as a term. "File-associated file lock" feels a _little_
better (especially when contrasted with "process-associated file
locks"). "File-handle lock" feels even better (because snappier), but
I acknowledge your point about the ambiguities of "file handle".
"File-description locks" is probably a term that works only for people
who read the standards for fun ;-).

>> Please consider moving to one of those pieces of terminology, or a
>> better one of your own creation. Note that also implies that the names
>> of the constants should probably change. (Already, I found the
>> spelling of F_SETLKPW a little strange, since I expected F_SETLKWP.)
>> For example, if you went with the "file handle" terminology, then
>> something like:
>>
>> F_FH_SETLK
>> F_FH_SETLKW
>> F_FH_GETLK
>>
>> That also has the advantage of making the new constants more visually
>> distinctive in code. (I could easily see someone accidentally omitting
>> the "P" in F_SETLKP, and it would not be immediately obvious at a
>> casual scan.)
>>
>
> Where were these comments over the last several months? :)

I have very limited time, unfortunately, so only get to some things
late... (Sorry about that, because you did do a good job of sending
documentation patches early...)

> It would have been better to do this before the v3.15 merge window. As
> it stands now, I think we can reasonably get the public facing
> interface changed as long as we do it in time for v3.15. I'd need to do
> some cleanup of the internal naming for v3.16 to make it all
> consistent, but that's doable.
>
> Given my track record on naming things, I don't really trust myself to
> do this well, so you tell me...what's the ideal name and set of
> constants here?

I have no final answer for you, sorry. I do think we can do better
than "file-private locks", and I think the constants should have more
visually distinctive names. "File-associated file locks" seems to be
our best shot so far, but maybe someone else has something better.

Cheers,

Michael


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]