Re: [PATCH 00/18] prevent bounds-check bypass via speculative execution

2018-01-08 Thread Ingo Molnar

* Alan Cox  wrote:

> On Mon, 8 Jan 2018 11:08:36 +0100
> Peter Zijlstra  wrote:
> 
> > On Fri, Jan 05, 2018 at 10:30:16PM -0800, Dan Williams wrote:
> > > On Fri, Jan 5, 2018 at 6:22 PM, Eric W. Biederman  
> > > wrote:  
> > > > In at least one place (mpls) you are patching a fast path.  Compile out
> > > > or don't load mpls by all means.  But it is not acceptable to change the
> > > > fast path without even considering performance.  
> > > 
> > > Performance matters greatly, but I need help to identify a workload
> > > that is representative for this fast path to see what, if any, impact
> > > is incurred. Even better is a review that says "nope, 'index' is not
> > > subject to arbitrary userspace control at this point, drop the patch."  
> > 
> > I think we're focussing a little too much on pure userspace. That is, we
> > should be saying under the attackers control. Inbound network packets
> > could equally be under the attackers control.
> 
> Inbound network packets don't come with a facility to read back and do
> cache timimg. [...]

But the reply packets can be measured on the sending side, and the total delay 
timing would thus carry the timing information.

Yes, a lot of noise gets added that way if we think 'packet goes through the 
Internet' - but with gigabit local network access or even through localhost
access a lot of noise can be removed as well.

It's not as dangerous as a near instantaneous local attack, but 'needs a day of 
runtime to brute-force through localhost or 10GigE' is still worrying in many 
real-world security contexts.

So I concur with Peter that we should generally consider making all of our 
responses to external data (maybe with the exception of pigeon post messages) 
Spectre-safe.

Thanks,

Ingo


Re: dvb usb issues since kernel 4.9

2018-01-08 Thread Ingo Molnar

* Linus Torvalds <torva...@linux-foundation.org> wrote:

> On Sat, Jan 6, 2018 at 11:54 AM, Mauro Carvalho Chehab
> <mche...@s-opensource.com> wrote:
> >
> > Em Sat, 6 Jan 2018 16:04:16 +0100
> > "Josef Griebichler" <griebichler.jo...@gmx.at> escreveu:
> >>
> >> the causing commit has been identified.
> >> After reverting commit 
> >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=4cd13c21b207e80ddb1144c576500098f2d5f882
> >> its working again.
> >
> > Just replying to me won't magically fix this. The ones that were involved on
> > this patch should also be c/c, plus USB people. Just added them.
> 
> Actually, you seem to have added an odd subset of the people involved.
> 
> For example, Ingo - who actually committed that patch - wasn't on the cc.
> 
> I do think we need to simply revert that patch. It's very simple: it
> has been reported to lead to actual problems for people, and we don't
> fix one problem and then say "well, it fixed something else" when
> something breaks.
> 
> When something breaks, we either unbreak it, or we revert the change
> that caused the breakage.
> 
> It's really that simple. That's what "no regressions" means.  We don't
> accept changes that cause regressions. This one did.

Yeah, absolutely - for the revert:

   Acked-by: Ingo Molnar <mi...@kernel.org>

as I doubt we have enough time to root-case this properly.

Thanks,

Ingo


Re: [WARNING: A/V UNSCANNABLE][Merge tag 'media/v4.11-1' of git] ff58d005cd: BUG: unable to handle kernel NULL pointer dereference at 0000039c

2017-02-27 Thread Ingo Molnar

* Linus Torvalds  wrote:

> In other words: what will happen is that distros start getting bootup problem 
> reports six months or a year after we've done it, and *if* they figure out 
> it's 
> the irq enabling, they'll disable it, because they have no way to solve it 
> either.
> 
> And core developers will just maybe see the occasional "4.12 doesn't boot for 
> me" reports, but by then developers will ahve moved on to 4.16 or something.

Yeah, you are right, there's over 2,100 request_irq() calls in the kernel and 
perhaps only 1% of them gets tested on real hardware by the time a change gets 
upstream :-/

So in theory we could require all *new* drivers handle this properly, as new 
drivers tend to come through developers who can fix such bugs - which would at 
least guarantee that with time the problem would obsolete itself.

But I cannot see an easy non-intrusive way to do that - we'd have to rename all 
existing request_irq() calls:

 - We could rename request_irq() to request_irq_legacy() - which does not do 
the 
   tests.

 - The 'new' request_irq() function would do the tests unconditionally.

... and that's just too much churn - unless you think it's worth it, or if 
anyone 
can think of a better method to phase in the new behavior without affecting old 
users.

Another, less intrusive method would be to introduce a new request_irq_shared() 
API call, mark request_irq() obsolete (without putting warnings into the build 
though), and put a check into checkpatch that warns about request_irq() use.

The flip side would be that:

 - request_irq() is such a nice and well-known name to waste

 - plus request_irq_shared() is a misnomer, as this has nothing to do with 
sharing 
   IRQs, it's about getting IRQs in unexpected moments.

I'd rather do the renaming that is easy to automate and the pain is one time 
only.

Or revert the retrigger change and muddle through, although as per Thomas's 
observations spurious interrupts are very common.

Thanks,

Ingo


Re: [WARNING: A/V UNSCANNABLE][Merge tag 'media/v4.11-1' of git] ff58d005cd: BUG: unable to handle kernel NULL pointer dereference at 0000039c

2017-02-27 Thread Ingo Molnar

* Thomas Gleixner  wrote:

> The pending interrupt issue happens, at least on my test boxen, mostly on
> the 'legacy' interrupts (0 - 15). But even the IOAPIC interrupts >=16
> happen occasionally.
>
> 
>  - Spurious interrupts on IRQ7, which are triggered by IRQ 0 (PIT/HPET). On
>one of the affected machines this stops when the interrupt system is
>switched to interrupt remapping !?!?!?
> 
>  - Spurious interrupts on various interrupt lines, which are triggered by
>IOAPIC interrupts >= IRQ16. That's a known issue on quite some chipsets
>that the legacy PCI interrupt (which is used when IOAPIC is disabled) is
>triggered when the IOAPIC >=16 interrupt fires.
> 
>  - Spurious interrupt caused by driver probing itself. I.e. the driver
>probing code causes an interrupt issued from the device
>inadvertently. That happens even on IRQ >= 16.
> 
>This problem might be handled by the device driver code itself, but
>that's going to be ugly. See below.

That's pretty colorful behavior...

> We can try to sample more data from the machines of affected users, but I 
> doubt 
> that it will give us more information than confirming that we really have to 
> deal with all that hardware wreckage out there in some way or the other.

BTW., instead of trying to avoid the scenario, wow about moving in the other 
direction: making CONFIG_DEBUG_SHIRQ=y unconditional property in the IRQ core 
code 
starting from v4.12 or so, i.e. requiring device driver IRQ handlers to handle 
the 
invocation of IRQ handlers at pretty much any moment. (We could also extend it 
a 
bit, such as invoking IRQ handlers early after suspend/resume wakeup.)

Because it's not the requirement that hurts primarily, but the resulting 
non-determinism and the sporadic crashes. Which can be solved by making the 
race 
deterministic via the debug facility.

If the IRQ handler crashed the moment it was first written by the driver author 
we'd never see these problems.

Thanks,

Ingo


Re: [WARNING: A/V UNSCANNABLE][Merge tag 'media/v4.11-1' of git] ff58d005cd: BUG: unable to handle kernel NULL pointer dereference at 0000039c

2017-02-25 Thread Ingo Molnar

* Linus Torvalds <torva...@linux-foundation.org> wrote:

