Messages in this thread | | | Subject | Re: [PATCH -mm 0/3] proc: task->signal can't be NULL | From | (Eric W. Biederman) | Date | Mon, 22 Mar 2010 19:18:38 -0700 |
| |
Oleg Nesterov <oleg@redhat.com> writes:
> With the recent changes in -mm it is always safe to dereference > task->signal. It can't be NULL and it is pinned to task_struct. > > fs/proc becomes the only valid user of signal->count which should > either die or become "int nr_threads". > > > Alexey, Eric. > > Can't we kill this counter? Afaics, get_nr_threads() doesn't need to > be "precise", we probably can estimate the number of threads using > signal->live (yes sure, we can't use ->live as nr_threads). > > Except: first_tid() uses get_nr_threads() for optimization. Is this > optimization really important? Afaics, it only helps in the unlikely > case, probably in that case the extra lockless while_each_thread() > doesn't hurt. > > IOW, how about > > --- a/fs/proc/base.c > +++ b/fs/proc/base.c > @@ -3071,11 +3071,6 @@ static struct task_struct *first_tid(str > goto found; > } > > - /* If nr exceeds the number of threads there is nothing todo */ > - pos = NULL; > - if (nr && nr >= get_nr_threads(leader)) > - goto out; > - > /* If we haven't found our starting place yet start > * with the leader and walk nr threads forward. > */ > > ? > > Not that I think it is terribly important to kill this counter, and > probably signal->nr_threads can make sense anyway, so far I am just > curious.
I think that was just a sanity check since it was easy. I want to say it prevents a DOS attack with user space passing unreasonably large file position but that DOS attack is handled by ensuring we don't walk through the list if threads more than once.
However: proc_task_getattr uses get_nr_threads to get it's nlink count correct.
Not walking the thread list to get the number of threads seems like an important cpu time saving measure.
Eric
| |