Re: [PATCH] net: stmmac: make const array route_possibilities static
From: Colin King Date: Tue, 11 Jul 2017 12:18:48 +0100 > From: Colin Ian King > > Don't populate array route_possibilities on the stack but make it > static const. Makes the object code a little smaller by 85 bytes: > > Before: >text data bss dec hex filename >9901 2448 0 12349303d dwmac4_core.o > > After: >text data bss dec hex filename >9760 2504 0 122642fe8 dwmac4_core.o > > Signed-off-by: Colin Ian King Applied.
[PATCH v5 10/14] drm: mgag200: remove dead code and pointless local lut storage
The redundant fb helpers .load_lut, .gamma_set and .gamma_get are no longer used. Remove the dead code and hook up the crtc .gamma_set to use the crtc gamma_store directly instead of duplicating that info locally. Acked-by: Daniel Vetter Signed-off-by: Peter Rosin --- drivers/gpu/drm/mgag200/mgag200_drv.h | 5 --- drivers/gpu/drm/mgag200/mgag200_fb.c | 2 -- drivers/gpu/drm/mgag200/mgag200_mode.c | 62 -- 3 files changed, 15 insertions(+), 54 deletions(-) diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h index c88b6ec88dd2..04f1dfba12e5 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.h +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h @@ -237,11 +237,6 @@ mgag200_bo(struct ttm_buffer_object *bo) { return container_of(bo, struct mgag200_bo, bo); } - /* mgag200_crtc.c */ -void mga_crtc_fb_gamma_set(struct drm_crtc *crtc, u16 red, u16 green, -u16 blue, int regno); -void mga_crtc_fb_gamma_get(struct drm_crtc *crtc, u16 *red, u16 *green, -u16 *blue, int regno); /* mgag200_mode.c */ int mgag200_modeset_init(struct mga_device *mdev); diff --git a/drivers/gpu/drm/mgag200/mgag200_fb.c b/drivers/gpu/drm/mgag200/mgag200_fb.c index 5d3b1fac906f..5cf980a2e9e9 100644 --- a/drivers/gpu/drm/mgag200/mgag200_fb.c +++ b/drivers/gpu/drm/mgag200/mgag200_fb.c @@ -258,8 +258,6 @@ static int mga_fbdev_destroy(struct drm_device *dev, } static const struct drm_fb_helper_funcs mga_fb_helper_funcs = { - .gamma_set = mga_crtc_fb_gamma_set, - .gamma_get = mga_crtc_fb_gamma_get, .fb_probe = mgag200fb_create, }; diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index f4b53588e071..5e9cd4c0e8b6 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -27,15 +27,19 @@ static void mga_crtc_load_lut(struct drm_crtc *crtc) { - struct mga_crtc *mga_crtc = to_mga_crtc(crtc); struct drm_device *dev = crtc->dev; struct mga_device *mdev = dev->dev_private; struct drm_framebuffer *fb = crtc->primary->fb; + u16 *r_ptr, *g_ptr, *b_ptr; int i; if (!crtc->enabled) return; + r_ptr = crtc->gamma_store; + g_ptr = r_ptr + crtc->gamma_size; + b_ptr = g_ptr + crtc->gamma_size; + WREG8(DAC_INDEX + MGA1064_INDEX, 0); if (fb && fb->format->cpp[0] * 8 == 16) { @@ -46,25 +50,27 @@ static void mga_crtc_load_lut(struct drm_crtc *crtc) if (i > (MGAG200_LUT_SIZE >> 1)) { r = b = 0; } else { - r = mga_crtc->lut_r[i << 1]; - b = mga_crtc->lut_b[i << 1]; + r = *r_ptr++ >> 8; + b = *b_ptr++ >> 8; + r_ptr++; + b_ptr++; } } else { - r = mga_crtc->lut_r[i]; - b = mga_crtc->lut_b[i]; + r = *r_ptr++ >> 8; + b = *b_ptr++ >> 8; } /* VGA registers */ WREG8(DAC_INDEX + MGA1064_COL_PAL, r); - WREG8(DAC_INDEX + MGA1064_COL_PAL, mga_crtc->lut_g[i]); + WREG8(DAC_INDEX + MGA1064_COL_PAL, *g_ptr++ >> 8); WREG8(DAC_INDEX + MGA1064_COL_PAL, b); } return; } for (i = 0; i < MGAG200_LUT_SIZE; i++) { /* VGA registers */ - WREG8(DAC_INDEX + MGA1064_COL_PAL, mga_crtc->lut_r[i]); - WREG8(DAC_INDEX + MGA1064_COL_PAL, mga_crtc->lut_g[i]); - WREG8(DAC_INDEX + MGA1064_COL_PAL, mga_crtc->lut_b[i]); + WREG8(DAC_INDEX + MGA1064_COL_PAL, *r_ptr++ >> 8); + WREG8(DAC_INDEX + MGA1064_COL_PAL, *g_ptr++ >> 8); + WREG8(DAC_INDEX + MGA1064_COL_PAL, *b_ptr++ >> 8); } } @@ -1399,14 +1405,6 @@ static int mga_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green, u16 *blue, uint32_t size, struct drm_modeset_acquire_ctx *ctx) { - struct mga_crtc *mga_crtc = to_mga_crtc(crtc); - int i; - - for (i = 0; i < size; i++) { - mga_crtc->lut_r[i] = red[i] >> 8; - mga_crtc->lut_g[i] = green[i] >> 8; - mga_crtc->lut_b[i] = blue[i] >> 8; - } mga_crtc_load_lut(crtc); return 0; @@ -1455,14 +1453,12 @@ static const struct drm_crtc_helper_funcs mga_helper_funcs
Re: [PATCH] dccp: make const array error_code static
From: Colin King Date: Thu, 13 Jul 2017 12:22:24 +0100 > From: Colin Ian King > > Don't populate array error_code on the stack but make it static. Makes > the object code smaller by almost 250 bytes: > > Before: >text data bss dec hex filename > 10366 983 0 113492c55 net/dccp/input.o > > After: >text data bss dec hex filename > 10161 1039 0 112002bc0 net/dccp/input.o > > Signed-off-by: Colin Ian King Applied.
[PATCH v5 08/14] drm: gma500: remove dead code and pointless local lut storage
The redundant fb helpers .gamma_set and .gamma_get are no longer used. Remove the dead code and hook up the crtc .gamma_set to use the crtc gamma_store directly instead of duplicating that info locally. Acked-by: Daniel Vetter Signed-off-by: Peter Rosin --- drivers/gpu/drm/gma500/framebuffer.c | 22 drivers/gpu/drm/gma500/gma_display.c | 32 ++ drivers/gpu/drm/gma500/psb_intel_display.c | 7 +-- drivers/gpu/drm/gma500/psb_intel_drv.h | 1 - 4 files changed, 12 insertions(+), 50 deletions(-) diff --git a/drivers/gpu/drm/gma500/framebuffer.c b/drivers/gpu/drm/gma500/framebuffer.c index 7da70b6c83f0..2570c7f647a6 100644 --- a/drivers/gpu/drm/gma500/framebuffer.c +++ b/drivers/gpu/drm/gma500/framebuffer.c @@ -479,26 +479,6 @@ static struct drm_framebuffer *psb_user_framebuffer_create return psb_framebuffer_create(dev, cmd, r); } -static void psbfb_gamma_set(struct drm_crtc *crtc, u16 red, u16 green, - u16 blue, int regno) -{ - struct gma_crtc *gma_crtc = to_gma_crtc(crtc); - - gma_crtc->lut_r[regno] = red >> 8; - gma_crtc->lut_g[regno] = green >> 8; - gma_crtc->lut_b[regno] = blue >> 8; -} - -static void psbfb_gamma_get(struct drm_crtc *crtc, u16 *red, - u16 *green, u16 *blue, int regno) -{ - struct gma_crtc *gma_crtc = to_gma_crtc(crtc); - - *red = gma_crtc->lut_r[regno] << 8; - *green = gma_crtc->lut_g[regno] << 8; - *blue = gma_crtc->lut_b[regno] << 8; -} - static int psbfb_probe(struct drm_fb_helper *helper, struct drm_fb_helper_surface_size *sizes) { @@ -525,8 +505,6 @@ static int psbfb_probe(struct drm_fb_helper *helper, } static const struct drm_fb_helper_funcs psb_fb_helper_funcs = { - .gamma_set = psbfb_gamma_set, - .gamma_get = psbfb_gamma_get, .fb_probe = psbfb_probe, }; diff --git a/drivers/gpu/drm/gma500/gma_display.c b/drivers/gpu/drm/gma500/gma_display.c index e7fd356acf2e..f3c48a2be71b 100644 --- a/drivers/gpu/drm/gma500/gma_display.c +++ b/drivers/gpu/drm/gma500/gma_display.c @@ -144,33 +144,32 @@ void gma_crtc_load_lut(struct drm_crtc *crtc) struct gma_crtc *gma_crtc = to_gma_crtc(crtc); const struct psb_offset *map = &dev_priv->regmap[gma_crtc->pipe]; int palreg = map->palette; + u16 *r, *g, *b; int i; /* The clocks have to be on to load the palette. */ if (!crtc->enabled) return; + r = crtc->gamma_store; + g = r + crtc->gamma_size; + b = g + crtc->gamma_size; + if (gma_power_begin(dev, false)) { for (i = 0; i < 256; i++) { REG_WRITE(palreg + 4 * i, - ((gma_crtc->lut_r[i] + - gma_crtc->lut_adj[i]) << 16) | - ((gma_crtc->lut_g[i] + - gma_crtc->lut_adj[i]) << 8) | - (gma_crtc->lut_b[i] + - gma_crtc->lut_adj[i])); + (((*r++ >> 8) + gma_crtc->lut_adj[i]) << 16) | + (((*g++ >> 8) + gma_crtc->lut_adj[i]) << 8) | + ((*b++ >> 8) + gma_crtc->lut_adj[i])); } gma_power_end(dev); } else { for (i = 0; i < 256; i++) { /* FIXME: Why pipe[0] and not pipe[..._crtc->pipe]? */ dev_priv->regs.pipe[0].palette[i] = - ((gma_crtc->lut_r[i] + - gma_crtc->lut_adj[i]) << 16) | - ((gma_crtc->lut_g[i] + - gma_crtc->lut_adj[i]) << 8) | - (gma_crtc->lut_b[i] + - gma_crtc->lut_adj[i]); + (((*r++ >> 8) + gma_crtc->lut_adj[i]) << 16) | + (((*g++ >> 8) + gma_crtc->lut_adj[i]) << 8) | + ((*b++ >> 8) + gma_crtc->lut_adj[i]); } } @@ -180,15 +179,6 @@ int gma_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green, u16 *blue, u32 size, struct drm_modeset_acquire_ctx *ctx) { - struct gma_crtc *gma_crtc = to_gma_crtc(crtc); - int i; - - for (i = 0; i < size; i++) { - gma_crtc->lut_r[i] = red[i] >> 8; - gma_crtc->lut_g[i] = green[i] >> 8; - gma_crtc->lut_b[i] = blue[i] >> 8; - } - gma_crtc_load_lut(crtc); return 0; diff --git a/drivers/gpu/drm/gma500/psb_intel_display.c b/drivers/gpu/drm/gma500/psb_intel_display.c index 7b6c84925098..8762efaef283 100644 --- a/drivers/gpu/drm/gma500/psb_intel
[PATCH v5 07/14] drm: cirrus: remove dead code and pointless local lut storage
The redundant fb helpers .load_lut, .gamma_set and .gamma_get are no longer used. Remove the dead code and hook up the crtc .gamma_set to use the crtc gamma_store directly instead of duplicating that info locally. Acked-by: Daniel Vetter Signed-off-by: Peter Rosin --- drivers/gpu/drm/cirrus/cirrus_drv.h | 8 drivers/gpu/drm/cirrus/cirrus_fbdev.c | 2 - drivers/gpu/drm/cirrus/cirrus_mode.c | 71 --- 3 files changed, 16 insertions(+), 65 deletions(-) diff --git a/drivers/gpu/drm/cirrus/cirrus_drv.h b/drivers/gpu/drm/cirrus/cirrus_drv.h index 8690352d96f7..be2d7e488062 100644 --- a/drivers/gpu/drm/cirrus/cirrus_drv.h +++ b/drivers/gpu/drm/cirrus/cirrus_drv.h @@ -96,7 +96,6 @@ struct cirrus_crtc { struct drm_crtc base; - u8 lut_r[256], lut_g[256], lut_b[256]; int last_dpms; boolenabled; }; @@ -180,13 +179,6 @@ cirrus_bo(struct ttm_buffer_object *bo) #define to_cirrus_obj(x) container_of(x, struct cirrus_gem_object, base) #define DRM_FILE_PAGE_OFFSET (0x1ULL >> PAGE_SHIFT) - /* cirrus_mode.c */ -void cirrus_crtc_fb_gamma_set(struct drm_crtc *crtc, u16 red, u16 green, -u16 blue, int regno); -void cirrus_crtc_fb_gamma_get(struct drm_crtc *crtc, u16 *red, u16 *green, -u16 *blue, int regno); - - /* cirrus_main.c */ int cirrus_device_init(struct cirrus_device *cdev, struct drm_device *ddev, diff --git a/drivers/gpu/drm/cirrus/cirrus_fbdev.c b/drivers/gpu/drm/cirrus/cirrus_fbdev.c index 7fa58eeadc9d..1fedab03f659 100644 --- a/drivers/gpu/drm/cirrus/cirrus_fbdev.c +++ b/drivers/gpu/drm/cirrus/cirrus_fbdev.c @@ -265,8 +265,6 @@ static int cirrus_fbdev_destroy(struct drm_device *dev, } static const struct drm_fb_helper_funcs cirrus_fb_helper_funcs = { - .gamma_set = cirrus_crtc_fb_gamma_set, - .gamma_get = cirrus_crtc_fb_gamma_get, .fb_probe = cirrusfb_create, }; diff --git a/drivers/gpu/drm/cirrus/cirrus_mode.c b/drivers/gpu/drm/cirrus/cirrus_mode.c index 53f6f0f84206..a4c4a465b385 100644 --- a/drivers/gpu/drm/cirrus/cirrus_mode.c +++ b/drivers/gpu/drm/cirrus/cirrus_mode.c @@ -31,25 +31,6 @@ * This file contains setup code for the CRTC. */ -static void cirrus_crtc_load_lut(struct drm_crtc *crtc) -{ - struct cirrus_crtc *cirrus_crtc = to_cirrus_crtc(crtc); - struct drm_device *dev = crtc->dev; - struct cirrus_device *cdev = dev->dev_private; - int i; - - if (!crtc->enabled) - return; - - for (i = 0; i < CIRRUS_LUT_SIZE; i++) { - /* VGA registers */ - WREG8(PALETTE_INDEX, i); - WREG8(PALETTE_DATA, cirrus_crtc->lut_r[i]); - WREG8(PALETTE_DATA, cirrus_crtc->lut_g[i]); - WREG8(PALETTE_DATA, cirrus_crtc->lut_b[i]); - } -} - /* * The DRM core requires DPMS functions, but they make little sense in our * case and so are just stubs @@ -330,15 +311,25 @@ static int cirrus_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green, u16 *blue, uint32_t size, struct drm_modeset_acquire_ctx *ctx) { - struct cirrus_crtc *cirrus_crtc = to_cirrus_crtc(crtc); + struct drm_device *dev = crtc->dev; + struct cirrus_device *cdev = dev->dev_private; + u16 *r, *g, *b; int i; - for (i = 0; i < size; i++) { - cirrus_crtc->lut_r[i] = red[i]; - cirrus_crtc->lut_g[i] = green[i]; - cirrus_crtc->lut_b[i] = blue[i]; + if (!crtc->enabled) + return 0; + + r = crtc->gamma_store; + g = r + crtc->gamma_size; + b = g + crtc->gamma_size; + + for (i = 0; i < CIRRUS_LUT_SIZE; i++) { + /* VGA registers */ + WREG8(PALETTE_INDEX, i); + WREG8(PALETTE_DATA, *r++ >> 8); + WREG8(PALETTE_DATA, *g++ >> 8); + WREG8(PALETTE_DATA, *b++ >> 8); } - cirrus_crtc_load_lut(crtc); return 0; } @@ -365,7 +356,6 @@ static const struct drm_crtc_helper_funcs cirrus_helper_funcs = { .mode_set_base = cirrus_crtc_mode_set_base, .prepare = cirrus_crtc_prepare, .commit = cirrus_crtc_commit, - .load_lut = cirrus_crtc_load_lut, }; /* CRTC setup */ @@ -373,7 +363,6 @@ static void cirrus_crtc_init(struct drm_device *dev) { struct cirrus_device *cdev = dev->dev_private; struct cirrus_crtc *cirrus_crtc; - int i; cirrus_crtc = kzalloc(sizeof(struct cirrus_crtc) + (CIRRUSFB_CONN_LIMIT * sizeof(struct drm_connector *)), @@ -387,37 +376,9 @@ static void cirrus_crtc_init(struct drm_device *dev) drm_mode_crtc_set_gamma_
Re: [PATCH] net: broadcom: bnx2x: make a couple of const arrays static
From: Colin King Date: Tue, 11 Jul 2017 11:52:23 +0100 > From: Colin Ian King > > Don't populate various tables on the stack but make them static const. > Makes the object code smaller by nearly 200 bytes: > > Before: >text data bss dec hex filename > 113468 11200 0 124668 1e6fc bnx2x_ethtool.o > > After: >text data bss dec hex filename > 113129 11344 0 124473 1e639 bnx2x_ethtool.o > > Signed-off-by: Colin Ian King Applied.
[PATCH v5 05/14] drm: armada: remove dead empty functions
The redundant fb helpers .gamma_set and .gamma_get are no longer used. Remove the dead code. Acked-by: Daniel Vetter Signed-off-by: Peter Rosin --- drivers/gpu/drm/armada/armada_crtc.c | 10 -- drivers/gpu/drm/armada/armada_crtc.h | 2 -- drivers/gpu/drm/armada/armada_fbdev.c | 2 -- 3 files changed, 14 deletions(-) diff --git a/drivers/gpu/drm/armada/armada_crtc.c b/drivers/gpu/drm/armada/armada_crtc.c index b57fb80acec1..5d5cc3289a81 100644 --- a/drivers/gpu/drm/armada/armada_crtc.c +++ b/drivers/gpu/drm/armada/armada_crtc.c @@ -334,16 +334,6 @@ static void armada_drm_vblank_off(struct armada_crtc *dcrtc) armada_drm_plane_work_run(dcrtc, dcrtc->crtc.primary); } -void armada_drm_crtc_gamma_set(struct drm_crtc *crtc, u16 r, u16 g, u16 b, - int idx) -{ -} - -void armada_drm_crtc_gamma_get(struct drm_crtc *crtc, u16 *r, u16 *g, u16 *b, - int idx) -{ -} - /* The mode_config.mutex will be held for this call */ static void armada_drm_crtc_dpms(struct drm_crtc *crtc, int dpms) { diff --git a/drivers/gpu/drm/armada/armada_crtc.h b/drivers/gpu/drm/armada/armada_crtc.h index 7e8906d3ae26..bab11f483575 100644 --- a/drivers/gpu/drm/armada/armada_crtc.h +++ b/drivers/gpu/drm/armada/armada_crtc.h @@ -102,8 +102,6 @@ struct armada_crtc { }; #define drm_to_armada_crtc(c) container_of(c, struct armada_crtc, crtc) -void armada_drm_crtc_gamma_set(struct drm_crtc *, u16, u16, u16, int); -void armada_drm_crtc_gamma_get(struct drm_crtc *, u16 *, u16 *, u16 *, int); void armada_drm_crtc_update_regs(struct armada_crtc *, struct armada_regs *); void armada_drm_crtc_plane_disable(struct armada_crtc *dcrtc, diff --git a/drivers/gpu/drm/armada/armada_fbdev.c b/drivers/gpu/drm/armada/armada_fbdev.c index 602dfead8eef..5fa076d6fabc 100644 --- a/drivers/gpu/drm/armada/armada_fbdev.c +++ b/drivers/gpu/drm/armada/armada_fbdev.c @@ -118,8 +118,6 @@ static int armada_fb_probe(struct drm_fb_helper *fbh, } static const struct drm_fb_helper_funcs armada_fb_helper_funcs = { - .gamma_set = armada_drm_crtc_gamma_set, - .gamma_get = armada_drm_crtc_gamma_get, .fb_probe = armada_fb_probe, }; -- 2.11.0
[PATCH v5 00/14] improve the fb_setcmap helper
Hi! While trying to get CLUT support for the atmel_hlcdc driver, and specifically for the emulated fbdev interface, I received some push-back that my feeble in-driver attempts should be solved by the core. This is my attempt to do it right. I have obviously not tested all of this with more than a compile, but patches 1 and 3 are enough to make the atmel-hlcdc driver do what I need. The rest is just lots of removals and cleanup made possible by the other improvements. Please test, I would not be surprised if I have fouled up some bit-manipulation somewhere, or if I have misunderstood something about atomics... Changes since v5: - Rebased onto fresher drm-misc-next. - Instead of just exporting drm_atomic_replace_propery_blob(), move it to drm_propery.c, rename it to drm_property_replace_blob() and change its semantics to return if the blob was replaced. - Install the same gamma_lut blob for all crtc, regardless of any variation in the gamma_lut state for the individual crtc prior to the fbdev setcmap call. - Add acks from Daniel for patches 4-14. Changes since v3: - Rebased onto drm-misc-next and dropped patches 1-3 from v3, since they are already merged. - Dropped the v3 patch 4/16 ("drm/color-mgmt: move atomic state/commit out from .gamma_set") since the atomic setcmap no longer uses the crtc .gamma_set callback. - Added patch 1/14 which exports drm_atomic_replace_property_blob... - ...and patch 2/14 which uses this new export to simplify drm_atomic_helper_legacy_gamma_set. - Big changes to patch 3/14 (was 5/16 in v3). It had various locking issues and the atomic setcmap is rather different. Changes since v2: - Added patch 1/16 which factors out pseudo-palette handling. - Removed the if (cmap->start + cmap->len < cmap->start) sanity check on the assumption that the fbdev core handles it. - Added patch 4/16 which factors out atomic state and commit handling from drm_atomic_helper_legacy_gamma_set to drm_mode_gamma_set_ioctl. - Do one atomic commit for all affected crtc. - Removed a now obsolete note in include/drm/drm_crtc.h (ammended the last patch). - Cc list is getting long, so I have redused the list for the individual patches. If you would like to get the full series (or nothing at all) for the next round (if that is needed) just say so. Changes since v1: - Rebased to next-20170621 - Split 1/11 into a preparatory patch, a cleanup patch and then the meat in 3/14. - Handle pseudo-palette for FB_VISUAL_TRUECOLOR. - Removed the empty .gamma_get/.gamma_set fb helpers from the armada driver that I had somehow managed to ignore but which 0day found real quick. - Be less judgemental on drivers only providing .gamma_get and .gamma_set, but no .load_lut. That's actually a valid thing to do if you only need pseudo-palette for FB_VISUAL_TRUECOLOR. - Add a comment about colliding bitfields in the nouveau driver. - Remove gamma_set/gamma_get declarations from the radeon driver (the definitions were removed in v1). Cheers, peda Peter Rosin (14): drm: rename, adjust and export drm_atomic_replace_property_blob drm/atomic-helper: update lut props directly in ..._legacy_gamma_set drm/fb-helper: separate the fb_setcmap helper into atomic and legacy paths drm: amd: remove dead code and pointless local lut storage drm: armada: remove dead empty functions drm: ast: remove dead code and pointless local lut storage drm: cirrus: remove dead code and pointless local lut storage drm: gma500: remove dead code and pointless local lut storage drm: i915: remove dead code and pointless local lut storage drm: mgag200: remove dead code and pointless local lut storage drm: nouveau: remove dead code and pointless local lut storage drm: radeon: remove dead code and pointless local lut storage drm: stm: remove dead code and pointless local lut storage drm: remove unused and redundant callbacks drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c | 24 --- drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h| 1 - drivers/gpu/drm/amd/amdgpu/dce_v10_0.c | 27 +--- drivers/gpu/drm/amd/amdgpu/dce_v11_0.c | 27 +--- drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 27 +--- drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 27 +--- drivers/gpu/drm/amd/amdgpu/dce_virtual.c| 23 --- drivers/gpu/drm/armada/armada_crtc.c| 10 -- drivers/gpu/drm/armada/armada_crtc.h| 2 - drivers/gpu/drm/armada/armada_fbdev.c | 2 - drivers/gpu/drm/ast/ast_drv.h | 1 - drivers/gpu/drm/ast/ast_fb.c| 20 --- drivers/gpu/drm/ast/ast_mode.c | 26 +--- drivers/gpu/drm/cirrus/cirrus_drv.h | 8 - drivers/gpu/drm/cirrus/cirrus_fbdev.c | 2 - drivers/gpu/drm/cirrus/cirrus_mode.c| 71 ++--- drivers/gpu/drm/drm_atomic.c| 30 +--- drivers/gpu/drm/drm_atomic_helper.c | 20 +-- drivers/gpu/drm/drm_fb_helper.c | 231 +++- drivers/gpu/d
[PATCH v5 03/14] drm/fb-helper: separate the fb_setcmap helper into atomic and legacy paths
The legacy path implements setcmap in terms of crtc .gamma_set. The atomic path implements setcmap by directly updating the crtc gamma_lut property. This has a couple of benefits: - it makes the redundant fb helpers .load_lut, .gamma_set and .gamma_get completely obsolete. They are now unused and subject for removal. - atomic drivers that support clut modes get fbdev support for those from the drm core. This includes atmel-hlcdc, but perhaps others as well? Signed-off-by: Peter Rosin --- drivers/gpu/drm/drm_fb_helper.c | 231 1 file changed, 160 insertions(+), 71 deletions(-) diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 721511da4de6..42090fe00ef9 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -1195,27 +1195,6 @@ void drm_fb_helper_set_suspend_unlocked(struct drm_fb_helper *fb_helper, } EXPORT_SYMBOL(drm_fb_helper_set_suspend_unlocked); -static int setcolreg(struct drm_crtc *crtc, u16 red, u16 green, -u16 blue, u16 regno, struct fb_info *info) -{ - struct drm_fb_helper *fb_helper = info->par; - struct drm_framebuffer *fb = fb_helper->fb; - - /* -* The driver really shouldn't advertise pseudo/directcolor -* visuals if it can't deal with the palette. -*/ - if (WARN_ON(!fb_helper->funcs->gamma_set || - !fb_helper->funcs->gamma_get)) - return -EINVAL; - - WARN_ON(fb->format->cpp[0] != 1); - - fb_helper->funcs->gamma_set(crtc, red, green, blue, regno); - - return 0; -} - static int setcmap_pseudo_palette(struct fb_cmap *cmap, struct fb_info *info) { u32 *palette = (u32 *)info->pseudo_palette; @@ -1248,57 +1227,138 @@ static int setcmap_pseudo_palette(struct fb_cmap *cmap, struct fb_info *info) return 0; } -/** - * drm_fb_helper_setcmap - implementation for &fb_ops.fb_setcmap - * @cmap: cmap to set - * @info: fbdev registered by the helper - */ -int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct fb_info *info) +static int setcmap_legacy(struct fb_cmap *cmap, struct fb_info *info) { struct drm_fb_helper *fb_helper = info->par; - struct drm_device *dev = fb_helper->dev; - const struct drm_crtc_helper_funcs *crtc_funcs; - u16 *red, *green, *blue, *transp; struct drm_crtc *crtc; u16 *r, *g, *b; - int i, j, rc = 0; - int start; + int i, ret = 0; - if (oops_in_progress) - return -EBUSY; + drm_modeset_lock_all(fb_helper->dev); + for (i = 0; i < fb_helper->crtc_count; i++) { + crtc = fb_helper->crtc_info[i].mode_set.crtc; + if (!crtc->funcs->gamma_set || !crtc->gamma_size) + return -EINVAL; - mutex_lock(&fb_helper->lock); - if (!drm_fb_helper_is_bound(fb_helper)) { - mutex_unlock(&fb_helper->lock); - return -EBUSY; + if (cmap->start + cmap->len > crtc->gamma_size) + return -EINVAL; + + r = crtc->gamma_store; + g = r + crtc->gamma_size; + b = g + crtc->gamma_size; + + memcpy(r + cmap->start, cmap->red, cmap->len * sizeof(*r)); + memcpy(g + cmap->start, cmap->green, cmap->len * sizeof(*g)); + memcpy(b + cmap->start, cmap->blue, cmap->len * sizeof(*b)); + + ret = crtc->funcs->gamma_set(crtc, r, g, b, +crtc->gamma_size, NULL); + if (ret) + return ret; } + drm_modeset_unlock_all(fb_helper->dev); - drm_modeset_lock_all(dev); - if (info->fix.visual == FB_VISUAL_TRUECOLOR) { - rc = setcmap_pseudo_palette(cmap, info); - goto out; + return ret; +} + +static struct drm_property_blob *setcmap_new_gamma_lut(struct drm_crtc *crtc, + struct fb_cmap *cmap) +{ + struct drm_device *dev = crtc->dev; + struct drm_property_blob *gamma_lut; + struct drm_color_lut *lut; + int size = crtc->gamma_size; + int i; + + if (!size || cmap->start + cmap->len > size) + return ERR_PTR(-EINVAL); + + gamma_lut = drm_property_create_blob(dev, sizeof(*lut) * size, NULL); + if (IS_ERR(gamma_lut)) + return gamma_lut; + + lut = (struct drm_color_lut *)gamma_lut->data; + if (cmap->start || cmap->len != size) { + u16 *r = crtc->gamma_store; + u16 *g = r + crtc->gamma_size; + u16 *b = g + crtc->gamma_size; + + for (i = 0; i < cmap->start; i++) { + lut[i].red = r[i]; + lut[i].green = g[i]; + lut[i].blue = b[i]; + } + for (i = cmap->start + cmap->len; i
Re: [PATCH v1] staging: rts5208: Change fixed function names with "%s: ", __func__
On Thu, Jul 13, 2017 at 09:30:20PM +0530, Gaurav Pathak wrote: > Adding public mailing list address and Joe. > > Hi Gerg, > > I am trying my best to avoid such silly mistakes, but this is first > time I am sending patch to a maintainer. Please disregard my > mistakes. > > I will send it to the correct mailing list, but when I ran > get_maintainer.pl on my patch your Name and email was displayed on > top. That's fine, but there was also another mailing list on that it showed as well, right? Please use that for patches for staging drivers. thanks, greg k-h
Re: [RFC PATCH 1/1] mm/mremap: add MREMAP_MIRROR flag for existing mirroring functionality
On Thu, Jul 13, 2017 at 09:01:54AM -0700, Mike Kravetz wrote: > Sent a patch (in separate e-mail thread) to return EINVAL for private > mappings. The way old_len == 0 behaves for MAP_PRIVATE seems more sane to me than the alternative of copying pagetables for anon pages (as behaving the way that way avoids to break anon pages invariants), despite it's not creating an exact mirror of what was in the original vma as it excludes any modification done to cowed anon pages. By nullifying move_page_tables old_len == 0 is simply duping the vma which is equivalent to a new mmap on the file for the MAP_PRIVATE case, it has a deterministic result. The real question is if it anybody is using it. So an alternative would be to start by adding a WARN_ON_ONCE deprecation warning instead of -EINVAL right away. The vma->vm_flags VM_ACCOUNT being wiped on the original vma as side effect of using the old_len == 0 trick looks like a bug, I guess it should get fixed if we intend to keep old_len and document it for the long term. Overall I'm more concerned about the fact an allocation failure in do_munmap is unreported to userland and it will leave the old vma intact like old_len == 0 would do (unless I'm misreading something there). The VM_ACCOUNT wipe as side effect of old_len == 0 is not major short term concern. Thanks, Andrea
Re: [RFC V2 4/6] cpufreq: Use transition_delay_us for legacy governors as well
On Thu, Jul 13, 2017 at 7:40 AM, Viresh Kumar wrote: > The policy->transition_delay_us field is used only by the schedutil > governor currently, and this field describes how fast the driver wants > the cpufreq governor to change CPUs frequency. It should rather be a > common thing across all governors, as it doesn't have any schedutil > dependency here. > > Create a new helper cpufreq_policy_transition_delay_us() to get the > transition delay across all governors. > > Signed-off-by: Viresh Kumar > --- > drivers/cpufreq/cpufreq_governor.c | 9 + > include/linux/cpufreq.h| 15 +++ > kernel/sched/cpufreq_schedutil.c | 11 +-- > 3 files changed, 17 insertions(+), 18 deletions(-) > > diff --git a/drivers/cpufreq/cpufreq_governor.c > b/drivers/cpufreq/cpufreq_governor.c > index 858081f9c3d7..eed069ecfd5e 100644 > --- a/drivers/cpufreq/cpufreq_governor.c > +++ b/drivers/cpufreq/cpufreq_governor.c > @@ -389,7 +389,6 @@ int cpufreq_dbs_governor_init(struct cpufreq_policy > *policy) > struct dbs_governor *gov = dbs_governor_of(policy); > struct dbs_data *dbs_data; > struct policy_dbs_info *policy_dbs; > - unsigned int latency; > int ret = 0; > > /* State should be equivalent to EXIT */ > @@ -428,13 +427,7 @@ int cpufreq_dbs_governor_init(struct cpufreq_policy > *policy) > if (ret) > goto free_policy_dbs_info; > > - /* policy latency is in ns. Convert it to us first */ > - latency = policy->cpuinfo.transition_latency / 1000; > - if (latency == 0) > - latency = 1; > - > - /* Bring kernel and HW constraints together */ > - dbs_data->sampling_rate = LATENCY_MULTIPLIER * latency; > + dbs_data->sampling_rate = cpufreq_policy_transition_delay_us(policy); > > if (!have_governor_per_policy()) > gov->gdbs_data = dbs_data; > diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h > index 00e4c40a3249..14f0ab61ed17 100644 > --- a/include/linux/cpufreq.h > +++ b/include/linux/cpufreq.h > @@ -532,6 +532,21 @@ static inline void cpufreq_policy_apply_limits(struct > cpufreq_policy *policy) > __cpufreq_driver_target(policy, policy->min, > CPUFREQ_RELATION_L); > } > > +static inline unsigned int > +cpufreq_policy_transition_delay_us(struct cpufreq_policy *policy) > +{ > + unsigned int delay_us = LATENCY_MULTIPLIER, latency; > + > + if (policy->transition_delay_us) > + return policy->transition_delay_us; > + > + latency = policy->cpuinfo.transition_latency / NSEC_PER_USEC; > + if (latency) > + delay_us *= latency; > + > + return delay_us; > +} Not in the header, please, and I don't think you need delay_us: latency = policy->cpuinfo.transition_latency / NSEC_PER_USEC; if (latency) return latency * LATENCY_MULTIPLIER; return LATENCY_MULTIPLIER; Thanks, Rafael
Re: [PATCH] libiscsi: Fix use-after-free race during iscsi_session_teardown
Hi Khazhismel, [auto build test WARNING on mkp-scsi/for-next] [also build test WARNING on v4.12 next-20170713] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Khazhismel-Kumykov/libiscsi-Fix-use-after-free-race-during-iscsi_session_teardown/20170713-231300 base: https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next config: xtensa-allyesconfig (attached as .config) compiler: xtensa-linux-gcc (GCC) 4.9.0 reproduce: wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=xtensa All warnings (new ones prefixed by >>): drivers//scsi/libiscsi.c: In function 'iscsi_session_teardown': >> drivers//scsi/libiscsi.c:2863:2: warning: passing argument 1 of >> 'iscsi_remove_session' from incompatible pointer type iscsi_remove_session(session); ^ In file included from drivers//scsi/libiscsi.c:41:0: include/scsi/scsi_transport_iscsi.h:435:13: note: expected 'struct iscsi_cls_session *' but argument is of type 'struct iscsi_session *' extern void iscsi_remove_session(struct iscsi_cls_session *session); ^ vim +/iscsi_remove_session +2863 drivers//scsi/libiscsi.c 2850 2851 /** 2852 * iscsi_session_teardown - destroy session, host, and cls_session 2853 * @cls_session: iscsi session 2854 */ 2855 void iscsi_session_teardown(struct iscsi_cls_session *cls_session) 2856 { 2857 struct iscsi_session *session = cls_session->dd_data; 2858 struct module *owner = cls_session->transport->owner; 2859 struct Scsi_Host *shost = session->host; 2860 2861 iscsi_pool_free(&session->cmdpool); 2862 > 2863 iscsi_remove_session(session); 2864 2865 kfree(session->password); 2866 kfree(session->password_in); 2867 kfree(session->username); 2868 kfree(session->username_in); 2869 kfree(session->targetname); 2870 kfree(session->targetalias); 2871 kfree(session->initiatorname); 2872 kfree(session->boot_root); 2873 kfree(session->boot_nic); 2874 kfree(session->boot_target); 2875 kfree(session->ifacename); 2876 kfree(session->portal_type); 2877 kfree(session->discovery_parent_type); 2878 2879 iscsi_free_session(cls_session); 2880 2881 iscsi_host_dec_session_cnt(shost); 2882 module_put(owner); 2883 } 2884 EXPORT_SYMBOL_GPL(iscsi_session_teardown); 2885 --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip
Re: [PATCH 1/3] mfd: Add support for FTDI FT232H devices
On Wed, 12 Jul 2017 10:50:03 +0200 Johan Hovold jo...@kernel.org wrote: ... >IIRC we should be able read from the EEPROM, and I would at least expect >there to be a way to retrieve the current mode as well. I've just send a patch for ftdi_sio. Thanks, Anatolij
[GIT PULL] Kbuild updates for v4.13 (2nd)
Hi Linus, This is a late Kbuild pull request for v4.13. I needed to rebase this after arch changes were pulled in. I think you can merge this cleanly. Please pull! The following changes since commit 2b976203417cf033079e0be30cae5f41d88e385e: Merge branch 'x86-urgent-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip (2017-07-09 11:21:31 -0700) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild.git tags/kbuild-v4.13-2 for you to fetch changes up to d7f14c66c273b00aaa626f419d3155773a88d460: kbuild: Enable Large File Support for hostprogs (2017-07-11 21:33:54 +0900) Kbuild updates for v4.13 (2nd) - Move generic-y of exported headers to uapi/asm/Kbuild for complete de-coupling of UAPI - Clean up scripts/Makefile.headersinst - Fix host programs for 32 bit machine with XFS file system Masahiro Yamada (28): kbuild: remove useless $(gen) variable in Makefile.headersinst kbuild: fix comment about dst of headers_{install, check}_all kbuild: pass dst= to Makefile.headersinst from top Makefile arm64: move generic-y of exported headers to uapi/asm/Kbuild ARM: move generic-y of exported headers to uapi/asm/Kbuild arc: move generic-y of exported headers to uapi/asm/Kbuild blackfin: move generic-y of exported headers to uapi/asm/Kbuild c6x: move generic-y of exported headers to uapi/asm/Kbuild cris: move generic-y of exported headers to uapi/asm/Kbuild h8300: move generic-y of exported headers to uapi/asm/Kbuild hexagon: move generic-y of exported headers to uapi/asm/Kbuild ia64: remove redundant generic-y += kvm_para.h from asm/Kbuild m32r: move generic-y of exported headers to uapi/asm/Kbuild m68k: move generic-y of exported headers to uapi/asm/Kbuild metag: move generic-y of exported headers to uapi/asm/Kbuild microblaze: move generic-y of exported headers to uapi/asm/Kbuild nios2: remove unneeded arch/nios2/include/(generated/)asm/signal.h nios2: move generic-y of exported headers to uapi/asm/Kbuild openrisc: move generic-y of exported headers to uapi/asm/Kbuild parisc: move generic-y of exported headers to uapi/asm/Kbuild sh: move generic-y of exported headers to uapi/asm/Kbuild sparc: move generic-y of exported headers to uapi/asm/Kbuild tile: move generic-y of exported headers to uapi/asm/Kbuild unicore32: move generic-y of exported headers to uapi/asm/Kbuild xtensa: move generic-y of exported headers to uapi/asm/Kbuild kbuild: do not include old-kbuild-file from Makefile.headersinst kbuild: split exported generic header creation into uapi-asm-generic kbuild: remove wrapper files handling from Makefile.headersinst Uwe Kleine-König (1): kbuild: Enable Large File Support for hostprogs Makefile| 26 +++- arch/arc/include/asm/Kbuild | 24 -- arch/arc/include/uapi/asm/Kbuild| 24 ++ arch/arm/include/asm/Kbuild | 16 --- arch/arm/include/uapi/asm/Kbuild| 14 + arch/arm64/include/asm/Kbuild | 17 arch/arm64/include/uapi/asm/Kbuild | 16 +++ arch/blackfin/include/asm/Kbuild| 24 +- arch/blackfin/include/uapi/asm/Kbuild | 22 arch/c6x/include/asm/Kbuild | 28 +- arch/c6x/include/uapi/asm/Kbuild| 25 +++ arch/cris/include/asm/Kbuild| 21 ++- arch/cris/include/uapi/asm/Kbuild | 17 arch/h8300/include/asm/Kbuild | 30 ++-- arch/h8300/include/uapi/asm/Kbuild | 26 arch/hexagon/include/asm/Kbuild | 24 +- arch/hexagon/include/uapi/asm/Kbuild| 22 arch/ia64/include/asm/Kbuild| 2 -- arch/m32r/include/asm/Kbuild| 4 +--- arch/m32r/include/uapi/asm/Kbuild | 3 ++- arch/m68k/include/asm/Kbuild| 13 +--- arch/m68k/include/uapi/asm/Kbuild | 10 ++ arch/metag/include/asm/Kbuild | 26 +--- arch/metag/include/uapi/asm/Kbuild | 24 ++ arch/microblaze/include/asm/Kbuild | 25 --- arch/microblaze/include/uapi/asm/Kbuild | 25 ++- arch/nios2/include/asm/Kbuild | 26 +--- arch/nios2/include/asm/signal.h | 22 arch/nios2/include/uapi/asm/Kbuild | 23 + arch/openrisc/include/asm/Kbuild| 29 +-- arc
[PULL] Documentation fixes
The following changes since commit 1cb566ba5634d7593b8b2a0a5c83f1c9e14b2e09: scripts/kernel-doc: handle DECLARE_HASHTABLE (2017-07-03 06:57:09 -0600) are available in the git repository at: git://git.lwn.net/linux.git tags/4.13-fixes for you to fetch changes up to 51e988f4092428e3d2c9f141fba9f86583bc82f3: kokr/memory-barriers.txt: Fix obsolete link to atomic_ops.txt (2017-07-12 16:56:40 -0600) A set of fixes for various warnings, including the one caused by the removal of kernel/rcu/srcu.c. Also correct a stray pointer in memory-barriers.txt. Jonathan Corbet (3): docs: Do not include from kernel/rcu/srcu.c docs: Include uaccess docs from the right file docs: Turn off section numbering for the input docs SeongJae Park (2): memory-barriers.txt: Fix broken link to atomic_ops.txt kokr/memory-barriers.txt: Fix obsolete link to atomic_ops.txt Documentation/core-api/kernel-api.rst| 2 +- Documentation/driver-api/basics.rst | 3 --- Documentation/input/index.rst| 1 - Documentation/memory-barriers.txt| 6 +++--- Documentation/translations/ko_KR/memory-barriers.txt | 14 +++--- 5 files changed, 11 insertions(+), 15 deletions(-)
Re: [PATCH] nvmet: preserve controller serial number between reboots
It seems weird that a subsystem has a serial. But that's actually how NVMe defines them. Where is that wording? Which mean we first need to fix our code to generate a serial number per subsystem, and on top of that the patch from Johannes seems perfectly reasonable. If that is indeed the case then sure.
Re: [PATCH v2] xattr: Enable security.capability in user namespaces
On Thu, Jul 13, 2017 at 07:11:36AM -0500, Eric W. Biederman wrote: > The concise summary: > > Today we have the xattr security.capable that holds a set of > capabilities that an application gains when executed. AKA setuid root exec > without actually being setuid root. > > User namespaces have the concept of capabilities that are not global but > are limited to their user namespace. We do not currently have > filesystem support for this concept. So correct me if I am wrong; in general, there will only be one variant of the form: security.foo@uid=15000 It's not like there will be: security.foo@uid=1000 security.foo@uid=2000 Except if you have an Distribution root directory which is shared by many containers, you would need to put the xattrs in the overlay inodes. Worse, each time you launch a new container, with a new subuid allocation, you will have to iterate over all files with capabilities and do a copy-up operations on the xattrs in overlayfs. So that's actually a bit of a disaster. So for distribution overlays, you will need to do things a different way, which is to map the distro subdirectory so you know that the capability with the global uid 0 should be used for the container "root" uid, right? So this hack of using security.foo@uid=1000 is *only* useful when the subcontainer root wants to create the privileged executable. You still have to do things the other way. So can we make perhaps the assertion that *either*: security.foo exists, *or* security.foo@uid=BAR exists, but never both? And there BAR is exclusive to only one instances? Otherwise, I suspect that the architecture is going to turn around and bite us in the *ss eventually, because someone will want to do something crazy and the solution will not be scalable. -Ted
Re: [PATCH V4] PCI: handle CRS returned by device after FLR
On 7/13/2017 12:29 PM, Keith Busch wrote: >> When used, DRS and FRS allow an improved behavior over the CRS mechanism, >> and eliminate >> its associated periodic polling time of up to 1 second following a reset." > That wording is just confusing. It looks to me the 1 second polling is > to be used following a reset if CRS is not implemented. > > > https://pcisig.com/sites/default/files/specification_documents/ECN_RN_29_Aug_2013.pdf > > " > Through the mechanisms defined by this ECR, we can avoid the long, > architected, fixed delays following various forms of reset before > software is permitted to perform its first Configuration Request. These > delays are very large: > > 1 second if Configuration Retry Status (CRS) is not used > " > > It goes on to say CRS is usually much lower, but doesn't specify an > upper bound either. > I see, we got caught on spec language where we don't know what 'its' is. Bjorn, Since there is no upper cap on how long, what is your preference (stick to 60), give incremental warning updates every 5 seconds? I can certainly rewrite the commit message. Sinan -- Sinan Kaya Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
Re: [PATCH 4.9 00/25] 4.9.38-stable review
Hi Greg, On 13 July 2017 at 21:10, Greg Kroah-Hartman wrote: > This is the start of the stable review cycle for the 4.9.38 release. > There are 25 patches in this series, all will be posted as a response > to this one. If anyone has any issues with these being applied, please > let me know. > > Responses should be made by Sat Jul 15 15:39:46 UTC 2017. > Anything received after that time might be too late. > > The whole patch series can be found in one patch at: > kernel.org/pub/linux/kernel/v4.x/stable-review/patch-4.9.38-rc1.gz > or in the git tree and branch at: > git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git > linux-4.9.y > and the diffstat can be found below. > For arm64, Build and boot-tested with arm64 defconfig on hikey - no regressions noted. > thanks, > > greg k-h >
Re: [PATCHv3 3/3] ARM: drm: Intel FPGA VIP Frame Buffer II drm driver
Hi Hean, [auto build test WARNING on drm/drm-next] [also build test WARNING on v4.12 next-20170713] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/hean-loong-ong-intel-com/Intel-FPGA-VIP-Frame-Buffer-II-drm-driver/20170713-235417 base: git://people.freedesktop.org/~airlied/linux.git drm-next config: ia64-allmodconfig (attached as .config) compiler: ia64-linux-gcc (GCC) 6.2.0 reproduce: wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=ia64 All warnings (new ones prefixed by >>): In file included from include/linux/cdev.h:7:0, from include/drm/drmP.h:36, from drivers/gpu//drm/ivip/intel_vip_core.c:28: drivers/gpu//drm/ivip/intel_vip_core.c: In function 'intelvipfb_enable': >> drivers/gpu//drm/ivip/intel_vip_core.c:57:33: warning: format '%x' expects >> argument of type 'unsigned int', but argument 3 has type 'dma_addr_t {aka >> long long unsigned int}' [-Wformat=] dev_info(pipe->plane.dev->dev, "Address 0x%x\n", addr); ^ include/linux/device.h:1317:51: note: in definition of macro 'dev_info' #define dev_info(dev, fmt, arg...) _dev_info(dev, fmt, ##arg) ^~~ vim +57 drivers/gpu//drm/ivip/intel_vip_core.c 27 > 28 #include 29 #include 30 #include 31 #include 32 #include 33 #include 34 #include 35 #include 36 #include 37 38 #include "intel_vip_drv.h" 39 40 static void intelvipfb_enable(struct drm_simple_display_pipe *pipe, 41 struct drm_crtc_state *crtc_state) 42 { 43 /* 44 * The frameinfo variable has to correspond to the size of the VIP Suite 45 * Frame Reader register 7 which will determine the maximum size used 46 * in this frameinfo 47 */ 48 49 u32 frameinfo; 50 struct intelvipfb_priv *priv = pipe->plane.dev->dev_private; 51 void __iomem *base = priv->base; 52 struct drm_plane_state *state = pipe->plane.state; 53 dma_addr_t addr; 54 55 addr = drm_fb_cma_get_gem_addr(state->fb, state, 0); 56 > 57 dev_info(pipe->plane.dev->dev, "Address 0x%x\n", addr); 58 59 frameinfo = 60 readl(base + INTELVIPFB_FRAME_READER) & 0x00ff; 61 writel(frameinfo, base + INTELVIPFB_FRAME_INFO); 62 writel(addr, base + INTELVIPFB_FRAME_START); 63 /* Finally set the control register to 1 to start streaming */ 64 writel(1, base + INTELVIPFB_CONTROL); 65 } 66 --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip
Re: [PATCH 4/4] perf annotate browser: Support the toggle number of samples
On 07/13/2017 05:16 AM, Arnaldo Carvalho de Melo wrote: Em Wed, Jul 12, 2017 at 07:14:19AM +0900, Taeung Song escreveu: Cc: Namhyung Kim Cc: Milian Wolff Cc: Jiri Olsa Signed-off-by: Taeung Song Humm, this is cycling thru different things to show on a column, I think 'toggle' would best be applied to showing or not some set of columns, i.e. if you 'toggle' "number of samples" we would see or not a column with that, if while showing the "number of samples" column we toggle a previously hidden "period" column, then we would end up with two columns, etc, got it? - Arnaldo Understood! NG case: the total period view <-(toggle)-> the number of samples view OK case: the percentage view <-(toggle)-> the number of samples view OK case: the percentage view <-(toggle)-> the total period view I'll fix it! Thanks, Taeung --- tools/perf/ui/browsers/annotate.c | 17 - 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c index 0ddc3b2..237903d 100644 --- a/tools/perf/ui/browsers/annotate.c +++ b/tools/perf/ui/browsers/annotate.c @@ -42,6 +42,7 @@ static struct annotate_browser_opt { jump_arrows, show_linenr, show_nr_jumps, +show_nr_samples, show_total_period; } annotate_browser__opts = { .use_offset = true, @@ -157,6 +158,9 @@ static void annotate_browser__write(struct ui_browser *browser, void *entry, int if (annotate_browser__opts.show_total_period) { ui_browser__printf(browser, "%10" PRIu64 " ", bdl->samples[i].period); + } else if (annotate_browser__opts.show_nr_samples) { + ui_browser__printf(browser, "%6" PRIu64 " ", + bdl->samples[i].nr); } else { ui_browser__printf(browser, "%6.2f ", bdl->samples[i].percent); @@ -170,6 +174,8 @@ static void annotate_browser__write(struct ui_browser *browser, void *entry, int else { if (annotate_browser__opts.show_total_period) ui_browser__printf(browser, "%*s", 11, "Event count"); + else if (annotate_browser__opts.show_nr_samples) + ui_browser__printf(browser, "%*s", 7, "Samples"); else ui_browser__printf(browser, "%*s", 7, "Percent"); } @@ -836,6 +842,7 @@ static int annotate_browser__run(struct annotate_browser *browser, "o Toggle disassembler output/simplified view\n" "s Toggle source code view\n" "t Toggle total period view\n" + "e Toggle number of samples\n" "/ Search string\n" "k Toggle line numbers\n" "r Run available scripts\n" @@ -916,6 +923,11 @@ static int annotate_browser__run(struct annotate_browser *browser, !annotate_browser__opts.show_total_period; annotate_browser__update_addr_width(browser); continue; + case 'e': + annotate_browser__opts.show_nr_samples = + !annotate_browser__opts.show_nr_samples; + annotate_browser__update_addr_width(browser); + continue; case K_LEFT: case K_ESC: case 'q': @@ -936,9 +948,11 @@ static int annotate_browser__run(struct annotate_browser *browser, int map_symbol__tui_annotate(struct map_symbol *ms, struct perf_evsel *evsel, struct hist_browser_timer *hbt) { - /* Set default value for show_total_period. */ + /* Set default value for show_total_period and show_nr_samples */ annotate_browser__opts.show_total_period = symbol_conf.show_total_period; + annotate_browser__opts.show_nr_samples = + symbol_conf.show_nr_samples; return symbol__tui_annotate(ms->sym, ms->map, evsel, hbt); } @@ -1189,6 +1203,7 @@ static struct annotate_config { ANNOTATE_CFG(jump_arrows), ANNOTATE_CFG(show_linenr), ANNOTATE_CFG(show_nr_jumps), + ANNOTATE_CFG(show_nr_samples), ANNOTATE_CFG(show_total_period), ANNOTATE_CFG(use_offset), }; -- 2.7.4
[PATCH] scsi: qedf: Limit number of CQs
From: Thomas Bogendoerfer FCOE offloading on qedf devices fails with: [qed_sp_fcoe_func_start:150(sp-0-3b:00.02)]Cannot satisfy CQ amount. CQs requested 8, CQs available 6. Aborting function start [qed_fcoe_start:821()]Failed to start fcoe [__qedf_probe:3041]:6: Cannot start FCoE function. The reason is a newly introduced check in the qed main part. This change also provides the information about how many CQs are available, so we simply limit the number of requested CQs. Fixes: 3c5da9427802 qed: Share additional information with qedf Signed-off-by: Thomas Bogendoerfer --- drivers/scsi/qedf/qedf_main.c | 16 +--- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/drivers/scsi/qedf/qedf_main.c b/drivers/scsi/qedf/qedf_main.c index b58bba4604e8..5778218734fa 100644 --- a/drivers/scsi/qedf/qedf_main.c +++ b/drivers/scsi/qedf/qedf_main.c @@ -2765,6 +2765,8 @@ static int qedf_set_fcoe_pf_param(struct qedf_ctx *qedf) */ qedf->num_queues = min((unsigned int)QEDF_MAX_NUM_CQS, num_online_cpus()); + /* limit to acutal available CQs */ + qedf->num_queues = min(qedf->num_queues, qedf->dev_info.num_cqs); QEDF_INFO(&(qedf->dbg_ctx), QEDF_LOG_DISC, "Number of CQs is %d.\n", qedf->num_queues); @@ -2962,6 +2964,13 @@ static int __qedf_probe(struct pci_dev *pdev, int mode) goto err1; } + /* Learn information crucial for qedf to progress */ + rc = qed_ops->fill_dev_info(qedf->cdev, &qedf->dev_info); + if (rc) { + QEDF_ERR(&(qedf->dbg_ctx), "Failed to dev info.\n"); + goto err1; + } + /* queue allocation code should come here * order should be * slowpath_start @@ -2977,13 +2986,6 @@ static int __qedf_probe(struct pci_dev *pdev, int mode) } qed_ops->common->update_pf_params(qedf->cdev, &qedf->pf_params); - /* Learn information crucial for qedf to progress */ - rc = qed_ops->fill_dev_info(qedf->cdev, &qedf->dev_info); - if (rc) { - QEDF_ERR(&(qedf->dbg_ctx), "Failed to dev info.\n"); - goto err1; - } - /* Record BDQ producer doorbell addresses */ qedf->bdq_primary_prod = qedf->dev_info.primary_dbq_rq_addr; qedf->bdq_secondary_prod = qedf->dev_info.secondary_bdq_rq_addr; -- 2.12.3
Re: [RFC v5 34/38] procfs: display the protection-key number associated with a vma
On Thu, Jul 13, 2017 at 07:07:48AM -0700, Dave Hansen wrote: > On 07/13/2017 01:03 AM, Ram Pai wrote: > > On Tue, Jul 11, 2017 at 11:13:56AM -0700, Dave Hansen wrote: > >> On 07/05/2017 02:22 PM, Ram Pai wrote: > >>> +#ifdef CONFIG_PPC64_MEMORY_PROTECTION_KEYS > >>> +void arch_show_smap(struct seq_file *m, struct vm_area_struct *vma) > >>> +{ > >>> + seq_printf(m, "ProtectionKey: %8u\n", vma_pkey(vma)); > >>> +} > >>> +#endif /* CONFIG_PPC64_MEMORY_PROTECTION_KEYS */ > >> > >> This seems like kinda silly unnecessary duplication. Could we just put > >> this in the fs/proc/ code and #ifdef it on ARCH_HAS_PKEYS? > > > > Well x86 predicates it based on availability of X86_FEATURE_OSPKE. > > > > powerpc doesn't need that check or any similar check. So trying to > > generalize the code does not save much IMHO. > > I know all your hardware doesn't support it. :) Wow! you bring a good point which I had not considered yet. I need some runtime checks for RPT. But regardless, my above statement is still partially true. x86 predicates it based on availability of X86_FEATURE_OSPKE, and powerpc should predicate it based on HPT. So we have our own customized checks. Hence a unified function won't suffice. > > So, for instance, if you are running on a new POWER9 with radix page > tables, you will just always output "ProtectionKey: 0" in every VMA, > regardless? > > > maybe have a seperate inline function that does > > seq_printf(m, "ProtectionKey: %8u\n", vma_pkey(vma)); > > and is called from x86 and powerpc's arch_show_smap()? > > At least will keep the string format captured in > > one single place. > > Now that we have two architectures, is there a strong reason we can't > just have an arch_pkeys_enabled(), and stick the seq_printf() back in > generic code? correct. that looks like the correct approach. Was trying to avoid touching arch neutral code. But this approach will force me do so. Will do. -- Ram Pai
Re: [PATCH v2] xattr: Enable security.capability in user namespaces
On 07/13/2017 12:40 PM, Theodore Ts'o wrote: On Thu, Jul 13, 2017 at 07:11:36AM -0500, Eric W. Biederman wrote: The concise summary: Today we have the xattr security.capable that holds a set of capabilities that an application gains when executed. AKA setuid root exec without actually being setuid root. User namespaces have the concept of capabilities that are not global but are limited to their user namespace. We do not currently have filesystem support for this concept. So correct me if I am wrong; in general, there will only be one variant of the form: security.foo@uid=15000 It's not like there will be: security.foo@uid=1000 security.foo@uid=2000 A file shared by 2 containers, one mapping root to uid=1000, the other mapping root to uid=2000, will show these two xattrs on the host (init_user_ns) once these containers set xattrs on that file. Except if you have an Distribution root directory which is shared by many containers, you would need to put the xattrs in the overlay inodes. Worse, each time you launch a new container, with a new subuid allocation, you will have to iterate over all files with capabilities and do a copy-up operations on the xattrs in overlayfs. So that's actually a bit of a disaster. Note that we do keep compatibility to existing behavior. The security.foo of the host is visible inside any container for as long as the container root user doesn't set its own security.foo on that file, which then hides it. Does that address this concern? So for distribution overlays, you will need to do things a different way, which is to map the distro subdirectory so you know that the capability with the global uid 0 should be used for the container "root" uid, right? So this hack of using security.foo@uid=1000 is *only* useful when the subcontainer root wants to create the privileged executable. You still have to do things the other way. So can we make perhaps the assertion that *either*: security.foo exists, *or* security.foo@uid=BAR exists, but never both? And there BAR is exclusive to only one instances? In the current implementation BAR is visible inside of any instance that 'covers' this uid with the mapping range. Above example of security.foo@uid=1000 appears as security.foo inside the container with root mapping to uid 1000 (@uid=0 is suppressed) but also appears as security.foo@uid=100 with root uid mapping to 900 (and range of at least 101). Otherwise, I suspect that the architecture is going to turn around and bite us in the *ss eventually, because someone will want to do something crazy and the solution will not be scalable. Can you define what 'scalable' means for you in this context? From what I can see sharing a filesystem between multiple containers doesn't 'scale well' for virtualizing the xattrs primarily because of size limitations of xattrs per file. Stefan -Ted
Re: [PATCH] lustre: check copy_from_iter/copy_to_iter return code
> We now get a helpful warning for code that calls copy_{from,to}_iter > without checking the return value, introduced by commit aa28de275a24 > ("iov_iter/hardening: move object size checks to inlined part"). > > drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c: In function > 'kiblnd_send': > drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c:1643:2: error: > ignoring return value of 'copy_from_iter', declared with attribute > warn_unused_result [-Werror=unused-result] > drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c: In function > 'kiblnd_recv': > drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c:1744:3: error: > ignoring return value of 'copy_to_iter', declared with attribute > warn_unused_result [-Werror=unused-result] > > In case we get short copies here, we may get incorrect behavior. > I've added failure handling for both rx and tx now, returning > -EFAULT as expected. > > Cc: sta...@vger.kernel.org > Signed-off-by: Arnd Bergmann > --- > This warning now shows up in 'allmodconfig' builds, so it would be > good to get it fixed quickly for 4.13, but my patch should not go > in without careful review since I'm not familiar with with code > and the error handling is a bit tricky here. > > I added 'Cc: stable' since this is a preexisting condition that we > only started warning about now. > --- > drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c | 15 +-- > 1 file changed, 13 insertions(+), 2 deletions(-) > > diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c > b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c > index 85b242ec5f9b..70256a0ffd2e 100644 > --- a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c > +++ b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c > @@ -1640,8 +1640,13 @@ kiblnd_send(struct lnet_ni *ni, void *private, struct > lnet_msg *lntmsg) > ibmsg = tx->tx_msg; > ibmsg->ibm_u.immediate.ibim_hdr = *hdr; > > - copy_from_iter(&ibmsg->ibm_u.immediate.ibim_payload, IBLND_MSG_SIZE, > + rc = copy_from_iter(&ibmsg->ibm_u.immediate.ibim_payload, > IBLND_MSG_SIZE, > &from); I have to Nak this to prevent it from landing but for some reason in my testing its returning zero from copy_from_iter. > + if (rc != IBLND_MSG_SIZE) { > + kiblnd_pool_free_node(&tx->tx_pool->tpo_pool, &tx->tx_list); > + return -EFAULT; > + } > + > nob = offsetof(struct kib_immediate_msg, ibim_payload[payload_nob]); > kiblnd_init_tx_msg(ni, tx, IBLND_MSG_IMMEDIATE, nob); > > @@ -1741,8 +1746,14 @@ kiblnd_recv(struct lnet_ni *ni, void *private, struct > lnet_msg *lntmsg, > break; > } > > - copy_to_iter(&rxmsg->ibm_u.immediate.ibim_payload, > + rc = copy_to_iter(&rxmsg->ibm_u.immediate.ibim_payload, >IBLND_MSG_SIZE, to); > + if (rc != IBLND_MSG_SIZE) { > + rc = -EFAULT; > + break; > + } > + > + rc = 0; > lnet_finalize(ni, lntmsg, 0); > break; > > -- > 2.9.0 > >
Re: [PATCH v4 3/3] KVM: nVMX: Emulate EPTP switching for the L1 hypervisor
David Hildenbrand writes: + /* + * If the (L2) guest does a vmfunc to the currently + * active ept pointer, we don't have to do anything else + */ + if (vmcs12->ept_pointer != address) { + if (address >> cpuid_maxphyaddr(vcpu) || + !IS_ALIGNED(address, 4096)) >>> >>> Couldn't the pfn still be invalid and make kvm_mmu_reload() fail? >>> (triggering a KVM_REQ_TRIPLE_FAULT) >> >> If there's a triple fault, I think it's a good idea to inject it >> back. Basically, there's no need to take care of damage control >> that L1 is intentionally doing. > > I quickly rushed over the massive amount of comments. Sounds like you'll > be preparing a v5. Would be great if you could add some comments that > were the result of this discussion (for parts that are not that obvious > - triple faults) - thanks! Will do. Basically, we agreed that we don't need to do anything with mmu_reload() faillures because the invalid eptp that mmu_unload will write to root_hpa will result in an ept violation. Bandan
Re: [PATCH] drm/udl: Make page_flip asynchronous
On Thu, Jul 13, 2017 at 6:25 PM, Stéphane Marchesin wrote: >> Can't we roll this driver over to the atomic helpers instead? There >> you get nonblocking pretty much for free ... I'm not sure extending >> the old modeset code has all that much benefit really. > > This code certainly has value by itself; it makes the driver more > efficient. I think the best can sometimes be the enemy of the good -- > this code is here and written, but I don't think any of us is going to > tackle atomic for udl. Sure, I guess I should have clarified this with "If you want me to review and merge this, then pls look into atomic, since that seems actually beneficial for my own interest". I'm not paid by intel to review driver patches at random, but to keep overall drm in nice shape. Moving drivers to atomic and using more shared infrastructure is in that interest, reviewing random driver patches isn't. Sorry for not making clear, I kinda have that as my implicit context. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
Re: [PATCH 3/4] perf annotate: Support --show-nr-samples option
On 07/13/2017 05:13 AM, Arnaldo Carvalho de Melo wrote: Em Wed, Jul 12, 2017 at 07:14:14AM +0900, Taeung Song escreveu: Add --show-nr-samples option to perf-annotate so that it corresponds with perf-report. Additionally h->sum is properly renamed h->nr_samples. Please first do a patch renaming ->sum to ->nr_samples, then a patch supportign --show-nr-samples. We can have this new --show-nr-samples option, but perhaps the best way would be to have a hotkey (I guess we have for period, right) for this? You mean we need to add a hotkey for --show-nr-samples on the anntoate TUI browser, right ? I added a 'e' hotkey in other patch, not this patch Change it ? At some point would be good to have some 'S' or other hotkey to save the > current set of output modifiers (nr-samples column, period column, etc) in the .perfconfig file. I don't understand what you said.. Is related a hotkey with things in .perfconfig file ? I thought hotkeys exist on only annotate TUI browser.. You mean the short option for --show-nr-samples ? Thanks, Taeung - Arnaldo SNIP
Re: [PATCH] nvmet: preserve controller serial number between reboots
On Thu, Jul 13, 2017 at 07:38:27PM +0300, Sagi Grimberg wrote: > >>> It seems weird that a subsystem has a serial. >> >> But that's actually how NVMe defines them. > > Where is that wording? Right there in the SN field description: "Serial Number (SN): Contains the serial number for the NVM subsystem that is assigned by the vendor as an ASCII string. Refer to section 7.10 for unique identifier requirements. Refer to section 1.5 for ASCII string requirements"
Re: [PATCH V4] PCI: handle CRS returned by device after FLR
On Thu, Jul 13, 2017 at 12:42:44PM -0400, Sinan Kaya wrote: > On 7/13/2017 12:29 PM, Keith Busch wrote: > > That wording is just confusing. It looks to me the 1 second polling is > > to be used following a reset if CRS is not implemented. > > > > > > https://pcisig.com/sites/default/files/specification_documents/ECN_RN_29_Aug_2013.pdf > > > > " > > Through the mechanisms defined by this ECR, we can avoid the long, > > architected, fixed delays following various forms of reset before > > software is permitted to perform its first Configuration Request. These > > delays are very large: > > > > 1 second if Configuration Retry Status (CRS) is not used > > " > > > > It goes on to say CRS is usually much lower, but doesn't specify an > > upper bound either. > > > > I see, we got caught on spec language where we don't know what 'its' is. Well, I don't know for certain if your original interpretation is incorrect. Just saying the CRS intention doesn't explicitly stand out to me. On a side note, I'll also see if I can get clarification on what expectations the hardware people have for this particular product. Your observation seems a little high to me, but I don't know if that's outside the product's limits.
Re: [PATCH v5 1/2] clk: gate: expose clk_gate_ops::is_enabled
On 07/13/2017 07:02 AM, gabriel.fernan...@st.com wrote: > From: Gabriel Fernandez > > This patch exposes clk_gate_ops::is_enabled as functions > that can be directly called and assigned in places like this so > we don't need wrapper functions that do nothing besides forward > the call. > > Signed-off-by: Gabriel Fernandez > Sugested by Stephen Boyd > --- > drivers/clk/clk-gate.c | 2 +- > include/linux/clk-provider.h | 1 + > 2 files changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/clk/clk-gate.c b/drivers/clk/clk-gate.c > index 4e0c054a..e27e28f 100644 > --- a/drivers/clk/clk-gate.c > +++ b/drivers/clk/clk-gate.c > @@ -86,7 +86,7 @@ static void clk_gate_disable(struct clk_hw *hw) > clk_gate_endisable(hw, 0); > } > > -static int clk_gate_is_enabled(struct clk_hw *hw) > +int clk_gate_is_enabled(struct clk_hw *hw) > { > u32 reg; > struct clk_gate *gate = to_clk_gate(hw); Don't you need to add an EXPORT_SYMBOL_GPL(clk_gate_is_enabled) as well in case this gets used by modules? > diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h > index c59c625..e9587ab 100644 > --- a/include/linux/clk-provider.h > +++ b/include/linux/clk-provider.h > @@ -343,6 +343,7 @@ struct clk_hw *clk_hw_register_gate(struct device *dev, > const char *name, > u8 clk_gate_flags, spinlock_t *lock); > void clk_unregister_gate(struct clk *clk); > void clk_hw_unregister_gate(struct clk_hw *hw); > +int clk_gate_is_enabled(struct clk_hw *hw); > > struct clk_div_table { > unsigned intval; > -- Florian
Re: [PATCH v2] xattr: Enable security.capability in user namespaces
Theodore Ts'o writes: > On Thu, Jul 13, 2017 at 07:11:36AM -0500, Eric W. Biederman wrote: >> The concise summary: >> >> Today we have the xattr security.capable that holds a set of >> capabilities that an application gains when executed. AKA setuid root exec >> without actually being setuid root. >> >> User namespaces have the concept of capabilities that are not global but >> are limited to their user namespace. We do not currently have >> filesystem support for this concept. > > So correct me if I am wrong; in general, there will only be one > variant of the form: > >security.foo@uid=15000 > > It's not like there will be: > >security.foo@uid=1000 >security.foo@uid=2000 > > Except if you have an Distribution root directory which is shared > by many containers, you would need to put the xattrs in the overlay > inodes. Worse, each time you launch a new container, with a new > subuid allocation, you will have to iterate over all files with > capabilities and do a copy-up operations on the xattrs in overlayfs. > So that's actually a bit of a disaster. > > So for distribution overlays, you will need to do things a different > way, which is to map the distro subdirectory so you know that the > capability with the global uid 0 should be used for the container > "root" uid, right? > > So this hack of using security.foo@uid=1000 is *only* useful when the > subcontainer root wants to create the privileged executable. You > still have to do things the other way. > > So can we make perhaps the assertion that *either*: > >security.foo > > exists, *or* > >security.foo@uid=BAR > > exists, but never both? And there BAR is exclusive to only one > instances? > > Otherwise, I suspect that the architecture is going to turn around and > bite us in the *ss eventually, because someone will want to do > something crazy and the solution will not be scalable. Yep. That is what it looks like from here. Which is why I asked the question about scalability of the xattr implementations. It looks like trying to accomodate the general case just gets us in trouble, and sets unrealistic expectations. Which strongly suggests that Serge's previous version that just reved the format of security.capable so that a uid field could be added is likely to be the better approach. I want to see what Serge and Stefan have to say but the case looks pretty clear cut at the moment. Eric
Re: [PATCH v2] xattr: Enable security.capability in user namespaces
On 07/13/2017 01:14 PM, Eric W. Biederman wrote: Theodore Ts'o writes: On Thu, Jul 13, 2017 at 07:11:36AM -0500, Eric W. Biederman wrote: The concise summary: Today we have the xattr security.capable that holds a set of capabilities that an application gains when executed. AKA setuid root exec without actually being setuid root. User namespaces have the concept of capabilities that are not global but are limited to their user namespace. We do not currently have filesystem support for this concept. So correct me if I am wrong; in general, there will only be one variant of the form: security.foo@uid=15000 It's not like there will be: security.foo@uid=1000 security.foo@uid=2000 Except if you have an Distribution root directory which is shared by many containers, you would need to put the xattrs in the overlay inodes. Worse, each time you launch a new container, with a new subuid allocation, you will have to iterate over all files with capabilities and do a copy-up operations on the xattrs in overlayfs. So that's actually a bit of a disaster. So for distribution overlays, you will need to do things a different way, which is to map the distro subdirectory so you know that the capability with the global uid 0 should be used for the container "root" uid, right? So this hack of using security.foo@uid=1000 is *only* useful when the subcontainer root wants to create the privileged executable. You still have to do things the other way. So can we make perhaps the assertion that *either*: security.foo exists, *or* security.foo@uid=BAR exists, but never both? And there BAR is exclusive to only one instances? Otherwise, I suspect that the architecture is going to turn around and bite us in the *ss eventually, because someone will want to do something crazy and the solution will not be scalable. Yep. That is what it looks like from here. Which is why I asked the question about scalability of the xattr implementations. It looks like trying to accomodate the general case just gets us in trouble, and sets unrealistic expectations. Which strongly suggests that Serge's previous version that just reved the format of security.capable so that a uid field could be added is likely to be the better approach. I want to see what Serge and Stefan have to say but the case looks pretty clear cut at the moment. The approach of virtualizing the xattrs on the name-side, which is what this patch does, provides a more general approach than to virtualizing it on the value side, which is what Serge does in his other patch for security.capability alone. With the virtualizing on-the-value side virtualizing the xattr becomes an exercise that needs to be repeated for every xattr name that one would want to virtualize. With this patch you would just add another xattr name to a list, a one-line patch in the end. Xattr with prefixes like trusted.* need a bit more work but this can be woven in as well (https://github.com/stefanberger/linux/commit/397b1a3b24045c67405fc83465e544fc865d402f). For virtualizing the xattrs on the 'value' side I was looking for whether there's something like a 'wrapper' structure around the actual value of the xattr so that that wrapper could be extended to support different values at different uids and applied to any xattr. Unfortunately there's no such 'wrapper'. Stefan Eric
[PATCH 6/6] perf/x86/uncore: fix missing marker for skx_uncore_cha_extra_regs
From: Stephane Eranian This skx_uncore_cha_extra_regs array was missing an end-marker. Signed-off-by: Stephane Eranian Signed-off-by: Kan Liang --- arch/x86/events/intel/uncore_snbep.c | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/x86/events/intel/uncore_snbep.c b/arch/x86/events/intel/uncore_snbep.c index f9f825b..4f91276 100644 --- a/arch/x86/events/intel/uncore_snbep.c +++ b/arch/x86/events/intel/uncore_snbep.c @@ -3334,6 +3334,7 @@ static struct extra_reg skx_uncore_cha_extra_regs[] = { SNBEP_CBO_EVENT_EXTRA_REG(0x9134, 0x, 0x4), SNBEP_CBO_EVENT_EXTRA_REG(0x35, 0xff, 0x8), SNBEP_CBO_EVENT_EXTRA_REG(0x36, 0xff, 0x8), + EVENT_EXTRA_END }; static u64 skx_cha_filter_mask(int fields) -- 2.7.4
[PATCH 4/6] perf/x86/uncore: remove invalid Skylake server CHA filter field
From: Kan Liang There is no field c6 and link for CHA BOX FILTER. Signed-off-by: Kan Liang --- arch/x86/events/intel/uncore_snbep.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/arch/x86/events/intel/uncore_snbep.c b/arch/x86/events/intel/uncore_snbep.c index a30bf97..2401d06 100644 --- a/arch/x86/events/intel/uncore_snbep.c +++ b/arch/x86/events/intel/uncore_snbep.c @@ -351,7 +351,6 @@ DEFINE_UNCORE_FORMAT_ATTR(filter_cid, filter_cid, "config1:5"); DEFINE_UNCORE_FORMAT_ATTR(filter_link, filter_link, "config1:5-8"); DEFINE_UNCORE_FORMAT_ATTR(filter_link2, filter_link, "config1:6-8"); DEFINE_UNCORE_FORMAT_ATTR(filter_link3, filter_link, "config1:12"); -DEFINE_UNCORE_FORMAT_ATTR(filter_link4, filter_link, "config1:9-12"); DEFINE_UNCORE_FORMAT_ATTR(filter_nid, filter_nid, "config1:10-17"); DEFINE_UNCORE_FORMAT_ATTR(filter_nid2, filter_nid, "config1:32-47"); DEFINE_UNCORE_FORMAT_ATTR(filter_state, filter_state, "config1:18-22"); @@ -3302,7 +3301,6 @@ static struct attribute *skx_uncore_cha_formats_attr[] = { &format_attr_inv.attr, &format_attr_thresh8.attr, &format_attr_filter_tid4.attr, - &format_attr_filter_link4.attr, &format_attr_filter_state5.attr, &format_attr_filter_rem.attr, &format_attr_filter_loc.attr, @@ -3312,7 +3310,6 @@ static struct attribute *skx_uncore_cha_formats_attr[] = { &format_attr_filter_opc_0.attr, &format_attr_filter_opc_1.attr, &format_attr_filter_nc.attr, - &format_attr_filter_c6.attr, &format_attr_filter_isoc.attr, NULL, }; -- 2.7.4
[PATCH 3/6] perf/x86/uncore: fix Skylake server CHA LLC_LOOKUP event umask
From: Kan Liang Correct the umask for LLC_LOOKUP.LOCAL and LLC_LOOKUP.REMOTE events Signed-off-by: Kan Liang --- arch/x86/events/intel/uncore_snbep.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/x86/events/intel/uncore_snbep.c b/arch/x86/events/intel/uncore_snbep.c index fbf8f6e..a30bf97 100644 --- a/arch/x86/events/intel/uncore_snbep.c +++ b/arch/x86/events/intel/uncore_snbep.c @@ -,8 +,8 @@ static struct extra_reg skx_uncore_cha_extra_regs[] = { SNBEP_CBO_EVENT_EXTRA_REG(0x0534, 0x, 0x4), SNBEP_CBO_EVENT_EXTRA_REG(0x0934, 0x, 0x4), SNBEP_CBO_EVENT_EXTRA_REG(0x1134, 0x, 0x4), - SNBEP_CBO_EVENT_EXTRA_REG(0x2134, 0x, 0x4), - SNBEP_CBO_EVENT_EXTRA_REG(0x8134, 0x, 0x4), + SNBEP_CBO_EVENT_EXTRA_REG(0x3134, 0x, 0x4), + SNBEP_CBO_EVENT_EXTRA_REG(0x9134, 0x, 0x4), }; static u64 skx_cha_filter_mask(int fields) -- 2.7.4
[PATCH 0/6] several fixes for SKX uncore
From: Kan Liang This series fixes the following issues: - wrong event_mask and event_ext_mask for UPI PMU - wrong PCU event format - wrong umask for LLC_LOOKUP.LOCAL and LLC_LOOKUP.REMOTE events - invalid CHA BOX FILTER - missing event filters support for TOR_INSERTS and TOR_OCCUPANCY - missing end marker for skx_uncore_cha_extra_regs Kan Liang (3): perf/x86/uncore: fix Skylake server PCU PMU event format perf/x86/uncore: fix Skylake server CHA LLC_LOOKUP event umask perf/x86/uncore: remove invalid Skylake server CHA filter field Stephane Eranian (3): perf/x86/uncore: fix Skylake UPI PMU event masks perf/x86/uncore: fix SKX CHA event extra regs perf/x86/uncore: fix missing marker for skx_uncore_cha_extra_regs arch/x86/events/intel/uncore_snbep.c | 51 +--- 1 file changed, 41 insertions(+), 10 deletions(-) -- 2.7.4
[PATCH 5/6] perf/x86/uncore: fix SKX CHA event extra regs
From: Stephane Eranian This patch adds two missing event extra regs for Skylake Server CHA PMU: - TOR_INSERTS - TOR_OCCUPANCY Were missing support for all the filters, including opcode matchers. Signed-off-by: Stephane Eranian Signed-off-by: Kan Liang --- arch/x86/events/intel/uncore_snbep.c | 13 + 1 file changed, 13 insertions(+) diff --git a/arch/x86/events/intel/uncore_snbep.c b/arch/x86/events/intel/uncore_snbep.c index 2401d06..f9f825b 100644 --- a/arch/x86/events/intel/uncore_snbep.c +++ b/arch/x86/events/intel/uncore_snbep.c @@ -3332,6 +3332,8 @@ static struct extra_reg skx_uncore_cha_extra_regs[] = { SNBEP_CBO_EVENT_EXTRA_REG(0x1134, 0x, 0x4), SNBEP_CBO_EVENT_EXTRA_REG(0x3134, 0x, 0x4), SNBEP_CBO_EVENT_EXTRA_REG(0x9134, 0x, 0x4), + SNBEP_CBO_EVENT_EXTRA_REG(0x35, 0xff, 0x8), + SNBEP_CBO_EVENT_EXTRA_REG(0x36, 0xff, 0x8), }; static u64 skx_cha_filter_mask(int fields) @@ -3344,6 +3346,17 @@ static u64 skx_cha_filter_mask(int fields) mask |= SKX_CHA_MSR_PMON_BOX_FILTER_LINK; if (fields & 0x4) mask |= SKX_CHA_MSR_PMON_BOX_FILTER_STATE; + if (fields & 0x8) { + mask |= SKX_CHA_MSR_PMON_BOX_FILTER_REM; + mask |= SKX_CHA_MSR_PMON_BOX_FILTER_LOC; + mask |= SKX_CHA_MSR_PMON_BOX_FILTER_ALL_OPC; + mask |= SKX_CHA_MSR_PMON_BOX_FILTER_NM; + mask |= SKX_CHA_MSR_PMON_BOX_FILTER_NOT_NM; + mask |= SKX_CHA_MSR_PMON_BOX_FILTER_OPC0; + mask |= SKX_CHA_MSR_PMON_BOX_FILTER_OPC1; + mask |= SKX_CHA_MSR_PMON_BOX_FILTER_NC; + mask |= SKX_CHA_MSR_PMON_BOX_FILTER_ISOC; + } return mask; } -- 2.7.4
Re: [PATCH] gpio: altera-a10sr: constify gpio_chip structure
On 07/11/2017 05:15 PM, Gustavo A. R. Silva wrote: This structure is only used to copy into another structure, so declare it as const. Signed-off-by: Gustavo A. R. Silva --- drivers/gpio/gpio-altera-a10sr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpio/gpio-altera-a10sr.c b/drivers/gpio/gpio-altera-a10sr.c index 16a8951..6b11f13 100644 --- a/drivers/gpio/gpio-altera-a10sr.c +++ b/drivers/gpio/gpio-altera-a10sr.c @@ -71,7 +71,7 @@ static int altr_a10sr_gpio_direction_output(struct gpio_chip *gc, return -EINVAL; } -static struct gpio_chip altr_a10sr_gc = { +static const struct gpio_chip altr_a10sr_gc = { .label = "altr_a10sr_gpio", .owner = THIS_MODULE, .get = altr_a10sr_gpio_get, Reviewed-by: Thor Thayer
[PATCH 1/6] perf/x86/uncore: fix Skylake UPI PMU event masks
From: Stephane Eranian This patch fixes the event_mask and event_ext_mask for the Intel Skylake Server UPI PMU. Bit 21 is not used as a filter. The extended umask is from bit 32 to bit 55. Correct both umasks. Signed-off-by: Stephane Eranian Signed-off-by: Kan Liang --- arch/x86/events/intel/uncore_snbep.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/arch/x86/events/intel/uncore_snbep.c b/arch/x86/events/intel/uncore_snbep.c index dae2fed..19a00a7 100644 --- a/arch/x86/events/intel/uncore_snbep.c +++ b/arch/x86/events/intel/uncore_snbep.c @@ -316,7 +316,7 @@ #define SKX_UPI_PCI_PMON_CTL0 0x350 #define SKX_UPI_PCI_PMON_CTR0 0x318 #define SKX_UPI_PCI_PMON_BOX_CTL 0x378 -#define SKX_PMON_CTL_UMASK_EXT 0xff +#define SKX_UPI_CTL_UMASK_EXT 0xffefff /* SKX M2M */ #define SKX_M2M_PCI_PMON_CTL0 0x228 @@ -328,7 +328,7 @@ DEFINE_UNCORE_FORMAT_ATTR(event2, event, "config:0-6"); DEFINE_UNCORE_FORMAT_ATTR(event_ext, event, "config:0-7,21"); DEFINE_UNCORE_FORMAT_ATTR(use_occ_ctr, use_occ_ctr, "config:7"); DEFINE_UNCORE_FORMAT_ATTR(umask, umask, "config:8-15"); -DEFINE_UNCORE_FORMAT_ATTR(umask_ext, umask, "config:8-15,32-39"); +DEFINE_UNCORE_FORMAT_ATTR(umask_ext, umask, "config:8-15,32-43,45-55"); DEFINE_UNCORE_FORMAT_ATTR(qor, qor, "config:16"); DEFINE_UNCORE_FORMAT_ATTR(edge, edge, "config:18"); DEFINE_UNCORE_FORMAT_ATTR(tid_en, tid_en, "config:19"); @@ -3603,8 +3603,8 @@ static struct intel_uncore_type skx_uncore_upi = { .perf_ctr_bits = 48, .perf_ctr = SKX_UPI_PCI_PMON_CTR0, .event_ctl = SKX_UPI_PCI_PMON_CTL0, - .event_mask = SNBEP_QPI_PCI_PMON_RAW_EVENT_MASK, - .event_mask_ext = SKX_PMON_CTL_UMASK_EXT, + .event_mask = SNBEP_PMON_RAW_EVENT_MASK, + .event_mask_ext = SKX_UPI_CTL_UMASK_EXT, .box_ctl= SKX_UPI_PCI_PMON_BOX_CTL, .ops= &skx_upi_uncore_pci_ops, .format_group = &skx_upi_uncore_format_group, -- 2.7.4
[PATCH 2/6] perf/x86/uncore: fix Skylake server PCU PMU event format
From: Kan Liang PCU event format for SKX are different from snbep. Introduce a new format group for SKX PCU. Signed-off-by: Kan Liang --- arch/x86/events/intel/uncore_snbep.c | 22 +- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/arch/x86/events/intel/uncore_snbep.c b/arch/x86/events/intel/uncore_snbep.c index 19a00a7..fbf8f6e 100644 --- a/arch/x86/events/intel/uncore_snbep.c +++ b/arch/x86/events/intel/uncore_snbep.c @@ -3492,6 +3492,26 @@ static struct intel_uncore_type skx_uncore_irp = { .format_group = &skx_uncore_format_group, }; +static struct attribute *skx_uncore_pcu_formats_attr[] = { + &format_attr_event.attr, + &format_attr_umask.attr, + &format_attr_edge.attr, + &format_attr_inv.attr, + &format_attr_thresh8.attr, + &format_attr_occ_invert.attr, + &format_attr_occ_edge_det.attr, + &format_attr_filter_band0.attr, + &format_attr_filter_band1.attr, + &format_attr_filter_band2.attr, + &format_attr_filter_band3.attr, + NULL, +}; + +static struct attribute_group skx_uncore_pcu_format_group = { + .name = "format", + .attrs = skx_uncore_pcu_formats_attr, +}; + static struct intel_uncore_ops skx_uncore_pcu_ops = { IVBEP_UNCORE_MSR_OPS_COMMON_INIT(), .hw_config = hswep_pcu_hw_config, @@ -3510,7 +3530,7 @@ static struct intel_uncore_type skx_uncore_pcu = { .box_ctl= HSWEP_PCU_MSR_PMON_BOX_CTL, .num_shared_regs= 1, .ops= &skx_uncore_pcu_ops, - .format_group = &snbep_uncore_pcu_format_group, + .format_group = &skx_uncore_pcu_format_group, }; static struct intel_uncore_type *skx_msr_uncores[] = { -- 2.7.4
Re: [PATCH v1] xen/grant-table: log the lack of grants
Hi, Anyone can you please review this patch? thanks, wengang On 07/07/2017 11:23 AM, Wengang Wang wrote: log a message when we enter this situation: 1) we already allocated the max number of available grants from hypervisor and 2) we still need more (but the request fails because of 1)). Sometimes the lack of grants causes IO hangs in xen_blkfront devices. Adding this log would help debuging. Signed-off-by: Wengang Wang Reviewed-by: Konrad Rzeszutek Wilk Reviewed-by: Junxiao Bi --- drivers/xen/grant-table.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c index d6786b8..2c6a911 100644 --- a/drivers/xen/grant-table.c +++ b/drivers/xen/grant-table.c @@ -42,6 +42,7 @@ #include #include #include +#include #include #include @@ -1072,8 +1073,14 @@ static int gnttab_expand(unsigned int req_entries) cur = nr_grant_frames; extra = ((req_entries + (grefs_per_grant_frame-1)) / grefs_per_grant_frame); - if (cur + extra > gnttab_max_grant_frames()) + if (cur + extra > gnttab_max_grant_frames()) { + pr_warn_ratelimited("xen/grant-table: max_grant_frames reached" + " cur=%u extra=%u limit=%u" + " gnttab_free_count=%u req_entries=%u\n", + cur, extra, gnttab_max_grant_frames(), + gnttab_free_count, req_entries); return -ENOSPC; + } rc = gnttab_map(cur, cur + extra - 1); if (rc == 0)
Re: [Intel-gfx] [PATCH RESEND] drm/i915: Fix pipe/transcoder enum mismatches
On Thu, Jul 13, 2017 at 09:23:11AM -0700, Stéphane Marchesin wrote: > On Thu, Jul 13, 2017 at 3:13 AM, Ville Syrjälä > wrote: > > On Wed, Jul 12, 2017 at 07:28:14PM -0700, Stéphane Marchesin wrote: > >> On Fri, May 5, 2017 at 10:40 AM, Ville Syrjälä > >> wrote: > >> > > >> > On Fri, May 05, 2017 at 10:26:36AM -0700, Matthias Kaehlcke wrote: > >> > > El Thu, Apr 20, 2017 at 02:56:05PM -0700 Matthias Kaehlcke ha dit: > >> > > > >> > > > In several instances the driver passes an 'enum pipe' value to a > >> > > > function expecting an 'enum transcoder' and viceversa. Since PIPE_x > >> > > > and > >> > > > TRANSCODER_x have the same values this doesn't cause functional > >> > > > problems. Still it is incorrect and causes clang to generate warnings > >> > > > like this: > >> > > > > >> > > > drivers/gpu/drm/i915/intel_display.c:1844:34: warning: implicit > >> > > > conversion from enumeration type 'enum transcoder' to different > >> > > > enumeration type 'enum pipe' [-Wenum-conversion] > >> > > > assert_fdi_rx_enabled(dev_priv, TRANSCODER_A); > >> > > > > >> > > > Change the code to pass values of the type expected by the callee. > >> > > > > >> > > > Signed-off-by: Matthias Kaehlcke > >> > > > --- > >> > > > drivers/gpu/drm/i915/intel_display.c | 4 ++-- > >> > > > drivers/gpu/drm/i915/intel_dp.c | 6 -- > >> > > > drivers/gpu/drm/i915/intel_hdmi.c| 6 -- > >> > > > drivers/gpu/drm/i915/intel_sdvo.c| 6 -- > >> > > > 4 files changed, 14 insertions(+), 8 deletions(-) > >> > > > >> > > Ping, any comments on this patch? > >> > > >> > I'm not convinced the patch is making things any better really. To > >> > fix this really properly, I think we'd need to introduce a new enum > >> > pch_transcoder and thus avoid the confusion of which type of > >> > transcoder we're talking about. Currently most places expect an > >> > enum pipe when dealing with PCH transcoders, and enum transcoder > >> > when dealing with CPU transcoders. But there are some exceptions > >> > of course. > >> > >> > >> I don't follow -- these functions take an enum transcoder; what's > >> wrong about passing what they expect? It seems like what you are > >> asking for has nothing to do with the warning here... > > > > There's a warning? I don't get any. > > Yup, clang generates a warning. > > > > > Anyways, I just don't see much point in blindly changing the types > > because it doesn't actually solve the underlying confusion for human > > readers. It might even make it worse, not sure. > > The function expects type A, you pass type B, how can that ever be the > right thing to do? Because maybe the function should be taking in type B instead. -- Ville Syrjälä Intel OTC
Re: [RFC 0/2] arm-smmu-v3 tlbi-on-map option
On Thu, Jul 13, 2017 at 10:29:42AM +0100, Jean-Philippe Brucker wrote: > On 12/07/17 23:07, Michael S. Tsirkin wrote: > [...] > > I think using hardware support for nesting is the right final > > solution. It will take some time though. Given this, what should > > we do meanwhile? > > > > Assuming that's the final destination, a simple quirk like this > > seems to be preferable to virtio-iommu which we'll never be > > able to offload to hardware. > > That's not entirely true. virtio-iommu will have an extension for hardware > nesting support. It was presented in my initial proposal, and I've made > significant progress since then. > > Thanks, > Jean I don't recall seeing this. Hardware specific extensions to virtio would be interesting, the difficulty is in finding the balance between enabling minor quirks and each vendor going their own way. Is this the proposal you refer to? https://www.spinics.net/lists/kvm/msg147990.html I couldn't find any mention of nesting, it seems to say that map requests are relayed through host. -- MST
[PATCH v2 0/9] perf annotate: Fix --show-total-period and support --show-nr-samples
Hello, Currently the --show-total-period option of perf-annotate is different from perf-report's. It has two problem like below: (Reported by Namhyung Kim and Milian Wolff) 1) Wrong column i.e. 'Percent' (even though using --show-total-period) 2) Show number of samples, not period So fix this option on both the annotate stdio and TUI browser. And support --show-nr-samples into perf-anntate so that it correpond with perf-report's. The code is available on 'perf/ann-fix-period-v2' branch at git://github.com/taeung/linux-perf.git Thanks, Taeung v2: - Separate the first patch into respective paches for easy review (Arnaldo) - Add a patch to introduce 'struct sym_hist_entry' (Arnaldo) - disasm__calc_percent() could receive a pointer to a struct sym_hist_entry instead of two pointer (Arnaldo) - Add a sym_hist_entry into struct disasm_line_samples (Arnaldo) - Fix a case that can't switch 'the total period view' to 'the number of samples view' on the annotate TUI browser (Arnaldo) - Calculate the percentage with 'period', not number of samples Taeung Song (9): perf annotate: Introduce struct sym_hist_entry perf annotate: Properly rename 'sum' to 'total_samples' in struct sym_hist perf annotate: Fix wrong --show-total-period option showing number of samples perf annotate: Show the proper header when using --show-total-period perf anntoate browser: Fix the toggle total period view to show period, not number of samples perf annotate browser: Show the proper header when using --show-total-period perf annotate: Support --show-nr-samples option perf annotate browser: Support the toggle number of samples with a 'e' hotkey perf annotate: Use the sample period when calculating the percentage tools/perf/builtin-annotate.c | 6 +- tools/perf/builtin-report.c | 13 ++-- tools/perf/builtin-top.c | 6 +- tools/perf/ui/browsers/annotate.c | 42 ++--- tools/perf/ui/gtk/annotate.c | 4 +- tools/perf/util/annotate.c| 125 ++ tools/perf/util/annotate.h| 15 +++-- 7 files changed, 148 insertions(+), 63 deletions(-) -- 2.7.4
[PATCH v2 1/9] perf annotate: Introduce struct sym_hist_entry
struct sym_hist has addr[] but it should have not only number of samples but also the sample period. So use new struct symhist_entry for that. Cc: Namhyung Kim Cc: Jiri Olsa Signed-off-by: Taeung Song --- tools/perf/ui/browsers/annotate.c | 6 +++--- tools/perf/ui/gtk/annotate.c | 4 ++-- tools/perf/util/annotate.c| 45 --- tools/perf/util/annotate.h| 9 ++-- 4 files changed, 35 insertions(+), 29 deletions(-) diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c index b376637..0cd9935 100644 --- a/tools/perf/ui/browsers/annotate.c +++ b/tools/perf/ui/browsers/annotate.c @@ -449,14 +449,14 @@ static void annotate_browser__calc_percent(struct annotate_browser *browser, next = disasm__get_next_ip_line(¬es->src->source, pos); for (i = 0; i < browser->nr_events; i++) { - u64 nr_samples; + struct sym_hist_entry sample; bpos->samples[i].percent = disasm__calc_percent(notes, evsel->idx + i, pos->offset, next ? next->offset : len, - &path, &nr_samples); - bpos->samples[i].nr = nr_samples; + &path, &sample); + bpos->samples[i].nr = sample.nr_samples; if (max_percent < bpos->samples[i].percent) max_percent = bpos->samples[i].percent; diff --git a/tools/perf/ui/gtk/annotate.c b/tools/perf/ui/gtk/annotate.c index 87e3760..d736fd5 100644 --- a/tools/perf/ui/gtk/annotate.c +++ b/tools/perf/ui/gtk/annotate.c @@ -34,10 +34,10 @@ static int perf_gtk__get_percent(char *buf, size_t size, struct symbol *sym, return 0; symhist = annotation__histogram(symbol__annotation(sym), evidx); - if (!symbol_conf.event_group && !symhist->addr[dl->offset]) + if (!symbol_conf.event_group && !symhist->addr[dl->offset].nr_samples) return 0; - percent = 100.0 * symhist->addr[dl->offset] / symhist->sum; + percent = 100.0 * symhist->addr[dl->offset].nr_samples / symhist->sum; markup = perf_gtk__get_percent_color(percent); if (markup) diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c index ef434b5..655c645 100644 --- a/tools/perf/util/annotate.c +++ b/tools/perf/util/annotate.c @@ -610,10 +610,10 @@ int symbol__alloc_hist(struct symbol *sym) size_t sizeof_sym_hist; /* Check for overflow when calculating sizeof_sym_hist */ - if (size > (SIZE_MAX - sizeof(struct sym_hist)) / sizeof(u64)) + if (size > (SIZE_MAX - sizeof(struct sym_hist)) / sizeof(struct sym_hist_entry)) return -1; - sizeof_sym_hist = (sizeof(struct sym_hist) + size * sizeof(u64)); + sizeof_sym_hist = (sizeof(struct sym_hist) + size * sizeof(struct sym_hist_entry)); /* Check for overflow in zalloc argument */ if (sizeof_sym_hist > (SIZE_MAX - sizeof(*notes->src)) @@ -714,11 +714,11 @@ static int __symbol__inc_addr_samples(struct symbol *sym, struct map *map, offset = addr - sym->start; h = annotation__histogram(notes, evidx); h->sum++; - h->addr[offset]++; + h->addr[offset].nr_samples++; pr_debug3("%#" PRIx64 " %s: period++ [addr: %#" PRIx64 ", %#" PRIx64 ", evidx=%d] => %" PRIu64 "\n", sym->start, sym->name, - addr, addr - sym->start, evidx, h->addr[offset]); + addr, addr - sym->start, evidx, h->addr[offset].nr_samples); return 0; } @@ -928,11 +928,12 @@ struct disasm_line *disasm__get_next_ip_line(struct list_head *head, struct disa } double disasm__calc_percent(struct annotation *notes, int evidx, s64 offset, - s64 end, const char **path, u64 *nr_samples) + s64 end, const char **path, struct sym_hist_entry *sample) { struct source_line *src_line = notes->src->lines; double percent = 0.0; - *nr_samples = 0; + + sample->nr_samples = 0; if (src_line) { size_t sizeof_src_line = sizeof(*src_line) + @@ -946,7 +947,7 @@ double disasm__calc_percent(struct annotation *notes, int evidx, s64 offset, *path = src_line->path; percent += src_line->samples[evidx].percent; - *nr_samples += src_line->samples[evidx].nr; + sample->nr_samples += src_line->samples[evidx].nr; offset++; } } else { @@ -954,10 +955,10 @@ double disasm__calc_percent(struct annotation *notes, int evidx, s64 offset, unsigned int h
[PATCH v2 4/9] perf annotate: Show the proper header when using --show-total-period
Currently a first column is only "Percent", so fix it to show correct column name based on given options. (e.g. if using --show-total-period, show "Event count" as a first column) Reported-by: Milian Wolff Cc: Namhyung Kim Cc: Jiri Olsa Signed-off-by: Taeung Song --- tools/perf/util/annotate.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c index 7311a00..1ac621a 100644 --- a/tools/perf/util/annotate.c +++ b/tools/perf/util/annotate.c @@ -1852,7 +1852,9 @@ int symbol__annotate_printf(struct symbol *sym, struct map *map, 4 * evsel->nr_members : 4; graph_dotted_len = printf(" %-*.*s| Source code & Disassembly of %s for %s (%" PRIu64 " %s)\n", - width, width, "Percent", d_filename, evsel_name, + width, width, + symbol_conf.show_total_period ? "Event count" : "Percent", + d_filename, evsel_name, symbol_conf.show_total_period ? h->total_period : h->total_samples, symbol_conf.show_total_period ? "event count" : "samples"); -- 2.7.4
[PATCH v2 3/9] perf annotate: Fix wrong --show-total-period option showing number of samples
Currently the --show-total-period option of perf-annotate is different from perf-report's. For example, perf-report ordinarily shows period and number of samples. # Overhead SamplesPeriod Command Shared Object Symbol # ... .. # 9.83% 102 84813704 test test[.] knapsack But --show-total-period of perf-annotate has the problem that show number of samples, not period. So fix this option to show period instead of number of samples. Before: Percent | Source code & Disassembly of old for cycles:ppp (102 samples) - : : : : Disassembly of section .text: : : 00400816 : : knapsack(): 1 :400816: push %rbp After: Percent | Source code & Disassembly of old for cycles:ppp (84813704 event count) -- : : : : Disassembly of section .text: : : 00400816 : : knapsack(): 743737 :400816: push %rbp Reported-by: Namhyung Kim Cc: Milian Wolff Cc: Jiri Olsa Signed-off-by: Taeung Song --- tools/perf/builtin-annotate.c | 4 +-- tools/perf/builtin-report.c | 13 ++ tools/perf/builtin-top.c | 6 +++-- tools/perf/util/annotate.c| 57 +-- tools/perf/util/annotate.h| 4 ++- 5 files changed, 65 insertions(+), 19 deletions(-) diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c index 7a5dc7e..f314661 100644 --- a/tools/perf/builtin-annotate.c +++ b/tools/perf/builtin-annotate.c @@ -177,14 +177,12 @@ static int perf_evsel__add_sample(struct perf_evsel *evsel, */ process_branch_stack(sample->branch_stack, al, sample); - sample->period = 1; sample->weight = 1; - he = hists__add_entry(hists, al, NULL, NULL, NULL, sample, true); if (he == NULL) return -ENOMEM; - ret = hist_entry__inc_addr_samples(he, evsel->idx, al->addr); + ret = hist_entry__inc_addr_samples(he, sample, evsel->idx, al->addr); hists__inc_nr_samples(hists, true); return ret; } diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c index 79a33eb..5f1894c 100644 --- a/tools/perf/builtin-report.c +++ b/tools/perf/builtin-report.c @@ -113,13 +113,14 @@ static int hist_iter__report_callback(struct hist_entry_iter *iter, struct report *rep = arg; struct hist_entry *he = iter->he; struct perf_evsel *evsel = iter->evsel; + struct perf_sample *sample = iter->sample; struct mem_info *mi; struct branch_info *bi; if (!ui__has_annotation()) return 0; - hist__account_cycles(iter->sample->branch_stack, al, iter->sample, + hist__account_cycles(sample->branch_stack, al, sample, rep->nonany_branch_mode); if (sort__mode == SORT_MODE__BRANCH) { @@ -136,14 +137,16 @@ static int hist_iter__report_callback(struct hist_entry_iter *iter, if (err) goto out; - err = hist_entry__inc_addr_samples(he, evsel->idx, al->addr); + err = hist_entry__inc_addr_samples(he, sample, + evsel->idx, al->addr); } else if (symbol_conf.cumulate_callchain) { if (single) - err = hist_entry__inc_addr_samples(he, evsel->idx, - al->addr); + err = hist_entry__inc_addr_samples(he, sample, + evsel->idx, al->addr); } else { - err = hist_entry__inc_addr_samples(he, evsel->idx, al->addr); + err = hist_entry__inc_addr_samples(he, sample, + evsel->idx, al->addr); } out: diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c index 022486d..09885a4 100644 --- a/tools/perf/builtin-top.c +++ b/tools/perf/builtin-top.c @@ -183,6 +183,7 @@ static void ui__warn_map_erange(struct map *map, struct symbol *sym, u64 ip) static void perf_top__record_precise_ip(struct perf_top *top, struct hist_entry *he, + struct perf_sample *sample, int counter, u64 ip) { struct annotation *notes; @@ -199,7 +200,7 @@ static void perf_top__record_precise_ip(struct perf_top *top, if (pthread_mutex_trylock(¬es->lock)) return; - err = hist_entry__inc_addr_samples(he, count
[PATCH v2 8/9] perf annotate browser: Support the toggle number of samples with a 'e' hotkey
Cc: Namhyung Kim Cc: Milian Wolff Cc: Jiri Olsa Signed-off-by: Taeung Song --- tools/perf/ui/browsers/annotate.c | 18 +- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c index 34b3189..19173b1 100644 --- a/tools/perf/ui/browsers/annotate.c +++ b/tools/perf/ui/browsers/annotate.c @@ -41,6 +41,7 @@ static struct annotate_browser_opt { jump_arrows, show_linenr, show_nr_jumps, +show_nr_samples, show_total_period; } annotate_browser__opts = { .use_offset = true, @@ -156,6 +157,9 @@ static void annotate_browser__write(struct ui_browser *browser, void *entry, int if (annotate_browser__opts.show_total_period) { ui_browser__printf(browser, "%10" PRIu64 " ", bdl->samples[i].sample.period); + } else if (annotate_browser__opts.show_nr_samples) { + ui_browser__printf(browser, "%6" PRIu64 " ", + bdl->samples[i].sample.nr_samples); } else { ui_browser__printf(browser, "%6.2f ", bdl->samples[i].percent); @@ -169,6 +173,8 @@ static void annotate_browser__write(struct ui_browser *browser, void *entry, int else { if (annotate_browser__opts.show_total_period) ui_browser__printf(browser, "%*s", 11, "Event count"); + else if (annotate_browser__opts.show_nr_samples) + ui_browser__printf(browser, "%*s", 7, "Samples"); else ui_browser__printf(browser, "%*s", 7, "Percent"); } @@ -834,6 +840,7 @@ static int annotate_browser__run(struct annotate_browser *browser, "o Toggle disassembler output/simplified view\n" "s Toggle source code view\n" "t Toggle total period view\n" + "e Toggle number of samples\n" "/ Search string\n" "k Toggle line numbers\n" "r Run available scripts\n" @@ -914,6 +921,12 @@ static int annotate_browser__run(struct annotate_browser *browser, !annotate_browser__opts.show_total_period; annotate_browser__update_addr_width(browser); continue; + case 'e': + annotate_browser__opts.show_total_period = false; + annotate_browser__opts.show_nr_samples = + !annotate_browser__opts.show_nr_samples; + annotate_browser__update_addr_width(browser); + continue; case K_LEFT: case K_ESC: case 'q': @@ -934,9 +947,11 @@ static int annotate_browser__run(struct annotate_browser *browser, int map_symbol__tui_annotate(struct map_symbol *ms, struct perf_evsel *evsel, struct hist_browser_timer *hbt) { - /* Set default value for show_total_period. */ + /* Set default value for show_total_period and show_nr_samples */ annotate_browser__opts.show_total_period = symbol_conf.show_total_period; + annotate_browser__opts.show_nr_samples = + symbol_conf.show_nr_samples; return symbol__tui_annotate(ms->sym, ms->map, evsel, hbt); } @@ -1187,6 +1202,7 @@ static struct annotate_config { ANNOTATE_CFG(jump_arrows), ANNOTATE_CFG(show_linenr), ANNOTATE_CFG(show_nr_jumps), + ANNOTATE_CFG(show_nr_samples), ANNOTATE_CFG(show_total_period), ANNOTATE_CFG(use_offset), }; -- 2.7.4
[PATCH v2 9/9] perf annotate: Use the sample period when calculating the percentage
Currently the percentages of perf-annotate are calculated with number of samples, not the sample period. So fix it to correspond with perf-report using the sample period for the calculation. Cc: Namhyung Kim Cc: Jiri Olsa Signed-off-by: Taeung Song --- tools/perf/util/annotate.c | 11 ++- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c index d9bdedf..28a6d11 100644 --- a/tools/perf/util/annotate.c +++ b/tools/perf/util/annotate.c @@ -988,7 +988,7 @@ double disasm__calc_percent(struct annotation *notes, int evidx, s64 offset, if (h->total_samples) { sample->nr_samples = hits; sample->period = p; - percent = 100.0 * hits / h->total_samples; + percent = 100.0 * p / h->total_period; } } @@ -1730,8 +1730,9 @@ static int symbol__get_source_line(struct symbol *sym, struct map *map, start = map__rip_2objdump(map, sym->start); for (i = 0; i < len; i++) { - u64 offset, nr_samples; + u64 offset; double percent_max = 0.0; + struct sym_hist_entry sample; src_line->nr_pcnt = nr_pcnt; @@ -1739,15 +1740,15 @@ static int symbol__get_source_line(struct symbol *sym, struct map *map, double percent = 0.0; h = annotation__histogram(notes, evidx + k); - nr_samples = h->addr[i].nr_samples; + sample = h->addr[i]; if (h->total_samples) - percent = 100.0 * nr_samples / h->total_samples; + percent = 100.0 * sample.period / h->total_period; if (percent > percent_max) percent_max = percent; src_line->samples[k].percent = percent; - src_line->samples[k].nr = nr_samples; + src_line->samples[k].nr = sample.nr_samples; } if (percent_max <= 0.5) -- 2.7.4
[PATCH v2 7/9] perf annotate: Support --show-nr-samples option
Add --show-nr-samples option to perf-annotate so that it corresponds with perf-report. Cc: Namhyung Kim Cc: Milian Wolff Cc: Jiri Olsa Signed-off-by: Taeung Song --- tools/perf/builtin-annotate.c | 2 ++ tools/perf/util/annotate.c| 6 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c index f314661..3cb0223 100644 --- a/tools/perf/builtin-annotate.c +++ b/tools/perf/builtin-annotate.c @@ -444,6 +444,8 @@ int cmd_annotate(int argc, const char **argv) "Show event group information together"), OPT_BOOLEAN(0, "show-total-period", &symbol_conf.show_total_period, "Show a column with the sum of periods"), + OPT_BOOLEAN('n', "show-nr-samples", &symbol_conf.show_nr_samples, + "Show a column with the number of samples"), OPT_CALLBACK_DEFAULT(0, "stdio-color", NULL, "mode", "'always' (default), 'never' or 'auto' only applicable to --stdio mode", stdio__config_color, "always"), diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c index 1ac621a..d9bdedf 100644 --- a/tools/perf/util/annotate.c +++ b/tools/perf/util/annotate.c @@ -1162,6 +1162,9 @@ static int disasm_line__print(struct disasm_line *dl, struct symbol *sym, u64 st if (symbol_conf.show_total_period) color_fprintf(stdout, color, " %11" PRIu64, sample.period); + else if (symbol_conf.show_nr_samples) + color_fprintf(stdout, color, " %7" PRIu64, + sample.nr_samples); else color_fprintf(stdout, color, " %7.2f", percent); } @@ -1853,7 +1856,8 @@ int symbol__annotate_printf(struct symbol *sym, struct map *map, graph_dotted_len = printf(" %-*.*s| Source code & Disassembly of %s for %s (%" PRIu64 " %s)\n", width, width, - symbol_conf.show_total_period ? "Event count" : "Percent", + symbol_conf.show_total_period ? "Event count" : + symbol_conf.show_nr_samples ? "Samples" : "Percent", d_filename, evsel_name, symbol_conf.show_total_period ? h->total_period : h->total_samples, symbol_conf.show_total_period ? "event count" : "samples"); -- 2.7.4
Re: [PATCH v2] xattr: Enable security.capability in user namespaces
Stefan Berger writes: > On 07/13/2017 12:40 PM, Theodore Ts'o wrote: >> On Thu, Jul 13, 2017 at 07:11:36AM -0500, Eric W. Biederman wrote: >>> The concise summary: >>> >>> Today we have the xattr security.capable that holds a set of >>> capabilities that an application gains when executed. AKA setuid root exec >>> without actually being setuid root. >>> >>> User namespaces have the concept of capabilities that are not global but >>> are limited to their user namespace. We do not currently have >>> filesystem support for this concept. >> So correct me if I am wrong; in general, there will only be one >> variant of the form: >> >> security.foo@uid=15000 >> >> It's not like there will be: >> >> security.foo@uid=1000 >> security.foo@uid=2000 > > A file shared by 2 containers, one mapping root to uid=1000, the other > mapping root to uid=2000, will show these two xattrs on the host > (init_user_ns) once these containers set xattrs on that file. There is an interesting solution for shared directory trees containing executables. Overlayfs is needed if you need those directory trees to be writable and for the files to show up as owned by uid 0. An overlayfs will have to do something with the security.capable attribute. So ignoring that case. If you don't care about the ownership of the files, and read only is acceptable, and you still don't want to give these executables capabilities in the initial user namespace. What you can do is make everything owned by some non-zero uid including the security capability. Call this non-zero uid image-root. When the container starts it creates two nested user namespaces first with image-root mapped to 0. Then with the containers choice of uid mapped to 0 image-root unmapped. This will ensure the capability attributes work for all containers that share that root image. And it ensures the file are read-only from the container. So I don't think there is ever a case where we would share a filesystem image where we would need to set multiple security attributes on a file. >> Otherwise, I suspect that the architecture is going to turn around and >> bite us in the *ss eventually, because someone will want to do >> something crazy and the solution will not be scalable. > > Can you define what 'scalable' means for you in this context? > From what I can see sharing a filesystem between multiple containers > doesn't 'scale well' for virtualizing the xattrs primarily because of > size limitations of xattrs per file. Worse than that I believe you will find that filesystems are built on the assumption that there will be a small number of xattrs per file. So even if the vfs limitations were lifted the filesystem performance would suffer. Even if the filesystem performed well I believe there are other issues with stat, and simply not having so much meta-data that adminstrators and tools get confused. So I believe there are some very good fundamental reasons why we want to limit the amount of meta-data per file. Eric
[PATCH v2 6/9] perf annotate browser: Show the proper header when using --show-total-period
Currently a first column is only "Percent", so fix it to show correct column name based on given options. (e.g. if using --show-total-period or a 't' hotkey, show "Event count" as a first column) Reported-by: Milian Wolff Cc: Namhyung Kim Cc: Jiri Olsa Signed-off-by: Taeung Song --- tools/perf/ui/browsers/annotate.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c index 883f6f2..34b3189 100644 --- a/tools/perf/ui/browsers/annotate.c +++ b/tools/perf/ui/browsers/annotate.c @@ -166,8 +166,12 @@ static void annotate_browser__write(struct ui_browser *browser, void *entry, int if (!show_title) ui_browser__write_nstring(browser, " ", pcnt_width); - else - ui_browser__printf(browser, "%*s", 7, "Percent"); + else { + if (annotate_browser__opts.show_total_period) + ui_browser__printf(browser, "%*s", 11, "Event count"); + else + ui_browser__printf(browser, "%*s", 7, "Percent"); + } } if (ab->have_cycles) { if (dl->ipc) -- 2.7.4
[PATCH v2 2/9] perf annotate: Properly rename 'sum' to 'total_samples' in struct sym_hist
Cc: Namhyung Kim Cc: Jiri Olsa Signed-off-by: Taeung Song --- tools/perf/ui/gtk/annotate.c | 2 +- tools/perf/util/annotate.c | 22 +++--- tools/perf/util/annotate.h | 2 +- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/tools/perf/ui/gtk/annotate.c b/tools/perf/ui/gtk/annotate.c index d736fd5..c088abe 100644 --- a/tools/perf/ui/gtk/annotate.c +++ b/tools/perf/ui/gtk/annotate.c @@ -37,7 +37,7 @@ static int perf_gtk__get_percent(char *buf, size_t size, struct symbol *sym, if (!symbol_conf.event_group && !symhist->addr[dl->offset].nr_samples) return 0; - percent = 100.0 * symhist->addr[dl->offset].nr_samples / symhist->sum; + percent = 100.0 * symhist->addr[dl->offset].nr_samples / symhist->total_samples; markup = perf_gtk__get_percent_color(percent); if (markup) diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c index 655c645..9c2a6e0 100644 --- a/tools/perf/util/annotate.c +++ b/tools/perf/util/annotate.c @@ -713,7 +713,7 @@ static int __symbol__inc_addr_samples(struct symbol *sym, struct map *map, offset = addr - sym->start; h = annotation__histogram(notes, evidx); - h->sum++; + h->total_samples++; h->addr[offset].nr_samples++; pr_debug3("%#" PRIx64 " %s: period++ [addr: %#" PRIx64 ", %#" PRIx64 @@ -957,9 +957,9 @@ double disasm__calc_percent(struct annotation *notes, int evidx, s64 offset, while (offset < end) hits += h->addr[offset++].nr_samples; - if (h->sum) { + if (h->total_samples) { sample->nr_samples = hits; - percent = 100.0 * hits / h->sum; + percent = 100.0 * hits / h->total_samples; } } @@ -1672,13 +1672,13 @@ static int symbol__get_source_line(struct symbol *sym, struct map *map, struct sym_hist *h = annotation__histogram(notes, evidx); struct rb_root tmp_root = RB_ROOT; int nr_pcnt = 1; - u64 h_sum = h->sum; + u64 h_sum = h->total_samples; size_t sizeof_src_line = sizeof(struct source_line); if (perf_evsel__is_group_event(evsel)) { for (i = 1; i < evsel->nr_members; i++) { h = annotation__histogram(notes, evidx + i); - h_sum += h->sum; + h_sum += h->total_samples; } nr_pcnt = evsel->nr_members; sizeof_src_line += (nr_pcnt - 1) * sizeof(src_line->samples); @@ -1704,8 +1704,8 @@ static int symbol__get_source_line(struct symbol *sym, struct map *map, h = annotation__histogram(notes, evidx + k); nr_samples = h->addr[i].nr_samples; - if (h->sum) - percent = 100.0 * nr_samples / h->sum; + if (h->total_samples) + percent = 100.0 * nr_samples / h->total_samples; if (percent > percent_max) percent_max = percent; @@ -1777,7 +1777,7 @@ static void symbol__annotate_hits(struct symbol *sym, struct perf_evsel *evsel) if (h->addr[offset].nr_samples != 0) printf("%*" PRIx64 ": %" PRIu64 "\n", BITS_PER_LONG / 2, sym->start + offset, h->addr[offset].nr_samples); - printf("%*s: %" PRIu64 "\n", BITS_PER_LONG / 2, "h->sum", h->sum); + printf("%*s: %" PRIu64 "\n", BITS_PER_LONG / 2, "h->total_samples", h->total_samples); } int symbol__annotate_printf(struct symbol *sym, struct map *map, @@ -1813,7 +1813,7 @@ int symbol__annotate_printf(struct symbol *sym, struct map *map, width *= evsel->nr_members; graph_dotted_len = printf(" %-*.*s| Source code & Disassembly of %s for %s (%" PRIu64 " samples)\n", - width, width, "Percent", d_filename, evsel_name, h->sum); + width, width, "Percent", d_filename, evsel_name, h->total_samples); printf("%-*.*s\n", graph_dotted_len, graph_dotted_len, graph_dotted_line); @@ -1877,10 +1877,10 @@ void symbol__annotate_decay_histogram(struct symbol *sym, int evidx) struct sym_hist *h = annotation__histogram(notes, evidx); int len = symbol__size(sym), offset; - h->sum = 0; + h->total_samples = 0; for (offset = 0; offset < len; ++offset) { h->addr[offset].nr_samples = h->addr[offset].nr_samples * 7 / 8; - h->sum += h->addr[offset].nr_samples; + h->total_samples += h->addr[offset].nr_samples; } } diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h index 3a17663..bef1d02 100644 --- a/tools/perf/util/annotate.h +++ b/tools/perf/util/annotate.h @@ -87,7 +87,7 @@ double disasm__calc_percent(s
[PATCH v2 5/9] perf anntoate browser: Fix the toggle total period view to show period, not number of samples
Currently the toggle total period view on the annotate TUI shows the number of samples, not period like below. So fix the toggle total period view on the annotate TUI like below. $ perf annotate --show-total-period Before: │Disassembly of section .text: │ │00109a90 <_mcount@@GLIBC_2.2.5>: │ sub$0x38,%rsp 3 │ mov%rax,(%rsp) 3 │ mov%rcx,0x8(%rsp) After: │Disassembly of section .text: │ │00109a90 <_mcount@@GLIBC_2.2.5>: │ sub$0x38,%rsp 2204022 │ mov%rax,(%rsp) 2207405 │ mov%rcx,0x8(%rsp) Reported-by: Namhyung Kim Cc: Milian Wolff Cc: Jiri Olsa Signed-off-by: Taeung Song --- tools/perf/ui/browsers/annotate.c | 14 +- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c index 0cd9935..883f6f2 100644 --- a/tools/perf/ui/browsers/annotate.c +++ b/tools/perf/ui/browsers/annotate.c @@ -17,7 +17,7 @@ struct disasm_line_samples { double percent; - u64 nr; + struct sym_hist_entry sample; }; #define IPC_WIDTH 6 @@ -113,6 +113,10 @@ static int annotate_browser__pcnt_width(struct annotate_browser *ab) if (ab->have_cycles) w += IPC_WIDTH + CYCLES_WIDTH; + + if (annotate_browser__opts.show_total_period) + w += 4 * ab->nr_events; + return w; } @@ -150,8 +154,8 @@ static void annotate_browser__write(struct ui_browser *browser, void *entry, int bdl->samples[i].percent, current_entry); if (annotate_browser__opts.show_total_period) { - ui_browser__printf(browser, "%6" PRIu64 " ", - bdl->samples[i].nr); + ui_browser__printf(browser, "%10" PRIu64 " ", + bdl->samples[i].sample.period); } else { ui_browser__printf(browser, "%6.2f ", bdl->samples[i].percent); @@ -161,7 +165,7 @@ static void annotate_browser__write(struct ui_browser *browser, void *entry, int ui_browser__set_percent_color(browser, 0, current_entry); if (!show_title) - ui_browser__write_nstring(browser, " ", 7 * ab->nr_events); + ui_browser__write_nstring(browser, " ", pcnt_width); else ui_browser__printf(browser, "%*s", 7, "Percent"); } @@ -456,7 +460,7 @@ static void annotate_browser__calc_percent(struct annotate_browser *browser, pos->offset, next ? next->offset : len, &path, &sample); - bpos->samples[i].nr = sample.nr_samples; + bpos->samples[i].sample = sample; if (max_percent < bpos->samples[i].percent) max_percent = bpos->samples[i].percent; -- 2.7.4
Re: [PATCH V1]: clk: qcom: support qpnp clock divider configuration
On 07/13, Tirupathi Reddy wrote: > This patch adds a new device driver for configuring the clkdiv module > presents on the Qualcomm QPNP PMIC. The driver configures the frequency > of clkdiv outputs. The driver provides callback functions for different > clock operations and register the clkdiv module to clock framework. > Please don't send cover letters for single patches. -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH v3 01/18] clk: ingenic: Use const pointer to clk_ops in struct
On 07/13, Ralf Baechle wrote: > On Thu, Jul 13, 2017 at 12:07:25PM +0200, Paul Cercueil wrote: > > > > Sorry I forgot, did you want an ack for these clk patches or for > > > me to take them through clk tree. If it's the ack case, > > > > > > Acked-by: Stephen Boyd > > > > > > for patches 1 through 6. > > > > I think ACK; then Ralf can take them in 4.13 :) > > My pull request for 4.13 is already finalized so it'd be great if this > could make it to 4.13 through the clk tree. If that should be impossible > I'd like to merge this via the MIPS tree for 4.14. > It's too late for v4.13, so you can take it for v4.14. -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
[PATCH][net-next] svcrdma: fix an incorrect check on -E2BIG and -EINVAL
From: Colin Ian King The current check will always be true and will always jump to err1, this looks dubious to me. I believe && should be used instead of ||. Detected by CoverityScan, CID#1450120 ("Logically Dead Code") Fixes: 107c1d0a991a ("svcrdma: Avoid Send Queue overflow") Signed-off-by: Colin Ian King --- net/sunrpc/xprtrdma/svc_rdma_sendto.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/sunrpc/xprtrdma/svc_rdma_sendto.c b/net/sunrpc/xprtrdma/svc_rdma_sendto.c index 19fd01e4b690..7c3a211e0e9a 100644 --- a/net/sunrpc/xprtrdma/svc_rdma_sendto.c +++ b/net/sunrpc/xprtrdma/svc_rdma_sendto.c @@ -684,7 +684,7 @@ int svc_rdma_sendto(struct svc_rqst *rqstp) return 0; err2: - if (ret != -E2BIG || ret != -EINVAL) + if (ret != -E2BIG && ret != -EINVAL) goto err1; ret = svc_rdma_post_recv(rdma, GFP_KERNEL); -- 2.11.0
Re: [PATCH v2 07/22] fpga: intel: pcie: parse feature list and create platform device for features.
On Sun, Jun 25, 2017 at 8:52 PM, Wu Hao wrote: > From: Xiao Guangrong > > Device Feature List structure creates a link list of feature headers > within the MMIO space to provide an extensible way of adding features. > > The Intel FPGA PCIe driver walks through the feature headers to enumerate > feature devices, FPGA Management Engine (FME) and FPGA Port for Accelerated > Function Unit (AFU), and their private sub features. For feature devices, > it creates the platform devices and linked the private sub features into > their platform data. > > Signed-off-by: Tim Whisonant > Signed-off-by: Enno Luebbers > Signed-off-by: Shiva Rao > Signed-off-by: Christopher Rauer > Signed-off-by: Kang Luwei > Signed-off-by: Zhang Yi > Signed-off-by: Xiao Guangrong > Signed-off-by: Wu Hao > --- > v2: moved the code to drivers/fpga folder as suggested by Alan Tull. > switched to GPLv2 license. > fixed comments from Moritz Fischer. > fixed kbuild warning, typos and clean up the code. > --- > drivers/fpga/Makefile| 2 +- > drivers/fpga/intel-feature-dev.c | 130 ++ > drivers/fpga/intel-feature-dev.h | 341 > drivers/fpga/intel-pcie.c| 841 > ++- > 4 files changed, 1311 insertions(+), 3 deletions(-) > create mode 100644 drivers/fpga/intel-feature-dev.c > create mode 100644 drivers/fpga/intel-feature-dev.h > > diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile > index 5613133..ad24b3d 100644 > --- a/drivers/fpga/Makefile > +++ b/drivers/fpga/Makefile > @@ -31,4 +31,4 @@ obj-$(CONFIG_OF_FPGA_REGION) += of-fpga-region.o > # Intel FPGA Support > obj-$(CONFIG_INTEL_FPGA_PCI) += intel-fpga-pci.o > > -intel-fpga-pci-objs := intel-pcie.o > +intel-fpga-pci-objs := intel-pcie.o intel-feature-dev.o > diff --git a/drivers/fpga/intel-feature-dev.c > b/drivers/fpga/intel-feature-dev.c > new file mode 100644 > index 000..68f9cba > --- /dev/null > +++ b/drivers/fpga/intel-feature-dev.c > @@ -0,0 +1,130 @@ > +/* > + * Intel FPGA Feature Device Driver > + * > + * Copyright (C) 2017 Intel Corporation, Inc. > + * > + * Authors: > + * Kang Luwei > + * Zhang Yi > + * Wu Hao > + * Xiao Guangrong > + * > + * This work is licensed under the terms of the GNU GPL version 2. See > + * the COPYING file in the top-level directory. > + */ > + > +#include "intel-feature-dev.h" > + > +void feature_platform_data_add(struct feature_platform_data *pdata, > + int index, const char *name, > + int resource_index, void __iomem *ioaddr) > +{ > + WARN_ON(index >= pdata->num); > + > + pdata->features[index].name = name; > + pdata->features[index].resource_index = resource_index; > + pdata->features[index].ioaddr = ioaddr; > +} > + > +struct feature_platform_data * > +feature_platform_data_alloc_and_init(struct platform_device *dev, int num) > +{ > + struct feature_platform_data *pdata; > + > + pdata = kzalloc(feature_platform_data_size(num), GFP_KERNEL); > + if (pdata) { > + pdata->dev = dev; > + pdata->num = num; > + mutex_init(&pdata->lock); > + } > + > + return pdata; > +} > + > +int fme_feature_num(void) > +{ > + return FME_FEATURE_ID_MAX; > +} > + > +int port_feature_num(void) > +{ > + return PORT_FEATURE_ID_MAX; > +} > + > +int fpga_port_id(struct platform_device *pdev) > +{ > + struct feature_port_header *port_hdr; > + struct feature_port_capability capability; > + > + port_hdr = get_feature_ioaddr_by_index(&pdev->dev, > + PORT_FEATURE_ID_HEADER); > + WARN_ON(!port_hdr); > + > + capability.csr = readq(&port_hdr->capability); > + return capability.port_number; > +} > +EXPORT_SYMBOL_GPL(fpga_port_id); > + > +/* > + * Enable Port by clear the port soft reset bit, which is set by default. > + * The User AFU is unable to respond to any MMIO access while in reset. > + * __fpga_port_enable function should only be used after __fpga_port_disable > + * function. > + */ > +void __fpga_port_enable(struct platform_device *pdev) > +{ > + struct feature_platform_data *pdata = dev_get_platdata(&pdev->dev); > + struct feature_port_header *port_hdr; > + struct feature_port_control control; > + > + WARN_ON(!pdata->disable_count); > + > + if (--pdata->disable_count != 0) > + return; > + > + port_hdr = get_feature_ioaddr_by_index(&pdev->dev, > + PORT_FEATURE_ID_HEADER); > + WARN_ON(!port_hdr); > + > + control.csr = readq(&port_hdr->control); > + control.port_sftrst = 0x0; > + writeq(control.csr, &port_hdr->control); > +} > +EXPORT_SYMBOL_GPL(__fpga_port_enable); > + > +#define RST_POLL_INVL 10 /* us */ > +#define RST_POLL_TIMEOUT 1000 /* us */ > + > +int __fpga_port_disable(st
Re: [PATCH][net-next] svcrdma: fix an incorrect check on -E2BIG and -EINVAL
> On Jul 13, 2017, at 1:51 PM, Colin King wrote: > > From: Colin Ian King > > The current check will always be true and will always jump to > err1, this looks dubious to me. I believe && should be used > instead of ||. > > Detected by CoverityScan, CID#1450120 ("Logically Dead Code") > > Fixes: 107c1d0a991a ("svcrdma: Avoid Send Queue overflow") > Signed-off-by: Colin Ian King Reviewed-by: Chuck Lever Dan reported this today, and I have a similar patch in my "pending for-rc" series. This one works too. > --- > net/sunrpc/xprtrdma/svc_rdma_sendto.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/net/sunrpc/xprtrdma/svc_rdma_sendto.c > b/net/sunrpc/xprtrdma/svc_rdma_sendto.c > index 19fd01e4b690..7c3a211e0e9a 100644 > --- a/net/sunrpc/xprtrdma/svc_rdma_sendto.c > +++ b/net/sunrpc/xprtrdma/svc_rdma_sendto.c > @@ -684,7 +684,7 @@ int svc_rdma_sendto(struct svc_rqst *rqstp) > return 0; > > err2: > - if (ret != -E2BIG || ret != -EINVAL) > + if (ret != -E2BIG && ret != -EINVAL) > goto err1; > > ret = svc_rdma_post_recv(rdma, GFP_KERNEL); > -- > 2.11.0 > -- Chuck Lever
Re: [PATCH] Documentation: ABI: mtd: describe "offset" more precisely
On Sun, Jun 25, 2017 at 01:11:54PM +0200, Rafał Miłecki wrote: > From: Rafał Miłecki > > So far Linux supported only two levels of MTD devices so we didn't need > a very precise description for this sysfs file. With commit > 97519dc52b44a ("mtd: partitions: add support for subpartitions") there > is support for a tree structure so we should have more precise > description. Using "parent" and "flash device" makes it more accurate. > > Signed-off-by: Rafał Miłecki Applied to linux-mtd.git
Re: [kernel-hardening] Re: [RFC PATCH 6/6] arm64: add VMAP_STACK and detect out-of-bounds SP
On Thu, Jul 13, 2017 at 05:10:50PM +0100, Mark Rutland wrote: > On Thu, Jul 13, 2017 at 12:49:48PM +0100, Ard Biesheuvel wrote: > > On 13 July 2017 at 11:49, Mark Rutland wrote: > > > On Thu, Jul 13, 2017 at 07:58:50AM +0100, Ard Biesheuvel wrote: > > >> On 12 July 2017 at 23:33, Mark Rutland wrote: > > Given that the very first stp in kernel_entry will fault if we have > > less than S_FRAME_SIZE bytes of stack left, I think we should check > > that we have at least that much space available. > > I was going to reply saying that I didn't agree, but in writing up > examples, I mostly convinced myself that this is the right thing to do. > So I mostly agree! > > This would mean we treat the first impossible-to-handle exception as > that fatal case, which is similar to x86's double-fault, triggered when > the HW can't stack the regs. All other cases are just arbitrary faults. > > However, to provide that consistently, we'll need to perform this check > at every exception boundary, or some of those cases will result in a > recursive fault first. > > So I think there are three choices: > > 1) In el1_sync, only check SP bounds, and live with the recursive >faults. > > 2) in el1_sync, check there's room for the regs, and live with the >recursive faults for overflow on other exceptions. > > 3) In all EL1 entry paths, check there's room for the regs. FWIW, for the moment I've applied (2), as you suggested, to my arm64/vmap-stack branch, adding an additional: sub x0, x0, #S_FRAME_SIZE ... to the entry path. I think it's worth trying (3) so that we consistently report these cases, benchmarks permitting. It's probably worth putting the fast-path check directly into the vectors, where we currently only use 1/32 of the instruction slots available to us. > As above, I think it's helpful to think of this as something closer to a > double-fault handler (i.e. aiming to catch when we can't take the > exception safely), rather than something that's trying to catch logical > stack overflows. Does this make sense to you? I've tried to reword the log output, as below, to give this impression. [ 49.288232] Insufficient stack space to handle exception! [ 49.288245] CPU: 5 PID: 2208 Comm: bash Not tainted 4.12.0-5-ga781af2 #81 [ 49.300680] Hardware name: ARM Juno development board (r1) (DT) [ 49.306549] task: 800974955100 task.stack: 0d6f [ 49.312426] PC is at recursive_loop+0x10/0x50 [ 49.316747] LR is at recursive_loop+0x34/0x50 [ 49.321066] pc : [] lr : [] pstate: 4145 [ 49.328398] sp : 0d6eff30 [ 49.331682] x29: 0d6f0350 x28: 800974955100 [ 49.336953] x27: 08942000 x26: 08f0d758 [ 49.342223] x25: 0d6f3eb8 x24: 0d6f3eb8 [ 49.347493] x23: 08f0d490 x22: 0009 [ 49.352764] x21: 800974a57000 x20: 08f0d4e0 [ 49.358034] x19: 0013 x18: e7e2e4f0 [ 49.363304] x17: 9c1256a4 x16: 081f8b88 [ 49.368574] x15: 2a81b800 x14: fff0 [ 49.373845] x13: 08f6278a x12: 08e62818 [ 49.379115] x11: x10: 019e [ 49.384385] x9 : 0004 x8 : 0d6f0770 [ 49.389656] x7 : 1313131313131313 x6 : 019e [ 49.394925] x5 : x4 : [ 49.400205] x3 : x2 : 0400 [ 49.405484] x1 : 0013 x0 : 0012 [ 49.410764] Task stack: [0x0d6f..0x0d6f4000] [ 49.416728] IRQ stack: [0x80097ffb90a0..0x80097ffbd0a0] [ 49.422692] ESR: 0x9647 -- DABT (current EL) [ 49.427277] FAR: 0x0d6eff30 [ 49.430742] Kernel panic - not syncing: kernel stack overflow [ 49.436451] CPU: 5 PID: 2208 Comm: bash Not tainted 4.12.0-5-ga781af2 #81 [ 49.443534] Hardware name: ARM Juno development board (r1) (DT) [ 49.449412] Call trace: [ 49.451852] [] dump_backtrace+0x0/0x230 [ 49.457218] [] show_stack+0x14/0x20 [ 49.462240] [] dump_stack+0x9c/0xc0 [ 49.467261] [] panic+0x11c/0x294 [ 49.472024] [] handle_bad_stack+0xe4/0xe8 [ 49.477561] [] recursive_loop+0x34/0x50 [ 49.482926] SMP: stopping secondary CPUs [ 49.487145] Kernel Offset: disabled [ 49.490609] Memory Limit: none [ 49.493649] ---[ end Kernel panic - not syncing: kernel stack overflow ... I still need to attack the backtracing to walk across stacks. Thanks, Mark.
Re: [PATCH v2] xattr: Enable security.capability in user namespaces
Stefan Berger writes: > On 07/13/2017 01:14 PM, Eric W. Biederman wrote: >> Theodore Ts'o writes: >> >>> On Thu, Jul 13, 2017 at 07:11:36AM -0500, Eric W. Biederman wrote: The concise summary: Today we have the xattr security.capable that holds a set of capabilities that an application gains when executed. AKA setuid root exec without actually being setuid root. User namespaces have the concept of capabilities that are not global but are limited to their user namespace. We do not currently have filesystem support for this concept. >>> So correct me if I am wrong; in general, there will only be one >>> variant of the form: >>> >>> security.foo@uid=15000 >>> >>> It's not like there will be: >>> >>> security.foo@uid=1000 >>> security.foo@uid=2000 >>> >>> Except if you have an Distribution root directory which is shared >>> by many containers, you would need to put the xattrs in the overlay >>> inodes. Worse, each time you launch a new container, with a new >>> subuid allocation, you will have to iterate over all files with >>> capabilities and do a copy-up operations on the xattrs in overlayfs. >>> So that's actually a bit of a disaster. >>> >>> So for distribution overlays, you will need to do things a different >>> way, which is to map the distro subdirectory so you know that the >>> capability with the global uid 0 should be used for the container >>> "root" uid, right? >>> >>> So this hack of using security.foo@uid=1000 is *only* useful when the >>> subcontainer root wants to create the privileged executable. You >>> still have to do things the other way. >>> >>> So can we make perhaps the assertion that *either*: >>> >>> security.foo >>> >>> exists, *or* >>> >>> security.foo@uid=BAR >>> >>> exists, but never both? And there BAR is exclusive to only one >>> instances? >>> >>> Otherwise, I suspect that the architecture is going to turn around and >>> bite us in the *ss eventually, because someone will want to do >>> something crazy and the solution will not be scalable. >> Yep. That is what it looks like from here. >> >> Which is why I asked the question about scalability of the xattr >> implementations. It looks like trying to accomodate the general >> case just gets us in trouble, and sets unrealistic expectations. >> >> Which strongly suggests that Serge's previous version that >> just reved the format of security.capable so that a uid field could >> be added is likely to be the better approach. >> >> I want to see what Serge and Stefan have to say but the case looks >> pretty clear cut at the moment. > > The approach of virtualizing the xattrs on the name-side, which is > what this patch does, provides a more general approach than to > virtualizing it on the value side, which is what Serge does in his > other patch for security.capability alone. With the virtualizing > on-the-value side virtualizing the xattr becomes an exercise that > needs to be repeated for every xattr name that one would want to > virtualize. With this patch you would just add another xattr name to a > list, a one-line patch in the end. Xattr with prefixes like trusted.* > need a bit more work but this can be woven in as well > (https://github.com/stefanberger/linux/commit/397b1a3b24045c67405fc83465e544fc865d402f). Reusable code has merit, as it reduces the maintenance burden. My big question right now is can you implement Ted's suggested restriction. Only one security.foo or secuirty.foo@... attribute ? The maintenance gains are definitely worth taking if they do not penalize the common case. > For virtualizing the xattrs on the 'value' side I was looking for > whether there's something like a 'wrapper' structure around the actual > value of the xattr so that that wrapper could be extended to support > different values at different uids and applied to any > xattr. Unfortunately there's no such 'wrapper'. Different values at different uids currently appear to be undesirable. At least for security.capable it does not appear to be useful. A wrapper structure is also a reasonable suggestion. Put it's magic number/version code where the existing version code is away we go. Do you know of cases where we will truly want to have different attributes for different containers? The case that I can think of for IMA is that the signatures want to be conntected to a key that goes with the filesystem image (so not a system key) but that would not be something that would need to be changed between containers. Eric
Re: [PATCH v12 8/8] virtio-balloon: VIRTIO_BALLOON_F_CMD_VQ
On Thu, Jul 13, 2017 at 04:46:29PM +0800, Wei Wang wrote: > On 07/13/2017 08:22 AM, Michael S. Tsirkin wrote: > > On Wed, Jul 12, 2017 at 08:40:21PM +0800, Wei Wang wrote: > > > Add a new vq, cmdq, to handle requests between the device and driver. > > > > > > This patch implements two commands sent from the device and handled in > > > the driver. > > > 1) VIRTIO_BALLOON_CMDQ_REPORT_STATS: this command is used to report > > > the guest memory statistics to the host. The stats_vq mechanism is not > > > used when the cmdq mechanism is enabled. > > > 2) VIRTIO_BALLOON_CMDQ_REPORT_UNUSED_PAGES: this command is used to > > > report the guest unused pages to the host. > > > > > > Since now we have a vq to handle multiple commands, we need to keep only > > > one vq operation at a time. Here, we change the existing START_USE() > > > and END_USE() to lock on each vq operation. > > > > > > Signed-off-by: Wei Wang > > > Signed-off-by: Liang Li > > > --- > > > drivers/virtio/virtio_balloon.c | 245 > > > ++-- > > > drivers/virtio/virtio_ring.c| 25 +++- > > > include/linux/virtio.h | 2 + > > > include/uapi/linux/virtio_balloon.h | 10 ++ > > > 4 files changed, 265 insertions(+), 17 deletions(-) > > > > > > diff --git a/drivers/virtio/virtio_balloon.c > > > b/drivers/virtio/virtio_balloon.c > > > index aa4e7ec..ae91fbf 100644 > > > --- a/drivers/virtio/virtio_balloon.c > > > +++ b/drivers/virtio/virtio_balloon.c > > > @@ -54,11 +54,12 @@ static struct vfsmount *balloon_mnt; > > > struct virtio_balloon { > > > struct virtio_device *vdev; > > > - struct virtqueue *inflate_vq, *deflate_vq, *stats_vq; > > > + struct virtqueue *inflate_vq, *deflate_vq, *stats_vq, *cmd_vq; > > > /* The balloon servicing is delegated to a freezable workqueue. > > > */ > > > struct work_struct update_balloon_stats_work; > > > struct work_struct update_balloon_size_work; > > > + struct work_struct cmdq_handle_work; > > > /* Prevent updating balloon when it is being canceled. */ > > > spinlock_t stop_update_lock; > > > @@ -90,6 +91,12 @@ struct virtio_balloon { > > > /* Memory statistics */ > > > struct virtio_balloon_stat stats[VIRTIO_BALLOON_S_NR]; > > > + /* Cmdq msg buffer for memory statistics */ > > > + struct virtio_balloon_cmdq_hdr cmdq_stats_hdr; > > > + > > > + /* Cmdq msg buffer for reporting ununsed pages */ typo above btw > > > + struct virtio_balloon_cmdq_hdr cmdq_unused_page_hdr; > > > + > > > /* To register callback in oom notifier call chain */ > > > struct notifier_block nb; > > > }; > > > @@ -485,25 +492,214 @@ static void update_balloon_size_func(struct > > > work_struct *work) > > > queue_work(system_freezable_wq, work); > > > } > > > +static unsigned int cmdq_hdr_add(struct virtqueue *vq, > > > + struct virtio_balloon_cmdq_hdr *hdr, > > > + bool in) > > > +{ > > > + unsigned int id = VIRTQUEUE_DESC_ID_INIT; > > > + uint64_t hdr_pa = (uint64_t)virt_to_phys((void *)hdr); > > > + > > > + virtqueue_add_chain_desc(vq, hdr_pa, sizeof(*hdr), &id, &id, in); > > > + > > > + /* Deliver the hdr for the host to send commands. */ > > > + if (in) { > > > + hdr->flags = 0; > > > + virtqueue_add_chain(vq, id, 0, NULL, hdr, NULL); > > > + virtqueue_kick(vq); > > > + } > > > + > > > + return id; > > > +} > > > + > > > +static void cmdq_add_chain_desc(struct virtio_balloon *vb, > > > + struct virtio_balloon_cmdq_hdr *hdr, > > > + uint64_t addr, > > > + uint32_t len, > > > + unsigned int *head_id, > > > + unsigned int *prev_id) > > > +{ > > > +retry: > > > + if (*head_id == VIRTQUEUE_DESC_ID_INIT) { > > > + *head_id = cmdq_hdr_add(vb->cmd_vq, hdr, 0); > > > + *prev_id = *head_id; > > > + } > > > + > > > + virtqueue_add_chain_desc(vb->cmd_vq, addr, len, head_id, prev_id, 0); > > > + if (*head_id == *prev_id) { > > That's an ugly way to detect ring full. > > It's actually not detecting ring full. I will call it tail_id, instead of > prev_id. > So, *head_id == *tail_id is the case that the first desc was just added by > virtqueue_add_chain_desc(). > > Best, > Wei Oh so it's adding header before each list. Ugh. I don't think we should stay with this API. It's just too tricky to use. If we have an API that fails when it can't add descriptors (you can reserve space for the last descriptor) the balloon knows whether it's the first descriptor in a chain and can just use a boolean that tells it whether that is the case. -- MST
Re: [PATCH] Revert "x86/uaccess: Add stack frame output operand in get_user() inline asm"
On Wed, Jul 12, 2017 at 04:22:13PM -0700, Matthias Kaehlcke wrote: > El Wed, Jul 12, 2017 at 05:36:30PM -0500 Josh Poimboeuf ha dit: > > > On Wed, Jul 12, 2017 at 05:35:47PM -0500, Josh Poimboeuf wrote: > > > On Wed, Jul 12, 2017 at 03:20:40PM -0700, Matthias Kaehlcke wrote: > > > > > This is admittedly an awkward way of achieving this goal, but it's the > > > > > only way I know how to do it with GCC. > > > > > > > > > > What extra instruction does clang add? > > > > > > > > I was looking at the get_user() call in drm_mode_setcrtc(). The code > > > > generated by clang without the patch is: > > > > > > > > if (get_user(out_id, &set_connectors_ptr[i])) { > > > > 81386955: 4a 8d 04 bd 00 00 00lea0x0(,%r15,4),%rax > > > > 8138695c: 00 > > > > 8138695d: 49 03 06add(%r14),%rax > > > > 81386960: e8 2b a5 f0 ff callq 81290e90 > > > > <__get_user_4> > > > > > > > > And with the patch: > > > > > > > > if (get_user(out_id, &set_connectors_ptr[i])) { > > > > 81386a56: 4a 8d 04 bd 00 00 00lea0x0(,%r15,4),%rax > > > > 81386a5d: 00 > > > > 81386a5e: 49 03 06add(%r14),%rax > > > > 81386a61: 48 8b 64 24 28 mov0x28(%rsp),%rsp > > > > 81386a66: e8 15 a5 f0 ff callq > > > > 81290f80 <__get_user_4> > > > > > > Hm, that seems odd. Can you sure the disassembly for the whole > > > function? > > > > Er, share :-) > > Sure, please find below the disassemblies with and without the > patch. The exact extra instruction differs from the one above, the > disassembly above is from a debug session with some 'random' kernel > version (bisect), the ones below from a v4.12ish kernel. At the bottom > you also find a log of a double faults observed with the patch. > > If you are interested in building the kernel with clang yourself I can > provide instructions, it is fairly painless nowadays as long as you > have a recent version of clang (a somewhat older version should also > do for this issue with some extra kernel patches). Here's the reason for the double fault. First it puts zero on the stack at offset -0x58: > 81367616: 31 c0 xor%eax,%eax > 81367618: 48 89 45 c8 mov%rax,-0x38(%rbp) > 8136761c: 45 31 ffxor%r15d,%r15d > 8136761f: 48 89 45 a8 mov%rax,-0x58(%rbp) Then, later, it copies that zeroed word from the stack to RSP: > 81367874: 48 8b 65 a8 mov-0x58(%rbp),%rsp Then it double faults because the call instruction tries to write RIP on the stack, but RSP is zero: > 81367878: e8 73 26 f1 ff callq 81279ef0 > <__get_user_4> Then clang tries to put RSP's value on the stack, at the same stack slot where the original zero was stored (though it never reaches this point): > 8136787d: 49 89 d4mov%rdx,%r12 > 81367880: 48 89 65 a8 mov%rsp,-0x58(%rbp) The panic is consistent with the above. RIP points to the call instruction, RSP is zero: > [3.798722] PANIC: double fault, error_code: 0x0 > [3.807387] CPU: 1 PID: 605 Comm: frecon Not tainted > 4.12.0-00023-g711d82c128ff #107 > [3.816040] Hardware name: GOOGLE Squawks, BIOS Google_Squawks.5216.152.76 > 03/04/2016 > [3.824792] task: 880075b92f00 task.stack: c9d6c000 > [3.829599] EXT4-fs (mmcblk0p1): re-mounted. Opts: commit=600,data=ordered > [3.839092] RIP: 0010:drm_mode_setcrtc+0x328/0x51f > [3.83] RSP: 0018: EFLAGS: 00010206 > [3.850280] RAX: 559e707c4d60 RBX: RCX: > 0008 > [3.858253] RDX: 0001 RSI: c9d6fcc8 RDI: > 81367805 > [3.866225] RBP: c9d6fd90 R08: 014000c0 R09: > 0308 > [3.874199] R10: 0300 R11: 0556 R12: > > [3.882163] R13: 880077a25000 R14: c9d6fdd0 R15: > > [3.890136] FS: 7fb2dbd62740() GS:88007ad0() > knlGS: > [3.899177] CS: 0010 DS: ES: CR0: 80050033 > [3.905595] CR2: fff8 CR3: 7596b000 CR4: > 001006e0 > [3.913568] Call Trace: > [3.916296] Code: b8 48 8d b5 38 ff ff ff 41 8b 56 08 39 d3 0f 83 85 00 00 > 00 4c 63 fb 4a 83 24 f8 00 4a 8d 04 bd 00 00 00 00 49 03 06 48 8b 65 a8 > 73 26 f1 ff 49 89 d4 48 89 65 a8 85 c0 0f 85 a0 00 00 00 ba > [3.937440] Kernel panic - not syncing: Machine halted. > [3.943268] CPU: 1 PID: 605 Comm: frecon Not tainted > 4.12.0-00023-g711d82c128ff #107 > [3.951921] Hardware name: GOOGLE Squawks, BIOS Google_Squawks.5216.152.76 > 03/04/2016 > [3.960671] Call
[PATCH] sctp: don't dereference ptr before leaving _sctp_walk_{params,errors}()
If the iterator (|pos.p| or |err|) has already reached the end of chunk, we shouldn't access iterator->length. This bug has been detected by KMSAN. For the following pair of system calls: socket(PF_INET6, SOCK_STREAM, 0x84 /* IPPROTO_??? */) = 3 sendto(3, "A", 1, MSG_OOB, {sa_family=AF_INET6, sin6_port=htons(0), inet_pton(AF_INET6, "::1", &sin6_addr), sin6_flowinfo=0, sin6_scope_id=0}, 28) = 1 the tool has reported a use of uninitialized memory: == BUG: KMSAN: use of uninitialized memory in sctp_rcv+0x17b8/0x43b0 CPU: 1 PID: 2940 Comm: probe Not tainted 4.11.0-rc5+ #2926 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011 Call Trace: __dump_stack lib/dump_stack.c:16 dump_stack+0x172/0x1c0 lib/dump_stack.c:52 kmsan_report+0x12a/0x180 mm/kmsan/kmsan.c:927 __msan_warning_32+0x61/0xb0 mm/kmsan/kmsan_instr.c:469 __sctp_rcv_init_lookup net/sctp/input.c:1074 __sctp_rcv_lookup_harder net/sctp/input.c:1233 __sctp_rcv_lookup net/sctp/input.c:1255 sctp_rcv+0x17b8/0x43b0 net/sctp/input.c:170 sctp6_rcv+0x32/0x70 net/sctp/ipv6.c:984 ip6_input_finish+0x82f/0x1ee0 net/ipv6/ip6_input.c:279 NF_HOOK ./include/linux/netfilter.h:257 ip6_input+0x239/0x290 net/ipv6/ip6_input.c:322 dst_input ./include/net/dst.h:492 ip6_rcv_finish net/ipv6/ip6_input.c:69 NF_HOOK ./include/linux/netfilter.h:257 ipv6_rcv+0x1dbd/0x22e0 net/ipv6/ip6_input.c:203 __netif_receive_skb_core+0x2f6f/0x3a20 net/core/dev.c:4208 __netif_receive_skb net/core/dev.c:4246 process_backlog+0x667/0xba0 net/core/dev.c:4866 napi_poll net/core/dev.c:5268 net_rx_action+0xc95/0x1590 net/core/dev.c:5333 __do_softirq+0x485/0x942 kernel/softirq.c:284 do_softirq_own_stack+0x1c/0x30 arch/x86/entry/entry_64.S:902 do_softirq kernel/softirq.c:328 __local_bh_enable_ip+0x25b/0x290 kernel/softirq.c:181 local_bh_enable+0x37/0x40 ./include/linux/bottom_half.h:31 rcu_read_unlock_bh ./include/linux/rcupdate.h:931 ip6_finish_output2+0x19b2/0x1cf0 net/ipv6/ip6_output.c:124 ip6_finish_output+0x764/0x970 net/ipv6/ip6_output.c:149 NF_HOOK_COND ./include/linux/netfilter.h:246 ip6_output+0x456/0x520 net/ipv6/ip6_output.c:163 dst_output ./include/net/dst.h:486 NF_HOOK ./include/linux/netfilter.h:257 ip6_xmit+0x1841/0x1c00 net/ipv6/ip6_output.c:261 sctp_v6_xmit+0x3b7/0x470 net/sctp/ipv6.c:225 sctp_packet_transmit+0x38cb/0x3a20 net/sctp/output.c:632 sctp_outq_flush+0xeb3/0x46e0 net/sctp/outqueue.c:885 sctp_outq_uncork+0xb2/0xd0 net/sctp/outqueue.c:750 sctp_side_effects net/sctp/sm_sideeffect.c:1773 sctp_do_sm+0x6962/0x6ec0 net/sctp/sm_sideeffect.c:1147 sctp_primitive_ASSOCIATE+0x12c/0x160 net/sctp/primitive.c:88 sctp_sendmsg+0x43e5/0x4f90 net/sctp/socket.c:1954 inet_sendmsg+0x498/0x670 net/ipv4/af_inet.c:762 sock_sendmsg_nosec net/socket.c:633 sock_sendmsg net/socket.c:643 SYSC_sendto+0x608/0x710 net/socket.c:1696 SyS_sendto+0x8a/0xb0 net/socket.c:1664 do_syscall_64+0xe6/0x130 arch/x86/entry/common.c:285 entry_SYSCALL64_slow_path+0x25/0x25 arch/x86/entry/entry_64.S:246 RIP: 0033:0x401133 RSP: 002b:7fff6d99cd38 EFLAGS: 0246 ORIG_RAX: 002c RAX: ffda RBX: 004002b0 RCX: 00401133 RDX: 0001 RSI: 00494088 RDI: 0003 RBP: 7fff6d99cd90 R08: 7fff6d99cd50 R09: 001c R10: 0001 R11: 0246 R12: R13: 004063d0 R14: 00406460 R15: origin: save_stack_trace+0x37/0x40 arch/x86/kernel/stacktrace.c:59 kmsan_save_stack_with_flags mm/kmsan/kmsan.c:302 kmsan_internal_poison_shadow+0xb1/0x1a0 mm/kmsan/kmsan.c:198 kmsan_poison_shadow+0x6d/0xc0 mm/kmsan/kmsan.c:211 slab_alloc_node mm/slub.c:2743 __kmalloc_node_track_caller+0x200/0x360 mm/slub.c:4351 __kmalloc_reserve net/core/skbuff.c:138 __alloc_skb+0x26b/0x840 net/core/skbuff.c:231 alloc_skb ./include/linux/skbuff.h:933 sctp_packet_transmit+0x31e/0x3a20 net/sctp/output.c:570 sctp_outq_flush+0xeb3/0x46e0 net/sctp/outqueue.c:885 sctp_outq_uncork+0xb2/0xd0 net/sctp/outqueue.c:750 sctp_side_effects net/sctp/sm_sideeffect.c:1773 sctp_do_sm+0x6962/0x6ec0 net/sctp/sm_sideeffect.c:1147 sctp_primitive_ASSOCIATE+0x12c/0x160 net/sctp/primitive.c:88 sctp_sendmsg+0x43e5/0x4f90 net/sctp/socket.c:1954 inet_sendmsg+0x498/0x670 net/ipv4/af_inet.c:762 sock_sendmsg_nosec net/socket.c:633 sock_sendmsg net/socket.c:643 SYSC_sendto+0x608/0x710 net/socket.c:1696 SyS_sendto+0x8a/0xb0 net/socket.c:1664 do_syscall_64+0xe6/0x130 arch/x86/entry/common.c:285 return_from_SYSCALL_64+0x0/0x6a arch/x86/entry/entry_64.S:246 == Signed-off-by: Alexander Potapenko --- include/net/sctp/sctp.h | 2 ++ 1 file changed,
[GIT PULL] MTD updates for 4.13-rc1
Hi Linus, This is a bit on the late side of the merge window, but here goes anyway. The following changes since commit 08332893e37af6ae779367e78e444f8f9571511d: Linux 4.12-rc2 (2017-05-21 19:30:23 -0700) are available in the git repository at: git://git.infradead.org/linux-mtd.git tags/for-linus-20170713 for you to fetch changes up to 7d84120b5ba61912a5333f5fe2c4e8f35ef9514f: Documentation: ABI: mtd: describe "offset" more precisely (2017-07-13 10:54:45 -0700) MTD updates for v4.13-rc1: General updates * Cleanups and additional flash support for "dataflash" driver * new driver for mchp23k256 SPI SRAM device * improve handling of MTDs without eraseblocks (i.e., MTD_NO_ERASE) * refactor and improve "sub-partition" handling with TRX partition parser; partitions can now be created as sub-partitions of another partition SPI NOR updates, from Cyrille Pitchen and Marek Vasut: * introduce support to the SPI 1-2-2 and 1-4-4 protocols. * introduce support to the Double Data Rate (DDR) mode. * introduce support to the Octo SPI protocols. * add support to new memory parts for Spansion, Macronix and Winbond. * add fixes for the Aspeed, STM32 and Cadence QSPI controler drivers. * clean up the st_spi_fsm driver. NAND updates, from Boris Brezillon: * addition of on-die ECC support to Micron driver * addition of helpers to help drivers choose most appropriate ECC settings * deletion of dead-code (cached programming and ->errstat() hook) * make sure drivers that do not support the SET/GET FEATURES command return ENOTSUPP use a dummy ->set/get_features implementation returning -ENOTSUPP (required for Micron on-die ECC) * change the semantic of ecc->write_page() for drivers setting the NAND_ECC_CUSTOM_PAGE_ACCESS flag * support exiting 'GET STATUS' command in default ->cmdfunc() implementations * change the prototype of ->setup_data_interface() A bunch of driver related changes: * various cleanup, fixes and improvements of the MTK driver * OMAP DT bindings fixes * support for ->setup_data_interface() in the fsmc driver * support for imx7 in the gpmi driver * finalization of the denali driver rework (thanks to Masahiro for the work he's done on this driver) * fix "bitflips in erased pages" handling in the ifc driver * addition of PM ops and dynamic timing configuration to the atmel driver Alexander Couzens (1): mtd: nand: davinci: set ECC algorithm explicitly for HW based ECC Alexander Sverdlin (1): mtd: spi-nor: Add support for mx66u51235f Alexandre Belloni (1): mtd: nand: atmel: drop unused include Andres Galacho (1): mtd: nand: tango: Export OF device ID table as module aliases Andrew Lunn (1): mtd: mchp23k256: Add driver for this SPI SRAM device Andrey Smirnov (6): mtd: dataflash: Replace C99 types with their kernel counterparts mtd: dataflash: Improve coding style in jedec_probe() mtd: dataflash: Replace pr_debug, printk with dev_* functions mtd: dataflash: Get rid of loop counter in jedec_probe() mtd: dataflash: Make use of "extened device information" mtd: dataflash: Add flash_info for AT45DB641E Arnd Bergmann (2): mtd: nand: atmel: mark resume function __maybe_unused mtd: spi-nor: cqspi: remove duplicate const Arvind Yadav (2): mtd: nand: orion: Handle return value of clk_prepare_enable mtd: spi-nor: cqspi: make of_device_ids const Benjamin Herrenschmidt (1): spi-nor: Add Winbond w25m512jv Boris Brezillon (16): mtd: nand: jz4780: Use mtd_set_ooblayout() to set the ooblayout mtd: nand: Make sure drivers not supporting SET/GET_FEATURES return -ENOTSUPP mtd: nand: gpmi: Fix gpmi_nand_init() error path mtd: nand: gpmi: Kill gpmi_nand_exit() mtd: nand: Pass the CS line to ->setup_data_interface() mtd: nand: atmel: Add ->setup_data_interface() hooks mtd: nand: atmel: Add PM ops mtd: nand: Drop unused cached programming support mtd: nand: Drop the ->errstat() hook mtd: nand: sunxi: Actually use DMA for subpage reads mtd: nand: sunxi: Remove unneeded ->cmdfunc(NAND_CMD_READ0, 0, page) mtd: nand: tango: Fix incorrect use of SEQIN command mtd: nand: Wait for PAGEPROG to finish in drivers setting NAND_ECC_CUSTOM_PAGE_ACCESS mtd: nand: Support 'EXIT GET STATUS' command in nand_command[_lp]() mtd: nand: fsl_ifc: fix handing of bit flips in erased pages mtd: Fix check in mtd_unpoint() Brian Norris (13): Merge 'v4.12-rc1' into MTD mtd: spi-nor: stm32-quadspi: allow building with COMPILE_TEST mtd: nand: don't leak buffers when ->scan_bbt() fails mtd: nand: drop unneeded module.h include
Re: [RFC PATCH 1/1] mm/mremap: add MREMAP_MIRROR flag for existing mirroring functionality
On 07/13/2017 09:30 AM, Andrea Arcangeli wrote: > On Thu, Jul 13, 2017 at 09:01:54AM -0700, Mike Kravetz wrote: >> Sent a patch (in separate e-mail thread) to return EINVAL for private >> mappings. > > The way old_len == 0 behaves for MAP_PRIVATE seems more sane to me > than the alternative of copying pagetables for anon pages (as behaving > the way that way avoids to break anon pages invariants), despite it's > not creating an exact mirror of what was in the original vma as it > excludes any modification done to cowed anon pages. > > By nullifying move_page_tables old_len == 0 is simply duping the vma > which is equivalent to a new mmap on the file for the MAP_PRIVATE > case, it has a deterministic result. The real question is if it > anybody is using it. As previously discussed, copying pagetables (via move_page_tables) does not happen if old_len == 0. This is true for both for private and shared mappings. Here is my understanding of how things work for old_len == 0 of anon mappings: - shared mappings - New vma is created at new virtual address - vma refers to the same underlying object/pages as old vma - after mremap, no page tables exist for new vma, they are created as pages are accessed/faulted - page at new_address is same as page at old_address - private mappings - New vma is created at new virtual address - vma does not refer to same pages as old vma. It is a 'new' private anon mapping. - after mremap, no page tables exist for new vma. access to the range of the new vma will result in faults that allocate a new page. - page at new_address is different than page at old_address the new vma will result in new So, the result of mremap(old_len == 0) on a private mapping is that it simply creates a new private mapping. IMO, this is contrary to the purpose of mremap. mremap should return a mapping that is somehow related to the original mapping. Perhaps you are thinking about mremap of a private file mapping? I was not considering that case. I believe you are right. In this case a private COW mapping based on the original mapping would be created. So, this seems more in line with the intent of mremap. The new mapping is still related to the old mapping. With this in mind, what about returning EINVAL only for the anon private mapping case? However, if you have a fd (for a file mapping) then I can not see why someone would be using the old_len == 0 trick. It would be more straight forward to simply use mmap to create the additional mapping. > So an alternative would be to start by adding a WARN_ON_ONCE deprecation > warning instead of -EINVAL right away. > > The vma->vm_flags VM_ACCOUNT being wiped on the original vma as side > effect of using the old_len == 0 trick looks like a bug, I guess it > should get fixed if we intend to keep old_len and document it for the > long term. Others seem to think we should keep old_len == 0 and document. > Overall I'm more concerned about the fact an allocation failure in > do_munmap is unreported to userland and it will leave the old vma > intact like old_len == 0 would do (unless I'm misreading something > there). The VM_ACCOUNT wipe as side effect of old_len == 0 is not > major short term concern. I assume you are concerned about the do_munmap call in move_vma? That does indeed look to be of concern. This happens AFTER setting up the new mapping. So, I'm thinking we should tear down the new mapping in the case do_munmap of the old mapping fails? That 'should' simply be a matter of: - moving page tables back to original mapping - remove/delete new vma - I don't think we need to 'unmap' the new vma as there should be no associated pages. I'll look into doing this as well. Just curious, do those userfaultfd callouts still work as desired in the case of map duplication (old_len == 0)? -- Mike Kravetz
Re: [PATCH v7 3/3] net/cxgb4: Use new PCI_DEV_FLAGS_NO_RELAXED_ORDERING flag
On Thu, Jul 13, 2017 at 7:21 AM, Ding Tianhong wrote: > From: Casey Leedom > > cxgb4 Ethernet driver now queries PCIe configuration space to determine > if it can send TLPs to it with the Relaxed Ordering Attribute set. > > Remove the enable_pcie_relaxed_ordering() to avoid enable PCIe Capability > Device Control[Relaxed Ordering Enable] at probe routine, to make sure > the driver will not send the Relaxed Ordering TLPs to the Root Complex which > could not deal the Relaxed Ordering TLPs. > > Signed-off-by: Casey Leedom > Signed-off-by: Ding Tianhong Ding, You can probably just drop this patch. If I am understanding Casey correctly just the fact that the relaxed ordering enable bit is cleared in the configuration should be enough to do this for the device automatically. - Alex > --- > drivers/net/ethernet/chelsio/cxgb4/cxgb4.h | 1 + > drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c | 23 +-- > drivers/net/ethernet/chelsio/cxgb4/sge.c| 5 +++-- > 3 files changed, 21 insertions(+), 8 deletions(-) > > diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h > b/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h > index ef4be78..09ea62e 100644 > --- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h > +++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h > @@ -529,6 +529,7 @@ enum { /* adapter flags */ > USING_SOFT_PARAMS = (1 << 6), > MASTER_PF = (1 << 7), > FW_OFLD_CONN = (1 << 9), > + ROOT_NO_RELAXED_ORDERING = (1 << 10), > }; > > enum { > diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c > b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c > index e403fa1..391e484 100644 > --- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c > +++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c > @@ -4654,11 +4654,6 @@ static void print_port_info(const struct net_device > *dev) > dev->name, adap->params.vpd.id, adap->name, buf); > } > > -static void enable_pcie_relaxed_ordering(struct pci_dev *dev) > -{ > - pcie_capability_set_word(dev, PCI_EXP_DEVCTL, > PCI_EXP_DEVCTL_RELAX_EN); > -} > - > /* > * Free the following resources: > * - memory used for tables > @@ -4908,7 +4903,6 @@ static int init_one(struct pci_dev *pdev, const struct > pci_device_id *ent) > } > > pci_enable_pcie_error_reporting(pdev); > - enable_pcie_relaxed_ordering(pdev); > pci_set_master(pdev); > pci_save_state(pdev); > > @@ -4947,6 +4941,23 @@ static int init_one(struct pci_dev *pdev, const struct > pci_device_id *ent) > adapter->msg_enable = DFLT_MSG_ENABLE; > memset(adapter->chan_map, 0xff, sizeof(adapter->chan_map)); > > + /* If possible, we use PCIe Relaxed Ordering Attribute to deliver > +* Ingress Packet Data to Free List Buffers in order to allow for > +* chipset performance optimizations between the Root Complex and > +* Memory Controllers. (Messages to the associated Ingress Queue > +* notifying new Packet Placement in the Free Lists Buffers will be > +* send without the Relaxed Ordering Attribute thus guaranteeing that > +* all preceding PCIe Transaction Layer Packets will be processed > +* first.) But some Root Complexes have various issues with Upstream > +* Transaction Layer Packets with the Relaxed Ordering Attribute set. > +* The PCIe devices which under the Root Complexes will be cleared the > +* Relaxed Ordering bit in the configuration space, So we check our > +* PCIe configuration space to see if it's flagged with advice against > +* using Relaxed Ordering. > +*/ > + if (!pcie_relaxed_ordering_supported(pdev)) > + adapter->flags |= ROOT_NO_RELAXED_ORDERING; > + > spin_lock_init(&adapter->stats_lock); > spin_lock_init(&adapter->tid_release_lock); > spin_lock_init(&adapter->win0_lock); > diff --git a/drivers/net/ethernet/chelsio/cxgb4/sge.c > b/drivers/net/ethernet/chelsio/cxgb4/sge.c > index ede1220..4ef68f6 100644 > --- a/drivers/net/ethernet/chelsio/cxgb4/sge.c > +++ b/drivers/net/ethernet/chelsio/cxgb4/sge.c > @@ -2719,6 +2719,7 @@ int t4_sge_alloc_rxq(struct adapter *adap, struct > sge_rspq *iq, bool fwevtq, > struct fw_iq_cmd c; > struct sge *s = &adap->sge; > struct port_info *pi = netdev_priv(dev); > + int relaxed = !(adap->flags & ROOT_NO_RELAXED_ORDERING); > > /* Size needs to be multiple of 16, including status entry. */ > iq->size = roundup(iq->size, 16); > @@ -2772,8 +2773,8 @@ int t4_sge_alloc_rxq(struct adapter *adap, struct > sge_rspq *iq, bool fwevtq, > > flsz = fl->size / 8 + s->stat_len / sizeof(struct tx_desc); > c.iqns_to_fl0congen |= htonl(FW_IQ_CMD_FL0PACKEN_F | > -FW_IQ_CMD_FL0FETCHRO_F | > -
Re: [PATCH] sctp: don't dereference ptr before leaving _sctp_walk_{params,errors}()
On Thu, Jul 13, 2017 at 8:10 PM, Alexander Potapenko wrote: > If the iterator (|pos.p| or |err|) has already reached the end of > chunk, we shouldn't access iterator->length. > > This bug has been detected by KMSAN. For the following pair of system > calls: > > socket(PF_INET6, SOCK_STREAM, 0x84 /* IPPROTO_??? */) = 3 > sendto(3, "A", 1, MSG_OOB, {sa_family=AF_INET6, sin6_port=htons(0), > inet_pton(AF_INET6, "::1", &sin6_addr), sin6_flowinfo=0, > sin6_scope_id=0}, 28) = 1 > > the tool has reported a use of uninitialized memory: > > == > BUG: KMSAN: use of uninitialized memory in sctp_rcv+0x17b8/0x43b0 > CPU: 1 PID: 2940 Comm: probe Not tainted 4.11.0-rc5+ #2926 > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs > 01/01/2011 > Call Trace: > >__dump_stack lib/dump_stack.c:16 >dump_stack+0x172/0x1c0 lib/dump_stack.c:52 >kmsan_report+0x12a/0x180 mm/kmsan/kmsan.c:927 >__msan_warning_32+0x61/0xb0 mm/kmsan/kmsan_instr.c:469 >__sctp_rcv_init_lookup net/sctp/input.c:1074 >__sctp_rcv_lookup_harder net/sctp/input.c:1233 >__sctp_rcv_lookup net/sctp/input.c:1255 >sctp_rcv+0x17b8/0x43b0 net/sctp/input.c:170 >sctp6_rcv+0x32/0x70 net/sctp/ipv6.c:984 >ip6_input_finish+0x82f/0x1ee0 net/ipv6/ip6_input.c:279 >NF_HOOK ./include/linux/netfilter.h:257 >ip6_input+0x239/0x290 net/ipv6/ip6_input.c:322 >dst_input ./include/net/dst.h:492 >ip6_rcv_finish net/ipv6/ip6_input.c:69 >NF_HOOK ./include/linux/netfilter.h:257 >ipv6_rcv+0x1dbd/0x22e0 net/ipv6/ip6_input.c:203 >__netif_receive_skb_core+0x2f6f/0x3a20 net/core/dev.c:4208 >__netif_receive_skb net/core/dev.c:4246 >process_backlog+0x667/0xba0 net/core/dev.c:4866 >napi_poll net/core/dev.c:5268 >net_rx_action+0xc95/0x1590 net/core/dev.c:5333 >__do_softirq+0x485/0x942 kernel/softirq.c:284 >do_softirq_own_stack+0x1c/0x30 arch/x86/entry/entry_64.S:902 > >do_softirq kernel/softirq.c:328 >__local_bh_enable_ip+0x25b/0x290 kernel/softirq.c:181 >local_bh_enable+0x37/0x40 ./include/linux/bottom_half.h:31 >rcu_read_unlock_bh ./include/linux/rcupdate.h:931 >ip6_finish_output2+0x19b2/0x1cf0 net/ipv6/ip6_output.c:124 >ip6_finish_output+0x764/0x970 net/ipv6/ip6_output.c:149 >NF_HOOK_COND ./include/linux/netfilter.h:246 >ip6_output+0x456/0x520 net/ipv6/ip6_output.c:163 >dst_output ./include/net/dst.h:486 >NF_HOOK ./include/linux/netfilter.h:257 >ip6_xmit+0x1841/0x1c00 net/ipv6/ip6_output.c:261 >sctp_v6_xmit+0x3b7/0x470 net/sctp/ipv6.c:225 >sctp_packet_transmit+0x38cb/0x3a20 net/sctp/output.c:632 >sctp_outq_flush+0xeb3/0x46e0 net/sctp/outqueue.c:885 >sctp_outq_uncork+0xb2/0xd0 net/sctp/outqueue.c:750 >sctp_side_effects net/sctp/sm_sideeffect.c:1773 >sctp_do_sm+0x6962/0x6ec0 net/sctp/sm_sideeffect.c:1147 >sctp_primitive_ASSOCIATE+0x12c/0x160 net/sctp/primitive.c:88 >sctp_sendmsg+0x43e5/0x4f90 net/sctp/socket.c:1954 >inet_sendmsg+0x498/0x670 net/ipv4/af_inet.c:762 >sock_sendmsg_nosec net/socket.c:633 >sock_sendmsg net/socket.c:643 >SYSC_sendto+0x608/0x710 net/socket.c:1696 >SyS_sendto+0x8a/0xb0 net/socket.c:1664 >do_syscall_64+0xe6/0x130 arch/x86/entry/common.c:285 >entry_SYSCALL64_slow_path+0x25/0x25 arch/x86/entry/entry_64.S:246 > RIP: 0033:0x401133 > RSP: 002b:7fff6d99cd38 EFLAGS: 0246 ORIG_RAX: 002c > RAX: ffda RBX: 004002b0 RCX: 00401133 > RDX: 0001 RSI: 00494088 RDI: 0003 > RBP: 7fff6d99cd90 R08: 7fff6d99cd50 R09: 001c > R10: 0001 R11: 0246 R12: > R13: 004063d0 R14: 00406460 R15: > origin: >save_stack_trace+0x37/0x40 arch/x86/kernel/stacktrace.c:59 >kmsan_save_stack_with_flags mm/kmsan/kmsan.c:302 >kmsan_internal_poison_shadow+0xb1/0x1a0 mm/kmsan/kmsan.c:198 >kmsan_poison_shadow+0x6d/0xc0 mm/kmsan/kmsan.c:211 >slab_alloc_node mm/slub.c:2743 >__kmalloc_node_track_caller+0x200/0x360 mm/slub.c:4351 >__kmalloc_reserve net/core/skbuff.c:138 >__alloc_skb+0x26b/0x840 net/core/skbuff.c:231 >alloc_skb ./include/linux/skbuff.h:933 >sctp_packet_transmit+0x31e/0x3a20 net/sctp/output.c:570 >sctp_outq_flush+0xeb3/0x46e0 net/sctp/outqueue.c:885 >sctp_outq_uncork+0xb2/0xd0 net/sctp/outqueue.c:750 >sctp_side_effects net/sctp/sm_sideeffect.c:1773 >sctp_do_sm+0x6962/0x6ec0 net/sctp/sm_sideeffect.c:1147 >sctp_primitive_ASSOCIATE+0x12c/0x160 net/sctp/primitive.c:88 >sctp_sendmsg+0x43e5/0x4f90 net/sctp/socket.c:1954 >inet_sendmsg+0x498/0x670 net/ipv4/af_inet.c:762 >sock_sendmsg_nosec net/socket.c:633 >sock_sendmsg net/socket.c:643 >SYSC_sendto+0x608/0x710 net/socket.c:1696 >SyS_sendto+0x8a/0xb0 net/socket.c:1664 >do_syscall_64+0xe6/
Re: [PATCH v7 3/3] net/cxgb4: Use new PCI_DEV_FLAGS_NO_RELAXED_ORDERING flag
On Thu, Jul 13, 2017 at 11:14 AM, Alexander Duyck wrote: > On Thu, Jul 13, 2017 at 7:21 AM, Ding Tianhong > wrote: >> From: Casey Leedom >> >> cxgb4 Ethernet driver now queries PCIe configuration space to determine >> if it can send TLPs to it with the Relaxed Ordering Attribute set. >> >> Remove the enable_pcie_relaxed_ordering() to avoid enable PCIe Capability >> Device Control[Relaxed Ordering Enable] at probe routine, to make sure >> the driver will not send the Relaxed Ordering TLPs to the Root Complex which >> could not deal the Relaxed Ordering TLPs. >> >> Signed-off-by: Casey Leedom >> Signed-off-by: Ding Tianhong > > Ding, > > You can probably just drop this patch. If I am understanding Casey > correctly just the fact that the relaxed ordering enable bit is > cleared in the configuration should be enough to do this for the > device automatically. > > - Alex Actually I take that back. I hadn't caught the most recent parts of the thread. If this is good for Casey then this works for me. - Alex
Re: [alsa-devel] [PATCH] ASoC: fsl_asrc: constify snd_soc_dai_ops structure
On Thu, 2017-07-13 at 10:18 -0500, Gustavo A. R. Silva wrote: > Hi Mark, > > Quoting Mark Brown : > > > On Thu, Jul 13, 2017 at 09:32:41AM +0200, Takashi Iwai wrote: > > > > > please stop posting in this style. It's really annoying to see > > > spontaneously popping-up almost same patch for more than two hours > > > long. > > > If you have a series of the same fix patches, send them as a patch > > > set in a shot with a thread. git-send-email does it right. > > > I don't mind a couple of patches posted separately, but this is over > > > the limit. > > > > Or at least just collect them up and send them all at one time even if > > not as a single thread (you don't want to CC everyone affected by a > > single patch in the set on everything, that's harder to avoid when > > sending a series via git, but it can be confusing to get one item in a > > large patch series without context). > > I like this idea better. I will do so next time. :) I don't it's better. It's not that confusing if the 0/n patch cover letter is cc'd to all the appropriate mailing lists and all the [1..n]/n patches are sent with in-reply-to of the cover letter and send to the maintainers and appropriate mailing lists.
Re: [PATCH] sctp: don't dereference ptr before leaving _sctp_walk_{params,errors}()
On Thu, Jul 13, 2017 at 8:14 PM, Alexander Potapenko wrote: > On Thu, Jul 13, 2017 at 8:10 PM, Alexander Potapenko > wrote: >> If the iterator (|pos.p| or |err|) has already reached the end of >> chunk, we shouldn't access iterator->length. >> >> This bug has been detected by KMSAN. For the following pair of system >> calls: >> >> socket(PF_INET6, SOCK_STREAM, 0x84 /* IPPROTO_??? */) = 3 >> sendto(3, "A", 1, MSG_OOB, {sa_family=AF_INET6, sin6_port=htons(0), >> inet_pton(AF_INET6, "::1", &sin6_addr), sin6_flowinfo=0, >> sin6_scope_id=0}, 28) = 1 >> >> the tool has reported a use of uninitialized memory: >> >> == >> BUG: KMSAN: use of uninitialized memory in sctp_rcv+0x17b8/0x43b0 >> CPU: 1 PID: 2940 Comm: probe Not tainted 4.11.0-rc5+ #2926 >> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs >> 01/01/2011 >> Call Trace: >> >>__dump_stack lib/dump_stack.c:16 >>dump_stack+0x172/0x1c0 lib/dump_stack.c:52 >>kmsan_report+0x12a/0x180 mm/kmsan/kmsan.c:927 >>__msan_warning_32+0x61/0xb0 mm/kmsan/kmsan_instr.c:469 >>__sctp_rcv_init_lookup net/sctp/input.c:1074 >>__sctp_rcv_lookup_harder net/sctp/input.c:1233 >>__sctp_rcv_lookup net/sctp/input.c:1255 >>sctp_rcv+0x17b8/0x43b0 net/sctp/input.c:170 >>sctp6_rcv+0x32/0x70 net/sctp/ipv6.c:984 >>ip6_input_finish+0x82f/0x1ee0 net/ipv6/ip6_input.c:279 >>NF_HOOK ./include/linux/netfilter.h:257 >>ip6_input+0x239/0x290 net/ipv6/ip6_input.c:322 >>dst_input ./include/net/dst.h:492 >>ip6_rcv_finish net/ipv6/ip6_input.c:69 >>NF_HOOK ./include/linux/netfilter.h:257 >>ipv6_rcv+0x1dbd/0x22e0 net/ipv6/ip6_input.c:203 >>__netif_receive_skb_core+0x2f6f/0x3a20 net/core/dev.c:4208 >>__netif_receive_skb net/core/dev.c:4246 >>process_backlog+0x667/0xba0 net/core/dev.c:4866 >>napi_poll net/core/dev.c:5268 >>net_rx_action+0xc95/0x1590 net/core/dev.c:5333 >>__do_softirq+0x485/0x942 kernel/softirq.c:284 >>do_softirq_own_stack+0x1c/0x30 arch/x86/entry/entry_64.S:902 >> >>do_softirq kernel/softirq.c:328 >>__local_bh_enable_ip+0x25b/0x290 kernel/softirq.c:181 >>local_bh_enable+0x37/0x40 ./include/linux/bottom_half.h:31 >>rcu_read_unlock_bh ./include/linux/rcupdate.h:931 >>ip6_finish_output2+0x19b2/0x1cf0 net/ipv6/ip6_output.c:124 >>ip6_finish_output+0x764/0x970 net/ipv6/ip6_output.c:149 >>NF_HOOK_COND ./include/linux/netfilter.h:246 >>ip6_output+0x456/0x520 net/ipv6/ip6_output.c:163 >>dst_output ./include/net/dst.h:486 >>NF_HOOK ./include/linux/netfilter.h:257 >>ip6_xmit+0x1841/0x1c00 net/ipv6/ip6_output.c:261 >>sctp_v6_xmit+0x3b7/0x470 net/sctp/ipv6.c:225 >>sctp_packet_transmit+0x38cb/0x3a20 net/sctp/output.c:632 >>sctp_outq_flush+0xeb3/0x46e0 net/sctp/outqueue.c:885 >>sctp_outq_uncork+0xb2/0xd0 net/sctp/outqueue.c:750 >>sctp_side_effects net/sctp/sm_sideeffect.c:1773 >>sctp_do_sm+0x6962/0x6ec0 net/sctp/sm_sideeffect.c:1147 >>sctp_primitive_ASSOCIATE+0x12c/0x160 net/sctp/primitive.c:88 >>sctp_sendmsg+0x43e5/0x4f90 net/sctp/socket.c:1954 >>inet_sendmsg+0x498/0x670 net/ipv4/af_inet.c:762 >>sock_sendmsg_nosec net/socket.c:633 >>sock_sendmsg net/socket.c:643 >>SYSC_sendto+0x608/0x710 net/socket.c:1696 >>SyS_sendto+0x8a/0xb0 net/socket.c:1664 >>do_syscall_64+0xe6/0x130 arch/x86/entry/common.c:285 >>entry_SYSCALL64_slow_path+0x25/0x25 arch/x86/entry/entry_64.S:246 >> RIP: 0033:0x401133 >> RSP: 002b:7fff6d99cd38 EFLAGS: 0246 ORIG_RAX: 002c >> RAX: ffda RBX: 004002b0 RCX: 00401133 >> RDX: 0001 RSI: 00494088 RDI: 0003 >> RBP: 7fff6d99cd90 R08: 7fff6d99cd50 R09: 001c >> R10: 0001 R11: 0246 R12: >> R13: 004063d0 R14: 00406460 R15: >> origin: >>save_stack_trace+0x37/0x40 arch/x86/kernel/stacktrace.c:59 >>kmsan_save_stack_with_flags mm/kmsan/kmsan.c:302 >>kmsan_internal_poison_shadow+0xb1/0x1a0 mm/kmsan/kmsan.c:198 >>kmsan_poison_shadow+0x6d/0xc0 mm/kmsan/kmsan.c:211 >>slab_alloc_node mm/slub.c:2743 >>__kmalloc_node_track_caller+0x200/0x360 mm/slub.c:4351 >>__kmalloc_reserve net/core/skbuff.c:138 >>__alloc_skb+0x26b/0x840 net/core/skbuff.c:231 >>alloc_skb ./include/linux/skbuff.h:933 >>sctp_packet_transmit+0x31e/0x3a20 net/sctp/output.c:570 >>sctp_outq_flush+0xeb3/0x46e0 net/sctp/outqueue.c:885 >>sctp_outq_uncork+0xb2/0xd0 net/sctp/outqueue.c:750 >>sctp_side_effects net/sctp/sm_sideeffect.c:1773 >>sctp_do_sm+0x6962/0x6ec0 net/sctp/sm_sideeffect.c:1147 >>sctp_primitive_ASSOCIATE+0x12c/0x160 net/sctp/primitive.c:88 >>sctp_sendmsg+0x43e5/0x4f90 net/sctp/socket.c:1954 >>inet_sendmsg+0x498/0x670 net/ipv4/af_inet.c:762 >>sock_sendmsg_nosec net/s
Re: [GIT PULL] RTC for 4.13
Me again... On 13/07/2017 at 11:22:39 +0200, Alexandre Belloni wrote: > On 13/07/2017 at 11:01:51 +0200, Alexandre Belloni wrote: > > Hi Linus, > > > > Here is the pull-request for the RTC subsystem for 4.13. > > > > And I forgot to add that you will see a small Add/Add merge conflict in > tools/testing/selftests/timers/Makefile which is easy to solve. > And its not actually add/add but both patches are adding a target on a different line. Sorry about the miscommunication, I'll get back to wiping baby poop. -- Alexandre Belloni, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
Re: Decreasing time for `rsa_init`
Dear Stephan, Am Mittwoch, den 12.07.2017, 19:38 +0200 schrieb Paul Menzel: > On 07/12/17 19:28, Stephan Müller wrote: > > Am Mittwoch, 12. Juli 2017, 12:59:58 CEST schrieb Paul Menzel: > > > Building CRYPTO_RSA not as module, but into the Linux kernel, > > > `rsa_init()` takes 130 ms on an ASRock E350M1. > > > > > > (Timings are shown by adding `initcall_debug` to Linux command > > > line [1]. > > > The times are visualized by `analyze_boot.py` from pm-graph [2] > > > or `systemd-bootchart`.) > > > > > > This is quite a lot of time compared to other modules, and I > > > wonder if > > > there are ways to decrease that time other than building it as a > > > module, > > > and not signing modules? > > > > Is the testmgr compiled? If yes, the self test may take that time. > > It looks like it is, as the tests are not disabled. > > ``` > $ grep MANAGER_DISABLE_TESTS .config > # CONFIG_CRYPTO_MANAGER_DISABLE_TESTS is not set > ``` > > I’ll try an image without the tests, and will report back. Thank you. That was it. Disabling the tests reduces the time to 51 μs. ``` kernel: calling rsa_init+0x0/0x40 @ 1 kernel: initcall rsa_init+0x0/0x40 returned 0 after 51 usecs ``` It’d be nice to be able to disable the testmgr during run-time by adding an option to the Linux Kernel command line for example. Kind regards, Paul > > > [1] http://elinux.org/Initcall_Debug > > > [2] https://github.com/01org/pm-graph
[PATCH] perf: xgene: Remove unnecessary managed resources cleanup
Managed resources in the driver should be automatically cleaned up on driver detach. It's unnecessary to manually free/unmmap these resources. One of the manual cleanup causes static checkers to complain. The bug is reported by Dan Carpenter in [1] [1] https://www.spinics.net/lists/arm-kernel/msg593012.html This patch gets rid of all the unnecessary manual cleanup and properly unregister all the registered PMU devices by the driver on driver detach. Signed-off-by: Tai Nguyen --- drivers/perf/xgene_pmu.c | 74 ++-- 1 file changed, 22 insertions(+), 52 deletions(-) diff --git a/drivers/perf/xgene_pmu.c b/drivers/perf/xgene_pmu.c index e841282..eb23311 100644 --- a/drivers/perf/xgene_pmu.c +++ b/drivers/perf/xgene_pmu.c @@ -1147,7 +1147,6 @@ static int xgene_init_perf(struct xgene_pmu_dev *pmu_dev, char *name) { struct device *dev = xgene_pmu->dev; struct xgene_pmu_dev *pmu; - int rc; pmu = devm_kzalloc(dev, sizeof(*pmu), GFP_KERNEL); if (!pmu) @@ -1159,7 +1158,7 @@ static int xgene_init_perf(struct xgene_pmu_dev *pmu_dev, char *name) switch (pmu->inf->type) { case PMU_TYPE_L3C: if (!(xgene_pmu->l3c_active_mask & pmu->inf->enable_mask)) - goto dev_err; + return -ENODEV; if (xgene_pmu->version == PCP_PMU_V3) pmu->attr_groups = l3c_pmu_v3_attr_groups; else @@ -1177,7 +1176,7 @@ static int xgene_init_perf(struct xgene_pmu_dev *pmu_dev, char *name) break; case PMU_TYPE_MCB: if (!(xgene_pmu->mcb_active_mask & pmu->inf->enable_mask)) - goto dev_err; + return -ENODEV; if (xgene_pmu->version == PCP_PMU_V3) pmu->attr_groups = mcb_pmu_v3_attr_groups; else @@ -1185,7 +1184,7 @@ static int xgene_init_perf(struct xgene_pmu_dev *pmu_dev, char *name) break; case PMU_TYPE_MC: if (!(xgene_pmu->mc_active_mask & pmu->inf->enable_mask)) - goto dev_err; + return -ENODEV; if (xgene_pmu->version == PCP_PMU_V3) pmu->attr_groups = mc_pmu_v3_attr_groups; else @@ -1195,19 +1194,14 @@ static int xgene_init_perf(struct xgene_pmu_dev *pmu_dev, char *name) return -EINVAL; } - rc = xgene_init_perf(pmu, ctx->name); - if (rc) { + if (xgene_init_perf(pmu, ctx->name)) { dev_err(dev, "%s PMU: Failed to init perf driver\n", ctx->name); - goto dev_err; + return -ENODEV; } dev_info(dev, "%s PMU registered\n", ctx->name); - return rc; - -dev_err: - devm_kfree(dev, pmu); - return -ENODEV; + return 0; } static void _xgene_pmu_isr(int irq, struct xgene_pmu_dev *pmu_dev) @@ -1515,13 +1509,13 @@ xgene_pmu_dev_ctx *acpi_get_pmu_hw_inf(struct xgene_pmu *xgene_pmu, acpi_dev_free_resource_list(&resource_list); if (rc < 0) { dev_err(dev, "PMU type %d: No resource address found\n", type); - goto err; + return NULL; } dev_csr = devm_ioremap_resource(dev, &res); if (IS_ERR(dev_csr)) { dev_err(dev, "PMU type %d: Fail to map resource\n", type); - goto err; + return NULL; } /* A PMU device node without enable-bit-index is always enabled */ @@ -1535,7 +1529,7 @@ xgene_pmu_dev_ctx *acpi_get_pmu_hw_inf(struct xgene_pmu *xgene_pmu, ctx->name = xgene_pmu_dev_name(dev, type, enable_bit); if (!ctx->name) { dev_err(dev, "PMU type %d: Fail to get device name\n", type); - goto err; + return NULL; } inf = &ctx->inf; inf->type = type; @@ -1543,9 +1537,6 @@ xgene_pmu_dev_ctx *acpi_get_pmu_hw_inf(struct xgene_pmu *xgene_pmu, inf->enable_mask = 1 << enable_bit; return ctx; -err: - devm_kfree(dev, ctx); - return NULL; } static const struct acpi_device_id xgene_pmu_acpi_type_match[] = { @@ -1663,20 +1654,20 @@ xgene_pmu_dev_ctx *fdt_get_pmu_hw_inf(struct xgene_pmu *xgene_pmu, void __iomem *dev_csr; struct resource res; int enable_bit; - int rc; ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL); if (!ctx) return NULL; - rc = of_address_to_resource(np, 0, &res); - if (rc < 0) { + + if (of_address_to_resource(np, 0, &res) < 0) { dev_err(dev, "PMU type %d: No resource address found\n", type); - goto err; + return NULL; } + dev_csr = devm_ioremap_resource(dev, &res); if (IS_ERR(dev_csr)) { dev_err(dev, "PMU type %d: Fail to ma
Re: [RFC PATCH v1 00/11] Create fast idle path for short idle periods
On Thu, Jul 13, 2017 at 11:13:28PM +0800, Li, Aubrey wrote: > On 2017/7/13 22:53, Peter Zijlstra wrote: > > Fixing C-state selection by creating an alternative idle path sounds so > > very wrong. > > This only happens on the arch which has multiple hardware idle cstates, like > Intel's processor. As long as we want to support multiple cstates, we have to > make a selection(with cost of timestamp update and computation). That's fine > in the normal idle path, but if we want a fast idle switch, we can make a > tradeoff to use a low-latency one directly, that's why I proposed a fast idle > path, so that we don't need to mix fast idle condition judgement in both idle > entry and idle exit path. That doesn't make sense. If you can decide to pick a shallow C state in any way, you can fix the general selection too.
Re: [kernel-hardening] Re: [RFC PATCH 6/6] arm64: add VMAP_STACK and detect out-of-bounds SP
On 13 July 2017 at 18:55, Mark Rutland wrote: > On Thu, Jul 13, 2017 at 05:10:50PM +0100, Mark Rutland wrote: >> On Thu, Jul 13, 2017 at 12:49:48PM +0100, Ard Biesheuvel wrote: >> > On 13 July 2017 at 11:49, Mark Rutland wrote: >> > > On Thu, Jul 13, 2017 at 07:58:50AM +0100, Ard Biesheuvel wrote: >> > >> On 12 July 2017 at 23:33, Mark Rutland wrote: > >> > Given that the very first stp in kernel_entry will fault if we have >> > less than S_FRAME_SIZE bytes of stack left, I think we should check >> > that we have at least that much space available. >> >> I was going to reply saying that I didn't agree, but in writing up >> examples, I mostly convinced myself that this is the right thing to do. >> So I mostly agree! >> >> This would mean we treat the first impossible-to-handle exception as >> that fatal case, which is similar to x86's double-fault, triggered when >> the HW can't stack the regs. All other cases are just arbitrary faults. >> >> However, to provide that consistently, we'll need to perform this check >> at every exception boundary, or some of those cases will result in a >> recursive fault first. >> >> So I think there are three choices: >> >> 1) In el1_sync, only check SP bounds, and live with the recursive >>faults. >> >> 2) in el1_sync, check there's room for the regs, and live with the >>recursive faults for overflow on other exceptions. >> >> 3) In all EL1 entry paths, check there's room for the regs. > > FWIW, for the moment I've applied (2), as you suggested, to my > arm64/vmap-stack branch, adding an additional: > > sub x0, x0, #S_FRAME_SIZE > > ... to the entry path. > > I think it's worth trying (3) so that we consistently report these > cases, benchmarks permitting. > OK, so here's a crazy idea: what if we a) carve out a dedicated range in the VMALLOC area for stacks b) for each stack, allocate a naturally aligned window of 2x the stack size, and map the stack inside it, leaving the remaining space unmapped That way, we can compare SP (minus S_FRAME_SIZE) against a mask that is a build time constant, to decide whether its value points into a stack or not. Of course, it may be pointing into the wrong stack, but that should not prevent us from taking the exception, and we can deal with that later. It would give us a very cheap way to perform this test on the hot paths. >> I believe that determining whether the exception was caused by a stack >> overflow is not something we can do robustly or efficiently. >> Actually, if the stack pointer is within S_FRAME_SIZE of the base, and the faulting address points into the guard page, that is a pretty strong indicator that the stack overflowed. That shouldn't be too costly? > It's probably worth putting the fast-path check directly into the > vectors, where we currently only use 1/32 of the instruction slots > available to us. > >> As above, I think it's helpful to think of this as something closer to a >> double-fault handler (i.e. aiming to catch when we can't take the >> exception safely), rather than something that's trying to catch logical >> stack overflows. > > Does this make sense to you? > > I've tried to reword the log output, as below, to give this impression. > > [ 49.288232] Insufficient stack space to handle exception! This could be a separate warning, if we find out that the actual exception was caused by something else. > [ 49.288245] CPU: 5 PID: 2208 Comm: bash Not tainted 4.12.0-5-ga781af2 > #81 > [ 49.300680] Hardware name: ARM Juno development board (r1) (DT) > [ 49.306549] task: 800974955100 task.stack: 0d6f > [ 49.312426] PC is at recursive_loop+0x10/0x50 > [ 49.316747] LR is at recursive_loop+0x34/0x50 > [ 49.321066] pc : [] lr : [] pstate: > 4145 > [ 49.328398] sp : 0d6eff30 > [ 49.331682] x29: 0d6f0350 x28: 800974955100 > [ 49.336953] x27: 08942000 x26: 08f0d758 > [ 49.342223] x25: 0d6f3eb8 x24: 0d6f3eb8 > [ 49.347493] x23: 08f0d490 x22: 0009 > [ 49.352764] x21: 800974a57000 x20: 08f0d4e0 > [ 49.358034] x19: 0013 x18: e7e2e4f0 > [ 49.363304] x17: 9c1256a4 x16: 081f8b88 > [ 49.368574] x15: 2a81b800 x14: fff0 > [ 49.373845] x13: 08f6278a x12: 08e62818 > [ 49.379115] x11: x10: 019e > [ 49.384385] x9 : 0004 x8 : 0d6f0770 > [ 49.389656] x7 : 1313131313131313 x6 : 019e > [ 49.394925] x5 : x4 : > [ 49.400205] x3 : x2 : 0400 > [ 49.405484] x1 : 0013 x0 : 0012 > [ 49.410764] Task stack: [0x0d6f..0x0d6f4000] > [ 49.416728] IRQ stack: [0x80097ffb90a0..0x80097ffbd0a0] > [ 49.422692] ESR: 0x9647 -- DABT (current EL) > [ 49.427277] FAR: 0x0d6eff30 > [ 49.430742] Kernel panic - not synci
Re: [PATCH 02/17] pci: Add a generic, weakly-linked pcibios_align_resource
On Wed, 12 Jul 2017 15:50:42 PDT (-0700), helg...@kernel.org wrote: > On Tue, Jul 11, 2017 at 06:31:15PM -0700, Palmer Dabbelt wrote: >> Multiple architectures define this as trivial function, and I'm adding >> another one as part of the RISC-V port. This adds a __weak version of >> pcibios_align_resource and deletes the now obselete ones in a handful of >> ports. >> >> The only functional change should be that a handful of ports used to >> export pcibios_fixup_bus. Only some architectures export this, so I >> just dropped it. >> >> Signed-off-by: Palmer Dabbelt >> --- >> arch/arc/kernel/pcibios.c| 13 - >> arch/arm64/kernel/pci.c | 17 - >> arch/ia64/pci/pci.c | 7 --- >> arch/microblaze/pci/pci-common.c | 7 --- >> arch/sparc/kernel/leon_pci.c | 6 -- >> arch/sparc/kernel/pci.c | 10 -- >> arch/sparc/kernel/pcic.c | 6 -- >> arch/tile/kernel/pci.c | 10 -- >> arch/tile/kernel/pci_gx.c| 9 - >> drivers/pci/setup-res.c | 12 >> 10 files changed, 12 insertions(+), 85 deletions(-) > > I think you're making your life harder by including these cleanup > patches in your RISC-V support. This patch makes sense (after sorting > out the issues Luis pointed out), but I think the simplest thing to > expedite merging is to add the empty stubs for RISC-V like everybody > else does, then come back after RISC-V gets merged and do the cleanup. > Then the cleanup clearly goes via the PCI tree and isn't entangled > with anything else. Works for me.
Re: [PATCH] sctp: don't dereference ptr before leaving _sctp_walk_{params,errors}()
From: Alexander Potapenko Date: Thu, 13 Jul 2017 20:10:34 +0200 > diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h > index a9519a06a23b..f13632ee33f0 100644 > --- a/include/net/sctp/sctp.h > +++ b/include/net/sctp/sctp.h > @@ -469,6 +469,7 @@ _sctp_walk_params((pos), (chunk), > ntohs((chunk)->chunk_hdr.length), member) > > #define _sctp_walk_params(pos, chunk, end, member)\ > for (pos.v = chunk->member;\ > + pos.v < (void *)chunk + end &&\ > pos.v <= (void *)chunk + end - ntohs(pos.p->length) &&\ > ntohs(pos.p->length) >= sizeof(struct sctp_paramhdr);\ > pos.v += SCTP_PAD4(ntohs(pos.p->length))) > @@ -479,6 +480,7 @@ _sctp_walk_errors((err), (chunk_hdr), > ntohs((chunk_hdr)->length)) > #define _sctp_walk_errors(err, chunk_hdr, end)\ > for (err = (sctp_errhdr_t *)((void *)chunk_hdr + \ > sizeof(struct sctp_chunkhdr));\ > + (void *)err <= (void *)chunk_hdr + end &&\ > (void *)err <= (void *)chunk_hdr + end - ntohs(err->length) &&\ > ntohs(err->length) >= sizeof(sctp_errhdr_t); \ > err = (sctp_errhdr_t *)((void *)err + SCTP_PAD4(ntohs(err->length Even with the "err < ..." fixed in the second hunk, I still think you need to tweak these checks some more. What is necessary is that the first two members of sctp_paramhdr or sctp_errhdr are in the range ptr to end. struct sctp_paramhdr { __be16 type; __be16 length; }; typedef struct sctp_errhdr { __be16 cause; __be16 length; __u8 variable[0]; } sctp_errhdr_t; so that we can legally dereference ->length. But that is not what your new check is doing. Your new check is only making sure there is at least one byte there. I guess it's not easy to write a clean test that doesn't hardcode the offset of the length field and it's size. Something like: pos.v + offsetof(pos.v, length) + sizeof(pos.v->length) <= (void *) chunk + end and (void *)err + offsetof(err, length) + sizeof(err->length) <= (void *) chunk_hdr + end And yeah, that isn't exactly concise nor pretty...
Re: [PATCH 13/21] x86/intel_rdt/cqm: Add cpus file support
On Thu, 6 Jul 2017, Thomas Gleixner wrote: On Thu, 6 Jul 2017, Shivappa Vikas wrote: On Sun, 2 Jul 2017, Thomas Gleixner wrote: + /* Check whether cpus belong to parent ctrl group */ + cpumask_andnot(tmpmask, newmask, &pr->cpu_mask); + if (cpumask_weight(tmpmask)) { + ret = -EINVAL; + goto out; + } + + /* Check whether cpus are dropped from this group */ + cpumask_andnot(tmpmask, &rdtgrp->cpu_mask, newmask); + if (cpumask_weight(tmpmask)) { + /* Give any dropped cpus to parent rdtgroup */ + cpumask_or(&pr->cpu_mask, &pr->cpu_mask, tmpmask); This does not make any sense. The check above verifies that all cpus in newmask belong to the parent->cpu_mask. If they don't then you return -EINVAL, but here you give them back to parent->cpu_mask. How is that supposed to work? You never get into this code path! The parent->cpu_mask always is the parent->cpus_valid_mask if i understand right. With monitor group, the cpu is present is always present in "one" ctrl_mon group and one mon_group. And the mon group can have only cpus in its parent. May be it needs a comment? (its explaind in the documentation patch). Sigh, the code needs to be written in a way that it is halfways obvious what's going on. # mkdir /sys/fs/resctrl/p1 # mkdir /sys/fs/resctrl/p1/mon_groups/m1 # echo 5-10 > /sys/fs/resctr/p1/cpus_list Say p1 has RMID 2 cpus 5-10 have RMID 2 So what you say, is that parent is always the resource control group itself. Can we please have a proper distinction in the code? I tripped over that ambigiousities several times. The normal meaning of parent->child relations is that both have the same type. While this is the case at the implementation detail level (both are type struct rdtgroup), from a conceptual level they are different: parent is a resource group and child is a monitoring group That should be expressed in the code, at the very least by variable naming, so it becomes immediately clear that this operates on two different entities. The proper solution is to have different data types or at least embedd the monitoring bits in a seperate entity inside of struct rdtgroup. Yes they are conceptually different. There were data which were specific to monitoring only but they share a lot of data. So I was still thinking whats best but kept a type which seperates them both. But the monitoring only data seems like only the 'parent' so we can embed the monitoring bits in a seperate struct (The parent is initialized for ctrl_mon group but never really used). Thanks, Vikas
[GIT PULL] VFIO updates for v4.13-rc1
The following changes since commit 32c1431eea4881a6b17bd7c639315010aeefa452: Linux 4.12-rc5 (2017-06-11 16:48:20 -0700) are available in the git repository at: git://github.com/awilliam/linux-vfio.git tags/vfio-v4.13-rc1 for you to fetch changes up to 7f56c30bd0a232822aca38d288da475613bdff9b: vfio: Remove unnecessary uses of vfio_container.group_lock (2017-07-07 15:37:38 -0600) VFIO updates for v4.13-rc1 - Include Intel XXV710 in INTx workaround (Alex Williamson) - Make use of ERR_CAST() for error return (Dan Carpenter) - Fix vfio_group release deadlock from iommu notifier (Alex Williamson) - Unset KVM-VFIO attributes only on group match (Alex Williamson) - Fix release path group/file matching with KVM-VFIO (Alex Williamson) - Remove unnecessary lock uses triggering lockdep splat (Alex Williamson) Alex Williamson (5): vfio/pci: Add Intel XXV710 to hidden INTx devices vfio: Fix group release deadlock kvm-vfio: Decouple only when we match a group vfio: New external user group/file match vfio: Remove unnecessary uses of vfio_container.group_lock Dan Carpenter (1): vfio: Use ERR_CAST() instead of open coding it drivers/vfio/pci/vfio_pci.c | 4 +-- drivers/vfio/vfio.c | 86 - include/linux/vfio.h| 2 ++ virt/kvm/vfio.c | 40 + 4 files changed, 75 insertions(+), 57 deletions(-)
[PATCH] x86/platform/uv/BAU: Fix congested_response_us not taking effect
Bug fix for the BAU tunable congested_cycles not being set to the user defined value. Instead of referencing a global variable when deciding on BAU shutdown, a node will reference its own tunable set value ( cong_response_us). This results in the user set tunable value congested_response_us taking effect correctly. Signed-off-by: Justin Ernst Acked-by: Andrew Banman --- arch/x86/platform/uv/tlb_uv.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/arch/x86/platform/uv/tlb_uv.c b/arch/x86/platform/uv/tlb_uv.c index 2983faa..8ce4657 100644 --- a/arch/x86/platform/uv/tlb_uv.c +++ b/arch/x86/platform/uv/tlb_uv.c @@ -40,7 +40,6 @@ static int timeout_us; static bool nobau = true; static int nobau_perm; -static cycles_t congested_cycles; /* tunables: */ static int max_concurr = MAX_BAU_CONCURRENT; @@ -849,10 +848,10 @@ static void record_send_stats(cycles_t time1, cycles_t time2, if ((completion_status == FLUSH_COMPLETE) && (try == 1)) { bcp->period_requests++; bcp->period_time += elapsed; - if ((elapsed > congested_cycles) && + if ((elapsed > usec_2_cycles(bcp->cong_response_us)) && (bcp->period_requests > bcp->cong_reps) && ((bcp->period_time / bcp->period_requests) > - congested_cycles)) { + usec_2_cycles(bcp->cong_response_us))) { stat->s_congested++; disable_for_period(bcp, stat); } @@ -2247,7 +2246,6 @@ static int __init uv_bau_init(void) } nuvhubs = uv_num_possible_blades(); - congested_cycles = usec_2_cycles(congested_respns_us); uv_base_pnode = 0x7fff; for (uvhub = 0; uvhub < nuvhubs; uvhub++) { -- 1.8.5.6
RE: [PATCH 4.4 29/57] RDMA/uverbs: Check port number supplied by user verbs cmds
> -Original Message- > From: Greg Kroah-Hartman [mailto:gre...@linuxfoundation.org] > Sent: Thursday, July 13, 2017 11:26 AM > To: Ismail, Mustafa > Cc: linux-kernel@vger.kernel.org; linux-r...@vger.kernel.org; > sta...@vger.kernel.org; Yevgeny Kliteynik ; > Tziporet Koren ; Alex Polak > ; Boris Pismenny ; Leon > Romanovsky ; Doug Ledford > Subject: Re: [PATCH 4.4 29/57] RDMA/uverbs: Check port number supplied > by user verbs cmds > > On Thu, Jul 13, 2017 at 03:54:28PM +, Ismail, Mustafa wrote: > > > Subject: [PATCH 4.4 29/57] RDMA/uverbs: Check port number supplied > > > by user verbs cmds > > > > > > 4.4-stable review patch. If anyone has any objections, please let me > know. > > > > Yes, this breaks modify qp. > > See https://patchwork.kernel.org/patch/9830663/ > > I don't understand this response at all, sorry. > > What should I do about this? Is this patch alone a problem? Is there some > other patch I should apply that is in Linus's tree? Where is the problem, > only in this old release? > Applying this patch will break RDMA functionality with respect to modify_qp. Specifically this part: + if (cmd.port_num < rdma_start_port(ib_dev) || + cmd.port_num > rdma_end_port(ib_dev)) + return -EINVAL; + The cmd.port_num is only valid if (cmd->base.attr_mask & IB_QP_PORT). So the above can be fixed with: if ((cmd->base.attr_mask & IB_QP_PORT) && (cmd.port_num < rdma_start_port(ib_dev) || cmd.port_num > rdma_end_port(ib_dev))) return -EINVAL; A version of this patch is in Linus's tree and it has the same problem: commit 5ecce4c9b17bed4dc9cb58bfb10447307569b77b"RDMA/uverbs: Check port number supplied by user verbs cmds" We will be submitting a patch to fix this shortly. Mustafa
Re: [PATCH] Revert "x86/uaccess: Add stack frame output operand in get_user() inline asm"
El Thu, Jul 13, 2017 at 01:00:01PM -0500 Josh Poimboeuf ha dit: > On Wed, Jul 12, 2017 at 04:22:13PM -0700, Matthias Kaehlcke wrote: > > El Wed, Jul 12, 2017 at 05:36:30PM -0500 Josh Poimboeuf ha dit: > > > > > On Wed, Jul 12, 2017 at 05:35:47PM -0500, Josh Poimboeuf wrote: > > > > On Wed, Jul 12, 2017 at 03:20:40PM -0700, Matthias Kaehlcke wrote: > > > > > > This is admittedly an awkward way of achieving this goal, but it's > > > > > > the > > > > > > only way I know how to do it with GCC. > > > > > > > > > > > > What extra instruction does clang add? > > > > > > > > > > I was looking at the get_user() call in drm_mode_setcrtc(). The code > > > > > generated by clang without the patch is: > > > > > > > > > > if (get_user(out_id, &set_connectors_ptr[i])) > > > > > { > > > > > 81386955: 4a 8d 04 bd 00 00 00lea > > > > > 0x0(,%r15,4),%rax > > > > > 8138695c: 00 > > > > > 8138695d: 49 03 06add(%r14),%rax > > > > > 81386960: e8 2b a5 f0 ff callq > > > > > 81290e90 <__get_user_4> > > > > > > > > > > And with the patch: > > > > > > > > > > if (get_user(out_id, &set_connectors_ptr[i])) > > > > > { > > > > > 81386a56: 4a 8d 04 bd 00 00 00lea > > > > > 0x0(,%r15,4),%rax > > > > > 81386a5d: 00 > > > > > 81386a5e: 49 03 06add(%r14),%rax > > > > > 81386a61: 48 8b 64 24 28 mov0x28(%rsp),%rsp > > > > > 81386a66: e8 15 a5 f0 ff callq > > > > > 81290f80 <__get_user_4> > > > > > > > > Hm, that seems odd. Can you sure the disassembly for the whole > > > > function? > > > > > > Er, share :-) > > > > Sure, please find below the disassemblies with and without the > > patch. The exact extra instruction differs from the one above, the > > disassembly above is from a debug session with some 'random' kernel > > version (bisect), the ones below from a v4.12ish kernel. At the bottom > > you also find a log of a double faults observed with the patch. > > > > If you are interested in building the kernel with clang yourself I can > > provide instructions, it is fairly painless nowadays as long as you > > have a recent version of clang (a somewhat older version should also > > do for this issue with some extra kernel patches). > > Here's the reason for the double fault. First it puts zero on the stack > at offset -0x58: > > > 81367616: 31 c0 xor%eax,%eax > > 81367618: 48 89 45 c8 mov%rax,-0x38(%rbp) > > 8136761c: 45 31 ffxor%r15d,%r15d > > 8136761f: 48 89 45 a8 mov%rax,-0x58(%rbp) > > Then, later, it copies that zeroed word from the stack to RSP: > > > 81367874: 48 8b 65 a8 mov-0x58(%rbp),%rsp > > Then it double faults because the call instruction tries to write RIP on > the stack, but RSP is zero: > > > 81367878: e8 73 26 f1 ff callq 81279ef0 > > <__get_user_4> > > Then clang tries to put RSP's value on the stack, at the same stack slot > where the original zero was stored (though it never reaches this point): > > > 8136787d: 49 89 d4mov%rdx,%r12 > > 81367880: 48 89 65 a8 mov%rsp,-0x58(%rbp) > > The panic is consistent with the above. RIP points to the call > instruction, RSP is zero: > > > [3.798722] PANIC: double fault, error_code: 0x0 > > [3.807387] CPU: 1 PID: 605 Comm: frecon Not tainted > > 4.12.0-00023-g711d82c128ff #107 > > [3.816040] Hardware name: GOOGLE Squawks, BIOS > > Google_Squawks.5216.152.76 03/04/2016 > > [3.824792] task: 880075b92f00 task.stack: c9d6c000 > > [3.829599] EXT4-fs (mmcblk0p1): re-mounted. Opts: > > commit=600,data=ordered > > [3.839092] RIP: 0010:drm_mode_setcrtc+0x328/0x51f > > [3.83] RSP: 0018: EFLAGS: 00010206 > > [3.850280] RAX: 559e707c4d60 RBX: RCX: > > 0008 > > [3.858253] RDX: 0001 RSI: c9d6fcc8 RDI: > > 81367805 > > [3.866225] RBP: c9d6fd90 R08: 014000c0 R09: > > 0308 > > [3.874199] R10: 0300 R11: 0556 R12: > > > > [3.882163] R13: 880077a25000 R14: c9d6fdd0 R15: > > > > [3.890136] FS: 7fb2dbd62740() GS:88007ad0() > > knlGS: > > [3.899177] CS: 0010 DS: ES: CR0: 80050033 > > [3.905595] CR2: fff8 CR3: 7596b000 CR4: > > 001006e0 > > [3.913568] Call Trace: > > [3.916296] Code: b8 48 8d b5 38 ff ff ff 41 8b 56 08 39 d3 0f 83 85 00 > > 00 00 4c 63 fb 4a 83 24 f8 00 4a 8d 04 bd 00 00 00 00 49 03 06 48 8b 65 a8 > > 73 26 f1 ff 49 89
Re: [PATCH v2 2/2] docs: disable KASLR when debugging kernel
On Thu, 13 Jul 2017 08:53:59 +0200 Jan Kiszka wrote: > FWIW: Acked-by: Jan Kiszka > > Who's taking this? Jon? I can take them once the merge window closes. Thanks, jon
[PATCH 0/3] Extend BGMAC driver for Stingray SoC
The patchset extends Broadcom BGMAC driver for Broadcom Stingray SoC. This patchset is based on Linux-4.12 and tested on NS2 and Stingray. Abhishek Shah (3): net: ethernet: bgmac: Remove unnecessary 'return' from platform_bgmac_idm_write net: ethernet: bgmac: Make IDM register space optional in BGMAC driver Documentation: devicetree: net: optional idm regs for bgmac .../devicetree/bindings/net/brcm,amac.txt | 1 + drivers/net/ethernet/broadcom/bgmac-platform.c | 21 --- drivers/net/ethernet/broadcom/bgmac.c | 70 +- drivers/net/ethernet/broadcom/bgmac.h | 1 + 4 files changed, 57 insertions(+), 36 deletions(-) -- 2.7.4
[PATCH 1/3] net: ethernet: bgmac: Remove unnecessary 'return' from platform_bgmac_idm_write
Return type for idm register write callback should be void as 'writel' API is used for write operation. However, there no need to have 'return' in this function. Signed-off-by: Abhishek Shah Reviewed-by: Oza Oza Reviewed-by: Ray Jui Reviewed-by: Scott Branden --- drivers/net/ethernet/broadcom/bgmac-platform.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/broadcom/bgmac-platform.c b/drivers/net/ethernet/broadcom/bgmac-platform.c index 73aca97..1ca75de 100644 --- a/drivers/net/ethernet/broadcom/bgmac-platform.c +++ b/drivers/net/ethernet/broadcom/bgmac-platform.c @@ -50,7 +50,7 @@ static u32 platform_bgmac_idm_read(struct bgmac *bgmac, u16 offset) static void platform_bgmac_idm_write(struct bgmac *bgmac, u16 offset, u32 value) { - return writel(value, bgmac->plat.idm_base + offset); + writel(value, bgmac->plat.idm_base + offset); } static bool platform_bgmac_clk_enabled(struct bgmac *bgmac) -- 2.7.4
[PATCH 3/3] Documentation: devicetree: net: optional idm regs for bgmac
Specifying IDM register space in DT is not mendatory for SoCs where firmware takes care of IDM operations. This patch updates BGMAC driver's DT binding documentation indicating the same. Signed-off-by: Abhishek Shah Reviewed-by: Ray Jui Reviewed-by: Oza Oza Reviewed-by: Scott Branden --- Documentation/devicetree/bindings/net/brcm,amac.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/net/brcm,amac.txt b/Documentation/devicetree/bindings/net/brcm,amac.txt index 2fefa1a..ad16c1f 100644 --- a/Documentation/devicetree/bindings/net/brcm,amac.txt +++ b/Documentation/devicetree/bindings/net/brcm,amac.txt @@ -11,6 +11,7 @@ Required properties: - reg-names: Names of the registers. "amac_base":Address and length of the GMAC registers "idm_base": Address and length of the GMAC IDM registers + (required for NSP and Northstar2) "nicpm_base": Address and length of the NIC Port Manager registers (required for Northstar2) - interrupts: Interrupt number -- 2.7.4
[PATCH 2/3] net: ethernet: bgmac: Make IDM register space optional
IDM operations are usually one time ops and should be done in firmware itself. Driver is not supposed to touch IDM registers. However, for some SoCs', driver is performing IDM read/writes. So this patch masks IDM operations in case firmware is taking care of IDM operations. Signed-off-by: Abhishek Shah Reviewed-by: Oza Oza Reviewed-by: Ray Jui Reviewed-by: Scott Branden --- drivers/net/ethernet/broadcom/bgmac-platform.c | 19 --- drivers/net/ethernet/broadcom/bgmac.c | 70 +++--- drivers/net/ethernet/broadcom/bgmac.h | 1 + 3 files changed, 55 insertions(+), 35 deletions(-) diff --git a/drivers/net/ethernet/broadcom/bgmac-platform.c b/drivers/net/ethernet/broadcom/bgmac-platform.c index 1ca75de..d937083d 100644 --- a/drivers/net/ethernet/broadcom/bgmac-platform.c +++ b/drivers/net/ethernet/broadcom/bgmac-platform.c @@ -55,6 +55,9 @@ static void platform_bgmac_idm_write(struct bgmac *bgmac, u16 offset, u32 value) static bool platform_bgmac_clk_enabled(struct bgmac *bgmac) { + if (!bgmac->plat.idm_base) + return true; + if ((bgmac_idm_read(bgmac, BCMA_IOCTL) & BGMAC_CLK_EN) != BGMAC_CLK_EN) return false; if (bgmac_idm_read(bgmac, BCMA_RESET_CTL) & BCMA_RESET_CTL_RESET) @@ -66,6 +69,9 @@ static void platform_bgmac_clk_enable(struct bgmac *bgmac, u32 flags) { u32 val; + if (!bgmac->plat.idm_base) + return; + /* The Reset Control register only contains a single bit to show if the * controller is currently in reset. Do a sanity check here, just in * case the bootloader happened to leave the device in reset. @@ -180,6 +186,7 @@ static int bgmac_probe(struct platform_device *pdev) bgmac->feature_flags |= BGMAC_FEAT_CMDCFG_SR_REV4; bgmac->feature_flags |= BGMAC_FEAT_TX_MASK_SETUP; bgmac->feature_flags |= BGMAC_FEAT_RX_MASK_SETUP; + bgmac->feature_flags |= BGMAC_FEAT_IDM_MASK; bgmac->dev = &pdev->dev; bgmac->dma_dev = &pdev->dev; @@ -207,15 +214,13 @@ static int bgmac_probe(struct platform_device *pdev) return PTR_ERR(bgmac->plat.base); regs = platform_get_resource_byname(pdev, IORESOURCE_MEM, "idm_base"); - if (!regs) { - dev_err(&pdev->dev, "Unable to obtain idm resource\n"); - return -EINVAL; + if (regs) { + bgmac->plat.idm_base = devm_ioremap_resource(&pdev->dev, regs); + if (IS_ERR(bgmac->plat.idm_base)) + return PTR_ERR(bgmac->plat.idm_base); + bgmac->feature_flags &= ~BGMAC_FEAT_IDM_MASK; } - bgmac->plat.idm_base = devm_ioremap_resource(&pdev->dev, regs); - if (IS_ERR(bgmac->plat.idm_base)) - return PTR_ERR(bgmac->plat.idm_base); - regs = platform_get_resource_byname(pdev, IORESOURCE_MEM, "nicpm_base"); if (regs) { bgmac->plat.nicpm_base = devm_ioremap_resource(&pdev->dev, diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c index ba4d2e1..48d672b 100644 --- a/drivers/net/ethernet/broadcom/bgmac.c +++ b/drivers/net/ethernet/broadcom/bgmac.c @@ -622,9 +622,11 @@ static int bgmac_dma_alloc(struct bgmac *bgmac) BUILD_BUG_ON(BGMAC_MAX_TX_RINGS > ARRAY_SIZE(ring_base)); BUILD_BUG_ON(BGMAC_MAX_RX_RINGS > ARRAY_SIZE(ring_base)); - if (!(bgmac_idm_read(bgmac, BCMA_IOST) & BCMA_IOST_DMA64)) { - dev_err(bgmac->dev, "Core does not report 64-bit DMA\n"); - return -ENOTSUPP; + if (!(bgmac->feature_flags & BGMAC_FEAT_IDM_MASK)) { + if (!(bgmac_idm_read(bgmac, BCMA_IOST) & BCMA_IOST_DMA64)) { + dev_err(bgmac->dev, "Core does not report 64-bit DMA\n"); + return -ENOTSUPP; + } } for (i = 0; i < BGMAC_MAX_TX_RINGS; i++) { @@ -855,9 +857,11 @@ static void bgmac_mac_speed(struct bgmac *bgmac) static void bgmac_miiconfig(struct bgmac *bgmac) { if (bgmac->feature_flags & BGMAC_FEAT_FORCE_SPEED_2500) { - bgmac_idm_write(bgmac, BCMA_IOCTL, - bgmac_idm_read(bgmac, BCMA_IOCTL) | 0x40 | - BGMAC_BCMA_IOCTL_SW_CLKEN); + if (!(bgmac->feature_flags & BGMAC_FEAT_IDM_MASK)) { + bgmac_idm_write(bgmac, BCMA_IOCTL, + bgmac_idm_read(bgmac, BCMA_IOCTL) | + 0x40 | BGMAC_BCMA_IOCTL_SW_CLKEN); + } bgmac->mac_speed = SPEED_2500; bgmac->mac_duplex = DUPLEX_FULL; bgmac_mac_speed(bgmac); @@ -874,11 +878,36 @@ static void bgmac_miiconfig(struct bgmac *bgmac) } } +static void bgmac_chip_reset_idm_config(struct bgmac *bgmac) +{ + u32 iost; + + iost = bgmac_idm_read(bgmac, BCMA_IOST); +
[GIT PULL] platform-drivers-x86 for 4.13-2
Hi Linus, I'm following your advice from: https://www.spinics.net/lists/kernel/msg2492566.html and including a cherry-picked patch from my previous 'fixes' pull request, rather than merging the 'fixes' branch into the development branches. I'm going to keep an eye on these types of commits going forward as it seems merging my 'fixes' pull request tags into my 'testing' and 'for-next' branches once you merge them might actually be cleaner - but I need to verify this wouldn't pull in additional changes as you warned against in the above thread. So for now, I'll cherry-pick and add a note in the tag message explaining it. The following changes since commit d791db9a57ab7f390916dce0fa1315130bb6664c: platform/x86: sony-laptop: constify attribute_group and input index array (2017-06-30 20:26:54 -0700) are available in the git repository at: git://git.infradead.org/linux-platform-drivers-x86.git tags/platform-drivers-x86-v4.13-2 for you to fetch changes up to c3a73ed8a82b666ac01466d3badd7824eae89c44: platform/x86: silead_dmi: Add entry for Ployer Momo7w tablet touchscreen (2017-07-12 13:57:42 -0700) Thanks, Darren Hart VMware Open Source Technology Center platform-drivers-x86 for v4.13-2 Add new platform matches for silead_dmi and ideapad-laptop. Several constify patches for attribute_group structures. Fixes for peaq-wmi and intel_telemetry*. *Note: 74a1eb565c platform/x86: intel_telemetry_debugfs: fix oops when load/unload module is a duplicate cherry-picked from the 4.12 RC cycle (platform-drivers-x86-v4.12-2). silead_dmi: - Add entry for Ployer Momo7w tablet touchscreen - Add touchscreen info for I.T.Works TW891 2-in-1 toshiba_acpi: - constify attribute_group structures. asus-wmi: - constify attribute_group structures. panasonic-laptop: - constify attribute_group structures. alienware-wmi: - constify attribute_group structures. samsung-laptop: - constify attribute_group structures. compal-laptop: - constify attribute_group structures. fujitsu-laptop: - constify attribute_group structures. - add NULL check on devm_kzalloc() return value peaq-wmi: - Fix peaq_ignore_events_counter handling off by 1 ideapad-laptop: - Fix indentation in DMI table - Add several models to no_hw_rfkill - Add IdeaPad V510-15IKB to no_hw_rfkill intel_telemetry: - Add debugfs entry for S0ix residency intel_telemetry_debugfs: - fix some error codes in init - fix oops when load/unload module Andy Shevchenko (1): platform/x86: ideapad-laptop: Fix indentation in DMI table Arvind Yadav (7): platform/x86: fujitsu-laptop: constify attribute_group structures. platform/x86: compal-laptop: constify attribute_group structures. platform/x86: samsung-laptop: constify attribute_group structures. platform/x86: alienware-wmi: constify attribute_group structures. platform/x86: panasonic-laptop: constify attribute_group structures. platform/x86: asus-wmi: constify attribute_group structures. platform/x86: toshiba_acpi: constify attribute_group structures. Dan Carpenter (1): platform/x86: intel_telemetry_debugfs: fix some error codes in init Gustavo A. R. Silva (1): platform/x86: fujitsu-laptop: add NULL check on devm_kzalloc() return value Hans de Goede (3): platform/x86: silead_dmi: Add touchscreen info for I.T.Works TW891 2-in-1 platform/x86: peaq-wmi: Fix peaq_ignore_events_counter handling off by 1 platform/x86: silead_dmi: Add entry for Ployer Momo7w tablet touchscreen Priyalee Kushwaha (1): platform/x86: intel_telemetry_debugfs: fix oops when load/unload module Rajneesh Bhardwaj (1): platform/x86: intel_telemetry: Add debugfs entry for S0ix residency Sven Eckelmann (1): platform/x86: ideapad-laptop: Add IdeaPad V510-15IKB to no_hw_rfkill Yang Jiaxun (1): platform/x86: ideapad-laptop: Add several models to no_hw_rfkill drivers/platform/x86/alienware-wmi.c | 6 +- drivers/platform/x86/asus-wmi.c| 4 +- drivers/platform/x86/compal-laptop.c | 2 +- drivers/platform/x86/fujitsu-laptop.c | 14 - drivers/platform/x86/ideapad-laptop.c | 85 -- drivers/platform/x86/intel_telemetry_debugfs.c | 41 ++--- drivers/platform/x86/panasonic-laptop.c| 2 +- drivers/platform/x86/peaq-wmi.c| 2 +- drivers/platform/x86/samsung-laptop.c | 2 +- drivers/platform/x86/silead_dmi.c | 32 ++ drivers/platform/x86/toshiba_acpi.c| 2 +- 11 files changed, 168 insertions(+), 24 deletions(-) -- Darren Hart VMware Open Source Technology Center
Re: [PATCH] mm/mremap: Fail map duplication attempts for private mappings
[+CC linux-api] On 07/13/2017 05:58 PM, Mike Kravetz wrote: > mremap will create a 'duplicate' mapping if old_size == 0 is > specified. Such duplicate mappings make no sense for private > mappings. If duplication is attempted for a private mapping, > mremap creates a separate private mapping unrelated to the > original mapping and makes no modifications to the original. > This is contrary to the purpose of mremap which should return > a mapping which is in some way related to the original. > > Therefore, return EINVAL in the case where if an attempt is > made to duplicate a private mapping. > > Signed-off-by: Mike Kravetz Acked-by: Vlastimil Babka > --- > mm/mremap.c | 7 +++ > 1 file changed, 7 insertions(+) > > diff --git a/mm/mremap.c b/mm/mremap.c > index cd8a1b1..076f506 100644 > --- a/mm/mremap.c > +++ b/mm/mremap.c > @@ -383,6 +383,13 @@ static struct vm_area_struct *vma_to_resize(unsigned > long addr, > if (!vma || vma->vm_start > addr) > return ERR_PTR(-EFAULT); > > + /* > + * !old_len is a special case where a mapping is 'duplicated'. > + * Do not allow this for private mappings. > + */ > + if (!old_len && !(vma->vm_flags & (VM_SHARED | VM_MAYSHARE))) > + return ERR_PTR(-EINVAL); > + > if (is_vm_hugetlb_page(vma)) > return ERR_PTR(-EINVAL); > >
Re: [PATCH v5 2/6] clk: sunxi-ng: Add sun4i/sun7i CCU driver
On Sun, Jul 09, 2017 at 10:25:23PM +1000, Jonathan Liu wrote: > Hi Priit, > > On 5 July 2017 at 06:04, Priit Laes wrote: > > Introduce a clock controller driver for sun4i A10 and sun7i A20 > > series SoCs. > > > > Signed-off-by: Priit Laes > > --- > > drivers/clk/sunxi-ng/Kconfig | 14 +- > > drivers/clk/sunxi-ng/Makefile |1 +- > > drivers/clk/sunxi-ng/ccu-sun4i-a10.c | 1448 ++- > > drivers/clk/sunxi-ng/ccu-sun4i-a10.h | 61 +- > > include/dt-bindings/clock/sun4i-a10-ccu.h | 200 +++- > > include/dt-bindings/clock/sun7i-a20-ccu.h | 53 +- > > include/dt-bindings/reset/sun4i-a10-ccu.h | 67 +- > > 7 files changed, 1844 insertions(+) > > create mode 100644 drivers/clk/sunxi-ng/ccu-sun4i-a10.c > > create mode 100644 drivers/clk/sunxi-ng/ccu-sun4i-a10.h > > create mode 100644 include/dt-bindings/clock/sun4i-a10-ccu.h > > create mode 100644 include/dt-bindings/clock/sun7i-a20-ccu.h > > create mode 100644 include/dt-bindings/reset/sun4i-a10-ccu.h > > > [snip] > > diff --git a/drivers/clk/sunxi-ng/ccu-sun4i-a10.c > > b/drivers/clk/sunxi-ng/ccu-sun4i-a10.c > > new file mode 100644 > > index 000..49052b7 > > --- /dev/null > > +++ b/drivers/clk/sunxi-ng/ccu-sun4i-a10.c > [snip] > > > +static const char *const hdmi_parents[] = { "pll-video0", "pll-video0-2x", > > + "pll-video1", "pll-video1-2x" }; > > +static SUNXI_CCU_M_WITH_MUX_GATE(hdmi_clk, "hdmi", hdmi_parents, > > +0x150, 0, 4, 24, 2, BIT(31), 0); > > hdmi_parents is in the wrong order. The correct order is "pll-video0", > "pll-video1", "pll-video0-2x", "pll-video1-2x". Ugh.. I'm really sorry. Päikest, Priit
Re: [PATCH v2] xattr: Enable security.capability in user namespaces
On Thu, Jul 13, 2017 at 12:39:10PM -0500, Eric W. Biederman wrote: > > Can you define what 'scalable' means for you in this context? > > From what I can see sharing a filesystem between multiple containers > > doesn't 'scale well' for virtualizing the xattrs primarily because of > > size limitations of xattrs per file. > > Worse than that I believe you will find that filesystems are built on > the assumption that there will be a small number of xattrs per file. > So even if the vfs limitations were lifted the filesystem performance > would suffer. That's why I've been pushing here. If people try to do security.capable@uid=1000 security.capable@uid=2000 security.capable@uid=3000 security.capable@uid=4000 security.capable@uid=5000 security.capable@uid=6000 security.capable@uid=7000 security.capable@uid=8000 security.capable@uid=9000 . . . ... where the values of all of these will be the same, this is going to be *awful* even if the file system can support it. So maybe we are better off if we define an xattr security.capable@guest-container ... so the property is that it is ignored by the host ("real") container, and in all of the subcontainers, it will be used if the local container root is trying to execute the file. Now, this doesn't support the solution of the "turtles all the way down" insane containers configuraiton. E.g., where in one container we boot a RHEL distro, which then launches another container running another RHEL distro, and repeat this 1000 times, for a very deeply nested subsubsubsubsub...container. I think this is insane and we shouldn't support this, but I know there are people who think this is a perfectly sane thing to do. The other thing this doesn't support is someone who wants to use IMA, and where the every single IMA is using a different signed HMAC: security.ima@uid=1000 security.ima@uid=2000 security.ima@uid=3000 security.ima@uid=4000 security.ima@uid=5000 security.ima@uid=6000 security.ima@uid=7000 security.ima@uid=8000 security.ima@uid=9000 . . . Where each security IMA could either be a 32 byte HMAC, or worse, a 256 byte RSA signed signature. Now let's assume there are 10,000 containers, each of which needs a separate RSA signature. This sounds insane but I've seen things that I've thought were more insane coming out of containerland, so it would be nice if we can get something signed in blood promisng that no, we would *never* do something that insane, or better yet, make it impossible to do from an architectural standpoint. - Ted
Re: [RFC 0/2] arm-smmu-v3 tlbi-on-map option
On 13/07/17 18:44, Michael S. Tsirkin wrote: > On Thu, Jul 13, 2017 at 10:29:42AM +0100, Jean-Philippe Brucker wrote: >> On 12/07/17 23:07, Michael S. Tsirkin wrote: >> [...] >>> I think using hardware support for nesting is the right final >>> solution. It will take some time though. Given this, what should >>> we do meanwhile? >>> >>> Assuming that's the final destination, a simple quirk like this >>> seems to be preferable to virtio-iommu which we'll never be >>> able to offload to hardware. >> >> That's not entirely true. virtio-iommu will have an extension for hardware >> nesting support. It was presented in my initial proposal, and I've made >> significant progress since then. >> >> Thanks, >> Jean > > I don't recall seeing this. > > Hardware specific extensions to virtio would be interesting, the > difficulty is in finding the balance between enabling minor quirks and > each vendor going their own way. Yes, in order to avoid too many quirks and vendor-specific formats, we'd like the guest to only manage page tables, which have relatively stable format, while the host kernel manages context tables (PASID tables) and other structures, that are more volatile across implementations. Unavoidably, a few architecture-specific details need to be described in the API, but it seems manageable. The host tells the guest which page table format is supported by hardware, and the guest sends back a page directory pointer along with a few configuration parameters. In the guest, page table operations may be abstracted in a module and used by multiple IOMMU drivers (what ARM does in drivers/iommu/io-pgtable-arm.c for the various vendor drivers). This abstraction is quite helpful to find out which information needs to be exchanged between virtio-iommu device and driver. For ARM-based IOMMUs it amounts to 6 configuration registers and 4 quirk bits. The other problem is forwarding TLB invalidations to the host kernel. There is an ongoing discussion [1] about the best way to do it in VFIO, and it seems like the format can stay mostly generic, with a few optimization hints depending on the hardware IOMMU. > Is this the proposal you refer to? > https://www.spinics.net/lists/kvm/msg147990.html Yes, I'm referring to "II. Page table sharing" in https://www.spinics.net/lists/kvm/msg147993.html But it is now mostly obsolete. Lots of things had to change while writing a prototype and following public discussions. At the moment my focus is on tidying up the base, but I will send another RFC afterwards. Thanks, Jean [1] https://lists.gnu.org/archive/html/qemu-devel/2017-07/msg01117.html
[PATCH] mtd: gpmi-nand: do not fail setting ONFI timing mode if available
GPMI NFC driver fails to apply timing mode if the ->onfi_get_features() does not return the mode that was previously applied. We can assume that a nand chip supports a timing as long as it is read from the ONFI parameter page. Reading back a different mode than the one previously applied does not mean the mode is unsupported but that the nand chip does not implement the ONFI feature because it probably does not need to. The output of ->onfi_get_feature() is irrelevant so delete it. Signed-off-by: Miquel Raynal --- drivers/mtd/nand/gpmi-nand/gpmi-lib.c | 7 --- 1 file changed, 7 deletions(-) diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-lib.c b/drivers/mtd/nand/gpmi-nand/gpmi-lib.c index 141bd70a49c2..4d137145439c 100644 --- a/drivers/mtd/nand/gpmi-nand/gpmi-lib.c +++ b/drivers/mtd/nand/gpmi-nand/gpmi-lib.c @@ -939,13 +939,6 @@ static int enable_edo_mode(struct gpmi_nand_data *this, int mode) if (ret) goto err_out; - /* [2] send GET FEATURE command to double-check the timing mode */ - memset(feature, 0, ONFI_SUBFEATURE_PARAM_LEN); - ret = nand->onfi_get_features(mtd, nand, - ONFI_FEATURE_ADDR_TIMING_MODE, feature); - if (ret || feature[0] != mode) - goto err_out; - nand->select_chip(mtd, -1); /* [3] set the main IO clock, 100MHz for mode 5, 80MHz for mode 4. */ -- 2.11.0
Re: [linux-sunxi] [PATCH v5 2/6] clk: sunxi-ng: Add sun4i/sun7i CCU driver
On Mon, Jul 10, 2017 at 11:45:32AM +0200, Olliver Schinagl wrote: > Hi Pleas, > > again, but this time with content :) > > On 04-07-17 22:04, Priit Laes wrote: > >Introduce a clock controller driver for sun4i A10 and sun7i A20 > >series SoCs. [ ... ] > >+++ b/drivers/clk/sunxi-ng/Kconfig > >@@ -11,6 +11,19 @@ config SUN50I_A64_CCU > > default ARM64 && ARCH_SUNXI > > depends on (ARM64 && ARCH_SUNXI) || COMPILE_TEST > > > >+config SUNXI_A10_CCU > I understand why you say sunXi here (it's support for both sun4i and sun7i) > but then why A10, as it also supports the A20. > > I guess the CCU is identical on the A20 and the A10, right? Thus would it > not be sensible to just call it sun4i_ccu (like we do for sun5i_ccu below? No, it's not identical. > >+bool "Support for the Allwinner A10/A20 CCU" > >+select SUNXI_CCU_DIV > >+select SUNXI_CCU_MULT > >+select SUNXI_CCU_NK > >+select SUNXI_CCU_NKM > >+select SUNXI_CCU_NM > >+select SUNXI_CCU_MP > >+select SUNXI_CCU_PHASE > >+default MACH_SUN4I > >+default MACH_SUN7I > >+depends on MACH_SUN4I || MACH_SUN7I || COMPILE_TEST > >+ > > config SUN5I_CCU > > bool "Support for the Allwinner sun5i family CCM" > > default MACH_SUN5I > >@@ -57,4 +70,5 @@ config SUN8I_R_CCU > > bool "Support for Allwinner SoCs' PRCM CCUs" > > default MACH_SUN8I || (ARCH_SUNXI && ARM64) > > > >+ > oops? OK > > >new file mode 100644 > >index 000..49052b7 > >--- /dev/null > >+++ b/drivers/clk/sunxi-ng/ccu-sun4i-a10.c > > > > >+static const char *const apb1_parents[] = { "hosc", "pll-periph", "osc32k" > >}; > >+static SUNXI_CCU_MP_WITH_MUX(apb1_clk, "apb1", apb1_parents, 0x058, > >+ 0, 5, /* M */ > >+ 16, 2, /* P */ > >+ 24, 2, /* mux */ > >+ 0); > >+ > >+/* Not present on A20 */ > >+static SUNXI_CCU_GATE(axi_dram_clk, "axi-dram", "ahb", > >+ 0x05c, BIT(31), 0); > > Same here I guess, two defines make this a bit more readable. You mean SUN4I_CCU_GATE? and SUN7I_CCU_GATE defines? I don't think it makes things more readable... > >+ > >+static SUNXI_CCU_GATE(ahb_otg_clk, "ahb-otg", "ahb", ... > >+ 0x060, BIT(14), CLK_IS_CRITICAL); > > > > >+static struct ccu_reset_map sun7i_a20_ccu_resets[] = { > >+[RST_USB_PHY0] = { 0x0cc, BIT(0) }, > >+[RST_USB_PHY1] = { 0x0cc, BIT(1) }, > >+[RST_USB_PHY2] = { 0x0cc, BIT(2) }, > >+[RST_GPS] = { 0x0d0, BIT(0) }, > >+[RST_DE_BE0]= { 0x104, BIT(30) }, > >+[RST_DE_BE1]= { 0x108, BIT(30) }, > >+[RST_DE_FE0]= { 0x10c, BIT(30) }, > >+[RST_DE_FE1]= { 0x110, BIT(30) }, > >+[RST_DE_MP] = { 0x114, BIT(30) }, > >+[RST_TCON0] = { 0x118, BIT(30) }, > >+[RST_TCON1] = { 0x11c, BIT(30) }, > You are missing the TV encoder reset: > + [RST_TVE0] = { 0x118, BIT(29) }, > + [RST_TVE1] = { 0x11c, BIT(29) }, > > (to match your table i did not use defines :p) Where did you get this information? This is not present in any datasheets I have: * A10 - 1.50 * A20 - 1.4 [...] Päikest, Priit Laes
Re: Potential scheduler regression
On Tue, Jul 11, 2017 at 5:55 AM, Greg KH wrote: > On Tue, Jul 11, 2017 at 10:30:14AM +0200, Ingo Molnar wrote: >> >> * Ben Guthro wrote: >> >> > > If people have experience with these in the "enterprise" distros, or any >> > > other >> > > tree, and want to provide me with backported, and tested, patches, I'll >> > > be >> > > glad to consider them for stable kernels. >> > > >> > > thanks, >> > > >> > > greg k-h >> > >> > I tried to do a simple cherry-pick of the suggested patches - but they >> > apply against files that don't exist in the 4.9 series. >> >> I think there are only two strategies to maintain a backport which work in >> the >> long run: >> >> - insist on the simplest fixes and pure cherry-picks >> >> - or pick up _everything_ to sync up the two versions. >> >> The latter would mean a lot of commits - and I'm afraid it would also >> involve the >> scheduler header split-up, which literally involves hundreds of files plus >> perpetual build-breakage risk, so it's a no-no. >> >> > In my release of 4.9 - I'm planning on doing the simpler revert of >> > 1b568f0aab >> > that introduced the performance degradation, rather than pulling in lots >> > of code >> > from newer kernels. >> >> That sounds much saner - I'd even Ack that approach for -stable as a special >> exception, than to complicate things with excessive backports. > > Ok, I'll revert that for the next stable release after this one that is > currently under review. > > thanks, > > greg k-h Greg, Just for clarity - is the "next one" 4.9.38 (posted today for review) - or the one following? Thanks, Ben
Re: [PATCH] Revert "x86/uaccess: Add stack frame output operand in get_user() inline asm"
On Thu, Jul 13, 2017 at 11:47:48AM -0700, Matthias Kaehlcke wrote: > > What happens if you try the below patch instead of the revert? Any > > chance the offending instruction goes away? > > > > diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h > > index 11433f9..beac907 100644 > > --- a/arch/x86/include/asm/uaccess.h > > +++ b/arch/x86/include/asm/uaccess.h > > @@ -171,7 +171,7 @@ __typeof__(__builtin_choose_expr(sizeof(x) > > > sizeof(0UL), 0ULL, 0UL)) > > might_fault(); \ > > asm volatile("call __get_user_%P4" \ > > : "=a" (__ret_gu), "=r" (__val_gu), "+r" (__sp)\ > > -: "0" (ptr), "i" (sizeof(*(ptr;\ > > +: "0" (ptr), "i" (sizeof(*(ptr))), "r" (__sp));\ > > (x) = (__force __typeof__(*(ptr))) __val_gu;\ > > __builtin_expect(__ret_gu, 0); \ > > }) > > The generated code is basically the same, only that now the value from > the stack is stored in a register and written twice to RSP: > > 813676ba: 31 c0 xor%eax,%eax > 813676bc: 48 89 45 c8 mov%rax,-0x38(%rbp) > 813676c0: 45 31 ffxor%r15d,%r15d > 813676c3: 48 89 45 a8 mov%rax,-0x58(%rbp) > ... > 81367918: 48 8b 4d a8 mov-0x58(%rbp),%rcx > 8136791c: 48 89 ccmov%rcx,%rsp > 8136791f: 48 89 ccmov%rcx,%rsp > 81367922: e8 69 26 f1 ff callq 81279f90 > <__get_user_4> LOL. Why corrupt the stack pointer with a single instruction (reading a zero from memory, no less) when you can instead do it with three instructions, including two duplicates? Anyway this seems like a clang bug to me. If I specify RSP as an input register then the compiler shouldn't overwrite it first. For that matter it has no reason to overwrite it if it's an output register either. -- Josh