> I'm pretty sure that the thing that triggered this is once more commit
> a9b4f08770b4 ("x86/ioapic: Restore IO-APIC irq_chip retrigger
> callback") which seems to retrigger stale irqs that simply should not
> be retriggered.
> 
> They aren't actually active any more, if they ever were.
> 
> So that commit seems to act like a random CONFIG_DEBUG_SHIRQ. It's
> good for testing, but not good for actual users.

Yeah, so some distros like Fedora already have CONFIG_DEBUG_SHIRQ=y enabled, 
but 
part of the problem is that CONFIG_DEBUG_SHIRQ=y has this:

#ifdef CONFIG_DEBUG_SHIRQ_FIXME
if (!retval && (irqflags & IRQF_SHARED)) {
/*
 * It's a shared IRQ -- the driver ought to be prepared for it
 * to happen immediately, so let's make sure
 * We disable the irq to make sure that a 'real' IRQ doesn't
 * run in parallel with our fake.
 */
unsigned long flags;

disable_irq(irq);
local_irq_save(flags);

handler(irq, dev_id);

local_irq_restore(flags);
enable_irq(irq);
}
#endif

Note that the '_FIXME' postfix effectively turns off this particular debug 
check 
...

Thomas and me realized this risk a week ago ago, and tried to resurrect full 
CONFIG_DEBUG_SHIRQ=y functionality to more reliably trigger these problems:

  https://lkml.org/lkml/2017/2/15/341

... but were forced to revert that fix because it's not working on x86 yet 
(it's 
crashing). We also thought we fixed the problems exposed in drivers, as the 
retrigger changes have been in -tip and -next for some time, but were clearly 
too 
optimistic about that.

So, should we revert the hw-retrigger change:

  a9b4f08770b4 x86/ioapic: Restore IO-APIC irq_chip retrigger callback

... until we managed to fix CONFIG_DEBUG_SHIRQ=y? If you'd like to revert it 
upstream straight away:

Acked-by: Ingo Molnar <mi...@kernel.org>

Thanks,

Ingo


Re: [PATCH v2 2/2] x86/mm/pat, drivers/media/ivtv: move pat warn and replace WARN() with pr_warn()

2015-07-07 Thread Ingo Molnar

* Luis R. Rodriguez mcg...@do-not-panic.com wrote:

 On Mon, Jul 6, 2015 at 5:44 PM, Luis R. Rodriguez mcg...@suse.com wrote:
  If we really wanted to we could consider arch_phys_wc_add()
 
 I mean adding a __arch_phys_wc_add() which does not check for pat_enabled().
 
  and
  deal with that this will not check for pat_enabled() and forces MTRR...
  I think Andy Luto won't like that very much though ? I at least don't
  like it since we did all this work to finally leave only 1 piece of
  code with direct MTRR access... Seems a bit sad. Since ipath will
  be removed we'd have only ivtv driver using this API, I am not sure if
  its worth it.
 
 Since ipath is going away soon we'd just have one driver with the icky
 #ifdef code. I'd rather see that and then require semantics / grammer
 rules to require ioremap_wc() when used with arch_phys_wc_add(). I don't 
 think 
 ivtv is worth to consider breaking the semantics and requirements.

Ok, let's keep your original approach with the warning then.

Thanks,

Ingo
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 2/2] x86/mm/pat, drivers/media/ivtv: move pat warn and replace WARN() with pr_warn()

2015-06-29 Thread Ingo Molnar

* Andy Walls a...@silverblocksystems.net wrote:

 On Fri, 2015-06-26 at 10:45 +0200, Ingo Molnar wrote:
  * Luis R. Rodriguez mcg...@suse.com wrote:
  
   On Thu, Jun 25, 2015 at 08:51:47AM +0200, Ingo Molnar wrote:

