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?
sound/core/compress_offload.c | 79 ++++++++++++++++++++++++++++++----------- 1 files changed, 58 insertions(+), 21 deletions(-)
diff --git a/sound/core/compress_offload.c b/sound/core/compress_offload.c index 99db892..868aefd 100644 --- a/sound/core/compress_offload.c +++ b/sound/core/compress_offload.c @@ -729,70 +729,107 @@ static int snd_compr_partial_drain(struct snd_compr_stream *stream) return retval; }
-static long snd_compr_ioctl(struct file *f, unsigned int cmd, unsigned long arg) +static int snd_compress_simple_ioctls(struct file *file,
struct snd_compr_stream *stream,unsigned int cmd, unsigned long arg){
struct snd_compr_file *data = f->private_data;
struct snd_compr_stream *stream; int retval = -ENOTTY;
if (snd_BUG_ON(!data))
return -EFAULT;stream = &data->stream;
if (snd_BUG_ON(!stream))
return -EFAULT;mutex_lock(&stream->device->lock); switch (_IOC_NR(cmd)) { case _IOC_NR(SNDRV_COMPRESS_IOCTL_VERSION): put_user(SNDRV_COMPRESS_VERSION, (int __user *)arg) ? -EFAULT : 0; break;
- case _IOC_NR(SNDRV_COMPRESS_GET_CAPS): retval = snd_compr_get_caps(stream, arg); break;
- case _IOC_NR(SNDRV_COMPRESS_GET_CODEC_CAPS): retval = snd_compr_get_codec_caps(stream, arg); break;
- case _IOC_NR(SNDRV_COMPRESS_TSTAMP):
retval = snd_compr_tstamp(stream, arg);break;- case _IOC_NR(SNDRV_COMPRESS_AVAIL):
retval = snd_compr_ioctl_avail(stream, arg);break;/* drain and partial drain need special handling* we need to drop the locks here as the streams would get blocked on the* dsp to get drained. The locking would be handled in respective* function here*/- case _IOC_NR(SNDRV_COMPRESS_DRAIN):
retval = snd_compr_drain(stream);break;- case _IOC_NR(SNDRV_COMPRESS_PARTIAL_DRAIN):
retval = snd_compr_partial_drain(stream);break;- }
- return retval;
+}
+static long snd_compr_ioctl(struct file *f, unsigned int cmd, unsigned long arg) +{
- struct snd_compr_file *data = f->private_data;
- struct snd_compr_stream *stream;
- int retval = -ENOTTY;
- if (snd_BUG_ON(!data))
return -EFAULT;- stream = &data->stream;
- if (snd_BUG_ON(!stream))
return -EFAULT;- mutex_lock(&stream->device->lock);
- switch (_IOC_NR(cmd)) { case _IOC_NR(SNDRV_COMPRESS_SET_PARAMS): retval = snd_compr_set_params(stream, arg); break;
- case _IOC_NR(SNDRV_COMPRESS_GET_PARAMS): retval = snd_compr_get_params(stream, arg); break;
- case _IOC_NR(SNDRV_COMPRESS_SET_METADATA): retval = snd_compr_set_metadata(stream, arg); break;
- case _IOC_NR(SNDRV_COMPRESS_GET_METADATA): retval = snd_compr_get_metadata(stream, arg); break;
- case _IOC_NR(SNDRV_COMPRESS_TSTAMP):
retval = snd_compr_tstamp(stream, arg);break;- case _IOC_NR(SNDRV_COMPRESS_AVAIL):
retval = snd_compr_ioctl_avail(stream, arg);break;
- case _IOC_NR(SNDRV_COMPRESS_PAUSE): retval = snd_compr_pause(stream); break;
- case _IOC_NR(SNDRV_COMPRESS_RESUME): retval = snd_compr_resume(stream); break;
- case _IOC_NR(SNDRV_COMPRESS_START): retval = snd_compr_start(stream); break;
- case _IOC_NR(SNDRV_COMPRESS_STOP): retval = snd_compr_stop(stream); break;
- case _IOC_NR(SNDRV_COMPRESS_DRAIN):
retval = snd_compr_drain(stream);break;- case _IOC_NR(SNDRV_COMPRESS_PARTIAL_DRAIN):
retval = snd_compr_partial_drain(stream);break;
case _IOC_NR(SNDRV_COMPRESS_NEXT_TRACK): retval = snd_compr_next_track(stream); break;
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); }
Takashi