2015-07-17 21:07:36

by Luis Chamberlain

[permalink] [raw]
Subject: [RESEND PATCH v2 0/2] x86/mm/pat: modify nopat requirement warning

From: "Luis R. Rodriguez" <[email protected]>

Ingo,

Boris is on vacation so sending this through you. This is just a resend of
the v2 series. The issue here was the WARN() splat on built-in kernels due
to ivtv's funky probe which will trigger even if you don't have any ivtv
hardware. I've moved this to only trigger upon a device detection.

We also spoke about not doing any of this and and letting it silently fail
for these drivers but since this is only for two drivers and the ipath diver
will be removed only the ivtv driver would be left with this work around. I'd
like to enforce the semantics for usage of arch_phys_wc_add() with ioremap_wc()
and making an exception just for ivtv does not seem worth the gains of having
strong semantics. After all the ioremap_wc() + arch_phys_wc_add() are in I'll
then try to add an SmPL rule check to enforce semantics on the ioremap_wc() +
arch_phys_wc_add() API. Hope is that maintainers can vet new code for its
correct usage in the future with 'make coccicheck M=path' on their subsystems.

For your reference we discussed this and I mentioned my semantics preference
and you were OK with this [0] so just resending this series as it fell through
the cracks as Boris is on vacation now.

Although the WARN() --> pr_warn() change is not techically needed for ipath,
we make both checks consistent and less chatty. Since the ivtv change is
splattering all v4.2 kernels that patch may be worthy for v4.2 inclusion. I
did not peg the Cc: stable tag so leave this up to you to decide.

Please let me know if this is OK or if there are any other oustandind issues.

[0] http://lkml.kernel.org/r/[email protected]

Luis R. Rodriguez (2):
x86/mm/pat, drivers/infiniband/ipath: replace WARN() with pr_warn()
x86/mm/pat, drivers/media/ivtv: move pat warn and replace WARN() with
pr_warn()

drivers/infiniband/hw/ipath/ipath_driver.c | 6 ++++--
drivers/media/pci/ivtv/ivtvfb.c | 15 +++++++++------
2 files changed, 13 insertions(+), 8 deletions(-)

--
2.3.2.209.gd67f9d5.dirty


2015-07-17 21:07:41

by Luis Chamberlain

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

From: "Luis R. Rodriguez" <[email protected]>

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 <[email protected]>
---
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 2d7e503d13cb..871dbe56216a 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;
}
--
2.3.2.209.gd67f9d5.dirty

2015-07-17 21:07:44

by Luis Chamberlain

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

From: "Luis R. Rodriguez" <[email protected]>

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 <[email protected]> [0-day test robot]
Signed-off-by: Luis R. Rodriguez <[email protected]>
---
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 4cb365d4ffdc..8b95eefb610b 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;
@@ -1265,12 +1274,6 @@ static int __init ivtvfb_init(void)
int registered = 0;
int err;

-#ifdef CONFIG_X86_64
- if (WARN(pat_enabled(),
- "ivtvfb needs PAT disabled, boot with nopat kernel parameter\n")) {
- return -ENODEV;
- }
-#endif

if (ivtvfb_card_id < -1 || ivtvfb_card_id >= IVTV_MAX_CARDS) {
printk(KERN_ERR "ivtvfb: ivtvfb_card_id parameter is out of range (valid range: -1 - %d)\n",
--
2.3.2.209.gd67f9d5.dirty

2015-07-20 18:08:22

by Doug Ledford

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


> On Jul 17, 2015, at 5:07 PM, Luis R. Rodriguez <[email protected]> wrote:
>
> From: "Luis R. Rodriguez" <[email protected]>
>
> 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 <[email protected]>

Acked-by: Doug Ledford <[email protected]>


Doug Ledford <[email protected]>
GPG Key ID: 0E572FDD






Attachments:
signature.asc (842.00 B)
Message signed with OpenPGP using GPGMail
Subject: [tip:x86/urgent] x86/mm/pat, drivers/infiniband/ipath: Replace WARN() with pr_warn()

Commit-ID: fd0a1b8607ef311a2c800dd54c9a4a3583756ea6
Gitweb: http://git.kernel.org/tip/fd0a1b8607ef311a2c800dd54c9a4a3583756ea6
Author: Luis R. Rodriguez <[email protected]>
AuthorDate: Fri, 17 Jul 2015 14:07:24 -0700
Committer: Ingo Molnar <[email protected]>
CommitDate: Tue, 21 Jul 2015 09:42:54 +0200

x86/mm/pat, drivers/infiniband/ipath: Replace WARN() with pr_warn()

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 <[email protected]>
Acked-by: Doug Ledford <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Ingo Molnar <[email protected]>
---
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;
}

Subject: [tip:x86/urgent] x86/mm/pat, drivers/media/ivtv: Move the PAT warning and replace WARN() with pr_warn()

Commit-ID: f5530d5af835ffa82a0607f5f1977d63ac02551f
Gitweb: http://git.kernel.org/tip/f5530d5af835ffa82a0607f5f1977d63ac02551f
Author: Luis R. Rodriguez <[email protected]>
AuthorDate: Fri, 17 Jul 2015 14:07:25 -0700
Committer: Ingo Molnar <[email protected]>
CommitDate: Tue, 21 Jul 2015 09:42:54 +0200

x86/mm/pat, drivers/media/ivtv: Move the PAT warning and replace WARN() with pr_warn()

On built-in kernels this warning will always splat, even if no ivtvfb
hardware is present, as this is part of the module init:

if (WARN(pat_enabled(),
"ivtvfb needs PAT disabled, boot with nopat kernel parameter\n")) {

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 <[email protected]> [0-day test robot]
Signed-off-by: Luis R. Rodriguez <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Ingo Molnar <[email protected]>
---
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;
@@ -1265,12 +1274,6 @@ static int __init ivtvfb_init(void)
int registered = 0;
int err;

-#ifdef CONFIG_X86_64
- if (WARN(pat_enabled(),
- "ivtvfb needs PAT disabled, boot with nopat kernel parameter\n")) {
- return -ENODEV;
- }
-#endif

if (ivtvfb_card_id < -1 || ivtvfb_card_id >= IVTV_MAX_CARDS) {
printk(KERN_ERR "ivtvfb: ivtvfb_card_id parameter is out of range (valid range: -1 - %d)\n",