On Friday, July 05, 2013 8:30 AM, Ian Abbott wrote:
> `comedi_alloc_spriv()` allocates private storage for a comedi subdevice
> and sets the `SRF_FREE_SPRIV` flag in the `runflags` member of the
> subdevice to allow the private storage to be automatically freed when
> the comedi device is being cleaned up. Unfortunately, the flag gets
> clobbered by `do_cmd_ioctl()` which calls
> `comedi_set_subdevice_runflags()` with a mask value `~0` and only the
> `SRF_USER` and `SRF_RUNNING` flags set, all the other SRF flags being
> cleared.
>
> Change the calls to `comedi_set_subdevice_runflags()` that currently use
> a mask value of `~0` to use a more relevant mask value. For
> `do_cmd_ioctl()`, the relevant SRF flags are `SRF_USER`, `SRF_ERROR` and
> `SRF_RUNNING`. (At one time, `SRF_RT` would be included in that set of
> flags, but it is no longer used.)
>
> Signed-off-by: Ian Abbott <abbotti@xxxxxxxxx>
> ---
> Currently, this change is only needed for 3.11-rc.
> ---
> drivers/staging/comedi/comedi_fops.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/comedi/comedi_fops.c b/drivers/staging/comedi/comedi_fops.c
> index 0794aac..ade8964 100644
> --- a/drivers/staging/comedi/comedi_fops.c
> +++ b/drivers/staging/comedi/comedi_fops.c
> @@ -543,7 +543,8 @@ void *comedi_alloc_spriv(struct comedi_subdevice *s, size_t size)
> {
> s->private = kzalloc(size, GFP_KERNEL);
> if (s->private)
> - comedi_set_subdevice_runflags(s, ~0, SRF_FREE_SPRIV);
> + comedi_set_subdevice_runflags(s, SRF_FREE_SPRIV,
> + SRF_FREE_SPRIV);
As we have discussed previously, this function is only called during a board
(*attach). During the attach the spinlock on the runflags should not be needed
so this could be simplified to just:
s->runflags |= SRF_FREE_SPRIV;
> return s->private;
> }
> EXPORT_SYMBOL_GPL(comedi_alloc_spriv);
> @@ -1489,7 +1490,8 @@ static int do_cmd_ioctl(struct comedi_device *dev,
> if (async->cmd.flags & TRIG_WAKE_EOS)
> async->cb_mask |= COMEDI_CB_EOS;
>
> - comedi_set_subdevice_runflags(s, ~0, SRF_USER | SRF_RUNNING);
> + comedi_set_subdevice_runflags(s, SRF_USER | SRF_ERROR | SRF_RUNNING,
> + SRF_USER | SRF_RUNNING);
This one makes sense. But to avoid any future similar problems, maybe
comedi_set_subdevice_runflags() should just mask the 'mask' so that
the SRF_FREE_SPRIV flag is never cleared. This flag is a "special" runflag
in that it is not really part of the subdevice running state.
> ret = s->do_cmd(dev, s);
> if (ret == 0)
Regards,
Hartley
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