* Luis R. Rodriguez mcg...@do-not-panic.com wrote:

 From: Luis R. Rodriguez mcg...@suse.com
 
 On built-in kernels this warning will always splat as this is part
 of the module init. Fix that by shifting the PAT requirement check
 out under the code that does the quasi-probe for the device. This
 device driver relies on an existing driver to find its own devices,
 it looks for that device driver and its own found devices, then
 uses driver_for_each_device() to try to see if it can probe each of
 those devices as a frambuffer device with ivtvfb_init_card(). We
 tuck the PAT requiremenet check then on the ivtvfb_init_card()
 call making the check at least require an ivtv device present
 before complaining.
 
 Reported-by: Fengguang Wu fengguang...@intel.com [0-day test robot]
 Signed-off-by: Luis R. Rodriguez mcg...@suse.com
 ---
  drivers/media/pci/ivtv/ivtvfb.c | 15 +--
  1 file changed, 9 insertions(+), 6 deletions(-)
 
 diff --git a/drivers/media/pci/ivtv/ivtvfb.c 
 b/drivers/media/pci/ivtv/ivtvfb.c
 index 4cb365d..8b95eef 100644
 --- a/drivers/media/pci/ivtv/ivtvfb.c
 +++ b/drivers/media/pci/ivtv/ivtvfb.c
 @@ -38,6 +38,8 @@
  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  
 02111-1307  USA
   */
  
 +#define pr_fmt(fmt) KBUILD_MODNAME :  fmt
 +
  #include linux/module.h
  #include linux/kernel.h
  #include linux/fb.h
 @@ -1171,6 +1173,13 @@ static int ivtvfb_init_card(struct ivtv *itv)
  {
   int rc;
  
 +#ifdef CONFIG_X86_64
 + if (pat_enabled()) {
 + pr_warn(ivtvfb needs PAT disabled, boot with nopat 
 kernel parameter\n);
 + return -ENODEV;
 + }
 +#endif
 +
   if (itv-osd_info) {
   IVTVFB_ERR(Card %d already initialised\n, 
 ivtvfb_card_id);
   return -EBUSY;

Same argument as for ipath: why not make arch_phys_wc_add() fail on PAT 
and 
return -1, and check it in arch_phys_wc_del()?
   
   The arch_phys_wc_add() is a no-op for PAT systems but for PAT to work we 
   need 
   not only need to add this in where we replace the MTRR call but we also 
   need to 
   convert ioremap_nocache() calls to ioremap_wc() but only if things were 
   split up 
   already.
  
 
 Hi Ingo,
 
  We don't need to do that: for such legacy drivers we can fall back to UC 
  just 
  fine, and inform the user that by booting with 'nopat' the old behavior 
  will be 
  back...
 
 This is really a user experience decision.
 
 IMO anyone who is still using ivtvfb and an old conventional PCI PVR-350 to 
 render, at SDTV resolution, an X Desktop display or video playback on a 
 television screen, isn't going to give a hoot about modern things like PAT.  
 The 
 user will simply want the framebuffer performance they are accustomed to 
 having 
 with their system.  UC will probably yield unsatisfactory performance for an 
 ivtvfb framebuffer.
 
 With that in mind, I would think it better to obviously and clearly disable 
 the 
 ivtvfb framebuffer module with PAT enabled, so the user will check the log 
 and 
 read the steps needed to obtain acceptable performance.
 
 Maybe that's me just wanting to head off the poor ivtvfb performance with 
 latest kernel bug reports.
 
 Whatever the decision, my stock response to bug reports related to this will 
 always be What do the logs say?.

So what if that frame buffer is their only (working) frame buffer? A slow 
framebuffer is still much better at giving people logs to look at than a 
non-working one.

Thanks,

Ingo
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 1/2] x86/mm/pat, drivers/infiniband/ipath: replace WARN() with pr_warn()

2015-06-26 Thread Ingo Molnar

* Luis R. Rodriguez mcg...@suse.com wrote:

 On Thu, Jun 25, 2015 at 08:49:22AM +0200, Ingo Molnar wrote:
  
  * Luis R. Rodriguez mcg...@do-not-panic.com wrote:
  
   From: Luis R. Rodriguez mcg...@suse.com
   
   WARN() may confuse users, fix that. ipath_init_one() is part the
   device's probe so this would only be triggered if a corresponding
   device was found.
   
   Signed-off-by: Luis R. Rodriguez mcg...@suse.com
   ---
drivers/infiniband/hw/ipath/ipath_driver.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
   
   diff --git a/drivers/infiniband/hw/ipath/ipath_driver.c 
   b/drivers/infiniband/hw/ipath/ipath_driver.c
   index 2d7e503..871dbe5 100644
   --- a/drivers/infiniband/hw/ipath/ipath_driver.c
   +++ b/drivers/infiniband/hw/ipath/ipath_driver.c
   @@ -31,6 +31,8 @@
 * SOFTWARE.
 */

   +#define pr_fmt(fmt) KBUILD_MODNAME :  fmt
   +
#include linux/sched.h
#include linux/spinlock.h
#include linux/idr.h
   @@ -399,8 +401,8 @@ static int ipath_init_one(struct pci_dev *pdev, const 
   struct pci_device_id *ent)
 u32 bar0 = 0, bar1 = 0;

#ifdef CONFIG_X86_64
   - if (WARN(pat_enabled(),
   -  ipath needs PAT disabled, boot with nopat kernel 
   parameter\n)) {
   + if (pat_enabled()) {
   + pr_warn(ipath needs PAT disabled, boot with nopat kernel 
   parameter\n);
 ret = -ENODEV;
 goto bail;
 }
  
  So driver init will always fail with this on modern kernels.
 
 Nope, I double checked this, ipath_init_one() is the PCI probe routine,
 not the module init call. It should probably be renamed.
 
  Btw., on a second thought, ipath uses MTRRs to enable WC:
  
  ret = ipath_enable_wc(dd);
  if (ret)
  ret = 0;
  
  Note how it ignores any failures - the driver still works even if WC was 
  not 
  enabled.
 
 Ah, well WC strategy requires a split of the MMIO registers and the desired
 WC area, right now they are combined for some type of ipath devices. There
 are two things to consider when thinking about whether or not we want to
 do the work required to do the split:

But ... why doing the 'split'?

With my suggested approach the driver will behave in two ways:

  - if booted with 'nopat' it will behave as always and have the WC MTRR 
entries 
added

  - if booted with a modern kernel without 'nopat' then instead of getting WC 
MTRR 
entries it will not get them - we'll fall back to UC. No 'split' or any 
other 
change is needed to the driver AFAICS: it might be slower, but it will 
still 
be functional. It will _not_ get PAT WC mappings - it will fall back to UC 
- 
which is still much better for any potential user than not working at all.

Same suggestion for the other affected driver.

what am I missing?

Thanks,

Ingo
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 2/2] x86/mm/pat, drivers/media/ivtv: move pat warn and replace WARN() with pr_warn()

2015-06-26 Thread Ingo Molnar

* Luis R. Rodriguez mcg...@suse.com wrote:

 On Thu, Jun 25, 2015 at 08:51:47AM +0200, Ingo Molnar wrote:
  
  * Luis R. Rodriguez mcg...@do-not-panic.com wrote:
  
   From: Luis R. Rodriguez mcg...@suse.com
   
   On built-in kernels this warning will always splat as this is part
   of the module init. Fix that by shifting the PAT requirement check
   out under the code that does the quasi-probe for the device. This
   device driver relies on an existing driver to find its own devices,
   it looks for that device driver and its own found devices, then
   uses driver_for_each_device() to try to see if it can probe each of
   those devices as a frambuffer device with ivtvfb_init_card(). We
   tuck the PAT requiremenet check then on the ivtvfb_init_card()
   call making the check at least require an ivtv device present
   before complaining.
   
   Reported-by: Fengguang Wu fengguang...@intel.com [0-day test robot]
   Signed-off-by: Luis R. Rodriguez mcg...@suse.com
   ---
drivers/media/pci/ivtv/ivtvfb.c | 15 +--
1 file changed, 9 insertions(+), 6 deletions(-)
   
   diff --git a/drivers/media/pci/ivtv/ivtvfb.c 
   b/drivers/media/pci/ivtv/ivtvfb.c
   index 4cb365d..8b95eef 100644
   --- a/drivers/media/pci/ivtv/ivtvfb.c
   +++ b/drivers/media/pci/ivtv/ivtvfb.c
   @@ -38,6 +38,8 @@
Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307 
USA
 */

   +#define pr_fmt(fmt) KBUILD_MODNAME :  fmt
   +
#include linux/module.h
#include linux/kernel.h
#include linux/fb.h
   @@ -1171,6 +1173,13 @@ static int ivtvfb_init_card(struct ivtv *itv)
{
 int rc;

   +#ifdef CONFIG_X86_64
   + if (pat_enabled()) {
   + pr_warn(ivtvfb needs PAT disabled, boot with nopat kernel 
   parameter\n);
   + return -ENODEV;
   + }
   +#endif
   +
 if (itv-osd_info) {
 IVTVFB_ERR(Card %d already initialised\n, ivtvfb_card_id);
 return -EBUSY;
  
  Same argument as for ipath: why not make arch_phys_wc_add() fail on PAT and 
  return -1, and check it in arch_phys_wc_del()?
 
 The arch_phys_wc_add() is a no-op for PAT systems but for PAT to work we need 
 not only need to add this in where we replace the MTRR call but we also need 
 to 
 convert ioremap_nocache() calls to ioremap_wc() but only if things were split 
 up 
 already.

We don't need to do that: for such legacy drivers we can fall back to UC just 
fine, and inform the user that by booting with 'nopat' the old behavior will be 
back...

Thanks,

Ingo
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 2/2] x86/mm/pat, drivers/media/ivtv: move pat warn and replace WARN() with pr_warn()

2015-06-25 Thread Ingo Molnar

* Luis R. Rodriguez mcg...@do-not-panic.com wrote:

 From: Luis R. Rodriguez mcg...@suse.com
 
 On built-in kernels this warning will always splat as this is part
 of the module init. Fix that by shifting the PAT requirement check
 out under the code that does the quasi-probe for the device. This
 device driver relies on an existing driver to find its own devices,
 it looks for that device driver and its own found devices, then
 uses driver_for_each_device() to try to see if it can probe each of
 those devices as a frambuffer device with ivtvfb_init_card(). We
 tuck the PAT requiremenet check then on the ivtvfb_init_card()
 call making the check at least require an ivtv device present
 before complaining.
 
 Reported-by: Fengguang Wu fengguang...@intel.com [0-day test robot]
 Signed-off-by: Luis R. Rodriguez mcg...@suse.com
 ---
  drivers/media/pci/ivtv/ivtvfb.c | 15 +--
  1 file changed, 9 insertions(+), 6 deletions(-)
 
 diff --git a/drivers/media/pci/ivtv/ivtvfb.c b/drivers/media/pci/ivtv/ivtvfb.c
 index 4cb365d..8b95eef 100644
 --- a/drivers/media/pci/ivtv/ivtvfb.c
 +++ b/drivers/media/pci/ivtv/ivtvfb.c
 @@ -38,6 +38,8 @@
  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
   */
  
 +#define pr_fmt(fmt) KBUILD_MODNAME :  fmt
 +
  #include linux/module.h
  #include linux/kernel.h
  #include linux/fb.h
 @@ -1171,6 +1173,13 @@ static int ivtvfb_init_card(struct ivtv *itv)
  {
   int rc;
  
 +#ifdef CONFIG_X86_64
 + if (pat_enabled()) {
 + pr_warn(ivtvfb needs PAT disabled, boot with nopat kernel 
 parameter\n);
 + return -ENODEV;
 + }
 +#endif
 +
   if (itv-osd_info) {
   IVTVFB_ERR(Card %d already initialised\n, ivtvfb_card_id);
   return -EBUSY;

Same argument as for ipath: why not make arch_phys_wc_add() fail on PAT and 
return 
-1, and check it in arch_phys_wc_del()?

That way we don't do anything drastic, the remaining few drivers still keep 
working (albeit suboptimally - can be worked around with the 'nopat' boot 
option) 
- yet we've reduced the use of MTRRs drastically.

Thanks,

Ingo
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 1/2] x86/mm/pat, drivers/infiniband/ipath: replace WARN() with pr_warn()

2015-06-25 Thread Ingo Molnar

* Luis R. Rodriguez mcg...@do-not-panic.com wrote:

 From: Luis R. Rodriguez mcg...@suse.com
 
 WARN() may confuse users, fix that. ipath_init_one() is part the
 device's probe so this would only be triggered if a corresponding
 device was found.
 
 Signed-off-by: Luis R. Rodriguez mcg...@suse.com
 ---
  drivers/infiniband/hw/ipath/ipath_driver.c | 6 --
  1 file changed, 4 insertions(+), 2 deletions(-)
 
 diff --git a/drivers/infiniband/hw/ipath/ipath_driver.c 
 b/drivers/infiniband/hw/ipath/ipath_driver.c
 index 2d7e503..871dbe5 100644
 --- a/drivers/infiniband/hw/ipath/ipath_driver.c
 +++ b/drivers/infiniband/hw/ipath/ipath_driver.c
 @@ -31,6 +31,8 @@
   * SOFTWARE.
   */
  
 +#define pr_fmt(fmt) KBUILD_MODNAME :  fmt
 +
  #include linux/sched.h
  #include linux/spinlock.h
  #include linux/idr.h
 @@ -399,8 +401,8 @@ static int ipath_init_one(struct pci_dev *pdev, const 
 struct pci_device_id *ent)
   u32 bar0 = 0, bar1 = 0;
  
  #ifdef CONFIG_X86_64
 - if (WARN(pat_enabled(),
 -  ipath needs PAT disabled, boot with nopat kernel 
 parameter\n)) {
 + if (pat_enabled()) {
 + pr_warn(ipath needs PAT disabled, boot with nopat kernel 
 parameter\n);
   ret = -ENODEV;
   goto bail;
   }

So driver init will always fail with this on modern kernels.

Btw., on a second thought, ipath uses MTRRs to enable WC:

ret = ipath_enable_wc(dd);
if (ret)
ret = 0;

Note how it ignores any failures - the driver still works even if WC was not 
enabled.

So why don't you simply extend ipath_enable_wc() to generate the warning 
message 
and return -EINVAL - which still keeps the driver working on modern kernels?

Just inform the user about 'nopat' if he wants WC for this driver.

Thanks,

Ingo
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] x86/mm/pat, drivers/media/ivtv: replace WARN() with pr_warn()

2015-06-23 Thread Ingo Molnar

* Luis R. Rodriguez mcg...@do-not-panic.com wrote:

 From: Luis R. Rodriguez mcg...@suse.com
 
 On built-in kernels this will always splat. Fix that.
 
 Reported-by: Fengguang Wu fengguang...@intel.com [0-day test robot]
 Signed-off-by: Luis R. Rodriguez mcg...@suse.com
 ---
  drivers/media/pci/ivtv/ivtvfb.c | 6 --
  1 file changed, 4 insertions(+), 2 deletions(-)
 
 diff --git a/drivers/media/pci/ivtv/ivtvfb.c b/drivers/media/pci/ivtv/ivtvfb.c
 index 4cb365d..6f0c364 100644
 --- a/drivers/media/pci/ivtv/ivtvfb.c
 +++ b/drivers/media/pci/ivtv/ivtvfb.c
 @@ -38,6 +38,8 @@
  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
   */
  
 +#define pr_fmt(fmt) KBUILD_MODNAME :  fmt
 +
  #include linux/module.h
  #include linux/kernel.h
  #include linux/fb.h
 @@ -1266,8 +1268,8 @@ static int __init ivtvfb_init(void)
   int err;
  
  #ifdef CONFIG_X86_64
 - if (WARN(pat_enabled(),
 -  ivtvfb needs PAT disabled, boot with nopat kernel 
 parameter\n)) {
 + if (pat_enabled()) {
 + pr_warn(ivtvfb needs PAT disabled, boot with nopat kernel 
 parameter\n);
   return -ENODEV;
   }

So why should a built-in kernel bzImage with this driver enabled but the driver 
not present print this warning?

Why not only print in a code path where we know the hardware is present? 

allyesconfig bootups are noisy enough as-is ...

Thanks,

Ingo
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] x86/mm/pat, drivers/infiniband/ipath: replace WARN() with pr_warn()

2015-06-23 Thread Ingo Molnar

* Luis R. Rodriguez mcg...@do-not-panic.com wrote:

 From: Luis R. Rodriguez mcg...@suse.com
 
 On built-in kernels this will always splat. Fix that.
 
 Reported-by: Fengguang Wu fengguang...@intel.com [0-day test robot]
 Signed-off-by: Luis R. Rodriguez mcg...@suse.com
 ---
  drivers/infiniband/hw/ipath/ipath_driver.c | 6 --
  1 file changed, 4 insertions(+), 2 deletions(-)
 
 diff --git a/drivers/infiniband/hw/ipath/ipath_driver.c 
 b/drivers/infiniband/hw/ipath/ipath_driver.c
 index 2d7e503..871dbe5 100644
 --- a/drivers/infiniband/hw/ipath/ipath_driver.c
 +++ b/drivers/infiniband/hw/ipath/ipath_driver.c
 @@ -31,6 +31,8 @@
   * SOFTWARE.
   */
  
 +#define pr_fmt(fmt) KBUILD_MODNAME :  fmt
 +
  #include linux/sched.h
  #include linux/spinlock.h
  #include linux/idr.h
 @@ -399,8 +401,8 @@ static int ipath_init_one(struct pci_dev *pdev, const 
 struct pci_device_id *ent)
   u32 bar0 = 0, bar1 = 0;
  
  #ifdef CONFIG_X86_64
 - if (WARN(pat_enabled(),
 -  ipath needs PAT disabled, boot with nopat kernel 
 parameter\n)) {
 + if (pat_enabled()) {
 + pr_warn(ipath needs PAT disabled, boot with nopat kernel 
 parameter\n);
   ret = -ENODEV;
   goto bail;
   }

Same observation as for the other patch: please only warn if the hardware is 
present and the driver tries to activate. No need to annoy others.

Thanks,

Ingo
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [x86/mm/pat, drivers/media/ivtv] WARNING: CPU: 0 PID: 1 at drivers/media/pci/ivtv/ivtvfb.c:1270 ivtvfb_init()

2015-06-22 Thread Ingo Molnar

* Borislav Petkov b...@suse.de wrote:

 On Sun, Jun 21, 2015 at 10:23:48PM +0200, Luis R. Rodriguez wrote:

  Nope, well the driver requires huge amounts of work to work with PAT, that 
  work will likely never be done, so hence the warning. Its our compromise as 
  only 2 drivers will live on Linux like this and they are both old and rare.
 
 Hmm, so wasn't the possibility discussed to fail loading instead and issue a 
 single-line pr_warn() when PAT is enabled? Those big WARN() splats will only 
 confuse people...

Absolutely - they are a regression.

Thanks,

Ingo
--
To unsubscribe from this list: send the line unsubscribe linux-media in


Re: [Xen-devel] RIP MTRR - status update for upcoming v4.2

2015-06-13 Thread Ingo Molnar

* Andy Lutomirski l...@amacapital.net wrote:

 On Jun 12, 2015 12:59 AM, Jan Beulich jbeul...@suse.com wrote:
 
   On 12.06.15 at 01:23, toshi.k...@hp.com wrote:
   There are two usages on MTRRs:
1) MTRR entries set by firmware
2) MTRR entries set by OS drivers
  
   We can obsolete 2), but we have no control over 1).  As UEFI firmwares
   also set this up, this usage will continue to stay.  So, we should not
   get rid of the MTRR code that looks up the MTRR entries, while we have
   no need to modify them.
  
   Such MTRR entries provide safe guard to /dev/mem, which allows privileged 
   user to access a range that may require UC mapping while the /dev/mem 
   driver 
   blindly maps it with WB.  MTRRs converts WB to UC in such a case.
 
  But it wouldn't be impossible to simply read the MTRRs upon boot, store the 
  information, disable MTRRs, and correctly use PAT to achieve the same 
  effect 
  (i.e. the blindly maps part of course would need fixing).
 
 This may crash and burn badly when we call a UEFI function or an SMI happens. 
  I 
 think we should just leave the MTRRs alone.

