Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1310463 > unrolled thread

Re: linux-next: build failure after merge of the akpm tree

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: linux-next: build failure after merge of the akpm tree Andrew Morton <akpm@linux-foundation.org> - 2016-01-15 22:10 +0100
    Re: linux-next: build failure after merge of the akpm tree Stephen Rothwell <sfr@canb.auug.org.au> - 2016-01-16 00:10 +0100
      Re: linux-next: build failure after merge of the akpm tree Andrew Morton <akpm@linux-foundation.org> - 2016-01-16 00:20 +0100
        Re: linux-next: build failure after merge of the akpm tree Takashi Iwai <tiwai@suse.de> - 2016-01-16 10:00 +0100
          Re: linux-next: build failure after merge of the akpm tree Takashi Iwai <tiwai@suse.de> - 2016-01-20 15:20 +0100
            Re: linux-next: build failure after merge of the akpm tree Stephen Rothwell <sfr@canb.auug.org.au> - 2016-01-20 21:40 +0100
              Re: linux-next: build failure after merge of the akpm tree Stephen Rothwell <sfr@canb.auug.org.au> - 2016-01-22 01:30 +0100
                Re: linux-next: build failure after merge of the akpm tree Stephen Rothwell <sfr@canb.auug.org.au> - 2016-01-22 03:50 +0100
                  Re: linux-next: build failure after merge of the akpm tree Takashi Iwai <tiwai@suse.de> - 2016-01-25 10:40 +0100
                    Re: linux-next: build failure after merge of the akpm tree Michael Ellerman <mpe@ellerman.id.au> - 2016-01-25 10:50 +0100
                      Re: linux-next: build failure after merge of the akpm tree Takashi Iwai <tiwai@suse.de> - 2016-01-25 11:20 +0100
                    Re: linux-next: build failure after merge of the akpm tree Vinod Koul <vinod.koul@intel.com> - 2016-01-25 11:40 +0100
                      Re: linux-next: build failure after merge of the akpm tree Takashi Iwai <tiwai@suse.de> - 2016-01-25 11:50 +0100
                    Re: linux-next: build failure after merge of the akpm tree Sudip Mukherjee <sudipm.mukherjee@gmail.com> - 2016-01-25 11:50 +0100
                      Re: linux-next: build failure after merge of the akpm tree Takashi Iwai <tiwai@suse.de> - 2016-01-25 12:10 +0100
                        Re: linux-next: build failure after merge of the akpm tree Sudip Mukherjee <sudipm.mukherjee@gmail.com> - 2016-01-25 12:20 +0100
                    Re: linux-next: build failure after merge of the akpm tree Mark Brown <broonie@kernel.org> - 2016-01-25 12:50 +0100
                      Re: linux-next: build failure after merge of the akpm tree Takashi Iwai <tiwai@suse.de> - 2016-01-25 14:20 +0100
                        Re: linux-next: build failure after merge of the akpm tree Takashi Iwai <tiwai@suse.de> - 2016-01-25 14:50 +0100
                          Re: linux-next: build failure after merge of the akpm tree Mark Brown <broonie@kernel.org> - 2016-01-25 15:00 +0100
                            Re: linux-next: build failure after merge of the akpm tree Vinod Koul <vinod.koul@intel.com> - 2016-01-26 13:50 +0100
                          Re: linux-next: build failure after merge of the akpm tree Sudip Mukherjee <sudipm.mukherjee@gmail.com> - 2016-01-25 15:10 +0100
                          Re: linux-next: build failure after merge of the akpm tree Vinod Koul <vinod.koul@intel.com> - 2016-01-26 13:30 +0100
                          Re: linux-next: build failure after merge of the akpm tree Stephen Rothwell <sfr@canb.auug.org.au> - 2016-01-26 22:00 +0100
                            Re: linux-next: build failure after merge of the akpm tree Takashi Iwai <tiwai@suse.de> - 2016-01-26 22:30 +0100
                Re: linux-next: build failure after merge of the akpm tree Stephen Rothwell <sfr@canb.auug.org.au> - 2016-01-22 04:10 +0100

