May 2019 - Linux-stable-mirror

[PATCH v2 6/9] media: ov6650: Fix .get_fmt() V4L2_SUBDEV_FORMAT_TRY support

by Janusz Krzysztofik

Commit da298c6d98d5 ("[media] v4l2: replace video op g_mbus_fmt by pad op get_fmt") converted a former ov6650_g_fmt() video operation callback to an ov6650_get_fmt() pad operation callback. However, the converted function disregards a format->which flag that pad operations should obey and always returns active frame format settings. That can be fixed by always responding to V4L2_SUBDEV_FORMAT_TRY with -EINVAL, or providing the response from a pad config argument, likely updated by a former user call to V4L2_SUBDEV_FORMAT_TRY .set_fmt(). Since implementation of the latter is trivial, go for it. Fixes: da298c6d98d5 ("[media] v4l2: replace video op g_mbus_fmt by pad op get_fmt") Signed-off-by: Janusz Krzysztofik <jmkrzyszt(a)gmail.com> Cc: stable(a)vger.kernel.org --- drivers/media/i2c/ov6650.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/drivers/media/i2c/ov6650.c b/drivers/media/i2c/ov6650.c index 731b03bef7a5..1915a43bff87 100644 --- a/drivers/media/i2c/ov6650.c +++ b/drivers/media/i2c/ov6650.c @@ -528,10 +528,16 @@ static int ov6650_get_fmt(struct v4l2_subdev *sd, *mf = ov6650_def_fmt; /* update media bus format code and frame size */ - mf->width = priv->rect.width >> priv->half_scale; - mf->height = priv->rect.height >> priv->half_scale; - mf->code = priv->code; + if (format->which == V4L2_SUBDEV_FORMAT_TRY) { + mf->width = cfg->try_fmt.width; + mf->height = cfg->try_fmt.height; + mf->code = cfg->try_fmt.code; + } else { + mf->width = priv->rect.width >> priv->half_scale; + mf->height = priv->rect.height >> priv->half_scale; + mf->code = priv->code; + } return 0; } -- 2.21.0

5 years, 9 months

1
0
0 0

[PATCH v2 5/9] media: ov6650: Fix some format attributes not under control

by Janusz Krzysztofik

