Re: [LKP] [drm/mgag200] 90f479ae51: vm-scalability.median -18.8% regression

[Rong Chen]

Hi,

On 8/1/19 7:58 PM, Thomas Zimmermann wrote:

Hi

Am 01.08.19 um 13:25 schrieb Feng Tang:

Hi Thomas,

On Thu, Aug 01, 2019 at 11:59:28AM +0200, Thomas Zimmermann wrote:

Hi

Am 01.08.19 um 10:37 schrieb Feng Tang:

On Thu, Aug 01, 2019 at 02:19:53PM +0800, Rong Chen wrote:

commit: 90f479ae51afa45efab97afdde9b94b9660dd3e4 ("drm/mgag200: Replace struct 
mga_fbdev with generic framebuffer emulation")
https://kernel.googlesource.com/pub/scm/linux/kernel/git/next/linux-next.git 
master

Daniel, Noralf, we may have to revert this patch.

I expected some change in display performance, but not in VM. Since it's
a server chipset, probably no one cares much about display performance.
So that seemed like a good trade-off for re-using shared code.

Part of the patch set is that the generic fb emulation now maps and
unmaps the fbdev BO when updating the screen. I guess that's the cause
of the performance regression. And it should be visible with other
drivers as well if they use a shadow FB for fbdev emulation.

For fbcon we should need to do any maps/unamps at all, this is for the
fbdev mmap support only. If the testcase mentioned here tests fbdev
mmap handling it's pretty badly misnamed :-) And as long as you don't
have an fbdev mmap there shouldn't be any impact at all.

The ast and mgag200 have only a few MiB of VRAM, so we have to get the
fbdev BO out if it's not being displayed. If not being mapped, it can be
evicted and make room for X, etc.

To make this work, the BO's memory is mapped and unmapped in
drm_fb_helper_dirty_work() before being updated from the shadow FB. [1]
That fbdev mapping is established on each screen update, more or less.
 From my (yet unverified) understanding, this causes the performance
regression in the VM code.

The original code in mgag200 used to kmap the fbdev BO while it's being
displayed; [2] and the drawing code only mapped it when necessary (i.e.,
not being display). [3]

Hm yeah, this vmap/vunmap is going to be pretty bad. We indeed should
cache this.


I think this could be added for VRAM helpers as well, but it's still a
workaround and non-VRAM drivers might also run into such a performance
regression if they use the fbdev's shadow fb.

Yeah agreed, fbdev emulation should try to cache the vmap.


Noralf mentioned that there are plans for other DRM clients besides the
console. They would as well run into similar problems.


The thing is that we'd need another generic fbdev emulation for ast and
mgag200 that handles this issue properly.

Yeah I dont think we want to jump the gun here.  If you can try to
repro locally and profile where we're wasting cpu time I hope that
should sched a light what's going wrong here.

I don't have much time ATM and I'm not even officially at work until
late Aug. I'd send you the revert and investigate later. I agree that
using generic fbdev emulation would be preferable.

Still not sure that's the right thing to do really. Yes it's a
regression, but vm testcases shouldn run a single line of fbcon or drm
code. So why this is impacted so heavily by a silly drm change is very
confusing to me. We might be papering over a deeper and much more
serious issue ...

It's a regression, the right thing is to revert first and then work
out the right thing to do.

Sure, but I have no idea whether the testcase is doing something
reasonable. If it's accidentally testing vm scalability of fbdev and
there's no one else doing something this pointless, then it's not a
real bug. Plus I think we're shooting the messenger here.


It's likely the test runs on the console and printfs stuff out while running.

But why did we not regress the world if a few prints on the console
have such a huge impact? We didn't get an entire stream of mails about
breaking stuff ...

The regression seems not related to the commit.  But we have retested
and confirmed the regression.  Hard to understand what happens.

Does the regressed test cause any output on console while it's
measuring? If so, it's probably accidentally measuring fbcon/DRM code in
addition to the workload it's trying to measure.


Sorry, I'm not familiar with DRM, we enabled the console to output logs, and
attached please find the log file.

"Command line: ... console=tty0 earlyprintk=ttyS0,115200
console=ttyS0,115200 vga=normal rw"

We did more check, and found this test machine does use the
mgag200 driver.

And we are suspecting the regression is caused by

commit cf1ca9aeb930df074bb5bbcde55f935fec04e529
Author: Thomas Zimmermann 
Date:   Wed Jul 3 09:58:24 2019 +0200

Yes, that's the commit. Unfortunately reverting it would require
reverting a hand full of other patches as well.

I have a potential fix for the problem. Could you run and verify that it
resolves the problem?

Sure, please send it to us. Rong and I will try it.

Fantastic, thank you! The patch set is available on dri-devel at

   https://lists.freedesktop.org/archives/dri-devel/2019-August/228950.html


The patch set improves the performance slightl

[Bug 111283] pending questions can not be downloaded by s admin,privileged admin,privileged examiner

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=111283

Bug ID: 111283
   Summary: pending questions can not be downloaded by s
admin,privileged admin,privileged examiner
   Product: DRI
   Version: unspecified
  Hardware: Other
OS: Windows (All)
Status: NEW
  Severity: minor
  Priority: medium
 Component: General
  Assignee: dri-devel@lists.freedesktop.org
  Reporter: dhrisyasb...@gmail.com

Test step:
" 1"".Launch mozilla firefox
2.type URL :http://http://testwestkutt.targetentrance.com/logi  
3.Login with the Email: sad...@oqbs.com and password:admin""   
 rrejeena...@gmail.com and
password:diamondshunter   
 rrejeena...@gmail.com   and
password:rejeenai
click on sign in
Click on question bank
Click on download questions
Select question type
Select subject
Select chapter
Select Question catagory
Click on download
Click on ok

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 111283] pending questions can not be downloaded by s admin,privileged admin,privileged examiner

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=111283

dhrisya  changed:

   What|Removed |Added

   Assignee|dri-devel@lists.freedesktop |terrence...@intel.com
   |.org|
 QA Contact||terrence...@intel.com
 Whiteboard||OQBS
  Component|General |DRM/iGVT-g

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH/RFC 00/12] Add dual-LVDS panel support to EK874

[Fabrizio Castro]

Dear All,

this series adds support for dual-LVDS panel IDK-2121WR
from Advantech:
https://buy.advantech.eu/Displays/Embedded-LCD-Kits-High-Brightness/model-IDK-2121WR-K2FHA2E.htm
Dual link support is very recent for R-Car Gen3, and I couldn't
find much on dual link panels in the kernel either, therefore
comments are very welcome to get this right.

The panel doesn't come with the EK874 kit, but it's advertised as
supported, therefore this series adds a new dts file to support
the configuration of the EK874 + IDK-2121WR.

Finally, this series depends on a fix that's still pending:
https://patchwork.kernel.org/patch/11054755/

Thanks,
Fab

Fabrizio Castro (12):
  dt-bindings: display: renesas: lvds: RZ/G2E needs renesas,companion
too
  dt-bindings: display: renesas: lvds: Document renesas,swap-data
  dt-bindings: panel: lvds: Add dual-link LVDS display support
  dt-bindings: display: Add bindings for Advantech IDK-2121WR
  drm: rcar-du: lvds: Add data swap support
  drm: rcar-du: lvds: Do not look at ports for identifying bridges
  drm: rcar-du: lvds: Add support for dual link panels
  drm: rcar-du: lvds: Fix bridge_to_rcar_lvds
  drm: rcar-du: lvds: Fix companion's mode
  arm64: dts: renesas: r8a774c0: Point LVDS0 to its companion LVDS1
  arm64: dts: renesas: cat874: Add definition for 12V regulator
  arm64: dts: renesas: Add EK874 board with idk-2121wr display support

 .../bindings/display/bridge/renesas,lvds.txt   |  11 +-
 .../display/panel/advantech,idk-2121wr.txt |  62 
 .../bindings/display/panel/panel-lvds.txt  |  91 -
 arch/arm64/boot/dts/renesas/Makefile   |   3 +-
 arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts|   9 ++
 .../boot/dts/renesas/r8a774c0-ek874-idk-2121wr.dts | 112 +
 arch/arm64/boot/dts/renesas/r8a774c0.dtsi  |   2 +
 drivers/gpu/drm/rcar-du/rcar_lvds.c|  65 ++--
 8 files changed, 291 insertions(+), 64 deletions(-)
 create mode 100644 
Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.txt
 create mode 100644 arch/arm64/boot/dts/renesas/r8a774c0-ek874-idk-2121wr.dts

-- 
2.7.4

[PATCH/RFC 01/12] dt-bindings: display: renesas: lvds: RZ/G2E needs renesas,companion too

[Fabrizio Castro]

Document RZ/G2E support for property renesas,companion.

Signed-off-by: Fabrizio Castro 
---
 Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt 
b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
index c6a196d..dece79e 100644
--- a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
+++ b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
@@ -49,9 +49,9 @@ Each port shall have a single endpoint.
 Optional properties:
 
 - renesas,companion : phandle to the companion LVDS encoder. This property is
-  mandatory for the first LVDS encoder on D3 and E3 SoCs, and shall point to
-  the second encoder to be used as a companion in dual-link mode. It shall not
-  be set for any other LVDS encoder.
+  mandatory for the first LVDS encoder on R-Car D3, R-Car E3, and RZ/G2E SoCs,
+  and shall point to the second encoder to be used as a companion in dual-link
+  mode. It shall not be set for any other LVDS encoder.
 
 
 Example:
-- 
2.7.4

[PATCH/RFC 03/12] dt-bindings: panel: lvds: Add dual-link LVDS display support

[Fabrizio Castro]

Dual-link LVDS displays have two ports, therefore document this
with the bindings.

Signed-off-by: Fabrizio Castro 
---
 .../bindings/display/panel/panel-lvds.txt  | 91 --
 1 file changed, 67 insertions(+), 24 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/panel/panel-lvds.txt 
b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
index 250850a..07795441 100644
--- a/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
+++ b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
@@ -41,7 +41,8 @@ Required nodes:
 
 - panel-timing: See panel-common.txt.
 - ports: See panel-common.txt. These bindings require a single port subnode
-  corresponding to the panel LVDS input.
+  (for a single link display) or two port subnodes (for a dual link display)
+  corresponding to the panel LVDS input(s).
 
 
 LVDS data mappings are defined as follows.
@@ -92,30 +93,72 @@ CTL3: 0
 Example
 ---
 
-panel {
-   compatible = "mitsubishi,aa121td01", "panel-lvds";
-
-   width-mm = <261>;
-   height-mm = <163>;
-
-   data-mapping = "jeida-24";
-
-   panel-timing {
-   /* 1280x800 @60Hz */
-   clock-frequency = <7100>;
-   hactive = <1280>;
-   vactive = <800>;
-   hsync-len = <70>;
-   hfront-porch = <20>;
-   hback-porch = <70>;
-   vsync-len = <5>;
-   vfront-porch = <3>;
-   vback-porch = <15>;
+Single port:
+   panel {
+   compatible = "mitsubishi,aa121td01", "panel-lvds";
+
+   width-mm = <261>;
+   height-mm = <163>;
+
+   data-mapping = "jeida-24";
+
+   panel-timing {
+   /* 1280x800 @60Hz */
+   clock-frequency = <7100>;
+   hactive = <1280>;
+   vactive = <800>;
+   hsync-len = <70>;
+   hfront-porch = <20>;
+   hback-porch = <70>;
+   vsync-len = <5>;
+   vfront-porch = <3>;
+   vback-porch = <15>;
+   };
+
+   port {
+   panel_in: endpoint {
+   remote-endpoint = <&lvds_encoder>;
+   };
+   };
};
 
-   port {
-   panel_in: endpoint {
-   remote-endpoint = <&lvds_encoder>;
+Two ports:
+   panel {
+   compatible = "advantech,idk-2121wr", "panel-lvds";
+
+   width-mm = <476>;
+   height-mm = <268>;
+
+   data-mapping = "vesa-24";
+
+   panel-timing {
+   clock-frequency = <14850>;
+   hactive = <1920>;
+   vactive = <1080>;
+   hsync-len = <44>;
+   hfront-porch = <88>;
+   hback-porch = <148>;
+   vfront-porch = <4>;
+   vback-porch = <36>;
+   vsync-len = <5>;
+   };
+
+   ports {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   port@0 {
+   reg = <0>;
+   lvds0_panel_in: endpoint {
+   remote-endpoint = <&lvds0_out>;
+   };
+   };
+
+   port@1 {
+   reg = <1>;
+   lvds1_panel_in: endpoint {
+   remote-endpoint = <&lvds1_out>;
+   };
+   };
};
};
-};
-- 
2.7.4

[PATCH/RFC 05/12] drm: rcar-du: lvds: Add data swap support

[Fabrizio Castro]

When in vertical stripe mode of operation, there is the option
of swapping even data and odd data on the two LVDS interfaces
used to drive the video output.
Add data swap support by exposing a new DT property named
"renesas,swap-data".

Signed-off-by: Fabrizio Castro 
---
 drivers/gpu/drm/rcar-du/rcar_lvds.c | 23 ---
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c 
b/drivers/gpu/drm/rcar-du/rcar_lvds.c
index 3aeaf9e..c306fab 100644
--- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
+++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
@@ -69,6 +69,7 @@ struct rcar_lvds {
 
struct drm_bridge *companion;
bool dual_link;
+   bool stripe_swap_data;
 };
 
 #define bridge_to_rcar_lvds(bridge) \
@@ -439,12 +440,16 @@ static void rcar_lvds_enable(struct drm_bridge *bridge)
rcar_lvds_write(lvds, LVDCHCR, lvdhcr);
 
if (lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK) {
-   /*
-* Configure vertical stripe based on the mode of operation of
-* the connected device.
-*/
-   rcar_lvds_write(lvds, LVDSTRIPE,
-   lvds->dual_link ? LVDSTRIPE_ST_ON : 0);
+   u32 lvdstripe = 0;
+
+   if (lvds->dual_link)
+   /*
+* Configure vertical stripe based on the mode of
+* operation of the connected device.
+*/
+   lvdstripe = LVDSTRIPE_ST_ON | (lvds->stripe_swap_data ?
+  LVDSTRIPE_ST_SWAP : 0);
+   rcar_lvds_write(lvds, LVDSTRIPE, lvdstripe);
}
 
/*
@@ -770,8 +775,12 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds)
}
}
 
-   if (lvds->dual_link)
+   if (lvds->dual_link) {
+   lvds->stripe_swap_data = of_property_read_bool(
+   lvds->dev->of_node,
+   "renesas,swap-data");
ret = rcar_lvds_parse_dt_companion(lvds);
+   }
 
 done:
of_node_put(local_output);
-- 
2.7.4

[PATCH/RFC 02/12] dt-bindings: display: renesas: lvds: Document renesas,swap-data

[Fabrizio Castro]

R-Car D3, R-Car E3, and RZ/G2E support dual-link mode.
In such a mode, the first LVDS encoder emits even data, and the
second LVDS encoder emits odd data. This patch documents property
renesas,swap-data, used to swap even and odd data around.

Signed-off-by: Fabrizio Castro 
---
 Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt | 5 +
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt 
b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
index dece79e..8980179 100644
--- a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
+++ b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
@@ -52,6 +52,11 @@ Optional properties:
   mandatory for the first LVDS encoder on R-Car D3, R-Car E3, and RZ/G2E SoCs,
   and shall point to the second encoder to be used as a companion in dual-link
   mode. It shall not be set for any other LVDS encoder.
+- renesas,swap-data : when in dual-link mode, the first LVDS encoder normally
+  emits even data, and the second LVDS encoder emits odd data. When property
+  renesas,swap-data is specified, the data emitted by the two encoders will be
+  swapped around. This property can only be used in conjunction with property
+  renesas,companion.
 
 
 Example:
-- 
2.7.4

[PATCH/RFC 04/12] dt-bindings: display: Add bindings for Advantech IDK-2121WR

[Fabrizio Castro]

This panel is handled through the generic lvds-panel bindings,
so only needs its additional compatible specified.

Some panel specific documentation can be found here:
https://buy.advantech.eu/Displays/Embedded-LCD-Kits-High-Brightness/model-IDK-2121WR-K2FHA2E.htm

Signed-off-by: Fabrizio Castro 
---
 .../display/panel/advantech,idk-2121wr.txt | 62 ++
 1 file changed, 62 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.txt

diff --git 
a/Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.txt 
b/Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.txt
new file mode 100644
index 000..70b15b6
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.txt
@@ -0,0 +1,62 @@
+Advantech Co., Ltd. IDK-2121WR 21.5" LVDS panel
+===
+
+Required properties:
+- compatible: should be "advantech,idk-2121wr" followed by "panel-lvds"
+
+This binding is compatible with the lvds-panel binding, which is specified
+in panel-lvds.txt in this directory.
+
+Example
+---
+
+   panel {
+   compatible = "advantech,idk-2121wr", "panel-lvds";
+
+   width-mm = <476>;
+   height-mm = <268>;
+
+   data-mapping = "vesa-24";
+
+   panel-timing {
+   clock-frequency = <14850>;
+   hactive = <1920>;
+   vactive = <1080>;
+   hsync-len = <44>;
+   hfront-porch = <88>;
+   hback-porch = <148>;
+   vfront-porch = <4>;
+   vback-porch = <36>;
+   vsync-len = <5>;
+   };
+
+   ports {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   port@0 {
+   reg = <0>;
+   lvds0_panel_in: endpoint {
+   remote-endpoint = <&lvds0_out>;
+   };
+   };
+
+   port@1 {
+   reg = <1>;
+   lvds1_panel_in: endpoint {
+   remote-endpoint = <&lvds1_out>;
+   };
+   };
+   };
+   };
+
+   backlight: backlight {
+   compatible = "pwm-backlight";
+   pwms = <&pwm5 0 5>;
+
+   brightness-levels = <0 4 8 16 32 64 128 255>;
+   default-brightness-level = <6>;
+
+   power-supply = <®_12p0v>;
+   enable-gpios = <&gpio6 12 GPIO_ACTIVE_HIGH>;
+   };
-- 
2.7.4

[PATCH/RFC 07/12] drm: rcar-du: lvds: Add support for dual link panels

[Fabrizio Castro]

If the display comes with two ports, assume it supports dual
link.

Signed-off-by: Fabrizio Castro 
---
 drivers/gpu/drm/rcar-du/rcar_lvds.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c 
b/drivers/gpu/drm/rcar-du/rcar_lvds.c
index 2d54ae5..97c51c2 100644
--- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
+++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
@@ -751,6 +751,9 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds)
ret = -EPROBE_DEFER;
goto done;
}
+   if (lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK)
+   lvds->dual_link = of_graph_get_endpoint_count(remote)
+   == 2;
}
 
if (lvds->dual_link) {
-- 
2.7.4

[PATCH/RFC 09/12] drm: rcar-du: lvds: Fix companion's mode

[Fabrizio Castro]

The companion encoder needs to be told to use the same
mode as the primary encoder.

Fixes: e9e8798ab7b8 ("drm: rcar-du: lvds: Add support for dual-link mode")
Signed-off-by: Fabrizio Castro 
---
 drivers/gpu/drm/rcar-du/rcar_lvds.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c 
b/drivers/gpu/drm/rcar-du/rcar_lvds.c
index edd63f5..7944ae9 100644
--- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
+++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
@@ -415,8 +415,12 @@ static void rcar_lvds_enable(struct drm_bridge *bridge)
return;
 
/* Enable the companion LVDS encoder in dual-link mode. */
-   if (lvds->dual_link && lvds->companion)
+   if (lvds->dual_link && lvds->companion) {
+   struct rcar_lvds *companion_lvds = bridge_to_rcar_lvds(
+   lvds->companion);
+   companion_lvds->mode = lvds->mode;
lvds->companion->funcs->enable(lvds->companion);
+   }
 
/*
 * Hardcode the channels and control signals routing for now.
-- 
2.7.4

[PATCH/RFC 06/12] drm: rcar-du: lvds: Do not look at ports for identifying bridges

[Fabrizio Castro]

We may be connected to a dual LVDS display, therefore checking
if node != remote_input for identifying bridges is not going to
work anymore.
We could try to match the ports on the remote end to the LVDS
encoders, however the companion LVDS encoder instance doesn't
hold a reference to the primary LVDS encoder instance.
We know we could be connected to either a bridge, or a panel,
therefore look through the registered bridges and panels, until
we have a match.

Signed-off-by: Fabrizio Castro 
---
 drivers/gpu/drm/rcar-du/rcar_lvds.c | 29 +++--
 1 file changed, 3 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c 
b/drivers/gpu/drm/rcar-du/rcar_lvds.c
index c306fab..2d54ae5 100644
--- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
+++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
@@ -711,10 +711,7 @@ static int rcar_lvds_parse_dt_companion(struct rcar_lvds 
*lvds)
 static int rcar_lvds_parse_dt(struct rcar_lvds *lvds)
 {
struct device_node *local_output = NULL;
-   struct device_node *remote_input = NULL;
struct device_node *remote = NULL;
-   struct device_node *node;
-   bool is_bridge = false;
int ret = 0;
 
local_output = of_graph_get_endpoint_by_regs(lvds->dev->of_node, 1, 0);
@@ -742,27 +739,8 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds)
goto done;
}
 
-   remote_input = of_graph_get_remote_endpoint(local_output);
-
-   for_each_endpoint_of_node(remote, node) {
-   if (node != remote_input) {
-   /*
-* We've found one endpoint other than the input, this
-* must be a bridge.
-*/
-   is_bridge = true;
-   of_node_put(node);
-   break;
-   }
-   }
-
-   if (is_bridge) {
-   lvds->next_bridge = of_drm_find_bridge(remote);
-   if (!lvds->next_bridge) {
-   ret = -EPROBE_DEFER;
-   goto done;
-   }
-
+   lvds->next_bridge = of_drm_find_bridge(remote);
+   if (lvds->next_bridge) {
if (lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK)
lvds->dual_link = lvds->next_bridge->timings
? lvds->next_bridge->timings->dual_link
@@ -770,7 +748,7 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds)
} else {
lvds->panel = of_drm_find_panel(remote);
if (IS_ERR(lvds->panel)) {
-   ret = PTR_ERR(lvds->panel);
+   ret = -EPROBE_DEFER;
goto done;
}
}
@@ -784,7 +762,6 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds)
 
 done:
of_node_put(local_output);
-   of_node_put(remote_input);
of_node_put(remote);
 
switch (ret) {
-- 
2.7.4

[PATCH/RFC 08/12] drm: rcar-du: lvds: Fix bridge_to_rcar_lvds

[Fabrizio Castro]

Using name "bridge" for macro bridge_to_rcar_lvds argument doesn't
work when the pointer name used by the caller is not "bridge".
Rename the argument to "bridge_ptr" to allow for any pointer
name.

Fixes: c6a27fa41fab ("drm: rcar-du: Convert LVDS encoder code to bridge driver")
Signed-off-by: Fabrizio Castro 
---
 drivers/gpu/drm/rcar-du/rcar_lvds.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c 
b/drivers/gpu/drm/rcar-du/rcar_lvds.c
index 97c51c2..edd63f5 100644
--- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
+++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
@@ -72,8 +72,8 @@ struct rcar_lvds {
bool stripe_swap_data;
 };
 
-#define bridge_to_rcar_lvds(bridge) \
-   container_of(bridge, struct rcar_lvds, bridge)
+#define bridge_to_rcar_lvds(bridge_ptr) \
+   container_of(bridge_ptr, struct rcar_lvds, bridge)
 
 #define connector_to_rcar_lvds(connector) \
container_of(connector, struct rcar_lvds, connector)
-- 
2.7.4

Re: [PATCH v9 04/18] kunit: test: add kunit_stream a std::stream like logger

[John Ogness]

On 2019-08-01, Brendan Higgins  wrote:
> On Fri, Jul 26, 2019 at 1:31 AM Petr Mladek  wrote:
>> On Thu 2019-07-25 13:21:12, Brendan Higgins wrote:
>>> On Wed, Jul 24, 2019 at 12:31 AM Petr Mladek  wrote:
 On Mon 2019-07-22 16:54:10, Stephen Boyd wrote:
> Quoting Brendan Higgins (2019-07-22 15:30:49)
>> On Mon, Jul 22, 2019 at 1:03 PM Stephen Boyd  wrote:
>>> What's the calling context of the assertions and expectations? I
>>> still don't like the fact that string stream needs to allocate
>>> buffers and throw them into a list somewhere because the calling
>>> context matters there.
>>
>> The calling context is the same as before, which is anywhere.
>
> Ok. That's concerning then.
>
>>> I'd prefer we just wrote directly to the console/log via printk
>>> instead. That way things are simple because we use the existing
>>> buffering path of printk, but maybe there's some benefit to the
>>> string stream that I don't see? Right now it looks like it
>>> builds a string and then dumps it to printk so I'm sort of lost
>>> what the benefit is over just writing directly with printk.
>>
>> It's just buffering it so the whole string gets printed
>> uninterrupted.  If we were to print out piecemeal to printk,
>> couldn't we have another call to printk come in causing it to
>> garble the KUnit message we are in the middle of printing?
>
> Yes, printing piecemeal by calling printk many times could lead to
> interleaving of messages if something else comes in such as an
> interrupt printing something. Printk has some support to hold
> "records" but I'm not sure how that would work here because
> KERN_CONT talks about only being used early on in boot code. I
> haven't looked at printk in detail though so maybe I'm all wrong
> and KERN_CONT just works?

 KERN_CONT does not guarantee that the message will get printed
 together. The pieces get interleaved with messages printed in
 parallel.

 Note that KERN_CONT was originally really meant to be used only
 during boot. It was later used more widely and ended in the best
 effort category.

 There were several attempts to make it more reliable. But it was
 always either too complicated or error prone or both.

 You need to use your own buffering if you rely want perfect output.
 The question is if it is really worth the complexity. Also note
 that any buffering reduces the chance that the messages will reach
 the console.
>>>
>>> Seems like that settles it then. Thanks!
>>>
 BTW: There is a work in progress on a lockless printk ring buffer.
 It will make printk() more secure regarding deadlocks. But it might
 make transparent handling of continuous lines even more tricky.

 I guess that local buffering, before calling printk(), will be
 even more important then. Well, it might really force us to create
 an API for it.
>>>
>>> Cool! Can you CC me on that discussion?
>>
>> Adding John Oggness into CC.
>>
>> John, please CC Brendan Higgins on the patchsets eventually switching
>> printk() into the lockless buffer. The test framework is going to
>> do its own buffering to keep the related messages together.
>>
>> The lockless ringbuffer might make handling of related (partial)
>> lines worse or better. It might justify KUnit's extra buffering
>> or it might allow to get rid of it.
>
> Thanks for CC'ing me on the printk ringbuffer thread. It looks like it
> actually probably won't affect my needs for KUnit logging. The biggest
> reason I need some sort of buffering system is to be able to compose
> messages piece meal into a single message that will be printed out to
> the user as a single message with no messages from other printk
> callers printed out in the middle of mine.

printk has this same requirement for its CONT messages. You can read
about how I propose to implement that here[0], using a separate prb
ringbuffer for buffered storage until all the pieces are available.

It is not my goal that multiple subsystems start making use of the prb
ringbuffer. However, its features can be attractive if you don't want to
worry about multiple writers/readers or context (including NMI). Before
writing "yet another ringbuffer" [1] it might be worth the effort to at
least see if one of the existing implementations can work (or be
extended to work) for you.

John Ogness

[0] https://lkml.kernel.org/r/87imt2bl0k@linutronix.de
[1] https://lwn.net/Articles/789603/

> The prb does look interesting; however, it appears that to get the
> semantics that I need, I would have to put my entire message in a
> single data block and would consequently need to know the size of my
> message a priori, which is problematic. Consequently, it seems as
> though I will probably need to compose my entire message using my own
> buffering system.
>
 Note that stroring the messages into the pr

[Bug 111281] When the s admin try to create different types of question in various difficulty level by single upload mode database error occurred

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=111281

dhrisya  changed:

   What|Removed |Added

 Status|ASSIGNED|NEW

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH/RFC 02/12] dt-bindings: display: renesas: lvds: Document renesas,swap-data

[Laurent Pinchart]

Hi Fabrizio,

Thank you for the patch.