Page 2 of 2 — ← Prev page 1 [2]

#1317842

[Multipart message — attachments visible in raw view] — view raw

On Mon, Jan 25, 2016 at 01:55:46PM +0000, Mark Brown wrote:
> 
> though we will need to come up with an alternative solution for the
> affected architectures if anyone ends up caring (MIPS or PowerPC might
> possibly).

Okay I did some rethink on this. So we should be able to make
MAX_NUM_CODEC_DESCRIPTORS to 16. But that will be ABI change :(

Also further down the road if people start hitting this limit add
additional ioctl to get more descriptors, but for Intel devices this is
okay, WM doesn't seem to implement this..

Thanks
-- 
~Vinod

[toc] | [prev] | [next] | [standalone]

#1316727

On Mon, Jan 25, 2016 at 02:45:55PM +0100, Takashi Iwai wrote:
> On Mon, 25 Jan 2016 14:10:37 +0100,
> Takashi Iwai wrote:
> > 
> > On Mon, 25 Jan 2016 12:41:50 +0100,
> > Mark Brown wrote:
> > > 
> > 
> > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> > ---
> >  sound/core/compress_offload.c | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> > 
> > diff --git a/sound/core/compress_offload.c b/sound/core/compress_offload.c
> > index 18b8dc45bb8f..36b0083fd9f4 100644
> > --- a/sound/core/compress_offload.c
> > +++ b/sound/core/compress_offload.c
> > @@ -46,6 +46,13 @@
> >  #include <sound/compress_offload.h>
> >  #include <sound/compress_driver.h>
> >  
> > +/* FIXME: struct snd_compr_codec_caps overflows the ioctl bit size for some
> > + *        architectures, so we need to disable the relevant ioctls.
> > + */
> > +#if _IOC_SIZEBITS < 13
> 
> Argh, a typo here.  I forgot to refresh the patch, sorry.
> 
> Below is the revised one.

powerpc allmodconfig builds properly now.

Acked-by: Sudip Mukherjee <sudipm.mukherjee@gmail.com>

regards
sudip

[toc] | [prev] | [next] | [standalone]

#1317830

On Mon, Jan 25, 2016 at 02:45:55PM +0100, Takashi Iwai wrote:
> Below is the revised one.

Acked-by: Vinod Koul <vinod.koul@intel.com>

Thanks for fixing this up...

-- 
~Vinod

[toc] | [prev] | [next] | [standalone]

#1318364

Hi Takashi,

On Mon, 25 Jan 2016 14:45:55 +0100 Takashi Iwai <tiwai@suse.de> wrote:
>
> From: Takashi Iwai <tiwai@suse.de>
> Subject: [PATCH v2] ALSA: compress: Disable GET_CODEC_CAPS ioctl for some
>  architectures
> 
> Some architectures like PowerPC can handle the maximum struct size in
> an ioctl only up to 13 bits, and struct snd_compr_codec_caps used by
> SNDRV_COMPRESS_GET_CODEC_CAPS ioctl overflows this limit.  This
> problem was revealed recently by a powerpc change, as it's now treated
> as a fatal build error.
> 
> This patch is a workaround for that: for architectures with less than
> 14 bit ioctl struct size, get rid of the handling of the relevant
> ioctl.  We should provide an alternative equivalent ioctl code later,
> but for now just paper over it.  Luckily, the compress API hasn't been
> used on such architectures, so the impact must be effectively zero.
> 
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
>  sound/core/compress_offload.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/sound/core/compress_offload.c b/sound/core/compress_offload.c
> index 18b8dc45bb8f..0609e618107a 100644
> --- a/sound/core/compress_offload.c
> +++ b/sound/core/compress_offload.c
> @@ -46,6 +46,13 @@
>  #include <sound/compress_offload.h>
>  #include <sound/compress_driver.h>
>  
> +/* FIXME: struct snd_compr_codec_caps overflows the ioctl bit size for some
> + *        architectures, so we need to disable the relevant ioctls.
> + */
> +#if _IOC_SIZEBITS < 14
> +#define COMPR_CODEC_CAPS_OVERFLOW
> +#endif
> +
>  /* TODO:
>   * - add substream support for multiple devices in case of
>   *	SND_DYNAMIC_MINORS is not used
> @@ -440,6 +447,7 @@ out:
>  	return retval;
>  }
>  
> +#ifndef COMPR_CODEC_CAPS_OVERFLOW
>  static int
>  snd_compr_get_codec_caps(struct snd_compr_stream *stream, unsigned long arg)
>  {
> @@ -463,6 +471,7 @@ out:
>  	kfree(caps);
>  	return retval;
>  }
> +#endif /* !COMPR_CODEC_CAPS_OVERFLOW */
>  
>  /* revisit this with snd_pcm_preallocate_xxx */
>  static int snd_compr_allocate_buffer(struct snd_compr_stream *stream,
> @@ -801,9 +810,11 @@ static long snd_compr_ioctl(struct file *f, unsigned int cmd, unsigned long arg)
>  	case _IOC_NR(SNDRV_COMPRESS_GET_CAPS):
>  		retval = snd_compr_get_caps(stream, arg);
>  		break;
> +#ifndef COMPR_CODEC_CAPS_OVERFLOW
>  	case _IOC_NR(SNDRV_COMPRESS_GET_CODEC_CAPS):
>  		retval = snd_compr_get_codec_caps(stream, arg);
>  		break;
> +#endif
>  	case _IOC_NR(SNDRV_COMPRESS_SET_PARAMS):
>  		retval = snd_compr_set_params(stream, arg);
>  		break;
> -- 
> 2.7.0

I have replaced my other patch in my fixes tree until someone gets this
patch to Linus.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

[toc] | [prev] | [next] | [standalone]

#1318408

On Tue, 26 Jan 2016 21:51:30 +0100,
Stephen Rothwell wrote:
> 
> Hi Takashi,
> 
> On Mon, 25 Jan 2016 14:45:55 +0100 Takashi Iwai <tiwai@suse.de> wrote:
> >
> > From: Takashi Iwai <tiwai@suse.de>
> > Subject: [PATCH v2] ALSA: compress: Disable GET_CODEC_CAPS ioctl for some
> >  architectures
> > 
> > Some architectures like PowerPC can handle the maximum struct size in
> > an ioctl only up to 13 bits, and struct snd_compr_codec_caps used by
> > SNDRV_COMPRESS_GET_CODEC_CAPS ioctl overflows this limit.  This
> > problem was revealed recently by a powerpc change, as it's now treated
> > as a fatal build error.
> > 
> > This patch is a workaround for that: for architectures with less than
> > 14 bit ioctl struct size, get rid of the handling of the relevant
> > ioctl.  We should provide an alternative equivalent ioctl code later,
> > but for now just paper over it.  Luckily, the compress API hasn't been
> > used on such architectures, so the impact must be effectively zero.
> > 
> > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> > ---
> >  sound/core/compress_offload.c | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> > 
> > diff --git a/sound/core/compress_offload.c b/sound/core/compress_offload.c
> > index 18b8dc45bb8f..0609e618107a 100644
> > --- a/sound/core/compress_offload.c
> > +++ b/sound/core/compress_offload.c
> > @@ -46,6 +46,13 @@
> >  #include <sound/compress_offload.h>
> >  #include <sound/compress_driver.h>
> >  
> > +/* FIXME: struct snd_compr_codec_caps overflows the ioctl bit size for some
> > + *        architectures, so we need to disable the relevant ioctls.
> > + */
> > +#if _IOC_SIZEBITS < 14
> > +#define COMPR_CODEC_CAPS_OVERFLOW
> > +#endif
> > +
> >  /* TODO:
> >   * - add substream support for multiple devices in case of
> >   *	SND_DYNAMIC_MINORS is not used
> > @@ -440,6 +447,7 @@ out:
> >  	return retval;
> >  }
> >  
> > +#ifndef COMPR_CODEC_CAPS_OVERFLOW
> >  static int
> >  snd_compr_get_codec_caps(struct snd_compr_stream *stream, unsigned long arg)
> >  {
> > @@ -463,6 +471,7 @@ out:
> >  	kfree(caps);
> >  	return retval;
> >  }
> > +#endif /* !COMPR_CODEC_CAPS_OVERFLOW */
> >  
> >  /* revisit this with snd_pcm_preallocate_xxx */
> >  static int snd_compr_allocate_buffer(struct snd_compr_stream *stream,
> > @@ -801,9 +810,11 @@ static long snd_compr_ioctl(struct file *f, unsigned int cmd, unsigned long arg)
> >  	case _IOC_NR(SNDRV_COMPRESS_GET_CAPS):
> >  		retval = snd_compr_get_caps(stream, arg);
> >  		break;
> > +#ifndef COMPR_CODEC_CAPS_OVERFLOW
> >  	case _IOC_NR(SNDRV_COMPRESS_GET_CODEC_CAPS):
> >  		retval = snd_compr_get_codec_caps(stream, arg);
> >  		break;
> > +#endif
> >  	case _IOC_NR(SNDRV_COMPRESS_SET_PARAMS):
> >  		retval = snd_compr_set_params(stream, arg);
> >  		break;
> > -- 
> > 2.7.0
> 
> I have replaced my other patch in my fixes tree until someone gets this
> patch to Linus.

I've merged this to for-linus branch, so it'll be included in the next
pull request.


thanks,

Takashi

[toc] | [prev] | [next] | [standalone]

#1314723

Hi all,

On Fri, 22 Jan 2016 11:24:42 +1100 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> On Thu, 21 Jan 2016 07:38:59 +1100 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> >
> > On Wed, 20 Jan 2016 15:09:47 +0100 Takashi Iwai <tiwai@suse.de> wrote:  
> > >
> > > On Sat, 16 Jan 2016 09:51:29 +0100,
> > > Takashi Iwai wrote:    
> > > > 
> > > > There are a few ways to fix this, but all are not comfortable.
> > > > 
> > > > A. Disable compress API for powerpc.    
> > 
> > This also affects alpha, mips and (maybe) sparc.  
> 
> This was exposed on PowerPC by commit bf76f73c5f65 ("powerpc: enable
> UBSAN support") which is in Linus' tree as of this morning.  The only
> relevant change that made was in the compiler flags (I tested this by
> building the file without that commit but with these new compiler flags:
> 
> -fsanitize=shift -fsanitize=integer-divide-by-zero
> -fsanitize=unreachable -fsanitize=vla-bound -fsanitize=null
> -fsanitize=signed-integer-overflow -fsanitize=bounds
> -fsanitize=object-size -fsanitize=returns-nonnull-attribute
> -fsanitize=bool -fsanitize=enum -fsanitize=alignment
> 
> The preprocessed file is the same in both cases, but with these flags
> the compiler errors.

I have discussed this with the PowerPC maintainer (Michael) and he
figured out why the compiler does not produce an error (normally).  It
is because this driver is using _IOC_NR(xxx) to match ioctls instead of
the full ioctl number.  Because of that, the compiler can figure out
that it does not care about the undefined reference to
__invalid_size_argument_for_IOC that the size check shouold generate
(since _IOC_NR shifts and masks it out).

So, the switch statement in snd_compr_ioctl() should be rewritten to
check against the full ioctl number (since currently it could
theoretically match any number of ioctls, not just the relevant ones).
And then something needs to be done about the very large structure
being passed.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

[toc] | [prev] | [standalone]

Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel

csiph-web