User arguments passed to .get/set_fmt() pad operation callbacks may contain unsupported values. The driver takes control over frame size and pixel code as well as colorspace and field attributes but has never cared for remainig format attributes, i.e., ycbcr_enc, quantization and xfer_func, introduced by commit 11ff030c7365 ("[media] v4l2-mediabus: improve colorspace support"). Fix it. Set up a static v4l2_mbus_framefmt structure with attributes initialized to reasonable defaults and use it for updating content of user provided arguments. In case of V4L2_SUBDEV_FORMAT_ACTIVE, postpone frame size update, now performed from inside ov6650_s_fmt() helper, util the user argument is first updated in ov6650_set_fmt() with default frame format content. For V4L2_SUBDEV_FORMAT_TRY, don't copy all attributes to pad config, only those handled by the driver, then fill the response with the default frame format updated with updated pad config format code and frame size. Fixes: 11ff030c7365 ("[media] v4l2-mediabus: improve colorspace support") Signed-off-by: Janusz Krzysztofik <jmkrzyszt(a)gmail.com> Cc: stable(a)vger.kernel.org --- drivers/media/i2c/ov6650.c | 51 +++++++++++++++++++++++++++++--------- 1 file changed, 39 insertions(+), 12 deletions(-) diff --git a/drivers/media/i2c/ov6650.c b/drivers/media/i2c/ov6650.c index e7790e9b8887..731b03bef7a5 100644 --- a/drivers/media/i2c/ov6650.c +++ b/drivers/media/i2c/ov6650.c @@ -215,6 +215,17 @@ static u32 ov6650_codes[] = { MEDIA_BUS_FMT_Y8_1X8, }; +static const struct v4l2_mbus_framefmt ov6650_def_fmt = { + .width = W_CIF, + .height = H_CIF, + .code = MEDIA_BUS_FMT_SBGGR8_1X8, + .colorspace = V4L2_COLORSPACE_SRGB, + .field = V4L2_FIELD_NONE, + .ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT, + .quantization = V4L2_QUANTIZATION_DEFAULT, + .xfer_func = V4L2_XFER_FUNC_DEFAULT, +}; + /* read a register */ static int ov6650_reg_read(struct i2c_client *client, u8 reg, u8 *val) { @@ -513,11 +524,13 @@ static int ov6650_get_fmt(struct v4l2_subdev *sd, if (format->pad) return -EINVAL; + /* initialize response with default media bus frame format */ + *mf = ov6650_def_fmt; + + /* update media bus format code and frame size */ mf->width = priv->rect.width >> priv->half_scale; mf->height = priv->rect.height >> priv->half_scale; mf->code = priv->code; - mf->colorspace = V4L2_COLORSPACE_SRGB; - mf->field = V4L2_FIELD_NONE; return 0; } @@ -653,10 +666,6 @@ static int ov6650_s_fmt(struct v4l2_subdev *sd, struct v4l2_mbus_framefmt *mf) if (!ret) ret = ov6650_reg_rmw(client, REG_COML, coml_set, coml_mask); - if (!ret) { - mf->width = priv->rect.width >> half_scale; - mf->height = priv->rect.height >> half_scale; - } return ret; } @@ -675,9 +684,6 @@ static int ov6650_set_fmt(struct v4l2_subdev *sd, v4l_bound_align_image(&mf->width, 2, W_CIF, 1, &mf->height, 2, H_CIF, 1, 0); - mf->field = V4L2_FIELD_NONE; - mf->colorspace = V4L2_COLORSPACE_SRGB; - switch (mf->code) { case MEDIA_BUS_FMT_Y10_1X10: mf->code = MEDIA_BUS_FMT_Y8_1X8; @@ -695,10 +701,31 @@ static int ov6650_set_fmt(struct v4l2_subdev *sd, break; } - if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE) - return ov6650_s_fmt(sd, mf); - cfg->try_fmt = *mf; + if (format->which == V4L2_SUBDEV_FORMAT_TRY) { + /* store media bus format code and frame size in pad config */ + cfg->try_fmt.width = mf->width; + cfg->try_fmt.height = mf->height; + cfg->try_fmt.code = mf->code; + /* return default mbus frame format updated with pad config */ + *mf = ov6650_def_fmt; + mf->width = cfg->try_fmt.width; + mf->height = cfg->try_fmt.height; + mf->code = cfg->try_fmt.code; + + } else { + /* apply new media bus format code and frame size */ + int ret = ov6650_s_fmt(sd, mf); + + if (ret) + return ret; + + /* return default format updated with active size and code */ + *mf = ov6650_def_fmt; + mf->width = priv->rect.width >> priv->half_scale; + mf->height = priv->rect.height >> priv->half_scale; + mf->code = priv->code; + } return 0; } -- 2.21.0

5 years, 9 months

1
0
0 0

[PATCH v2 4/9] media: ov6650: Fix incorrect use of JPEG colorspace

by Janusz Krzysztofik