On Fri, Aug 02, 2019 at 08:33:59AM +0100, Fabrizio Castro wrote:
> R-Car D3, R-Car E3, and RZ/G2E support dual-link mode.
> In such a mode, the first LVDS encoder emits even data, and the
> second LVDS encoder emits odd data. This patch documents property
> renesas,swap-data, used to swap even and odd data around.
> 
> Signed-off-by: Fabrizio Castro 
> ---
>  Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git 
> a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt 
> b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
> index dece79e..8980179 100644
> --- a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
> +++ b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
> @@ -52,6 +52,11 @@ Optional properties:
>mandatory for the first LVDS encoder on R-Car D3, R-Car E3, and RZ/G2E 
> SoCs,
>and shall point to the second encoder to be used as a companion in 
> dual-link
>mode. It shall not be set for any other LVDS encoder.
> +- renesas,swap-data : when in dual-link mode, the first LVDS encoder normally
> +  emits even data, and the second LVDS encoder emits odd data. When property
> +  renesas,swap-data is specified, the data emitted by the two encoders will 
> be
> +  swapped around. This property can only be used in conjunction with property
> +  renesas,companion.

>From an LVDS encoder point of view this is more a configuration option
than a description of the hardware. Wouldn't it be better for the LVDS
sink to report which of the odd or even pixels it expects on each of its
endpoints ? The LVDS encoder driver could then query that at runtime and
configure itself accordingly. Ideally this should be queried through the
drm_bridge_timings structure (or through a similar mean), not through
DT. An LVDS sink that has a fixed mapping of odd/even pixels to
endpoints wouldn't need the information to be specified in DT at all.

>  
>  
>  Example:

-- 
Regards,

Laurent Pinchart

Re: [PATCH/RFC 01/12] dt-bindings: display: renesas: lvds: RZ/G2E needs renesas,companion too

[Laurent Pinchart]

Hello Fabrizio,

Thank you for the patch.

On Fri, Aug 02, 2019 at 08:33:58AM +0100, Fabrizio Castro wrote:
> Document RZ/G2E support for property renesas,companion.
> 
> Signed-off-by: Fabrizio Castro 

Reviewed-by: Laurent Pinchart 

and taken in my tree.

> ---
>  Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git 
> a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt 
> b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
> index c6a196d..dece79e 100644
> --- a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
> +++ b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
> @@ -49,9 +49,9 @@ Each port shall have a single endpoint.
>  Optional properties:
>  
>  - renesas,companion : phandle to the companion LVDS encoder. This property is
> -  mandatory for the first LVDS encoder on D3 and E3 SoCs, and shall point to
> -  the second encoder to be used as a companion in dual-link mode. It shall 
> not
> -  be set for any other LVDS encoder.
> +  mandatory for the first LVDS encoder on R-Car D3, R-Car E3, and RZ/G2E 
> SoCs,
> +  and shall point to the second encoder to be used as a companion in 
> dual-link
> +  mode. It shall not be set for any other LVDS encoder.
>  
>  
>  Example:

-- 
Regards,

Laurent Pinchart

[Bug 111281] When the s admin try to create different types of question in various difficulty level by single upload mode database error occurred

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=111281

Andre Klapper  changed:

   What|Removed |Added

 Status|NEW |RESOLVED
  Group||spam
 Resolution|--- |INVALID
Product|DRI |Spam
  Component|General |Two

--- Comment #1 from Andre Klapper  ---
Go away and test somewhere else.

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm/komeda: Initialize and enable output polling on Komeda

[Lowry Li (Arm Technology China)]

From: "Lowry Li (Arm Technology China)" 

Initialize and enable output polling on Komeda.

Changes since v1:
1. Enable the polling before registering the driver;
2. Disable the polling after unregistering the driver.

Signed-off-by: Lowry Li (Arm Technology China) 
---
 drivers/gpu/drm/arm/display/komeda/komeda_kms.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c 
b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
index 419a8b0..25716a30 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
@@ -15,6 +15,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "komeda_dev.h"
 #include "komeda_framebuffer.h"
@@ -315,6 +316,8 @@ struct komeda_kms_dev *komeda_kms_attach(struct komeda_dev 
*mdev)
 
drm->irq_enabled = true;
 
+   drm_kms_helper_poll_init(drm);
+
err = drm_dev_register(drm, 0);
if (err)
goto cleanup_mode_config;
@@ -338,6 +341,7 @@ void komeda_kms_detach(struct komeda_kms_dev *kms)
drm->irq_enabled = false;
mdev->funcs->disable_irq(mdev);
drm_dev_unregister(drm);
+   drm_kms_helper_poll_fini(drm);
component_unbind_all(mdev->dev, drm);
komeda_kms_cleanup_private_objs(kms);
drm_mode_config_cleanup(drm);
-- 
1.9.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH/RFC 03/12] dt-bindings: panel: lvds: Add dual-link LVDS display support

[Laurent Pinchart]

Hi Fabrizio,

Thank you for the patch.

On Fri, Aug 02, 2019 at 08:34:00AM +0100, Fabrizio Castro wrote:
> Dual-link LVDS displays have two ports, therefore document this
> with the bindings.
> 
> Signed-off-by: Fabrizio Castro 
> ---
>  .../bindings/display/panel/panel-lvds.txt  | 91 
> --
>  1 file changed, 67 insertions(+), 24 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/display/panel/panel-lvds.txt 
> b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> index 250850a..07795441 100644
> --- a/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> +++ b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> @@ -41,7 +41,8 @@ Required nodes:
>  
>  - panel-timing: See panel-common.txt.
>  - ports: See panel-common.txt. These bindings require a single port subnode
> -  corresponding to the panel LVDS input.
> +  (for a single link display) or two port subnodes (for a dual link display)
> +  corresponding to the panel LVDS input(s).

I think you should expand this a bit to explain what the ports
correspond to in the dual link mode.

>  LVDS data mappings are defined as follows.
> @@ -92,30 +93,72 @@ CTL3: 0
>  Example
>  ---
>  
> -panel {
> - compatible = "mitsubishi,aa121td01", "panel-lvds";
> -
> - width-mm = <261>;
> - height-mm = <163>;
> -
> - data-mapping = "jeida-24";
> -
> - panel-timing {
> - /* 1280x800 @60Hz */
> - clock-frequency = <7100>;
> - hactive = <1280>;
> - vactive = <800>;
> - hsync-len = <70>;
> - hfront-porch = <20>;
> - hback-porch = <70>;
> - vsync-len = <5>;
> - vfront-porch = <3>;
> - vback-porch = <15>;
> +Single port:
> + panel {
> + compatible = "mitsubishi,aa121td01", "panel-lvds";
> +
> + width-mm = <261>;
> + height-mm = <163>;
> +
> + data-mapping = "jeida-24";
> +
> + panel-timing {
> + /* 1280x800 @60Hz */
> + clock-frequency = <7100>;
> + hactive = <1280>;
> + vactive = <800>;
> + hsync-len = <70>;
> + hfront-porch = <20>;
> + hback-porch = <70>;
> + vsync-len = <5>;
> + vfront-porch = <3>;
> + vback-porch = <15>;
> + };
> +
> + port {
> + panel_in: endpoint {
> + remote-endpoint = <&lvds_encoder>;
> + };
> + };
>   };
>  
> - port {
> - panel_in: endpoint {
> - remote-endpoint = <&lvds_encoder>;
> +Two ports:
> + panel {
> + compatible = "advantech,idk-2121wr", "panel-lvds";
> +
> + width-mm = <476>;
> + height-mm = <268>;
> +
> + data-mapping = "vesa-24";
> +
> + panel-timing {
> + clock-frequency = <14850>;
> + hactive = <1920>;
> + vactive = <1080>;
> + hsync-len = <44>;
> + hfront-porch = <88>;
> + hback-porch = <148>;
> + vfront-porch = <4>;
> + vback-porch = <36>;
> + vsync-len = <5>;
> + };
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> + lvds0_panel_in: endpoint {

I would name the label panel_in0 and panel_in1 below to have a common
prefix showing that both refer to the same panel.

> + remote-endpoint = <&lvds0_out>;
> + };
> + };
> +
> + port@1 {
> + reg = <1>;
> + lvds1_panel_in: endpoint {
> + remote-endpoint = <&lvds1_out>;
> + };
> + };
>   };
>   };
> -};

-- 
Regards,

Laurent Pinchart

Re: [PATCH] drm/virtio: kick vq outside of the vq lock

[Gerd Hoffmann]

> @@ -291,11 +291,9 @@ static int virtio_gpu_queue_ctrl_buffer_locked(struct 
> virtio_gpu_device *vgdev,
>   trace_virtio_gpu_cmd_queue(vq,
>   (struct virtio_gpu_ctrl_hdr *)vbuf->buf);
>  
> - virtqueue_kick(vq);
> + ret = virtqueue_kick_prepare(vq);
>   }
>  
> - if (!ret)
> - ret = vq->num_free;

Hmm.  Change looks unrelated.

On a closer look it seems this is basically dead code.
virtio_gpu_queue_ctrl_buffer_locked is called by
virtio_gpu_queue_ctrl_buffer and virtio_gpu_queue_fenced_ctrl_buffer.
The call sites for these two functions all ignore the return value.

So it is a valid change, but it should go to a separate patch.  And
while being at it virtio_gpu_queue_ctrl_buffer and
virtio_gpu_queue_fenced_ctrl_buffer can be changed to return void.

Otherwise the patch looks fine.  Nice analysis btw.

cheers,
  Gerd

Re: [PATCH/RFC 04/12] dt-bindings: display: Add bindings for Advantech IDK-2121WR

[Laurent Pinchart]

Hi Fabrizio,

Thank you for the patch.

On Fri, Aug 02, 2019 at 08:34:01AM +0100, Fabrizio Castro wrote:
> This panel is handled through the generic lvds-panel bindings,
> so only needs its additional compatible specified.
> 
> Some panel specific documentation can be found here:

s/panel specific/panel-specific/

> https://buy.advantech.eu/Displays/Embedded-LCD-Kits-High-Brightness/model-IDK-2121WR-K2FHA2E.htm
> 
> Signed-off-by: Fabrizio Castro 
> ---
>  .../display/panel/advantech,idk-2121wr.txt | 62 
> ++
>  1 file changed, 62 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.txt
> 
> diff --git 
> a/Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.txt 
> b/Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.txt
> new file mode 100644
> index 000..70b15b6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.txt
> @@ -0,0 +1,62 @@
> +Advantech Co., Ltd. IDK-2121WR 21.5" LVDS panel
> +===
> +
> +Required properties:
> +- compatible: should be "advantech,idk-2121wr" followed by "panel-lvds"
> +
> +This binding is compatible with the lvds-panel binding, which is specified
> +in panel-lvds.txt in this directory.

How about adding "The panel operates in dual-link mode and thus requires
two port nodes." ?

> +
> +Example
> +---
> +
> + panel {
> + compatible = "advantech,idk-2121wr", "panel-lvds";
> +
> + width-mm = <476>;
> + height-mm = <268>;
> +
> + data-mapping = "vesa-24";
> +
> + panel-timing {
> + clock-frequency = <14850>;
> + hactive = <1920>;
> + vactive = <1080>;
> + hsync-len = <44>;
> + hfront-porch = <88>;
> + hback-porch = <148>;
> + vfront-porch = <4>;
> + vback-porch = <36>;
> + vsync-len = <5>;
> + };
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> + lvds0_panel_in: endpoint {
> + remote-endpoint = <&lvds0_out>;
> + };
> + };
> +
> + port@1 {
> + reg = <1>;
> + lvds1_panel_in: endpoint {
> + remote-endpoint = <&lvds1_out>;
> + };
> + };
> + };
> + };
> +
> + backlight: backlight {
> + compatible = "pwm-backlight";
> + pwms = <&pwm5 0 5>;
> +
> + brightness-levels = <0 4 8 16 32 64 128 255>;
> + default-brightness-level = <6>;
> +
> + power-supply = <®_12p0v>;
> + enable-gpios = <&gpio6 12 GPIO_ACTIVE_HIGH>;
> + };

I think you can drop the backlight here, it's a bit out of scope.

-- 
Regards,

Laurent Pinchart
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH/RFC 05/12] drm: rcar-du: lvds: Add data swap support

[Laurent Pinchart]

Hi Fabrizio,

Thank you for the patch.

On Fri, Aug 02, 2019 at 08:34:02AM +0100, Fabrizio Castro wrote:
> When in vertical stripe mode of operation, there is the option
> of swapping even data and odd data on the two LVDS interfaces
> used to drive the video output.
> Add data swap support by exposing a new DT property named
> "renesas,swap-data".
> 
> Signed-off-by: Fabrizio Castro 
> ---
>  drivers/gpu/drm/rcar-du/rcar_lvds.c | 23 ---
>  1 file changed, 16 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c 
> b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> index 3aeaf9e..c306fab 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> @@ -69,6 +69,7 @@ struct rcar_lvds {
>  
>   struct drm_bridge *companion;
>   bool dual_link;
> + bool stripe_swap_data;
>  };
>  
>  #define bridge_to_rcar_lvds(bridge) \
> @@ -439,12 +440,16 @@ static void rcar_lvds_enable(struct drm_bridge *bridge)
>   rcar_lvds_write(lvds, LVDCHCR, lvdhcr);
>  
>   if (lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK) {
> - /*
> -  * Configure vertical stripe based on the mode of operation of
> -  * the connected device.
> -  */
> - rcar_lvds_write(lvds, LVDSTRIPE,
> - lvds->dual_link ? LVDSTRIPE_ST_ON : 0);
> + u32 lvdstripe = 0;
> +
> + if (lvds->dual_link)
> + /*
> +  * Configure vertical stripe based on the mode of
> +  * operation of the connected device.
> +  */
> + lvdstripe = LVDSTRIPE_ST_ON | (lvds->stripe_swap_data ?
> +LVDSTRIPE_ST_SWAP : 0);

Would the following be simpler ?

lvdstripe = (lvds->dual_link ? LVDSTRIPE_ST_ON : 0)
  | (lvds->stripe_swap_data ? LVDSTRIPE_ST_SWAP : 0);

> + rcar_lvds_write(lvds, LVDSTRIPE, lvdstripe);
>   }
>  
>   /*
> @@ -770,8 +775,12 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds)
>   }
>   }
>  
> - if (lvds->dual_link)
> + if (lvds->dual_link) {
> + lvds->stripe_swap_data = of_property_read_bool(
> + lvds->dev->of_node,
> + "renesas,swap-data");
>   ret = rcar_lvds_parse_dt_companion(lvds);
> + }

As explained in the review of the corresponding DT bindings, I think
this should be queried from the remote device rather than specified in
DT.

>  
>  done:
>   of_node_put(local_output);

-- 
Regards,

Laurent Pinchart

Re: [PATCH 00/34] put_user_pages(): miscellaneous call sites

[Peter Zijlstra]

On Thu, Aug 01, 2019 at 07:16:19PM -0700, john.hubb...@gmail.com wrote:

> This is part a tree-wide conversion, as described in commit fc1d8e7cca2d
> ("mm: introduce put_user_page*(), placeholder versions"). That commit
> has an extensive description of the problem and the planned steps to
> solve it, but the highlites are:

That is one horridly mangled Changelog there :-/ It looks like it's
partially duplicated.

Anyway; no objections to any of that, but I just wanted to mention that
there are other problems with long term pinning that haven't been
mentioned, notably they inhibit compaction.

A long time ago I proposed an interface to mark pages as pinned, such
that we could run compaction before we actually did the pinning.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm/komeda: Initialize and enable output polling on Komeda

[Lowry Li (Arm Technology China)]

From: "Lowry Li (Arm Technology China)" 

Initialize and enable output polling on Komeda.

Changes since v1:
1. Enable the polling before registering the driver;
2. Disable the polling after unregistering the driver.

Changes since v2:
1. If driver register failed, disable the polling.

Signed-off-by: Lowry Li (Arm Technology China) 
---
 drivers/gpu/drm/arm/display/komeda/komeda_kms.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c 
b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
index 419a8b0..d50e75f 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
@@ -15,6 +15,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "komeda_dev.h"
 #include "komeda_framebuffer.h"
@@ -315,6 +316,8 @@ struct komeda_kms_dev *komeda_kms_attach(struct komeda_dev 
*mdev)
 
drm->irq_enabled = true;
 
+   drm_kms_helper_poll_init(drm);
+
err = drm_dev_register(drm, 0);
if (err)
goto cleanup_mode_config;
@@ -322,6 +325,7 @@ struct komeda_kms_dev *komeda_kms_attach(struct komeda_dev 
*mdev)
return kms;
 
 cleanup_mode_config:
+   drm_kms_helper_poll_fini(drm);
drm->irq_enabled = false;
drm_mode_config_cleanup(drm);
komeda_kms_cleanup_private_objs(kms);
@@ -338,6 +342,7 @@ void komeda_kms_detach(struct komeda_kms_dev *kms)
drm->irq_enabled = false;
mdev->funcs->disable_irq(mdev);
drm_dev_unregister(drm);
+   drm_kms_helper_poll_fini(drm);
component_unbind_all(mdev->dev, drm);
komeda_kms_cleanup_private_objs(kms);
drm_mode_config_cleanup(drm);
-- 
1.9.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH/RFC 06/12] drm: rcar-du: lvds: Do not look at ports for identifying bridges

[Laurent Pinchart]

Hi Fabrizio,

Thank you for the patch.

On Fri, Aug 02, 2019 at 08:34:03AM +0100, Fabrizio Castro wrote:
> We may be connected to a dual LVDS display, therefore checking
> if node != remote_input for identifying bridges is not going to
> work anymore.
> We could try to match the ports on the remote end to the LVDS
> encoders, however the companion LVDS encoder instance doesn't
> hold a reference to the primary LVDS encoder instance.
> We know we could be connected to either a bridge, or a panel,
> therefore look through the registered bridges and panels, until
> we have a match.
> 
> Signed-off-by: Fabrizio Castro 
> ---
>  drivers/gpu/drm/rcar-du/rcar_lvds.c | 29 +++--
>  1 file changed, 3 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c 
> b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> index c306fab..2d54ae5 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> @@ -711,10 +711,7 @@ static int rcar_lvds_parse_dt_companion(struct rcar_lvds 
> *lvds)
>  static int rcar_lvds_parse_dt(struct rcar_lvds *lvds)
>  {
>   struct device_node *local_output = NULL;
> - struct device_node *remote_input = NULL;
>   struct device_node *remote = NULL;
> - struct device_node *node;
> - bool is_bridge = false;
>   int ret = 0;
>  
>   local_output = of_graph_get_endpoint_by_regs(lvds->dev->of_node, 1, 0);
> @@ -742,27 +739,8 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds)
>   goto done;
>   }
>  
> - remote_input = of_graph_get_remote_endpoint(local_output);
> -
> - for_each_endpoint_of_node(remote, node) {
> - if (node != remote_input) {
> - /*
> -  * We've found one endpoint other than the input, this
> -  * must be a bridge.
> -  */
> - is_bridge = true;
> - of_node_put(node);
> - break;
> - }
> - }
> -
> - if (is_bridge) {
> - lvds->next_bridge = of_drm_find_bridge(remote);
> - if (!lvds->next_bridge) {
> - ret = -EPROBE_DEFER;
> - goto done;
> - }
> -
> + lvds->next_bridge = of_drm_find_bridge(remote);

How about using drm_of_find_panel_or_bridge() ?

> + if (lvds->next_bridge) {
>   if (lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK)
>   lvds->dual_link = lvds->next_bridge->timings
>   ? lvds->next_bridge->timings->dual_link
> @@ -770,7 +748,7 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds)
>   } else {
>   lvds->panel = of_drm_find_panel(remote);
>   if (IS_ERR(lvds->panel)) {
> - ret = PTR_ERR(lvds->panel);
> + ret = -EPROBE_DEFER;
>   goto done;
>   }
>   }
> @@ -784,7 +762,6 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds)
>  
>  done:
>   of_node_put(local_output);
> - of_node_put(remote_input);
>   of_node_put(remote);
>  
>   switch (ret) {

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 1/8] drm/etnaviv: simplify unbind checks

[Guido Günther]