Not to mention suspend/resume, reboot and other goodies where the firmware 
might 
pop up expecting intact MTRRs.

Btw., doesn't a lack of MTRRs imply UC? So is 'crash and burn' possible in most 
cases? Isn't it just 'executes slower than before'?

Thanks,

Ingo
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v5 2/7] mutex: add support for wound/wait style locks, v5

2013-06-20 Thread Ingo Molnar

* Maarten Lankhorst maarten.lankho...@canonical.com wrote:

 Changes since RFC patch v1:
  - Updated to use atomic_long instead of atomic, since the reservation_id was 
 a long.
  - added mutex_reserve_lock_slow and mutex_reserve_lock_intr_slow
  - removed mutex_locked_set_reservation_id (or w/e it was called)
 Changes since RFC patch v2:
  - remove use of __mutex_lock_retval_arg, add warnings when using wrong 
 combination of
mutex_(,reserve_)lock/unlock.
 Changes since v1:
  - Add __always_inline to __mutex_lock_common, otherwise reservation paths 
 can be
triggered from normal locks, because __builtin_constant_p might evaluate 
 to false
for the constant 0 in that case. Tests for this have been added in the 
 next patch.
  - Updated documentation slightly.
 Changes since v2:
  - Renamed everything to ww_mutex. (mlankhorst)
  - Added ww_acquire_ctx and ww_class. (mlankhorst)
  - Added a lot of checks for wrong api usage. (mlankhorst)
  - Documentation updates. (danvet)
 Changes since v3:
  - Small documentation fixes (robclark)
  - Memory barrier fix (danvet)
 Changes since v4:
  - Remove ww_mutex_unlock_single and ww_mutex_lock_single.
  - Rename ww_mutex_trylock_single to ww_mutex_trylock.
  - Remove separate implementations of ww_mutex_lock_slow*, normal
