At Tue, 27 Aug 2013 15:14:15 +0530,
Vinod Koul wrote:
>
> On Tue, Aug 27, 2013 at 12:22:31PM +0200, Takashi Iwai wrote:
> > At Tue, 27 Aug 2013 12:10:31 +0530,
> > Vinod Koul wrote:
> > >
> > > Some simple ioctls like timsetamp query, capabities query can be done anytime
> > > and should not be under the stream lock. Move these to
> > > snd_compress_simple_iotcls() which is invoked without lock held
> > >
> > > While at it, improve readblity a bit by sprinkling some empty lines
> > >
> > > Signed-off-by: Vinod Koul <vinod.koul@xxxxxxxxx>
> > > Cc: <stable@xxxxxxxxxxxxxxx>
> >
> > Why it's needed for stable? Fixing any real bugs?
> yup, users are complaining that while streams are draining they can't read
> timstamps. Also one case where a user hit pause didnt go thru as lock preveted
> the pause to be executed..
Then write the problems more clearly. I saw no such information in
the changelog.
> Since 3.10 is next LTS kernel, I forsee lots of folks using taht for a while so
> makes sense to fix there as well
Depends on the fix and the problem. The fact that this can't be
tested only with 3.10 kernel (no real driver exists), I doubt it's
worth. Stable kernels aren't for out-of-tree drivers.
And, if you really target to the stable tree, don't put any
unnecessary changes like white space fixes. Read
stable_kernel_rules.txt once more.
thanks,
Takashi
> > > + default:
> > > + mutex_unlock(&stream->device->lock);
> > > + return snd_compress_simple_ioctls(f, stream, cmd, arg);
> >
> > In this code, it still locks/unlocks shortly unnecessarily.
> > It should be rather like:
> > switch (_IOC_NR(cmd)) {
> > case _IOC_NR(SNDRV_COMPRESS_IOCTL_VERSION):
> > ...
> > case _IOC_NR(SNDRV_COMPRESS_GET_CAPS):
> > ....
> > default:
> > retval = snd_compress_locked_ioctls(f, stream, cmd, arg);
> > }
> Hmmm, okay no point in blocking. I will reverse the flow
>
> ~Vinod
>
> --
>
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html