Since its initial submission, the driver selects V4L2_COLORSPACE_JPEG for supported formats other than V4L2_MBUS_FMT_SBGGR8_1X8. According to v4l2-compliance test program, V4L2_COLORSPACE_JPEG applies exclusively to V4L2_PIX_FMT_JPEG. Since the sensor does not support JPEG format, fix it to always select V4L2_COLORSPACE_SRGB. Fixes: 2f6e2404799a ("[media] SoC Camera: add driver for OV6650 sensor") Signed-off-by: Janusz Krzysztofik <jmkrzyszt(a)gmail.com> Cc: stable(a)vger.kernel.org --- drivers/media/i2c/ov6650.c | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/drivers/media/i2c/ov6650.c b/drivers/media/i2c/ov6650.c index 751b48483f27..e7790e9b8887 100644 --- a/drivers/media/i2c/ov6650.c +++ b/drivers/media/i2c/ov6650.c @@ -203,7 +203,6 @@ struct ov6650 { unsigned long pclk_max; /* from resolution and format */ struct v4l2_fract tpf; /* as requested with s_frame_interval */ u32 code; - enum v4l2_colorspace colorspace; }; @@ -517,7 +516,7 @@ static int ov6650_get_fmt(struct v4l2_subdev *sd, mf->width = priv->rect.width >> priv->half_scale; mf->height = priv->rect.height >> priv->half_scale; mf->code = priv->code; - mf->colorspace = priv->colorspace; + mf->colorspace = V4L2_COLORSPACE_SRGB; mf->field = V4L2_FIELD_NONE; return 0; @@ -625,11 +624,6 @@ static int ov6650_s_fmt(struct v4l2_subdev *sd, struct v4l2_mbus_framefmt *mf) priv->pclk_max = 8000000; } - if (code == MEDIA_BUS_FMT_SBGGR8_1X8) - priv->colorspace = V4L2_COLORSPACE_SRGB; - else if (code != 0) - priv->colorspace = V4L2_COLORSPACE_JPEG; - if (half_scale) { dev_dbg(&client->dev, "max resolution: QCIF\n"); coma_set |= COMA_QCIF; @@ -660,7 +654,6 @@ static int ov6650_s_fmt(struct v4l2_subdev *sd, struct v4l2_mbus_framefmt *mf) ret = ov6650_reg_rmw(client, REG_COML, coml_set, coml_mask); if (!ret) { - mf->colorspace = priv->colorspace; mf->width = priv->rect.width >> half_scale; mf->height = priv->rect.height >> half_scale; } @@ -683,6 +676,7 @@ static int ov6650_set_fmt(struct v4l2_subdev *sd, &mf->height, 2, H_CIF, 1, 0); mf->field = V4L2_FIELD_NONE; + mf->colorspace = V4L2_COLORSPACE_SRGB; switch (mf->code) { case MEDIA_BUS_FMT_Y10_1X10: @@ -693,13 +687,11 @@ static int ov6650_set_fmt(struct v4l2_subdev *sd, case MEDIA_BUS_FMT_YUYV8_2X8: case MEDIA_BUS_FMT_VYUY8_2X8: case MEDIA_BUS_FMT_UYVY8_2X8: - mf->colorspace = V4L2_COLORSPACE_JPEG; break; default: mf->code = MEDIA_BUS_FMT_SBGGR8_1X8; /* fall through */ case MEDIA_BUS_FMT_SBGGR8_1X8: - mf->colorspace = V4L2_COLORSPACE_SRGB; break; } @@ -1007,7 +999,6 @@ static int ov6650_probe(struct i2c_client *client, priv->rect.height = H_CIF; priv->half_scale = false; priv->code = MEDIA_BUS_FMT_YUYV8_2X8; - priv->colorspace = V4L2_COLORSPACE_JPEG; priv->subdev.internal_ops = &ov6650_internal_ops; priv->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE; -- 2.21.0

5 years, 9 months

1
0
0 0

[PATCH v2 3/9] media: ov6650: Fix crop rectangle alignment not passed back

by Janusz Krzysztofik

Commit 4f996594ceaf ("[media] v4l2: make vidioc_s_crop const") introduced a writable copy of constified user requested crop rectangle in order to be able to perform hardware alignments on it. Later on, commit 10d5509c8d50 ("[media] v4l2: remove g/s_crop from video ops") replaced s_crop() video operaion using that const argument with set_selection() pad operation which had a corresponding argument not constified, however the original behavior of the driver was not restored. Since that time, any hardware alignment applied on a user requested crop rectangle is not passed back to the user calling .set_selection() as it should be. Fix the issue by dropping the copy and replacing all references to it with references to the crop rectangle embedded in the user argument. Fixes: 10d5509c8d50 ("[media] v4l2: remove g/s_crop from video ops") Signed-off-by: Janusz Krzysztofik <jmkrzyszt(a)gmail.com> Cc: stable(a)vger.kernel.org --- drivers/media/i2c/ov6650.c | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/drivers/media/i2c/ov6650.c b/drivers/media/i2c/ov6650.c index 007f0ca24913..751b48483f27 100644 --- a/drivers/media/i2c/ov6650.c +++ b/drivers/media/i2c/ov6650.c @@ -468,38 +468,37 @@ static int ov6650_set_selection(struct v4l2_subdev *sd, { struct i2c_client *client = v4l2_get_subdevdata(sd); struct ov6650 *priv = to_ov6650(client); - struct v4l2_rect rect = sel->r; int ret; if (sel->which != V4L2_SUBDEV_FORMAT_ACTIVE || sel->target != V4L2_SEL_TGT_CROP) return -EINVAL; - v4l_bound_align_image(&rect.width, 2, W_CIF, 1, - &rect.height, 2, H_CIF, 1, 0); - v4l_bound_align_image(&rect.left, DEF_HSTRT << 1, - (DEF_HSTRT << 1) + W_CIF - (__s32)rect.width, 1, - &rect.top, DEF_VSTRT << 1, - (DEF_VSTRT << 1) + H_CIF - (__s32)rect.height, 1, - 0); + v4l_bound_align_image(&sel->r.width, 2, W_CIF, 1, + &sel->r.height, 2, H_CIF, 1, 0); + v4l_bound_align_image(&sel->r.left, DEF_HSTRT << 1, + (DEF_HSTRT << 1) + W_CIF - (__s32)sel->r.width, 1, + &sel->r.top, DEF_VSTRT << 1, + (DEF_VSTRT << 1) + H_CIF - (__s32)sel->r.height, + 1, 0); - ret = ov6650_reg_write(client, REG_HSTRT, rect.left >> 1); + ret = ov6650_reg_write(client, REG_HSTRT, sel->r.left >> 1); if (!ret) { - priv->rect.left = rect.left; + priv->rect.left = sel->r.left; ret = ov6650_reg_write(client, REG_HSTOP, - (rect.left + rect.width) >> 1); + (sel->r.left + sel->r.width) >> 1); } if (!ret) { - priv->rect.width = rect.width; - ret = ov6650_reg_write(client, REG_VSTRT, rect.top >> 1); + priv->rect.width = sel->r.width; + ret = ov6650_reg_write(client, REG_VSTRT, sel->r.top >> 1); } if (!ret) { - priv->rect.top = rect.top; + priv->rect.top = sel->r.top; ret = ov6650_reg_write(client, REG_VSTOP, - (rect.top + rect.height) >> 1); + (sel->r.top + sel->r.height) >> 1); } if (!ret) - priv->rect.height = rect.height; + priv->rect.height = sel->r.height; return ret; } -- 2.21.0

5 years, 9 months

1
0
0 0

[PATCH v2 2/9] media: ov6650: Fix control handler not freed on init error

by Janusz Krzysztofik

Since commit afd9690c72c3 ("[media] ov6650: convert to the control framework"), if an error occurs during initialization of a control handler, resources possibly allocated to the handler are not freed before device initialiaton is aborted. Fix it. Fixes: afd9690c72c3 ("[media] ov6650: convert to the control framework") Signed-off-by: Janusz Krzysztofik <jmkrzyszt(a)gmail.com> Cc: stable(a)vger.kernel.org --- drivers/media/i2c/ov6650.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/drivers/media/i2c/ov6650.c b/drivers/media/i2c/ov6650.c index a3d00afcb0c8..007f0ca24913 100644 --- a/drivers/media/i2c/ov6650.c +++ b/drivers/media/i2c/ov6650.c @@ -992,8 +992,10 @@ static int ov6650_probe(struct i2c_client *client, V4L2_CID_GAMMA, 0, 0xff, 1, 0x12); priv->subdev.ctrl_handler = &priv->hdl; - if (priv->hdl.error) - return priv->hdl.error; + if (priv->hdl.error) { + ret = priv->hdl.error; + goto ectlhdlfree; + } v4l2_ctrl_auto_cluster(2, &priv->autogain, 0, true); v4l2_ctrl_auto_cluster(3, &priv->autowb, 0, true); @@ -1012,8 +1014,10 @@ static int ov6650_probe(struct i2c_client *client, priv->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE; ret = v4l2_async_register_subdev(&priv->subdev); - if (ret) - v4l2_ctrl_handler_free(&priv->hdl); + if (!ret) + return 0; +ectlhdlfree: + v4l2_ctrl_handler_free(&priv->hdl); return ret; } -- 2.21.0

5 years, 9 months

1
0
0 0

[PATCH 1/2] perf/x86/amd/uncore: Do not set ThreadMask and SliceMask for non-L3 PMCs

by Phillips, Kim

From: Kim Phillips <kim.phillips(a)amd.com> Commit d7cbbe49a930 ("perf/x86/amd/uncore: Set ThreadMask and SliceMask for L3 Cache perf events") enables L3 PMC events for all threads and slices by writing 1s in ChL3PmcCfg (L3 PMC PERF_CTL) register fields. Those bitfields overlap with high order event select bits in the Data Fabric PMC control register, however. So when a user requests raw Data Fabric events (-e amd_df/event=0xYYY/), the two highest order bits get inadvertently set, changing the counter select to events that don't exist, and for which no counts are read. This patch changes the logic to write the L3 masks only when dealing with L3 PMC counters. AMD Family 16h and below Northbridge (NB) counters were not affected. Signed-off-by: Kim Phillips <kim.phillips(a)amd.com> Cc: <stable(a)vger.kernel.org> # v4.19+ Cc: Peter Zijlstra <peterz(a)infradead.org> Cc: Ingo Molnar <mingo(a)redhat.com> Cc: Arnaldo Carvalho de Melo <acme(a)kernel.org> Cc: Alexander Shishkin <alexander.shishkin(a)linux.intel.com> Cc: Jiri Olsa <jolsa(a)redhat.com> Cc: Namhyung Kim <namhyung(a)kernel.org> Cc: Thomas Gleixner <tglx(a)linutronix.de> Cc: Borislav Petkov <bp(a)alien8.de> Cc: "H. Peter Anvin" <hpa(a)zytor.com> Cc: Martin Liška <mliska(a)suse.cz> Cc: Suravee Suthikulpanit <Suravee.Suthikulpanit(a)amd.com> Cc: Janakarajan Natarajan <Janakarajan.Natarajan(a)amd.com> Cc: Gary Hook <Gary.Hook(a)amd.com> Cc: Pu Wen <puwen(a)hygon.cn> Cc: Stephane Eranian <eranian(a)google.com> Cc: Vince Weaver <vincent.weaver(a)maine.edu> Cc: x86(a)kernel.org Fixes: d7cbbe49a930 ("perf/x86/amd/uncore: Set ThreadMask and SliceMask for L3 Cache perf events") --- arch/x86/events/amd/uncore.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/events/amd/uncore.c b/arch/x86/events/amd/uncore.c index 79cfd3b30ceb..a8dc2635a719 100644 --- a/arch/x86/events/amd/uncore.c +++ b/arch/x86/events/amd/uncore.c @@ -209,7 +209,7 @@ static int amd_uncore_event_init(struct perf_event *event) * SliceMask and ThreadMask need to be set for certain L3 events in * Family 17h. For other events, the two fields do not affect the count. */ - if (l3_mask) + if (l3_mask && is_llc_event(event)) hwc->config |= (AMD64_L3_SLICE_MASK | AMD64_L3_THREAD_MASK); if (event->cpu < 0) -- 2.21.0

5 years, 9 months

1
0
0 0

[PATCH 1/4] drm/msm: Fix improper uses of smp_mb__{before, after}_atomic()

by Andrea Parri

These barriers only apply to the read-modify-write operations; in particular, they do not apply to the atomic_set() primitive. Replace the barriers with smp_mb()s. Fixes: b1fc2839d2f92 ("drm/msm: Implement preemption for A5XX targets") Cc: stable(a)vger.kernel.org Reported-by: "Paul E. McKenney" <paulmck(a)linux.ibm.com> Reported-by: Peter Zijlstra <peterz(a)infradead.org> Signed-off-by: Andrea Parri <andrea.parri(a)amarulasolutions.com> Cc: Rob Clark <robdclark(a)gmail.com> Cc: Sean Paul <sean(a)poorly.run> Cc: David Airlie <airlied(a)linux.ie> Cc: Daniel Vetter <daniel(a)ffwll.ch> Cc: Jordan Crouse <jcrouse(a)codeaurora.org> Cc: linux-arm-msm(a)vger.kernel.org Cc: dri-devel(a)lists.freedesktop.org Cc: freedreno(a)lists.freedesktop.org Cc: "Paul E. McKenney" <paulmck(a)linux.ibm.com> Cc: Peter Zijlstra <peterz(a)infradead.org> --- drivers/gpu/drm/msm/adreno/a5xx_preempt.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/msm/adreno/a5xx_preempt.c b/drivers/gpu/drm/msm/adreno/a5xx_preempt.c index 3d62310a535fb..ee0820ee0c664 100644 --- a/drivers/gpu/drm/msm/adreno/a5xx_preempt.c +++ b/drivers/gpu/drm/msm/adreno/a5xx_preempt.c @@ -39,10 +39,10 @@ static inline void set_preempt_state(struct a5xx_gpu *gpu, * preemption or in the interrupt handler so barriers are needed * before... */ - smp_mb__before_atomic(); + smp_mb(); atomic_set(&gpu->preempt_state, new); /* ... and after*/ - smp_mb__after_atomic(); + smp_mb(); } /* Write the most recent wptr for the given ring into the hardware */ -- 2.7.4

5 years, 9 months

2
1
0 0

[PATCH 1/2] media: dvb: warning about dvb frequency limits produces too much noise

by Sean Young

This can be a debug message. dvb_frontend: dvb_frontend_get_frequency_limits: frequency interval: tuner: 45000000...860000000, frontend: 44250000...867250000 Fixes: 00ecd6bc7128 ("media: dvb_frontend: add debug message for frequency intervals") Cc: <stable(a)vger.kernel.org> # 5.0 Signed-off-by: Sean Young <sean(a)mess.org> --- drivers/media/dvb-core/dvb_frontend.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/media/dvb-core/dvb_frontend.c b/drivers/media/dvb-core/dvb_frontend.c index fbdb4ecc7c50..d3c0f6267bf8 100644 --- a/drivers/media/dvb-core/dvb_frontend.c +++ b/drivers/media/dvb-core/dvb_frontend.c @@ -917,8 +917,9 @@ static void dvb_frontend_get_frequency_limits(struct dvb_frontend *fe, "DVB: adapter %i frontend %u frequency limits undefined - fix the driver\n", fe->dvb->num, fe->id); - dprintk("frequency interval: tuner: %u...%u, frontend: %u...%u", - tuner_min, tuner_max, frontend_min, frontend_max); + if (dvb_frontend_debug) + dprintk("frequency interval: tuner: %u...%u, frontend: %u...%u", + tuner_min, tuner_max, frontend_min, frontend_max); /* If the standard is for satellite, convert frequencies to kHz */ switch (c->delivery_system) { -- 2.20.1

5 years, 9 months

1
0
0 0

Re: [PATCH 2/2] md/raid0: Do not bypass blocking queue entered for raid0 bios

by Wols Lists

On 08/05/19 15:52, Guilherme G. Piccoli wrote: > Hi, I understand your concern. But all other raid levels contains > failure-event mechanisms. For example, in all my tests with raid5 or > raid1, it first complained the device was removed, then it failed in > super_written() when no other available device was present. > On the other hand, raid0 does "blind-writes": it just selects the device > in which that bio should be written (given the stripe math) and change > the bio's device, sending it back via generic_make_request(). It's > dummy, but not in a bad way, but rather for performance reasons. It has > no "intelligence" for failures, as all other raid levels. > > That said, we could fix md.c for all raid levels, but I personally think > it's a bazooka shot, only raid0 shows consistently this issue. > The academic in me says we should push that error handling into generic_make_request() or some raid function in md.c that deals with those problems. Sounds like there's a fair bit of duplicate functionality that could be re-factored out. >> >> Academic purity versus engineering practicality :-) > > Heheh you have good points here! Thanks for the input =) > Cheers, > Doesn't help when there's not an architect to design an overall "grand scheme", but my usual way of working is to design top down academically, and then ask myself "what do I need" before implementing bottom-up. Hopefully with a load of documentation saying "I haven't done this because I don't need it, but this is where it goes". Cheers, Wol

