* Re: [PATCH 1/3] dt-bindings: mmc: sdhci-am654: Update Output tap delay binding
From: Faiz Abbas @ 2020-02-20 11:24 UTC (permalink / raw)
To: Rob Herring
Cc: linux-kernel, linux-mmc, devicetree, ulf.hansson, adrian.hunter
In-Reply-To: <24e6ac71-00c7-f140-86d8-fa5ec0dcaff0@ti.com>
Rob,
On 14/02/20 4:28 pm, Faiz Abbas wrote:
> Rob,
>
> On 07/02/20 3:07 pm, Faiz Abbas wrote:
>> Rob,
>>
>> On 20/01/20 11:00 am, Faiz Abbas wrote:
>>> Hi Rob,
>>>
>>> On 15/01/20 7:20 am, Rob Herring wrote:
>>>> On Wed, Jan 08, 2020 at 08:39:18PM +0530, Faiz Abbas wrote:
>>>>> According to latest AM65x Data Manual[1], a different output tap delay
>>>>> value is recommended for all speed modes. Therefore, replace the
>>>>> ti,otap-del-sel binding with one ti,otap-del-sel- for each MMC/SD speed
>>>>> mode.
>>>>>
>>>>> [1] http://www.ti.com/lit/gpn/am6526
>>>>>
>>>>> Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
>>>>> ---
>>>>> .../devicetree/bindings/mmc/sdhci-am654.txt | 21 +++++++++++++++++--
>>>>> 1 file changed, 19 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-am654.txt b/Documentation/devicetree/bindings/mmc/sdhci-am654.txt
>>>>> index 50e87df47971..c6ccecb9ae5a 100644
>>>>> --- a/Documentation/devicetree/bindings/mmc/sdhci-am654.txt
>>>>> +++ b/Documentation/devicetree/bindings/mmc/sdhci-am654.txt
>>>>> @@ -18,7 +18,20 @@ Required Properties:
>>>>> - clocks: Handles to the clock inputs.
>>>>> - clock-names: Tuple including "clk_xin" and "clk_ahb"
>>>>> - interrupts: Interrupt specifiers
>>>>> - - ti,otap-del-sel: Output Tap Delay select
>>>>> + Output tap delay for each speed mode:
>>>>> + - ti,otap-del-sel-legacy
>>>>> + - ti,otap-del-sel-mmc-hs
>>>>> + - ti,otap-del-sel-sd-hs
>>>>> + - ti,otap-del-sel-sdr12
>>>>> + - ti,otap-del-sel-sdr25
>>>>> + - ti,otap-del-sel-sdr50
>>>>> + - ti,otap-del-sel-sdr104
>>>>> + - ti,otap-del-sel-ddr50
>>>>> + - ti,otap-del-sel-ddr52
>>>>> + - ti,otap-del-sel-hs200
>>>>> + - ti,otap-del-sel-hs400
>>>>> + These bindings must be provided otherwise the driver will disable the
>>>>> + corresponding speed mode (i.e. all nodes must provide at least -legacy)
>>>>
>>>> Why not just extend the existing property to be an array. We already
>>>> have properties to enable/disable speed modes.
>>>>
>>>
>>> Its hard to keep track of which modes have values and which don't when
>>> you add an array. This scheme is just easier on anyone adding new values
>>> or updating old values.
>>>
>>> We already disable speed modes based on platform specific properties in
>>> other drivers. In sdhci-omap.c, the driver disables the corresponding
>>> speed mode if the corresponding pinmux and iodelay values are not populated.
>>>
>>
>> Do you agree on above?
>>
>
> Gentle ping.
>
Ping.
Thanks,
Faiz
^ permalink raw reply
* Re: Hard Disk consumes lots of power in s2idle
From: Kai-Heng Feng @ 2020-02-20 11:21 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Srinivas Pandruvada, Linux PM,
open list:LIBATA SUBSYSTEM (Serial and Parallel ATA drivers),
open list, Kent Lin, Tejun Heo
In-Reply-To: <CAJZ5v0in-v9iu88Wj0gAg_th-E-QWDSHZ_XvgyoOuqE=CZPtqQ@mail.gmail.com>
> On Feb 20, 2020, at 18:34, Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> On Thu, Feb 20, 2020 at 11:25 AM Kai-Heng Feng
> <kai.heng.feng@canonical.com> wrote:
>>
>>
>>
>>> On Feb 20, 2020, at 18:12, Rafael J. Wysocki <rafael@kernel.org> wrote:
>>>
>>> On Thu, Feb 20, 2020 at 9:08 AM Kai-Heng Feng
>>> <kai.heng.feng@canonical.com> wrote:
>>>>
>>>> Hi Srinivas,
>>>>
>>>>> On Feb 20, 2020, at 02:36, Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> wrote:
>>>>>
>>>>> Hi Kai,
>>>>>
>>>>> On Wed, 2020-02-19 at 22:22 +0800, Kai-Heng Feng wrote:
>>>>>> Hi Srinivas,
>>>>>>
>>>>>> Your previous work to support DEVSLP works well on SATA SSDs, so I am
>>>>>> asking you the issue I am facing:
>>>>>> Once a laptop has a HDD installed, the power consumption during
>>>>>> S2Idle increases ~0.4W, which is quite a lot.
>>>>>> However, HDDs don't seem to support DEVSLP, so I wonder if you know
>>>>>> to do proper power management for HDDs?
>>>>> What is the default here
>>>>> cat /sys/power/mem_sleep
>>>>> s2idle or deep?
>>>>
>>>> It defaults to s2idle.
>>>>
>>>>>
>>>>> Please follow debug steps here:
>>>>> https://01.org/blogs/qwang59/2018/how-achieve-s0ix-states-linux
>>>>>
>>>>> We need to check whether you get any PC10 residency or not.
>>>>
>>>> Yes it reaches PC10. It doesn't reach SLP_S0 though.
>>>> The real number on S2Idle power consumption:
>>>> No HDD: ~1.4W
>>>> One HDD: ~1.8W
>>>>
>>>> If the SoC doesn't hit PC10 the number should be significantly higher.
>>>> That's why I think the issue is the power management on HDD itself.
>>>
>>> I'm assuming that you mean a non-SSD device here.
>>
>> Yes, it's spinning rust here.
>>
>>>
>>> That would be handled via ata_port_suspend() I gather and whatever
>>> that does should do the right thing.
>>>
>>> Do you think that the disk doesn't spin down or it spins down, but the
>>> logic stays on?
>>
>> The spin sound is audible, so I am certain the HDD spins down during S2Idle.
>
> OK
>
>> How do I know if the logic is on or off?
>
> Well, if it were off, it would not draw power. :-)
>
> So IMO it is reasonable to assume that the logic on the drive stays
> on. I'm not aware of anything that can be done to turn it off,
> however.
Okay, thanks for the info.
I'll ask vendors the possibility to handle it at system firmware level.
Kai-Heng
^ permalink raw reply
* Re: [PATCH net-next v3 1/9] sysfs: add sysfs_file_change_owner{_by_name}()
From: Greg Kroah-Hartman @ 2020-02-20 11:20 UTC (permalink / raw)
To: Christian Brauner
Cc: David S. Miller, linux-kernel, netdev, Rafael J. Wysocki,
Pavel Machek, Jakub Kicinski, Eric Dumazet, Stephen Hemminger,
linux-pm
In-Reply-To: <20200218162943.2488012-2-christian.brauner@ubuntu.com>
On Tue, Feb 18, 2020 at 05:29:35PM +0100, Christian Brauner wrote:
> +/**
> + * sysfs_file_change_owner - change owner of a file.
> + * @kobj: object.
> + * @kuid: new owner's kuid
> + * @kgid: new owner's kgid
> + */
> +int sysfs_file_change_owner(struct kobject *kobj, kuid_t kuid, kgid_t kgid)
> +{
> + struct kernfs_node *kn;
> + int error;
> +
> + if (!kobj->state_in_sysfs)
> + return -EINVAL;
> +
> + kernfs_get(kobj->sd);
> +
> + kn = kobj->sd;
> + error = internal_change_owner(kn, kobj, kuid, kgid);
> +
> + kernfs_put(kn);
> +
> + return error;
> +}
> +EXPORT_SYMBOL_GPL(sysfs_file_change_owner);
Oops, wait, what "file" are you changing here? You aren't changing the
kobject's attributes, but rather a file in the kobject's directory,
right? But kobj->sd is the directory of the kobject itself, so why
isn't this function just the same thing as sysfs_change_owner()?
Why would you call this function at all?
confused,
greg k-h
^ permalink raw reply
* Re: [PATCHv5 34/34] drm/rockchip: Add support for afbc
From: Boris Brezillon @ 2020-02-20 11:20 UTC (permalink / raw)
To: Andrzej Pietrasiewicz
Cc: kernel, Mihail Atanassov, David Airlie, Liviu Dudau, Sandy Huang,
dri-devel, James Wang, Ayan Halder, Sean Paul
In-Reply-To: <20191217145020.14645-35-andrzej.p@collabora.com>
On Tue, 17 Dec 2019 15:50:20 +0100
Andrzej Pietrasiewicz <andrzej.p@collabora.com> wrote:
> static const struct drm_mode_config_funcs rockchip_drm_mode_config_funcs = {
> - .fb_create = drm_gem_fb_create_with_dirty,
> + .fb_create = rockchip_fb_create,
> .output_poll_changed = drm_fb_helper_output_poll_changed,
> .atomic_check = drm_atomic_helper_check,
> .atomic_commit = drm_atomic_helper_commit,
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> index d04b3492bdac..ffe1e4b9a9ca 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> @@ -91,9 +91,21 @@
> #define VOP_WIN_TO_INDEX(vop_win) \
> ((vop_win) - (vop_win)->vop->win)
>
> +#define VOP_AFBC_SET(vop, name, v) \
> + do { \
> + if ((vop)->data->afbc) \
> + vop_reg_set((vop), &(vop)->data->afbc->name, 0, ~0, v, #name); \
> + } while (0)
> +
> #define to_vop(x) container_of(x, struct vop, crtc)
> #define to_vop_win(x) container_of(x, struct vop_win, base)
>
> +#define AFBC_FMT_RGB565 0x0
> +#define AFBC_FMT_U8U8U8U8 0x5
> +#define AFBC_FMT_U8U8U8 0x4
> +
> +#define AFBC_TILE_16x16 BIT(4)
> +
> /*
> * The coefficients of the following matrix are all fixed points.
> * The format is S2.10 for the 3x3 part of the matrix, and S9.12 for the offsets.
> @@ -166,6 +178,7 @@ struct vop {
> /* optional internal rgb encoder */
> struct rockchip_rgb *rgb;
>
> + struct vop_win *afbc_win;
Looks like something that should be in the VOP specialized crtc_state.
Also, do we really need a vop_win pointer or can it be replaced by an
index directly?
> struct vop_win win[];
> };
>
> @@ -274,6 +287,29 @@ static enum vop_data_format vop_convert_format(uint32_t format)
> }
> }
>
> +static int vop_convert_afbc_format(uint32_t format)
> +{
> + switch (format) {
> + case DRM_FORMAT_XRGB8888:
> + case DRM_FORMAT_ARGB8888:
> + case DRM_FORMAT_XBGR8888:
> + case DRM_FORMAT_ABGR8888:
> + return AFBC_FMT_U8U8U8U8;
> + case DRM_FORMAT_RGB888:
> + case DRM_FORMAT_BGR888:
> + return AFBC_FMT_U8U8U8;
> + case DRM_FORMAT_RGB565:
> + case DRM_FORMAT_BGR565:
> + return AFBC_FMT_RGB565;
> + /* either of the below should not be reachable */
> + default:
> + DRM_WARN_ONCE("unsupported AFBC format[%08x]\n", format);
> + return -EINVAL;
> + }
> +
> + return -EINVAL;
> +}
> +
> static uint16_t scl_vop_cal_scale(enum scale_mode mode, uint32_t src,
> uint32_t dst, bool is_horizontal,
> int vsu_mode, int *vskiplines)
> @@ -598,6 +634,15 @@ static int vop_enable(struct drm_crtc *crtc, struct drm_crtc_state *old_state)
> vop_win_disable(vop, vop_win);
> }
> }
> +
> + if (vop->data->afbc) {
> + /*
> + * Disable AFBC and forget there was a vop window with AFBC
> + */
> + VOP_AFBC_SET(vop, enable, 0);
> + vop->afbc_win = NULL;
> + }
> +
> spin_unlock(&vop->reg_lock);
>
> vop_cfg_done(vop);
> @@ -710,6 +755,37 @@ static void vop_plane_destroy(struct drm_plane *plane)
> drm_plane_cleanup(plane);
> }
>
> +static bool rockchip_afbc(u64 modifier)
> +{
> + return modifier == DRM_FORMAT_MOD_ARM_AFBC(AFBC_FORMAT_MOD_BLOCK_SIZE_16x16 | AFBC_FORMAT_MOD_SPARSE);
> +}
> +
> +static bool rockchip_mod_supported(struct drm_plane *plane,
> + u32 format, u64 modifier)
> +{
> + const struct drm_format_info *info;
> +
> + if (WARN_ON(modifier == DRM_FORMAT_MOD_INVALID))
> + return false;
> +
> + if (modifier == DRM_FORMAT_MOD_LINEAR)
> + return true;
> +
> + if (!rockchip_afbc(modifier)) {
> + DRM_DEBUG_KMS("Unsupported format modifer 0x%llx\n", modifier);
> +
> + return false;
> + }
> +
> + info = drm_format_info(format);
> + if (info->num_planes != 1) {
> + DRM_DEBUG_KMS("AFBC buffers expect one plane\n");
> + return false;
> + }
Should we have a call to vop_convert_afbc_format() to make sure AFBC
can be applied to this specific format?
> +
> + return true;
> +}
> +
> static int vop_plane_atomic_check(struct drm_plane *plane,
> struct drm_plane_state *state)
> {
> @@ -719,10 +795,8 @@ static int vop_plane_atomic_check(struct drm_plane *plane,
> struct vop_win *vop_win = to_vop_win(plane);
> const struct vop_win_data *win = vop_win->data;
> int ret;
> - int min_scale = win->phy->scl ? FRAC_16_16(1, 8) :
> - DRM_PLANE_HELPER_NO_SCALING;
> - int max_scale = win->phy->scl ? FRAC_16_16(8, 1) :
> - DRM_PLANE_HELPER_NO_SCALING;
> + int min_scale = win->phy->scl ? FRAC_16_16(1, 8) : DRM_PLANE_HELPER_NO_SCALING;
> + int max_scale = win->phy->scl ? FRAC_16_16(8, 1) : DRM_PLANE_HELPER_NO_SCALING;
Looks like those coding-style fixes have nothing to do in this patch :).
>
> if (!crtc || !fb)
> return 0;
> @@ -731,9 +805,7 @@ static int vop_plane_atomic_check(struct drm_plane *plane,
> if (WARN_ON(!crtc_state))
> return -EINVAL;
>
> - ret = drm_atomic_helper_check_plane_state(state, crtc_state,
> - min_scale, max_scale,
> - true, true);
> + ret = drm_atomic_helper_check_plane_state(state, crtc_state, min_scale, max_scale, true, true);
Same here, plus, following the 80 char rule when it doesn't hurt
readability is a good thing I think.
> if (ret)
> return ret;
>
> @@ -758,6 +830,31 @@ static int vop_plane_atomic_check(struct drm_plane *plane,
> return -EINVAL;
> }
>
> + if (rockchip_afbc(fb->modifier)) {
> + struct vop *vop = to_vop(crtc);
> +
> + if (!vop->data->afbc) {
> + DRM_ERROR("vop does not support AFBC\n");
> + return -EINVAL;
> + }
> +
> + ret = vop_convert_afbc_format(fb->format->format);
> + if (ret < 0)
> + return ret;
> +
> + if (state->src.x1 || state->src.y1) {
> + DRM_ERROR("afbc does not support offset display\n");
> + DRM_ERROR("xpos=%d, ypos=%d, offset=%d\n", state->src.x1, state->src.y1, fb->offsets[0]);
Why splitting that in 2 error messages?
> + return -EINVAL;
> + }
> +
> + if (state->rotation && state->rotation != DRM_MODE_ROTATE_0) {
> + DRM_ERROR("afbc does not support rotation\n");
> + DRM_ERROR("rotation=%d\n", state->rotation);
Ditto.
> + return -EINVAL;
> + }
> + }
> +
> return 0;
> }
>
> @@ -773,6 +870,11 @@ static void vop_plane_atomic_disable(struct drm_plane *plane,
> spin_lock(&vop->reg_lock);
>
> vop_win_disable(vop, vop_win);
> + /*
> + * Forget about the AFBC window if it is being disabled
> + */
> + if (vop_win == vop->afbc_win)
> + vop->afbc_win = NULL;
>
> spin_unlock(&vop->reg_lock);
> }
> @@ -812,6 +914,13 @@ static void vop_plane_atomic_update(struct drm_plane *plane,
> if (WARN_ON(!vop->is_enabled))
> return;
>
> + /*
> + * If updating the AFBC window then assume that
> + * after the update there will be no AFBC window.
> + */
> + if (vop_win == vop->afbc_win)
> + vop->afbc_win = NULL;
> +
> if (!state->visible) {
> vop_plane_atomic_disable(plane, old_state);
> return;
> @@ -846,6 +955,21 @@ static void vop_plane_atomic_update(struct drm_plane *plane,
>
> spin_lock(&vop->reg_lock);
>
> + if (rockchip_afbc(fb->modifier)) {
> + int afbc_format = vop_convert_afbc_format(fb->format->format);
> +
> + VOP_AFBC_SET(vop, format, afbc_format | AFBC_TILE_16x16);
> + VOP_AFBC_SET(vop, hreg_block_split, 0);
> + VOP_AFBC_SET(vop, win_sel, VOP_WIN_TO_INDEX(vop_win));
IIUC, only one plane can use AFBC at a given time, but you don't seem
to check the value of vop->afbc_win in your atomic_check. What happens
if the user tries to enable this modifier on 2+ planes?
> + VOP_AFBC_SET(vop, hdr_ptr, dma_addr);
> + VOP_AFBC_SET(vop, pic_size, act_info);
> +
> + /*
> + * The window being udated becomes the AFBC window
> + */
> + vop->afbc_win = vop_win;
> + }
> +
> VOP_WIN_SET(vop, win, format, format);
> VOP_WIN_SET(vop, win, yrgb_vir, DIV_ROUND_UP(fb->pitches[0], 4));
> VOP_WIN_SET(vop, win, yrgb_mst, dma_addr);
> @@ -1001,6 +1125,7 @@ static const struct drm_plane_funcs vop_plane_funcs = {
> .reset = drm_atomic_helper_plane_reset,
> .atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
> .atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
> + .format_mod_supported = rockchip_mod_supported,
> };
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [cip-dev] [PATCH 4.19.y-cip 15/23] usb: typec: driver for TI HD3SS3220 USB Type-C DRP port controller
From: Marian-Cristian Rotariu @ 2020-02-20 11:20 UTC (permalink / raw)
To: Pavel Machek; +Cc: cip-dev@lists.cip-project.org
In-Reply-To: <20200219081234.GD31996@amd>
Hi!
> -----Original Message-----
> From: Pavel Machek <pavel@denx.de>
> Sent: 19 February 2020 08:13
> To: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>
> Cc: cip-dev@lists.cip-project.org
> Subject: Re: [cip-dev] [PATCH 4.19.y-cip 15/23] usb: typec: driver for TI
> HD3SS3220 USB Type-C DRP port controller
>
> Hi!
>
> > From: Biju Das <biju.das@bp.renesas.com>
> >
> > commit 1c48c759ef4bb9031b3347277f04484e07e27d97 upstream.
> >
> > Driver for TI HD3SS3220 USB Type-C DRP port controller.
> >
> > The driver currently registers the port and supports data role
> > swapping.
>
> Ok, so this has few problems that are fixed by subsequent patches, but I
> believe I see some more:
>
> > +static enum usb_role hd3ss3220_get_attached_state(struct hd3ss3220
> > +*hd3ss3220) {
> > + unsigned int reg_val;
> > + enum usb_role attached_state;
> > + int ret;
> > +
> > + ret = regmap_read(hd3ss3220->regmap,
> HD3SS3220_REG_CN_STAT_CTRL,
> > + ®_val);
> > + if (ret < 0)
> > + return ret;
>
> This function claims to return "enum usb_role", but here it returns errno
> from regmap_read.
>
> > +static int hd3ss3220_dr_set(const struct typec_capability *cap,
> > + enum typec_data_role role)
> > +{
> ...
> > + ret = hd3ss3220_set_source_pref(hd3ss3220, pref);
> > + usleep_range(10, 100);
>
> Would udelay() make more sense here? Are your CPUs / timer subsystem so
> fast that sleeping for 10usec is possible and reasonable to do?
I think Biju followed the overall indications from:
Documentation/timers-howto.txt (.rst)
According to the documentation the inflection point is at 10us, so this is kind
of a grey area. Just to put it in context, the RZ/G2E has 2x1.2Ghz ARM Cortex A53.
Therefore this is kind of a high-performance embedded platform. At least, we
like to advertise it like that 😊
So, I am inclined to keep this as it is if you agree.
Cheers,
Marian.
_______________________________________________
cip-dev mailing list
cip-dev@lists.cip-project.org
https://lists.cip-project.org/mailman/listinfo/cip-dev
^ permalink raw reply
* Re: [Intel-gfx] [PATCH 04/12] drm/i915: Add i9xx_lut_8()
From: Emil Velikov @ 2020-02-20 11:20 UTC (permalink / raw)
To: Ville Syrjala; +Cc: Intel Graphics Development, ML dri-devel
In-Reply-To: <20191107151725.10507-5-ville.syrjala@linux.intel.com>
On Thu, 7 Nov 2019 at 15:17, Ville Syrjala
<ville.syrjala@linux.intel.com> wrote:
>
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> We have a nice little helper to compute a single LUT entry
> for everything except the 8bpc legacy gamma mode. Let's
> complete the set.
>
At a later stage one could rename this & the 10bit one, moving them to
include/drm/.
There are other drivers doing the same thing... not sure if that's
worth it though.
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
-Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [Intel-gfx] [PATCH 04/12] drm/i915: Add i9xx_lut_8()
From: Emil Velikov @ 2020-02-20 11:20 UTC (permalink / raw)
To: Ville Syrjala; +Cc: Intel Graphics Development, ML dri-devel
In-Reply-To: <20191107151725.10507-5-ville.syrjala@linux.intel.com>
On Thu, 7 Nov 2019 at 15:17, Ville Syrjala
<ville.syrjala@linux.intel.com> wrote:
>
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> We have a nice little helper to compute a single LUT entry
> for everything except the 8bpc legacy gamma mode. Let's
> complete the set.
>
At a later stage one could rename this & the 10bit one, moving them to
include/drm/.
There are other drivers doing the same thing... not sure if that's
worth it though.
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
-Emil
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply
* Re: [PATCH] aio-posix: avoid reacquiring rcu_read_lock() when polling
From: Paolo Bonzini @ 2020-02-20 11:19 UTC (permalink / raw)
To: Stefan Hajnoczi, qemu-devel; +Cc: Fam Zheng, qemu-block
In-Reply-To: <20200218182708.914552-1-stefanha@redhat.com>
On 18/02/20 19:27, Stefan Hajnoczi wrote:
> The first rcu_read_lock/unlock() is expensive. Nested calls are cheap.
>
> This optimization increases IOPS from 73k to 162k with a Linux guest
> that has 2 virtio-blk,num-queues=1 and 99 virtio-blk,num-queues=32
> devices.
>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
> util/aio-posix.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/util/aio-posix.c b/util/aio-posix.c
> index a4977f538e..f67f5b34e9 100644
> --- a/util/aio-posix.c
> +++ b/util/aio-posix.c
> @@ -15,6 +15,7 @@
>
> #include "qemu/osdep.h"
> #include "block/block.h"
> +#include "qemu/rcu.h"
> #include "qemu/rcu_queue.h"
> #include "qemu/sockets.h"
> #include "qemu/cutils.h"
> @@ -514,6 +515,16 @@ static bool run_poll_handlers_once(AioContext *ctx, int64_t *timeout)
> bool progress = false;
> AioHandler *node;
>
> + /*
> + * Optimization: ->io_poll() handlers often contain RCU read critical
> + * sections and we therefore see many rcu_read_lock() -> rcu_read_unlock()
> + * -> rcu_read_lock() -> ... sequences with expensive memory
> + * synchronization primitives. Make the entire polling loop an RCU
> + * critical section because nested rcu_read_lock()/rcu_read_unlock() calls
> + * are cheap.
> + */
> + RCU_READ_LOCK_GUARD();
> +
> QLIST_FOREACH_RCU(node, &ctx->aio_handlers, node) {
> if (!node->deleted && node->io_poll &&
> aio_node_check(ctx, node->is_external) &&
>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
^ permalink raw reply
* Re: [PATCH v6 0/5] Reftable support git-core
From: Jeff King @ 2020-02-20 11:19 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Han-Wen Nienhuys, Han-Wen Nienhuys via GitGitGadget, git
In-Reply-To: <xmqq8skyfnxm.fsf@gitster-ct.c.googlers.com>
On Wed, Feb 19, 2020 at 10:10:29AM -0800, Junio C Hamano wrote:
> Han-Wen Nienhuys <hanwenn@gmail.com> writes:
>
> > On Wed, Feb 19, 2020 at 6:02 PM Junio C Hamano <gitster@pobox.com> wrote:
> >
> >> > your checker is tripping over code imported from zlib. I added a /*
> >> > clang-format off */ comment to avoid reformatting this code. What do
> >> > you suggest?
> >>
> >> Use zlib from the system instead?
> >
> > uncompress2 is a 2016 addition to zlib. It doesn't pass on
> > gitgitgadget's CI, because it is using an older version of zlib.
>
> Ahh.
>
> It is OK (and indeed you're right that you cannot avoid it) to ship
> a reasonably new snapshot as a fallback in such a case, but it still
> is far more preferrable to avoid linking with the fallback snapshot
> copy when a working one is available on the system, especially for a
> widely used and established library like zlib, because we have one
> less thing to keep up-to-date with the security patches made to the
> upstream.
If this were substantial code, I'd agree. But this is really just a thin
wrapper around the usual loop zlib inflate() loop that we already do in
a dozen places in our code (e.g., unpack_loose_rest(), which even does
the "did we consume all the bytes" check that uncompress2 allows).
I'm not sure it is worth the mental energy of adding a Makefile knob to
use the zlib version if we can just open-code it ourselves (or provide
our own custom helper that we always use).
And if it _is_ worth taking the zlib version because we're concerned
that it may contain or gain bugfixes[1], then possibly we should
be using it in lots more places (though probably not everywhere, as we
do sometimes need the streaming behavior of the loop).
[1] I'll admit we've hit some subtleties with that loop in the past,
especially around corrupted or bogus inputs, but I think we've
shaken them all out these days.
-Peff
^ permalink raw reply
* Re: [PATCH v4 3/5] dt-bindings: clock: Add YAML schemas for the QCOM MSS clock bindings
From: Sibi Sankar @ 2020-02-20 11:18 UTC (permalink / raw)
To: Taniya Das
Cc: Rob Herring, Stephen Boyd, Michael Turquette, David Brown,
Rajendra Nayak, linux-arm-msm, linux-soc, linux-clk, linux-kernel,
Andy Gross, devicetree, robh+dt, linux-soc-owner
In-Reply-To: <6298769e-09bc-eb69-bf72-5aedd0e87f16@codeaurora.org>
Hey Taniya,
+ <&gcc GCC_MSS_NAV_AXIS_CLK>,
error is because of ^^ typo
it should be GCC_MSS_NAV_AXI_CLK
instead, with that dt_bindings
check will go though.
On 2020-02-19 09:35, Taniya Das wrote:
> Hi Rob,
>
> On 2/19/2020 4:30 AM, Rob Herring wrote:
>> On Tue, 18 Feb 2020 23:45:31 +0530, Taniya Das wrote:
>>> The Modem Subsystem clock provider have a bunch of generic properties
>>> that are needed in a device tree. Add a YAML schemas for those.
>>>
>>> Signed-off-by: Taniya Das <tdas@codeaurora.org>
>>> ---
>>> .../devicetree/bindings/clock/qcom,sc7180-mss.yaml | 62
>>> ++++++++++++++++++++++
>>> 1 file changed, 62 insertions(+)
>>> create mode 100644
>>> Documentation/devicetree/bindings/clock/qcom,sc7180-mss.yaml
>>>
>>
>> My bot found errors running 'make dt_binding_check' on your patch:
>>
>> Error:
>> Documentation/devicetree/bindings/clock/qcom,sc7180-mss.example.dts:21.26-27
>> syntax error
>> FATAL ERROR: Unable to parse input tree
>> scripts/Makefile.lib:300: recipe for target
>> 'Documentation/devicetree/bindings/clock/qcom,sc7180-mss.example.dt.yaml'
>> failed
>> make[1]: ***
>> [Documentation/devicetree/bindings/clock/qcom,sc7180-mss.example.dt.yaml]
>> Error 1
>> Makefile:1263: recipe for target 'dt_binding_check' failed
>> make: *** [dt_binding_check] Error 2
>>
>> See https://patchwork.ozlabs.org/patch/1240251
>> Please check and re-submit.
>>
>
> I did see the same issue and then when I re-ordered by patches
> dt-bindings: clock: Add support for Modem clocks in GCC (dependent) on
> this binding patch, I no longer encountered the issue.
> https://patchwork.kernel.org/patch/11389243/
>
> Please let me know.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project.
^ permalink raw reply
* Re: [PATCH net-next v3 3/9] sysfs: add sysfs_group{s}_change_owner()
From: Greg Kroah-Hartman @ 2020-02-20 11:15 UTC (permalink / raw)
To: Christian Brauner
Cc: David S. Miller, linux-kernel, netdev, Rafael J. Wysocki,
Pavel Machek, Jakub Kicinski, Eric Dumazet, Stephen Hemminger,
linux-pm
In-Reply-To: <20200218162943.2488012-4-christian.brauner@ubuntu.com>
On Tue, Feb 18, 2020 at 05:29:37PM +0100, Christian Brauner wrote:
> Add helpers to change the owner of sysfs groups.
> This function will be used to correctly account for kobject ownership
> changes, e.g. when moving network devices between network namespaces.
>
> Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
> ---
> /* v2 */
> - Greg Kroah-Hartman <gregkh@linuxfoundation.org>:
> - Add comment how ownership of sysfs object is changed.
>
> /* v3 */
> - Greg Kroah-Hartman <gregkh@linuxfoundation.org>:
> - Add explicit uid/gid parameters.
> - Christian Brauner <christian.brauner@ubuntu.com>:
> - Collapse groups ownership helper patches into a single patch.
> ---
> fs/sysfs/group.c | 117 ++++++++++++++++++++++++++++++++++++++++++
> include/linux/sysfs.h | 20 ++++++++
> 2 files changed, 137 insertions(+)
>
> diff --git a/fs/sysfs/group.c b/fs/sysfs/group.c
> index c4ab045926b7..bae562d3cba1 100644
> --- a/fs/sysfs/group.c
> +++ b/fs/sysfs/group.c
> @@ -13,6 +13,7 @@
> #include <linux/dcache.h>
> #include <linux/namei.h>
> #include <linux/err.h>
> +#include <linux/fs.h>
> #include "sysfs.h"
>
>
> @@ -457,3 +458,119 @@ int __compat_only_sysfs_link_entry_to_kobj(struct kobject *kobj,
> return PTR_ERR_OR_ZERO(link);
> }
> EXPORT_SYMBOL_GPL(__compat_only_sysfs_link_entry_to_kobj);
> +
> +static int sysfs_group_attrs_change_owner(struct kernfs_node *grp_kn,
> + const struct attribute_group *grp,
> + struct iattr *newattrs)
> +{
> + struct kernfs_node *kn;
> + int error;
> +
> + if (grp->attrs) {
> + struct attribute *const *attr;
> +
> + for (attr = grp->attrs; *attr; attr++) {
> + kn = kernfs_find_and_get(grp_kn, (*attr)->name);
> + if (!kn)
> + return -ENOENT;
> +
> + error = kernfs_setattr(kn, newattrs);
> + kernfs_put(kn);
> + if (error)
> + return error;
> + }
> + }
> +
> + if (grp->bin_attrs) {
> + struct bin_attribute *const *bin_attr;
> +
> + for (bin_attr = grp->bin_attrs; *bin_attr; bin_attr++) {
> + kn = kernfs_find_and_get(grp_kn, (*bin_attr)->attr.name);
> + if (!kn)
> + return -ENOENT;
> +
> + error = kernfs_setattr(kn, newattrs);
> + kernfs_put(kn);
> + if (error)
> + return error;
> + }
> + }
> +
> + return 0;
> +}
> +
> +/**
> + * sysfs_group_change_owner - change owner of an attribute group.
> + * @kobj: The kobject containing the group.
> + * @grp: The attribute group.
> + * @kuid: new owner's kuid
> + * @kgid: new owner's kgid
> + *
> + * Returns 0 on success or error code on failure.
This is fine to document, just funny it's the only one documented about
the return value so far in this series.
Anyway, looks good to me:
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
^ permalink raw reply
* [Intel-gfx] [PULL] drm-intel-fixes
From: Jani Nikula @ 2020-02-20 11:14 UTC (permalink / raw)
To: Dave Airlie, Daniel Vetter; +Cc: dim-tools, dri-devel, Maxime Ripard, intel-gfx
Hi Dave & Daniel -
Due to issues in s2idle in v5.6-rc2, I've gotten CI results on these
with two hack reverts on top, and I threw them out just before making
the pull request. I had to revert:
fdde0ff8590b ("ACPI: PM: s2idle: Prevent spurious SCIs from waking up the system")
e3728b50cd9b ("ACPI: PM: s2idle: Avoid possible race related to the EC GPE")
I've been told the issues have been reported, hopefully we'll get the
fixes in Linus' upstream soon too.
drm-intel-fixes-2020-02-20:
drm/i915 fixes for v5.6-rc3:
- Workaround missing Display Stream Compression (DSC) state readout by
forcing modeset when its enabled at probe
- Fix EHL port clock voltage level requirements
- Fix queuing retire workers on the virtual engine
- Fix use of partially initialized waiters
- Stop using drm_pci_alloc/drm_pci/free
- Fix rewind of RING_TAIL by forcing a context reload
- Fix locking on resetting ring->head
- Propagate our bug filing URL change to stable kernels
BR,
Jani.
The following changes since commit 11a48a5a18c63fd7621bb050228cebf13566e4d8:
Linux 5.6-rc2 (2020-02-16 13:16:59 -0800)
are available in the Git repository at:
git://anongit.freedesktop.org/drm/drm-intel tags/drm-intel-fixes-2020-02-20
for you to fetch changes up to 15de9cb5c9c83a23be92b8f7a1178cead1486587:
drm/i915/gt: Avoid resetting ring->head outside of its timeline mutex (2020-02-18 09:53:18 +0200)
----------------------------------------------------------------
drm/i915 fixes for v5.6-rc3:
- Workaround missing Display Stream Compression (DSC) state readout by
forcing modeset when its enabled at probe
- Fix EHL port clock voltage level requirements
- Fix queuing retire workers on the virtual engine
- Fix use of partially initialized waiters
- Stop using drm_pci_alloc/drm_pci/free
- Fix rewind of RING_TAIL by forcing a context reload
- Fix locking on resetting ring->head
- Propagate our bug filing URL change to stable kernels
----------------------------------------------------------------
Chris Wilson (7):
drm/i915/gem: Require per-engine reset support for non-persistent contexts
drm/i915: Initialise basic fence before acquiring seqno
drm/i915/gt: Prevent queuing retire workers on the virtual engine
drm/i915/gt: Protect defer_request() from new waiters
drm/i915: Wean off drm_pci_alloc/drm_pci_free
drm/i915/execlists: Always force a context reload when rewinding RING_TAIL
drm/i915/gt: Avoid resetting ring->head outside of its timeline mutex
Jani Nikula (3):
MAINTAINERS: Update drm/i915 bug filing URL
drm/i915: Update drm/i915 bug filing URL
drm/i915/dsc: force full modeset whenever DSC is enabled at probe
Matt Roper (1):
drm/i915/ehl: Update port clock voltage level requirements
MAINTAINERS | 2 +-
drivers/gpu/drm/i915/Kconfig | 5 +-
drivers/gpu/drm/i915/display/intel_ddi.c | 4 +-
drivers/gpu/drm/i915/display/intel_display.c | 20 ++++-
drivers/gpu/drm/i915/gem/i915_gem_context.c | 16 ++++
drivers/gpu/drm/i915/gem/i915_gem_object_types.h | 3 -
drivers/gpu/drm/i915/gem/i915_gem_phys.c | 98 ++++++++++++------------
drivers/gpu/drm/i915/gt/intel_breadcrumbs.c | 3 +
drivers/gpu/drm/i915/gt/intel_gt_requests.c | 3 +
drivers/gpu/drm/i915/gt/intel_lrc.c | 61 +++++++--------
drivers/gpu/drm/i915/gt/intel_ring.c | 1 +
drivers/gpu/drm/i915/gt/intel_ring.h | 8 ++
drivers/gpu/drm/i915/gt/intel_ring_types.h | 7 +-
drivers/gpu/drm/i915/gt/selftest_lrc.c | 2 +-
drivers/gpu/drm/i915/i915_gem.c | 8 +-
drivers/gpu/drm/i915/i915_gpu_error.c | 3 +-
drivers/gpu/drm/i915/i915_request.c | 21 +++--
drivers/gpu/drm/i915/i915_scheduler.c | 6 +-
drivers/gpu/drm/i915/i915_utils.c | 5 +-
19 files changed, 168 insertions(+), 108 deletions(-)
--
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply
* [PULL] drm-intel-fixes
From: Jani Nikula @ 2020-02-20 11:14 UTC (permalink / raw)
To: Dave Airlie, Daniel Vetter
Cc: dim-tools, dri-devel, Rodrigo Vivi, Sean Paul, intel-gfx
Hi Dave & Daniel -
Due to issues in s2idle in v5.6-rc2, I've gotten CI results on these
with two hack reverts on top, and I threw them out just before making
the pull request. I had to revert:
fdde0ff8590b ("ACPI: PM: s2idle: Prevent spurious SCIs from waking up the system")
e3728b50cd9b ("ACPI: PM: s2idle: Avoid possible race related to the EC GPE")
I've been told the issues have been reported, hopefully we'll get the
fixes in Linus' upstream soon too.
drm-intel-fixes-2020-02-20:
drm/i915 fixes for v5.6-rc3:
- Workaround missing Display Stream Compression (DSC) state readout by
forcing modeset when its enabled at probe
- Fix EHL port clock voltage level requirements
- Fix queuing retire workers on the virtual engine
- Fix use of partially initialized waiters
- Stop using drm_pci_alloc/drm_pci/free
- Fix rewind of RING_TAIL by forcing a context reload
- Fix locking on resetting ring->head
- Propagate our bug filing URL change to stable kernels
BR,
Jani.
The following changes since commit 11a48a5a18c63fd7621bb050228cebf13566e4d8:
Linux 5.6-rc2 (2020-02-16 13:16:59 -0800)
are available in the Git repository at:
git://anongit.freedesktop.org/drm/drm-intel tags/drm-intel-fixes-2020-02-20
for you to fetch changes up to 15de9cb5c9c83a23be92b8f7a1178cead1486587:
drm/i915/gt: Avoid resetting ring->head outside of its timeline mutex (2020-02-18 09:53:18 +0200)
----------------------------------------------------------------
drm/i915 fixes for v5.6-rc3:
- Workaround missing Display Stream Compression (DSC) state readout by
forcing modeset when its enabled at probe
- Fix EHL port clock voltage level requirements
- Fix queuing retire workers on the virtual engine
- Fix use of partially initialized waiters
- Stop using drm_pci_alloc/drm_pci/free
- Fix rewind of RING_TAIL by forcing a context reload
- Fix locking on resetting ring->head
- Propagate our bug filing URL change to stable kernels
----------------------------------------------------------------
Chris Wilson (7):
drm/i915/gem: Require per-engine reset support for non-persistent contexts
drm/i915: Initialise basic fence before acquiring seqno
drm/i915/gt: Prevent queuing retire workers on the virtual engine
drm/i915/gt: Protect defer_request() from new waiters
drm/i915: Wean off drm_pci_alloc/drm_pci_free
drm/i915/execlists: Always force a context reload when rewinding RING_TAIL
drm/i915/gt: Avoid resetting ring->head outside of its timeline mutex
Jani Nikula (3):
MAINTAINERS: Update drm/i915 bug filing URL
drm/i915: Update drm/i915 bug filing URL
drm/i915/dsc: force full modeset whenever DSC is enabled at probe
Matt Roper (1):
drm/i915/ehl: Update port clock voltage level requirements
MAINTAINERS | 2 +-
drivers/gpu/drm/i915/Kconfig | 5 +-
drivers/gpu/drm/i915/display/intel_ddi.c | 4 +-
drivers/gpu/drm/i915/display/intel_display.c | 20 ++++-
drivers/gpu/drm/i915/gem/i915_gem_context.c | 16 ++++
drivers/gpu/drm/i915/gem/i915_gem_object_types.h | 3 -
drivers/gpu/drm/i915/gem/i915_gem_phys.c | 98 ++++++++++++------------
drivers/gpu/drm/i915/gt/intel_breadcrumbs.c | 3 +
drivers/gpu/drm/i915/gt/intel_gt_requests.c | 3 +
drivers/gpu/drm/i915/gt/intel_lrc.c | 61 +++++++--------
drivers/gpu/drm/i915/gt/intel_ring.c | 1 +
drivers/gpu/drm/i915/gt/intel_ring.h | 8 ++
drivers/gpu/drm/i915/gt/intel_ring_types.h | 7 +-
drivers/gpu/drm/i915/gt/selftest_lrc.c | 2 +-
drivers/gpu/drm/i915/i915_gem.c | 8 +-
drivers/gpu/drm/i915/i915_gpu_error.c | 3 +-
drivers/gpu/drm/i915/i915_request.c | 21 +++--
drivers/gpu/drm/i915/i915_scheduler.c | 6 +-
drivers/gpu/drm/i915/i915_utils.c | 5 +-
19 files changed, 168 insertions(+), 108 deletions(-)
--
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH net-next v3 2/9] sysfs: add sysfs_link_change_owner()
From: Greg Kroah-Hartman @ 2020-02-20 11:14 UTC (permalink / raw)
To: Christian Brauner
Cc: David S. Miller, linux-kernel, netdev, Rafael J. Wysocki,
Pavel Machek, Jakub Kicinski, Eric Dumazet, Stephen Hemminger,
linux-pm
In-Reply-To: <20200218162943.2488012-3-christian.brauner@ubuntu.com>
On Tue, Feb 18, 2020 at 05:29:36PM +0100, Christian Brauner wrote:
> Add a helper to change the owner of a sysfs link.
> This function will be used to correctly account for kobject ownership
> changes, e.g. when moving network devices between network namespaces.
>
> Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
> ---
> /* v2 */
> - Greg Kroah-Hartman <gregkh@linuxfoundation.org>:
> - Add comment how ownership of sysfs object is changed.
>
> /* v3 */
> - Greg Kroah-Hartman <gregkh@linuxfoundation.org>:
> - Add explicit uid/gid parameters.
> ---
> fs/sysfs/file.c | 40 ++++++++++++++++++++++++++++++++++++++++
> include/linux/sysfs.h | 10 ++++++++++
> 2 files changed, 50 insertions(+)
>
> diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
> index 32bb04b4d9d9..df5107d7b3fd 100644
> --- a/fs/sysfs/file.c
> +++ b/fs/sysfs/file.c
> @@ -570,6 +570,46 @@ static int internal_change_owner(struct kernfs_node *kn, struct kobject *kobj,
> return kernfs_setattr(kn, &newattrs);
> }
>
> +/**
> + * sysfs_link_change_owner - change owner of a link.
> + * @kobj: object of the kernfs_node the symlink is located in.
> + * @targ: object of the kernfs_node the symlink points to.
> + * @name: name of the link.
> + * @kuid: new owner's kuid
> + * @kgid: new owner's kgid
> + */
> +int sysfs_link_change_owner(struct kobject *kobj, struct kobject *targ,
> + const char *name, kuid_t kuid, kgid_t kgid)
> +{
> + struct kernfs_node *parent, *kn = NULL;
> + int error;
> +
> + if (!kobj)
> + parent = sysfs_root_kn;
> + else
> + parent = kobj->sd;
I don't understand this, why would (!kobj) ever be a valid situation?
> + if (!targ->state_in_sysfs)
> + return -EINVAL;
Should you also check kobj->state_in_sysfs as well?
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH 16/18] clocksource: Replace setup_irq() by request_irq()
From: Daniel Lezcano @ 2020-02-20 11:13 UTC (permalink / raw)
To: afzal mohammed, linux-kernel, linux-rpi-kernel, linux-arm-kernel,
linux-samsung-soc, linux-amlogic
Cc: Kate Stewart, Linus Walleij, Shawn Guo, Fabio Estevam,
Florian Fainelli, Kevin Hilman, Krzysztof Kozlowski, Kukjin Kim,
bcm-kernel-feedback-list, NXP Linux Team, Uwe Kleine-König,
Ray Jui, Sascha Hauer, Thomas Gleixner, Allison Randal,
Barry Song, Scott Branden, Greg Kroah-Hartman, Tony Prisk,
Pengutronix Kernel Team, Enrico Weigelt, Nicolas Saenz Julienne
In-Reply-To: <109d17402bc75ed186a2e151dfda1edf05463b5a.1581478324.git.afzal.mohd.ma@gmail.com>
On 12/02/2020 09:05, afzal mohammed wrote:
> request_irq() is preferred over setup_irq(). Existing callers of
> setup_irq() reached mostly via 'init_IRQ()' & 'time_init()', while
> memory allocators are ready by 'mm_init()'.
>
> Per tglx[1], setup_irq() existed in olden days when allocators were not
> ready by the time early interrupts were initialized.
>
> Hence replace setup_irq() by request_irq().
>
> Seldom remove_irq() usage has been observed coupled with setup_irq(),
> wherever that has been found, it too has been replaced by free_irq().
>
> [1] https://lkml.kernel.org/r/alpine.DEB.2.20.1710191609480.1971@nanos
>
> Signed-off-by: afzal mohammed <afzal.mohd.ma@gmail.com>
Nice, thanks for this cleanup
Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
[ ... ]
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply
* Re: [Intel-gfx] [PATCH 03/12] drm/i915: Introduce some local intel_dp variables
From: Emil Velikov @ 2020-02-20 11:13 UTC (permalink / raw)
To: Ville Syrjala; +Cc: Intel Graphics Development, ML dri-devel
In-Reply-To: <20200219203544.31013-4-ville.syrjala@linux.intel.com>
On Wed, 19 Feb 2020 at 20:36, Ville Syrjala
<ville.syrjala@linux.intel.com> wrote:
>
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> The drrs code dereferences mode->vrefresh via some really long chain
> of structures/pointers. Couldn't get coccinelle to see through all
> that so let's add some local variables to help it.
>
There's a limit to the madness that coccinelle can take :-P
Other drrs functions already use intel_dp, so might as well be consistent.
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
-Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [Intel-gfx] [PATCH 03/12] drm/i915: Introduce some local intel_dp variables
From: Emil Velikov @ 2020-02-20 11:13 UTC (permalink / raw)
To: Ville Syrjala; +Cc: Intel Graphics Development, ML dri-devel
In-Reply-To: <20200219203544.31013-4-ville.syrjala@linux.intel.com>
On Wed, 19 Feb 2020 at 20:36, Ville Syrjala
<ville.syrjala@linux.intel.com> wrote:
>
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> The drrs code dereferences mode->vrefresh via some really long chain
> of structures/pointers. Couldn't get coccinelle to see through all
> that so let's add some local variables to help it.
>
There's a limit to the madness that coccinelle can take :-P
Other drrs functions already use intel_dp, so might as well be consistent.
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
-Emil
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply
* Re: [PATCH 16/18] clocksource: Replace setup_irq() by request_irq()
From: Daniel Lezcano @ 2020-02-20 11:13 UTC (permalink / raw)
To: afzal mohammed, linux-kernel, linux-rpi-kernel, linux-arm-kernel,
linux-samsung-soc, linux-amlogic
Cc: Kate Stewart, Linus Walleij, Shawn Guo, Fabio Estevam,
Florian Fainelli, Kevin Hilman, Krzysztof Kozlowski, Kukjin Kim,
bcm-kernel-feedback-list, NXP Linux Team, Uwe Kleine-König,
Ray Jui, Sascha Hauer, Thomas Gleixner, Allison Randal,
Barry Song, Scott Branden, Greg Kroah-Hartman, Tony Prisk,
Pengutronix Kernel Team, Enrico Weigelt, Nicolas Saenz Julienne
In-Reply-To: <109d17402bc75ed186a2e151dfda1edf05463b5a.1581478324.git.afzal.mohd.ma@gmail.com>
On 12/02/2020 09:05, afzal mohammed wrote:
> request_irq() is preferred over setup_irq(). Existing callers of
> setup_irq() reached mostly via 'init_IRQ()' & 'time_init()', while
> memory allocators are ready by 'mm_init()'.
>
> Per tglx[1], setup_irq() existed in olden days when allocators were not
> ready by the time early interrupts were initialized.
>
> Hence replace setup_irq() by request_irq().
>
> Seldom remove_irq() usage has been observed coupled with setup_irq(),
> wherever that has been found, it too has been replaced by free_irq().
>
> [1] https://lkml.kernel.org/r/alpine.DEB.2.20.1710191609480.1971@nanos
>
> Signed-off-by: afzal mohammed <afzal.mohd.ma@gmail.com>
Nice, thanks for this cleanup
Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
[ ... ]
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [LTP] [PATCH] pty: Test data transmission with SLIP line discipline
From: Richard Palethorpe @ 2020-02-20 11:13 UTC (permalink / raw)
To: ltp
Basic transmission test and try to trigger a possible race between receiving
and hangup (it appears there is none from my testing).
This only includes SLIP for now, but more disciplines can be added. Probably
at least CAN. However the packet generation will be different for each.
Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
---
FYI, I considered patching the kernel so that SLIP sends unthrottle after each
read, but on some physical serial lines this will cause one of the pins to
change state which some crappy device was abusing for non-flow control
purposes. Also it may interfer with flow control implemented somewhere else in
the stack. I doubt this will really cause any problems, but OTOH I don't care
about making SLIP work nicely with blocking PTYs enough to risk any change to
the kernel. AFAICT there is no reason to use SLIP with a PTY except for
virtual testing of SLIP.
runtest/pty | 1 +
testcases/kernel/pty/.gitignore | 1 +
testcases/kernel/pty/pty04.c | 250 ++++++++++++++++++++++++++++++++
3 files changed, 252 insertions(+)
create mode 100644 testcases/kernel/pty/pty04.c
diff --git a/runtest/pty b/runtest/pty
index e484da0ff..5587312d3 100644
--- a/runtest/pty
+++ b/runtest/pty
@@ -2,6 +2,7 @@
pty01 pty01
pty02 pty02
pty03 pty03
+pty04 pty04
ptem01 ptem01
hangup01 hangup01
diff --git a/testcases/kernel/pty/.gitignore b/testcases/kernel/pty/.gitignore
index 5b4f6a8c9..c67d723d2 100644
--- a/testcases/kernel/pty/.gitignore
+++ b/testcases/kernel/pty/.gitignore
@@ -3,3 +3,4 @@
/pty01
/pty02
/pty03
+/pty04
diff --git a/testcases/kernel/pty/pty04.c b/testcases/kernel/pty/pty04.c
new file mode 100644
index 000000000..65d1d51d4
--- /dev/null
+++ b/testcases/kernel/pty/pty04.c
@@ -0,0 +1,250 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2020 SUSE
+ *
+ * Test transmitting data over a PTY/TTY line discipline and reading from the
+ * virtual netdev created by the line discipline. Also hangup the PTY while
+ * data is in flight to try to cause a race between the netdev being deleted
+ * and the discipline receive function writing to the netdev.
+ *
+ * Test flow:
+ * 1. Create PTY with ldisc X which creates netdev Y
+ * 2. Open raw packet socket and bind to netdev Y
+ * 3. Send data on ptmx and read packets from socket
+ * 4. Hangup while transmission in progress
+ *
+ * Note that not all line disciplines call unthrottle when they are ready to
+ * read more bytes. So it is possible to fill all the write buffers causing
+ * write to block forever (because once write sleeps it needs unthrottle to
+ * wake it). So we write with O_NONBLOCK.
+ *
+ * Also the max buffer size for PTYs is 8192, so even if the protocol MTU is
+ * greater everything may still be processed in 8129 byte chunks. At least
+ * until we are in the netdev code which can have a bigger buffer. Of course
+ * the MTU still decides exactly where the packet delimiter goes, this just
+ * concerns choosing the best packet size to cause a race.
+ */
+
+#define _GNU_SOURCE
+#include "tst_test.h"
+#include "config.h"
+
+#if defined(HAVE_LINUX_IF_PACKET_H) && defined(HAVE_LINUX_IF_ETHER_H)
+
+#include <linux/if_packet.h>
+#include <linux/if_ether.h>
+
+#include <stdlib.h>
+#include <stdio.h>
+#include <errno.h>
+#include <unistd.h>
+#include <termios.h>
+#include <sys/types.h>
+#include <sys/socket.h>
+#include <net/if.h>
+#include "lapi/ioctl.h"
+
+#include "tst_safe_stdio.h"
+
+struct ldisc_info {
+ int n;
+ char *name;
+ int max_mtu;
+};
+
+static struct ldisc_info ldiscs[] = {
+ {0, "TTY", 0},
+ {1, "SLIP", 8192},
+};
+
+static volatile int ptmx, pts, sk, mtu;
+
+static int set_ldisc(int tty, struct ldisc_info *ldisc)
+{
+ TEST(ioctl(tty, TIOCSETD, &ldisc->n));
+
+ if (!TST_RET)
+ return 0;
+
+ if (TST_ERR == EINVAL) {
+ tst_res(TCONF | TTERRNO,
+ "You don't appear to have the %s TTY line discipline",
+ ldisc->name);
+ } else {
+ tst_res(TBROK | TTERRNO,
+ "Failed to set the %s line discipline", ldisc->name);
+ }
+
+ return 1;
+}
+
+static void open_pty(struct ldisc_info *ldisc)
+{
+ char pts_path[PATH_MAX];
+
+ ptmx = SAFE_OPEN("/dev/ptmx", O_RDWR);
+ if (grantpt(ptmx))
+ tst_brk(TBROK | TERRNO, "grantpt(ptmx)");
+ if (unlockpt(ptmx))
+ tst_brk(TBROK | TERRNO, "unlockpt(ptmx)");
+ if (ptsname_r(ptmx, pts_path, sizeof(pts_path)))
+ tst_brk(TBROK | TERRNO, "ptsname_r(ptmx, ...)");
+
+ SAFE_FCNTL(ptmx, F_SETFL, O_NONBLOCK);
+
+ tst_res(TINFO, "PTS path is %s", pts_path);
+ pts = SAFE_OPEN(pts_path, O_RDWR);
+
+ set_ldisc(pts, ldisc);
+}
+
+static ssize_t try_write(int fd, char *data, ssize_t size, ssize_t *written)
+{
+ ssize_t ret = write(fd, data, size);
+
+ if (ret < 0)
+ return -(errno != EAGAIN);
+
+ return !written || (*written += ret) >= size;
+}
+
+static void write_pty(void)
+{
+ char *data = SAFE_MALLOC(mtu);
+ ssize_t written, ret;
+
+ memset(data, '_', mtu - 1);
+ data[mtu - 1] = 0300;
+
+ written = 0;
+ ret = TST_RETRY_FUNC(try_write(ptmx, data, mtu, &written), TST_RETVAL_NOTNULL);
+ if (ret < 0)
+ tst_brk(TBROK | TERRNO, "Failed 1st write to PTY");
+ tst_res(TPASS, "Wrote PTY 1");
+
+ written = 0;
+ ret = TST_RETRY_FUNC(try_write(ptmx, data, mtu, &written), TST_RETVAL_NOTNULL);
+ if (ret < 0)
+ tst_brk(TBROK | TERRNO, "Failed 2nd write to PTY");
+
+ if (tcflush(ptmx, TCIFLUSH))
+ tst_brk(TBROK | TERRNO, "tcflush(ptmx, TCIFLUSH)");
+
+ tst_res(TPASS, "Wrote PTY 2");
+
+ while (try_write(ptmx, data, mtu, NULL) >= 0)
+ ;
+
+ tst_res(TPASS, "Writing to PTY interrupted by hangup");
+
+ free(data);
+}
+
+static void open_netdev(struct ldisc_info *ldisc)
+{
+ struct ifreq ifreq = { 0 };
+ struct sockaddr_ll lla = { 0 };
+
+ SAFE_IOCTL(pts, SIOCGIFNAME, ifreq.ifr_name);
+ tst_res(TINFO, "Netdev is %s", ifreq.ifr_name);
+
+ sk = SAFE_SOCKET(PF_PACKET, SOCK_RAW, htons(ETH_P_ALL));
+
+ ifreq.ifr_mtu = ldisc->max_mtu;
+ if (ioctl(sk, SIOCSIFMTU, &ifreq))
+ tst_res(TWARN | TERRNO, "Failed to set netdev MTU to maximum");
+ SAFE_IOCTL(sk, SIOCGIFMTU, &ifreq);
+ mtu = ifreq.ifr_mtu;
+ tst_res(TINFO, "Netdev MTU is %d (we set %d)", mtu, ldisc->max_mtu);
+
+ SAFE_IOCTL(sk, SIOCGIFFLAGS, &ifreq);
+ ifreq.ifr_flags |= IFF_UP | IFF_RUNNING;
+ SAFE_IOCTL(sk, SIOCSIFFLAGS, &ifreq);
+ SAFE_IOCTL(sk, SIOCGIFFLAGS, &ifreq);
+
+ if (!(ifreq.ifr_flags & IFF_UP))
+ tst_brk(TBROK, "Netdev did not come up");
+
+ SAFE_IOCTL(sk, SIOCGIFINDEX, &ifreq);
+
+ lla.sll_family = PF_PACKET;
+ lla.sll_protocol = htons(ETH_P_ALL);
+ lla.sll_ifindex = ifreq.ifr_ifindex;
+ SAFE_BIND(sk, (struct sockaddr *)&lla, sizeof(struct sockaddr_ll));
+
+ tst_res(TINFO, "Bound netdev %d to socket %d", ifreq.ifr_ifindex, sk);
+}
+
+static void read_netdev(void)
+{
+ char *data = SAFE_MALLOC(mtu - 1);
+
+ tst_res(TINFO, "Reading from socket %d", sk);
+
+ SAFE_READ(1, sk, data, mtu - 1);
+ tst_res(TPASS, "Read netdev 1");
+ SAFE_READ(1, sk, data, mtu - 1);
+
+ tst_res(TPASS, "Read netdev 2");
+
+ TST_CHECKPOINT_WAKE(0);
+ while(read(sk, data, mtu - 1) > 0)
+ ;
+
+ tst_res(TPASS, "Reading data from netdev interrupted by hangup");
+
+ free(data);
+}
+
+static void do_test(unsigned int n)
+{
+ struct ldisc_info *ldisc = &ldiscs[n+1];
+
+ open_pty(ldisc);
+ open_netdev(ldisc);
+
+ if (!SAFE_FORK()) {
+ read_netdev();
+ return;
+ }
+
+ if (!SAFE_FORK()) {
+ write_pty();
+ return;
+ }
+
+ if (!SAFE_FORK()) {
+ TST_CHECKPOINT_WAIT(0);
+ SAFE_IOCTL(pts, TIOCVHANGUP);
+ tst_res(TINFO, "Sent hangup ioctl to PTS");
+ SAFE_IOCTL(ptmx, TIOCVHANGUP);
+ tst_res(TINFO, "Sent hangup ioctl to PTM");
+ return;
+ }
+
+ tst_reap_children();
+}
+
+static void cleanup(void)
+{
+ ioctl(pts, TIOCVHANGUP);
+ ioctl(ptmx, TIOCVHANGUP);
+
+ tst_reap_children();
+}
+
+static struct tst_test test = {
+ .test = do_test,
+ .cleanup = cleanup,
+ .tcnt = 1,
+ .forks_child = 1,
+ .needs_checkpoints = 1,
+ .needs_root = 1,
+ .min_kver = "4.10"
+};
+
+#else
+
+TST_TEST_TCONF("Need <linux/if_packet.h> and <linux/if_ether.h>");
+
+#endif
--
2.24.0
^ permalink raw reply related
* Re: [PATCH 16/18] clocksource: Replace setup_irq() by request_irq()
From: Daniel Lezcano @ 2020-02-20 11:13 UTC (permalink / raw)
To: afzal mohammed, linux-kernel, linux-rpi-kernel, linux-arm-kernel,
linux-samsung-soc, linux-amlogic
Cc: Thomas Gleixner, Florian Fainelli, Ray Jui, Scott Branden,
bcm-kernel-feedback-list, Nicolas Saenz Julienne, Kukjin Kim,
Krzysztof Kozlowski, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
Barry Song, Uwe Kleine-König, Kevin Hilman, Linus Walleij,
Tony Prisk, Allison Randal, Enrico Weigelt, Greg Kroah-Hartman,
Kate Stewart
In-Reply-To: <109d17402bc75ed186a2e151dfda1edf05463b5a.1581478324.git.afzal.mohd.ma@gmail.com>
On 12/02/2020 09:05, afzal mohammed wrote:
> request_irq() is preferred over setup_irq(). Existing callers of
> setup_irq() reached mostly via 'init_IRQ()' & 'time_init()', while
> memory allocators are ready by 'mm_init()'.
>
> Per tglx[1], setup_irq() existed in olden days when allocators were not
> ready by the time early interrupts were initialized.
>
> Hence replace setup_irq() by request_irq().
>
> Seldom remove_irq() usage has been observed coupled with setup_irq(),
> wherever that has been found, it too has been replaced by free_irq().
>
> [1] https://lkml.kernel.org/r/alpine.DEB.2.20.1710191609480.1971@nanos
>
> Signed-off-by: afzal mohammed <afzal.mohd.ma@gmail.com>
Nice, thanks for this cleanup
Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
[ ... ]
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply
* Re: Requesting review about optimizing large guest start up time
From: Paolo Bonzini @ 2020-02-20 11:11 UTC (permalink / raw)
To: bauerchen(陈蒙蒙), qemu-devel
In-Reply-To: <e9dfa1311de74824983e769ea197c2e6@tencent.com>
I queued the patch, thanks!
I simplified a bit the computation of the number of pages per thread, like this:
numpages_per_thread = numpages / memset_num_threads;
leftover = numpages % memset_num_threads;
for (i = 0; i < memset_num_threads; i++) {
memset_thread[i].addr = addr;
memset_thread[i].numpages = numpages_per_thread + (i < leftover);
memset_thread[i].hpagesize = hpagesize;
...
This removes the need for a separate array and function.
Thanks,
Paolo
On 11/02/20 13:08, bauerchen(陈蒙蒙) wrote:
> From c882b155466313fcd85ac330a45a573e608b0d74 Mon Sep 17 00:00:00 2001
> From: bauerchen <bauerchen@tencent.com>
> Date: Tue, 11 Feb 2020 17:10:35 +0800
> Subject: [PATCH] Optimize: large guest start-up in mem-prealloc
> MIME-Version: 1.0
> Content-Type: text/plain; charset=utf-8
> Content-Transfer-Encoding: 8bit
>
> [desc]:
> Large memory VM starts slowly when using -mem-prealloc, and
> there are some areas to optimize in current method;
>
> 1、mmap will be used to alloc threads stack during create page
> clearing threads, and it will attempt mm->mmap_sem for write
> lock, but clearing threads have hold read lock, this competition
> will cause threads createion very slow;
>
> 2、methods of calcuating pages for per threads is not well;if we use
> 64 threads to split 160 hugepage,63 threads clear 2page,1 thread
> clear 34 page,so the entire speed is very slow;
>
> to solve the first problem,we add a mutex in thread function,and
> start all threads when all threads finished createion;
> and the second problem, we spread remainder to other threads,in
> situation that 160 hugepage and 64 threads, there are 32 threads
> clear 3 pages,and 32 threads clear 2 pages;
> [test]:
> 320G 84c VM start time can be reduced to 10s
> 680G 84c VM start time can be reduced to 18s
>
> Signed-off-by: bauerchen <bauerchen@tencent.com>
> Reviewed-by:Pan Rui <ruippan@tencent.com>
> Reviewed-by:Ivan Ren <ivanren@tencent.com>
> ---
> util/oslib-posix.c | 44 ++++++++++++++++++++++++++++++++++++--------
> 1 file changed, 36 insertions(+), 8 deletions(-)
>
> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
> index 5a291cc..e97369b 100644
> --- a/util/oslib-posix.c
> +++ b/util/oslib-posix.c
> @@ -76,6 +76,10 @@ static MemsetThread *memset_thread;
> static int memset_num_threads;
> static bool memset_thread_failed;
>
> +static QemuMutex page_mutex;
> +static QemuCond page_cond;
> +static volatile bool thread_create_flag;
> +
> int qemu_get_thread_id(void)
> {
> #if defined(__linux__)
> @@ -403,6 +407,14 @@ static void *do_touch_pages(void *arg)
> MemsetThread *memset_args = (MemsetThread *)arg;
> sigset_t set, oldset;
>
> + /*wait for all threads create finished */
> + qemu_mutex_lock(&page_mutex);
> + while(!thread_create_flag){
> + qemu_cond_wait(&page_cond, &page_mutex);
> + }
> + qemu_mutex_unlock(&page_mutex);
> +
> +
> /* unblock SIGBUS */
> sigemptyset(&set);
> sigaddset(&set, SIGBUS);
> @@ -448,30 +460,46 @@ static inline int get_memset_num_threads(int smp_cpus)
> return ret;
> }
>
> +static void calc_page_per_thread(size_t numpages, int memset_threads, size_t *pages_per_thread){
> + int avg = numpages / memset_threads + 1;
> + int i = 0;
> + int last = avg * memset_threads - numpages;
> + for (i = 0; i < memset_threads; i++)
> + {
> + if(memset_threads - i <= last){
> + pages_per_thread[i] = avg - 1;
> + }else
> + pages_per_thread[i] = avg;
> + }
> +}
> +
> static bool touch_all_pages(char *area, size_t hpagesize, size_t numpages,
> int smp_cpus)
> {
> - size_t numpages_per_thread;
> - size_t size_per_thread;
> + size_t *numpages_per_thread;
> char *addr = area;
> int i = 0;
>
> memset_thread_failed = false;
> + thread_create_flag = false;
> memset_num_threads = get_memset_num_threads(smp_cpus);
> + numpages_per_thread = g_new0(size_t, memset_num_threads);
> memset_thread = g_new0(MemsetThread, memset_num_threads);
> - numpages_per_thread = (numpages / memset_num_threads);
> - size_per_thread = (hpagesize * numpages_per_thread);
> + calc_page_per_thread(numpages, memset_num_threads, numpages_per_thread);
> +
> for (i = 0; i < memset_num_threads; i++) {
> memset_thread[i].addr = addr;
> - memset_thread[i].numpages = (i == (memset_num_threads - 1)) ?
> - numpages : numpages_per_thread;
> + memset_thread[i].numpages = numpages_per_thread[i];
> memset_thread[i].hpagesize = hpagesize;
> qemu_thread_create(&memset_thread[i].pgthread, "touch_pages",
> do_touch_pages, &memset_thread[i],
> QEMU_THREAD_JOINABLE);
> - addr += size_per_thread;
> - numpages -= numpages_per_thread;
> + addr += numpages_per_thread[i] * hpagesize;
> + numpages -= numpages_per_thread[i];
> }
> + thread_create_flag = true;
> + qemu_cond_broadcast(&page_cond);
> +
> for (i = 0; i < memset_num_threads; i++) {
> qemu_thread_join(&memset_thread[i].pgthread);
> }
> --
> 1.8.3.1
>
^ permalink raw reply
* Re: [PATCH net-next v3 1/9] sysfs: add sysfs_file_change_owner{_by_name}()
From: Greg Kroah-Hartman @ 2020-02-20 11:11 UTC (permalink / raw)
To: Christian Brauner
Cc: David S. Miller, linux-kernel, netdev, Rafael J. Wysocki,
Pavel Machek, Jakub Kicinski, Eric Dumazet, Stephen Hemminger,
linux-pm
In-Reply-To: <20200218162943.2488012-2-christian.brauner@ubuntu.com>
On Tue, Feb 18, 2020 at 05:29:35PM +0100, Christian Brauner wrote:
> Add helpers to change the owner of a sysfs files.
> This function will be used to correctly account for kobject ownership
> changes, e.g. when moving network devices between network namespaces.
>
> Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
> ---
> /* v2 */
> - Greg Kroah-Hartman <gregkh@linuxfoundation.org>:
> - Better naming for sysfs_file_change_owner() to reflect the fact that it
> can be used to change the owner of the kobject itself by passing NULL as
> argument.
> - Christian Brauner <christian.brauner@ubuntu.com>:
> - Split sysfs_file_change_owner() into two helpers sysfs_change_owner() and
> sysfs_change_owner_by_name(). The former changes the owner of the kobject
> itself, the latter the owner of the kobject looked up via the name
> argument.
>
> /* v3 */
> - Greg Kroah-Hartman <gregkh@linuxfoundation.org>:
> - Add explicit uid/gid parameters.
Looks much better, thanks for doing these changes. I'll review the
whole series now...
greg k-h
^ permalink raw reply
* Re: [PATCH net-next v3 1/9] sysfs: add sysfs_file_change_owner{_by_name}()
From: Greg Kroah-Hartman @ 2020-02-20 11:10 UTC (permalink / raw)
To: Christian Brauner
Cc: David S. Miller, linux-kernel, netdev, Rafael J. Wysocki,
Pavel Machek, Jakub Kicinski, Eric Dumazet, Stephen Hemminger,
linux-pm
In-Reply-To: <20200218162943.2488012-2-christian.brauner@ubuntu.com>
On Tue, Feb 18, 2020 at 05:29:35PM +0100, Christian Brauner wrote:
> Add helpers to change the owner of a sysfs files.
> This function will be used to correctly account for kobject ownership
> changes, e.g. when moving network devices between network namespaces.
>
> Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
^ permalink raw reply
* Re: [Xen-devel] [PATCH 1/2] xen/mm: fold PGC_broken into PGC_state bits
From: Jan Beulich @ 2020-02-20 11:10 UTC (permalink / raw)
To: David Woodhouse
Cc: Stefano Stabellini, Julien Grall, Wei Liu, Konrad Rzeszutek Wilk,
George Dunlap, Andrew Cooper, Ian Jackson, George Dunlap,
Jeff Kubascik, Stewart Hildebrand, xen-devel
In-Reply-To: <20200207155701.2781820-1-dwmw2@infradead.org>
On 07.02.2020 16:57, David Woodhouse wrote:
> @@ -1145,16 +1145,19 @@ static int reserve_offlined_page(struct page_info *head)
>
> for ( cur_head = head; cur_head < head + ( 1UL << head_order); cur_head++ )
> {
> - if ( !page_state_is(cur_head, offlined) )
> + struct page_list_head *list;
> + if ( page_state_is(cur_head, offlined) )
> + list = &page_offlined_list;
> + else if (page_state_is(cur_head, broken) )
> + list = &page_broken_list;
> + else
> continue;
>
> avail[node][zone]--;
> total_avail_pages--;
> ASSERT(total_avail_pages >= 0);
>
> - page_list_add_tail(cur_head,
> - test_bit(_PGC_broken, &cur_head->count_info) ?
> - &page_broken_list : &page_offlined_list);
> + page_list_add_tail(cur_head, list);
While I realize it's fewer comparisons this way, I still wonder
whether for the reader's sake it wouldn't better be
page_is_offlined() first and then page_is_broken() down here.
> @@ -1699,14 +1714,14 @@ unsigned int online_page(mfn_t mfn, uint32_t *status)
> do {
> ret = *status = 0;
>
> - if ( y & PGC_broken )
> + if ( (y & PGC_state) == PGC_state_broken ||
> + (y & PGC_state) == PGC_state_broken_offlining )
> {
> ret = -EINVAL;
> *status = PG_ONLINE_FAILED |PG_ONLINE_BROKEN;
> break;
> }
> -
> - if ( (y & PGC_state) == PGC_state_offlined )
> + else if ( (y & PGC_state) == PGC_state_offlined )
I don't see a need for adding "else" here.
> --- a/xen/include/asm-x86/mm.h
> +++ b/xen/include/asm-x86/mm.h
> @@ -67,18 +67,27 @@
> /* 3-bit PAT/PCD/PWT cache-attribute hint. */
> #define PGC_cacheattr_base PG_shift(6)
> #define PGC_cacheattr_mask PG_mask(7, 6)
> - /* Page is broken? */
> -#define _PGC_broken PG_shift(7)
> -#define PGC_broken PG_mask(1, 7)
> - /* Mutually-exclusive page states: { inuse, offlining, offlined, free }. */
> -#define PGC_state PG_mask(3, 9)
> -#define PGC_state_inuse PG_mask(0, 9)
> -#define PGC_state_offlining PG_mask(1, 9)
> -#define PGC_state_offlined PG_mask(2, 9)
> -#define PGC_state_free PG_mask(3, 9)
> -#define page_state_is(pg, st) (((pg)->count_info&PGC_state) == PGC_state_##st)
> -
> - /* Count of references to this frame. */
> + /*
> + * Mutually-exclusive page states:
> + * { inuse, offlining, offlined, free, broken_offlining, broken }
> + */
> +#define PGC_state PG_mask(7, 9)
> +#define PGC_state_inuse PG_mask(0, 9)
> +#define PGC_state_offlining PG_mask(1, 9)
> +#define PGC_state_offlined PG_mask(2, 9)
> +#define PGC_state_free PG_mask(3, 9)
> +#define PGC_state_broken_offlining PG_mask(4, 9)
TBH I'd prefer PGC_state_offlining_broken, as it's not the
offlining which is broken, but a broken page is being
offlined.
> +#define PGC_state_broken PG_mask(5, 9)
> +
> +#define page_state_is(pg, st) (((pg)->count_info&PGC_state) == PGC_state_##st)
Blanks around & please.
> +#define page_is_broken(pg) (page_state_is((pg), broken_offlining) || \
> + page_state_is((pg), broken))
> +#define page_is_offlined(pg) (page_state_is((pg), broken) || \
> + page_state_is((pg), offlined))
The inclusion of "broken" here would seem to deserve a (brief)
comment, either here or next to PGC_state_broken's #define.
> +#define page_is_offlining(pg) (page_state_is((pg), broken_offlining) || \
> + page_state_is((pg), offlining))
Overall I wonder whether the PGC_state_* ordering couldn't be
adjusted such that at least some of these three won't need
two comparisons (by masking off a bit before comparing).
Also for all three - no need for extra parentheses around pg
(or in general macro arguments which get handed on without
being part of an expression).
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply
* [PATCH v2 1/2] sparc,x86: vdso: remove meaningless undefining CONFIG_OPTIMIZE_INLINING
From: Masahiro Yamada @ 2020-02-20 11:08 UTC (permalink / raw)
To: Andrew Morton
Cc: Arnd Bergmann, Ingo Molnar, Thomas Gleixner, x86,
clang-built-linux, Miguel Ojeda, sparclinux, Masahiro Yamada,
Andy Lutomirski, Borislav Petkov, David S. Miller, H. Peter Anvin,
linux-kernel
The code, #undef CONFIG_OPTIMIZE_INLINING, is not working as expected
because <linux/compiler_types.h> is parsed before vclock_gettime.c
since 28128c61e08e ("kconfig.h: Include compiler types to avoid missed
struct attributes").
Since then, <linux/compiler_types.h> is included really early by
using the '-include' option. So, you cannot negate the decision of
<linux/compiler_types.h> in this way.
You can confirm it by checking the pre-processed code, like this:
$ make arch/x86/entry/vdso/vdso32/vclock_gettime.i
There is no difference with/without CONFIG_CC_OPTIMIZE_FOR_SIZE.
It is about two years since 28128c61e08e. Nobody has reported a
problem (or, nobody has even noticed the fact that this code is not
working).
It is ugly and unreliable to attempt to undefine a CONFIG option from
C files, and anyway the inlining heuristic is up to the compiler.
Just remove the broken code.
Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
Acked-by: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
---
Changes in v2:
- fix a type
- add Acked-by
arch/sparc/vdso/vdso32/vclock_gettime.c | 4 ----
arch/x86/entry/vdso/vdso32/vclock_gettime.c | 4 ----
2 files changed, 8 deletions(-)
diff --git a/arch/sparc/vdso/vdso32/vclock_gettime.c b/arch/sparc/vdso/vdso32/vclock_gettime.c
index 026abb3b826c..d7f99e6745ea 100644
--- a/arch/sparc/vdso/vdso32/vclock_gettime.c
+++ b/arch/sparc/vdso/vdso32/vclock_gettime.c
@@ -4,10 +4,6 @@
#define BUILD_VDSO32
-#ifndef CONFIG_CC_OPTIMIZE_FOR_SIZE
-#undef CONFIG_OPTIMIZE_INLINING
-#endif
-
#ifdef CONFIG_SPARC64
/*
diff --git a/arch/x86/entry/vdso/vdso32/vclock_gettime.c b/arch/x86/entry/vdso/vdso32/vclock_gettime.c
index 9242b28418d5..3c26488db94d 100644
--- a/arch/x86/entry/vdso/vdso32/vclock_gettime.c
+++ b/arch/x86/entry/vdso/vdso32/vclock_gettime.c
@@ -1,10 +1,6 @@
// SPDX-License-Identifier: GPL-2.0
#define BUILD_VDSO32
-#ifndef CONFIG_CC_OPTIMIZE_FOR_SIZE
-#undef CONFIG_OPTIMIZE_INLINING
-#endif
-
#ifdef CONFIG_X86_64
/*
--
2.17.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.