Hi,
On Fri, Jul 05, 2019 at 07:17:20PM +0200, Lucas Stach wrote:
> Remember if the GPU has been sucessfully initialized. Only in that case
> do we need to clean up various structures in the unbind path. If the
> GPU hasn't been sucessfully initialized all the cleanups should happen
> in the failure paths of the init function.
> 
> Signed-off-by: Lucas Stach 
> ---
>  drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 20 +++-
>  drivers/gpu/drm/etnaviv/etnaviv_gpu.h |  1 +
>  2 files changed, 8 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c 
> b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> index 4822549500ee..e84a0ed904aa 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> @@ -799,17 +799,16 @@ int etnaviv_gpu_init(struct etnaviv_gpu *gpu)
>   pm_runtime_mark_last_busy(gpu->dev);
>   pm_runtime_put_autosuspend(gpu->dev);
>  
> + gpu->initialized = true;
> +
>   return 0;
>  
>  free_buffer:
>   etnaviv_cmdbuf_free(&gpu->buffer);
> - gpu->buffer.suballoc = NULL;
>  destroy_suballoc:
>   etnaviv_cmdbuf_suballoc_destroy(gpu->cmdbuf_suballoc);
> - gpu->cmdbuf_suballoc = NULL;
>  destroy_iommu:
>   etnaviv_iommu_destroy(gpu->mmu);
> - gpu->mmu = NULL;
>  fail:
>   pm_runtime_mark_last_busy(gpu->dev);
>   pm_runtime_put_autosuspend(gpu->dev);
> @@ -1521,7 +1520,7 @@ int etnaviv_gpu_wait_idle(struct etnaviv_gpu *gpu, 
> unsigned int timeout_ms)
>  
>  static int etnaviv_gpu_hw_suspend(struct etnaviv_gpu *gpu)
>  {
> - if (gpu->buffer.suballoc) {
> + if (gpu->initialized) {
>   /* Replace the last WAIT with END */
>   mutex_lock(&gpu->lock);
>   etnaviv_buffer_end(gpu);
> @@ -1680,19 +1679,14 @@ static void etnaviv_gpu_unbind(struct device *dev, 
> struct device *master,
>   etnaviv_gpu_hw_suspend(gpu);
>  #endif
>  
> - if (gpu->buffer.suballoc)
> + if (gpu->initialized) {
>   etnaviv_cmdbuf_free(&gpu->buffer);
> -
> - if (gpu->cmdbuf_suballoc) {
>   etnaviv_cmdbuf_suballoc_destroy(gpu->cmdbuf_suballoc);
> - gpu->cmdbuf_suballoc = NULL;
> - }
> -
> - if (gpu->mmu) {
>   etnaviv_iommu_destroy(gpu->mmu);
> - gpu->mmu = NULL;
> + gpu->initialized = false;
>   }
>  
> +

Maybe drop this line, otherwise:

Reviewed-by: Guido Günther  

>   gpu->drm = NULL;
>   idr_destroy(&gpu->fence_idr);
>  
> @@ -1827,7 +1821,7 @@ static int etnaviv_gpu_rpm_resume(struct device *dev)
>   return ret;
>  
>   /* Re-initialise the basic hardware state */
> - if (gpu->drm && gpu->buffer.suballoc) {
> + if (gpu->drm && gpu->initialized) {
>   ret = etnaviv_gpu_hw_resume(gpu);
>   if (ret) {
>   etnaviv_gpu_clk_disable(gpu);
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h 
> b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> index 9bcf151f706b..b06c7c98d522 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> @@ -99,6 +99,7 @@ struct etnaviv_gpu {
>   enum etnaviv_sec_mode sec_mode;
>   struct workqueue_struct *wq;
>   struct drm_gpu_scheduler sched;
> + bool initialized;
>  
>   /* 'ring'-buffer: */
>   struct etnaviv_cmdbuf buffer;
> -- 
> 2.20.1
> 
> ___
> etnaviv mailing list
> etna...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/etnaviv
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [LKP] [drm/mgag200] 90f479ae51: vm-scalability.median -18.8% regression

[Thomas Zimmermann]

Hi

Am 01.08.19 um 15:30 schrieb Michel Dänzer:
> On 2019-08-01 8:19 a.m., Rong Chen wrote:
>> Hi,
>>
>> On 7/31/19 6:21 PM, Michel Dänzer wrote:
>>> On 2019-07-31 11:25 a.m., Huang, Ying wrote:
 Hi, Daniel,

 Daniel Vetter  writes:

> On Tue, Jul 30, 2019 at 10:27 PM Dave Airlie  wrote:
>> On Wed, 31 Jul 2019 at 05:00, Daniel Vetter  wrote:
>>> On Tue, Jul 30, 2019 at 8:50 PM Thomas Zimmermann
>>>  wrote:
 Hi

 Am 30.07.19 um 20:12 schrieb Daniel Vetter:
> On Tue, Jul 30, 2019 at 7:50 PM Thomas Zimmermann
>  wrote:
>> Am 29.07.19 um 11:51 schrieb kernel test robot:
>>> Greeting,
>>>
>>> FYI, we noticed a -18.8% regression of vm-scalability.median
>>> due to commit:>
>>>
>>> commit: 90f479ae51afa45efab97afdde9b94b9660dd3e4
>>> ("drm/mgag200: Replace struct mga_fbdev with generic
>>> framebuffer emulation")
>>> https://kernel.googlesource.com/pub/scm/linux/kernel/git/next/linux-next.git
>>> master
>> Daniel, Noralf, we may have to revert this patch.
>>
>> I expected some change in display performance, but not in VM.
>> Since it's
>> a server chipset, probably no one cares much about display
>> performance.
>> So that seemed like a good trade-off for re-using shared code.
>>
>> Part of the patch set is that the generic fb emulation now maps
>> and
>> unmaps the fbdev BO when updating the screen. I guess that's
>> the cause
>> of the performance regression. And it should be visible with other
>> drivers as well if they use a shadow FB for fbdev emulation.
> For fbcon we should need to do any maps/unamps at all, this is
> for the
> fbdev mmap support only. If the testcase mentioned here tests fbdev
> mmap handling it's pretty badly misnamed :-) And as long as you
> don't
> have an fbdev mmap there shouldn't be any impact at all.
 The ast and mgag200 have only a few MiB of VRAM, so we have to
 get the
 fbdev BO out if it's not being displayed. If not being mapped, it
 can be
 evicted and make room for X, etc.

 To make this work, the BO's memory is mapped and unmapped in
 drm_fb_helper_dirty_work() before being updated from the shadow
 FB. [1]
 That fbdev mapping is established on each screen update, more or
 less.
  From my (yet unverified) understanding, this causes the performance
 regression in the VM code.

 The original code in mgag200 used to kmap the fbdev BO while it's
 being
 displayed; [2] and the drawing code only mapped it when necessary
 (i.e.,
 not being display). [3]
>>> Hm yeah, this vmap/vunmap is going to be pretty bad. We indeed should
>>> cache this.
>>>
 I think this could be added for VRAM helpers as well, but it's
 still a
 workaround and non-VRAM drivers might also run into such a
 performance
 regression if they use the fbdev's shadow fb.
>>> Yeah agreed, fbdev emulation should try to cache the vmap.
>>>
 Noralf mentioned that there are plans for other DRM clients
 besides the
 console. They would as well run into similar problems.

>> The thing is that we'd need another generic fbdev emulation for
>> ast and
>> mgag200 that handles this issue properly.
> Yeah I dont think we want to jump the gun here.  If you can try to
> repro locally and profile where we're wasting cpu time I hope that
> should sched a light what's going wrong here.
 I don't have much time ATM and I'm not even officially at work until
 late Aug. I'd send you the revert and investigate later. I agree
 that
 using generic fbdev emulation would be preferable.
>>> Still not sure that's the right thing to do really. Yes it's a
>>> regression, but vm testcases shouldn run a single line of fbcon or
>>> drm
>>> code. So why this is impacted so heavily by a silly drm change is
>>> very
>>> confusing to me. We might be papering over a deeper and much more
>>> serious issue ...
>> It's a regression, the right thing is to revert first and then work
>> out the right thing to do.
> Sure, but I have no idea whether the testcase is doing something
> reasonable. If it's accidentally testing vm scalability of fbdev and
> there's no one else doing something this pointless, then it's not a
> real bug. Plus I think we're shooting the messenger here.
>
>> It's likely the test runs on the console and printfs stuff out
>> while running.
> But why did we not regress the world if a few prints on the console
> have such a huge impact? We di

[PATCH 29/34] mm/process_vm_access.c: convert put_page() to put_user_page*()

[john . hubbard]

From: John Hubbard 

For pages that were retained via get_user_pages*(), release those pages
via the new put_user_page*() routines, instead of via put_page() or
release_pages().

This is part a tree-wide conversion, as described in commit fc1d8e7cca2d
("mm: introduce put_user_page*(), placeholder versions").

Cc: Al Viro 
Cc: Andrea Arcangeli 
Cc: Christopher Yeoh 
Cc: Dave Hansen 
Cc: Heiko Carstens 
Cc: Ingo Molnar 
Cc: Jann Horn 
Cc: Lorenzo Stoakes 
Cc: Mathieu Desnoyers 
Cc: Mike Rapoport 
Cc: Rashika Kheria 
Signed-off-by: John Hubbard 
---
 mm/process_vm_access.c | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/mm/process_vm_access.c b/mm/process_vm_access.c
index 357aa7bef6c0..4d29d54ec93f 100644
--- a/mm/process_vm_access.c
+++ b/mm/process_vm_access.c
@@ -96,7 +96,7 @@ static int process_vm_rw_single_vec(unsigned long addr,
flags |= FOLL_WRITE;
 
while (!rc && nr_pages && iov_iter_count(iter)) {
-   int pages = min(nr_pages, max_pages_per_loop);
+   int pinned_pages = min(nr_pages, max_pages_per_loop);
int locked = 1;
size_t bytes;
 
@@ -106,14 +106,15 @@ static int process_vm_rw_single_vec(unsigned long addr,
 * current/current->mm
 */
down_read(&mm->mmap_sem);
-   pages = get_user_pages_remote(task, mm, pa, pages, flags,
- process_pages, NULL, &locked);
+   pinned_pages = get_user_pages_remote(task, mm, pa, pinned_pages,
+flags, process_pages, NULL,
+&locked);
if (locked)
up_read(&mm->mmap_sem);
-   if (pages <= 0)
+   if (pinned_pages <= 0)
return -EFAULT;
 
-   bytes = pages * PAGE_SIZE - start_offset;
+   bytes = pinned_pages * PAGE_SIZE - start_offset;
if (bytes > len)
bytes = len;
 
@@ -122,10 +123,9 @@ static int process_vm_rw_single_vec(unsigned long addr,
 vm_write);
len -= bytes;
start_offset = 0;
-   nr_pages -= pages;
-   pa += pages * PAGE_SIZE;
-   while (pages)
-   put_page(process_pages[--pages]);
+   nr_pages -= pinned_pages;
+   pa += pinned_pages * PAGE_SIZE;
+   put_user_pages(process_pages, pinned_pages);
}
 
return rc;
-- 
2.22.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 06/34] drm/i915: convert put_page() to put_user_page*()

[john . hubbard]

From: John Hubbard 

For pages that were retained via get_user_pages*(), release those pages
via the new put_user_page*() routines, instead of via put_page() or
release_pages().

This is part a tree-wide conversion, as described in commit fc1d8e7cca2d
("mm: introduce put_user_page*(), placeholder versions").

Note that this effectively changes the code's behavior in
i915_gem_userptr_put_pages(): it now calls set_page_dirty_lock(),
instead of set_page_dirty(). This is probably more accurate.

As Christophe Hellwig put it, "set_page_dirty() is only safe if we are
dealing with a file backed page where we have reference on the inode it
hangs off." [1]

[1] https://lore.kernel.org/r/20190723153640.gb...@lst.de

Cc: Jani Nikula 
Cc: Joonas Lahtinen 
Cc: Rodrigo Vivi 
Cc: David Airlie 
Cc: intel-...@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: John Hubbard 
---
 drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c 
b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
index 528b61678334..c18008d3cc2a 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
@@ -527,7 +527,7 @@ __i915_gem_userptr_get_pages_worker(struct work_struct 
*_work)
}
mutex_unlock(&obj->mm.lock);
 
-   release_pages(pvec, pinned);
+   put_user_pages(pvec, pinned);
kvfree(pvec);
 
i915_gem_object_put(obj);
@@ -640,7 +640,7 @@ static int i915_gem_userptr_get_pages(struct 
drm_i915_gem_object *obj)
__i915_gem_userptr_set_active(obj, true);
 
if (IS_ERR(pages))
-   release_pages(pvec, pinned);
+   put_user_pages(pvec, pinned);
kvfree(pvec);
 
return PTR_ERR_OR_ZERO(pages);
@@ -663,11 +663,8 @@ i915_gem_userptr_put_pages(struct drm_i915_gem_object *obj,
i915_gem_gtt_finish_pages(obj, pages);
 
for_each_sgt_page(page, sgt_iter, pages) {
-   if (obj->mm.dirty)
-   set_page_dirty(page);
-
mark_page_accessed(page);
-   put_page(page);
+   put_user_pages_dirty_lock(&page, 1, obj->mm.dirty);
}
obj->mm.dirty = false;
 
-- 
2.22.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v19 00/15] arm64: untag user pointers passed to the kernel

[Andrey Konovalov]

On Thu, Aug 1, 2019 at 2:11 PM Kevin Brodsky  wrote:
>
> On 31/07/2019 17:50, Dave Hansen wrote:
> > On 7/23/19 10:58 AM, Andrey Konovalov wrote:
> >> The mmap and mremap (only new_addr) syscalls do not currently accept
> >> tagged addresses. Architectures may interpret the tag as a background
> >> colour for the corresponding vma.
> > What the heck is a "background colour"? :)
>
> Good point, this is some jargon that we started using for MTE, the idea being 
> that
> the kernel could set a tag value (specified during mmap()) as "background 
> colour" for
> anonymous pages allocated in that range.
>
> Anyway, this patch series is not about MTE. Andrey, for v20 (if any), I think 
> it's
> best to drop this last sentence to avoid any confusion.

Sure, thanks!

>
> Kevin
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 30/34] crypt: convert put_page() to put_user_page*()

[john . hubbard]

From: John Hubbard 

For pages that were retained via get_user_pages*(), release those pages
via the new put_user_page*() routines, instead of via put_page() or
release_pages().

This is part a tree-wide conversion, as described in commit fc1d8e7cca2d
("mm: introduce put_user_page*(), placeholder versions").

Cc: Herbert Xu 
Cc: David S. Miller 
Cc: linux-cry...@vger.kernel.org
Signed-off-by: John Hubbard 
---
 crypto/af_alg.c | 7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/crypto/af_alg.c b/crypto/af_alg.c
index 879cf23f7489..edd358ea64da 100644
--- a/crypto/af_alg.c
+++ b/crypto/af_alg.c
@@ -428,10 +428,7 @@ static void af_alg_link_sg(struct af_alg_sgl *sgl_prev,
 
 void af_alg_free_sg(struct af_alg_sgl *sgl)
 {
-   int i;
-
-   for (i = 0; i < sgl->npages; i++)
-   put_page(sgl->pages[i]);
+   put_user_pages(sgl->pages, sgl->npages);
 }
 EXPORT_SYMBOL_GPL(af_alg_free_sg);
 
@@ -668,7 +665,7 @@ static void af_alg_free_areq_sgls(struct af_alg_async_req 
*areq)
for_each_sg(tsgl, sg, areq->tsgl_entries, i) {
if (!sg_page(sg))
continue;
-   put_page(sg_page(sg));
+   put_user_page(sg_page(sg));
}
 
sock_kfree_s(sk, tsgl, areq->tsgl_entries * sizeof(*tsgl));
-- 
2.22.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 25/34] mm/frame_vector.c: convert put_page() to put_user_page*()

[john . hubbard]

From: John Hubbard 

For pages that were retained via get_user_pages*(), release those pages
via the new put_user_page*() routines, instead of via put_page() or
release_pages().

This is part a tree-wide conversion, as described in commit fc1d8e7cca2d
("mm: introduce put_user_page*(), placeholder versions").

Cc: Dan Williams 
Cc: Jan Kara 
Cc: Mel Gorman 
Cc: Vlastimil Babka 
Signed-off-by: John Hubbard 
---
 mm/frame_vector.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/mm/frame_vector.c b/mm/frame_vector.c
index c64dca6e27c2..f590badac776 100644
--- a/mm/frame_vector.c
+++ b/mm/frame_vector.c
@@ -120,7 +120,6 @@ EXPORT_SYMBOL(get_vaddr_frames);
  */
 void put_vaddr_frames(struct frame_vector *vec)
 {
-   int i;
struct page **pages;
 
if (!vec->got_ref)
@@ -133,8 +132,7 @@ void put_vaddr_frames(struct frame_vector *vec)
 */
if (WARN_ON(IS_ERR(pages)))
goto out;
-   for (i = 0; i < vec->nr_frames; i++)
-   put_page(pages[i]);
+   put_user_pages(pages, vec->nr_frames);
vec->got_ref = false;
 out:
vec->nr_frames = 0;
-- 
2.22.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 32/34] goldfish_pipe: convert put_page() to put_user_page*()

[john . hubbard]

From: John Hubbard 

For pages that were retained via get_user_pages*(), release those pages
via the new put_user_page*() routines, instead of via put_page() or
release_pages().

This is part a tree-wide conversion, as described in commit fc1d8e7cca2d
("mm: introduce put_user_page*(), placeholder versions").

Note that this effectively changes the code's behavior in
qp_release_pages(): it now ultimately calls set_page_dirty_lock(),
instead of set_page_dirty(). This is probably more accurate.

As Christophe Hellwig put it, "set_page_dirty() is only safe if we are
dealing with a file backed page where we have reference on the inode it
hangs off." [1]

[1] https://lore.kernel.org/r/20190723153640.gb...@lst.de

Cc: Greg Kroah-Hartman 
Cc: Roman Kiryanov 
Signed-off-by: John Hubbard 
---
 drivers/platform/goldfish/goldfish_pipe.c | 9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/platform/goldfish/goldfish_pipe.c 
b/drivers/platform/goldfish/goldfish_pipe.c
index cef0133aa47a..2bd21020e288 100644
--- a/drivers/platform/goldfish/goldfish_pipe.c
+++ b/drivers/platform/goldfish/goldfish_pipe.c
@@ -288,15 +288,12 @@ static int pin_user_pages(unsigned long first_page,
 static void release_user_pages(struct page **pages, int pages_count,
   int is_write, s32 consumed_size)
 {
-   int i;
+   bool dirty = !is_write && consumed_size > 0;
 
-   for (i = 0; i < pages_count; i++) {
-   if (!is_write && consumed_size > 0)
-   set_page_dirty(pages[i]);
-   put_page(pages[i]);
-   }
+   put_user_pages_dirty_lock(pages, pages_count, dirty);
 }
 
+
 /* Populate the call parameters, merging adjacent pages together */
 static void populate_rw_params(struct page **pages,
   int pages_count,
-- 
2.22.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 23/34] uprobes: convert put_page() to put_user_page*()

[john . hubbard]

From: John Hubbard 

For pages that were retained via get_user_pages*(), release those pages
via the new put_user_page*() routines, instead of via put_page() or
release_pages().

This is part a tree-wide conversion, as described in commit fc1d8e7cca2d
("mm: introduce put_user_page*(), placeholder versions").

Cc: Peter Zijlstra 
Cc: Ingo Molnar 
Cc: Arnaldo Carvalho de Melo 
Cc: Alexander Shishkin 
Cc: Jiri Olsa 
Cc: Namhyung Kim 
Signed-off-by: John Hubbard 
---
 kernel/events/uprobes.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index 84fa00497c49..4a575de8cec8 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -397,7 +397,7 @@ __update_ref_ctr(struct mm_struct *mm, unsigned long vaddr, 
short d)
ret = 0;
 out:
kunmap_atomic(kaddr);
-   put_page(page);
+   put_user_page(page);
return ret;
 }
 
@@ -504,7 +504,7 @@ int uprobe_write_opcode(struct arch_uprobe *auprobe, struct 
mm_struct *mm,
ret = __replace_page(vma, vaddr, old_page, new_page);
put_page(new_page);
 put_old:
-   put_page(old_page);
+   put_user_page(old_page);
 
if (unlikely(ret == -EAGAIN))
goto retry;
@@ -1981,7 +1981,7 @@ static int is_trap_at_addr(struct mm_struct *mm, unsigned 
long vaddr)
return result;
 
copy_from_page(page, vaddr, &opcode, UPROBE_SWBP_INSN_SIZE);
-   put_page(page);
+   put_user_page(page);
  out:
/* This needs to return true for any variant of the trap insn */
return is_trap_insn(&opcode);
-- 
2.22.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 28/34] mm/madvise.c: convert put_page() to put_user_page*()

[john . hubbard]

From: John Hubbard 

For pages that were retained via get_user_pages*(), release those pages
via the new put_user_page*() routines, instead of via put_page() or
release_pages().

This is part a tree-wide conversion, as described in commit fc1d8e7cca2d
("mm: introduce put_user_page*(), placeholder versions").

Cc: Dan Williams 
Cc: Daniel Black 
Cc: Jan Kara 
Cc: Jérôme Glisse 
Cc: Matthew Wilcox 
Cc: Mike Kravetz 
Signed-off-by: John Hubbard 
---
 mm/madvise.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/madvise.c b/mm/madvise.c
index 968df3aa069f..1c6881a761a5 100644
--- a/mm/madvise.c
+++ b/mm/madvise.c
@@ -672,7 +672,7 @@ static int madvise_inject_error(int behavior,
 * routine is responsible for pinning the page to prevent it
 * from being released back to the page allocator.
 */
-   put_page(page);
+   put_user_page(page);
ret = memory_failure(pfn, 0);
if (ret)
return ret;
-- 
2.22.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 01/34] mm/gup: add make_dirty arg to put_user_pages_dirty_lock()

[john . hubbard]

From: John Hubbard 

Provide more capable variation of put_user_pages_dirty_lock(),
and delete put_user_pages_dirty(). This is based on the
following:

1. Lots of call sites become simpler if a bool is passed
into put_user_page*(), instead of making the call site
choose which put_user_page*() variant to call.

2. Christoph Hellwig's observation that set_page_dirty_lock()
is usually correct, and set_page_dirty() is usually a
bug, or at least questionable, within a put_user_page*()
calling chain.

This leads to the following API choices:

* put_user_pages_dirty_lock(page, npages, make_dirty)

* There is no put_user_pages_dirty(). You have to
  hand code that, in the rare case that it's
  required.

Cc: Matthew Wilcox 
Cc: Jan Kara 
Cc: Christoph Hellwig 
Cc: Ira Weiny 
Cc: Jason Gunthorpe 
Signed-off-by: John Hubbard 
---
 drivers/infiniband/core/umem.c |   5 +-
 drivers/infiniband/hw/hfi1/user_pages.c|   5 +-
 drivers/infiniband/hw/qib/qib_user_pages.c |   5 +-
 drivers/infiniband/hw/usnic/usnic_uiom.c   |   5 +-
 drivers/infiniband/sw/siw/siw_mem.c|  10 +-
 include/linux/mm.h |   5 +-
 mm/gup.c   | 115 +
 7 files changed, 58 insertions(+), 92 deletions(-)

diff --git a/drivers/infiniband/core/umem.c b/drivers/infiniband/core/umem.c
index 08da840ed7ee..965cf9dea71a 100644
--- a/drivers/infiniband/core/umem.c
+++ b/drivers/infiniband/core/umem.c
@@ -54,10 +54,7 @@ static void __ib_umem_release(struct ib_device *dev, struct 
ib_umem *umem, int d
 
for_each_sg_page(umem->sg_head.sgl, &sg_iter, umem->sg_nents, 0) {
page = sg_page_iter_page(&sg_iter);
-   if (umem->writable && dirty)
-   put_user_pages_dirty_lock(&page, 1);
-   else
-   put_user_page(page);
+   put_user_pages_dirty_lock(&page, 1, umem->writable && dirty);
}
 
sg_free_table(&umem->sg_head);
diff --git a/drivers/infiniband/hw/hfi1/user_pages.c 
b/drivers/infiniband/hw/hfi1/user_pages.c
index b89a9b9aef7a..469acb961fbd 100644
--- a/drivers/infiniband/hw/hfi1/user_pages.c
+++ b/drivers/infiniband/hw/hfi1/user_pages.c
@@ -118,10 +118,7 @@ int hfi1_acquire_user_pages(struct mm_struct *mm, unsigned 
long vaddr, size_t np
 void hfi1_release_user_pages(struct mm_struct *mm, struct page **p,
 size_t npages, bool dirty)
 {
-   if (dirty)
-   put_user_pages_dirty_lock(p, npages);
-   else
-   put_user_pages(p, npages);
+   put_user_pages_dirty_lock(p, npages, dirty);
 
if (mm) { /* during close after signal, mm can be NULL */
atomic64_sub(npages, &mm->pinned_vm);
diff --git a/drivers/infiniband/hw/qib/qib_user_pages.c 
b/drivers/infiniband/hw/qib/qib_user_pages.c
index bfbfbb7e0ff4..6bf764e41891 100644
--- a/drivers/infiniband/hw/qib/qib_user_pages.c
+++ b/drivers/infiniband/hw/qib/qib_user_pages.c
@@ -40,10 +40,7 @@
 static void __qib_release_user_pages(struct page **p, size_t num_pages,
 int dirty)
 {
-   if (dirty)
-   put_user_pages_dirty_lock(p, num_pages);
-   else
-   put_user_pages(p, num_pages);
+   put_user_pages_dirty_lock(p, num_pages, dirty);
 }
 
 /**
diff --git a/drivers/infiniband/hw/usnic/usnic_uiom.c 
b/drivers/infiniband/hw/usnic/usnic_uiom.c
index 0b0237d41613..62e6ffa9ad78 100644
--- a/drivers/infiniband/hw/usnic/usnic_uiom.c
+++ b/drivers/infiniband/hw/usnic/usnic_uiom.c
@@ -75,10 +75,7 @@ static void usnic_uiom_put_pages(struct list_head 
*chunk_list, int dirty)
for_each_sg(chunk->page_list, sg, chunk->nents, i) {
page = sg_page(sg);
pa = sg_phys(sg);
-   if (dirty)
-   put_user_pages_dirty_lock(&page, 1);
-   else
-   put_user_page(page);
+   put_user_pages_dirty_lock(&page, 1, dirty);
usnic_dbg("pa: %pa\n", &pa);
}
kfree(chunk);
diff --git a/drivers/infiniband/sw/siw/siw_mem.c 
b/drivers/infiniband/sw/siw/siw_mem.c
index 67171c82b0c4..ab83a9cec562 100644
--- a/drivers/infiniband/sw/siw/siw_mem.c
+++ b/drivers/infiniband/sw/siw/siw_mem.c
@@ -63,15 +63,7 @@ struct siw_mem *siw_mem_id2obj(struct siw_device *sdev, int 
stag_index)
 static void siw_free_plist(struct siw_page_chunk *chunk, int num_pages,
   bool dirty)
 {
-   struct page **p = chunk->plist;
-
-   while (num_pages--) {
-   if (!PageDirty(*p) && dirty)
-   put_user_pages_dirty_lock(p, 1);
-   else
-   put_user_page(*p);
-   p++;
-   }
+   put_user_pages_dirty_lock(chunk->plist, num_pages, dirty);
 }
 
 void siw_umem_release(struct siw_umem *ume

[PATCH 17/34] vfio: convert put_page() to put_user_page*()

[john . hubbard]

From: John Hubbard 

For pages that were retained via get_user_pages*(), release those pages
via the new put_user_page*() routines, instead of via put_page() or
release_pages().

This is part a tree-wide conversion, as described in commit fc1d8e7cca2d
("mm: introduce put_user_page*(), placeholder versions").

Note that this effectively changes the code's behavior in
qp_release_pages(): it now ultimately calls set_page_dirty_lock(),
instead of set_page_dirty(). This is probably more accurate.

As Christophe Hellwig put it, "set_page_dirty() is only safe if we are
dealing with a file backed page where we have reference on the inode it
hangs off." [1]

[1] https://lore.kernel.org/r/20190723153640.gb...@lst.de

Cc: Alex Williamson 
Cc: k...@vger.kernel.org
Signed-off-by: John Hubbard 
---
 drivers/vfio/vfio_iommu_type1.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index 054391f30fa8..5a5461a14299 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -320,9 +320,9 @@ static int put_pfn(unsigned long pfn, int prot)
 {
if (!is_invalid_reserved_pfn(pfn)) {
struct page *page = pfn_to_page(pfn);
-   if (prot & IOMMU_WRITE)
-   SetPageDirty(page);
-   put_page(page);
+   bool dirty = prot & IOMMU_WRITE;
+
+   put_user_pages_dirty_lock(&page, 1, dirty);
return 1;
}
return 0;
@@ -356,7 +356,7 @@ static int vaddr_get_pfn(struct mm_struct *mm, unsigned 
long vaddr,
 */
if (ret > 0 && vma_is_fsdax(vmas[0])) {
ret = -EOPNOTSUPP;
-   put_page(page[0]);
+   put_user_page(page[0]);
}
}
up_read(&mm->mmap_sem);
-- 
2.22.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 21/34] fs/exec.c: convert put_page() to put_user_page*()

[john . hubbard]

From: John Hubbard 

For pages that were retained via get_user_pages*(), release those pages
via the new put_user_page*() routines, instead of via put_page() or
release_pages().

This is part a tree-wide conversion, as described in commit fc1d8e7cca2d
("mm: introduce put_user_page*(), placeholder versions").

Cc: Alexander Viro 
Cc: linux-fsde...@vger.kernel.org
Signed-off-by: John Hubbard 
---
 fs/exec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/exec.c b/fs/exec.c
index f7f6a140856a..ee442151582f 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -227,7 +227,7 @@ static struct page *get_arg_page(struct linux_binprm *bprm, 
unsigned long pos,
 
 static void put_arg_page(struct page *page)
 {
-   put_page(page);
+   put_user_page(page);
 }
 
 static void free_arg_pages(struct linux_binprm *bprm)
-- 
2.22.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 10/34] genwqe: convert put_page() to put_user_page*()

[john . hubbard]

From: John Hubbard 

For pages that were retained via get_user_pages*(), release those pages
via the new put_user_page*() routines, instead of via put_page() or
release_pages().

This is part a tree-wide conversion, as described in commit fc1d8e7cca2d
("mm: introduce put_user_page*(), placeholder versions").

This changes the release code slightly, because each page slot in the
page_list[] array is no longer checked for NULL. However, that check
was wrong anyway, because the get_user_pages() pattern of usage here
never allowed for NULL entries within a range of pinned pages.

Cc: Frank Haverkamp 
Cc: "Guilherme G. Piccoli" 
Cc: Arnd Bergmann 
Cc: Greg Kroah-Hartman 
Signed-off-by: John Hubbard 
---
 drivers/misc/genwqe/card_utils.c | 17 +++--
 1 file changed, 3 insertions(+), 14 deletions(-)

diff --git a/drivers/misc/genwqe/card_utils.c b/drivers/misc/genwqe/card_utils.c
index 2e1c4d2905e8..2a888f31d2c5 100644
--- a/drivers/misc/genwqe/card_utils.c
+++ b/drivers/misc/genwqe/card_utils.c
@@ -517,24 +517,13 @@ int genwqe_free_sync_sgl(struct genwqe_dev *cd, struct 
genwqe_sgl *sgl)
 /**
  * genwqe_free_user_pages() - Give pinned pages back
  *
- * Documentation of get_user_pages is in mm/gup.c:
- *
- * If the page is written to, set_page_dirty (or set_page_dirty_lock,
- * as appropriate) must be called after the page is finished with, and
- * before put_page is called.
+ * The pages may have been written to, so we call put_user_pages_dirty_lock(),
+ * rather than put_user_pages().
  */
 static int genwqe_free_user_pages(struct page **page_list,
unsigned int nr_pages, int dirty)
 {
-   unsigned int i;
-
-   for (i = 0; i < nr_pages; i++) {
-   if (page_list[i] != NULL) {
-   if (dirty)
-   set_page_dirty_lock(page_list[i]);
-   put_page(page_list[i]);
-   }
-   }
+   put_user_pages_dirty_lock(page_list, nr_pages, dirty);
return 0;
 }
 
-- 
2.22.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 22/34] orangefs: convert put_page() to put_user_page*()

[john . hubbard]

From: John Hubbard 

For pages that were retained via get_user_pages*(), release those pages
via the new put_user_page*() routines, instead of via put_page() or
release_pages().

This is part a tree-wide conversion, as described in commit fc1d8e7cca2d
("mm: introduce put_user_page*(), placeholder versions").

Cc: Mike Marshall 
Cc: Martin Brandenburg 
Cc: de...@lists.orangefs.org
Signed-off-by: John Hubbard 
---
 fs/orangefs/orangefs-bufmap.c | 7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/fs/orangefs/orangefs-bufmap.c b/fs/orangefs/orangefs-bufmap.c
index 2bb916d68576..f2f33a16d604 100644
--- a/fs/orangefs/orangefs-bufmap.c
+++ b/fs/orangefs/orangefs-bufmap.c
@@ -168,10 +168,7 @@ static DEFINE_SPINLOCK(orangefs_bufmap_lock);
 static void
 orangefs_bufmap_unmap(struct orangefs_bufmap *bufmap)
 {
-   int i;
-
-   for (i = 0; i < bufmap->page_count; i++)
-   put_page(bufmap->page_array[i]);
+   put_user_pages(bufmap->page_array, bufmap->page_count);
 }
 
 static void
@@ -280,7 +277,7 @@ orangefs_bufmap_map(struct orangefs_bufmap *bufmap,
 
for (i = 0; i < ret; i++) {
SetPageError(bufmap->page_array[i]);
-   put_page(bufmap->page_array[i]);
+   put_user_page(bufmap->page_array[i]);
}
return -ENOMEM;
}
-- 
2.22.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 00/34] put_user_pages(): miscellaneous call sites

[john . hubbard]

From: John Hubbard 

Hi,

These are best characterized as miscellaneous conversions: many (not all)
call sites that don't involve biovec or iov_iter, nor mm/. It also leaves
out a few call sites that require some more work. These are mostly pretty
simple ones.

It's probably best to send all of these via Andrew's -mm tree, assuming
that there are no significant merge conflicts with ongoing work in other
trees (which I doubt, given that these are small changes).

These patches apply to the latest linux.git. Patch #1 is also already in
Andrew's tree, but given the broad non-linux-mm Cc list, I thought it
would be more convenient to just include that patch here, so that people
can use linux.git as the base--even though these are probably destined
for linux-mm.

This is part a tree-wide conversion, as described in commit fc1d8e7cca2d
("mm: introduce put_user_page*(), placeholder versions"). That commit
has an extensive description of the problem and the planned steps to
solve it, but the highlites are:

1) Provide put_user_page*() routines, intended to be used
for releasing pages that were pinned via get_user_pages*().

2) Convert all of the call sites for get_user_pages*(), to
invoke put_user_page*(), instead of put_page(). This involves dozens of
call sites, and will take some time.

3) After (2) is complete, use get_user_pages*() and put_user_page*() to
implement tracking of these pages. This tracking will be separate from
the existing struct page refcounting.

4) Use the tracking and identification of these pages, to implement
special handling (especially in writeback paths) when the pages are
backed by a filesystem.

And a few references, also from that commit:

[1] https://lwn.net/Articles/774411/ : "DMA and get_user_pages()"
[2] https://lwn.net/Articles/753027/ : "The Trouble with get_user_pages()"


Ira Weiny (1):
  fs/binfmt_elf: convert put_page() to put_user_page*()

John Hubbard (33):
  mm/gup: add make_dirty arg to put_user_pages_dirty_lock()
  net/rds: convert put_page() to put_user_page*()
  net/ceph: convert put_page() to put_user_page*()
  x86/kvm: convert put_page() to put_user_page*()
  drm/etnaviv: convert release_pages() to put_user_pages()
  drm/i915: convert put_page() to put_user_page*()
  drm/radeon: convert put_page() to put_user_page*()
  media/ivtv: convert put_page() to put_user_page*()
  media/v4l2-core/mm: convert put_page() to put_user_page*()
  genwqe: convert put_page() to put_user_page*()
  scif: convert put_page() to put_user_page*()
  vmci: convert put_page() to put_user_page*()
  rapidio: convert put_page() to put_user_page*()
  oradax: convert put_page() to put_user_page*()
  staging/vc04_services: convert put_page() to put_user_page*()
  drivers/tee: convert put_page() to put_user_page*()
  vfio: convert put_page() to put_user_page*()
  fbdev/pvr2fb: convert put_page() to put_user_page*()
  fsl_hypervisor: convert put_page() to put_user_page*()
  xen: convert put_page() to put_user_page*()
  fs/exec.c: convert put_page() to put_user_page*()
  orangefs: convert put_page() to put_user_page*()
  uprobes: convert put_page() to put_user_page*()
  futex: convert put_page() to put_user_page*()
  mm/frame_vector.c: convert put_page() to put_user_page*()
  mm/gup_benchmark.c: convert put_page() to put_user_page*()
  mm/memory.c: convert put_page() to put_user_page*()
  mm/madvise.c: convert put_page() to put_user_page*()
  mm/process_vm_access.c: convert put_page() to put_user_page*()
  crypt: convert put_page() to put_user_page*()
  nfs: convert put_page() to put_user_page*()
  goldfish_pipe: convert put_page() to put_user_page*()
  kernel/events/core.c: convert put_page() to put_user_page*()

 arch/x86/kvm/svm.c|   4 +-
 crypto/af_alg.c   |   7 +-
 drivers/gpu/drm/etnaviv/etnaviv_gem.c |   4 +-
 drivers/gpu/drm/i915/gem/i915_gem_userptr.c   |   9 +-
 drivers/gpu/drm/radeon/radeon_ttm.c   |   2 +-
 drivers/infiniband/core/umem.c|   5 +-
 drivers/infiniband/hw/hfi1/user_pages.c   |   5 +-
 drivers/infiniband/hw/qib/qib_user_pages.c|   5 +-
 drivers/infiniband/hw/usnic/usnic_uiom.c  |   5 +-
 drivers/infiniband/sw/siw/siw_mem.c   |  10 +-
 drivers/media/pci/ivtv/ivtv-udma.c|  14 +--
 drivers/media/pci/ivtv/ivtv-yuv.c |  10 +-
 drivers/media/v4l2-core/videobuf-dma-sg.c |   3 +-
 drivers/misc/genwqe/card_utils.c  |  17 +--
 drivers/misc/mic/scif/scif_rma.c  |  17 ++-
 drivers/misc/vmw_vmci/vmci_context.c  |   2 +-
 drivers/misc/vmw_vmci/vmci_queue_pair.c   |  11 +-
 drivers/platform/goldfish/goldfish_pipe.c |   9 +-
 drivers/rapidio/devices/rio_mport_cdev.c  |   9 +-
 drivers/sbus/char/oradax.c|   2 +-
 .../interface/vchiq_arm/vchiq_2835_arm.c  |  10 +-
 drivers/tee/tee_shm.c |  10 +-
 drivers/vfio/vfio_iommu_type1.c   |   8 +-
 drivers/video/fbde

[PATCH 18/34] fbdev/pvr2fb: convert put_page() to put_user_page*()

[john . hubbard]

From: John Hubbard 

For pages that were retained via get_user_pages*(), release those pages
via the new put_user_page*() routines, instead of via put_page() or
release_pages().

This is part a tree-wide conversion, as described in commit fc1d8e7cca2d
("mm: introduce put_user_page*(), placeholder versions").

Cc: Bartlomiej Zolnierkiewicz 
Cc: Kees Cook 
Cc: Al Viro 
Cc: Bhumika Goyal 
Cc: Arvind Yadav 
Cc: dri-devel@lists.freedesktop.org
Cc: linux-fb...@vger.kernel.org
Signed-off-by: John Hubbard 
---
 drivers/video/fbdev/pvr2fb.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/video/fbdev/pvr2fb.c b/drivers/video/fbdev/pvr2fb.c
index 7ff4b6b84282..0e4f9aa6444d 100644
--- a/drivers/video/fbdev/pvr2fb.c
+++ b/drivers/video/fbdev/pvr2fb.c
@@ -700,8 +700,7 @@ static ssize_t pvr2fb_write(struct fb_info *info, const 
char *buf,
ret = count;
 
 out_unmap:
-   for (i = 0; i < nr_pages; i++)
-   put_page(pages[i]);
+   put_user_pages(pages, nr_pages);
 
kfree(pages);
 
-- 
2.22.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 08/34] media/ivtv: convert put_page() to put_user_page*()

[john . hubbard]

From: John Hubbard 

For pages that were retained via get_user_pages*(), release those pages
via the new put_user_page*() routines, instead of via put_page() or
release_pages().

This is part a tree-wide conversion, as described in commit fc1d8e7cca2d
("mm: introduce put_user_page*(), placeholder versions").

Cc: Andy Walls 
Cc: Mauro Carvalho Chehab 
Cc: linux-me...@vger.kernel.org
Signed-off-by: John Hubbard 
---
 drivers/media/pci/ivtv/ivtv-udma.c | 14 --
 drivers/media/pci/ivtv/ivtv-yuv.c  | 10 +++---
 2 files changed, 7 insertions(+), 17 deletions(-)

diff --git a/drivers/media/pci/ivtv/ivtv-udma.c 
b/drivers/media/pci/ivtv/ivtv-udma.c
index 5f8883031c9c..7c7f33c2412b 100644
--- a/drivers/media/pci/ivtv/ivtv-udma.c
+++ b/drivers/media/pci/ivtv/ivtv-udma.c
@@ -92,7 +92,7 @@ int ivtv_udma_setup(struct ivtv *itv, unsigned long 
ivtv_dest_addr,
 {
struct ivtv_dma_page_info user_dma;
struct ivtv_user_dma *dma = &itv->udma;
-   int i, err;
+   int err;
 
IVTV_DEBUG_DMA("ivtv_udma_setup, dst: 0x%08x\n", (unsigned 
int)ivtv_dest_addr);
 
@@ -119,8 +119,7 @@ int ivtv_udma_setup(struct ivtv *itv, unsigned long 
ivtv_dest_addr,
IVTV_DEBUG_WARN("failed to map user pages, returned %d instead 
of %d\n",
   err, user_dma.page_count);
if (err >= 0) {
-   for (i = 0; i < err; i++)
-   put_page(dma->map[i]);
+   put_user_pages(dma->map, err);
return -EINVAL;
}
return err;
@@ -130,9 +129,7 @@ int ivtv_udma_setup(struct ivtv *itv, unsigned long 
ivtv_dest_addr,
 
/* Fill SG List with new values */
if (ivtv_udma_fill_sg_list(dma, &user_dma, 0) < 0) {
-   for (i = 0; i < dma->page_count; i++) {
-   put_page(dma->map[i]);
-   }
+   put_user_pages(dma->map, dma->page_count);
dma->page_count = 0;
return -ENOMEM;
}
@@ -153,7 +150,6 @@ int ivtv_udma_setup(struct ivtv *itv, unsigned long 
ivtv_dest_addr,
 void ivtv_udma_unmap(struct ivtv *itv)
 {
struct ivtv_user_dma *dma = &itv->udma;
-   int i;
 
IVTV_DEBUG_INFO("ivtv_unmap_user_dma\n");
 
@@ -170,9 +166,7 @@ void ivtv_udma_unmap(struct ivtv *itv)
ivtv_udma_sync_for_cpu(itv);
 
/* Release User Pages */
-   for (i = 0; i < dma->page_count; i++) {
-   put_page(dma->map[i]);
-   }
+   put_user_pages(dma->map, dma->page_count);
dma->page_count = 0;
 }
 
diff --git a/drivers/media/pci/ivtv/ivtv-yuv.c 
b/drivers/media/pci/ivtv/ivtv-yuv.c
index cd2fe2d444c0..9465a7d450b6 100644
--- a/drivers/media/pci/ivtv/ivtv-yuv.c
+++ b/drivers/media/pci/ivtv/ivtv-yuv.c
@@ -81,8 +81,7 @@ static int ivtv_yuv_prep_user_dma(struct ivtv *itv, struct 
ivtv_user_dma *dma,
 uv_pages, uv_dma.page_count);
 
if (uv_pages >= 0) {
-   for (i = 0; i < uv_pages; i++)
-   put_page(dma->map[y_pages + i]);
+   put_user_pages(&dma->map[y_pages], uv_pages);
rc = -EFAULT;
} else {
rc = uv_pages;
@@ -93,8 +92,7 @@ static int ivtv_yuv_prep_user_dma(struct ivtv *itv, struct 
ivtv_user_dma *dma,
 y_pages, y_dma.page_count);
}
if (y_pages >= 0) {
-   for (i = 0; i < y_pages; i++)
-   put_page(dma->map[i]);
+   put_user_pages(dma->map, y_pages);
/*
 * Inherit the -EFAULT from rc's
 * initialization, but allow it to be
@@ -112,9 +110,7 @@ static int ivtv_yuv_prep_user_dma(struct ivtv *itv, struct 
ivtv_user_dma *dma,
/* Fill & map SG List */
if (ivtv_udma_fill_sg_list (dma, &uv_dma, ivtv_udma_fill_sg_list (dma, 
&y_dma, 0)) < 0) {
IVTV_DEBUG_WARN("could not allocate bounce buffers for highmem 
userspace buffers\n");
-   for (i = 0; i < dma->page_count; i++) {
-   put_page(dma->map[i]);
-   }
+   put_user_pages(dma->map, dma->page_count);
dma->page_count = 0;
return -ENOMEM;
}
-- 
2.22.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 16/34] drivers/tee: convert put_page() to put_user_page*()

[john . hubbard]

From: John Hubbard 

For pages that were retained via get_user_pages*(), release those pages
via the new put_user_page*() routines, instead of via put_page() or
release_pages().

This is part a tree-wide conversion, as described in commit fc1d8e7cca2d
("mm: introduce put_user_page*(), placeholder versions").

Cc: Jens Wiklander 
Signed-off-by: John Hubbard 
---
 drivers/tee/tee_shm.c | 10 ++
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/tee/tee_shm.c b/drivers/tee/tee_shm.c
index 2da026fd12c9..c967d0420b67 100644
--- a/drivers/tee/tee_shm.c
+++ b/drivers/tee/tee_shm.c
@@ -31,16 +31,13 @@ static void tee_shm_release(struct tee_shm *shm)
 
poolm->ops->free(poolm, shm);
} else if (shm->flags & TEE_SHM_REGISTER) {
-   size_t n;
int rc = teedev->desc->ops->shm_unregister(shm->ctx, shm);
 
if (rc)
dev_err(teedev->dev.parent,
"unregister shm %p failed: %d", shm, rc);
 
-   for (n = 0; n < shm->num_pages; n++)
-   put_page(shm->pages[n]);
-
+   put_user_pages(shm->pages, shm->num_pages);
kfree(shm->pages);
}
 
@@ -313,16 +310,13 @@ struct tee_shm *tee_shm_register(struct tee_context *ctx, 
unsigned long addr,
return shm;
 err:
if (shm) {
-   size_t n;
-
if (shm->id >= 0) {
mutex_lock(&teedev->mutex);
idr_remove(&teedev->idr, shm->id);
mutex_unlock(&teedev->mutex);
}
if (shm->pages) {
-   for (n = 0; n < shm->num_pages; n++)
-   put_page(shm->pages[n]);
+   put_user_pages(shm->pages, shm->num_pages);
kfree(shm->pages);
}
}
-- 
2.22.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 00/34] put_user_pages(): miscellaneous call sites

[john . hubbard]

From: John Hubbard 

Hi,

These are best characterized as miscellaneous conversions: many (not all)
call sites that don't involve biovec or iov_iter, nor mm/. It also leaves
out a few call sites that require some more work. These are mostly pretty
simple ones.

It's probably best to send all of these via Andrew's -mm tree, assuming
that there are no significant merge conflicts with ongoing work in other
trees (which I doubt, given that these are small changes).

These patches apply to the latest linux.git. Patch #1 is also already in
Andrew's tree, but given the broad non-linux-mm Cc list, I thought it
would be more convenient to just include that patch here, so that people
can use linux.git as the base--even though these are probably destined
for linux-mm.

This is part a tree-wide conversion, as described in commit fc1d8e7cca2d
("mm: introduce put_user_page*(), placeholder versions"). That commit
has an extensive description of the problem and the planned steps to
solve it, but the highlites are:

1) Provide put_user_page*() routines, intended to be used
for releasing pages that were pinned via get_user_pages*().

2) Convert all of the call sites for get_user_pages*(), to
invoke put_user_page*(), instead of put_page(). This involves dozens of
call sites, and will take some time.

3) After (2) is complete, use get_user_pages*() and put_user_page*() to
implement tracking of these pages. This tracking will be separate from
the existing struct page refcounting.

4) Use the tracking and identification of these pages, to implement
special handling (especially in writeback paths) when the pages are
backed by a filesystem.

And a few references, also from that commit:

[1] https://lwn.net/Articles/774411/ : "DMA and get_user_pages()"
[2] https://lwn.net/Articles/753027/ : "The Trouble with get_user_pages()"


Ira Weiny (1):
  fs/binfmt_elf: convert put_page() to put_user_page*()

John Hubbard (33):
  mm/gup: add make_dirty arg to put_user_pages_dirty_lock()
  net/rds: convert put_page() to put_user_page*()
  net/ceph: convert put_page() to put_user_page*()
  x86/kvm: convert put_page() to put_user_page*()
  drm/etnaviv: convert release_pages() to put_user_pages()
  drm/i915: convert put_page() to put_user_page*()
  drm/radeon: convert put_page() to put_user_page*()
  media/ivtv: convert put_page() to put_user_page*()
  media/v4l2-core/mm: convert put_page() to put_user_page*()
  genwqe: convert put_page() to put_user_page*()
  scif: convert put_page() to put_user_page*()
  vmci: convert put_page() to put_user_page*()
  rapidio: convert put_page() to put_user_page*()
  oradax: convert put_page() to put_user_page*()
  staging/vc04_services: convert put_page() to put_user_page*()
  drivers/tee: convert put_page() to put_user_page*()
  vfio: convert put_page() to put_user_page*()
  fbdev/pvr2fb: convert put_page() to put_user_page*()
  fsl_hypervisor: convert put_page() to put_user_page*()
  xen: convert put_page() to put_user_page*()
  fs/exec.c: convert put_page() to put_user_page*()
  orangefs: convert put_page() to put_user_page*()
  uprobes: convert put_page() to put_user_page*()
  futex: convert put_page() to put_user_page*()
  mm/frame_vector.c: convert put_page() to put_user_page*()
  mm/gup_benchmark.c: convert put_page() to put_user_page*()
  mm/memory.c: convert put_page() to put_user_page*()
  mm/madvise.c: convert put_page() to put_user_page*()
  mm/process_vm_access.c: convert put_page() to put_user_page*()
  crypt: convert put_page() to put_user_page*()
  nfs: convert put_page() to put_user_page*()
  goldfish_pipe: convert put_page() to put_user_page*()
  kernel/events/core.c: convert put_page() to put_user_page*()

 arch/x86/kvm/svm.c|   4 +-
 crypto/af_alg.c   |   7 +-
 drivers/gpu/drm/etnaviv/etnaviv_gem.c |   4 +-
 drivers/gpu/drm/i915/gem/i915_gem_userptr.c   |   9 +-
 drivers/gpu/drm/radeon/radeon_ttm.c   |   2 +-
 drivers/infiniband/core/umem.c|   5 +-
 drivers/infiniband/hw/hfi1/user_pages.c   |   5 +-
 drivers/infiniband/hw/qib/qib_user_pages.c|   5 +-
 drivers/infiniband/hw/usnic/usnic_uiom.c  |   5 +-
 drivers/infiniband/sw/siw/siw_mem.c   |  10 +-
 drivers/media/pci/ivtv/ivtv-udma.c|  14 +--
 drivers/media/pci/ivtv/ivtv-yuv.c |  10 +-
 drivers/media/v4l2-core/videobuf-dma-sg.c |   3 +-
 drivers/misc/genwqe/card_utils.c  |  17 +--
 drivers/misc/mic/scif/scif_rma.c  |  17 ++-
 drivers/misc/vmw_vmci/vmci_context.c  |   2 +-
 drivers/misc/vmw_vmci/vmci_queue_pair.c   |  11 +-
 drivers/platform/goldfish/goldfish_pipe.c |   9 +-
 drivers/rapidio/devices/rio_mport_cdev.c  |   9 +-
 drivers/sbus/char/oradax.c|   2 +-
 .../interface/vchiq_arm/vchiq_2835_arm.c  |  10 +-
 drivers/tee/tee_shm.c |  10 +-
 drivers/vfio/vfio_iommu_type1.c   |   8 +-
 drivers/video/fbde

Re: [PATCH v2 6/7] drm/panfrost: Add support for GPU heap allocations

[Steven Price]

On 30/07/2019 20:08, Rob Herring wrote:
> On Tue, Jul 30, 2019 at 12:55 PM Alyssa Rosenzweig
>  wrote:
>>
>>> In any case, per process AS is a prerequisite to all this.
>>
>> Oh, I hadn't realized that was still a todo. In the meantime, what's the
>> implication of shipping without it? (I.e. in which threat model are
>> users vulnerable without it?) Malicious userspace process snooping on
>> other framebuffers (on X11, they could do that anyway...)? Malicious
>> userspace actually interfering with operation of other processes (is
>> this really exploitable or just a theoretical concern)? Malicious 3D
>> apps breaking out of the sandbox (i.e. WebGL) via a driver bug and
>> snooping on other processes?

Having a single address space means:

Malicious userspace can: snoop and overwrite graphics buffers from other
applications. I don't believe you can directly map the buffer into the
other application, but the GPU provides plenty of functionality to
implement a memcpy-like shader which can copy to/from buffers you don't own.

WebGL: In *theory* the driver should ensure that any buffer accesses are
contained before letting the shaders run. So this shouldn't actually
allow breaking out of the sandbox. This is because the browser may use
one process for multiple tabs (and one process = one AS in most cases)
and they should be sandboxed. But obviously it only requires one driver
bug for this to break as well.


Also note that if the driver "trusts" any of the data shared with the
GPU (e.g. follows pointers stored in GPU accessible memory or uses
values to look up in an array without bounds checking) then a malicious
userspace may be able to inject code into another process. So this could
be a privilege escalation route.

> I don't know. However, it's not that uncommon. freedreno is only now
> in the process of supporting it. vc4 can't. v3d doesn't yet support
> separate address spaces.

Indeed it doesn't seem to actually concern many people. For example
almost all GPUs allow any process to effectively DoS the GPU by
submitting extremely long running jobs. Most OSes just reset the GPU in
this case - which might take work from another process with it. Although
kbase actually does try fairly hard to prevent that.

Although somewhat amusingly, you might have noticed this lovely
workaround in kbase:

>   if (kbase_hw_has_issue(kbdev, BASE_HW_ISSUE_8987)) {
>   bool use_workaround;
> 
>   use_workaround = DEFAULT_SECURE_BUT_LOSS_OF_PERFORMANCE;
>   if (use_workaround) {
>   dev_dbg(kbdev->dev, "GPU has HW ISSUE 8987, and driver 
> configured for security workaround: 1 address space only");
>   kbdev->nr_user_address_spaces = 1;
>   }
>   }

I seriously doubt anyone ever set
DEFAULT_SECURE_BUT_LOSS_OF_PERFORMANCE... But this was a hardware bug
that broke the isolation between address spaces.

Steve
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 15/34] staging/vc04_services: convert put_page() to put_user_page*()

[john . hubbard]

From: John Hubbard 

For pages that were retained via get_user_pages*(), release those pages
via the new put_user_page*() routines, instead of via put_page() or
release_pages().

This is part a tree-wide conversion, as described in commit fc1d8e7cca2d
("mm: introduce put_user_page*(), placeholder versions").

Cc: Eric Anholt 
Cc: Stefan Wahren 
Cc: Greg Kroah-Hartman 
Cc: Mihaela Muraru 
Cc: Suniel Mahesh 
Cc: Al Viro 
Cc: Sidong Yang 
Cc: Kishore KP 
Cc: linux-rpi-ker...@lists.infradead.org
Cc: linux-arm-ker...@lists.infradead.org
Cc: de...@driverdev.osuosl.org
Signed-off-by: John Hubbard 
---
 .../vc04_services/interface/vchiq_arm/vchiq_2835_arm.c | 10 ++
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c 
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
index 61c69f353cdb..ec92b4c50e95 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
@@ -336,10 +336,7 @@ cleanup_pagelistinfo(struct vchiq_pagelist_info 
*pagelistinfo)
}
 
if (pagelistinfo->pages_need_release) {
-   unsigned int i;
-
-   for (i = 0; i < pagelistinfo->num_pages; i++)
-   put_page(pagelistinfo->pages[i]);
+   put_user_pages(pagelistinfo->pages, pagelistinfo->num_pages);
}
 
dma_free_coherent(g_dev, pagelistinfo->pagelist_buffer_size,
@@ -454,10 +451,7 @@ create_pagelist(char __user *buf, size_t count, unsigned 
short type)
   __func__, actual_pages, num_pages);
 
/* This is probably due to the process being killed */
-   while (actual_pages > 0) {
-   actual_pages--;
-   put_page(pages[actual_pages]);
-   }
+   put_user_pages(pages, actual_pages);
cleanup_pagelistinfo(pagelistinfo);
return NULL;
}
-- 
2.22.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 00/34] put_user_pages(): miscellaneous call sites

[John Hubbard]

On 8/1/19 7:16 PM, john.hubb...@gmail.com wrote:
> From: John Hubbard 
> 
> Hi,
> 
> These are best characterized as miscellaneous conversions: many (not all)
> call sites that don't involve biovec or iov_iter, nor mm/. It also leaves
> out a few call sites that require some more work. These are mostly pretty
> simple ones.
> 
> It's probably best to send all of these via Andrew's -mm tree, assuming
> that there are no significant merge conflicts with ongoing work in other
> trees (which I doubt, given that these are small changes).
> 

In case anyone is wondering, this truncated series is due to a script failure:
git-send-email chokes when it hits email addresses whose names have a
comma in them, as happened here with patch 0003.  

Please disregard this set and reply to the other thread.

thanks,
-- 
John Hubbard
NVIDIA
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 09/34] media/v4l2-core/mm: convert put_page() to put_user_page*()

[john . hubbard]

From: John Hubbard 

For pages that were retained via get_user_pages*(), release those pages
via the new put_user_page*() routines, instead of via put_page() or
release_pages().

This is part a tree-wide conversion, as described in commit fc1d8e7cca2d
("mm: introduce put_user_page*(), placeholder versions").

Cc: Mauro Carvalho Chehab 
Cc: Kees Cook 
Cc: Hans Verkuil 
Cc: Sakari Ailus 
Cc: Jan Kara 
Cc: Robin Murphy 
Cc: Souptick Joarder 
Cc: Dan Williams 
Cc: linux-me...@vger.kernel.org
Signed-off-by: John Hubbard 
---
 drivers/media/v4l2-core/videobuf-dma-sg.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf-dma-sg.c 
b/drivers/media/v4l2-core/videobuf-dma-sg.c
index 66a6c6c236a7..d6eeb437ec19 100644
--- a/drivers/media/v4l2-core/videobuf-dma-sg.c
+++ b/drivers/media/v4l2-core/videobuf-dma-sg.c
@@ -349,8 +349,7 @@ int videobuf_dma_free(struct videobuf_dmabuf *dma)
BUG_ON(dma->sglen);
 
if (dma->pages) {
-   for (i = 0; i < dma->nr_pages; i++)
-   put_page(dma->pages[i]);
+   put_user_pages(dma->pages, dma->nr_pages);
kfree(dma->pages);
dma->pages = NULL;
}
-- 
2.22.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 20/34] xen: convert put_page() to put_user_page*()

