Re: [PATCH] Cygwin: getdtablesize: always return OPEN_MAX_MAX
On Feb 1 10:50, Corinna Vinschen via Cygwin-patches wrote: > On Jan 28 15:33, Ken Brown via Cygwin-patches wrote: > > On 1/28/2021 11:07 AM, Corinna Vinschen via Cygwin-patches wrote: > > > One problem is that there are some applications in the wild which run > > > loops up to either sysconf(_SC_OPEN_MAX) or OPEN_MAX to handle open > > > descriptors. tcsh is one of them. It may slow done tcsh quite a bit > > > if the loop runs to 3200 now every time. > > > > I don't use tcsh. Is it easy to test this? > > I just checked the source. In the olden days, before the invention of > close-on-exec, tcsh closed all descriptors > 2 up to OPEN_MAX prior to > starting any executable. > > With close-on-exec this happens only at startup and after an error > occured. > > So testing should be easy: The tcsh startup may be noticably slower. I checked this right now and I don't see a noticable, i. .e, any, difference. Corinna
Re: [PATCH] Cygwin: getdtablesize: always return OPEN_MAX_MAX
On Jan 28 15:33, Ken Brown via Cygwin-patches wrote: > On 1/28/2021 11:07 AM, Corinna Vinschen via Cygwin-patches wrote: > > One problem is that there are some applications in the wild which run > > loops up to either sysconf(_SC_OPEN_MAX) or OPEN_MAX to handle open > > descriptors. tcsh is one of them. It may slow done tcsh quite a bit > > if the loop runs to 3200 now every time. > > I don't use tcsh. Is it easy to test this? I just checked the source. In the olden days, before the invention of close-on-exec, tcsh closed all descriptors > 2 up to OPEN_MAX prior to starting any executable. With close-on-exec this happens only at startup and after an error occured. So testing should be easy: The tcsh startup may be noticably slower. Corinna
Re: [PATCH] Cygwin: getdtablesize: always return OPEN_MAX_MAX
On Jan 29 14:23, Ken Brown via Cygwin-patches wrote: > On 1/28/2021 5:28 PM, Ken Brown via Cygwin-patches wrote: > > > ...ideally by adding a file include/cygwin/limits.h included by > > > include/limits.h, which defines __OPEN_MAX et al, as required. > > > > I'm not completely sure I follow. Do you mean include/cygwin/limits.h > > should contain > > > > #define __OPEN_MAX 3200 > > > > and include/limits.h should contain > > > > #define OPEN_MAX __OPEN_MAX ? > > > > For the sake of my education, could you explain the reason for this? > > Trying to answer my own question, I guess the idea is to hide implementation > details from viewers of limits.h. Is that right? Yes, that was the idea, kind of like a poor mans include/bits dir on Linux... > I took a stab at this and > am about to send a patchset. I'm not sure whether I made a reasonable > choice of "et al" in "__OPEN_MAX et al". Sorry, I didn't mean to imply you will have to do that right away. It was just a thought to move over more values in later patches. Corinna
Re: [PATCH] Cygwin: getdtablesize: always return OPEN_MAX_MAX
On 1/28/2021 5:28 PM, Ken Brown via Cygwin-patches wrote: On 1/28/2021 11:13 AM, Corinna Vinschen via Cygwin-patches wrote: On Jan 28 17:07, Corinna Vinschen via Cygwin-patches wrote: On Jan 28 08:42, Ken Brown via Cygwin-patches wrote: On 1/28/2021 5:20 AM, Corinna Vinschen via Cygwin-patches wrote: On Jan 27 21:51, Ken Brown via Cygwin-patches wrote: According to the Linux man page for getdtablesize(3), the latter is supposed to return "the maximum number of files a process can have open, one more than the largest possible value for a file descriptor." The constant OPEN_MAX_MAX is the only limit enforced by Cygwin, so we now return that. Previously getdtablesize returned the current size of cygheap->fdtab, Cygwin's internal file descriptor table. But this is a dynamically growing table, and its current size does not reflect an actual limit on the number of open files. With this change, gnulib now reports that getdtablesize and fcntl(F_DUPFD) work on Cygwin. Packages like GNU tar that use the corresponding gnulib modules will no longer use gnulib replacements on Cygwin. --- winsup/cygwin/syscalls.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc index 5da05b18a..1f16d54b9 100644 --- a/winsup/cygwin/syscalls.cc +++ b/winsup/cygwin/syscalls.cc @@ -2887,7 +2887,7 @@ setdtablesize (int size) extern "C" int getdtablesize () { - return cygheap->fdtab.size; + return OPEN_MAX_MAX; } getdtablesize is used internally, too. After this change, the values returned by sysconf and getrlimit should be revisited as well. They will now return OPEN_MAX_MAX, as I think they should. The only question in my mind is whether to simplify the code by removing the calls to getdtablesize, something like this (untested): But then again, what happens with OPEN_MAX in limits.h? Linux removed it entirely. Given we have such a limit and it's not flexible as on Linux, should we go ahead, drop OPEN_MAX_MAX entirely and define OPEN_MAX as 3200? ...ideally by adding a file include/cygwin/limits.h included by include/limits.h, which defines __OPEN_MAX et al, as required. I'm not completely sure I follow. Do you mean include/cygwin/limits.h should contain #define __OPEN_MAX 3200 and include/limits.h should contain #define OPEN_MAX __OPEN_MAX ? For the sake of my education, could you explain the reason for this? Trying to answer my own question, I guess the idea is to hide implementation details from viewers of limits.h. Is that right? I took a stab at this and am about to send a patchset. I'm not sure whether I made a reasonable choice of "et al" in "__OPEN_MAX et al". Ken
Re: [PATCH] Cygwin: getdtablesize: always return OPEN_MAX_MAX
On 1/28/2021 11:13 AM, Corinna Vinschen via Cygwin-patches wrote: On Jan 28 17:07, Corinna Vinschen via Cygwin-patches wrote: On Jan 28 08:42, Ken Brown via Cygwin-patches wrote: On 1/28/2021 5:20 AM, Corinna Vinschen via Cygwin-patches wrote: On Jan 27 21:51, Ken Brown via Cygwin-patches wrote: According to the Linux man page for getdtablesize(3), the latter is supposed to return "the maximum number of files a process can have open, one more than the largest possible value for a file descriptor." The constant OPEN_MAX_MAX is the only limit enforced by Cygwin, so we now return that. Previously getdtablesize returned the current size of cygheap->fdtab, Cygwin's internal file descriptor table. But this is a dynamically growing table, and its current size does not reflect an actual limit on the number of open files. With this change, gnulib now reports that getdtablesize and fcntl(F_DUPFD) work on Cygwin. Packages like GNU tar that use the corresponding gnulib modules will no longer use gnulib replacements on Cygwin. --- winsup/cygwin/syscalls.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc index 5da05b18a..1f16d54b9 100644 --- a/winsup/cygwin/syscalls.cc +++ b/winsup/cygwin/syscalls.cc @@ -2887,7 +2887,7 @@ setdtablesize (int size) extern "C" int getdtablesize () { - return cygheap->fdtab.size; + return OPEN_MAX_MAX; } getdtablesize is used internally, too. After this change, the values returned by sysconf and getrlimit should be revisited as well. They will now return OPEN_MAX_MAX, as I think they should. The only question in my mind is whether to simplify the code by removing the calls to getdtablesize, something like this (untested): But then again, what happens with OPEN_MAX in limits.h? Linux removed it entirely. Given we have such a limit and it's not flexible as on Linux, should we go ahead, drop OPEN_MAX_MAX entirely and define OPEN_MAX as 3200? ...ideally by adding a file include/cygwin/limits.h included by include/limits.h, which defines __OPEN_MAX et al, as required. I'm not completely sure I follow. Do you mean include/cygwin/limits.h should contain #define __OPEN_MAX 3200 and include/limits.h should contain #define OPEN_MAX __OPEN_MAX ? For the sake of my education, could you explain the reason for this? Thanks. Ken
Re: [PATCH] Cygwin: getdtablesize: always return OPEN_MAX_MAX
On 1/28/2021 11:07 AM, Corinna Vinschen via Cygwin-patches wrote: On Jan 28 08:42, Ken Brown via Cygwin-patches wrote: On 1/28/2021 5:20 AM, Corinna Vinschen via Cygwin-patches wrote: On Jan 27 21:51, Ken Brown via Cygwin-patches wrote: According to the Linux man page for getdtablesize(3), the latter is supposed to return "the maximum number of files a process can have open, one more than the largest possible value for a file descriptor." The constant OPEN_MAX_MAX is the only limit enforced by Cygwin, so we now return that. Previously getdtablesize returned the current size of cygheap->fdtab, Cygwin's internal file descriptor table. But this is a dynamically growing table, and its current size does not reflect an actual limit on the number of open files. With this change, gnulib now reports that getdtablesize and fcntl(F_DUPFD) work on Cygwin. Packages like GNU tar that use the corresponding gnulib modules will no longer use gnulib replacements on Cygwin. --- winsup/cygwin/syscalls.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc index 5da05b18a..1f16d54b9 100644 --- a/winsup/cygwin/syscalls.cc +++ b/winsup/cygwin/syscalls.cc @@ -2887,7 +2887,7 @@ setdtablesize (int size) extern "C" int getdtablesize () { - return cygheap->fdtab.size; + return OPEN_MAX_MAX; } getdtablesize is used internally, too. After this change, the values returned by sysconf and getrlimit should be revisited as well. They will now return OPEN_MAX_MAX, as I think they should. The only question in my mind is whether to simplify the code by removing the calls to getdtablesize, something like this (untested): But then again, what happens with OPEN_MAX in limits.h? Linux removed it entirely. Given we have such a limit and it's not flexible as on Linux, should we go ahead, drop OPEN_MAX_MAX entirely and define OPEN_MAX as 3200? Makes sense to me. One problem is that there are some applications in the wild which run loops up to either sysconf(_SC_OPEN_MAX) or OPEN_MAX to handle open descriptors. tcsh is one of them. It may slow done tcsh quite a bit if the loop runs to 3200 now every time. I don't use tcsh. Is it easy to test this? Ken
Re: [PATCH] Cygwin: getdtablesize: always return OPEN_MAX_MAX
On Jan 28 17:07, Corinna Vinschen via Cygwin-patches wrote: > On Jan 28 08:42, Ken Brown via Cygwin-patches wrote: > > On 1/28/2021 5:20 AM, Corinna Vinschen via Cygwin-patches wrote: > > > On Jan 27 21:51, Ken Brown via Cygwin-patches wrote: > > > > According to the Linux man page for getdtablesize(3), the latter is > > > > supposed to return "the maximum number of files a process can have > > > > open, one more than the largest possible value for a file descriptor." > > > > The constant OPEN_MAX_MAX is the only limit enforced by Cygwin, so we > > > > now return that. > > > > > > > > Previously getdtablesize returned the current size of cygheap->fdtab, > > > > Cygwin's internal file descriptor table. But this is a dynamically > > > > growing table, and its current size does not reflect an actual limit > > > > on the number of open files. > > > > > > > > With this change, gnulib now reports that getdtablesize and > > > > fcntl(F_DUPFD) work on Cygwin. Packages like GNU tar that use the > > > > corresponding gnulib modules will no longer use gnulib replacements on > > > > Cygwin. > > > > --- > > > > winsup/cygwin/syscalls.cc | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc > > > > index 5da05b18a..1f16d54b9 100644 > > > > --- a/winsup/cygwin/syscalls.cc > > > > +++ b/winsup/cygwin/syscalls.cc > > > > @@ -2887,7 +2887,7 @@ setdtablesize (int size) > > > > extern "C" int > > > > getdtablesize () > > > > { > > > > - return cygheap->fdtab.size; > > > > + return OPEN_MAX_MAX; > > > > } > > > > > > getdtablesize is used internally, too. After this change, the values > > > returned by sysconf and getrlimit should be revisited as well. > > > > They will now return OPEN_MAX_MAX, as I think they should. The only > > question in my mind is whether to simplify the code by removing the calls to > > getdtablesize, something like this (untested): > > But then again, what happens with OPEN_MAX in limits.h? Linux removed > it entirely. Given we have such a limit and it's not flexible as on > Linux, should we go ahead, drop OPEN_MAX_MAX entirely and define > OPEN_MAX as 3200? ...ideally by adding a file include/cygwin/limits.h included by include/limits.h, which defines __OPEN_MAX et al, as required. Corinna
Re: [PATCH] Cygwin: getdtablesize: always return OPEN_MAX_MAX
On Jan 28 08:42, Ken Brown via Cygwin-patches wrote: > On 1/28/2021 5:20 AM, Corinna Vinschen via Cygwin-patches wrote: > > On Jan 27 21:51, Ken Brown via Cygwin-patches wrote: > > > According to the Linux man page for getdtablesize(3), the latter is > > > supposed to return "the maximum number of files a process can have > > > open, one more than the largest possible value for a file descriptor." > > > The constant OPEN_MAX_MAX is the only limit enforced by Cygwin, so we > > > now return that. > > > > > > Previously getdtablesize returned the current size of cygheap->fdtab, > > > Cygwin's internal file descriptor table. But this is a dynamically > > > growing table, and its current size does not reflect an actual limit > > > on the number of open files. > > > > > > With this change, gnulib now reports that getdtablesize and > > > fcntl(F_DUPFD) work on Cygwin. Packages like GNU tar that use the > > > corresponding gnulib modules will no longer use gnulib replacements on > > > Cygwin. > > > --- > > > winsup/cygwin/syscalls.cc | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc > > > index 5da05b18a..1f16d54b9 100644 > > > --- a/winsup/cygwin/syscalls.cc > > > +++ b/winsup/cygwin/syscalls.cc > > > @@ -2887,7 +2887,7 @@ setdtablesize (int size) > > > extern "C" int > > > getdtablesize () > > > { > > > - return cygheap->fdtab.size; > > > + return OPEN_MAX_MAX; > > > } > > > > getdtablesize is used internally, too. After this change, the values > > returned by sysconf and getrlimit should be revisited as well. > > They will now return OPEN_MAX_MAX, as I think they should. The only > question in my mind is whether to simplify the code by removing the calls to > getdtablesize, something like this (untested): But then again, what happens with OPEN_MAX in limits.h? Linux removed it entirely. Given we have such a limit and it's not flexible as on Linux, should we go ahead, drop OPEN_MAX_MAX entirely and define OPEN_MAX as 3200? One problem is that there are some applications in the wild which run loops up to either sysconf(_SC_OPEN_MAX) or OPEN_MAX to handle open descriptors. tcsh is one of them. It may slow done tcsh quite a bit if the loop runs to 3200 now every time. Corinna
Re: [PATCH] Cygwin: getdtablesize: always return OPEN_MAX_MAX
On 1/28/2021 5:20 AM, Corinna Vinschen via Cygwin-patches wrote: On Jan 27 21:51, Ken Brown via Cygwin-patches wrote: According to the Linux man page for getdtablesize(3), the latter is supposed to return "the maximum number of files a process can have open, one more than the largest possible value for a file descriptor." The constant OPEN_MAX_MAX is the only limit enforced by Cygwin, so we now return that. Previously getdtablesize returned the current size of cygheap->fdtab, Cygwin's internal file descriptor table. But this is a dynamically growing table, and its current size does not reflect an actual limit on the number of open files. With this change, gnulib now reports that getdtablesize and fcntl(F_DUPFD) work on Cygwin. Packages like GNU tar that use the corresponding gnulib modules will no longer use gnulib replacements on Cygwin. --- winsup/cygwin/syscalls.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc index 5da05b18a..1f16d54b9 100644 --- a/winsup/cygwin/syscalls.cc +++ b/winsup/cygwin/syscalls.cc @@ -2887,7 +2887,7 @@ setdtablesize (int size) extern "C" int getdtablesize () { - return cygheap->fdtab.size; + return OPEN_MAX_MAX; } getdtablesize is used internally, too. After this change, the values returned by sysconf and getrlimit should be revisited as well. They will now return OPEN_MAX_MAX, as I think they should. The only question in my mind is whether to simplify the code by removing the calls to getdtablesize, something like this (untested): diff --git a/winsup/cygwin/resource.cc b/winsup/cygwin/resource.cc index 9e39d3a04..ac56acf8c 100644 --- a/winsup/cygwin/resource.cc +++ b/winsup/cygwin/resource.cc @@ -182,10 +182,7 @@ getrlimit (int resource, struct rlimit *rlp) __get_rlimit_stack (rlp); break; case RLIMIT_NOFILE: - rlp->rlim_cur = getdtablesize (); - if (rlp->rlim_cur < OPEN_MAX) - rlp->rlim_cur = OPEN_MAX; - rlp->rlim_max = OPEN_MAX_MAX; + rlp->rlim_cur = rlp->rlim_max = OPEN_MAX_MAX; break; case RLIMIT_CORE: rlp->rlim_cur = cygheap->rlim_core; diff --git a/winsup/cygwin/sysconf.cc b/winsup/cygwin/sysconf.cc index 001da96ad..d5d82bb4a 100644 --- a/winsup/cygwin/sysconf.cc +++ b/winsup/cygwin/sysconf.cc @@ -21,15 +21,6 @@ details. */ #include "cpuid.h" #include "clock.h" -static long -get_open_max (int in) -{ - long max = getdtablesize (); - if (max < OPEN_MAX) -max = OPEN_MAX; - return max; -} - static long get_page_size (int in) { @@ -520,7 +511,7 @@ static struct {cons, {c:CHILD_MAX}}, /* 1, _SC_CHILD_MAX */ {cons, {c:CLOCKS_PER_SEC}}, /* 2, _SC_CLK_TCK */ {cons, {c:NGROUPS_MAX}}, /* 3, _SC_NGROUPS_MAX */ - {func, {f:get_open_max}},/* 4, _SC_OPEN_MAX */ + {cons, {c:OPEN_MAX_MAX}},/* 4, _SC_OPEN_MAX */ {cons, {c:_POSIX_JOB_CONTROL}}, /* 5, _SC_JOB_CONTROL */ {cons, {c:_POSIX_SAVED_IDS}},/* 6, _SC_SAVED_IDS */ {cons, {c:_POSIX_VERSION}}, /* 7, _SC_VERSION */ WDYT? Ken
Re: [PATCH] Cygwin: getdtablesize: always return OPEN_MAX_MAX
On Jan 27 21:51, Ken Brown via Cygwin-patches wrote: > According to the Linux man page for getdtablesize(3), the latter is > supposed to return "the maximum number of files a process can have > open, one more than the largest possible value for a file descriptor." > The constant OPEN_MAX_MAX is the only limit enforced by Cygwin, so we > now return that. > > Previously getdtablesize returned the current size of cygheap->fdtab, > Cygwin's internal file descriptor table. But this is a dynamically > growing table, and its current size does not reflect an actual limit > on the number of open files. > > With this change, gnulib now reports that getdtablesize and > fcntl(F_DUPFD) work on Cygwin. Packages like GNU tar that use the > corresponding gnulib modules will no longer use gnulib replacements on > Cygwin. > --- > winsup/cygwin/syscalls.cc | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc > index 5da05b18a..1f16d54b9 100644 > --- a/winsup/cygwin/syscalls.cc > +++ b/winsup/cygwin/syscalls.cc > @@ -2887,7 +2887,7 @@ setdtablesize (int size) > extern "C" int > getdtablesize () > { > - return cygheap->fdtab.size; > + return OPEN_MAX_MAX; > } getdtablesize is used internally, too. After this change, the values returned by sysconf and getrlimit should be revisited as well. Thanks, Corinna
[PATCH] Cygwin: getdtablesize: always return OPEN_MAX_MAX
According to the Linux man page for getdtablesize(3), the latter is supposed to return "the maximum number of files a process can have open, one more than the largest possible value for a file descriptor." The constant OPEN_MAX_MAX is the only limit enforced by Cygwin, so we now return that. Previously getdtablesize returned the current size of cygheap->fdtab, Cygwin's internal file descriptor table. But this is a dynamically growing table, and its current size does not reflect an actual limit on the number of open files. With this change, gnulib now reports that getdtablesize and fcntl(F_DUPFD) work on Cygwin. Packages like GNU tar that use the corresponding gnulib modules will no longer use gnulib replacements on Cygwin. --- winsup/cygwin/syscalls.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc index 5da05b18a..1f16d54b9 100644 --- a/winsup/cygwin/syscalls.cc +++ b/winsup/cygwin/syscalls.cc @@ -2887,7 +2887,7 @@ setdtablesize (int size) extern "C" int getdtablesize () { - return cygheap->fdtab.size; + return OPEN_MAX_MAX; } extern "C" int -- 2.30.0