On Fri, Feb 09, 2018 at 11:26:01AM +0300, Kirill Tkhai wrote:
> On 08.02.2018 19:02, Matthew Wilcox wrote:
> > On Thu, Feb 08, 2018 at 06:07:37PM +0300, Kirill Tkhai wrote:
> >> Since I need INOTIFY_IDR_END to check ioctl's third argument, it's better
> >> it's defined as positive number. But when not-zero value is passed
> >> to idr_get_free(), this function decrements it. Also, idr_alloc_cyclic()
> >> defined @end as int argument. So, it's impossible to pass positive @end
> >> argument to idr_alloc_cyclic() to get INT_MAX id. And after this patch
> >> inotify watch descriptors ids will take numbers [1, INT_MAX-1], INT_MAX
> >> will be unavailable.
> >
> > Ummm. Why not just do:
> >
> > +#ifdef CONFIG_CHECKPOINT_RESTORE
> > + case INOTIFY_IOC_SETNEXTWD:
> > + ret = -EINVAL;
> > + if (arg >= 1 && arg <= INT_MAX) {
> > + spin_lock(&data->idr_lock);
> > + idr_set_cursor(&data->idr, (unsigned int)arg);
> > + spin_unlock(&data->idr_lock);
> > + ret = 0;
> > + }
> > + break;
> > +#endif /* CONFIG_CHECKPOINT_RESTORE */
>
> INT_MAX is generic constant, and the fact, it is used in the above hunk,
> does not make feel it's the same constant as used in inotify_add_to_idr().
But the IDR guarantees you a number between 0 and INT_MAX. If the
reader doesn't understand that, then let them find it out. They'll be
better off for knowing that.
I improved the documentation for idr_alloc recently, although Linus hasn't
pulled it yet. Now it reads:
/**
* idr_alloc() - Allocate an ID.
* @idr: IDR handle.
* @ptr: Pointer to be associated with the new ID.
* @start: The minimum ID (inclusive).
* @end: The maximum ID (exclusive).
* @gfp: Memory allocation flags.
*
* Allocates an unused ID in the range specified by @start and @end. If
* @end is <= 0, it is treated as one larger than %INT_MAX. This allows
* callers to use @start + N as @end as long as N is within integer range.
*
* The caller should provide their own locking to ensure that two
* concurrent modifications to the IDR are not possible. Read-only
* accesses to the IDR may be done under the RCU read lock or may
* exclude simultaneous writers.
*
* Return: The newly allocated ID, -ENOMEM if memory allocation failed,
* or -ENOSPC if no free IDs could be found.
*/