[john . hubbard]

From: John Hubbard 

For pages that were retained via get_user_pages*(), release those pages
via the new put_user_page*() routines, instead of via put_page() or
release_pages().

This is part a tree-wide conversion, as described in commit fc1d8e7cca2d
("mm: introduce put_user_page*(), placeholder versions").

Cc: Boris Ostrovsky 
Cc: Juergen Gross 
Cc: xen-de...@lists.xenproject.org
Signed-off-by: John Hubbard 
---
 drivers/xen/gntdev.c  | 5 +
 drivers/xen/privcmd.c | 7 +--
 2 files changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
index 4c339c7e66e5..2586b3df2bb6 100644
--- a/drivers/xen/gntdev.c
+++ b/drivers/xen/gntdev.c
@@ -864,10 +864,7 @@ static int gntdev_get_page(struct gntdev_copy_batch 
*batch, void __user *virt,
 
 static void gntdev_put_pages(struct gntdev_copy_batch *batch)
 {
-   unsigned int i;
-
-   for (i = 0; i < batch->nr_pages; i++)
-   put_page(batch->pages[i]);
+   put_user_pages(batch->pages, batch->nr_pages);
batch->nr_pages = 0;
 }
 
diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c
index 2f5ce7230a43..29e461dbee2d 100644
--- a/drivers/xen/privcmd.c
+++ b/drivers/xen/privcmd.c
@@ -611,15 +611,10 @@ static int lock_pages(
 
 static void unlock_pages(struct page *pages[], unsigned int nr_pages)
 {
-   unsigned int i;
-
if (!pages)
return;
 
-   for (i = 0; i < nr_pages; i++) {
-   if (pages[i])
-   put_page(pages[i]);
-   }
+   put_user_pages(pages, nr_pages);
 }
 
 static long privcmd_ioctl_dm_op(struct file *file, void __user *udata)
-- 
2.22.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH/RFC 07/12] drm: rcar-du: lvds: Add support for dual link panels

[Laurent Pinchart]

Hi Fabrizio,

Thank you for the patch.

On Fri, Aug 02, 2019 at 08:34:04AM +0100, Fabrizio Castro wrote:
> If the display comes with two ports, assume it supports dual
> link.
> 
> Signed-off-by: Fabrizio Castro 
> ---
>  drivers/gpu/drm/rcar-du/rcar_lvds.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c 
> b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> index 2d54ae5..97c51c2 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> @@ -751,6 +751,9 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds)
>   ret = -EPROBE_DEFER;
>   goto done;
>   }
> + if (lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK)
> + lvds->dual_link = of_graph_get_endpoint_count(remote)
> + == 2;

This is a bit of a hack, as I think the information should be queried
from the panel, like we do for bridges. I'd say we can live with this
for now, but as the data swap flag should come from the panel as well,
we will need infrastructure for that, and we can as well through the
dual link flag there at the same time.

I think we should use the drm_bridge_timings structure for this purpose,
as it would make life more difficult for users of drm_bridge and
drm_panel to have two different structures (especially when wrapping a
drm_panel with drm_panel_bridge_add()). The structure could be renamed
if desired.

>   }
>  
>   if (lvds->dual_link) {

-- 
Regards,

Laurent Pinchart

[PATCH 05/34] drm/etnaviv: convert release_pages() to put_user_pages()

[john . hubbard]

From: John Hubbard 

For pages that were retained via get_user_pages*(), release those pages
via the new put_user_page*() routines, instead of via put_page() or
release_pages().

This is part a tree-wide conversion, as described in commit fc1d8e7cca2d
("mm: introduce put_user_page*(), placeholder versions").

Cc: Joerg Roedel 
Cc: Paolo Bonzini 
Cc: "Radim Krčmář" 
Cc: Thomas Gleixner 
Cc: Ingo Molnar 
Cc: Borislav Petkov 
Cc: "H. Peter Anvin" 
Cc: x...@kernel.org
Cc: k...@vger.kernel.org
Signed-off-by: John Hubbard 
---
 drivers/gpu/drm/etnaviv/etnaviv_gem.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c 
b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
index e8778ebb72e6..a0144a5ee325 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
@@ -686,7 +686,7 @@ static int etnaviv_gem_userptr_get_pages(struct 
etnaviv_gem_object *etnaviv_obj)
ret = get_user_pages_fast(ptr, num_pages,
  !userptr->ro ? FOLL_WRITE : 0, pages);
if (ret < 0) {
-   release_pages(pvec, pinned);
+   put_user_pages(pvec, pinned);
kvfree(pvec);
return ret;
}
@@ -710,7 +710,7 @@ static void etnaviv_gem_userptr_release(struct 
etnaviv_gem_object *etnaviv_obj)
if (etnaviv_obj->pages) {
int npages = etnaviv_obj->base.size >> PAGE_SHIFT;
 
-   release_pages(etnaviv_obj->pages, npages);
+   put_user_pages(etnaviv_obj->pages, npages);
kvfree(etnaviv_obj->pages);
}
 }
-- 
2.22.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 31/34] nfs: convert put_page() to put_user_page*()

