Re: [RFC] Simple userspace interface for PCI drivers

[Thomas Gleixner <tglx@linutronix.de>]

On Thu, 2006-08-31 at 00:40 -0700, Andrew Morton wrote:
> On Tue, 29 Aug 2006 23:23:38 -0700
> Greg KH <greg@kroah.com> wrote:
> 
> > +static ssize_t store_sig_pid(struct device *dev, struct device_attribute *attr,
> > +			     const char *buf, size_t count)
> > +{
> > +	iio_dummy_signal.pid = simple_strtol(buf, NULL, 10);
> > +	if (iio_dummy_signal.pid == 0) {
> > +		if (iio_dummy_signal.it_process) {
> > +			put_task_struct(iio_dummy_signal.it_process);
> > +			iio_dummy_signal.it_process = NULL;
> > +		}
> > +
> > +		iio_dummy_signal.pid = 0;
> > +		return count;
> > +	}
> > +
> > +	if (iio_dummy_signal.pid == 1)
> > +		goto out;
> > +
> > +	iio_dummy_signal.it_process = find_task_by_pid(iio_dummy_signal.pid);
> > +	if (iio_dummy_signal.it_process) {
> > +		get_task_struct(iio_dummy_signal.it_process);
> > +		iio_dummy_signal.it_sigev_notify = SIGEV_SIGNAL;
> > +		iio_dummy_signal.it_sigev_signo = SIGALRM;
> > +		iio_dummy_signal.it_sigev_value.sival_int = 0;
> > +
> > +		return count;
> > +	}
> > +out:
> > +	iio_dummy_signal.pid = 0;
> > +	return -EINVAL;
> > +}
> 
> This is racy: find_task_by_pid() needs tasklist_lock or rcu_read_lock().
> 
> It doesn't work as a module due to missing __put_task_struct.
> 
> 
> It is also rather nasty.  Why go shoving some random pid into a sysfs file,
> then hang onto a ref on a task_struct for some process which exitted last
> week?  It would be cleaner and more idiomatic to require that the
> controlling process hold an fd open against the instance so that resources
> can be managed correctly.  Maybe use SIGIO too, so the driver doesn't need
> to know about pids and task_structs and things.  Which all maps better onto
> ioctls than sysfs (ties self to stake)
> 
> <looks>
> 
> iio_dev.c seems to already be doing this.  Why does iio_dummy.c exist?

Uurg. That's an old dummy/test driver which never got updated.

	tglx