Add KERN_LOCKF

[kib]

Summary

An interface to retrieve the list of advisory file locks.

procstat(1) implementation might follow, but it is too much work ATM. Main target is lsof.

[kib]

Add libprocstat bits. Still no procstat(1).

[rmacklem]

I'll admit I didn't look at the code in detail, but it looks ok to me.

It won't be useful for NFSv4, since the locks are not kept in
the generic lock structures (except to the NFS server when
vfs.nfsd.enable_locallocks is set non-zero).

A VOP call would allow the NFSv4 client to report locks
(but not their file pathnames), so I don't know if it is worth
the bother?

ps: Good to see that you are at least able to do some stuff.

I can't imagine what living in Kyiv would be like right now.

[kib]

In D34756#787797, @rmacklem wrote:

I'll admit I didn't look at the code in detail, but it looks ok to me.

It won't be useful for NFSv4, since the locks are not kept in
the generic lock structures (except to the NFS server when
vfs.nfsd.enable_locallocks is set non-zero).

A VOP call would allow the NFSv4 client to report locks
(but not their file pathnames), so I don't know if it is worth
the bother?

pathes are documented in libprocstat(3) as optional.

When I thought about the code structure, I indeed considered two options: either iterate over lf_lock_states list, as is done in the current review, or iterate over all vnodes and then looking at v_lockf for it. Second option did not stayed for too long, because we typically have several millions of vnodes cached, while typical system only have several dozens of locks. Second option was considered at all because the locking is more natural for it: vnode lock is before lockf state locks. I have to do some trick which slightly decreases the consistency of the reported snapshot.

May be we could have some additional code to iterate over NFSv4+ mounts to gather fs-specific adv lock implementation data. This definitely cannot be VOP, it should be VFS_OP-like interface, for later.

[rmacklem]

sys/kern/kern_lockf.c

2525

How does fsidx get set. Maybe I just can't spot it,
but I don't see it being set in the above code?

2530

Although it would probably be less efficient,
due to making multiple passes through the above
for each local fs, if the code from line #2494 to here
were "per file system", then it could be a VFS_xxx() op,
with the above being the vfs_stdxxxop().
(That's why I was looking for where fsidx got set.)

And, yes, you were correct. For the NFS client, it
would need to be a VFS_xxx() call that returns all
locks for the file system.

Or, maybe it could do the above to cover the local
file systems and then a VFS_xxx() call for each remote
fs?

Anyhow, I'll leave it up to you.
As you noted, it could be a future enhancement, maybe?

[kib]

sys/kern/kern_lockf.c

2525

Right, it should has been moved to the loop below.

[kib]

Use per-fs vfs_report_lockf method. If it is null, standard implementation is used.

[rmacklem]

sys/kern/kern_lockf.c

2494

My concern with this version is performance when there
are a lot of file systems. Peter Errikson sets up storage
servers with 20,000 file systems on them. (I think he had
one that had over 80,000 file systems on it.)

I'm almost tempted to suggest going back to your previous
version (one pass through all lf_advlock() locks) and then
(it doesn't need to be done now) possibly a VFS_REPORT_LOCKF()
call done for all mounts. In this case, the default call would just
return. Then file systems that keep locks other ways, like the
NFSv4 client, could implement a non-NULL VFS_REPORT_LOCKF().

Anyhow, do whatever you think is best.

[kib]

sys/kern/kern_lockf.c

2494

This code is definitely not a hot path. In worst case, the report of the advisory locks list would take a long time.

If somebody complains, we can do some optimizations, for now I prefer to keep your advise to allow for eventual proper reporting for private adv locking implementations.

[rmacklem]

Looks fine to me now.

I can cobble to-gether a nfs_report_lockf() once this goes
into main.

[markj]

sys/kern/kern_lockf.c

2492

How is it possible to have vp == NULL?

2530

Did you mean to include LK_RETRY here? vn_lock(LK_RETRY) never fails.

2541

What's the purpose of initializing fullpath?

2651

This fits on one line.

[kib]

sys/kern/kern_lockf.c

2541

Because it is done by other places. I am not sure at what point vn_fullpath() started doing something to fullpath in case of error.

For now I kept it as is. If you insist, I would remove the initialization.

[markj]

lib/libprocstat/libprocstat.3

507

509

515

518

Maybe, "For each lock, unique identifiers for the locked file and its mount point are guaranteed to be provided."

sys/sys/user.h

475

Consider renaming them to KLOCKF_* to match the "kinfo_lockf" name.

[kib]

Rename symbols KLOCK_ -> KLOCKF_.
Man page editing.

[markj]

lib/libprocstat/libprocstat.3

507

509

missed it before, sorry