Re: [RFC PATCH v2 4/4] Allow to trace fd usage with rlimit-events

[Krzysztof Opasiak]

On 12/15/2017 12:59 AM, Andreas Dilger wrote:
> On Dec 14, 2017, at 3:00 PM, Krzysztof Opasiak <k.opasiak@xxxxxxxxxxx> wrote:
> > Add rlimit-events calls to file descriptors management
> > code to allow tracing of FD usage.
> >
> > This allows userspace process (monitor) to get notification when
> > other process (subject) uses given amount of file descriptors.
> >
> > This can be used to for example asynchronously monitor number
> > of open FD's in system services instead of polling with
> > predefined interval.
>
> I'm not involved in this area of code, but one optimization question:
>
> > +static unsigned int count_open_fds(struct fdtable *fdt)
> > +{
> > +	unsigned int maxfd = fdt->max_fds;
> > +	unsigned int maxbit = maxfd / BITS_PER_LONG;
> > +	unsigned int count = 0;
> > +	int i;
> > +
> > +	i = find_next_zero_bit(fdt->full_fds_bits, maxbit, 0);
> > +	/* If there is no free fds */
> > +	if (i > maxbit)
> > +		return maxfd;
> > +#if BITS_PER_LONG == 32
> > +#define HWEIGHT_LONG hweight32
> > +#else
> > +#define HWEIGHT_LONG hweight64
> > +#endif
> > +
> > +	count += i * BITS_PER_LONG;
> > +	for (; i < maxbit; ++i)
> > +		count += HWEIGHT_LONG(fdt->open_fds[i]);
> > +
> > +#undef HWEIGHT_LONG
> > +	return count;
> > +}
>
> Since find_next_zero_bit() needs to process all of the words anyway
> as well as lots of extra operations that add overhead, it looks more
> efficient to just compute HWEIGHT_LONG(open_fds[]) for the whole array.

I may try to measure this but I'm not sure now because there are two 
levels of bit maps. First one (open_fds) is a real bitmap of open file 
descriptors (which can be quite large) while full_fds_bits is a second 
level bitmap (which is 8 times smaller) and if bit there is set it means 
that whole byte in lower bitmap (open_fds) is set. Because we are always 
opening the lowest fd number that is available we should have quite 
large contiguous region in the beginning so using this 
find_next_zero_bit() at first should allow us to skip this region.

But anyway I can try your suggestion and check if it speeds up.

Best regards,
--
Krzysztof Opasiak
Samsung R&D Institute Poland
Samsung Electronics