5 years, 9 months

4
3
0 0

[PATCH v2] libnvdimm/pmem: Bypass CONFIG_HARDENED_USERCOPY overhead

by Dan Williams

Jeff discovered that performance improves from ~375K iops to ~519K iops on a simple psync-write fio workload when moving the location of 'struct page' from the default PMEM location to DRAM. This result is surprising because the expectation is that 'struct page' for dax is only needed for third party references to dax mappings. For example, a dax-mapped buffer passed to another system call for direct-I/O requires 'struct page' for sending the request down the driver stack and pinning the page. There is no usage of 'struct page' for first party access to a file via read(2)/write(2) and friends. However, this "no page needed" expectation is violated by CONFIG_HARDENED_USERCOPY and the check_copy_size() performed in copy_from_iter_full_nocache() and copy_to_iter_mcsafe(). The check_heap_object() helper routine assumes the buffer is backed by a slab allocator (DRAM) page and applies some checks. Those checks are invalid, dax pages do not originate from the slab, and redundant, dax_iomap_actor() has already validated that the I/O is within bounds. Specifically that routine validates that the logical file offset is within bounds of the file, then it does a sector-to-pfn translation which validates that the physical mapping is within bounds of the block device. Bypass additional hardened usercopy overhead and call the 'no check' versions of the copy_{to,from}_iter operations directly. Fixes: 0aed55af8834 ("x86, uaccess: introduce copy_from_iter_flushcache...") Cc: <stable(a)vger.kernel.org> Cc: Jeff Moyer <jmoyer(a)redhat.com> Cc: Ingo Molnar <mingo(a)redhat.com> Cc: Christoph Hellwig <hch(a)lst.de> Cc: Al Viro <viro(a)zeniv.linux.org.uk> Cc: Thomas Gleixner <tglx(a)linutronix.de> Cc: Matthew Wilcox <willy(a)infradead.org> Reported-and-tested-by: Jeff Smits <jeff.smits(a)intel.com> Acked-by: Kees Cook <keescook(a)chromium.org> Acked-by: Jan Kara <jack(a)suse.cz> Signed-off-by: Dan Williams <dan.j.williams(a)intel.com> --- Changes since v1 [1]: * Update the changelog to clarify which checks in dax_iomap_actor() obviate the need for "hardened" checks. (Jan) * Update the code comment in drivers/nvdimm/pmem.c to reflect the same. * Collect some Acks from Kees and Jan. [1]: https://lore.kernel.org/lkml/155805321833.867447.3864104616303535270.stgit@… drivers/nvdimm/pmem.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c index 845c5b430cdd..c894f45e5077 100644 --- a/drivers/nvdimm/pmem.c +++ b/drivers/nvdimm/pmem.c @@ -281,16 +281,21 @@ static long pmem_dax_direct_access(struct dax_device *dax_dev, return __pmem_direct_access(pmem, pgoff, nr_pages, kaddr, pfn); } +/* + * Use the 'no check' versions of copy_from_iter_flushcache() and + * copy_to_iter_mcsafe() to bypass HARDENED_USERCOPY overhead. Bounds + * checking is handled by dax_iomap_actor() + */ static size_t pmem_copy_from_iter(struct dax_device *dax_dev, pgoff_t pgoff, void *addr, size_t bytes, struct iov_iter *i) { - return copy_from_iter_flushcache(addr, bytes, i); + return _copy_from_iter_flushcache(addr, bytes, i); } static size_t pmem_copy_to_iter(struct dax_device *dax_dev, pgoff_t pgoff, void *addr, size_t bytes, struct iov_iter *i) { - return copy_to_iter_mcsafe(addr, bytes, i); + return _copy_to_iter_mcsafe(addr, bytes, i); } static const struct dax_operations pmem_dax_ops = {

5 years, 9 months

1
0
0 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

Linux-stable-mirror May 2019