Re: kern.maxlockf for byte range lock limit
On Tue, Jul 13, 2021 at 06:37:57AM +, Emmanuel Dreyfus wrote: > On Tue, Jul 13, 2021 at 03:36:49AM +, David Holland wrote: > > Well, that was the idea; make it some factor times the current open > > file limit or something like that. Not sure why the existing limit is > > apparently per-user rather than per-process or what that's supposed to > > accomplish. These lock objects are not exactly large so it's not > > necessary to be tightfisted with them. > > Then we could just use maxfiles, coulnd't we? You should be able to have multiple locks per file, but the whole point of the limit was that shouldn't be able to trivially exhaust kernel memory. Joerg
Re: kern.maxlockf for byte range lock limit
On Tue, Jul 13, 2021 at 03:36:49AM +, David Holland wrote: > Well, that was the idea; make it some factor times the current open > file limit or something like that. Not sure why the existing limit is > apparently per-user rather than per-process or what that's supposed to > accomplish. These lock objects are not exactly large so it's not > necessary to be tightfisted with them. Then we could just use maxfiles, coulnd't we? -- Emmanuel Dreyfus m...@netbsd.org
Re: kern.maxlockf for byte range lock limit
On Mon, Jul 12, 2021 at 08:03:54AM +, Emmanuel Dreyfus wrote: > On Fri, Jul 09, 2021 at 08:52:08PM +, David Holland wrote: > > This was discussed somewhere briefly (probably on chat) a couple weeks > > ago, with the conclusion that we should just make the limit something > > like twice the maximum number of open files and forget about it. No > > real need to add more knobs. > > Here is a patch that ties it to maxfiles, with documentation. Tying > to open files would be more complex. We could also tie to > curproc->p_rlimit[RLIMIT_NOFILE].rlim_cur if that is more relevant. Well, that was the idea; make it some factor times the current open file limit or something like that. Not sure why the existing limit is apparently per-user rather than per-process or what that's supposed to accomplish. These lock objects are not exactly large so it's not necessary to be tightfisted with them. note typo: > unprivilegied -- David A. Holland dholl...@netbsd.org
Re: kern.maxlockf for byte range lock limit
On Fri, Jul 09, 2021 at 08:52:08PM +, David Holland wrote:
> This was discussed somewhere briefly (probably on chat) a couple weeks
> ago, with the conclusion that we should just make the limit something
> like twice the maximum number of open files and forget about it. No
> real need to add more knobs.
Here is a patch that ties it to maxfiles, with documentation. Tying
to open files would be more complex. We could also tie to
curproc->p_rlimit[RLIMIT_NOFILE].rlim_cur if that is more relevant.
--
Emmanuel Dreyfus
m...@netbsd.org
Index: sys/kern/vfs_lockf.c
===
RCS file: /cvsroot/src/sys/kern/vfs_lockf.c,v
retrieving revision 1.73
diff -U4 -r1.73 vfs_lockf.c
--- sys/kern/vfs_lockf.c 31 Jan 2011 08:25:32 - 1.73
+++ sys/kern/vfs_lockf.c 12 Jul 2021 07:51:43 -
@@ -122,9 +122,9 @@
* If you're slightly above the limit, we still have to permit an allocation
* so that the unlock can succeed. If the unlocking causes too many splits,
* however, you're totally cutoff.
*/
-int maxlocksperuid = 1024;
+#define MAXLOCKSPERUID (2 * maxfiles)
#ifdef LOCKF_DEBUG
/*
* Print out a lock.
@@ -199,9 +199,9 @@
uip = uid_find(uid);
lcnt = atomic_inc_ulong_nv(>ui_lockcnt);
if (uid && allowfail && lcnt >
- (allowfail == 1 ? maxlocksperuid : (maxlocksperuid * 2))) {
+ (allowfail == 1 ? MAXLOCKSPERUID : (MAXLOCKSPERUID * 2))) {
atomic_dec_ulong(>ui_lockcnt);
return NULL;
}
Index: lib/libc/sys/fcntl.2
===
RCS file: /cvsroot/src/lib/libc/sys/fcntl.2,v
retrieving revision 1.45
diff -U4 -r1.45 fcntl.2
--- lib/libc/sys/fcntl.2 27 Sep 2019 07:20:07 - 1.45
+++ lib/libc/sys/fcntl.2 12 Jul 2021 07:51:43 -
@@ -548,8 +548,19 @@
.Fa cmd
is
.Dv F_GETPATH
and insufficient memory is available.
+.Pp
+The argument
+.Fa cmd
+is
+.Dv F_GETLK ,
+.Dv F_SETLK ,
+or
+.Dv F_SETLKW ,
+and the file lock limit for the current unprivilegied user
+has been reached. It can be modifed using sysctl
+.Li kern.maxfiles .
.It Bq Er ERANGE
The argument
.Fa cmd
is
Index: lib/libc/sys/flock.2
===
RCS file: /cvsroot/src/lib/libc/sys/flock.2,v
retrieving revision 1.22
diff -U4 -r1.22 flock.2
--- lib/libc/sys/flock.2 15 Oct 2011 21:35:50 - 1.22
+++ lib/libc/sys/flock.2 12 Jul 2021 07:51:43 -
@@ -136,8 +136,12 @@
.Dv LOCK_EX ,
.Dv LOCK_SH ,
or
.Dv LOCK_UN .
+.It Bq Eq ENOMEM
+The file lock limit for the current unprivilegied user
+has been reached. It can be modifed using sysctl
+.Li kern.maxfiles .
.It Bq Er EOPNOTSUPP
The argument
.Fa fd
refers to an object other than a file.
Index: share/man/man7/sysctl.7
===
RCS file: /cvsroot/src/share/man/man7/sysctl.7,v
retrieving revision 1.151
diff -U4 -r1.151 sysctl.7
--- share/man/man7/sysctl.7 17 Oct 2020 09:20:33 - 1.151
+++ share/man/man7/sysctl.7 12 Jul 2021 07:51:43 -
@@ -755,8 +755,13 @@
Memory Mapped Files Option is available on this system,
otherwise\ 0.
.It Li kern.maxfiles ( Dv KERN_MAXFILES )
The maximum number of open files that may be open in the system.
+This also controls the maximum file locks per unprivilegied user
+enforced by
+.Xr fnctl 2
+and
+.Xr flock 2 .
.It Li kern.maxpartitions ( Dv KERN_MAXPARTITIONS )
The maximum number of partitions allowed per disk.
.It Li kern.maxlwp
The maximum number of Lightweight Processes (threads) the system allows
Re: kern.maxlockf for byte range lock limit
On Fri, Jul 09, 2021 at 03:27:57PM +, Emmanuel Dreyfus wrote: > I recently hit a bug where mariadb import failed befause of > maxlocksperuid limit:a > http://mail-index.netbsd.org/netbsd-users/2021/07/09/msg027330.html > > Attached is a patch that documents ENOMEM for fcntl() and flock() > and that adds a sysctl kern.maxlockf to make the limit configurable. This was discussed somewhere briefly (probably on chat) a couple weeks ago, with the conclusion that we should just make the limit something like twice the maximum number of open files and forget about it. No real need to add more knobs. -- David A. Holland dholl...@netbsd.org
re: kern.maxlockf for byte range lock limit
> @@ -146,8 +146,10 @@
> {
> extern int kern_logsigexit; /* defined in kern/kern_sig.c */
> extern fixpt_t ccpu;/* defined in kern/kern_synch.c */
> extern int dumponpanic; /* defined in kern/subr_prf.c */
> + extern int maxlocksperuid; /* defined in kern/vfs_lockf.c */
> +
this part is gross, please don't add more of this.
all of those variabes should be put in a header file so
that the declarations can't be different. at the very
least, do that for your new one, even if you don't want
to fix the other 3 as well.
..or simply move the sysctl creation into the file that
owns that variable and you could avoid hard coding a
version number for it (not something we prefer since
the introduction of dynamic sysctl long ago.)
.mrg.