[john . hubbard]

From: John Hubbard 

For pages that were retained via get_user_pages*(), release those pages
via the new put_user_page*() routines, instead of via put_page() or
release_pages().

This is part a tree-wide conversion, as described in commit fc1d8e7cca2d
("mm: introduce put_user_page*(), placeholder versions").

Cc: Trond Myklebust 
Cc: Anna Schumaker 
Cc: linux-...@vger.kernel.org
Signed-off-by: John Hubbard 
---
 fs/nfs/direct.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/fs/nfs/direct.c b/fs/nfs/direct.c
index 0cb442406168..b00b89dda3c5 100644
--- a/fs/nfs/direct.c
+++ b/fs/nfs/direct.c
@@ -278,9 +278,7 @@ ssize_t nfs_direct_IO(struct kiocb *iocb, struct iov_iter 
*iter)
 
 static void nfs_direct_release_pages(struct page **pages, unsigned int npages)
 {
-   unsigned int i;
-   for (i = 0; i < npages; i++)
-   put_page(pages[i]);
+   put_user_pages(pages, npages);
 }
 
 void nfs_init_cinfo_from_dreq(struct nfs_commit_info *cinfo,
-- 
2.22.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 02/13] amdgpu: don't initialize range->list in amdgpu_hmm_init_range

[Jason Gunthorpe]

On Wed, Jul 31, 2019 at 01:25:06PM +, Kuehling, Felix wrote:
> On 2019-07-30 1:51 a.m., Christoph Hellwig wrote:
> > The list is used to add the range to another list as an entry in the
> > core hmm code, so there is no need to initialize it in a driver.
> 
> I've seen code that uses list_empty to check whether a list head has 
> been added to a list or not. For that to work, the list head needs to be 
> initialized, and it has to be removed with list_del_init. 

I think the ida is that 'list' is a private member of range and
drivers shouldn't touch it at all.

> ever do that with range->list, then this patch is Reviewed-by: Felix 
> Kuehling 

Please put tags on their own empty line so that patchworks will
collect them automatically..

Jason
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 27/34] mm/memory.c: convert put_page() to put_user_page*()

[john . hubbard]

From: John Hubbard 

For pages that were retained via get_user_pages*(), release those pages
via the new put_user_page*() routines, instead of via put_page() or
release_pages().

This is part a tree-wide conversion, as described in commit fc1d8e7cca2d
("mm: introduce put_user_page*(), placeholder versions").

Cc: Aneesh Kumar K.V 
Cc: Huang Ying 
Cc: Jérôme Glisse 
Cc: Matthew Wilcox 
Cc: Michal Hocko 
Cc: Peter Zijlstra 
Cc: Rik van Riel 
Cc: Souptick Joarder 
Cc: Will Deacon 
Signed-off-by: John Hubbard 
---
 mm/memory.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/memory.c b/mm/memory.c
index e2bb51b6242e..8870968496ea 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -4337,7 +4337,7 @@ int __access_remote_vm(struct task_struct *tsk, struct 
mm_struct *mm,
buf, maddr + offset, bytes);
}
kunmap(page);
-   put_page(page);
+   put_user_page(page);
}
len -= bytes;
buf += bytes;
-- 
2.22.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 19/34] fsl_hypervisor: convert put_page() to put_user_page*()

[john . hubbard]

From: John Hubbard 

For pages that were retained via get_user_pages*(), release those pages
via the new put_user_page*() routines, instead of via put_page() or
release_pages().

This is part a tree-wide conversion, as described in commit fc1d8e7cca2d
("mm: introduce put_user_page*(), placeholder versions").

This changes the release code slightly, because each page slot in the
page_list[] array is no longer checked for NULL. However, that check
was wrong anyway, because the get_user_pages() pattern of usage here
never allowed for NULL entries within a range of pinned pages.

Cc: Al Viro 
Cc: Kees Cook 
Cc: Rob Herring 
Signed-off-by: John Hubbard 
---
 drivers/virt/fsl_hypervisor.c | 7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/virt/fsl_hypervisor.c b/drivers/virt/fsl_hypervisor.c
index 93d5bebf9572..a8f78d572c45 100644
--- a/drivers/virt/fsl_hypervisor.c
+++ b/drivers/virt/fsl_hypervisor.c
@@ -292,11 +292,8 @@ static long ioctl_memcpy(struct fsl_hv_ioctl_memcpy __user 
*p)
virt_to_phys(sg_list), num_pages);
 
 exit:
-   if (pages) {
-   for (i = 0; i < num_pages; i++)
-   if (pages[i])
-   put_page(pages[i]);
-   }
+   if (pages)
+   put_user_pages(pages, num_pages);
 
kfree(sg_list_unaligned);
kfree(pages);
-- 
2.22.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 6/7] drm/panfrost: Add support for GPU heap allocations

[Steven Price]

On 30/07/2019 21:03, Rob Herring wrote:
> On Thu, Jul 25, 2019 at 9:35 AM Steven Price  wrote:
>>
>> On 25/07/2019 15:59, Steven Price wrote:
>> [...]
>>> It would appear that in the following call sgt==NULL:
  ret = sg_alloc_table_from_pages(sgt, pages + page_offset,
  NUM_FAULT_PAGES, 0, SZ_2M, 
 GFP_KERNEL);
>>>
>>> Which means we've ended up with a BO with bo->sgt==NULL, bo->pages set
>>> and bo->is_heap=true. My understanding is this should be impossible.
>>>
>>> I haven't yet figured out how this happens - it seems to be just before
>>> termination, so it might be a race with cleanup?
>>
>> That was a red herring - it's partly my test case doing something a bit
>> weird. This crash is caused by doing an mmap of a HEAP object before any
>> fault has occurred.
>>
>> drm_gem_shmem_mmap() calls drm_gem_shmem_get_pages() which will populate
>> bo->base.pages (even if bo->is_heap).
>>
>> Either we should prevent mapping of HEAP objects, or alternatively
>> bo->base.pages could be allocated upfront instead of during the first
>> fault. My preference would be allocating it upfront because optimising
>> for the case of a HEAP BO which isn't used seems a bit weird. Although
>> there's still the question of exactly what the behaviour should be of
>> accessing through the CPU pages which haven't been allocated yet.
> 
> As preventing getting the mmap fake offset should be sufficient, I'm
> planning on doing this change:
> 
> diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c
> b/drivers/gpu/drm/panfrost/panfrost_drv.c
> index 746eb4603bc2..186d5db892a9 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_drv.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
> @@ -270,6 +270,10 @@ static int panfrost_ioctl_mmap_bo(struct
> drm_device *dev, void *data,
> return -ENOENT;
> }
> 
> +   /* Don't allow mmapping of heap objects as pages are not pinned. */
> +   if (to_panfrost_bo(gem_obj)->is_heap))
> +   return -EINVAL;
> +
> ret = drm_gem_create_mmap_offset(gem_obj);
> if (ret == 0)
> args->offset = drm_vma_node_offset_addr(&gem_obj->vma_node);
> 

Seems reasonable to me - we can always add support for mmap()ing at a
later date if it becomes useful.

>> Also shmem->pages_use_count needs incrementing to stop
>> drm_gem_shmem_get_pages() replacing bo->base.pages. I haven't tested
>> what happens if you mmap *after* the first fault.
> 
> I set pages_use_count to 1 when we allocate pages on the first fault.
> Or do you mean we need to set it on creation in case
> drm_gem_shmem_get_pages() is called before any GPU faults?
> 
> Either way, that just shifts how/where we crash I think. We need to
> prevent drm_gem_shmem_get_pages() from being called. Besides mmap, the
> other cases are vmap and exporting. I don't think we have any paths
> that will cause vmap to get called in our case. For exporting, perhaps
> we need a wrapper around drm_gem_shmem_pin() to prevent it.

Yes, either every path to drm_gem_shmem_get_pages() needs to be blocked
for HEAP objects, or you need to set pages_use_count to 1 when you
allocate (which then means drm_gem_shmem_get_pages() will simply
increment the ref-count).

Of course if you are leaving any calls to drm_gem_shmem_get_pages()
reachable then you also need to ensure that the code that follows
understands how to deal with a sparse bo->pages array. Exporting would
be a good example - and again I suspect just preventing it is fine for now.

Steve
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH libdrm v2 4/4] meson.build: Fix meson script on FreeBSD

[Niclas Zeising]

On 2019-06-17 15:44, Eric Engestrom wrote:

On Monday, 2019-06-17 11:20:43 +0200, Niclas Zeising wrote:

On 2019-06-17 11:14, Eric Engestrom wrote:

On Sunday, 2019-06-16 14:23:43 +0100, Emil Velikov wrote:

From: Niclas Zeising 

FreeBSD requires sys/types.h for sys/sysctl.h, add it as part of the
includes when checking for headers.
Instead of splitting out the check for sys/sysctl.h from the other
header checks, just add sys/types.h to all header checks.

v2 [Emil]
   - add inline comment
   - drop bash/sh hunk

Signed-off-by: Emil Velikov 


Series is:
Reviewed-by: Eric Engestrom 

But I agree with Emil, it's a FreeBSD bug for it to not include
a required header, this should also be fixed upstream.


I'm not sure if my e-mails are getting to the mailing list.


They are :)

You can check the archive if you're not sure:
https://lists.freedesktop.org/archives/dri-devel/2019-June/thread.html


This is the way
it's documented in FreeBSD, and there is a lot of legacy reasons it is this
way.  I doubt it will change, and even if it does, there will be about 5
years of transition period before all supported releases has the change,
most likely.

This is the first time, to my knowledge, this issue has come up.

(I'm not saying I disagree with you, just saying it's probably easier to
patch here rather than try to change upstream.)

Regards
--
Niclas


If think we both agree then ;)
I definitely agree with should have the workaround downstream, all
I meant is that it's worth pushing upstream to fix the bug so that
*eventually* not all downstream users needs to have the workaround (but
yeah, that will take years to reach).



Just noticed that these patches still haven't been merged.  What's the 
hold up?

Thanks!
Regards
--
Niclas Zeising
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 13/34] rapidio: convert put_page() to put_user_page*()

[john . hubbard]

From: John Hubbard 

For pages that were retained via get_user_pages*(), release those pages
via the new put_user_page*() routines, instead of via put_page() or
release_pages().

This is part a tree-wide conversion, as described in commit fc1d8e7cca2d
("mm: introduce put_user_page*(), placeholder versions").

Cc: Matt Porter 
Cc: Alexandre Bounine 
Cc: Al Viro 
Cc: Logan Gunthorpe 
Cc: Christophe JAILLET 
Cc: Ioan Nicu 
Cc: Kees Cook 
Cc: Tvrtko Ursulin 
Signed-off-by: John Hubbard 
---
 drivers/rapidio/devices/rio_mport_cdev.c | 9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/rapidio/devices/rio_mport_cdev.c 
b/drivers/rapidio/devices/rio_mport_cdev.c
index 8155f59ece38..0e8ea0e5a89e 100644
--- a/drivers/rapidio/devices/rio_mport_cdev.c
+++ b/drivers/rapidio/devices/rio_mport_cdev.c
@@ -572,14 +572,12 @@ static void dma_req_free(struct kref *ref)
struct mport_dma_req *req = container_of(ref, struct mport_dma_req,
refcount);
struct mport_cdev_priv *priv = req->priv;
-   unsigned int i;
 
dma_unmap_sg(req->dmach->device->dev,
 req->sgt.sgl, req->sgt.nents, req->dir);
sg_free_table(&req->sgt);
if (req->page_list) {
-   for (i = 0; i < req->nr_pages; i++)
-   put_page(req->page_list[i]);
+   put_user_pages(req->page_list, req->nr_pages);
kfree(req->page_list);
}
 
