On Sun, Jun 02, 2024 at 10:58:03PM +0100, Al Viro wrote:
> On Sun, Jun 02, 2024 at 09:42:38PM +0100, Al Viro wrote:
> > We never had callers for __close_range() except for close_range(2)
> > itself. Nothing of that sort has appeared in four years and if any users
> > do show up, we can always separate those suckers again.
>
> BTW, looking through close_range()... We have
>
> static unsigned int sane_fdtable_size(struct fdtable *fdt, unsigned int max_fds)
> {
> unsigned int count;
>
> count = count_open_files(fdt);
> if (max_fds < NR_OPEN_DEFAULT)
> max_fds = NR_OPEN_DEFAULT;
> return ALIGN(min(count, max_fds), BITS_PER_LONG);
> }
>
> which decides how large a table would be needed for descriptors below max_fds
> that are opened in fdt. And we start with finding the last opened descriptor
> in fdt (well, rounded up to BITS_PER_LONG, if you look at count_open_files()).
>
> Why do we bother to look at _anything_ past the max_fds? Does anybody have
> objections to the following?
No, it's fine but folding count_open_files() into sane_fdtable_size() is
a bit less readable imho. In any case, could you please slap an
explanatory comment on top of
while (words > min_words && !fdt->open_fds[words - 1])
words--;
That code is unreadable enough as it is.
>
> diff --git a/fs/file.c b/fs/file.c
> index f9fcebc7c838..4976ede108e0 100644
> --- a/fs/file.c
> +++ b/fs/file.c
> @@ -276,20 +276,6 @@ static inline bool fd_is_open(unsigned int fd, const struct fdtable *fdt)
> return test_bit(fd, fdt->open_fds);
> }
>
> -static unsigned int count_open_files(struct fdtable *fdt)
> -{
> - unsigned int size = fdt->max_fds;
> - unsigned int i;
> -
> - /* Find the last open fd */
> - for (i = size / BITS_PER_LONG; i > 0; ) {
> - if (fdt->open_fds[--i])
> - break;
> - }
> - i = (i + 1) * BITS_PER_LONG;
> - return i;
> -}
> -
> /*
> * Note that a sane fdtable size always has to be a multiple of
> * BITS_PER_LONG, since we have bitmaps that are sized by this.
> @@ -305,12 +291,18 @@ static unsigned int count_open_files(struct fdtable *fdt)
> */
> static unsigned int sane_fdtable_size(struct fdtable *fdt, unsigned int max_fds)
> {
> - unsigned int count;
> + const unsigned int min_words = BITS_TO_LONGS(NR_OPEN_DEFAULT); // 1
> + unsigned int words;
> +
> + if (max_fds <= NR_OPEN_DEFAULT)
> + return NR_OPEN_DEFAULT;
> +
> + words = BITS_TO_LONGS(min(max_fds, fdt->max_fds)); // >= min_words
> +
> + while (words > min_words && !fdt->open_fds[words - 1])
> + words--;
>
> - count = count_open_files(fdt);
> - if (max_fds < NR_OPEN_DEFAULT)
> - max_fds = NR_OPEN_DEFAULT;
> - return ALIGN(min(count, max_fds), BITS_PER_LONG);
> + return words * BITS_PER_LONG;
> }
>
> /*