functions can be used. Inline versions still exist for extra
debugging.
  - Cleanup unneeded memory barriers, add comment to the remaining
smp_mb().

That's not a proper changelog. It should be a short description of what it 
does, possibly referring to the new Documentation/ww-mutex-design.txt file 
for more details.

 Signed-off-by: Maarten Lankhorst maarten.lankho...@canonical.com
 Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
 Signed-off-by: Rob Clark robdcl...@gmail.com

That's not a valid signoff chain: the last signoff in the chain is the 
person sending me the patch. The first signoff is the person who wrote the 
patch. The other two gents should be Acked-by I suspect?

Thanks,

Ingo
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v5 2/7] mutex: add support for wound/wait style locks, v5

2013-06-20 Thread Ingo Molnar

* Maarten Lankhorst maarten.lankho...@canonical.com wrote:

 +The algorithm that TTM came up with for dealing with this problem is quite
 +simple. [...]

'TTM' here reads like a person - but in reality it's the TTM graphics 
subsystem, right?

Please clarify this portion of the text.

Thanks,

Ingo
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v5 2/7] mutex: add support for wound/wait style locks, v5

2013-06-20 Thread Ingo Molnar

* Maarten Lankhorst maarten.lankho...@canonical.com wrote:

 Well they've helped me with some of the changes and contributed some 
 code and/or fixes, but if acked-by is preferred I'll use that..

Such contributions can be credited in the changelog, and/or copyright 
notices, and/or the code itself. The signoff chain on the other hand is 
strictly defined as a 'route the patch took', with a single point of 
origin, the main author. See Documentation/SubmittingPatches, pt 12.

[ A signoff chain _can_ signal multi-authored code where the code got 
  written by someone and then further fixed/developed by someone else - 
  who adds a SOB to the end - but in that case I expect to get the patch 
  from the last person in the signoff chain. ]

Thanks,

Ingo
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] V4L/DVB: mantis: Fix IR_CORE dependency

2010-08-24 Thread Ingo Molnar

* Mauro Carvalho Chehab mche...@redhat.com wrote:

 Linus,
 
 Please pull from:
   ssh://master.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-2.6.git 
 v4l_for_linus
 
 For 3 build fixes.
 
 Cheers,
 Mauro.
 
 The following changes since commit ad41a1e0cab07c5125456e8d38e5b1ab148d04aa:
 
   Merge branch 'io_remap_pfn_range' of git://www.jni.nu/cris (2010-08-12 
 10:17:19 -0700)
 
 are available in the git repository at:
 
   ssh://master.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-2.6.git 
 v4l_for_linus
 
 Mauro Carvalho Chehab (2):
   V4L/DVB: Fix IR_CORE dependencies
   V4L/DVB: fix Kconfig to depends on VIDEO_IR
 
 Randy Dunlap (1):
   V4L/DVB: v4l2-ctrls.c: needs to include slab.h

FYI, there's one more IR_CORE related build bug which triggers 
frequently in randconfig tests - see the fix below.

Thanks,

Ingo

---
From c56aef270d7ec01564c632c1f7ebab6b8f9f032c Mon Sep 17 00:00:00 2001
From: Ingo Molnar mi...@elte.hu
Date: Tue, 24 Aug 2010 10:41:33 +0200
Subject: [PATCH] V4L/DVB: mantis: Fix IR_CORE dependency

This build bug triggers:

 drivers/built-in.o: In function `mantis_exit':
 (.text+0x377413): undefined reference to `ir_input_unregister'
 drivers/built-in.o: In function `mantis_input_init':
 (.text+0x3774ff): undefined reference to `__ir_input_register'

If MANTIS_CORE is enabled but IR_CORE is not. Add the correct
dependency.

Signed-off-by: Ingo Molnar mi...@elte.hu
---
 drivers/media/dvb/mantis/Kconfig |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/media/dvb/mantis/Kconfig b/drivers/media/dvb/mantis/Kconfig
index decdeda..fd0830e 100644
--- a/drivers/media/dvb/mantis/Kconfig
+++ b/drivers/media/dvb/mantis/Kconfig
@@ -1,6 +1,6 @@
 config MANTIS_CORE
tristate Mantis/Hopper PCI bridge based devices
-   depends on PCI  I2C  INPUT
+   depends on PCI  I2C  INPUT  IR_CORE
 
help
  Support for PCI cards based on the Mantis and Hopper PCi bridge.
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] V4L/DVB: mantis: Fix IR_CORE dependency

2010-08-24 Thread Ingo Molnar

* Randy Dunlap randy.dun...@oracle.com wrote:

 On 08/24/10 01:45, Ingo Molnar wrote:
  
  * Mauro Carvalho Chehab mche...@redhat.com wrote:
  
  Linus,
 
  Please pull from:
