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@intel.com Cc: stable@vger.kernel.org
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
--