@@ -815,7 +813,7 @@ rio_dma_transfer(struct file *filp, u32 transfer_mode,
struct mport_dma_req *req;
struct mport_dev *md = priv->md;
struct dma_chan *chan;
-   int i, ret;
+   int ret;
int nents;
 
if (xfer->length == 0)
@@ -946,8 +944,7 @@ rio_dma_transfer(struct file *filp, u32 transfer_mode,
 
 err_pg:
if (!req->page_list) {
-   for (i = 0; i < nr_pages; i++)
-   put_page(page_list[i]);
+   put_user_pages(page_list, nr_pages);
kfree(page_list);
}
 err_req:
-- 
2.22.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 11/34] scif: convert put_page() to put_user_page*()

[john . hubbard]

From: John Hubbard 

For pages that were retained via get_user_pages*(), release those pages
via the new put_user_page*() routines, instead of via put_page() or
release_pages().

This is part a tree-wide conversion, as described in commit fc1d8e7cca2d
("mm: introduce put_user_page*(), placeholder versions").

Cc: Sudeep Dutt 
Cc: Ashutosh Dixit 
Cc: Arnd Bergmann 
Cc: Joerg Roedel 
Cc: Robin Murphy 
Cc: Zhen Lei 
Signed-off-by: John Hubbard 
---
 drivers/misc/mic/scif/scif_rma.c | 17 -
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/misc/mic/scif/scif_rma.c b/drivers/misc/mic/scif/scif_rma.c
index 01e27682ea30..d84ed9466920 100644
--- a/drivers/misc/mic/scif/scif_rma.c
+++ b/drivers/misc/mic/scif/scif_rma.c
@@ -113,13 +113,14 @@ static int scif_destroy_pinned_pages(struct 
scif_pinned_pages *pin)
int writeable = pin->prot & SCIF_PROT_WRITE;
int kernel = SCIF_MAP_KERNEL & pin->map_flags;
 
-   for (j = 0; j < pin->nr_pages; j++) {
-   if (pin->pages[j] && !kernel) {
+   if (kernel) {
+   for (j = 0; j < pin->nr_pages; j++) {
if (writeable)
-   SetPageDirty(pin->pages[j]);
+   set_page_dirty_lock(pin->pages[j]);
put_page(pin->pages[j]);
}
-   }
+   } else
+   put_user_pages_dirty_lock(pin->pages, pin->nr_pages, writeable);
 
scif_free(pin->pages,
  pin->nr_pages * sizeof(*pin->pages));
@@ -1385,11 +1386,9 @@ int __scif_pin_pages(void *addr, size_t len, int 
*out_prot,
if (ulimit)
__scif_dec_pinned_vm_lock(mm, nr_pages);
/* Roll back any pinned pages */
-   for (i = 0; i < pinned_pages->nr_pages; i++) {
-   if (pinned_pages->pages[i])
-   put_page(
-   pinned_pages->pages[i]);
-   }
+   put_user_pages(pinned_pages->pages,
+  pinned_pages->nr_pages);
+
prot &= ~SCIF_PROT_WRITE;
try_upgrade = false;
goto retry;
-- 
2.22.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 04/34] x86/kvm: convert put_page() to put_user_page*()

[john . hubbard]

From: John Hubbard 

For pages that were retained via get_user_pages*(), release those pages
via the new put_user_page*() routines, instead of via put_page().

This is part a tree-wide conversion, as described in commit fc1d8e7cca2d
("mm: introduce put_user_page*(), placeholder versions").

Cc: Joerg Roedel 
Cc: Paolo Bonzini 
Cc: "Radim Krčmář" 
Cc: Thomas Gleixner 
Cc: Ingo Molnar 
Cc: "H. Peter Anvin" 
Cc: x...@kernel.org
Cc: k...@vger.kernel.org
Signed-off-by: John Hubbard 
---
 arch/x86/kvm/svm.c  | 4 ++--
 virt/kvm/kvm_main.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 7eafc6907861..ff93c923ed36 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -1827,7 +1827,7 @@ static struct page **sev_pin_memory(struct kvm *kvm, 
unsigned long uaddr,
 
 err:
if (npinned > 0)
-   release_pages(pages, npinned);
+   put_user_pages(pages, npinned);
 
kvfree(pages);
return NULL;
@@ -1838,7 +1838,7 @@ static void sev_unpin_memory(struct kvm *kvm, struct page 
**pages,
 {
struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
 
-   release_pages(pages, npages);
+   put_user_pages(pages, npages);
kvfree(pages);
sev->pages_locked -= npages;
 }
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 887f3b0c2b60..4b6a596ea8e9 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -1499,7 +1499,7 @@ static int hva_to_pfn_slow(unsigned long addr, bool 
*async, bool write_fault,
 
if (__get_user_pages_fast(addr, 1, 1, &wpage) == 1) {
*writable = true;
-   put_page(page);
+   put_user_page(page);
page = wpage;
}
}
@@ -1831,7 +1831,7 @@ EXPORT_SYMBOL_GPL(kvm_release_page_clean);
 void kvm_release_pfn_clean(kvm_pfn_t pfn)
 {
if (!is_error_noslot_pfn(pfn) && !kvm_is_reserved_pfn(pfn))
-   put_page(pfn_to_page(pfn));
+   put_user_page(pfn_to_page(pfn));
 }
 EXPORT_SYMBOL_GPL(kvm_release_pfn_clean);
 
-- 
2.22.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 02/34] net/rds: convert put_page() to put_user_page*()

[john . hubbard]

From: John Hubbard 

For pages that were retained via get_user_pages*(), release those pages
via the new put_user_page*() routines, instead of via put_page() or
release_pages().

This is part a tree-wide conversion, as described in commit fc1d8e7cca2d
("mm: introduce put_user_page*(), placeholder versions").

Cc: Santosh Shilimkar 
Cc: David S. Miller 
Cc: net...@vger.kernel.org
Cc: linux-r...@vger.kernel.org
Cc: rds-de...@oss.oracle.com
Signed-off-by: John Hubbard 
---
 net/rds/info.c|  5 ++---
 net/rds/message.c |  2 +-
 net/rds/rdma.c| 15 +++
 3 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/net/rds/info.c b/net/rds/info.c
index 03f6fd56d237..ca6af2889adf 100644
--- a/net/rds/info.c
+++ b/net/rds/info.c
@@ -162,7 +162,6 @@ int rds_info_getsockopt(struct socket *sock, int optname, 
char __user *optval,
struct rds_info_lengths lens;
unsigned long nr_pages = 0;
unsigned long start;
-   unsigned long i;
rds_info_func func;
struct page **pages = NULL;
int ret;
@@ -235,8 +234,8 @@ int rds_info_getsockopt(struct socket *sock, int optname, 
char __user *optval,
ret = -EFAULT;
 
 out:
-   for (i = 0; pages && i < nr_pages; i++)
-   put_page(pages[i]);
+   if (pages)
+   put_user_pages(pages, nr_pages);
kfree(pages);
 
return ret;
diff --git a/net/rds/message.c b/net/rds/message.c
index 50f13f1d4ae0..d7b0d266c437 100644
--- a/net/rds/message.c
+++ b/net/rds/message.c
@@ -404,7 +404,7 @@ static int rds_message_zcopy_from_user(struct rds_message 
*rm, struct iov_iter *
int i;
 
for (i = 0; i < rm->data.op_nents; i++)
-   put_page(sg_page(&rm->data.op_sg[i]));
+   put_user_page(sg_page(&rm->data.op_sg[i]));
mmp = &rm->data.op_mmp_znotifier->z_mmp;
mm_unaccount_pinned_pages(mmp);
ret = -EFAULT;
diff --git a/net/rds/rdma.c b/net/rds/rdma.c
index 916f5ec373d8..6762e8696b99 100644
--- a/net/rds/rdma.c
+++ b/net/rds/rdma.c
@@ -162,8 +162,7 @@ static int rds_pin_pages(unsigned long user_addr, unsigned 
int nr_pages,
  pages);
 
if (ret >= 0 && ret < nr_pages) {
-   while (ret--)
-   put_page(pages[ret]);
+   put_user_pages(pages, ret);
ret = -EFAULT;
}
 
@@ -276,7 +275,7 @@ static int __rds_rdma_map(struct rds_sock *rs, struct 
rds_get_mr_args *args,
 
if (IS_ERR(trans_private)) {
for (i = 0 ; i < nents; i++)
-   put_page(sg_page(&sg[i]));
+   put_user_page(sg_page(&sg[i]));
kfree(sg);
ret = PTR_ERR(trans_private);
goto out;
@@ -464,9 +463,10 @@ void rds_rdma_free_op(struct rm_rdma_op *ro)
 * to local memory */
if (!ro->op_write) {
WARN_ON(!page->mapping && irqs_disabled());
-   set_page_dirty(page);
+   put_user_pages_dirty_lock(&page, 1, true);
+   } else {
+   put_user_page(page);
}
-   put_page(page);
}
 
kfree(ro->op_notifier);
@@ -481,8 +481,7 @@ void rds_atomic_free_op(struct rm_atomic_op *ao)
/* Mark page dirty if it was possibly modified, which
 * is the case for a RDMA_READ which copies from remote
 * to local memory */
-   set_page_dirty(page);
-   put_page(page);
+   put_user_pages_dirty_lock(&page, 1, true);
 
kfree(ao->op_notifier);
ao->op_notifier = NULL;
@@ -867,7 +866,7 @@ int rds_cmsg_atomic(struct rds_sock *rs, struct rds_message 
*rm,
return ret;
 err:
if (page)
-   put_page(page);
+   put_user_page(page);
rm->atomic.op_active = 0;
kfree(rm->atomic.op_notifier);
 
-- 
2.22.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v19 02/15] arm64: Introduce prctl() options to control the tagged user addresses ABI

[Kevin Brodsky]

On 31/07/2019 18:05, Dave Hansen wrote:

On 7/23/19 10:58 AM, Andrey Konovalov wrote:

+long set_tagged_addr_ctrl(unsigned long arg)
+{
+   if (!tagged_addr_prctl_allowed)
+   return -EINVAL;
+   if (is_compat_task())
+   return -EINVAL;
+   if (arg & ~PR_TAGGED_ADDR_ENABLE)
+   return -EINVAL;
+
+   update_thread_flag(TIF_TAGGED_ADDR, arg & PR_TAGGED_ADDR_ENABLE);
+
+   return 0;
+}

Instead of a plain enable/disable, a more flexible ABI would be to have
the tag mask be passed in.  That way, an implementation that has a
flexible tag size can select it.  It also ensures that userspace
actually knows what the tag size is and isn't surprised if a hardware
implementation changes the tag size or position.

Also, this whole set deals with tagging/untagging, but there's an
effective loss of address space when you do this.  Is that dealt with
anywhere?  How do we ensure that allocations don't get placed at a
tagged address before this gets turned on?  Where's that checking?


This patch series only changes what is allowed or not at the syscall interface. It 
does not change the address space size. On arm64, TBI (Top Byte Ignore) has always 
been enabled for userspace, so it has never been possible to use the upper 8 bits of 
user pointers for addressing.


If other architectures were to support a similar functionality, then I agree that a 
common and more generic interface (if needed) would be helpful, but as it stands this 
is an arm64-specific prctl, and on arm64 the address tag is defined by the 
architecture as bits [63:56].


Kevin
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 20/34] xen: convert put_page() to put_user_page*()

[Juergen Gross]

On 02.08.19 04:19, john.hubb...@gmail.com wrote:

From: John Hubbard 

For pages that were retained via get_user_pages*(), release those pages
via the new put_user_page*() routines, instead of via put_page() or
release_pages().

This is part a tree-wide conversion, as described in commit fc1d8e7cca2d
("mm: introduce put_user_page*(), placeholder versions").

Cc: Boris Ostrovsky 
Cc: Juergen Gross 
Cc: xen-de...@lists.xenproject.org
Signed-off-by: John Hubbard 
---
  drivers/xen/gntdev.c  | 5 +
  drivers/xen/privcmd.c | 7 +--
  2 files changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
index 4c339c7e66e5..2586b3df2bb6 100644
--- a/drivers/xen/gntdev.c
+++ b/drivers/xen/gntdev.c
@@ -864,10 +864,7 @@ static int gntdev_get_page(struct gntdev_copy_batch 
*batch, void __user *virt,
  
  static void gntdev_put_pages(struct gntdev_copy_batch *batch)

  {
-   unsigned int i;
-
-   for (i = 0; i < batch->nr_pages; i++)
-   put_page(batch->pages[i]);
+   put_user_pages(batch->pages, batch->nr_pages);
batch->nr_pages = 0;
  }
  
diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c

index 2f5ce7230a43..29e461dbee2d 100644
--- a/drivers/xen/privcmd.c
+++ b/drivers/xen/privcmd.c
@@ -611,15 +611,10 @@ static int lock_pages(
  
  static void unlock_pages(struct page *pages[], unsigned int nr_pages)

  {
-   unsigned int i;
-
if (!pages)
return;
  
-	for (i = 0; i < nr_pages; i++) {

-   if (pages[i])
-   put_page(pages[i]);
-   }
+   put_user_pages(pages, nr_pages);


You are not handling the case where pages[i] is NULL here. Or am I
missing a pending patch to put_user_pages() here?


Juergen
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/2] drm/etnaviv: fix etnaviv_cmdbuf_suballoc_new return value

[Guido Günther]

Hi,
On Fri, Jul 05, 2019 at 07:15:35PM +0200, Lucas Stach wrote:
> The call site expects to get either a valid suballoc or an error
> pointer, so a NULL return will not be treated as an error. Make
> sure to always return a proper error pointer in case something goes
> wrong.
> 
> Signed-off-by: Lucas Stach 
> ---
>  drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.c 
> b/drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.c
> index bb4900bc1c4c..7b77992f31c4 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.c
> @@ -48,8 +48,10 @@ etnaviv_cmdbuf_suballoc_new(struct etnaviv_gpu * gpu)
>  
>   suballoc->vaddr = dma_alloc_wc(gpu->dev, SUBALLOC_SIZE,
>  &suballoc->paddr, GFP_KERNEL);
> - if (!suballoc->vaddr)
> + if (!suballoc->vaddr) {
> + ret = -ENOMEM;
>   goto free_suballoc;
> + }
>  
>   ret = etnaviv_iommu_get_suballoc_va(gpu, suballoc->paddr,
>   &suballoc->vram_node, SUBALLOC_SIZE,
> @@ -64,7 +66,7 @@ etnaviv_cmdbuf_suballoc_new(struct etnaviv_gpu * gpu)
>  free_suballoc:
>   kfree(suballoc);
>  
> - return NULL;
> + return ERR_PTR(ret);
>  }
>  
>  void etnaviv_cmdbuf_suballoc_destroy(struct etnaviv_cmdbuf_suballoc
>  *suballoc)

Reviewed-by: Guido Günther 

Cheers,
 -- Guido

> -- 
> 2.20.1
> 
> ___
> etnaviv mailing list
> etna...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/etnaviv
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 20/34] xen: convert put_page() to put_user_page*()

[John Hubbard]

On 8/1/19 9:36 PM, Juergen Gross wrote:

On 02.08.19 04:19, john.hubb...@gmail.com wrote:

From: John Hubbard 

...

diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c
index 2f5ce7230a43..29e461dbee2d 100644
--- a/drivers/xen/privcmd.c
+++ b/drivers/xen/privcmd.c
@@ -611,15 +611,10 @@ static int lock_pages(
  static void unlock_pages(struct page *pages[], unsigned int nr_pages)
  {
-    unsigned int i;
-
  if (!pages)
  return;
-    for (i = 0; i < nr_pages; i++) {
-    if (pages[i])
-    put_page(pages[i]);
-    }
+    put_user_pages(pages, nr_pages);


You are not handling the case where pages[i] is NULL here. Or am I
missing a pending patch to put_user_pages() here?



Hi Juergen,

You are correct--this no longer handles the cases where pages[i]
is NULL. It's intentional, though possibly wrong. :)

I see that I should have added my standard blurb to this
commit description. I missed this one, but some of the other patches
have it. It makes the following, possibly incorrect claim:

"This changes the release code slightly, because each page slot in the
page_list[] array is no longer checked for NULL. However, that check
was wrong anyway, because the get_user_pages() pattern of usage here
never allowed for NULL entries within a range of pinned pages."

The way I've seen these page arrays used with get_user_pages(),
things are either done single page, or with a contiguous range. So
unless I'm missing a case where someone is either

a) releasing individual pages within a range (and thus likely messing
up their count of pages they have), or

b) allocating two gup ranges within the same pages[] array, with a
gap between the allocations,

...then it should be correct. If so, then I'll add the above blurb
to this patch's commit description.

If that's not the case (both here, and in 3 or 4 other patches in this
series, then as you said, I should add NULL checks to put_user_pages()
and put_user_pages_dirty_lock().


thanks,
--
John Hubbard
NVIDIA
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 12/34] vmci: convert put_page() to put_user_page*()

[john . hubbard]

From: John Hubbard 

For pages that were retained via get_user_pages*(), release those pages
via the new put_user_page*() routines, instead of via put_page() or
release_pages().

This is part a tree-wide conversion, as described in commit fc1d8e7cca2d
("mm: introduce put_user_page*(), placeholder versions").

Note that this effectively changes the code's behavior in
qp_release_pages(): it now ultimately calls set_page_dirty_lock(),
instead of set_page_dirty(). This is probably more accurate.

As Christophe Hellwig put it, "set_page_dirty() is only safe if we are
dealing with a file backed page where we have reference on the inode it
hangs off." [1]

[1] https://lore.kernel.org/r/20190723153640.gb...@lst.de

Cc: Arnd Bergmann 
Cc: Al Viro 
Cc: "Gustavo A. R. Silva" 
Cc: Kees Cook 
Signed-off-by: John Hubbard 
---
 drivers/misc/vmw_vmci/vmci_context.c|  2 +-
 drivers/misc/vmw_vmci/vmci_queue_pair.c | 11 ++-
 2 files changed, 3 insertions(+), 10 deletions(-)

diff --git a/drivers/misc/vmw_vmci/vmci_context.c 
b/drivers/misc/vmw_vmci/vmci_context.c
index 16695366ec92..9daa52ee63b7 100644
--- a/drivers/misc/vmw_vmci/vmci_context.c
+++ b/drivers/misc/vmw_vmci/vmci_context.c
@@ -587,7 +587,7 @@ void vmci_ctx_unset_notify(struct vmci_ctx *context)
 
if (notify_page) {
kunmap(notify_page);
-   put_page(notify_page);
+   put_user_page(notify_page);
}
 }
 
diff --git a/drivers/misc/vmw_vmci/vmci_queue_pair.c 
b/drivers/misc/vmw_vmci/vmci_queue_pair.c
index 8531ae781195..e5434551d0ef 100644
--- a/drivers/misc/vmw_vmci/vmci_queue_pair.c
+++ b/drivers/misc/vmw_vmci/vmci_queue_pair.c
@@ -626,15 +626,8 @@ static void qp_release_queue_mutex(struct vmci_queue 
*queue)
 static void qp_release_pages(struct page **pages,
 u64 num_pages, bool dirty)
 {
-   int i;
-
-   for (i = 0; i < num_pages; i++) {
-   if (dirty)
-   set_page_dirty(pages[i]);
-
-   put_page(pages[i]);
-   pages[i] = NULL;
-   }
+   put_user_pages_dirty_lock(pages, num_pages, dirty);
+   memset(pages, 0, num_pages * sizeof(struct page *));
 }
 
 /*
-- 
2.22.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 07/34] drm/radeon: convert put_page() to put_user_page*()

[john . hubbard]

From: John Hubbard 

For pages that were retained via get_user_pages*(), release those pages
via the new put_user_page*() routines, instead of via put_page() or
release_pages().

This is part a tree-wide conversion, as described in commit fc1d8e7cca2d
("mm: introduce put_user_page*(), placeholder versions").

Cc: Alex Deucher 
Cc: Christian König 
Cc: David (ChunMing) Zhou 
Cc: David Airlie 
Cc: amd-...@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: John Hubbard 
---
 drivers/gpu/drm/radeon/radeon_ttm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c 
b/drivers/gpu/drm/radeon/radeon_ttm.c
index fb3696bc616d..4c9943fa10df 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -540,7 +540,7 @@ static int radeon_ttm_tt_pin_userptr(struct ttm_tt *ttm)
kfree(ttm->sg);
 
 release_pages:
-   release_pages(ttm->pages, pinned);
+   put_user_pages(ttm->pages, pinned);
return r;
 }
 
-- 
2.22.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 16/34] drivers/tee: convert put_page() to put_user_page*()

[Jens Wiklander]

On Fri, Aug 2, 2019 at 4:20 AM  wrote:
>
> From: John Hubbard 
>
> For pages that were retained via get_user_pages*(), release those pages
> via the new put_user_page*() routines, instead of via put_page() or
> release_pages().
>
> This is part a tree-wide conversion, as described in commit fc1d8e7cca2d
> ("mm: introduce put_user_page*(), placeholder versions").
>
> Cc: Jens Wiklander 
> Signed-off-by: John Hubbard 
> ---
>  drivers/tee/tee_shm.c | 10 ++
>  1 file changed, 2 insertions(+), 8 deletions(-)

Acked-by: Jens Wiklander 

I suppose you're taking this via your own tree or such.

Thanks,
Jens

>
> diff --git a/drivers/tee/tee_shm.c b/drivers/tee/tee_shm.c
> index 2da026fd12c9..c967d0420b67 100644
> --- a/drivers/tee/tee_shm.c
> +++ b/drivers/tee/tee_shm.c
> @@ -31,16 +31,13 @@ static void tee_shm_release(struct tee_shm *shm)
>
> poolm->ops->free(poolm, shm);
> } else if (shm->flags & TEE_SHM_REGISTER) {
> -   size_t n;
> int rc = teedev->desc->ops->shm_unregister(shm->ctx, shm);
>
> if (rc)
> dev_err(teedev->dev.parent,
> "unregister shm %p failed: %d", shm, rc);
>
> -   for (n = 0; n < shm->num_pages; n++)
> -   put_page(shm->pages[n]);
> -
> +   put_user_pages(shm->pages, shm->num_pages);
> kfree(shm->pages);
> }
>
> @@ -313,16 +310,13 @@ struct tee_shm *tee_shm_register(struct tee_context 
> *ctx, unsigned long addr,
> return shm;
>  err:
> if (shm) {
> -   size_t n;
> -
> if (shm->id >= 0) {
> mutex_lock(&teedev->mutex);
> idr_remove(&teedev->idr, shm->id);
> mutex_unlock(&teedev->mutex);
> }
> if (shm->pages) {
> -   for (n = 0; n < shm->num_pages; n++)
> -   put_page(shm->pages[n]);
> +   put_user_pages(shm->pages, shm->num_pages);
> kfree(shm->pages);
> }
> }
> --
> 2.22.0
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC PATCH 01/11] devfreq: exynos-bus: Extract exynos_bus_profile_init()

[Artur Świgoń]

Hi,

On Wed, 2019-07-24 at 21:07 +0200, Krzysztof Kozlowski wrote:
> On Tue, Jul 23, 2019 at 02:20:06PM +0200, Artur Świgoń wrote:
> > This patch adds a new static function, exynos_bus_profile_init(), extracted
> > from exynos_bus_probe().
> > 
> > Signed-off-by: Artur Świgoń 
> > ---
> >  drivers/devfreq/exynos-bus.c | 106 ---
> >  1 file changed, 60 insertions(+), 46 deletions(-)
> > 
> > diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
> > index d9f377912c10..d8f1efaf2d49 100644
> > --- a/drivers/devfreq/exynos-bus.c
> > +++ b/drivers/devfreq/exynos-bus.c
> > @@ -372,12 +372,69 @@ static int exynos_bus_parse_of(struct device_node *np,
> > return ret;
> >  }
> >  
> > +static int exynos_bus_profile_init(struct exynos_bus *bus,
> > +  struct devfreq_dev_profile *profile)
> > +{
> > +   struct device *dev = bus->dev;
> > +   struct devfreq_simple_ondemand_data *ondemand_data;
> > +   int ret;
> > +
> > +   /* Initialize the struct profile and governor data for parent device */
> > +   profile->polling_ms = 50;
> > +   profile->target = exynos_bus_target;
> > +   profile->get_dev_status = exynos_bus_get_dev_status;
> > +   profile->exit = exynos_bus_exit;
> > +
> > +   ondemand_data = devm_kzalloc(dev, sizeof(*ondemand_data), GFP_KERNEL);
> > +   if (!ondemand_data) {
> > +   ret = -ENOMEM;
> > +   goto err;
> 
> Just return proper error code. Less lines, obvious code since you do not
> have any cleanup in error path.

I was advised to avoid modifying code being moved (in one patch). I do make
changes in these places in patch 04/11, i.e. change the original label 'err' to
'out'. What's your opinion on making the proposed changes to patches 01 and 02
(s/goto err/return ret/) in patch 04 instead?

> > +
> > +   /* Register opp_notifier to catch the change of OPP  */
> > +   ret = devm_devfreq_register_opp_notifier(dev, bus->devfreq);
> > +   if (ret < 0) {
> > +   dev_err(dev, "failed to register opp notifier\n");
> > +   goto err;
> 
> The same - return err.
> 
> Best regards,
> Krzysztof

Best regards,
-- 
Artur Świgoń
Samsung R&D Institute Poland
Samsung Electronics


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v19 00/15] arm64: untag user pointers passed to the kernel

[Kevin Brodsky]

On 31/07/2019 17:50, Dave Hansen wrote:

On 7/23/19 10:58 AM, Andrey Konovalov wrote:

The mmap and mremap (only new_addr) syscalls do not currently accept
tagged addresses. Architectures may interpret the tag as a background
colour for the corresponding vma.

What the heck is a "background colour"? :)


Good point, this is some jargon that we started using for MTE, the idea being that 
the kernel could set a tag value (specified during mmap()) as "background colour" for 
anonymous pages allocated in that range.


Anyway, this patch series is not about MTE. Andrey, for v20 (if any), I think it's 
best to drop this last sentence to avoid any confusion.


Kevin
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 01/34] mm/gup: add make_dirty arg to put_user_pages_dirty_lock()

[john . hubbard]

From: John Hubbard 

Provide more capable variation of put_user_pages_dirty_lock(),
and delete put_user_pages_dirty(). This is based on the
following:

1. Lots of call sites become simpler if a bool is passed
into put_user_page*(), instead of making the call site
choose which put_user_page*() variant to call.

2. Christoph Hellwig's observation that set_page_dirty_lock()
is usually correct, and set_page_dirty() is usually a
bug, or at least questionable, within a put_user_page*()
calling chain.

This leads to the following API choices:

* put_user_pages_dirty_lock(page, npages, make_dirty)

* There is no put_user_pages_dirty(). You have to
  hand code that, in the rare case that it's
  required.

Cc: Matthew Wilcox 
Cc: Jan Kara 
Cc: Christoph Hellwig 
Cc: Ira Weiny 
Cc: Jason Gunthorpe 
Signed-off-by: John Hubbard 
---
 drivers/infiniband/core/umem.c |   5 +-
 drivers/infiniband/hw/hfi1/user_pages.c|   5 +-
 drivers/infiniband/hw/qib/qib_user_pages.c |   5 +-
 drivers/infiniband/hw/usnic/usnic_uiom.c   |   5 +-
 drivers/infiniband/sw/siw/siw_mem.c|  10 +-
 include/linux/mm.h |   5 +-
 mm/gup.c   | 115 +
 7 files changed, 58 insertions(+), 92 deletions(-)

diff --git a/drivers/infiniband/core/umem.c b/drivers/infiniband/core/umem.c
index 08da840ed7ee..965cf9dea71a 100644
--- a/drivers/infiniband/core/umem.c
+++ b/drivers/infiniband/core/umem.c
@@ -54,10 +54,7 @@ static void __ib_umem_release(struct ib_device *dev, struct 
ib_umem *umem, int d
 
for_each_sg_page(umem->sg_head.sgl, &sg_iter, umem->sg_nents, 0) {
page = sg_page_iter_page(&sg_iter);
-   if (umem->writable && dirty)
-   put_user_pages_dirty_lock(&page, 1);
-   else
-   put_user_page(page);
+   put_user_pages_dirty_lock(&page, 1, umem->writable && dirty);
}
 
sg_free_table(&umem->sg_head);
diff --git a/drivers/infiniband/hw/hfi1/user_pages.c 
b/drivers/infiniband/hw/hfi1/user_pages.c
index b89a9b9aef7a..469acb961fbd 100644
--- a/drivers/infiniband/hw/hfi1/user_pages.c
+++ b/drivers/infiniband/hw/hfi1/user_pages.c
@@ -118,10 +118,7 @@ int hfi1_acquire_user_pages(struct mm_struct *mm, unsigned 
long vaddr, size_t np
 void hfi1_release_user_pages(struct mm_struct *mm, struct page **p,
 size_t npages, bool dirty)
 {
-   if (dirty)
-   put_user_pages_dirty_lock(p, npages);
-   else
-   put_user_pages(p, npages);
+   put_user_pages_dirty_lock(p, npages, dirty);
 
if (mm) { /* during close after signal, mm can be NULL */
atomic64_sub(npages, &mm->pinned_vm);
diff --git a/drivers/infiniband/hw/qib/qib_user_pages.c 
b/drivers/infiniband/hw/qib/qib_user_pages.c
index bfbfbb7e0ff4..6bf764e41891 100644
--- a/drivers/infiniband/hw/qib/qib_user_pages.c
+++ b/drivers/infiniband/hw/qib/qib_user_pages.c
@@ -40,10 +40,7 @@
 static void __qib_release_user_pages(struct page **p, size_t num_pages,
 int dirty)
 {
-   if (dirty)
-   put_user_pages_dirty_lock(p, num_pages);
-   else
-   put_user_pages(p, num_pages);
+   put_user_pages_dirty_lock(p, num_pages, dirty);
 }
 
 /**
diff --git a/drivers/infiniband/hw/usnic/usnic_uiom.c 
b/drivers/infiniband/hw/usnic/usnic_uiom.c
index 0b0237d41613..62e6ffa9ad78 100644
--- a/drivers/infiniband/hw/usnic/usnic_uiom.c
+++ b/drivers/infiniband/hw/usnic/usnic_uiom.c
@@ -75,10 +75,7 @@ static void usnic_uiom_put_pages(struct list_head 
*chunk_list, int dirty)
for_each_sg(chunk->page_list, sg, chunk->nents, i) {
page = sg_page(sg);
pa = sg_phys(sg);
-   if (dirty)
-   put_user_pages_dirty_lock(&page, 1);
-   else
-   put_user_page(page);
+   put_user_pages_dirty_lock(&page, 1, dirty);
usnic_dbg("pa: %pa\n", &pa);
}
kfree(chunk);
diff --git a/drivers/infiniband/sw/siw/siw_mem.c 
b/drivers/infiniband/sw/siw/siw_mem.c
index 67171c82b0c4..ab83a9cec562 100644
--- a/drivers/infiniband/sw/siw/siw_mem.c
+++ b/drivers/infiniband/sw/siw/siw_mem.c
@@ -63,15 +63,7 @@ struct siw_mem *siw_mem_id2obj(struct siw_device *sdev, int 
stag_index)
 static void siw_free_plist(struct siw_page_chunk *chunk, int num_pages,
   bool dirty)
 {
-   struct page **p = chunk->plist;
-
-   while (num_pages--) {
-   if (!PageDirty(*p) && dirty)
-   put_user_pages_dirty_lock(p, 1);
-   else
-   put_user_page(*p);
-   p++;
-   }
+   put_user_pages_dirty_lock(chunk->plist, num_pages, dirty);
 }
 
 void siw_umem_release(struct siw_umem *ume

[PATCH 24/34] futex: convert put_page() to put_user_page*()

[john . hubbard]

From: John Hubbard 

For pages that were retained via get_user_pages*(), release those pages
via the new put_user_page*() routines, instead of via put_page() or
release_pages().

This is part a tree-wide conversion, as described in commit fc1d8e7cca2d
("mm: introduce put_user_page*(), placeholder versions").

Cc: Thomas Gleixner 
Cc: Ingo Molnar 
Cc: Peter Zijlstra 
Cc: Darren Hart 
Signed-off-by: John Hubbard 
---
 kernel/futex.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/kernel/futex.c b/kernel/futex.c
index 6d50728ef2e7..4b4cae58ec57 100644
--- a/kernel/futex.c
+++ b/kernel/futex.c
@@ -623,7 +623,7 @@ get_futex_key(u32 __user *uaddr, int fshared, union 
futex_key *key, enum futex_a
lock_page(page);
shmem_swizzled = PageSwapCache(page) || page->mapping;
unlock_page(page);
-   put_page(page);
+   put_user_page(page);
 
if (shmem_swizzled)
goto again;
@@ -675,7 +675,7 @@ get_futex_key(u32 __user *uaddr, int fshared, union 
futex_key *key, enum futex_a
 
if (READ_ONCE(page->mapping) != mapping) {
rcu_read_unlock();
-   put_page(page);
+   put_user_page(page);
 
goto again;
}
@@ -683,7 +683,7 @@ get_futex_key(u32 __user *uaddr, int fshared, union 
futex_key *key, enum futex_a
inode = READ_ONCE(mapping->host);
if (!inode) {
rcu_read_unlock();
-   put_page(page);
+   put_user_page(page);
 
goto again;
}
@@ -702,7 +702,7 @@ get_futex_key(u32 __user *uaddr, int fshared, union 
futex_key *key, enum futex_a
 */
if (!atomic_inc_not_zero(&inode->i_count)) {
rcu_read_unlock();
-   put_page(page);
+   put_user_page(page);
 
goto again;
}
@@ -723,7 +723,7 @@ get_futex_key(u32 __user *uaddr, int fshared, union 
futex_key *key, enum futex_a
}
 
 out:
-   put_page(page);
+   put_user_page(page);
return err;
 }
 
-- 
2.22.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 26/34] mm/gup_benchmark.c: convert put_page() to put_user_page*()

[john . hubbard]

From: John Hubbard 

For pages that were retained via get_user_pages*(), release those pages
via the new put_user_page*() routines, instead of via put_page() or
release_pages().

This is part a tree-wide conversion, as described in commit fc1d8e7cca2d
("mm: introduce put_user_page*(), placeholder versions").

Cc: Dan Carpenter 
Cc: Greg Kroah-Hartman 
Cc: Keith Busch 
Cc: Kirill A. Shutemov 
Cc: Michael S. Tsirkin 
Cc: YueHaibing 
Signed-off-by: John Hubbard 
---
 mm/gup_benchmark.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/gup_benchmark.c b/mm/gup_benchmark.c
index 7dd602d7f8db..515ac8eeb6ee 100644
--- a/mm/gup_benchmark.c
+++ b/mm/gup_benchmark.c
@@ -79,7 +79,7 @@ static int __gup_benchmark_ioctl(unsigned int cmd,
for (i = 0; i < nr_pages; i++) {
if (!pages[i])
break;
-   put_page(pages[i]);
+   put_user_page(pages[i]);
}
end_time = ktime_get();
gup->put_delta_usec = ktime_us_delta(end_time, start_time);
-- 
2.22.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 02/34] net/rds: convert put_page() to put_user_page*()

[john . hubbard]

From: John Hubbard 

For pages that were retained via get_user_pages*(), release those pages
via the new put_user_page*() routines, instead of via put_page() or
release_pages().

This is part a tree-wide conversion, as described in commit fc1d8e7cca2d
("mm: introduce put_user_page*(), placeholder versions").

Cc: Santosh Shilimkar 
Cc: David S. Miller 
Cc: net...@vger.kernel.org
Cc: linux-r...@vger.kernel.org
Cc: rds-de...@oss.oracle.com
Signed-off-by: John Hubbard 
---
 net/rds/info.c|  5 ++---
 net/rds/message.c |  2 +-
 net/rds/rdma.c| 15 +++
 3 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/net/rds/info.c b/net/rds/info.c
index 03f6fd56d237..ca6af2889adf 100644
--- a/net/rds/info.c
+++ b/net/rds/info.c
@@ -162,7 +162,6 @@ int rds_info_getsockopt(struct socket *sock, int optname, 
char __user *optval,
struct rds_info_lengths lens;
unsigned long nr_pages = 0;
unsigned long start;
-   unsigned long i;
rds_info_func func;
struct page **pages = NULL;
int ret;
@@ -235,8 +234,8 @@ int rds_info_getsockopt(struct socket *sock, int optname, 
char __user *optval,
ret = -EFAULT;
 
 out:
-   for (i = 0; pages && i < nr_pages; i++)
-   put_page(pages[i]);
+   if (pages)
+   put_user_pages(pages, nr_pages);
kfree(pages);
 
return ret;
diff --git a/net/rds/message.c b/net/rds/message.c
index 50f13f1d4ae0..d7b0d266c437 100644
--- a/net/rds/message.c
+++ b/net/rds/message.c
@@ -404,7 +404,7 @@ static int rds_message_zcopy_from_user(struct rds_message 
*rm, struct iov_iter *
int i;
 
for (i = 0; i < rm->data.op_nents; i++)
-   put_page(sg_page(&rm->data.op_sg[i]));
+   put_user_page(sg_page(&rm->data.op_sg[i]));
mmp = &rm->data.op_mmp_znotifier->z_mmp;
mm_unaccount_pinned_pages(mmp);
ret = -EFAULT;
diff --git a/net/rds/rdma.c b/net/rds/rdma.c
index 916f5ec373d8..6762e8696b99 100644
--- a/net/rds/rdma.c
+++ b/net/rds/rdma.c
@@ -162,8 +162,7 @@ static int rds_pin_pages(unsigned long user_addr, unsigned 
int nr_pages,
  pages);
 
if (ret >= 0 && ret < nr_pages) {
-   while (ret--)
-   put_page(pages[ret]);
+   put_user_pages(pages, ret);
ret = -EFAULT;
}
 
@@ -276,7 +275,7 @@ static int __rds_rdma_map(struct rds_sock *rs, struct 
rds_get_mr_args *args,
 
if (IS_ERR(trans_private)) {
for (i = 0 ; i < nents; i++)
-   put_page(sg_page(&sg[i]));
+   put_user_page(sg_page(&sg[i]));
kfree(sg);
ret = PTR_ERR(trans_private);
goto out;
@@ -464,9 +463,10 @@ void rds_rdma_free_op(struct rm_rdma_op *ro)
 * to local memory */
if (!ro->op_write) {
WARN_ON(!page->mapping && irqs_disabled());
-   set_page_dirty(page);
+   put_user_pages_dirty_lock(&page, 1, true);
+   } else {
+   put_user_page(page);
}
-   put_page(page);
}
 
kfree(ro->op_notifier);
@@ -481,8 +481,7 @@ void rds_atomic_free_op(struct rm_atomic_op *ao)
/* Mark page dirty if it was possibly modified, which
 * is the case for a RDMA_READ which copies from remote
 * to local memory */
-   set_page_dirty(page);
-   put_page(page);
+   put_user_pages_dirty_lock(&page, 1, true);
 
kfree(ao->op_notifier);
ao->op_notifier = NULL;
@@ -867,7 +866,7 @@ int rds_cmsg_atomic(struct rds_sock *rs, struct rds_message 
*rm,
return ret;
 err:
if (page)
-   put_page(page);
+   put_user_page(page);
rm->atomic.op_active = 0;
kfree(rm->atomic.op_notifier);
 
-- 
2.22.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 14/34] oradax: convert put_page() to put_user_page*()

[john . hubbard]

From: John Hubbard 

For pages that were retained via get_user_pages*(), release those pages
via the new put_user_page*() routines, instead of via put_page() or
release_pages().

This is part a tree-wide conversion, as described in commit fc1d8e7cca2d
("mm: introduce put_user_page*(), placeholder versions").

Cc: David S. Miller 
Cc: Jonathan Helman 
Cc: Rob Gardner 
Cc: Andy Shevchenko 
Cc: Jonathan Corbet 
Cc: Wei Yongjun 
Cc: Mauro Carvalho Chehab 
Cc: sparcli...@vger.kernel.org
Signed-off-by: John Hubbard 
---
 drivers/sbus/char/oradax.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/sbus/char/oradax.c b/drivers/sbus/char/oradax.c
index 8af216287a84..029e619992fc 100644
--- a/drivers/sbus/char/oradax.c
+++ b/drivers/sbus/char/oradax.c
@@ -412,7 +412,7 @@ static void dax_unlock_pages(struct dax_ctx *ctx, int 
ccb_index, int nelem)
dax_dbg("freeing page %p", p);
if (j == OUT)
set_page_dirty(p);
-   put_page(p);
+   put_user_page(p);
ctx->pages[i][j] = NULL;
}
}
-- 
2.22.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 33/34] kernel/events/core.c: convert put_page() to put_user_page*()

[john . hubbard]

From: John Hubbard 

For pages that were retained via get_user_pages*(), release those pages
via the new put_user_page*() routines, instead of via put_page() or
release_pages().

This is part a tree-wide conversion, as described in commit fc1d8e7cca2d
("mm: introduce put_user_page*(), placeholder versions").

Cc: Peter Zijlstra 
Cc: Ingo Molnar 
Cc: Arnaldo Carvalho de Melo 
Cc: Alexander Shishkin 
Cc: Jiri Olsa 
Cc: Namhyung Kim 
Signed-off-by: John Hubbard 
---
 kernel/events/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index 0463c1151bae..7be52bbbfe87 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -6426,7 +6426,7 @@ static u64 perf_virt_to_phys(u64 virt)
phys_addr = page_to_phys(p) + virt % PAGE_SIZE;
 
if (p)
-   put_page(p);
+   put_user_page(p);
}
 
return phys_addr;
-- 
2.22.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 03/34] net/ceph: convert put_page() to put_user_page*()

[john . hubbard]

From: John Hubbard 

For pages that were retained via get_user_pages*(), release those pages
via the new put_user_page*() routines, instead of via put_page() or
release_pages().

This is part a tree-wide conversion, as described in commit fc1d8e7cca2d
("mm: introduce put_user_page*(), placeholder versions").

Cc: Ilya Dryomov 
Cc: Sage Weil 
Cc: David S. Miller 
Cc: ceph-de...@vger.kernel.org
Cc: net...@vger.kernel.org
Signed-off-by: John Hubbard 
---
 net/ceph/pagevec.c | 8 +---
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/net/ceph/pagevec.c b/net/ceph/pagevec.c
index 64305e7056a1..c88fff2ab9bd 100644
--- a/net/ceph/pagevec.c
+++ b/net/ceph/pagevec.c
@@ -12,13 +12,7 @@
 
 void ceph_put_page_vector(struct page **pages, int num_pages, bool dirty)
 {
-   int i;
-
-   for (i = 0; i < num_pages; i++) {
-   if (dirty)
-   set_page_dirty_lock(pages[i]);
-   put_page(pages[i]);
-   }
+   put_user_pages_dirty_lock(pages, num_pages, dirty);
kvfree(pages);
 }
 EXPORT_SYMBOL(ceph_put_page_vector);
-- 
2.22.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH/RFC 08/12] drm: rcar-du: lvds: Fix bridge_to_rcar_lvds

[Laurent Pinchart]

Hi Fabrizio,

Thank you for the patch.

On Fri, Aug 02, 2019 at 08:34:05AM +0100, Fabrizio Castro wrote:
> Using name "bridge" for macro bridge_to_rcar_lvds argument doesn't
> work when the pointer name used by the caller is not "bridge".
> Rename the argument to "bridge_ptr" to allow for any pointer
> name.
> 
> Fixes: c6a27fa41fab ("drm: rcar-du: Convert LVDS encoder code to bridge 
> driver")
> Signed-off-by: Fabrizio Castro 
> ---
>  drivers/gpu/drm/rcar-du/rcar_lvds.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c 
> b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> index 97c51c2..edd63f5 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> @@ -72,8 +72,8 @@ struct rcar_lvds {
>   bool stripe_swap_data;
>  };
>  
> -#define bridge_to_rcar_lvds(bridge) \
> - container_of(bridge, struct rcar_lvds, bridge)
> +#define bridge_to_rcar_lvds(bridge_ptr) \
> + container_of(bridge_ptr, struct rcar_lvds, bridge)

How about just 'b' instead of 'bridge_ptr' ? If that's fine with you
I'll take the modified patch in my tree, no need to resubmit.

Reviewed-by: Laurent Pinchart 

>  
>  #define connector_to_rcar_lvds(connector) \
>   container_of(connector, struct rcar_lvds, connector)

-- 
Regards,

Laurent Pinchart
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/2] drm/etnaviv: remove unused function etnaviv_gem_mapping_reference

[Guido Günther]

Hi,
On Fri, Jul 05, 2019 at 07:15:36PM +0200, Lucas Stach wrote:
> Hasn't been used for quite a while. There is no point in keeping
> unused code around.
> 
> Signed-off-by: Lucas Stach 
> ---
>  drivers/gpu/drm/etnaviv/etnaviv_gem.c | 12 
>  drivers/gpu/drm/etnaviv/etnaviv_gem.h |  1 -
>  2 files changed, 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c 
> b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> index 727bb3f5ceb2..e199a6833ff0 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> @@ -235,18 +235,6 @@ etnaviv_gem_get_vram_mapping(struct etnaviv_gem_object 
> *obj,
>   return NULL;
>  }
>  
> -void etnaviv_gem_mapping_reference(struct etnaviv_vram_mapping *mapping)
> -{
> - struct etnaviv_gem_object *etnaviv_obj = mapping->object;
> -
> - drm_gem_object_get(&etnaviv_obj->base);
> -
> - mutex_lock(&etnaviv_obj->lock);
> - WARN_ON(mapping->use == 0);
> - mapping->use += 1;
> - mutex_unlock(&etnaviv_obj->lock);
> -}
> -
>  void etnaviv_gem_mapping_unreference(struct etnaviv_vram_mapping *mapping)
>  {
>   struct etnaviv_gem_object *etnaviv_obj = mapping->object;
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.h 
> b/drivers/gpu/drm/etnaviv/etnaviv_gem.h
> index 753c458497d0..d7d8a835f379 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.h
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.h
> @@ -120,7 +120,6 @@ void etnaviv_gem_put_pages(struct etnaviv_gem_object 
> *obj);
>  
>  struct etnaviv_vram_mapping *etnaviv_gem_mapping_get(
>   struct drm_gem_object *obj, struct etnaviv_gpu *gpu);
> -void etnaviv_gem_mapping_reference(struct etnaviv_vram_mapping *mapping);
>  void etnaviv_gem_mapping_unreference(struct etnaviv_vram_mapping *mapping);
>  
>  #endif /* __ETNAVIV_GEM_H__ */

Reviewed-by: Guido Günther  
cheers,
 -- Guido

> -- 
> 2.20.1
> 
> ___
> etnaviv mailing list
> etna...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/etnaviv
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 20/34] xen: convert put_page() to put_user_page*()

[Juergen Gross]

On 02.08.19 07:48, John Hubbard wrote:

On 8/1/19 9:36 PM, Juergen Gross wrote:

On 02.08.19 04:19, john.hubb...@gmail.com wrote:

From: John Hubbard 

...

diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c
index 2f5ce7230a43..29e461dbee2d 100644
--- a/drivers/xen/privcmd.c
+++ b/drivers/xen/privcmd.c
@@ -611,15 +611,10 @@ static int lock_pages(
  static void unlock_pages(struct page *pages[], unsigned int nr_pages)
  {
-    unsigned int i;
-
  if (!pages)
  return;
-    for (i = 0; i < nr_pages; i++) {
-    if (pages[i])
-    put_page(pages[i]);
-    }
+    put_user_pages(pages, nr_pages);


You are not handling the case where pages[i] is NULL here. Or am I
missing a pending patch to put_user_pages() here?



Hi Juergen,

You are correct--this no longer handles the cases where pages[i]
is NULL. It's intentional, though possibly wrong. :)

I see that I should have added my standard blurb to this
commit description. I missed this one, but some of the other patches
have it. It makes the following, possibly incorrect claim:

"This changes the release code slightly, because each page slot in the
page_list[] array is no longer checked for NULL. However, that check
was wrong anyway, because the get_user_pages() pattern of usage here
never allowed for NULL entries within a range of pinned pages."

The way I've seen these page arrays used with get_user_pages(),
things are either done single page, or with a contiguous range. So
unless I'm missing a case where someone is either

a) releasing individual pages within a range (and thus likely messing
up their count of pages they have), or

b) allocating two gup ranges within the same pages[] array, with a
gap between the allocations,

...then it should be correct. If so, then I'll add the above blurb
to this patch's commit description.

If that's not the case (both here, and in 3 or 4 other patches in this
series, then as you said, I should add NULL checks to put_user_pages()
and put_user_pages_dirty_lock().


In this case it is not correct, but can easily be handled. The NULL case
can occur only in an error case with the pages array filled partially or
not at all.

I'd prefer something like the attached patch here.


Juergen
diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c
index 2f5ce7230a43..12bd3154126d 100644
--- a/drivers/xen/privcmd.c
+++ b/drivers/xen/privcmd.c
@@ -582,10 +582,11 @@ static long privcmd_ioctl_mmap_batch(
 
 static int lock_pages(
 	struct privcmd_dm_op_buf kbufs[], unsigned int num,
-	struct page *pages[], unsigned int nr_pages)
+	struct page *pages[], unsigned int *nr_pages)
 {
-	unsigned int i;
+	unsigned int i, free = *nr_pages;
 
+	*nr_pages = 0;
 	for (i = 0; i < num; i++) {
 		unsigned int requested;
 		int pinned;
@@ -593,35 +594,22 @@ static int lock_pages(
 		requested = DIV_ROUND_UP(
 			offset_in_page(kbufs[i].uptr) + kbufs[i].size,
 			PAGE_SIZE);
-		if (requested > nr_pages)
+		if (requested > free)
 			return -ENOSPC;
 
 		pinned = get_user_pages_fast(
 			(unsigned long) kbufs[i].uptr,
-			requested, FOLL_WRITE, pages);
+			requested, FOLL_WRITE, pages + *nr_pages);
 		if (pinned < 0)
 			return pinned;
 
-		nr_pages -= pinned;
-		pages += pinned;
+		free -= pinned;
+		*nr_pages += pinned;
 	}
 
 	return 0;
 }
 
-static void unlock_pages(struct page *pages[], unsigned int nr_pages)
-{
-	unsigned int i;
-
-	if (!pages)
-		return;
-
-	for (i = 0; i < nr_pages; i++) {
-		if (pages[i])
-			put_page(pages[i]);
-	}
-}
-
 static long privcmd_ioctl_dm_op(struct file *file, void __user *udata)
 {
 	struct privcmd_data *data = file->private_data;
@@ -681,11 +669,12 @@ static long privcmd_ioctl_dm_op(struct file *file, void __user *udata)
 
 	xbufs = kcalloc(kdata.num, sizeof(*xbufs), GFP_KERNEL);
 	if (!xbufs) {
+		nr_pages = 0;
 		rc = -ENOMEM;
 		goto out;
 	}
 
-	rc = lock_pages(kbufs, kdata.num, pages, nr_pages);
+	rc = lock_pages(kbufs, kdata.num, pages, &nr_pages);
 	if (rc)
 		goto out;
 
@@ -699,7 +688,8 @@ static long privcmd_ioctl_dm_op(struct file *file, void __user *udata)
 	xen_preemptible_hcall_end();
 
 out:
-	unlock_pages(pages, nr_pages);
+	if (pages)
+		put_user_pages(pages, nr_pages);
 	kfree(xbufs);
 	kfree(pages);
 	kfree(kbufs);
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 5/9] nouveau: simplify nouveau_dmem_migrate_to_ram

[Bharata B Rao]

On Mon, Jul 29, 2019 at 05:28:39PM +0300, Christoph Hellwig wrote:
> Factor the main copy page to ram routine out into a helper that acts on
> a single page and which doesn't require the nouveau_dmem_fault
> structure for argument passing.  Also remove the loop over multiple
> pages as we only handle one at the moment, although the structure of
> the main worker function makes it relatively easy to add multi page
> support back if needed in the future.  But at least for now this avoid
> the needed to dynamically allocate memory for the dma addresses in
> what is essentially the page fault path.
> 
> Signed-off-by: Christoph Hellwig 
> ---
>  drivers/gpu/drm/nouveau/nouveau_dmem.c | 158 ++---
>  1 file changed, 39 insertions(+), 119 deletions(-)
> 
> diff --git a/drivers/gpu/drm/nouveau/nouveau_dmem.c 
> b/drivers/gpu/drm/nouveau/nouveau_dmem.c
> index 21052a4aaf69..036e6c07d489 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_dmem.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_dmem.c
> @@ -86,13 +86,6 @@ static inline struct nouveau_dmem *page_to_dmem(struct 
> page *page)
>   return container_of(page->pgmap, struct nouveau_dmem, pagemap);
>  }
>  
> -struct nouveau_dmem_fault {
> - struct nouveau_drm *drm;
> - struct nouveau_fence *fence;
> - dma_addr_t *dma;
> - unsigned long npages;
> -};
> -
>  struct nouveau_migrate {
>   struct vm_area_struct *vma;
>   struct nouveau_drm *drm;
> @@ -146,130 +139,55 @@ static void nouveau_dmem_fence_done(struct 
> nouveau_fence **fence)
>   }
>  }
>  
> -static void
> -nouveau_dmem_fault_alloc_and_copy(struct vm_area_struct *vma,
> -   const unsigned long *src_pfns,
> -   unsigned long *dst_pfns,
> -   unsigned long start,
> -   unsigned long end,
> -   struct nouveau_dmem_fault *fault)
> +static vm_fault_t nouveau_dmem_fault_copy_one(struct nouveau_drm *drm,
> + struct vm_area_struct *vma, unsigned long addr,
> + unsigned long src, unsigned long *dst, dma_addr_t *dma_addr)
>  {
> - struct nouveau_drm *drm = fault->drm;
>   struct device *dev = drm->dev->dev;
> - unsigned long addr, i, npages = 0;
> - nouveau_migrate_copy_t copy;
> - int ret;
> -
> -
> - /* First allocate new memory */
> - for (addr = start, i = 0; addr < end; addr += PAGE_SIZE, i++) {
> - struct page *dpage, *spage;
> -
> - dst_pfns[i] = 0;
> - spage = migrate_pfn_to_page(src_pfns[i]);
> - if (!spage || !(src_pfns[i] & MIGRATE_PFN_MIGRATE))
> - continue;
> + struct page *dpage, *spage;
>  
> - dpage = alloc_page_vma(GFP_HIGHUSER, vma, addr);
> - if (!dpage) {
> - dst_pfns[i] = MIGRATE_PFN_ERROR;
> - continue;
> - }
> - lock_page(dpage);
> -
> - dst_pfns[i] = migrate_pfn(page_to_pfn(dpage)) |
> -   MIGRATE_PFN_LOCKED;
> - npages++;
> - }
> + spage = migrate_pfn_to_page(src);
> + if (!spage || !(src & MIGRATE_PFN_MIGRATE))
> + return 0;
>  
> - /* Allocate storage for DMA addresses, so we can unmap later. */
> - fault->dma = kmalloc(sizeof(*fault->dma) * npages, GFP_KERNEL);
> - if (!fault->dma)
> + dpage = alloc_page_vma(GFP_HIGHUSER, args->vma, addr);
> + if (!dpage)
>   goto error;
> + lock_page(dpage);
>  
> - /* Copy things over */
> - copy = drm->dmem->migrate.copy_func;
> - for (addr = start, i = 0; addr < end; addr += PAGE_SIZE, i++) {
> - struct page *spage, *dpage;
> -
> - dpage = migrate_pfn_to_page(dst_pfns[i]);
> - if (!dpage || dst_pfns[i] == MIGRATE_PFN_ERROR)
> - continue;
> -
> - spage = migrate_pfn_to_page(src_pfns[i]);
> - if (!spage || !(src_pfns[i] & MIGRATE_PFN_MIGRATE)) {
> - dst_pfns[i] = MIGRATE_PFN_ERROR;
> - __free_page(dpage);
> - continue;
> - }
> -
> - fault->dma[fault->npages] =
> - dma_map_page_attrs(dev, dpage, 0, PAGE_SIZE,
> -PCI_DMA_BIDIRECTIONAL,
> -DMA_ATTR_SKIP_CPU_SYNC);
> - if (dma_mapping_error(dev, fault->dma[fault->npages])) {
> - dst_pfns[i] = MIGRATE_PFN_ERROR;
> - __free_page(dpage);
> - continue;
> - }
> -
> - ret = copy(drm, 1, NOUVEAU_APER_HOST,
> - fault->dma[fault->npages++],
> - NOUVEAU_APER_VRAM,
> - nouveau_dmem_page_addr(spage));
> - if (ret) {
> - dst_pfns[i] = MIGRATE_PFN

[PATCH 34/34] fs/binfmt_elf: convert put_page() to put_user_page*()

[john . hubbard]

From: Ira Weiny 

For pages that were retained via get_user_pages*(), release those pages
via the new put_user_page*() routines, instead of via put_page() or
release_pages().

This is part a tree-wide conversion, as described in commit fc1d8e7cca2d
("mm: introduce put_user_page*(), placeholder versions").

get_dump_page calls get_user_page so put_user_page must be used
to match.

Signed-off-by: Ira Weiny 
Signed-off-by: John Hubbard 
---
 fs/binfmt_elf.c   | 2 +-
 fs/binfmt_elf_fdpic.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index d4e11b2e04f6..92e4a5ca99d8 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -2377,7 +2377,7 @@ static int elf_core_dump(struct coredump_params *cprm)
void *kaddr = kmap(page);
stop = !dump_emit(cprm, kaddr, PAGE_SIZE);
kunmap(page);
-   put_page(page);
+   put_user_page(page);
} else
stop = !dump_skip(cprm, PAGE_SIZE);
if (stop)
diff --git a/fs/binfmt_elf_fdpic.c b/fs/binfmt_elf_fdpic.c
index d86ebd0dcc3d..321724b3be22 100644
--- a/fs/binfmt_elf_fdpic.c
+++ b/fs/binfmt_elf_fdpic.c
@@ -1511,7 +1511,7 @@ static bool elf_fdpic_dump_segments(struct 
coredump_params *cprm)
void *kaddr = kmap(page);
res = dump_emit(cprm, kaddr, PAGE_SIZE);
kunmap(page);
-   put_page(page);
+   put_user_page(page);
} else {
res = dump_skip(cprm, PAGE_SIZE);
}
-- 
2.22.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [LKP] [drm/mgag200] 90f479ae51: vm-scalability.median -18.8% regression

[Thomas Zimmermann]

Hi

Am 02.08.19 um 09:11 schrieb Rong Chen:
> Hi,
> 
> On 8/1/19 7:58 PM, Thomas Zimmermann wrote:
>> Hi
>>
>> Am 01.08.19 um 13:25 schrieb Feng Tang:
>>> Hi Thomas,
>>>
>>> On Thu, Aug 01, 2019 at 11:59:28AM +0200, Thomas Zimmermann wrote:
 Hi

 Am 01.08.19 um 10:37 schrieb Feng Tang:
> On Thu, Aug 01, 2019 at 02:19:53PM +0800, Rong Chen wrote:
>>> commit: 90f479ae51afa45efab97afdde9b94b9660dd3e4
>>> ("drm/mgag200: Replace struct mga_fbdev with generic
>>> framebuffer emulation")
>>> https://kernel.googlesource.com/pub/scm/linux/kernel/git/next/linux-next.git
>>> master
>> Daniel, Noralf, we may have to revert this patch.
>>
>> I expected some change in display performance, but not in
>> VM. Since it's
>> a server chipset, probably no one cares much about display
>> performance.
>> So that seemed like a good trade-off for re-using shared
>> code.
>>
>> Part of the patch set is that the generic fb emulation now
>> maps and
>> unmaps the fbdev BO when updating the screen. I guess
>> that's the cause
>> of the performance regression. And it should be visible
>> with other
>> drivers as well if they use a shadow FB for fbdev emulation.
> For fbcon we should need to do any maps/unamps at all, this
> is for the
> fbdev mmap support only. If the testcase mentioned here
> tests fbdev
> mmap handling it's pretty badly misnamed :-) And as long as
> you don't
> have an fbdev mmap there shouldn't be any impact at all.
 The ast and mgag200 have only a few MiB of VRAM, so we have
 to get the
 fbdev BO out if it's not being displayed. If not being
 mapped, it can be
 evicted and make room for X, etc.

 To make this work, the BO's memory is mapped and unmapped in
 drm_fb_helper_dirty_work() before being updated from the
 shadow FB. [1]
 That fbdev mapping is established on each screen update,
 more or less.
  From my (yet unverified) understanding, this causes the
 performance
 regression in the VM code.

 The original code in mgag200 used to kmap the fbdev BO while
 it's being
 displayed; [2] and the drawing code only mapped it when
 necessary (i.e.,
 not being display). [3]
>>> Hm yeah, this vmap/vunmap is going to be pretty bad. We
>>> indeed should
>>> cache this.
>>>
 I think this could be added for VRAM helpers as well, but
 it's still a
 workaround and non-VRAM drivers might also run into such a
 performance
 regression if they use the fbdev's shadow fb.
>>> Yeah agreed, fbdev emulation should try to cache the vmap.
>>>
 Noralf mentioned that there are plans for other DRM clients
 besides the
 console. They would as well run into similar problems.

>> The thing is that we'd need another generic fbdev
>> emulation for ast and
>> mgag200 that handles this issue properly.
> Yeah I dont think we want to jump the gun here.  If you can
> try to
> repro locally and profile where we're wasting cpu time I
> hope that
> should sched a light what's going wrong here.
 I don't have much time ATM and I'm not even officially at
 work until
 late Aug. I'd send you the revert and investigate later. I
 agree that
 using generic fbdev emulation would be preferable.
>>> Still not sure that's the right thing to do really. Yes it's a
>>> regression, but vm testcases shouldn run a single line of
>>> fbcon or drm
>>> code. So why this is impacted so heavily by a silly drm
>>> change is very
>>> confusing to me. We might be papering over a deeper and much
>>> more
>>> serious issue ...
>> It's a regression, the right thing is to revert first and then
>> work
>> out the right thing to do.
> Sure, but I have no idea whether the testcase is doing something
> reasonable. If it's accidentally testing vm scalability of
> fbdev and
> there's no one else doing something this pointless, then it's
> not a
> real bug. Plus I think we're shooting the messenger here.
>
>> It's likely the test runs on the console and printfs stuff out
>> while running.
> But why did we not regress the world if a few p

Re: [PATCH/RFC 09/12] drm: rcar-du: lvds: Fix companion's mode

[Laurent Pinchart]

Hi Fabrizio,

Thank you for the patch.

On Fri, Aug 02, 2019 at 08:34:06AM +0100, Fabrizio Castro wrote:
> The companion encoder needs to be told to use the same
> mode as the primary encoder.
> 
> Fixes: e9e8798ab7b8 ("drm: rcar-du: lvds: Add support for dual-link mode")
> Signed-off-by: Fabrizio Castro 
> ---
>  drivers/gpu/drm/rcar-du/rcar_lvds.c | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c 
> b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> index edd63f5..7944ae9 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> @@ -415,8 +415,12 @@ static void rcar_lvds_enable(struct drm_bridge *bridge)
>   return;
>  
>   /* Enable the companion LVDS encoder in dual-link mode. */
> - if (lvds->dual_link && lvds->companion)
> + if (lvds->dual_link && lvds->companion) {
> + struct rcar_lvds *companion_lvds = bridge_to_rcar_lvds(
> + lvds->companion);
> + companion_lvds->mode = lvds->mode;
>   lvds->companion->funcs->enable(lvds->companion);
> + }

Would it make sense to do this in rcar_lvds_mode_set() instead, to keep
the mode set code grouped in a single place ?

>  
>   /*
>* Hardcode the channels and control signals routing for now.

-- 
Regards,

Laurent Pinchart

[PATCH] drm/tilcdc: Check priv->crtc validity in cpufreq_transition()

[Peter Ujfalusi]

The notifier can be called before we have crtc. With the check we can avoid
NULL pointer dereference within tilcdc_crtc_update_clk().

Signed-off-by: Peter Ujfalusi 
---
 drivers/gpu/drm/tilcdc/tilcdc_drv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c 
b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
index 3046a4a4232d..85093123722d 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
@@ -169,7 +169,7 @@ static int cpufreq_transition(struct notifier_block *nb,
struct tilcdc_drm_private *priv = container_of(nb,
struct tilcdc_drm_private, freq_transition);
 
-   if (val == CPUFREQ_POSTCHANGE)
+   if (val == CPUFREQ_POSTCHANGE && priv->crtc)
tilcdc_crtc_update_clk(priv->crtc);
 
return 0;
-- 
Peter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v6 11/22] clk: sunxi-ng: a64: Add minimum rate for PLL_MIPI

[Michael Nazzareno Trimarchi]

Hi Maxime

On Mon, Jul 29, 2019 at 8:59 AM Michael Nazzareno Trimarchi
 wrote:
>
> Hi
>
> On Wed, Jul 24, 2019 at 11:05 AM Maxime Ripard
>  wrote:
> >
> > On Mon, Jul 22, 2019 at 03:51:04PM +0530, Jagan Teki wrote:
> > > Hi Maxime,
> > >
> > > On Sat, Jul 20, 2019 at 3:02 PM Maxime Ripard  
> > > wrote:
> > > >
> > > > On Sat, Jul 20, 2019 at 12:46:27PM +0530, Jagan Teki wrote:
> > > > > On Sat, Jul 20, 2019 at 12:28 PM Maxime Ripard
> > > > >  wrote:
> > > > > >
> > > > > > On Thu, Jul 11, 2019 at 07:43:16PM +0200, Michael Nazzareno 
> > > > > > Trimarchi wrote:
> > > > > > > > > tcon-pixel clock is the rate that you want to achive on 
> > > > > > > > > display side
> > > > > > > > > and if you have 4 lanes 32bit or lanes and different bit 
> > > > > > > > > number that
> > > > > > > > > you need to have a clock that is able to put outside bits and 
> > > > > > > > > speed
> > > > > > > > > equal to pixel-clock * bits / lanes. so If you want a 
> > > > > > > > > pixel-clock of
> > > > > > > > > 40 mhz and you have 32bits and 4 lanes you need to have a 
> > > > > > > > > clock of
> > > > > > > > > 40 * 32 / 4 in no-burst mode. I think that this is done but 
> > > > > > > > > most of
> > > > > > > > > the display.
> > > > > > > >
> > > > > > > > So this is what the issue is then?
> > > > > > > >
> > > > > > > > This one does make sense, and you should just change the rate 
> > > > > > > > in the
> > > > > > > > call to clk_set_rate in sun4i_tcon0_mode_set_cpu.
> > > > > > > >
> > > > > > > > I'm still wondering why that hasn't been brought up in either 
> > > > > > > > the
> > > > > > > > discussion or the commit log before though.
> > > > > > > >
> > > > > > > Something like this?
> > > > > > >
> > > > > > > drivers/gpu/drm/sun4i/sun4i_tcon.c | 20 +++-
> > > > > > >  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h |  2 --
> > > > > > >  2 files changed, 11 insertions(+), 11 deletions(-)
> > > > > > >
> > > > > > > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > > > b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > > > index 64c43ee6bd92..42560d5c327c 100644
> > > > > > > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > > > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > > > @@ -263,10 +263,11 @@ static int sun4i_tcon_get_clk_delay(const 
> > > > > > > struct
> > > > > > > drm_display_mode *mode,
> > > > > > >  }
> > > > > > >
> > > > > > >  static void sun4i_tcon0_mode_set_common(struct sun4i_tcon *tcon,
> > > > > > > -   const struct 
> > > > > > > drm_display_mode *mode)
> > > > > > > +   const struct 
> > > > > > > drm_display_mode *mode,
> > > > > > > +   u32 tcon_mul)
> > > > > > >  {
> > > > > > > /* Configure the dot clock */
> > > > > > > -   clk_set_rate(tcon->dclk, mode->crtc_clock * 1000);
> > > > > > > +   clk_set_rate(tcon->dclk, mode->crtc_clock * tcon_mul * 
> > > > > > > 1000);
> > > > > > >
> > > > > > > /* Set the resolution */
> > > > > > > regmap_write(tcon->regs, SUN4I_TCON0_BASIC0_REG,
> > > > > > > @@ -335,12 +336,13 @@ static void sun4i_tcon0_mode_set_cpu(struct
> > > > > > > sun4i_tcon *tcon,
> > > > > > > u8 bpp = mipi_dsi_pixel_format_to_bpp(device->format);
> > > > > > > u8 lanes = device->lanes;
> > > > > > > u32 block_space, start_delay;
> > > > > > > -   u32 tcon_div;
> > > > > > > +   u32 tcon_div, tcon_mul;
> > > > > > >
> > > > > > > -   tcon->dclk_min_div = SUN6I_DSI_TCON_DIV;
> > > > > > > -   tcon->dclk_max_div = SUN6I_DSI_TCON_DIV;
> > > > > > > +   tcon->dclk_min_div = 4;
> > > > > > > +   tcon->dclk_max_div = 127;
> > > > > > >
> > > > > > > -   sun4i_tcon0_mode_set_common(tcon, mode);
> > > > > > > +   tcon_mul = bpp / lanes;
> > > > > > > +   sun4i_tcon0_mode_set_common(tcon, mode, tcon_mul);
> > > > > > >
> > > > > > > /* Set dithering if needed */
> > > > > > > sun4i_tcon0_mode_set_dithering(tcon, 
> > > > > > > sun4i_tcon_get_connector(encoder));
> > > > > > > @@ -366,7 +368,7 @@ static void sun4i_tcon0_mode_set_cpu(struct
> > > > > > > sun4i_tcon *tcon,
> > > > > > >  */
> > > > > > > regmap_read(tcon->regs, SUN4I_TCON0_DCLK_REG, &tcon_div);
> > > > > > > tcon_div &= GENMASK(6, 0);
> > > > > > > -   block_space = mode->htotal * bpp / (tcon_div * lanes);
> > > > > > > +   block_space = mode->htotal * tcon_div * tcon_mul;
> > > > > > > block_space -= mode->hdisplay + 40;
> > > > > > >
> > > > > > > regmap_write(tcon->regs, SUN4I_TCON0_CPU_TRI0_REG,
> > > > > > > @@ -408,7 +410,7 @@ static void sun4i_tcon0_mode_set_lvds(struct
> > > > > > > sun4i_tcon *tcon,
> > > > > > >
> > > > > > > tcon->dclk_min_div = 7;
> > > > > > > tcon->dclk_max_div = 7;
> > > > > > > -   sun4i_tcon0_mode_set_common(tcon, mode);
> > > > > > > +   sun4i_tcon0_mode_set_common(tcon, mod

[GIT PULL] exynos-drm-fixes

[Inki Dae]

Hi Dave,

   Just two fixups which fixes undefined reference error with NOMMU
   configuration and potential infinite issue of scaler module,
   and two trivial cleanups.

   Please kindly let me know if there is any problem.

Thanks,
Inki Dae

The following changes since commit f8981e0309e9004c6e86d218049045700c79d740:

  Merge tag 'msm-fixes-2019_08_01' of https://gitlab.freedesktop.org/drm/msm 
into drm-fixes (2019-08-02 10:17:25 +1000)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos 
tags/exynos-drm-fixes-for-v5.3-rc3

for you to fetch changes up to 1bbbab097a05276e312dd2462791d32b21ceb1ee:

  drm/exynos: fix missing decrement of retry counter (2019-08-02 16:50:18 +0900)


- Two cleanup patches
  . use dev_get_drvdata for readability instead of platform_get_drvdata
  . remove redundant assignment to node.
- Two fixup patches
  . fix undefined reference to 'vmf_insert_mixed' with NOMMU configuration.
  . fix potential infinite spin issue by decrementing 'retry' variable in
scaler_reset function of exynos_drm_scaler.c


Arnd Bergmann (1):
  drm/exynos: add CONFIG_MMU dependency

Colin Ian King (2):
  drm/exynos: remove redundant assignment to pointer 'node'
  drm/exynos: fix missing decrement of retry counter

Fuqian Huang (1):
  drm/exynos: using dev_get_drvdata directly

 drivers/gpu/drm/exynos/Kconfig | 1 +
 drivers/gpu/drm/exynos/exynos_drm_fimc.c   | 2 +-
 drivers/gpu/drm/exynos/exynos_drm_g2d.c| 2 +-
 drivers/gpu/drm/exynos/exynos_drm_gsc.c| 2 +-
 drivers/gpu/drm/exynos/exynos_drm_scaler.c | 4 ++--
 5 files changed, 6 insertions(+), 5 deletions(-)
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH/RFC 05/12] drm: rcar-du: lvds: Add data swap support

[Geert Uytterhoeven]

Hi Laurent,

On Fri, Aug 2, 2019 at 10:06 AM Laurent Pinchart
 wrote:
> On Fri, Aug 02, 2019 at 08:34:02AM +0100, Fabrizio Castro wrote:
> > When in vertical stripe mode of operation, there is the option
> > of swapping even data and odd data on the two LVDS interfaces
> > used to drive the video output.
> > Add data swap support by exposing a new DT property named
> > "renesas,swap-data".
> >
> > Signed-off-by: Fabrizio Castro 

> > --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c

> > @@ -439,12 +440,16 @@ static void rcar_lvds_enable(struct drm_bridge 
> > *bridge)
> >   rcar_lvds_write(lvds, LVDCHCR, lvdhcr);
> >
> >   if (lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK) {
> > - /*
> > -  * Configure vertical stripe based on the mode of operation of
> > -  * the connected device.
> > -  */
> > - rcar_lvds_write(lvds, LVDSTRIPE,
> > - lvds->dual_link ? LVDSTRIPE_ST_ON : 0);
> > + u32 lvdstripe = 0;
> > +
> > + if (lvds->dual_link)
> > + /*
> > +  * Configure vertical stripe based on the mode of
> > +  * operation of the connected device.
> > +  */
> > + lvdstripe = LVDSTRIPE_ST_ON | (lvds->stripe_swap_data 
> > ?
> > +LVDSTRIPE_ST_SWAP : 0);
>
> Would the following be simpler ?
>
> lvdstripe = (lvds->dual_link ? LVDSTRIPE_ST_ON : 0)
>   | (lvds->stripe_swap_data ? LVDSTRIPE_ST_SWAP : 0);

From the point of view of "wc -l": yes.
From the point of view of readability, I'd go for:

if (lvds->dual_link)
lvdstripe |= LVDSTRIPE_ST_ON;
if (lvds->stripe_swap_data)
lvdstripe |= LVDSTRIPE_ST_SWAP;

> > + rcar_lvds_write(lvds, LVDSTRIPE, lvdstripe);
> >   }
> >
> >   /*
> > @@ -770,8 +775,12 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds)

Gr{oetje,eeting}s,

Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 3/5] ARM: dts: stm32: add phy-dsi-supply property on stm32mp157c

[Yannick FERTRE]

Hi Alexandre,

this patch can be abandoned.

BR

-- 
Yannick Fertré | TINA: 166 7152 | Tel: +33 244027152 | Mobile: +33 620600270
Microcontrollers and Digital ICs Group | Microcontrolleurs Division


On 5/10/19 4:20 PM, Yannick Fertré wrote:

> The dsi physical layer is powered by the 1v8 power controller supply.
>
> Signed-off-by: Yannick Fertré 
> ---
>   arch/arm/boot/dts/stm32mp157c.dtsi | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm/boot/dts/stm32mp157c.dtsi 
> b/arch/arm/boot/dts/stm32mp157c.dtsi
> index 2afeee6..6b14f1e 100644
> --- a/arch/arm/boot/dts/stm32mp157c.dtsi
> +++ b/arch/arm/boot/dts/stm32mp157c.dtsi
> @@ -1156,6 +1156,7 @@
>   clock-names = "pclk", "ref", "px_clk";
>   resets = <&rcc DSI_R>;
>   reset-names = "apb";
> + phy-dsi-supply = <®18>;
>   status = "disabled";
>   };
>   
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 5/5] ARM: dts: stm32: remove phy-dsi-supply property on stm32mp157c-dk2 board

[Yannick FERTRE]

Hi Alexandre,

this patch can be abandoned.

BR

-- 
Yannick Fertré | TINA: 166 7152 | Tel: +33 244027152 | Mobile: +33 620600270
Microcontrollers and Digital ICs Group | Microcontrolleurs Division
On 5/10/19 4:20 PM, Yannick Fertré wrote:
> This property is already defined into stm32mp157c.dtsi file.
>
> Signed-off-by: Yannick Fertré 
> ---
>   arch/arm/boot/dts/stm32mp157c-dk2.dts | 1 -
>   1 file changed, 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/stm32mp157c-dk2.dts 
> b/arch/arm/boot/dts/stm32mp157c-dk2.dts
> index 020ea0f..09f6e7b 100644
> --- a/arch/arm/boot/dts/stm32mp157c-dk2.dts
> +++ b/arch/arm/boot/dts/stm32mp157c-dk2.dts
> @@ -17,7 +17,6 @@
>   #address-cells = <1>;
>   #size-cells = <0>;
>   status = "okay";
> - phy-dsi-supply = <®18>;
>   
>   ports {
>   #address-cells = <1>;
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 4/5] ARM: dts: stm32: move fixe regulators reg11 & reg18

[Yannick FERTRE]

Hi Alexandre,

this patch can be abandoned.

BR

-- 
Yannick Fertré | TINA: 166 7152 | Tel: +33 244027152 | Mobile: +33 620600270
Microcontrollers and Digital ICs Group | Microcontrolleurs Division

On 5/10/19 4:20 PM, Yannick Fertré wrote:
> Move regulators reg11 & reg18 from device-tree files stm32mp157c-ed1.dts
> & stm32mp157c-dk2.dts to file stm32mp157c.dtsi.
>
> Signed-off-by: Yannick Fertré 
> ---
>   arch/arm/boot/dts/stm32mp157c-dk2.dts |  8 
>   arch/arm/boot/dts/stm32mp157c-ed1.dts | 16 
>   arch/arm/boot/dts/stm32mp157c.dtsi| 16 
>   3 files changed, 16 insertions(+), 24 deletions(-)
>
> diff --git a/arch/arm/boot/dts/stm32mp157c-dk2.dts 
> b/arch/arm/boot/dts/stm32mp157c-dk2.dts
> index 20ea601..020ea0f 100644
> --- a/arch/arm/boot/dts/stm32mp157c-dk2.dts
> +++ b/arch/arm/boot/dts/stm32mp157c-dk2.dts
> @@ -11,14 +11,6 @@
>   / {
>   model = "STMicroelectronics STM32MP157C-DK2 Discovery Board";
>   compatible = "st,stm32mp157c-dk2", "st,stm32mp157";
> -
> - reg18: reg18 {
> - compatible = "regulator-fixed";
> - regulator-name = "reg18";
> - regulator-min-microvolt = <180>;
> - regulator-max-microvolt = <180>;
> - regulator-always-on;
> - };
>   };
>   
>   &dsi {
> diff --git a/arch/arm/boot/dts/stm32mp157c-ed1.dts 
> b/arch/arm/boot/dts/stm32mp157c-ed1.dts
> index 62a8c78..f41189c 100644
> --- a/arch/arm/boot/dts/stm32mp157c-ed1.dts
> +++ b/arch/arm/boot/dts/stm32mp157c-ed1.dts
> @@ -27,22 +27,6 @@
>   serial0 = &uart4;
>   };
>   
> - reg11: reg11 {
> - compatible = "regulator-fixed";
> - regulator-name = "reg11";
> - regulator-min-microvolt = <110>;
> - regulator-max-microvolt = <110>;
> - regulator-always-on;
> - };
> -
> - reg18: reg18 {
> - compatible = "regulator-fixed";
> - regulator-name = "reg18";
> - regulator-min-microvolt = <180>;
> - regulator-max-microvolt = <180>;
> - regulator-always-on;
> - };
> -
>   sd_switch: regulator-sd_switch {
>   compatible = "regulator-gpio";
>   regulator-name = "sd_switch";
> diff --git a/arch/arm/boot/dts/stm32mp157c.dtsi 
> b/arch/arm/boot/dts/stm32mp157c.dtsi
> index 6b14f1e..aaac51cd 100644
> --- a/arch/arm/boot/dts/stm32mp157c.dtsi
> +++ b/arch/arm/boot/dts/stm32mp157c.dtsi
> @@ -11,6 +11,22 @@
>   #address-cells = <1>;
>   #size-cells = <1>;
>   
> + reg11: reg11 {
> + compatible = "regulator-fixed";
> + regulator-name = "reg11";
> + regulator-min-microvolt = <110>;
> + regulator-max-microvolt = <110>;
> + regulator-always-on;
> + };
> +
> + reg18: reg18 {
> + compatible = "regulator-fixed";
> + regulator-name = "reg18";
> + regulator-min-microvolt = <180>;
> + regulator-max-microvolt = <180>;
> + regulator-always-on;
> + };
> +
>   cpus {
>   #address-cells = <1>;
>   #size-cells = <0>;
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 111272] [DRI_PRIME] Error on multi GPU with only one enabled

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=111272

Michel Dänzer  changed:

   What|Removed |Added

   Assignee|dri-devel@lists.freedesktop |mesa-dev@lists.freedesktop.
   |.org|org
  Component|DRM/AMDgpu  |GLX
Version|XOrg git|unspecified
Product|DRI |Mesa
 QA Contact||mesa-dev@lists.freedesktop.
   ||org

--- Comment #7 from Michel Dänzer  ---
Due to DRI_PRIME=1, libGL tries to initialize the i965 driver with DRI3, which
fails:

 libGL error: Different GPU, but blitImage not implemented for this driver
 libGL error: failed to load driver: i965

So it falls back to DRI2, which uses the AMD GPU, because the Intel one wasn't
initialized in Xorg. DRI2 doesn't support sync-to-vblank via PRIME.

Reassigning to Mesa for now, but DRI_PRIME=1 really isn't intended to be used
with a single GPU.

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: Threaded submission & semaphore sharing

[zhoucm1]

Hi Lionel,

For binary semaphore, I guess every one will think application will 
guarantee wait is behind the signal, whenever the semaphore is shared or 
used in internal-process.


I think below two options can fix your problem:

a. Can we extend vkWaitForFence so that it can be able to wait on 
fence-available? If fence is available, then it's safe to do semaphore 
wait in vkQueueSubmit.


b. Make waitBeforeSignal is valid for binary semaphore as well, as that 
way, It is reasonable to add wait/signal counting for binary syncobj.



-David


On 2019年08月02日 14:27, Lionel Landwerlin wrote:

On 02/08/2019 09:10, Koenig, Christian wrote:



Am 02.08.2019 07:38 schrieb Lionel Landwerlin 
:


On 02/08/2019 08:21, Koenig, Christian wrote:



Am 02.08.2019 07:17 schrieb Lionel Landwerlin

:

On 02/08/2019 08:08, Koenig, Christian wrote:

Hi Lionel,

Well that looks more like your test case is buggy.

According to the code the ctx1 queue always waits for
sem1 and ctx2 queue always waits for sem2.


That's supposed to be the same underlying syncobj because
it's exported from one VkDevice as opaque FD from sem1
and imported into sem2.


Well than that's still buggy and won't synchronize at all.

When ctx1 waits for a semaphore and then signals the same
semaphore there is no guarantee that ctx2 will run in between
jobs.

It's perfectly valid in this case to first run all jobs from
ctx1 and then all jobs from ctx2.


That's not really how I see the semaphores working.

The spec describe VkSemaphore as an interface to an internal
payload opaque to the application.


When ctx1 waits on the semaphore, it waits on the payload put
there by the previous iteration.


And who says that it's not waiting for it's own previous payload?



That's was I understood from you previous comment : "there is no 
guarantee that ctx2 will run in between jobs"





See if the payload is a counter this won't work either. Keep in mind 
that this has the semantic of a semaphore. Whoever grabs the 
semaphore first wins and can run, everybody else has to wait.



What performs the "grab" here?

I thought that would be vkQueueSubmit().

Since that occuring from a single application thread, that should then 
be ordered in execution of ctx1,ctx2,ctx1,...



Thanks for your time on this,


-Lionel




Then it proceeds to signal it by replacing the internal payload.


That's an implementation detail of our sync objects, but I don't 
think that this behavior is part of the Vulkan specification.


Regards,
Christian.


ctx2 then waits on that and replaces the payload again with the
new internal synchronization object.


The internal payload is a dma fence in our case and signaling
just replaces a dma fence by another or puts one where there was
none before.

So we should have created a dependecy link between all the
submissions and then should be executed in the order of
QueueSubmit() calls.


-Lionel



It only prevents running both at the same time and as far as
I can see that still works even with threaded submission.

You need at least two semaphores for a tandem submission.

Regards,
Christian.



This way there can't be any Synchronisation between
the two.

Regards,
Christian.

Am 02.08.2019 06:55 schrieb Lionel Landwerlin

:
Hey Christian,

The problem boils down to the fact that we don't
immediately create dma fences when calling
vkQueueSubmit().
This is delayed to a thread.

From a single application thread, you can
QueueSubmit() to 2 queues from 2 different devices.
Each QueueSubmit to one queue has a dependency on the
previous QueueSubmit on the other queue through an
exported/imported semaphore.

From the API point of view the state of the semaphore
should be changed after each QueueSubmit().
The problem is that it's not because of the thread
and because you might have those 2 submission threads
tied to different VkDevice/VkInstance or even
different applications (synchronizing themselves
outside the vulkan API).

Hope that makes sense.
It's not really easy to explain by mail, the best
explanation is probably reading the test :

https://gitlab.freedesktop.org/mesa/crucible/blob/master/src/tests/func/sync/semaphore-fd.c#L788

Re: [drm/mgag200] 90f479ae51: vm-scalability.median -18.8% regression

[Daniel Vetter]

On Wed, Jul 31, 2019 at 12:10:54PM +0200, Thomas Zimmermann wrote:
> Hi
> 
> Am 31.07.19 um 10:13 schrieb Daniel Vetter:
> > On Tue, Jul 30, 2019 at 10:27 PM Dave Airlie  wrote:
> >>
> >> On Wed, 31 Jul 2019 at 05:00, Daniel Vetter  wrote:
> >>>
> >>> On Tue, Jul 30, 2019 at 8:50 PM Thomas Zimmermann  
> >>> wrote:
> 
>  Hi
> 
>  Am 30.07.19 um 20:12 schrieb Daniel Vetter:
> > On Tue, Jul 30, 2019 at 7:50 PM Thomas Zimmermann  
> > wrote:
> >> Am 29.07.19 um 11:51 schrieb kernel test robot:
> >>> Greeting,
> >>>
> >>> FYI, we noticed a -18.8% regression of vm-scalability.median due to 
> >>> commit:>
> >>>
> >>> commit: 90f479ae51afa45efab97afdde9b94b9660dd3e4 ("drm/mgag200: 
> >>> Replace struct mga_fbdev with generic framebuffer emulation")
> >>> https://kernel.googlesource.com/pub/scm/linux/kernel/git/next/linux-next.git
> >>>  master
> >>
> >> Daniel, Noralf, we may have to revert this patch.
> >>
> >> I expected some change in display performance, but not in VM. Since 
> >> it's
> >> a server chipset, probably no one cares much about display performance.
> >> So that seemed like a good trade-off for re-using shared code.
> >>
> >> Part of the patch set is that the generic fb emulation now maps and
> >> unmaps the fbdev BO when updating the screen. I guess that's the cause
> >> of the performance regression. And it should be visible with other
> >> drivers as well if they use a shadow FB for fbdev emulation.
> >
> > For fbcon we should need to do any maps/unamps at all, this is for the
> > fbdev mmap support only. If the testcase mentioned here tests fbdev
> > mmap handling it's pretty badly misnamed :-) And as long as you don't
> > have an fbdev mmap there shouldn't be any impact at all.
> 
>  The ast and mgag200 have only a few MiB of VRAM, so we have to get the
>  fbdev BO out if it's not being displayed. If not being mapped, it can be
>  evicted and make room for X, etc.
> 
>  To make this work, the BO's memory is mapped and unmapped in
>  drm_fb_helper_dirty_work() before being updated from the shadow FB. [1]
>  That fbdev mapping is established on each screen update, more or less.
>  From my (yet unverified) understanding, this causes the performance
>  regression in the VM code.
> 
>  The original code in mgag200 used to kmap the fbdev BO while it's being
>  displayed; [2] and the drawing code only mapped it when necessary (i.e.,
>  not being display). [3]
> >>>
> >>> Hm yeah, this vmap/vunmap is going to be pretty bad. We indeed should
> >>> cache this.
> >>>
>  I think this could be added for VRAM helpers as well, but it's still a
>  workaround and non-VRAM drivers might also run into such a performance
>  regression if they use the fbdev's shadow fb.
> >>>
> >>> Yeah agreed, fbdev emulation should try to cache the vmap.
> >>>
>  Noralf mentioned that there are plans for other DRM clients besides the
>  console. They would as well run into similar problems.
> 
> >> The thing is that we'd need another generic fbdev emulation for ast and
> >> mgag200 that handles this issue properly.
> >
> > Yeah I dont think we want to jump the gun here.  If you can try to
> > repro locally and profile where we're wasting cpu time I hope that
> > should sched a light what's going wrong here.
> 
>  I don't have much time ATM and I'm not even officially at work until
>  late Aug. I'd send you the revert and investigate later. I agree that
>  using generic fbdev emulation would be preferable.
> >>>
> >>> Still not sure that's the right thing to do really. Yes it's a
> >>> regression, but vm testcases shouldn run a single line of fbcon or drm
> >>> code. So why this is impacted so heavily by a silly drm change is very
> >>> confusing to me. We might be papering over a deeper and much more
> >>> serious issue ...
> >>
> >> It's a regression, the right thing is to revert first and then work
> >> out the right thing to do.
> > 
> > Sure, but I have no idea whether the testcase is doing something
> > reasonable. If it's accidentally testing vm scalability of fbdev and
> > there's no one else doing something this pointless, then it's not a
> > real bug. Plus I think we're shooting the messenger here.
> > 
> >> It's likely the test runs on the console and printfs stuff out while 
> >> running.
> > 
> > But why did we not regress the world if a few prints on the console
> > have such a huge impact? We didn't get an entire stream of mails about
> > breaking stuff ...
> 
> The vmap/vunmap pair is only executed for fbdev emulation with a shadow
> FB. And most of those are with shmem helpers, which ref-count the vmap
> calls internally. My guess is that VRAM helpers are currently the only
> BOs triggering this problem.

I meant that surely this vm-scalability testcase isn't the 

Re: [PATCH 00/34] put_user_pages(): miscellaneous call sites

[Michal Hocko]

On Thu 01-08-19 19:19:31, john.hubb...@gmail.com wrote:
[...]
> 2) Convert all of the call sites for get_user_pages*(), to
> invoke put_user_page*(), instead of put_page(). This involves dozens of
> call sites, and will take some time.

How do we make sure this is the case and it will remain the case in the
future? There must be some automagic to enforce/check that. It is simply
not manageable to do it every now and then because then 3) will simply
be never safe.

Have you considered coccinele or some other scripted way to do the
transition? I have no idea how to deal with future changes that would
break the balance though.
-- 
Michal Hocko
SUSE Labs
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 06/34] drm/i915: convert put_page() to put_user_page*()

[Joonas Lahtinen]

Quoting john.hubb...@gmail.com (2019-08-02 05:19:37)
> From: John Hubbard 
> 
> For pages that were retained via get_user_pages*(), release those pages
> via the new put_user_page*() routines, instead of via put_page() or
> release_pages().
> 
> This is part a tree-wide conversion, as described in commit fc1d8e7cca2d
> ("mm: introduce put_user_page*(), placeholder versions").
> 
> Note that this effectively changes the code's behavior in
> i915_gem_userptr_put_pages(): it now calls set_page_dirty_lock(),
> instead of set_page_dirty(). This is probably more accurate.

We've already fixed this in drm-tip where the current code uses
set_page_dirty_lock().

This would conflict with our tree. Rodrigo is handling
drm-intel-next for 5.4, so you guys want to coordinate how
to merge.

Regards, Joonas
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [LKP] [drm/mgag200] 90f479ae51: vm-scalability.median -18.8% regression

[Thomas Zimmermann]

Hi

Am 02.08.19 um 09:11 schrieb Rong Chen:
> Hi,
> 
> On 8/1/19 7:58 PM, Thomas Zimmermann wrote:
>> Hi
>>
>> Am 01.08.19 um 13:25 schrieb Feng Tang:
>>> Hi Thomas,
>>>
>>> On Thu, Aug 01, 2019 at 11:59:28AM +0200, Thomas Zimmermann wrote:
 Hi

 Am 01.08.19 um 10:37 schrieb Feng Tang:
> On Thu, Aug 01, 2019 at 02:19:53PM +0800, Rong Chen wrote:
>>> commit: 90f479ae51afa45efab97afdde9b94b9660dd3e4
>>> ("drm/mgag200: Replace struct mga_fbdev with generic
>>> framebuffer emulation")
>>> https://kernel.googlesource.com/pub/scm/linux/kernel/git/next/linux-next.git
>>> master
>> Daniel, Noralf, we may have to revert this patch.
>>
>> I expected some change in display performance, but not in
>> VM. Since it's
>> a server chipset, probably no one cares much about display
>> performance.
>> So that seemed like a good trade-off for re-using shared
>> code.
>>
>> Part of the patch set is that the generic fb emulation now
>> maps and
>> unmaps the fbdev BO when updating the screen. I guess
>> that's the cause
>> of the performance regression. And it should be visible
>> with other
>> drivers as well if they use a shadow FB for fbdev emulation.
> For fbcon we should need to do any maps/unamps at all, this
> is for the
> fbdev mmap support only. If the testcase mentioned here
> tests fbdev
> mmap handling it's pretty badly misnamed :-) And as long as
> you don't
> have an fbdev mmap there shouldn't be any impact at all.
 The ast and mgag200 have only a few MiB of VRAM, so we have
 to get the
 fbdev BO out if it's not being displayed. If not being
 mapped, it can be
 evicted and make room for X, etc.

 To make this work, the BO's memory is mapped and unmapped in
 drm_fb_helper_dirty_work() before being updated from the
 shadow FB. [1]
 That fbdev mapping is established on each screen update,
 more or less.
  From my (yet unverified) understanding, this causes the
 performance
 regression in the VM code.

 The original code in mgag200 used to kmap the fbdev BO while
 it's being
 displayed; [2] and the drawing code only mapped it when
 necessary (i.e.,
 not being display). [3]
>>> Hm yeah, this vmap/vunmap is going to be pretty bad. We
>>> indeed should
>>> cache this.
>>>
 I think this could be added for VRAM helpers as well, but
 it's still a
 workaround and non-VRAM drivers might also run into such a
 performance
 regression if they use the fbdev's shadow fb.
>>> Yeah agreed, fbdev emulation should try to cache the vmap.
>>>
 Noralf mentioned that there are plans for other DRM clients
 besides the
 console. They would as well run into similar problems.

>> The thing is that we'd need another generic fbdev
>> emulation for ast and
>> mgag200 that handles this issue properly.
> Yeah I dont think we want to jump the gun here.  If you can
> try to
> repro locally and profile where we're wasting cpu time I
> hope that
> should sched a light what's going wrong here.
 I don't have much time ATM and I'm not even officially at
 work until
 late Aug. I'd send you the revert and investigate later. I
 agree that
 using generic fbdev emulation would be preferable.
>>> Still not sure that's the right thing to do really. Yes it's a
>>> regression, but vm testcases shouldn run a single line of
>>> fbcon or drm
>>> code. So why this is impacted so heavily by a silly drm
>>> change is very
>>> confusing to me. We might be papering over a deeper and much
>>> more
>>> serious issue ...
>> It's a regression, the right thing is to revert first and then
>> work
>> out the right thing to do.
> Sure, but I have no idea whether the testcase is doing something
> reasonable. If it's accidentally testing vm scalability of
> fbdev and
> there's no one else doing something this pointless, then it's
> not a
> real bug. Plus I think we're shooting the messenger here.
>
>> It's likely the test runs on the console and printfs stuff out
>> while running.
> But why did we not regress the world if a few p

Re: [PATCH] Revert "drm/vgem: fix cache synchronization on arm/arm64"

[Daniel Vetter]

On Thu, Aug 01, 2019 at 01:44:58PM +0100, Chris Wilson wrote:
> commit 7e9e5ead55be ("drm/vgem: fix cache synchronization on arm/arm64")
> broke all of the !llc i915-vgem coherency tests in CI, and left the HW
> very, very unhappy (which is even more scary).
> 
> Fixes: 7e9e5ead55be ("drm/vgem: fix cache synchronization on arm/arm64")
> Signed-off-by: Chris Wilson 
> Cc: Daniel Vetter 
> Cc: Rob Clark 
> Cc: Sean Paul 

Acked-by: Daniel Vetter 

Not that this counts a hole lot unfortunately :-/

Trouble here is that x86 assumes that everything is coherent (it's not)
and dma-api assumes that coherentness is a property of the device and
opaque to the device driver (it's not). So maybe we should do a mix of v1
and v2 of Rob's patches, i.e. not inflict the cache flushing into
drm_clflush.c (to avoid the pointless struct device nonsense), but then
also do this in vgem here only with #ifdef ARM64 and leave the clflush for
everything else.
-Daniel

> ---
>  drivers/gpu/drm/vgem/vgem_drv.c | 130 
>  1 file changed, 47 insertions(+), 83 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c
> index b98689fb0d5d..5bd60ded3d81 100644
> --- a/drivers/gpu/drm/vgem/vgem_drv.c
> +++ b/drivers/gpu/drm/vgem/vgem_drv.c
> @@ -54,16 +54,10 @@ static struct vgem_device {
>   struct platform_device *platform;
>  } *vgem_device;
>  
> -static void sync_and_unpin(struct drm_vgem_gem_object *bo);
> -static struct page **pin_and_sync(struct drm_vgem_gem_object *bo);
> -
>  static void vgem_gem_free_object(struct drm_gem_object *obj)
>  {
>   struct drm_vgem_gem_object *vgem_obj = to_vgem_bo(obj);
>  
> - if (!obj->import_attach)
> - sync_and_unpin(vgem_obj);
> -
>   kvfree(vgem_obj->pages);
>   mutex_destroy(&vgem_obj->pages_lock);
>  
> @@ -91,15 +85,40 @@ static vm_fault_t vgem_gem_fault(struct vm_fault *vmf)
>   return VM_FAULT_SIGBUS;
>  
>   mutex_lock(&obj->pages_lock);
> - if (!obj->pages)
> - pin_and_sync(obj);
>   if (obj->pages) {
>   get_page(obj->pages[page_offset]);
>   vmf->page = obj->pages[page_offset];
>   ret = 0;
>   }
>   mutex_unlock(&obj->pages_lock);
> + if (ret) {
> + struct page *page;
> +
> + page = shmem_read_mapping_page(
> + file_inode(obj->base.filp)->i_mapping,
> + page_offset);
> + if (!IS_ERR(page)) {
> + vmf->page = page;
> + ret = 0;
> + } else switch (PTR_ERR(page)) {
> + case -ENOSPC:
> + case -ENOMEM:
> + ret = VM_FAULT_OOM;
> + break;
> + case -EBUSY:
> + ret = VM_FAULT_RETRY;
> + break;
> + case -EFAULT:
> + case -EINVAL:
> + ret = VM_FAULT_SIGBUS;
> + break;
> + default:
> + WARN_ON(PTR_ERR(page));
> + ret = VM_FAULT_SIGBUS;
> + break;
> + }
>  
> + }
>   return ret;
>  }
>  
> @@ -265,93 +284,32 @@ static const struct file_operations vgem_driver_fops = {
>   .release= drm_release,
>  };
>  
> -/* Called under pages_lock, except in free path (where it can't race): */
> -static void sync_and_unpin(struct drm_vgem_gem_object *bo)
> -{
> - struct drm_device *dev = bo->base.dev;
> -
> - if (bo->table) {
> - dma_sync_sg_for_cpu(dev->dev, bo->table->sgl,
> - bo->table->nents, DMA_BIDIRECTIONAL);
> - sg_free_table(bo->table);
> - kfree(bo->table);
> - bo->table = NULL;
> - }
> -
> - if (bo->pages) {
> - drm_gem_put_pages(&bo->base, bo->pages, true, true);
> - bo->pages = NULL;
> - }
> -}
> -
> -static struct page **pin_and_sync(struct drm_vgem_gem_object *bo)
> -{
> - struct drm_device *dev = bo->base.dev;
> - int npages = bo->base.size >> PAGE_SHIFT;
> - struct page **pages;
> - struct sg_table *sgt;
> -
> - WARN_ON(!mutex_is_locked(&bo->pages_lock));
> -
> - pages = drm_gem_get_pages(&bo->base);
> - if (IS_ERR(pages)) {
> - bo->pages_pin_count--;
> - mutex_unlock(&bo->pages_lock);
> - return pages;
> - }
> -
> - sgt = drm_prime_pages_to_sg(pages, npages);
> - if (IS_ERR(sgt)) {
> - dev_err(dev->dev,
> - "failed to allocate sgt: %ld\n",
> - PTR_ERR(bo->table));
> - drm_gem_put_pages(&bo->base, pages, false, false);
> - mutex_unlock(&bo->pages_lock);
> - return ERR_CAST(bo->table)