ssh://master.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-2.6.git 
  v4l_for_linus
 
  For 3 build fixes.
 
  Cheers,
  Mauro.
 
  The following changes since commit 
  ad41a1e0cab07c5125456e8d38e5b1ab148d04aa:
 
Merge branch 'io_remap_pfn_range' of git://www.jni.nu/cris (2010-08-12 
  10:17:19 -0700)
 
  are available in the git repository at:
 
ssh://master.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-2.6.git 
  v4l_for_linus
 
  Mauro Carvalho Chehab (2):
V4L/DVB: Fix IR_CORE dependencies
V4L/DVB: fix Kconfig to depends on VIDEO_IR
 
  Randy Dunlap (1):
V4L/DVB: v4l2-ctrls.c: needs to include slab.h
  
  FYI, there's one more IR_CORE related build bug which triggers 
  frequently in randconfig tests - see the fix below.
  
  Thanks,
  
  Ingo
  
  ---
  From c56aef270d7ec01564c632c1f7ebab6b8f9f032c Mon Sep 17 00:00:00 2001
  From: Ingo Molnar mi...@elte.hu
  Date: Tue, 24 Aug 2010 10:41:33 +0200
  Subject: [PATCH] V4L/DVB: mantis: Fix IR_CORE dependency
  
  This build bug triggers:
  
   drivers/built-in.o: In function `mantis_exit':
   (.text+0x377413): undefined reference to `ir_input_unregister'
   drivers/built-in.o: In function `mantis_input_init':
   (.text+0x3774ff): undefined reference to `__ir_input_register'
  
  If MANTIS_CORE is enabled but IR_CORE is not. Add the correct
  dependency.
  
  Signed-off-by: Ingo Molnar mi...@elte.hu
  ---
   drivers/media/dvb/mantis/Kconfig |2 +-
   1 files changed, 1 insertions(+), 1 deletions(-)
  
  diff --git a/drivers/media/dvb/mantis/Kconfig 
  b/drivers/media/dvb/mantis/Kconfig
  index decdeda..fd0830e 100644
  --- a/drivers/media/dvb/mantis/Kconfig
  +++ b/drivers/media/dvb/mantis/Kconfig
  @@ -1,6 +1,6 @@
   config MANTIS_CORE
  tristate Mantis/Hopper PCI bridge based devices
  -   depends on PCI  I2C  INPUT
  +   depends on PCI  I2C  INPUT  IR_CORE
   
  help
Support for PCI cards based on the Mantis and Hopper PCi bridge.
 
 
 Acked-by: Randy Dunlap randy.dun...@oracle.com
 http://lkml.org/lkml/2010/8/17/341

Your patch came first :-)

Btw., the reason i missed your patch is that i grepped lkml for the 
static build failure - while your changelog contained the modular one. 
Oh well :)

Thanks,

Ingo
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH -mmotm] media: ak881x needs slab.h

2010-06-01 Thread Ingo Molnar

* Randy Dunlap randy.dun...@oracle.com wrote:

 From: Randy Dunlap randy.dun...@oracle.com
 
 Add slab.h to fix ak881x build:
 
 drivers/media/video/ak881x.c:265:error: implicit declaration of function 
 'kzalloc'
 drivers/media/video/ak881x.c:265:warning: assignment makes pointer from 
 integer without a cast
 drivers/media/video/ak881x.c:283:error: implicit declaration of function 
 'kfree'
 
 Signed-off-by: Randy Dunlap randy.dun...@oracle.com
 Cc: Guennadi Liakhovetski g.liakhovet...@gmx.de
 Cc: Mauro Carvalho Chehab mche...@infradead.org
 ---
  drivers/media/video/ak881x.c |1 +

This build bug is now triggering in .35-rc1 as well.

Ingo
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [GIT PULL for 2.6.33] V4L/DVB updates

2009-12-09 Thread Ingo Molnar

Hi,

FYI, there's a new build failure on 32-bit x86 caused by the new max2165 
tuner driver:

drivers/built-in.o: In function `max2165_set_params':
max2165.c:(.text+0x486293): undefined reference to `__floatunsidf'
max2165.c:(.text+0x4862bc): undefined reference to `__floatunsidf'
max2165.c:(.text+0x4862c5): undefined reference to `__adddf3'
max2165.c:(.text+0x4862cf): undefined reference to `__fixunsdfsi'

Ingo
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [GIT PULL for 2.6.33] V4L/DVB updates

2009-12-09 Thread Ingo Molnar

* David T. L. Wong davidtlw...@gmail.com wrote:

 Ingo Molnar wrote:
 Hi,
 
 FYI, there's a new build failure on 32-bit x86 caused by the new
 max2165 tuner driver:
 
 drivers/built-in.o: In function `max2165_set_params':
 max2165.c:(.text+0x486293): undefined reference to `__floatunsidf'
 max2165.c:(.text+0x4862bc): undefined reference to `__floatunsidf'
 max2165.c:(.text+0x4862c5): undefined reference to `__adddf3'
 max2165.c:(.text+0x4862cf): undefined reference to `__fixunsdfsi'
 
  Ingo
 
 
 A patch is already submitted to v4l mailing list:
 http://patchwork.kernel.org/patch/64538/

That patch you posted a week ago solves the build problem, thanks.

Ingo
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [origin tree build failure] [PATCH] media: video: Fix build in saa7164

2009-09-22 Thread Ingo Molnar

* Mauro Carvalho Chehab mche...@redhat.com wrote:

 Em Mon, 21 Sep 2009 20:23:45 +0200
 Ingo Molnar mi...@elte.hu escreveu:
 
  
  * Mauro Carvalho Chehab mche...@redhat.com wrote:
  
   This series also contains several new drivers:
   
  - new driver for NXP saa7164;
  
  -tip testing found that an allyesconfig build buglet found its way into 
  this driver - find the fix below.
  
 
  diff --git a/drivers/media/video/saa7164/saa7164-core.c 
  b/drivers/media/video/saa7164/saa7164-core.c
  index f0dbead..60f3214 100644
  --- a/drivers/media/video/saa7164/saa7164-core.c
  +++ b/drivers/media/video/saa7164/saa7164-core.c
  @@ -45,8 +45,8 @@ MODULE_LICENSE(GPL);
32 bus
*/
   
  -unsigned int debug;
  -module_param(debug, int, 0644);
  +unsigned int saa_debug;
  +module_param(saa_debug, int, 0644);
 
 Hmm... it is better to use module_param_named(debug, saa_debug, int, 0644), to
 keep presenting the parameter as just debug to userspace.

ah, yes, fully agreed - i thought i did that but apparently modified it 
in this instance.

  This is because recent saa7164 changes introduced a global symbol 
  named 'debug'. The x86 platform code already defines a 'debug' 
  symbol. (which is named in a too generic way as well - but it can be 
  used nicely to weed out too generic symbols in drivers ;-
 
 Agreed.
 
 Btw, I suggest to do a similar patch also for x86, to avoid such 
 future conflicts.

Actually, Andrew considers it a feature that allows us to filter out too 
generic names early on :-)

Ingo
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[origin tree build failure] [PATCH] media: video: Fix build in saa7164

2009-09-21 Thread Ingo Molnar

* Mauro Carvalho Chehab mche...@redhat.com wrote:

 This series also contains several new drivers:
 
- new driver for NXP saa7164;

-tip testing found that an allyesconfig build buglet found its way into 
this driver - find the fix below.

Thanks,

Ingo

-
From 67a0d8c7dfdf2de72ddec3d821ae87a80ecaed83 Mon Sep 17 00:00:00 2001
From: Ingo Molnar mi...@elte.hu
Date: Mon, 21 Sep 2009 20:14:47 +0200
Subject: [PATCH] media: video: Fix build in saa7164

-tip testing found that the x86 build (64-bit allyesconfig) fails due to:

  LD  vmlinux.o
  drivers/built-in.o:(.bss+0x4b648): multiple definition of `debug'
  arch/x86/built-in.o:(.kprobes.text+0x88): first defined here
  ld: Warning: size of symbol `debug' changed from 90 in arch/x86/built-in.o to 
4 in drivers/built-in.o
 make: *** [vmlinux.o] Error 1

This is because recent saa7164 changes introduced a global symbol
named 'debug'. The x86 platform code already defines a 'debug'
symbol. (which is named in a too generic way as well - but it
can be used nicely to weed out too generic symbols in drivers ;-)

Rename it to saa_debug.

Signed-off-by: Ingo Molnar mi...@elte.hu
---
 drivers/media/video/saa7164/saa7164-api.c  |8 
 drivers/media/video/saa7164/saa7164-cmd.c  |2 +-
 drivers/media/video/saa7164/saa7164-core.c |6 +++---
 drivers/media/video/saa7164/saa7164.h  |4 ++--
 4 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/media/video/saa7164/saa7164-api.c 
b/drivers/media/video/saa7164/saa7164-api.c
index bb6df1b..6f094a9 100644
--- a/drivers/media/video/saa7164/saa7164-api.c
+++ b/drivers/media/video/saa7164/saa7164-api.c
@@ -415,7 +415,7 @@ int saa7164_api_enum_subdevs(struct saa7164_dev *dev)
goto out;
}
 
-   if (debug  DBGLVL_API)
+   if (saa_debug  DBGLVL_API)
saa7164_dumphex16(dev, buf, (buflen/16)*16);
 
saa7164_api_dump_subdevs(dev, buf, buflen);
@@ -480,7 +480,7 @@ int saa7164_api_i2c_read(struct saa7164_i2c *bus, u8 addr, 
u32 reglen, u8 *reg,
 
dprintk(DBGLVL_API, %s() len = %d bytes\n, __func__, len);
 
-   if (debug  DBGLVL_I2C)
+   if (saa_debug  DBGLVL_I2C)
saa7164_dumphex16(dev, buf, 2 * 16);
 
ret = saa7164_cmd_send(bus-dev, unitid, GET_CUR,
@@ -488,7 +488,7 @@ int saa7164_api_i2c_read(struct saa7164_i2c *bus, u8 addr, 
u32 reglen, u8 *reg,
if (ret != SAA_OK)
printk(KERN_ERR %s() error, ret(2) = 0x%x\n, __func__, ret);
else {
-   if (debug  DBGLVL_I2C)
+   if (saa_debug  DBGLVL_I2C)
saa7164_dumphex16(dev, buf, sizeof(buf));
memcpy(data, (buf + 2 * sizeof(u32) + reglen), datalen);
}
@@ -548,7 +548,7 @@ int saa7164_api_i2c_write(struct saa7164_i2c *bus, u8 addr, 
u32 datalen,
*((u32 *)(buf + 1 * sizeof(u32))) = datalen - reglen;
memcpy((buf + 2 * sizeof(u32)), data, datalen);
 
-   if (debug  DBGLVL_I2C)
+   if (saa_debug  DBGLVL_I2C)
saa7164_dumphex16(dev, buf, sizeof(buf));
 
ret = saa7164_cmd_send(bus-dev, unitid, SET_CUR,
diff --git a/drivers/media/video/saa7164/saa7164-cmd.c 
b/drivers/media/video/saa7164/saa7164-cmd.c
index e097f1a..c45966e 100644
--- a/drivers/media/video/saa7164/saa7164-cmd.c
+++ b/drivers/media/video/saa7164/saa7164-cmd.c
@@ -250,7 +250,7 @@ int saa7164_cmd_wait(struct saa7164_dev *dev, u8 seqno)
unsigned long stamp;
int r;
 
-   if (debug = 4)
+   if (saa_debug = 4)
saa7164_bus_dump(dev);
 
dprintk(DBGLVL_CMD, %s(seqno=%d)\n, __func__, seqno);
diff --git a/drivers/media/video/saa7164/saa7164-core.c 
b/drivers/media/video/saa7164/saa7164-core.c
index f0dbead..60f3214 100644
--- a/drivers/media/video/saa7164/saa7164-core.c
+++ b/drivers/media/video/saa7164/saa7164-core.c
@@ -45,8 +45,8 @@ MODULE_LICENSE(GPL);
  32 bus
  */
 
-unsigned int debug;
-module_param(debug, int, 0644);
+unsigned int saa_debug;
+module_param(saa_debug, int, 0644);
 MODULE_PARM_DESC(debug, enable debug messages);
 
 unsigned int waitsecs = 10;
@@ -653,7 +653,7 @@ static int __devinit saa7164_initdev(struct pci_dev 
*pci_dev,
printk(KERN_ERR %s() Unsupported board detected, 
registering without firmware\n, __func__);
 
-   dprintk(1, %s() parameter debug = %d\n, __func__, debug);
+   dprintk(1, %s() parameter debug = %d\n, __func__, saa_debug);
dprintk(1, %s() parameter waitsecs = %d\n, __func__, waitsecs);
 
 fail_fw:
diff --git a/drivers/media/video/saa7164/saa7164.h 
b/drivers/media/video/saa7164/saa7164.h
index 6753008..42660b5 100644
--- a/drivers/media/video/saa7164/saa7164.h
+++ b/drivers/media/video/saa7164/saa7164.h
@@ -375,9 +375,9 @@ extern int saa7164_buffer_dealloc(struct saa7164_tsport 
*port,
 
 /* --- */
 
-extern unsigned int debug;
+extern

[build failure] Re: [GIT PATCHES for 2.6.29-rc8] V4L/DVB fixes

2009-03-15 Thread Ingo Molnar

* Ingo Molnar mi...@elte.hu wrote:

 FYI, latest upstream (2.6.29-rc8) fails to build with the 
 attached config:
 
   MODPOST 531 modules
 ERROR: dibusb_dib3000mc_frontend_attach 
 [drivers/media/dvb/dvb-usb/dvb-usb-dibusb-mc.ko] undefined!
 ERROR: dibusb_dib3000mc_tuner_attach 
 [drivers/media/dvb/dvb-usb/dvb-usb-dibusb-mc.ko] undefined!
 ERROR: dibusb_dib3000mc_frontend_attach 
 [drivers/media/dvb/dvb-usb/dvb-usb-a800.ko] undefined!
 ERROR: dibusb_dib3000mc_tuner_attach 
 [drivers/media/dvb/dvb-usb/dvb-usb-a800.ko] undefined!

Note, the tree i tried is post-rc8: v2.6.29-rc8-90-g326d851.

It already includes the latest V4L/DVB fixes.

Ingo
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [git pull] DVB + ieee1394: firedtv driver

2009-02-26 Thread Ingo Molnar

* Linus Torvalds torva...@linux-foundation.org wrote:

 Two choices that I can see:
 
  - do the ieee1394_init() as a fs_initcall(), earlier
 
  - move drivers/ieee1394 linking up to before drivers/media
 
 but I suspect that drivers/media wants to be early, in order to do the 
 _media_ layer initialization early.
 
 Does this work?

yes, i just tested it and your patch fixes the crash:

 mercury:~ uname -a
 Linux mercury 2.6.29-rc6-tip-02011-gb62a1ed-dirty #250 SMP Thu 
 Feb 26 19:00:54 CET 2009 i686 athlon i386 GNU/Linux
 mercury:~ uptime
  19:02:51 up 0 min,  1 user,  load average: 3.97, 1.10, 0.37

Tested-by: Ingo Molnar mi...@elte.hu

Thanks!

Ingo
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [GIT PULL -tip] fix 22 make headers_check - 200901

2009-01-26 Thread Ingo Molnar

* Jaswinder Singh Rajput jaswin...@kernel.org wrote:

 On Sat, 2009-01-24 at 18:26 +0530, Jaswinder Singh Rajput wrote:
  The following changes since commit aa52dcf69565512e7d285c1c40dc6e56aab6f789:
Ingo Molnar (1):
  Merge branch 'x86/urgent'
  
  are available in the git repository at:
  
git://git.kernel.org/pub/scm/linux/kernel/git/jaswinder/linux-2.6-tip.git 
  master
  
  Jaswinder Singh Rajput (11):
headers_check fix: linux/dvb/audio.h
headers_check fix: linux/acct.h
headers_check fix: linux/elf.h
headers_check fix: linux/flat.h
headers_check fix: linux/hid.h
headers_check fix: linux/if_frad.h
 
 'headers_check fix: linux/if_frad.h' is dropped because of some issues.
 So here is new pull request:
 
 The following changes since commit aa52dcf69565512e7d285c1c40dc6e56aab6f789:
   Ingo Molnar (1):
 Merge branch 'x86/urgent'
 
 are available in the git repository at:
 
   git://git.kernel.org/pub/scm/linux/kernel/git/jaswinder/linux-2.6-tip.git 
 master
 
 Jaswinder Singh Rajput (10):
   headers_check fix: linux/dvb/audio.h
   headers_check fix: linux/acct.h
   headers_check fix: linux/elf.h
   headers_check fix: linux/flat.h
   headers_check fix: linux/hid.h
   headers_check fix: linux/types.h
   headers_check fix: linux/videodev.h
   headers_check fix: x86, kvm.h
   headers_check fix: x86, swab.h
   headers_check fix: video/edid.h
 
  arch/x86/include/asm/kvm.h  |2 +-
  arch/x86/include/asm/swab.h |   12 +---
  include/linux/acct.h|2 +-
  include/linux/dvb/audio.h   |5 -
  include/linux/elf.h |3 ++-
  include/linux/flat.h|6 +++---
  include/linux/hid.h |2 ++
  include/linux/types.h   |2 ++
  include/linux/videodev.h|   10 --
  include/video/edid.h|2 +-
  10 files changed, 29 insertions(+), 17 deletions(-)

Pulled into tip/core/header-fixes, thanks Jaswinder!

Ingo
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [GIT PULL -tip] fix 33 make headers_check warnings

2009-01-18 Thread Ingo Molnar

* Sam Ravnborg s...@ravnborg.org wrote:

 On Sun, Jan 18, 2009 at 08:37:41AM +1100, David Woodhouse wrote:
  On Sun, 2009-01-18 at 01:47 +0530, Jaswinder Singh Rajput wrote:
   --- a/include/linux/dvb/audio.h
   +++ b/include/linux/dvb/audio.h
   @@ -24,9 +24,8 @@
#ifndef _DVBAUDIO_H_
#define _DVBAUDIO_H_

   -#ifdef __KERNEL__
#include linux/types.h
   -#else
   +#ifndef __KERNEL__
#include stdint.h
#endif

  
  That patch looks wrong, and unnecessary. It was fine before.
 Nope - include/linux/dvb/audio.h failed to include linux/types.h
 despite the fact that is uses __u32 etc.
 
 But why the _kernel_ should include a userspace header is
 much more questionable.

ok, i dropped this one for the time being.

Sam, Andrew, et al, i've got the commit lineup below, to fix the ~200 new 
CONFIG_HEADERS_CHECK=y warnings we have in current -git, on x86 
allyesconfig.

Note that i have restructured the tree and the commits so that this effort 
can be tracked more easily: each commit has a headers_check fix subject 
line prefix. So you can go back later and revisit these things - for 
example whether a header really needs to be exported to user-space, via:

  git log --grep='headers_check fix:' --pretty=format:%h: %s

to get an overview and a historic track record. We can follow this 
notation in the future too.

So if there are no objections, i'll send this to Linus tomorrow-ish and 
we'll have the worst of the fallout behind us and can embark on a much 
more fluid (and per maintainer) headers_check related workflow.

Ingo

-
The latest core/header-fixes git tree can be pulled from:

   git://git.kernel.org/pub/scm/linux/kernel/git/tip/linux-2.6-tip.git 
core/header-fixes

--
Cyrill Gorcunov (3):
  headers_check fix: x86, prctl.h
  headers_check fix: x86, sigcontext32.h
  headers_check fix: x86, setup.h

Jaswinder Singh Rajput (38):
  headers_check fix: include/linux/*, __[us]{8,16,32,64} types
  headers_check fix: capability.h: extern's make no sense in userspace
  headers_check fix: coda_psdev.h: extern's make no sense in userspace
  headers_check fix: in6.h: extern's make no sense in userspace
  headers_check fix: nubus.h: extern's make no sense in userspace
  headers_check fix: socket.h: extern's make no sense in userspace
  headers_check fix: x86, e820.h
  headers_check fix: x86, kvm.h
  headers_check fix: x86, mce.h
  headers_check fix: x86, mtrr.h
  headers_check fix: x86, ptrace-abi.h
  headers_check fix: x86, sigcontext.h
  headers_check fix: x86, swab.h
  headers_check fix: can/bcm.h
  headers_check fix: dvb/dmx.h
  headers_check fix: dvb/frontend.h
  headers_check fix: dvb/net.h
  headers_check fix: dvb/video.h
  headers_check fix: netfilter/xt_conntrack.h
  headers_check fix: nfsd/export.h
  headers_check fix: nfsd/nfsfh.h
  headers_check fix: nfsd/stats.h
  headers_check fix: nfsd/syscall.h
  headers_check fix: raid/md_p.h
  headers_check fix: spi/spidev.h
  headers_check fix: tc_act/tc_gact.h
  headers_check fix: tc_act/tc_mirred.h
  headers_check fix: tc_act/tc_pedit.h
  headers_check fix: tc_ematch/tc_em_cmp.h
  headers_check fix: tc_ematch/tc_em_meta.h
  headers_check fix: tc_ematch/tc_em_nbyte.h
  headers_check fix: tc_ematch/tc_em_text.h
  headers_check fix: usb/cdc.h
  headers_check fix: usb/gadgetfs.h
  headers_check fix: mtd/inftl-user.h
  headers_check fix: sound/hdsp.h
  headers_check fix: video/sisfb.h
  headers_check fix: video/uvesafb.h


 arch/x86/include/asm/e820.h|   11 ---
 arch/x86/include/asm/kvm.h |2 +-
 arch/x86/include/asm/mce.h |5 +
 arch/x86/include/asm/mtrr.h|1 +
 arch/x86/include/asm/prctl.h   |4 
 arch/x86/include/asm/ptrace-abi.h  |5 -
 arch/x86/include/asm/setup.h   |8 +++-
 arch/x86/include/asm/sigcontext.h  |2 +-
 arch/x86/include/asm/sigcontext32.h|2 ++
 arch/x86/include/asm/swab.h|   21 +++--
 include/linux/aio_abi.h|1 +
 include/linux/atalk.h  |1 +
 include/linux/atmbr2684.h  |1 +
 include/linux/auto_fs4.h   |1 +
 include/linux/bfs_fs.h |2 ++
 include/linux/blktrace_api.h   |2 ++
 include/linux/can/bcm.h|2 ++
 include/linux/capability.h |8 
 include/linux/cdrom.h  |1 +
 include/linux/cgroupstats.h|1 +
 include/linux/coda_psdev.h |2 ++
 include/linux/dlm_plock.h  |2 ++
 include/linux/dn.h |2 ++
 include/linux/dvb/dmx.h|2 +-
 include/linux/dvb/frontend.h   |3 +--
 include/linux/dvb/net.h|3