* Re: [EXT] Re: [PATCH] bus: fsl-mc: Add ACPI support for fsl-mc
From: Lorenzo Pieralisi @ 2020-02-14 17:49 UTC (permalink / raw)
To: Pankaj Bansal
Cc: Marc Zyngier, Ard Biesheuvel, Makarand Pawagi, Calvin Johnson,
stuyoder@gmail.com, nleeder@codeaurora.org, Ioana Ciornei,
Cristi Sovaiala, Hanjun Guo, Will Deacon, jon@solid-run.com,
Russell King, ACPI Devel Maling List, Len Brown, Jason Cooper,
Andy Wang, Varun Sethi, Thomas Gleixner, linux-arm-kernel,
Laurentiu Tudor, Paul Yang, netdev@vger.kernel.org,
Rafael J. Wysocki, Linux Kernel Mailing List,
Shameerali Kolothum Thodi, Sudeep Holla, Robin Murphy
In-Reply-To: <VI1PR0401MB2496800C88A3A2CF912959E6F1150@VI1PR0401MB2496.eurprd04.prod.outlook.com>
On Fri, Feb 14, 2020 at 04:35:10PM +0000, Pankaj Bansal wrote:
[...]
> > -----Original Message-----
> > From: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > Sent: Friday, February 14, 2020 9:50 PM
> > To: Pankaj Bansal <pankaj.bansal@nxp.com>
> > Cc: Marc Zyngier <maz@kernel.org>; Ard Biesheuvel
> > <ard.biesheuvel@linaro.org>; Makarand Pawagi <makarand.pawagi@nxp.com>;
> > Calvin Johnson <calvin.johnson@nxp.com>; stuyoder@gmail.com;
> > nleeder@codeaurora.org; Ioana Ciornei <ioana.ciornei@nxp.com>; Cristi
> > Sovaiala <cristian.sovaiala@nxp.com>; Hanjun Guo <guohanjun@huawei.com>;
> > Will Deacon <will@kernel.org>; jon@solid-run.com; Russell King
> > <linux@armlinux.org.uk>; ACPI Devel Maling List <linux-acpi@vger.kernel.org>;
> > Len Brown <lenb@kernel.org>; Jason Cooper <jason@lakedaemon.net>; Andy
> > Wang <Andy.Wang@arm.com>; Varun Sethi <V.Sethi@nxp.com>; Thomas
> > Gleixner <tglx@linutronix.de>; linux-arm-kernel <linux-arm-
> > kernel@lists.infradead.org>; Laurentiu Tudor <laurentiu.tudor@nxp.com>; Paul
> > Yang <Paul.Yang@arm.com>; netdev@vger.kernel.org; Rafael J. Wysocki
> > <rjw@rjwysocki.net>; Linux Kernel Mailing List <linux-kernel@vger.kernel.org>;
> > Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>;
> > Sudeep Holla <sudeep.holla@arm.com>; Robin Murphy
> > <robin.murphy@arm.com>
> > Subject: Re: [EXT] Re: [PATCH] bus: fsl-mc: Add ACPI support for fsl-mc
Side note: would you mind removing the email headers (as above) in your
replies please ?
> > On Fri, Feb 14, 2020 at 03:58:14PM +0000, Pankaj Bansal wrote:
> >
> > [...]
> >
> > > > Why should the device know about its own ID? That's a bus/interconnect
> > thing.
> > > > And nothing should be passed *to* IORT. IORT is the source.
> > >
> > > IORT is translation between Input IDs <-> Output IDs. The Input ID is still
> > expected to be passed to parse IORT table.
> >
> > Named components use an array of single mappings (as in entries with single
> > mapping flag set) - Input ID is irrelevant.
> >
> > Not sure what your named component is though and what you want to do with
> > it, the fact that IORT allows mapping for named components do not necessarily
> > mean that it can describe what your system really is, on that you need to
> > elaborate for us to be able to help.
>
> Details about MC bus can be read from here:
> https://elixir.bootlin.com/linux/latest/source/Documentation/networking/device_drivers/freescale/dpaa2/overview.rst#L324
>
> As stated above, in Linux MC is a bus (just like PCI bus, AMBA bus etc)
> There can be multiple devices attached to this bus. Moreover, we can dynamically create/destroy these devices.
> Now, we want to represent this BUS (not individual devices connected to bus) in IORT table.
> The only possible way right now we see is that we describe it as Named components having a pool of ID mappings.
> As and when devices are created and attached to bus, we sift through this pool to correctly determine the output ID for the device.
> Now the input ID that we provide, can come from device itself.
> Then we can use the Platform MSI framework for MC bus devices.
So are you asking me if that's OK ? Or there is something you can't
describe with IORT ?
Side note: can you explain to me please how the MSI allocation flow
and kernel data structures/drivers are modeled in DT ? I had a quick
look at:
drivers/irqchip/irq-gic-v3-its-fsl-mc-msi.c
and to start with, does that code imply that we create a
DOMAIN_BUS_FSL_MC_MSI on ALL DT systems with an ITS device node ?
I *think* you have a specific API to allocate MSIs for MC devices:
fsl_mc_msi_domain_alloc_irqs()
which hook into the IRQ domain created in the file above that handles
the cascading to an ITS domain, correct ?
Thanks,
Lorenzo
^ permalink raw reply
* [PATCH AUTOSEL 5.5 482/542] bpf: map_seq_next should always increase position index
From: Sasha Levin @ 2020-02-14 15:47 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Vasily Averin, Daniel Borkmann, Sasha Levin, netdev, bpf
In-Reply-To: <20200214154854.6746-1-sashal@kernel.org>
From: Vasily Averin <vvs@virtuozzo.com>
[ Upstream commit 90435a7891a2259b0f74c5a1bc5600d0d64cba8f ]
If seq_file .next fuction does not change position index,
read after some lseek can generate an unexpected output.
See also: https://bugzilla.kernel.org/show_bug.cgi?id=206283
v1 -> v2: removed missed increment in end of function
Signed-off-by: Vasily Averin <vvs@virtuozzo.com>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Link: https://lore.kernel.org/bpf/eca84fdd-c374-a154-d874-6c7b55fc3bc4@virtuozzo.com
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
kernel/bpf/inode.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/kernel/bpf/inode.c b/kernel/bpf/inode.c
index ecf42bec38c00..6f22e0e74ef24 100644
--- a/kernel/bpf/inode.c
+++ b/kernel/bpf/inode.c
@@ -196,6 +196,7 @@ static void *map_seq_next(struct seq_file *m, void *v, loff_t *pos)
void *key = map_iter(m)->key;
void *prev_key;
+ (*pos)++;
if (map_iter(m)->done)
return NULL;
@@ -208,8 +209,6 @@ static void *map_seq_next(struct seq_file *m, void *v, loff_t *pos)
map_iter(m)->done = true;
return NULL;
}
-
- ++(*pos);
return key;
}
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 5.5 484/542] mwifiex: Fix possible buffer overflows in mwifiex_ret_wmm_get_status()
From: Sasha Levin @ 2020-02-14 15:47 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Qing Xu, Kalle Valo, Sasha Levin, linux-wireless, netdev
In-Reply-To: <20200214154854.6746-1-sashal@kernel.org>
From: Qing Xu <m1s5p6688@gmail.com>
[ Upstream commit 3a9b153c5591548612c3955c9600a98150c81875 ]
mwifiex_ret_wmm_get_status() calls memcpy() without checking the
destination size.Since the source is given from remote AP which
contains illegal wmm elements , this may trigger a heap buffer
overflow.
Fix it by putting the length check before calling memcpy().
Signed-off-by: Qing Xu <m1s5p6688@gmail.com>
Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/net/wireless/marvell/mwifiex/wmm.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/net/wireless/marvell/mwifiex/wmm.c b/drivers/net/wireless/marvell/mwifiex/wmm.c
index 41f0231376c01..132f9e8ed68c1 100644
--- a/drivers/net/wireless/marvell/mwifiex/wmm.c
+++ b/drivers/net/wireless/marvell/mwifiex/wmm.c
@@ -970,6 +970,10 @@ int mwifiex_ret_wmm_get_status(struct mwifiex_private *priv,
"WMM Parameter Set Count: %d\n",
wmm_param_ie->qos_info_bitmap & mask);
+ if (wmm_param_ie->vend_hdr.len + 2 >
+ sizeof(struct ieee_types_wmm_parameter))
+ break;
+
memcpy((u8 *) &priv->curr_bss_params.bss_descriptor.
wmm_ie, wmm_param_ie,
wmm_param_ie->vend_hdr.len + 2);
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 5.5 485/542] mwifiex: Fix possible buffer overflows in mwifiex_cmd_append_vsie_tlv()
From: Sasha Levin @ 2020-02-14 15:47 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Qing Xu, Kalle Valo, Sasha Levin, linux-wireless, netdev
In-Reply-To: <20200214154854.6746-1-sashal@kernel.org>
From: Qing Xu <m1s5p6688@gmail.com>
[ Upstream commit b70261a288ea4d2f4ac7cd04be08a9f0f2de4f4d ]
mwifiex_cmd_append_vsie_tlv() calls memcpy() without checking
the destination size may trigger a buffer overflower,
which a local user could use to cause denial of service
or the execution of arbitrary code.
Fix it by putting the length check before calling memcpy().
Signed-off-by: Qing Xu <m1s5p6688@gmail.com>
Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/net/wireless/marvell/mwifiex/scan.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/net/wireless/marvell/mwifiex/scan.c b/drivers/net/wireless/marvell/mwifiex/scan.c
index 98f942b797f7b..a7968a84aaf88 100644
--- a/drivers/net/wireless/marvell/mwifiex/scan.c
+++ b/drivers/net/wireless/marvell/mwifiex/scan.c
@@ -2884,6 +2884,13 @@ mwifiex_cmd_append_vsie_tlv(struct mwifiex_private *priv,
vs_param_set->header.len =
cpu_to_le16((((u16) priv->vs_ie[id].ie[1])
& 0x00FF) + 2);
+ if (le16_to_cpu(vs_param_set->header.len) >
+ MWIFIEX_MAX_VSIE_LEN) {
+ mwifiex_dbg(priv->adapter, ERROR,
+ "Invalid param length!\n");
+ break;
+ }
+
memcpy(vs_param_set->ie, priv->vs_ie[id].ie,
le16_to_cpu(vs_param_set->header.len));
*buffer += le16_to_cpu(vs_param_set->header.len) +
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 5.4 382/459] powerpc/ptdump: Only enable PPC_CHECK_WX with STRICT_KERNEL_RWX
From: Sasha Levin @ 2020-02-14 16:00 UTC (permalink / raw)
To: linux-kernel, stable; +Cc: linuxppc-dev, Sasha Levin
In-Reply-To: <20200214160149.11681-1-sashal@kernel.org>
From: Christophe Leroy <christophe.leroy@c-s.fr>
[ Upstream commit f509247b08f2dcf7754d9ed85ad69a7972aa132b ]
ptdump_check_wx() is called from mark_rodata_ro() which only exists
when CONFIG_STRICT_KERNEL_RWX is selected.
Fixes: 453d87f6a8ae ("powerpc/mm: Warn if W+X pages found on boot")
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/922d4939c735c6b52b4137838bcc066fffd4fc33.1578989545.git.christophe.leroy@c-s.fr
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/powerpc/Kconfig.debug | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/Kconfig.debug b/arch/powerpc/Kconfig.debug
index c59920920ddc4..b915fe6589792 100644
--- a/arch/powerpc/Kconfig.debug
+++ b/arch/powerpc/Kconfig.debug
@@ -371,7 +371,7 @@ config PPC_PTDUMP
config PPC_DEBUG_WX
bool "Warn on W+X mappings at boot"
- depends on PPC_PTDUMP
+ depends on PPC_PTDUMP && STRICT_KERNEL_RWX
help
Generate a warning if any W+X mappings are found at boot.
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 5.5 483/542] powerpc/mm: Don't log user reads to 0xffffffff
From: Sasha Levin @ 2020-02-14 15:47 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Christophe Leroy, Michael Ellerman, Sasha Levin, linuxppc-dev
In-Reply-To: <20200214154854.6746-1-sashal@kernel.org>
From: Christophe Leroy <christophe.leroy@c-s.fr>
[ Upstream commit 0f9aee0cb9da7db7d96f63cfa2dc5e4f1bffeb87 ]
Running vdsotest leaves many times the following log:
[ 79.629901] vdsotest[396]: User access of kernel address (ffffffff) - exploit attempt? (uid: 0)
A pointer set to (-1) is likely a programming error similar to
a NULL pointer and is not worth logging as an exploit attempt.
Don't log user accesses to 0xffffffff.
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/0728849e826ba16f1fbd6fa7f5c6cc87bd64e097.1577087627.git.christophe.leroy@c-s.fr
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/powerpc/mm/fault.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
index 1baeb045f7f4b..e083a9f67f701 100644
--- a/arch/powerpc/mm/fault.c
+++ b/arch/powerpc/mm/fault.c
@@ -354,6 +354,9 @@ static void sanity_check_fault(bool is_write, bool is_user,
* Userspace trying to access kernel address, we get PROTFAULT for that.
*/
if (is_user && address >= TASK_SIZE) {
+ if ((long)address == -1)
+ return;
+
pr_crit_ratelimited("%s[%d]: User access of kernel address (%lx) - exploit attempt? (uid: %d)\n",
current->comm, current->pid, address,
from_kuid(&init_user_ns, current_uid()));
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 5.5 490/542] IB/mlx4: Fix leak in id_map_find_del
From: Sasha Levin @ 2020-02-14 15:48 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Håkon Bugge, Manjunath Patil, Rama Nichanamatlu,
Jack Morgenstein, Jason Gunthorpe, Sasha Levin, linux-rdma
In-Reply-To: <20200214154854.6746-1-sashal@kernel.org>
From: Håkon Bugge <haakon.bugge@oracle.com>
[ Upstream commit ea660ad7c1c476fd6e5e3b17780d47159db71dea ]
Using CX-3 virtual functions, either from a bare-metal machine or
pass-through from a VM, MAD packets are proxied through the PF driver.
Since the VF drivers have separate name spaces for MAD Transaction Ids
(TIDs), the PF driver has to re-map the TIDs and keep the book keeping in
a cache.
Following the RDMA Connection Manager (CM) protocol, it is clear when an
entry has to evicted from the cache. When a DREP is sent from
mlx4_ib_multiplex_cm_handler(), id_map_find_del() is called. Similar when
a REJ is received by the mlx4_ib_demux_cm_handler(), id_map_find_del() is
called.
This function wipes out the TID in use from the IDR or XArray and removes
the id_map_entry from the table.
In short, it does everything except the topping of the cake, which is to
remove the entry from the list and free it. In other words, for the REJ
case enumerated above, one id_map_entry will be leaked.
For the other case above, a DREQ has been received first. The reception of
the DREQ will trigger queuing of a delayed work to delete the
id_map_entry, for the case where the VM doesn't send back a DREP.
In the normal case, the VM _will_ send back a DREP, and id_map_find_del()
will be called.
But this scenario introduces a secondary leak. First, when the DREQ is
received, a delayed work is queued. The VM will then return a DREP, which
will call id_map_find_del(). As stated above, this will free the TID used
from the XArray or IDR. Now, there is window where that particular TID can
be re-allocated, lets say by an outgoing REQ. This TID will later be wiped
out by the delayed work, when the function id_map_ent_timeout() is
called. But the id_map_entry allocated by the outgoing REQ will not be
de-allocated, and we have a leak.
Both leaks are fixed by removing the id_map_find_del() function and only
using schedule_delayed(). Of course, a check in schedule_delayed() to see
if the work already has been queued, has been added.
Another benefit of always using the delayed version for deleting entries,
is that we do get a TimeWait effect; a TID no longer in use, will occupy
the XArray or IDR for CM_CLEANUP_CACHE_TIMEOUT time, without any ability
of being re-used for that time period.
Fixes: 3cf69cc8dbeb ("IB/mlx4: Add CM paravirtualization")
Link: https://lore.kernel.org/r/20200123155521.1212288-1-haakon.bugge@oracle.com
Signed-off-by: Håkon Bugge <haakon.bugge@oracle.com>
Signed-off-by: Manjunath Patil <manjunath.b.patil@oracle.com>
Reviewed-by: Rama Nichanamatlu <rama.nichanamatlu@oracle.com>
Reviewed-by: Jack Morgenstein <jackm@dev.mellanox.co.il>
Signed-off-by: Jason Gunthorpe <jgg@mellanox.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/infiniband/hw/mlx4/cm.c | 29 +++--------------------------
1 file changed, 3 insertions(+), 26 deletions(-)
diff --git a/drivers/infiniband/hw/mlx4/cm.c b/drivers/infiniband/hw/mlx4/cm.c
index ecd6cadd529a5..b591861934b3c 100644
--- a/drivers/infiniband/hw/mlx4/cm.c
+++ b/drivers/infiniband/hw/mlx4/cm.c
@@ -186,23 +186,6 @@ static void id_map_ent_timeout(struct work_struct *work)
kfree(ent);
}
-static void id_map_find_del(struct ib_device *ibdev, int pv_cm_id)
-{
- struct mlx4_ib_sriov *sriov = &to_mdev(ibdev)->sriov;
- struct rb_root *sl_id_map = &sriov->sl_id_map;
- struct id_map_entry *ent, *found_ent;
-
- spin_lock(&sriov->id_map_lock);
- ent = xa_erase(&sriov->pv_id_table, pv_cm_id);
- if (!ent)
- goto out;
- found_ent = id_map_find_by_sl_id(ibdev, ent->slave_id, ent->sl_cm_id);
- if (found_ent && found_ent == ent)
- rb_erase(&found_ent->node, sl_id_map);
-out:
- spin_unlock(&sriov->id_map_lock);
-}
-
static void sl_id_map_add(struct ib_device *ibdev, struct id_map_entry *new)
{
struct rb_root *sl_id_map = &to_mdev(ibdev)->sriov.sl_id_map;
@@ -294,7 +277,7 @@ static void schedule_delayed(struct ib_device *ibdev, struct id_map_entry *id)
spin_lock(&sriov->id_map_lock);
spin_lock_irqsave(&sriov->going_down_lock, flags);
/*make sure that there is no schedule inside the scheduled work.*/
- if (!sriov->is_going_down) {
+ if (!sriov->is_going_down && !id->scheduled_delete) {
id->scheduled_delete = 1;
schedule_delayed_work(&id->timeout, CM_CLEANUP_CACHE_TIMEOUT);
}
@@ -341,9 +324,6 @@ int mlx4_ib_multiplex_cm_handler(struct ib_device *ibdev, int port, int slave_id
if (mad->mad_hdr.attr_id == CM_DREQ_ATTR_ID)
schedule_delayed(ibdev, id);
- else if (mad->mad_hdr.attr_id == CM_DREP_ATTR_ID)
- id_map_find_del(ibdev, pv_cm_id);
-
return 0;
}
@@ -382,12 +362,9 @@ int mlx4_ib_demux_cm_handler(struct ib_device *ibdev, int port, int *slave,
*slave = id->slave_id;
set_remote_comm_id(mad, id->sl_cm_id);
- if (mad->mad_hdr.attr_id == CM_DREQ_ATTR_ID)
+ if (mad->mad_hdr.attr_id == CM_DREQ_ATTR_ID ||
+ mad->mad_hdr.attr_id == CM_REJ_ATTR_ID)
schedule_delayed(ibdev, id);
- else if (mad->mad_hdr.attr_id == CM_REJ_ATTR_ID ||
- mad->mad_hdr.attr_id == CM_DREP_ATTR_ID) {
- id_map_find_del(ibdev, (int) pv_cm_id);
- }
return 0;
}
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 5.5 491/542] drm/amd/display: do not allocate display_mode_lib unnecessarily
From: Sasha Levin @ 2020-02-14 15:48 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Dor Askayo, Leo Li, Alex Deucher, Sasha Levin, amd-gfx, dri-devel
In-Reply-To: <20200214154854.6746-1-sashal@kernel.org>
From: Dor Askayo <dor.askayo@gmail.com>
[ Upstream commit bb67bfd2e7101bf2ac5327b0b7a847cd9fb9723f ]
This allocation isn't required and can fail when resuming from suspend.
Bug: https://gitlab.freedesktop.org/drm/amd/issues/1009
Signed-off-by: Dor Askayo <dor.askayo@gmail.com>
Reviewed-by: Leo Li <sunpeng.li@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/gpu/drm/amd/display/dc/core/dc.c | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c b/drivers/gpu/drm/amd/display/dc/core/dc.c
index 32f31bf919151..8904a85186aab 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
@@ -2396,12 +2396,7 @@ void dc_set_power_state(
enum dc_acpi_cm_power_state power_state)
{
struct kref refcount;
- struct display_mode_lib *dml = kzalloc(sizeof(struct display_mode_lib),
- GFP_KERNEL);
-
- ASSERT(dml);
- if (!dml)
- return;
+ struct display_mode_lib *dml;
switch (power_state) {
case DC_ACPI_CM_POWER_STATE_D0:
@@ -2423,6 +2418,12 @@ void dc_set_power_state(
* clean state, and dc hw programming optimizations will not
* cause any trouble.
*/
+ dml = kzalloc(sizeof(struct display_mode_lib),
+ GFP_KERNEL);
+
+ ASSERT(dml);
+ if (!dml)
+ return;
/* Preserve refcount */
refcount = dc->current_state->refcount;
@@ -2436,10 +2437,10 @@ void dc_set_power_state(
dc->current_state->refcount = refcount;
dc->current_state->bw_ctx.dml = *dml;
+ kfree(dml);
+
break;
}
-
- kfree(dml);
}
void dc_resume(struct dc *dc)
--
2.20.1
^ permalink raw reply related
* Re: [PATCH 09/11] drm, cgroup: Introduce lgpu as DRM cgroup resource
From: Jason Ekstrand @ 2020-02-14 17:48 UTC (permalink / raw)
To: Kenny Ho
Cc: juan.zuniga-anaya, Kenny Ho, Kuehling, Felix, jsparks,
amd-gfx mailing list, lkaplan, Alex Deucher, nirmoy.das,
Maling list - DRI developers, Greathouse, Joseph, Tejun Heo,
cgroups, Christian König, damon.mcdougall
In-Reply-To: <CAOWid-f62Uv=GZXX2V2BsQGM5A1JJG_qmyrOwd=KwZBx_sr-bg@mail.gmail.com>
On Fri, Feb 14, 2020 at 11:08 AM Kenny Ho <y2kenny@gmail.com> wrote:
>
> Hi Jason,
>
> Thanks for the review.
>
> On Fri, Feb 14, 2020 at 11:44 AM Jason Ekstrand <jason@jlekstrand.net> wrote:
> >
> > Pardon my ignorance but I'm a bit confused by this. What is a "logical GPU"? What are we subdividing? Are we carving up memory? Compute power? Both?
>
> The intention is compute but it is up to the individual drm driver to decide.
>
> > If it's carving up compute power, what's actually being carved up? Time? Execution units/waves/threads? Even if that's the case, what advantage does it give to have it in terms of a fixed set of lgpus where each cgroup gets to pick a fixed set. Does affinity matter that much? Why not just say how many waves the GPU supports and that they have to be allocated in chunks of 16 waves (pulling a number out of thin air) and let the cgroup specify how many waves it wants.
> >
> > Don't get me wrong here. I'm all for the notion of being able to use cgroups to carve up GPU compute resources. However, this sounds to me like the most AMD-specific solution possible. We (Intel) could probably do some sort of carving up as well but we'd likely want to do it with preemption and time-slicing rather than handing out specific EUs.
>
> This has been discussed in the RFC before
> (https://www.spinics.net/lists/cgroups/msg23469.html.) As mentioned
> before, the idea of a compute unit is hardly an AMD specific thing as
> it is in the OpenCL standard and part of the architecture of many
> different vendors. In addition, the interface presented here supports
> Intel's use case. What you described is what I considered as the
> "anonymous resources" view of the lgpu. What you/Intel can do, is to
> register your device to drmcg to have 100 lgpu and users can specify
> simply by count. So if they want to allocate 5% for a cgroup, they
> would set count=5. Per the documentation in this patch: "Some DRM
> devices may only support lgpu as anonymous resources. In such case,
> the significance of the position of the set bits in list will be
> ignored." What Intel does with the user expressed configuration of "5
> out of 100" is entirely up to Intel (time slice if you like, change to
> specific EUs later if you like, or make it driver configurable to
> support both if you like.)
Sure, there's an OpenCL thing. However, just because there's an
OpenCL thing doesn't mean that it's as standardized as it looks. :-(
In particular,
1. The OpenCL thing has a query first to ask the driver what kind of
carving up of the GPU is allowed
2. When clCreateSubdevices is called, the type of partitioning is
specified so they can specifically ask for devices grouped by shared
L2 cache, for instance.
3. Just because the API exists and everyone implements it doesn't
mean that everyone implements it usefully. From my reading of the
spec, it looks like the API is very much designed towards a CPU
implementation of OpenCL. The Intel OpenCL GPU compute drivers, for
instance, implement it as a total no-op and no real sub-dividing is
allowed.
That said, that doesn't necessarily mean that carving up units of
compute power is a bad plan. It's just unclear (as Daniel said on the
above referenced chain) what those units mean. Maybe it's ok if they
mean nothing or if their meaning is HW-specific?
> Regards,
> Kenny
>
> >
> > On Fri, Feb 14, 2020 at 9:57 AM Kenny Ho <Kenny.Ho@amd.com> wrote:
> >>
> >> drm.lgpu
> >> A read-write nested-keyed file which exists on all cgroups.
> >> Each entry is keyed by the DRM device's major:minor.
> >>
> >> lgpu stands for logical GPU, it is an abstraction used to
> >> subdivide a physical DRM device for the purpose of resource
> >> management. This file stores user configuration while the
> >> drm.lgpu.effective reflects the actual allocation after
> >> considering the relationship between the cgroups and their
> >> configurations.
> >>
> >> The lgpu is a discrete quantity that is device specific (i.e.
> >> some DRM devices may have 64 lgpus while others may have 100
> >> lgpus.) The lgpu is a single quantity that can be allocated
> >> in three different ways denoted by the following nested keys.
> >>
> >> ===== ==============================================
> >> weight Allocate by proportion in relationship with
> >> active sibling cgroups
> >> count Allocate by amount statically, treat lgpu as
> >> anonymous resources
> >> list Allocate statically, treat lgpu as named
> >> resource
> >> ===== ==============================================
> >>
> >> For example:
> >> 226:0 weight=100 count=256 list=0-255
> >> 226:1 weight=100 count=4 list=0,2,4,6
> >> 226:2 weight=100 count=32 list=32-63
> >> 226:3 weight=100 count=0 list=
> >> 226:4 weight=500 count=0 list=
> >>
> >> lgpu is represented by a bitmap and uses the bitmap_parselist
> >> kernel function so the list key input format is a
> >> comma-separated list of decimal numbers and ranges.
> >>
> >> Consecutively set bits are shown as two hyphen-separated decimal
> >> numbers, the smallest and largest bit numbers set in the range.
> >> Optionally each range can be postfixed to denote that only parts
> >> of it should be set. The range will divided to groups of
> >> specific size.
> >> Syntax: range:used_size/group_size
> >> Example: 0-1023:2/256 ==> 0,1,256,257,512,513,768,769
> >>
> >> The count key is the hamming weight / hweight of the bitmap.
> >>
> >> Weight, count and list accept the max and default keywords.
> >>
> >> Some DRM devices may only support lgpu as anonymous resources.
> >> In such case, the significance of the position of the set bits
> >> in list will be ignored.
> >>
> >> The weight quantity is only in effect when static allocation
> >> is not used (by setting count=0) for this cgroup. The weight
> >> quantity distributes lgpus that are not statically allocated by
> >> the siblings. For example, given siblings cgroupA, cgroupB and
> >> cgroupC for a DRM device that has 64 lgpus, if cgroupA occupies
> >> 0-63, no lgpu is available to be distributed by weight.
> >> Similarly, if cgroupA has list=0-31 and cgroupB has list=16-63,
> >> cgroupC will be starved if it tries to allocate by weight.
> >>
> >> On the other hand, if cgroupA has weight=100 count=0, cgroupB
> >> has list=16-47, and cgroupC has weight=100 count=0, then 32
> >> lgpus are available to be distributed evenly between cgroupA
> >> and cgroupC. In drm.lgpu.effective, cgroupA will have
> >> list=0-15 and cgroupC will have list=48-63.
> >>
> >> This lgpu resource supports the 'allocation' and 'weight'
> >> resource distribution model.
> >>
> >> drm.lgpu.effective
> >> A read-only nested-keyed file which exists on all cgroups.
> >> Each entry is keyed by the DRM device's major:minor.
> >>
> >> lgpu stands for logical GPU, it is an abstraction used to
> >> subdivide a physical DRM device for the purpose of resource
> >> management. This file reflects the actual allocation after
> >> considering the relationship between the cgroups and their
> >> configurations in drm.lgpu.
> >>
> >> Change-Id: Idde0ef9a331fd67bb9c7eb8ef9978439e6452488
> >> Signed-off-by: Kenny Ho <Kenny.Ho@amd.com>
> >> ---
> >> Documentation/admin-guide/cgroup-v2.rst | 80 ++++++
> >> include/drm/drm_cgroup.h | 3 +
> >> include/linux/cgroup_drm.h | 22 ++
> >> kernel/cgroup/drm.c | 324 +++++++++++++++++++++++-
> >> 4 files changed, 427 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/Documentation/admin-guide/cgroup-v2.rst b/Documentation/admin-guide/cgroup-v2.rst
> >> index ce5dc027366a..d8a41956e5c7 100644
> >> --- a/Documentation/admin-guide/cgroup-v2.rst
> >> +++ b/Documentation/admin-guide/cgroup-v2.rst
> >> @@ -2120,6 +2120,86 @@ DRM Interface Files
> >> Set largest allocation for /dev/dri/card1 to 4MB
> >> echo "226:1 4m" > drm.buffer.peak.max
> >>
> >> + drm.lgpu
> >> + A read-write nested-keyed file which exists on all cgroups.
> >> + Each entry is keyed by the DRM device's major:minor.
> >> +
> >> + lgpu stands for logical GPU, it is an abstraction used to
> >> + subdivide a physical DRM device for the purpose of resource
> >> + management. This file stores user configuration while the
> >> + drm.lgpu.effective reflects the actual allocation after
> >> + considering the relationship between the cgroups and their
> >> + configurations.
> >> +
> >> + The lgpu is a discrete quantity that is device specific (i.e.
> >> + some DRM devices may have 64 lgpus while others may have 100
> >> + lgpus.) The lgpu is a single quantity that can be allocated
> >> + in three different ways denoted by the following nested keys.
> >> +
> >> + ===== ==============================================
> >> + weight Allocate by proportion in relationship with
> >> + active sibling cgroups
> >> + count Allocate by amount statically, treat lgpu as
> >> + anonymous resources
> >> + list Allocate statically, treat lgpu as named
> >> + resource
> >> + ===== ==============================================
> >> +
> >> + For example:
> >> + 226:0 weight=100 count=256 list=0-255
> >> + 226:1 weight=100 count=4 list=0,2,4,6
> >> + 226:2 weight=100 count=32 list=32-63
> >> + 226:3 weight=100 count=0 list=
> >> + 226:4 weight=500 count=0 list=
> >> +
> >> + lgpu is represented by a bitmap and uses the bitmap_parselist
> >> + kernel function so the list key input format is a
> >> + comma-separated list of decimal numbers and ranges.
> >> +
> >> + Consecutively set bits are shown as two hyphen-separated decimal
> >> + numbers, the smallest and largest bit numbers set in the range.
> >> + Optionally each range can be postfixed to denote that only parts
> >> + of it should be set. The range will divided to groups of
> >> + specific size.
> >> + Syntax: range:used_size/group_size
> >> + Example: 0-1023:2/256 ==> 0,1,256,257,512,513,768,769
> >> +
> >> + The count key is the hamming weight / hweight of the bitmap.
> >> +
> >> + Weight, count and list accept the max and default keywords.
> >> +
> >> + Some DRM devices may only support lgpu as anonymous resources.
> >> + In such case, the significance of the position of the set bits
> >> + in list will be ignored.
> >> +
> >> + The weight quantity is only in effect when static allocation
> >> + is not used (by setting count=0) for this cgroup. The weight
> >> + quantity distributes lgpus that are not statically allocated by
> >> + the siblings. For example, given siblings cgroupA, cgroupB and
> >> + cgroupC for a DRM device that has 64 lgpus, if cgroupA occupies
> >> + 0-63, no lgpu is available to be distributed by weight.
> >> + Similarly, if cgroupA has list=0-31 and cgroupB has list=16-63,
> >> + cgroupC will be starved if it tries to allocate by weight.
> >> +
> >> + On the other hand, if cgroupA has weight=100 count=0, cgroupB
> >> + has list=16-47, and cgroupC has weight=100 count=0, then 32
> >> + lgpus are available to be distributed evenly between cgroupA
> >> + and cgroupC. In drm.lgpu.effective, cgroupA will have
> >> + list=0-15 and cgroupC will have list=48-63.
> >> +
> >> + This lgpu resource supports the 'allocation' and 'weight'
> >> + resource distribution model.
> >> +
> >> + drm.lgpu.effective
> >> + A read-only nested-keyed file which exists on all cgroups.
> >> + Each entry is keyed by the DRM device's major:minor.
> >> +
> >> + lgpu stands for logical GPU, it is an abstraction used to
> >> + subdivide a physical DRM device for the purpose of resource
> >> + management. This file reflects the actual allocation after
> >> + considering the relationship between the cgroups and their
> >> + configurations in drm.lgpu.
> >> +
> >> GEM Buffer Ownership
> >> ~~~~~~~~~~~~~~~~~~~~
> >>
> >> diff --git a/include/drm/drm_cgroup.h b/include/drm/drm_cgroup.h
> >> index 2b41d4d22e33..619a110cc748 100644
> >> --- a/include/drm/drm_cgroup.h
> >> +++ b/include/drm/drm_cgroup.h
> >> @@ -17,6 +17,9 @@ struct drmcg_props {
> >>
> >> s64 bo_limits_total_allocated_default;
> >> s64 bo_limits_peak_allocated_default;
> >> +
> >> + int lgpu_capacity;
> >> + DECLARE_BITMAP(lgpu_slots, MAX_DRMCG_LGPU_CAPACITY);
> >> };
> >>
> >> void drmcg_bind(struct drm_minor (*(*acq_dm)(unsigned int minor_id)),
> >> diff --git a/include/linux/cgroup_drm.h b/include/linux/cgroup_drm.h
> >> index eae400f3d9b4..bb09704e7f71 100644
> >> --- a/include/linux/cgroup_drm.h
> >> +++ b/include/linux/cgroup_drm.h
> >> @@ -11,10 +11,14 @@
> >> /* limit defined per the way drm_minor_alloc operates */
> >> #define MAX_DRM_DEV (64 * DRM_MINOR_RENDER)
> >>
> >> +#define MAX_DRMCG_LGPU_CAPACITY 256
> >> +
> >> enum drmcg_res_type {
> >> DRMCG_TYPE_BO_TOTAL,
> >> DRMCG_TYPE_BO_PEAK,
> >> DRMCG_TYPE_BO_COUNT,
> >> + DRMCG_TYPE_LGPU,
> >> + DRMCG_TYPE_LGPU_EFF,
> >> __DRMCG_TYPE_LAST,
> >> };
> >>
> >> @@ -32,6 +36,24 @@ struct drmcg_device_resource {
> >> s64 bo_limits_peak_allocated;
> >>
> >> s64 bo_stats_count_allocated;
> >> +
> >> + /**
> >> + * Logical GPU
> >> + *
> >> + * *_cfg are properties configured by users
> >> + * *_eff are the effective properties being applied to the hardware
> >> + * *_stg is used to calculate _eff before applying to _eff
> >> + * after considering the entire hierarchy
> >> + */
> >> + DECLARE_BITMAP(lgpu_stg, MAX_DRMCG_LGPU_CAPACITY);
> >> + /* user configurations */
> >> + s64 lgpu_weight_cfg;
> >> + DECLARE_BITMAP(lgpu_cfg, MAX_DRMCG_LGPU_CAPACITY);
> >> + /* effective lgpu for the cgroup after considering
> >> + * relationship with other cgroup
> >> + */
> >> + s64 lgpu_count_eff;
> >> + DECLARE_BITMAP(lgpu_eff, MAX_DRMCG_LGPU_CAPACITY);
> >> };
> >>
> >> /**
> >> diff --git a/kernel/cgroup/drm.c b/kernel/cgroup/drm.c
> >> index 5fcbbc13fa1c..a4e88a3704bb 100644
> >> --- a/kernel/cgroup/drm.c
> >> +++ b/kernel/cgroup/drm.c
> >> @@ -9,6 +9,7 @@
> >> #include <linux/seq_file.h>
> >> #include <linux/mutex.h>
> >> #include <linux/kernel.h>
> >> +#include <linux/bitmap.h>
> >> #include <linux/cgroup_drm.h>
> >> #include <drm/drm_file.h>
> >> #include <drm/drm_drv.h>
> >> @@ -41,6 +42,10 @@ enum drmcg_file_type {
> >> DRMCG_FTYPE_DEFAULT,
> >> };
> >>
> >> +#define LGPU_LIMITS_NAME_LIST "list"
> >> +#define LGPU_LIMITS_NAME_COUNT "count"
> >> +#define LGPU_LIMITS_NAME_WEIGHT "weight"
> >> +
> >> /**
> >> * drmcg_bind - Bind DRM subsystem to cgroup subsystem
> >> * @acq_dm: function pointer to the drm_minor_acquire function
> >> @@ -98,6 +103,13 @@ static inline int init_drmcg_single(struct drmcg *drmcg, struct drm_device *dev)
> >> ddr->bo_limits_peak_allocated =
> >> dev->drmcg_props.bo_limits_peak_allocated_default;
> >>
> >> + bitmap_copy(ddr->lgpu_cfg, dev->drmcg_props.lgpu_slots,
> >> + MAX_DRMCG_LGPU_CAPACITY);
> >> + bitmap_copy(ddr->lgpu_stg, dev->drmcg_props.lgpu_slots,
> >> + MAX_DRMCG_LGPU_CAPACITY);
> >> +
> >> + ddr->lgpu_weight_cfg = CGROUP_WEIGHT_DFL;
> >> +
> >> return 0;
> >> }
> >>
> >> @@ -121,6 +133,120 @@ static inline void drmcg_update_cg_tree(struct drm_device *dev)
> >> mutex_unlock(&cgroup_mutex);
> >> }
> >>
> >> +static void drmcg_calculate_effective_lgpu(struct drm_device *dev,
> >> + const unsigned long *free_static,
> >> + const unsigned long *free_weighted,
> >> + struct drmcg *parent_drmcg)
> >> +{
> >> + int capacity = dev->drmcg_props.lgpu_capacity;
> >> + DECLARE_BITMAP(lgpu_unused, MAX_DRMCG_LGPU_CAPACITY);
> >> + DECLARE_BITMAP(lgpu_by_weight, MAX_DRMCG_LGPU_CAPACITY);
> >> + struct drmcg_device_resource *parent_ddr;
> >> + struct drmcg_device_resource *ddr;
> >> + int minor = dev->primary->index;
> >> + struct cgroup_subsys_state *pos;
> >> + struct drmcg *child;
> >> + s64 weight_sum = 0;
> >> + s64 unused;
> >> +
> >> + parent_ddr = parent_drmcg->dev_resources[minor];
> >> +
> >> + if (bitmap_empty(parent_ddr->lgpu_cfg, capacity))
> >> + /* no static cfg, use weight for calculating the effective */
> >> + bitmap_copy(parent_ddr->lgpu_stg, free_weighted, capacity);
> >> + else
> >> + /* lgpu statically configured, use the overlap as effective */
> >> + bitmap_and(parent_ddr->lgpu_stg, free_static,
> >> + parent_ddr->lgpu_cfg, capacity);
> >> +
> >> + /* calculate lgpu available for distribution by weight for children */
> >> + bitmap_copy(lgpu_unused, parent_ddr->lgpu_stg, capacity);
> >> + css_for_each_child(pos, &parent_drmcg->css) {
> >> + child = css_to_drmcg(pos);
> >> + ddr = child->dev_resources[minor];
> >> +
> >> + if (bitmap_empty(ddr->lgpu_cfg, capacity))
> >> + /* no static allocation, participate in weight dist */
> >> + weight_sum += ddr->lgpu_weight_cfg;
> >> + else
> >> + /* take out statically allocated lgpu by siblings */
> >> + bitmap_andnot(lgpu_unused, lgpu_unused, ddr->lgpu_cfg,
> >> + capacity);
> >> + }
> >> +
> >> + unused = bitmap_weight(lgpu_unused, capacity);
> >> +
> >> + css_for_each_child(pos, &parent_drmcg->css) {
> >> + child = css_to_drmcg(pos);
> >> + ddr = child->dev_resources[minor];
> >> +
> >> + bitmap_zero(lgpu_by_weight, capacity);
> >> + /* no static allocation, participate in weight distribution */
> >> + if (bitmap_empty(ddr->lgpu_cfg, capacity)) {
> >> + int c;
> >> + int p = 0;
> >> +
> >> + for (c = ddr->lgpu_weight_cfg * unused / weight_sum;
> >> + c > 0; c--) {
> >> + p = find_next_bit(lgpu_unused, capacity, p);
> >> + if (p < capacity) {
> >> + clear_bit(p, lgpu_unused);
> >> + set_bit(p, lgpu_by_weight);
> >> + }
> >> + }
> >> +
> >> + }
> >> +
> >> + drmcg_calculate_effective_lgpu(dev, parent_ddr->lgpu_stg,
> >> + lgpu_by_weight, child);
> >> + }
> >> +}
> >> +
> >> +static void drmcg_apply_effective_lgpu(struct drm_device *dev)
> >> +{
> >> + int capacity = dev->drmcg_props.lgpu_capacity;
> >> + int minor = dev->primary->index;
> >> + struct drmcg_device_resource *ddr;
> >> + struct cgroup_subsys_state *pos;
> >> + struct drmcg *drmcg;
> >> +
> >> + if (root_drmcg == NULL) {
> >> + WARN_ON(root_drmcg == NULL);
> >> + return;
> >> + }
> >> +
> >> + rcu_read_lock();
> >> +
> >> + /* process the entire cgroup tree from root to simplify the algorithm */
> >> + drmcg_calculate_effective_lgpu(dev, dev->drmcg_props.lgpu_slots,
> >> + dev->drmcg_props.lgpu_slots, root_drmcg);
> >> +
> >> + /* apply changes to effective only if there is a change */
> >> + css_for_each_descendant_pre(pos, &root_drmcg->css) {
> >> + drmcg = css_to_drmcg(pos);
> >> + ddr = drmcg->dev_resources[minor];
> >> +
> >> + if (!bitmap_equal(ddr->lgpu_stg, ddr->lgpu_eff, capacity)) {
> >> + bitmap_copy(ddr->lgpu_eff, ddr->lgpu_stg, capacity);
> >> + ddr->lgpu_count_eff =
> >> + bitmap_weight(ddr->lgpu_eff, capacity);
> >> + }
> >> + }
> >> + rcu_read_unlock();
> >> +}
> >> +
> >> +static void drmcg_apply_effective(enum drmcg_res_type type,
> >> + struct drm_device *dev, struct drmcg *changed_drmcg)
> >> +{
> >> + switch (type) {
> >> + case DRMCG_TYPE_LGPU:
> >> + drmcg_apply_effective_lgpu(dev);
> >> + break;
> >> + default:
> >> + break;
> >> + }
> >> +}
> >> +
> >> /**
> >> * drmcg_register_dev - register a DRM device for usage in drm cgroup
> >> * @dev: DRM device
> >> @@ -143,7 +269,13 @@ void drmcg_register_dev(struct drm_device *dev)
> >> {
> >> dev->driver->drmcg_custom_init(dev, &dev->drmcg_props);
> >>
> >> + WARN_ON(dev->drmcg_props.lgpu_capacity !=
> >> + bitmap_weight(dev->drmcg_props.lgpu_slots,
> >> + MAX_DRMCG_LGPU_CAPACITY));
> >> +
> >> drmcg_update_cg_tree(dev);
> >> +
> >> + drmcg_apply_effective(DRMCG_TYPE_LGPU, dev, root_drmcg);
> >> }
> >> mutex_unlock(&drmcg_mutex);
> >> }
> >> @@ -297,7 +429,8 @@ static void drmcg_print_stats(struct drmcg_device_resource *ddr,
> >> }
> >>
> >> static void drmcg_print_limits(struct drmcg_device_resource *ddr,
> >> - struct seq_file *sf, enum drmcg_res_type type)
> >> + struct seq_file *sf, enum drmcg_res_type type,
> >> + struct drm_device *dev)
> >> {
> >> if (ddr == NULL) {
> >> seq_puts(sf, "\n");
> >> @@ -311,6 +444,25 @@ static void drmcg_print_limits(struct drmcg_device_resource *ddr,
> >> case DRMCG_TYPE_BO_PEAK:
> >> seq_printf(sf, "%lld\n", ddr->bo_limits_peak_allocated);
> >> break;
> >> + case DRMCG_TYPE_LGPU:
> >> + seq_printf(sf, "%s=%lld %s=%d %s=%*pbl\n",
> >> + LGPU_LIMITS_NAME_WEIGHT,
> >> + ddr->lgpu_weight_cfg,
> >> + LGPU_LIMITS_NAME_COUNT,
> >> + bitmap_weight(ddr->lgpu_cfg,
> >> + dev->drmcg_props.lgpu_capacity),
> >> + LGPU_LIMITS_NAME_LIST,
> >> + dev->drmcg_props.lgpu_capacity,
> >> + ddr->lgpu_cfg);
> >> + break;
> >> + case DRMCG_TYPE_LGPU_EFF:
> >> + seq_printf(sf, "%s=%lld %s=%*pbl\n",
> >> + LGPU_LIMITS_NAME_COUNT,
> >> + ddr->lgpu_count_eff,
> >> + LGPU_LIMITS_NAME_LIST,
> >> + dev->drmcg_props.lgpu_capacity,
> >> + ddr->lgpu_eff);
> >> + break;
> >> default:
> >> seq_puts(sf, "\n");
> >> break;
> >> @@ -329,6 +481,17 @@ static void drmcg_print_default(struct drmcg_props *props,
> >> seq_printf(sf, "%lld\n",
> >> props->bo_limits_peak_allocated_default);
> >> break;
> >> + case DRMCG_TYPE_LGPU:
> >> + seq_printf(sf, "%s=%d %s=%d %s=%*pbl\n",
> >> + LGPU_LIMITS_NAME_WEIGHT,
> >> + CGROUP_WEIGHT_DFL,
> >> + LGPU_LIMITS_NAME_COUNT,
> >> + bitmap_weight(props->lgpu_slots,
> >> + props->lgpu_capacity),
> >> + LGPU_LIMITS_NAME_LIST,
> >> + props->lgpu_capacity,
> >> + props->lgpu_slots);
> >> + break;
> >> default:
> >> seq_puts(sf, "\n");
> >> break;
> >> @@ -358,7 +521,7 @@ static int drmcg_seq_show_fn(int id, void *ptr, void *data)
> >> drmcg_print_stats(ddr, sf, type);
> >> break;
> >> case DRMCG_FTYPE_LIMIT:
> >> - drmcg_print_limits(ddr, sf, type);
> >> + drmcg_print_limits(ddr, sf, type, minor->dev);
> >> break;
> >> case DRMCG_FTYPE_DEFAULT:
> >> drmcg_print_default(&minor->dev->drmcg_props, sf, type);
> >> @@ -415,6 +578,115 @@ static int drmcg_process_limit_s64_val(char *sval, bool is_mem,
> >> return rc;
> >> }
> >>
> >> +static void drmcg_nested_limit_parse(struct kernfs_open_file *of,
> >> + struct drm_device *dev, char *attrs)
> >> +{
> >> + DECLARE_BITMAP(tmp_bitmap, MAX_DRMCG_LGPU_CAPACITY);
> >> + DECLARE_BITMAP(chk_bitmap, MAX_DRMCG_LGPU_CAPACITY);
> >> + enum drmcg_res_type type =
> >> + DRMCG_CTF_PRIV2RESTYPE(of_cft(of)->private);
> >> + struct drmcg *drmcg = css_to_drmcg(of_css(of));
> >> + struct drmcg_props *props = &dev->drmcg_props;
> >> + char *cft_name = of_cft(of)->name;
> >> + int minor = dev->primary->index;
> >> + char *nested = strstrip(attrs);
> >> + struct drmcg_device_resource *ddr =
> >> + drmcg->dev_resources[minor];
> >> + char *attr;
> >> + char sname[256];
> >> + char sval[256];
> >> + s64 val;
> >> + int rc;
> >> +
> >> + while (nested != NULL) {
> >> + attr = strsep(&nested, " ");
> >> +
> >> + if (sscanf(attr, "%255[^=]=%255[^=]", sname, sval) != 2)
> >> + continue;
> >> +
> >> + switch (type) {
> >> + case DRMCG_TYPE_LGPU:
> >> + if (strncmp(sname, LGPU_LIMITS_NAME_LIST, 256) &&
> >> + strncmp(sname, LGPU_LIMITS_NAME_COUNT, 256) &&
> >> + strncmp(sname, LGPU_LIMITS_NAME_WEIGHT, 256))
> >> + continue;
> >> +
> >> + if (strncmp(sname, LGPU_LIMITS_NAME_WEIGHT, 256) &&
> >> + (!strcmp("max", sval) ||
> >> + !strcmp("default", sval))) {
> >> + bitmap_copy(ddr->lgpu_cfg, props->lgpu_slots,
> >> + props->lgpu_capacity);
> >> +
> >> + continue;
> >> + }
> >> +
> >> + if (strncmp(sname, LGPU_LIMITS_NAME_WEIGHT, 256) == 0) {
> >> + rc = drmcg_process_limit_s64_val(sval,
> >> + false, CGROUP_WEIGHT_DFL,
> >> + CGROUP_WEIGHT_MAX, &val);
> >> +
> >> + if (rc || val < CGROUP_WEIGHT_MIN ||
> >> + val > CGROUP_WEIGHT_MAX) {
> >> + drmcg_pr_cft_err(drmcg, rc, cft_name,
> >> + minor);
> >> + continue;
> >> + }
> >> +
> >> + ddr->lgpu_weight_cfg = val;
> >> + continue;
> >> + }
> >> +
> >> + if (strncmp(sname, LGPU_LIMITS_NAME_COUNT, 256) == 0) {
> >> + rc = drmcg_process_limit_s64_val(sval,
> >> + false, props->lgpu_capacity,
> >> + props->lgpu_capacity, &val);
> >> +
> >> + if (rc || val < 0) {
> >> + drmcg_pr_cft_err(drmcg, rc, cft_name,
> >> + minor);
> >> + continue;
> >> + }
> >> +
> >> + bitmap_zero(tmp_bitmap,
> >> + MAX_DRMCG_LGPU_CAPACITY);
> >> + bitmap_set(tmp_bitmap, 0, val);
> >> + }
> >> +
> >> + if (strncmp(sname, LGPU_LIMITS_NAME_LIST, 256) == 0) {
> >> + rc = bitmap_parselist(sval, tmp_bitmap,
> >> + MAX_DRMCG_LGPU_CAPACITY);
> >> +
> >> + if (rc) {
> >> + drmcg_pr_cft_err(drmcg, rc, cft_name,
> >> + minor);
> >> + continue;
> >> + }
> >> +
> >> + bitmap_andnot(chk_bitmap, tmp_bitmap,
> >> + props->lgpu_slots,
> >> + MAX_DRMCG_LGPU_CAPACITY);
> >> +
> >> + /* user setting does not intersect with
> >> + * available lgpu */
> >> + if (!bitmap_empty(chk_bitmap,
> >> + MAX_DRMCG_LGPU_CAPACITY)) {
> >> + drmcg_pr_cft_err(drmcg, 0, cft_name,
> >> + minor);
> >> + continue;
> >> + }
> >> + }
> >> +
> >> + bitmap_copy(ddr->lgpu_cfg, tmp_bitmap,
> >> + props->lgpu_capacity);
> >> +
> >> + break; /* DRMCG_TYPE_LGPU */
> >> + default:
> >> + break;
> >> + } /* switch (type) */
> >> + }
> >> +}
> >> +
> >> +
> >> /**
> >> * drmcg_limit_write - parse cgroup interface files to obtain user config
> >> *
> >> @@ -499,9 +771,15 @@ static ssize_t drmcg_limit_write(struct kernfs_open_file *of, char *buf,
> >>
> >> ddr->bo_limits_peak_allocated = val;
> >> break;
> >> + case DRMCG_TYPE_LGPU:
> >> + drmcg_nested_limit_parse(of, dm->dev, sattr);
> >> + break;
> >> default:
> >> break;
> >> }
> >> +
> >> + drmcg_apply_effective(type, dm->dev, drmcg);
> >> +
> >> mutex_unlock(&dm->dev->drmcg_mutex);
> >>
> >> mutex_lock(&drmcg_mutex);
> >> @@ -560,12 +838,51 @@ struct cftype files[] = {
> >> .private = DRMCG_CTF_PRIV(DRMCG_TYPE_BO_COUNT,
> >> DRMCG_FTYPE_STATS),
> >> },
> >> + {
> >> + .name = "lgpu",
> >> + .seq_show = drmcg_seq_show,
> >> + .write = drmcg_limit_write,
> >> + .private = DRMCG_CTF_PRIV(DRMCG_TYPE_LGPU,
> >> + DRMCG_FTYPE_LIMIT),
> >> + },
> >> + {
> >> + .name = "lgpu.default",
> >> + .seq_show = drmcg_seq_show,
> >> + .flags = CFTYPE_ONLY_ON_ROOT,
> >> + .private = DRMCG_CTF_PRIV(DRMCG_TYPE_LGPU,
> >> + DRMCG_FTYPE_DEFAULT),
> >> + },
> >> + {
> >> + .name = "lgpu.effective",
> >> + .seq_show = drmcg_seq_show,
> >> + .private = DRMCG_CTF_PRIV(DRMCG_TYPE_LGPU_EFF,
> >> + DRMCG_FTYPE_LIMIT),
> >> + },
> >> { } /* terminate */
> >> };
> >>
> >> +static int drmcg_online_fn(int id, void *ptr, void *data)
> >> +{
> >> + struct drm_minor *minor = ptr;
> >> + struct drmcg *drmcg = data;
> >> +
> >> + if (minor->type != DRM_MINOR_PRIMARY)
> >> + return 0;
> >> +
> >> + drmcg_apply_effective(DRMCG_TYPE_LGPU, minor->dev, drmcg);
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static int drmcg_css_online(struct cgroup_subsys_state *css)
> >> +{
> >> + return drm_minor_for_each(&drmcg_online_fn, css_to_drmcg(css));
> >> +}
> >> +
> >> struct cgroup_subsys drm_cgrp_subsys = {
> >> .css_alloc = drmcg_css_alloc,
> >> .css_free = drmcg_css_free,
> >> + .css_online = drmcg_css_online,
> >> .early_init = false,
> >> .legacy_cftypes = files,
> >> .dfl_cftypes = files,
> >> @@ -585,6 +902,9 @@ void drmcg_device_early_init(struct drm_device *dev)
> >> dev->drmcg_props.bo_limits_total_allocated_default = S64_MAX;
> >> dev->drmcg_props.bo_limits_peak_allocated_default = S64_MAX;
> >>
> >> + dev->drmcg_props.lgpu_capacity = MAX_DRMCG_LGPU_CAPACITY;
> >> + bitmap_fill(dev->drmcg_props.lgpu_slots, MAX_DRMCG_LGPU_CAPACITY);
> >> +
> >> drmcg_update_cg_tree(dev);
> >> }
> >> EXPORT_SYMBOL(drmcg_device_early_init);
> >> --
> >> 2.25.0
> >>
> >> _______________________________________________
> >> dri-devel mailing list
> >> dri-devel@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH 09/11] drm, cgroup: Introduce lgpu as DRM cgroup resource
From: Jason Ekstrand @ 2020-02-14 17:48 UTC (permalink / raw)
To: Kenny Ho
Cc: juan.zuniga-anaya, Daniel Vetter, Kenny Ho, Kuehling, Felix,
jsparks, amd-gfx mailing list, lkaplan, Alex Deucher, nirmoy.das,
Maling list - DRI developers, Greathouse, Joseph, Tejun Heo,
cgroups, Christian König, damon.mcdougall
In-Reply-To: <CAOWid-f62Uv=GZXX2V2BsQGM5A1JJG_qmyrOwd=KwZBx_sr-bg@mail.gmail.com>
On Fri, Feb 14, 2020 at 11:08 AM Kenny Ho <y2kenny@gmail.com> wrote:
>
> Hi Jason,
>
> Thanks for the review.
>
> On Fri, Feb 14, 2020 at 11:44 AM Jason Ekstrand <jason@jlekstrand.net> wrote:
> >
> > Pardon my ignorance but I'm a bit confused by this. What is a "logical GPU"? What are we subdividing? Are we carving up memory? Compute power? Both?
>
> The intention is compute but it is up to the individual drm driver to decide.
>
> > If it's carving up compute power, what's actually being carved up? Time? Execution units/waves/threads? Even if that's the case, what advantage does it give to have it in terms of a fixed set of lgpus where each cgroup gets to pick a fixed set. Does affinity matter that much? Why not just say how many waves the GPU supports and that they have to be allocated in chunks of 16 waves (pulling a number out of thin air) and let the cgroup specify how many waves it wants.
> >
> > Don't get me wrong here. I'm all for the notion of being able to use cgroups to carve up GPU compute resources. However, this sounds to me like the most AMD-specific solution possible. We (Intel) could probably do some sort of carving up as well but we'd likely want to do it with preemption and time-slicing rather than handing out specific EUs.
>
> This has been discussed in the RFC before
> (https://www.spinics.net/lists/cgroups/msg23469.html.) As mentioned
> before, the idea of a compute unit is hardly an AMD specific thing as
> it is in the OpenCL standard and part of the architecture of many
> different vendors. In addition, the interface presented here supports
> Intel's use case. What you described is what I considered as the
> "anonymous resources" view of the lgpu. What you/Intel can do, is to
> register your device to drmcg to have 100 lgpu and users can specify
> simply by count. So if they want to allocate 5% for a cgroup, they
> would set count=5. Per the documentation in this patch: "Some DRM
> devices may only support lgpu as anonymous resources. In such case,
> the significance of the position of the set bits in list will be
> ignored." What Intel does with the user expressed configuration of "5
> out of 100" is entirely up to Intel (time slice if you like, change to
> specific EUs later if you like, or make it driver configurable to
> support both if you like.)
Sure, there's an OpenCL thing. However, just because there's an
OpenCL thing doesn't mean that it's as standardized as it looks. :-(
In particular,
1. The OpenCL thing has a query first to ask the driver what kind of
carving up of the GPU is allowed
2. When clCreateSubdevices is called, the type of partitioning is
specified so they can specifically ask for devices grouped by shared
L2 cache, for instance.
3. Just because the API exists and everyone implements it doesn't
mean that everyone implements it usefully. From my reading of the
spec, it looks like the API is very much designed towards a CPU
implementation of OpenCL. The Intel OpenCL GPU compute drivers, for
instance, implement it as a total no-op and no real sub-dividing is
allowed.
That said, that doesn't necessarily mean that carving up units of
compute power is a bad plan. It's just unclear (as Daniel said on the
above referenced chain) what those units mean. Maybe it's ok if they
mean nothing or if their meaning is HW-specific?
> Regards,
> Kenny
>
> >
> > On Fri, Feb 14, 2020 at 9:57 AM Kenny Ho <Kenny.Ho@amd.com> wrote:
> >>
> >> drm.lgpu
> >> A read-write nested-keyed file which exists on all cgroups.
> >> Each entry is keyed by the DRM device's major:minor.
> >>
> >> lgpu stands for logical GPU, it is an abstraction used to
> >> subdivide a physical DRM device for the purpose of resource
> >> management. This file stores user configuration while the
> >> drm.lgpu.effective reflects the actual allocation after
> >> considering the relationship between the cgroups and their
> >> configurations.
> >>
> >> The lgpu is a discrete quantity that is device specific (i.e.
> >> some DRM devices may have 64 lgpus while others may have 100
> >> lgpus.) The lgpu is a single quantity that can be allocated
> >> in three different ways denoted by the following nested keys.
> >>
> >> ===== ==============================================
> >> weight Allocate by proportion in relationship with
> >> active sibling cgroups
> >> count Allocate by amount statically, treat lgpu as
> >> anonymous resources
> >> list Allocate statically, treat lgpu as named
> >> resource
> >> ===== ==============================================
> >>
> >> For example:
> >> 226:0 weight=100 count=256 list=0-255
> >> 226:1 weight=100 count=4 list=0,2,4,6
> >> 226:2 weight=100 count=32 list=32-63
> >> 226:3 weight=100 count=0 list=
> >> 226:4 weight=500 count=0 list=
> >>
> >> lgpu is represented by a bitmap and uses the bitmap_parselist
> >> kernel function so the list key input format is a
> >> comma-separated list of decimal numbers and ranges.
> >>
> >> Consecutively set bits are shown as two hyphen-separated decimal
> >> numbers, the smallest and largest bit numbers set in the range.
> >> Optionally each range can be postfixed to denote that only parts
> >> of it should be set. The range will divided to groups of
> >> specific size.
> >> Syntax: range:used_size/group_size
> >> Example: 0-1023:2/256 ==> 0,1,256,257,512,513,768,769
> >>
> >> The count key is the hamming weight / hweight of the bitmap.
> >>
> >> Weight, count and list accept the max and default keywords.
> >>
> >> Some DRM devices may only support lgpu as anonymous resources.
> >> In such case, the significance of the position of the set bits
> >> in list will be ignored.
> >>
> >> The weight quantity is only in effect when static allocation
> >> is not used (by setting count=0) for this cgroup. The weight
> >> quantity distributes lgpus that are not statically allocated by
> >> the siblings. For example, given siblings cgroupA, cgroupB and
> >> cgroupC for a DRM device that has 64 lgpus, if cgroupA occupies
> >> 0-63, no lgpu is available to be distributed by weight.
> >> Similarly, if cgroupA has list=0-31 and cgroupB has list=16-63,
> >> cgroupC will be starved if it tries to allocate by weight.
> >>
> >> On the other hand, if cgroupA has weight=100 count=0, cgroupB
> >> has list=16-47, and cgroupC has weight=100 count=0, then 32
> >> lgpus are available to be distributed evenly between cgroupA
> >> and cgroupC. In drm.lgpu.effective, cgroupA will have
> >> list=0-15 and cgroupC will have list=48-63.
> >>
> >> This lgpu resource supports the 'allocation' and 'weight'
> >> resource distribution model.
> >>
> >> drm.lgpu.effective
> >> A read-only nested-keyed file which exists on all cgroups.
> >> Each entry is keyed by the DRM device's major:minor.
> >>
> >> lgpu stands for logical GPU, it is an abstraction used to
> >> subdivide a physical DRM device for the purpose of resource
> >> management. This file reflects the actual allocation after
> >> considering the relationship between the cgroups and their
> >> configurations in drm.lgpu.
> >>
> >> Change-Id: Idde0ef9a331fd67bb9c7eb8ef9978439e6452488
> >> Signed-off-by: Kenny Ho <Kenny.Ho@amd.com>
> >> ---
> >> Documentation/admin-guide/cgroup-v2.rst | 80 ++++++
> >> include/drm/drm_cgroup.h | 3 +
> >> include/linux/cgroup_drm.h | 22 ++
> >> kernel/cgroup/drm.c | 324 +++++++++++++++++++++++-
> >> 4 files changed, 427 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/Documentation/admin-guide/cgroup-v2.rst b/Documentation/admin-guide/cgroup-v2.rst
> >> index ce5dc027366a..d8a41956e5c7 100644
> >> --- a/Documentation/admin-guide/cgroup-v2.rst
> >> +++ b/Documentation/admin-guide/cgroup-v2.rst
> >> @@ -2120,6 +2120,86 @@ DRM Interface Files
> >> Set largest allocation for /dev/dri/card1 to 4MB
> >> echo "226:1 4m" > drm.buffer.peak.max
> >>
> >> + drm.lgpu
> >> + A read-write nested-keyed file which exists on all cgroups.
> >> + Each entry is keyed by the DRM device's major:minor.
> >> +
> >> + lgpu stands for logical GPU, it is an abstraction used to
> >> + subdivide a physical DRM device for the purpose of resource
> >> + management. This file stores user configuration while the
> >> + drm.lgpu.effective reflects the actual allocation after
> >> + considering the relationship between the cgroups and their
> >> + configurations.
> >> +
> >> + The lgpu is a discrete quantity that is device specific (i.e.
> >> + some DRM devices may have 64 lgpus while others may have 100
> >> + lgpus.) The lgpu is a single quantity that can be allocated
> >> + in three different ways denoted by the following nested keys.
> >> +
> >> + ===== ==============================================
> >> + weight Allocate by proportion in relationship with
> >> + active sibling cgroups
> >> + count Allocate by amount statically, treat lgpu as
> >> + anonymous resources
> >> + list Allocate statically, treat lgpu as named
> >> + resource
> >> + ===== ==============================================
> >> +
> >> + For example:
> >> + 226:0 weight=100 count=256 list=0-255
> >> + 226:1 weight=100 count=4 list=0,2,4,6
> >> + 226:2 weight=100 count=32 list=32-63
> >> + 226:3 weight=100 count=0 list=
> >> + 226:4 weight=500 count=0 list=
> >> +
> >> + lgpu is represented by a bitmap and uses the bitmap_parselist
> >> + kernel function so the list key input format is a
> >> + comma-separated list of decimal numbers and ranges.
> >> +
> >> + Consecutively set bits are shown as two hyphen-separated decimal
> >> + numbers, the smallest and largest bit numbers set in the range.
> >> + Optionally each range can be postfixed to denote that only parts
> >> + of it should be set. The range will divided to groups of
> >> + specific size.
> >> + Syntax: range:used_size/group_size
> >> + Example: 0-1023:2/256 ==> 0,1,256,257,512,513,768,769
> >> +
> >> + The count key is the hamming weight / hweight of the bitmap.
> >> +
> >> + Weight, count and list accept the max and default keywords.
> >> +
> >> + Some DRM devices may only support lgpu as anonymous resources.
> >> + In such case, the significance of the position of the set bits
> >> + in list will be ignored.
> >> +
> >> + The weight quantity is only in effect when static allocation
> >> + is not used (by setting count=0) for this cgroup. The weight
> >> + quantity distributes lgpus that are not statically allocated by
> >> + the siblings. For example, given siblings cgroupA, cgroupB and
> >> + cgroupC for a DRM device that has 64 lgpus, if cgroupA occupies
> >> + 0-63, no lgpu is available to be distributed by weight.
> >> + Similarly, if cgroupA has list=0-31 and cgroupB has list=16-63,
> >> + cgroupC will be starved if it tries to allocate by weight.
> >> +
> >> + On the other hand, if cgroupA has weight=100 count=0, cgroupB
> >> + has list=16-47, and cgroupC has weight=100 count=0, then 32
> >> + lgpus are available to be distributed evenly between cgroupA
> >> + and cgroupC. In drm.lgpu.effective, cgroupA will have
> >> + list=0-15 and cgroupC will have list=48-63.
> >> +
> >> + This lgpu resource supports the 'allocation' and 'weight'
> >> + resource distribution model.
> >> +
> >> + drm.lgpu.effective
> >> + A read-only nested-keyed file which exists on all cgroups.
> >> + Each entry is keyed by the DRM device's major:minor.
> >> +
> >> + lgpu stands for logical GPU, it is an abstraction used to
> >> + subdivide a physical DRM device for the purpose of resource
> >> + management. This file reflects the actual allocation after
> >> + considering the relationship between the cgroups and their
> >> + configurations in drm.lgpu.
> >> +
> >> GEM Buffer Ownership
> >> ~~~~~~~~~~~~~~~~~~~~
> >>
> >> diff --git a/include/drm/drm_cgroup.h b/include/drm/drm_cgroup.h
> >> index 2b41d4d22e33..619a110cc748 100644
> >> --- a/include/drm/drm_cgroup.h
> >> +++ b/include/drm/drm_cgroup.h
> >> @@ -17,6 +17,9 @@ struct drmcg_props {
> >>
> >> s64 bo_limits_total_allocated_default;
> >> s64 bo_limits_peak_allocated_default;
> >> +
> >> + int lgpu_capacity;
> >> + DECLARE_BITMAP(lgpu_slots, MAX_DRMCG_LGPU_CAPACITY);
> >> };
> >>
> >> void drmcg_bind(struct drm_minor (*(*acq_dm)(unsigned int minor_id)),
> >> diff --git a/include/linux/cgroup_drm.h b/include/linux/cgroup_drm.h
> >> index eae400f3d9b4..bb09704e7f71 100644
> >> --- a/include/linux/cgroup_drm.h
> >> +++ b/include/linux/cgroup_drm.h
> >> @@ -11,10 +11,14 @@
> >> /* limit defined per the way drm_minor_alloc operates */
> >> #define MAX_DRM_DEV (64 * DRM_MINOR_RENDER)
> >>
> >> +#define MAX_DRMCG_LGPU_CAPACITY 256
> >> +
> >> enum drmcg_res_type {
> >> DRMCG_TYPE_BO_TOTAL,
> >> DRMCG_TYPE_BO_PEAK,
> >> DRMCG_TYPE_BO_COUNT,
> >> + DRMCG_TYPE_LGPU,
> >> + DRMCG_TYPE_LGPU_EFF,
> >> __DRMCG_TYPE_LAST,
> >> };
> >>
> >> @@ -32,6 +36,24 @@ struct drmcg_device_resource {
> >> s64 bo_limits_peak_allocated;
> >>
> >> s64 bo_stats_count_allocated;
> >> +
> >> + /**
> >> + * Logical GPU
> >> + *
> >> + * *_cfg are properties configured by users
> >> + * *_eff are the effective properties being applied to the hardware
> >> + * *_stg is used to calculate _eff before applying to _eff
> >> + * after considering the entire hierarchy
> >> + */
> >> + DECLARE_BITMAP(lgpu_stg, MAX_DRMCG_LGPU_CAPACITY);
> >> + /* user configurations */
> >> + s64 lgpu_weight_cfg;
> >> + DECLARE_BITMAP(lgpu_cfg, MAX_DRMCG_LGPU_CAPACITY);
> >> + /* effective lgpu for the cgroup after considering
> >> + * relationship with other cgroup
> >> + */
> >> + s64 lgpu_count_eff;
> >> + DECLARE_BITMAP(lgpu_eff, MAX_DRMCG_LGPU_CAPACITY);
> >> };
> >>
> >> /**
> >> diff --git a/kernel/cgroup/drm.c b/kernel/cgroup/drm.c
> >> index 5fcbbc13fa1c..a4e88a3704bb 100644
> >> --- a/kernel/cgroup/drm.c
> >> +++ b/kernel/cgroup/drm.c
> >> @@ -9,6 +9,7 @@
> >> #include <linux/seq_file.h>
> >> #include <linux/mutex.h>
> >> #include <linux/kernel.h>
> >> +#include <linux/bitmap.h>
> >> #include <linux/cgroup_drm.h>
> >> #include <drm/drm_file.h>
> >> #include <drm/drm_drv.h>
> >> @@ -41,6 +42,10 @@ enum drmcg_file_type {
> >> DRMCG_FTYPE_DEFAULT,
> >> };
> >>
> >> +#define LGPU_LIMITS_NAME_LIST "list"
> >> +#define LGPU_LIMITS_NAME_COUNT "count"
> >> +#define LGPU_LIMITS_NAME_WEIGHT "weight"
> >> +
> >> /**
> >> * drmcg_bind - Bind DRM subsystem to cgroup subsystem
> >> * @acq_dm: function pointer to the drm_minor_acquire function
> >> @@ -98,6 +103,13 @@ static inline int init_drmcg_single(struct drmcg *drmcg, struct drm_device *dev)
> >> ddr->bo_limits_peak_allocated =
> >> dev->drmcg_props.bo_limits_peak_allocated_default;
> >>
> >> + bitmap_copy(ddr->lgpu_cfg, dev->drmcg_props.lgpu_slots,
> >> + MAX_DRMCG_LGPU_CAPACITY);
> >> + bitmap_copy(ddr->lgpu_stg, dev->drmcg_props.lgpu_slots,
> >> + MAX_DRMCG_LGPU_CAPACITY);
> >> +
> >> + ddr->lgpu_weight_cfg = CGROUP_WEIGHT_DFL;
> >> +
> >> return 0;
> >> }
> >>
> >> @@ -121,6 +133,120 @@ static inline void drmcg_update_cg_tree(struct drm_device *dev)
> >> mutex_unlock(&cgroup_mutex);
> >> }
> >>
> >> +static void drmcg_calculate_effective_lgpu(struct drm_device *dev,
> >> + const unsigned long *free_static,
> >> + const unsigned long *free_weighted,
> >> + struct drmcg *parent_drmcg)
> >> +{
> >> + int capacity = dev->drmcg_props.lgpu_capacity;
> >> + DECLARE_BITMAP(lgpu_unused, MAX_DRMCG_LGPU_CAPACITY);
> >> + DECLARE_BITMAP(lgpu_by_weight, MAX_DRMCG_LGPU_CAPACITY);
> >> + struct drmcg_device_resource *parent_ddr;
> >> + struct drmcg_device_resource *ddr;
> >> + int minor = dev->primary->index;
> >> + struct cgroup_subsys_state *pos;
> >> + struct drmcg *child;
> >> + s64 weight_sum = 0;
> >> + s64 unused;
> >> +
> >> + parent_ddr = parent_drmcg->dev_resources[minor];
> >> +
> >> + if (bitmap_empty(parent_ddr->lgpu_cfg, capacity))
> >> + /* no static cfg, use weight for calculating the effective */
> >> + bitmap_copy(parent_ddr->lgpu_stg, free_weighted, capacity);
> >> + else
> >> + /* lgpu statically configured, use the overlap as effective */
> >> + bitmap_and(parent_ddr->lgpu_stg, free_static,
> >> + parent_ddr->lgpu_cfg, capacity);
> >> +
> >> + /* calculate lgpu available for distribution by weight for children */
> >> + bitmap_copy(lgpu_unused, parent_ddr->lgpu_stg, capacity);
> >> + css_for_each_child(pos, &parent_drmcg->css) {
> >> + child = css_to_drmcg(pos);
> >> + ddr = child->dev_resources[minor];
> >> +
> >> + if (bitmap_empty(ddr->lgpu_cfg, capacity))
> >> + /* no static allocation, participate in weight dist */
> >> + weight_sum += ddr->lgpu_weight_cfg;
> >> + else
> >> + /* take out statically allocated lgpu by siblings */
> >> + bitmap_andnot(lgpu_unused, lgpu_unused, ddr->lgpu_cfg,
> >> + capacity);
> >> + }
> >> +
> >> + unused = bitmap_weight(lgpu_unused, capacity);
> >> +
> >> + css_for_each_child(pos, &parent_drmcg->css) {
> >> + child = css_to_drmcg(pos);
> >> + ddr = child->dev_resources[minor];
> >> +
> >> + bitmap_zero(lgpu_by_weight, capacity);
> >> + /* no static allocation, participate in weight distribution */
> >> + if (bitmap_empty(ddr->lgpu_cfg, capacity)) {
> >> + int c;
> >> + int p = 0;
> >> +
> >> + for (c = ddr->lgpu_weight_cfg * unused / weight_sum;
> >> + c > 0; c--) {
> >> + p = find_next_bit(lgpu_unused, capacity, p);
> >> + if (p < capacity) {
> >> + clear_bit(p, lgpu_unused);
> >> + set_bit(p, lgpu_by_weight);
> >> + }
> >> + }
> >> +
> >> + }
> >> +
> >> + drmcg_calculate_effective_lgpu(dev, parent_ddr->lgpu_stg,
> >> + lgpu_by_weight, child);
> >> + }
> >> +}
> >> +
> >> +static void drmcg_apply_effective_lgpu(struct drm_device *dev)
> >> +{
> >> + int capacity = dev->drmcg_props.lgpu_capacity;
> >> + int minor = dev->primary->index;
> >> + struct drmcg_device_resource *ddr;
> >> + struct cgroup_subsys_state *pos;
> >> + struct drmcg *drmcg;
> >> +
> >> + if (root_drmcg == NULL) {
> >> + WARN_ON(root_drmcg == NULL);
> >> + return;
> >> + }
> >> +
> >> + rcu_read_lock();
> >> +
> >> + /* process the entire cgroup tree from root to simplify the algorithm */
> >> + drmcg_calculate_effective_lgpu(dev, dev->drmcg_props.lgpu_slots,
> >> + dev->drmcg_props.lgpu_slots, root_drmcg);
> >> +
> >> + /* apply changes to effective only if there is a change */
> >> + css_for_each_descendant_pre(pos, &root_drmcg->css) {
> >> + drmcg = css_to_drmcg(pos);
> >> + ddr = drmcg->dev_resources[minor];
> >> +
> >> + if (!bitmap_equal(ddr->lgpu_stg, ddr->lgpu_eff, capacity)) {
> >> + bitmap_copy(ddr->lgpu_eff, ddr->lgpu_stg, capacity);
> >> + ddr->lgpu_count_eff =
> >> + bitmap_weight(ddr->lgpu_eff, capacity);
> >> + }
> >> + }
> >> + rcu_read_unlock();
> >> +}
> >> +
> >> +static void drmcg_apply_effective(enum drmcg_res_type type,
> >> + struct drm_device *dev, struct drmcg *changed_drmcg)
> >> +{
> >> + switch (type) {
> >> + case DRMCG_TYPE_LGPU:
> >> + drmcg_apply_effective_lgpu(dev);
> >> + break;
> >> + default:
> >> + break;
> >> + }
> >> +}
> >> +
> >> /**
> >> * drmcg_register_dev - register a DRM device for usage in drm cgroup
> >> * @dev: DRM device
> >> @@ -143,7 +269,13 @@ void drmcg_register_dev(struct drm_device *dev)
> >> {
> >> dev->driver->drmcg_custom_init(dev, &dev->drmcg_props);
> >>
> >> + WARN_ON(dev->drmcg_props.lgpu_capacity !=
> >> + bitmap_weight(dev->drmcg_props.lgpu_slots,
> >> + MAX_DRMCG_LGPU_CAPACITY));
> >> +
> >> drmcg_update_cg_tree(dev);
> >> +
> >> + drmcg_apply_effective(DRMCG_TYPE_LGPU, dev, root_drmcg);
> >> }
> >> mutex_unlock(&drmcg_mutex);
> >> }
> >> @@ -297,7 +429,8 @@ static void drmcg_print_stats(struct drmcg_device_resource *ddr,
> >> }
> >>
> >> static void drmcg_print_limits(struct drmcg_device_resource *ddr,
> >> - struct seq_file *sf, enum drmcg_res_type type)
> >> + struct seq_file *sf, enum drmcg_res_type type,
> >> + struct drm_device *dev)
> >> {
> >> if (ddr == NULL) {
> >> seq_puts(sf, "\n");
> >> @@ -311,6 +444,25 @@ static void drmcg_print_limits(struct drmcg_device_resource *ddr,
> >> case DRMCG_TYPE_BO_PEAK:
> >> seq_printf(sf, "%lld\n", ddr->bo_limits_peak_allocated);
> >> break;
> >> + case DRMCG_TYPE_LGPU:
> >> + seq_printf(sf, "%s=%lld %s=%d %s=%*pbl\n",
> >> + LGPU_LIMITS_NAME_WEIGHT,
> >> + ddr->lgpu_weight_cfg,
> >> + LGPU_LIMITS_NAME_COUNT,
> >> + bitmap_weight(ddr->lgpu_cfg,
> >> + dev->drmcg_props.lgpu_capacity),
> >> + LGPU_LIMITS_NAME_LIST,
> >> + dev->drmcg_props.lgpu_capacity,
> >> + ddr->lgpu_cfg);
> >> + break;
> >> + case DRMCG_TYPE_LGPU_EFF:
> >> + seq_printf(sf, "%s=%lld %s=%*pbl\n",
> >> + LGPU_LIMITS_NAME_COUNT,
> >> + ddr->lgpu_count_eff,
> >> + LGPU_LIMITS_NAME_LIST,
> >> + dev->drmcg_props.lgpu_capacity,
> >> + ddr->lgpu_eff);
> >> + break;
> >> default:
> >> seq_puts(sf, "\n");
> >> break;
> >> @@ -329,6 +481,17 @@ static void drmcg_print_default(struct drmcg_props *props,
> >> seq_printf(sf, "%lld\n",
> >> props->bo_limits_peak_allocated_default);
> >> break;
> >> + case DRMCG_TYPE_LGPU:
> >> + seq_printf(sf, "%s=%d %s=%d %s=%*pbl\n",
> >> + LGPU_LIMITS_NAME_WEIGHT,
> >> + CGROUP_WEIGHT_DFL,
> >> + LGPU_LIMITS_NAME_COUNT,
> >> + bitmap_weight(props->lgpu_slots,
> >> + props->lgpu_capacity),
> >> + LGPU_LIMITS_NAME_LIST,
> >> + props->lgpu_capacity,
> >> + props->lgpu_slots);
> >> + break;
> >> default:
> >> seq_puts(sf, "\n");
> >> break;
> >> @@ -358,7 +521,7 @@ static int drmcg_seq_show_fn(int id, void *ptr, void *data)
> >> drmcg_print_stats(ddr, sf, type);
> >> break;
> >> case DRMCG_FTYPE_LIMIT:
> >> - drmcg_print_limits(ddr, sf, type);
> >> + drmcg_print_limits(ddr, sf, type, minor->dev);
> >> break;
> >> case DRMCG_FTYPE_DEFAULT:
> >> drmcg_print_default(&minor->dev->drmcg_props, sf, type);
> >> @@ -415,6 +578,115 @@ static int drmcg_process_limit_s64_val(char *sval, bool is_mem,
> >> return rc;
> >> }
> >>
> >> +static void drmcg_nested_limit_parse(struct kernfs_open_file *of,
> >> + struct drm_device *dev, char *attrs)
> >> +{
> >> + DECLARE_BITMAP(tmp_bitmap, MAX_DRMCG_LGPU_CAPACITY);
> >> + DECLARE_BITMAP(chk_bitmap, MAX_DRMCG_LGPU_CAPACITY);
> >> + enum drmcg_res_type type =
> >> + DRMCG_CTF_PRIV2RESTYPE(of_cft(of)->private);
> >> + struct drmcg *drmcg = css_to_drmcg(of_css(of));
> >> + struct drmcg_props *props = &dev->drmcg_props;
> >> + char *cft_name = of_cft(of)->name;
> >> + int minor = dev->primary->index;
> >> + char *nested = strstrip(attrs);
> >> + struct drmcg_device_resource *ddr =
> >> + drmcg->dev_resources[minor];
> >> + char *attr;
> >> + char sname[256];
> >> + char sval[256];
> >> + s64 val;
> >> + int rc;
> >> +
> >> + while (nested != NULL) {
> >> + attr = strsep(&nested, " ");
> >> +
> >> + if (sscanf(attr, "%255[^=]=%255[^=]", sname, sval) != 2)
> >> + continue;
> >> +
> >> + switch (type) {
> >> + case DRMCG_TYPE_LGPU:
> >> + if (strncmp(sname, LGPU_LIMITS_NAME_LIST, 256) &&
> >> + strncmp(sname, LGPU_LIMITS_NAME_COUNT, 256) &&
> >> + strncmp(sname, LGPU_LIMITS_NAME_WEIGHT, 256))
> >> + continue;
> >> +
> >> + if (strncmp(sname, LGPU_LIMITS_NAME_WEIGHT, 256) &&
> >> + (!strcmp("max", sval) ||
> >> + !strcmp("default", sval))) {
> >> + bitmap_copy(ddr->lgpu_cfg, props->lgpu_slots,
> >> + props->lgpu_capacity);
> >> +
> >> + continue;
> >> + }
> >> +
> >> + if (strncmp(sname, LGPU_LIMITS_NAME_WEIGHT, 256) == 0) {
> >> + rc = drmcg_process_limit_s64_val(sval,
> >> + false, CGROUP_WEIGHT_DFL,
> >> + CGROUP_WEIGHT_MAX, &val);
> >> +
> >> + if (rc || val < CGROUP_WEIGHT_MIN ||
> >> + val > CGROUP_WEIGHT_MAX) {
> >> + drmcg_pr_cft_err(drmcg, rc, cft_name,
> >> + minor);
> >> + continue;
> >> + }
> >> +
> >> + ddr->lgpu_weight_cfg = val;
> >> + continue;
> >> + }
> >> +
> >> + if (strncmp(sname, LGPU_LIMITS_NAME_COUNT, 256) == 0) {
> >> + rc = drmcg_process_limit_s64_val(sval,
> >> + false, props->lgpu_capacity,
> >> + props->lgpu_capacity, &val);
> >> +
> >> + if (rc || val < 0) {
> >> + drmcg_pr_cft_err(drmcg, rc, cft_name,
> >> + minor);
> >> + continue;
> >> + }
> >> +
> >> + bitmap_zero(tmp_bitmap,
> >> + MAX_DRMCG_LGPU_CAPACITY);
> >> + bitmap_set(tmp_bitmap, 0, val);
> >> + }
> >> +
> >> + if (strncmp(sname, LGPU_LIMITS_NAME_LIST, 256) == 0) {
> >> + rc = bitmap_parselist(sval, tmp_bitmap,
> >> + MAX_DRMCG_LGPU_CAPACITY);
> >> +
> >> + if (rc) {
> >> + drmcg_pr_cft_err(drmcg, rc, cft_name,
> >> + minor);
> >> + continue;
> >> + }
> >> +
> >> + bitmap_andnot(chk_bitmap, tmp_bitmap,
> >> + props->lgpu_slots,
> >> + MAX_DRMCG_LGPU_CAPACITY);
> >> +
> >> + /* user setting does not intersect with
> >> + * available lgpu */
> >> + if (!bitmap_empty(chk_bitmap,
> >> + MAX_DRMCG_LGPU_CAPACITY)) {
> >> + drmcg_pr_cft_err(drmcg, 0, cft_name,
> >> + minor);
> >> + continue;
> >> + }
> >> + }
> >> +
> >> + bitmap_copy(ddr->lgpu_cfg, tmp_bitmap,
> >> + props->lgpu_capacity);
> >> +
> >> + break; /* DRMCG_TYPE_LGPU */
> >> + default:
> >> + break;
> >> + } /* switch (type) */
> >> + }
> >> +}
> >> +
> >> +
> >> /**
> >> * drmcg_limit_write - parse cgroup interface files to obtain user config
> >> *
> >> @@ -499,9 +771,15 @@ static ssize_t drmcg_limit_write(struct kernfs_open_file *of, char *buf,
> >>
> >> ddr->bo_limits_peak_allocated = val;
> >> break;
> >> + case DRMCG_TYPE_LGPU:
> >> + drmcg_nested_limit_parse(of, dm->dev, sattr);
> >> + break;
> >> default:
> >> break;
> >> }
> >> +
> >> + drmcg_apply_effective(type, dm->dev, drmcg);
> >> +
> >> mutex_unlock(&dm->dev->drmcg_mutex);
> >>
> >> mutex_lock(&drmcg_mutex);
> >> @@ -560,12 +838,51 @@ struct cftype files[] = {
> >> .private = DRMCG_CTF_PRIV(DRMCG_TYPE_BO_COUNT,
> >> DRMCG_FTYPE_STATS),
> >> },
> >> + {
> >> + .name = "lgpu",
> >> + .seq_show = drmcg_seq_show,
> >> + .write = drmcg_limit_write,
> >> + .private = DRMCG_CTF_PRIV(DRMCG_TYPE_LGPU,
> >> + DRMCG_FTYPE_LIMIT),
> >> + },
> >> + {
> >> + .name = "lgpu.default",
> >> + .seq_show = drmcg_seq_show,
> >> + .flags = CFTYPE_ONLY_ON_ROOT,
> >> + .private = DRMCG_CTF_PRIV(DRMCG_TYPE_LGPU,
> >> + DRMCG_FTYPE_DEFAULT),
> >> + },
> >> + {
> >> + .name = "lgpu.effective",
> >> + .seq_show = drmcg_seq_show,
> >> + .private = DRMCG_CTF_PRIV(DRMCG_TYPE_LGPU_EFF,
> >> + DRMCG_FTYPE_LIMIT),
> >> + },
> >> { } /* terminate */
> >> };
> >>
> >> +static int drmcg_online_fn(int id, void *ptr, void *data)
> >> +{
> >> + struct drm_minor *minor = ptr;
> >> + struct drmcg *drmcg = data;
> >> +
> >> + if (minor->type != DRM_MINOR_PRIMARY)
> >> + return 0;
> >> +
> >> + drmcg_apply_effective(DRMCG_TYPE_LGPU, minor->dev, drmcg);
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static int drmcg_css_online(struct cgroup_subsys_state *css)
> >> +{
> >> + return drm_minor_for_each(&drmcg_online_fn, css_to_drmcg(css));
> >> +}
> >> +
> >> struct cgroup_subsys drm_cgrp_subsys = {
> >> .css_alloc = drmcg_css_alloc,
> >> .css_free = drmcg_css_free,
> >> + .css_online = drmcg_css_online,
> >> .early_init = false,
> >> .legacy_cftypes = files,
> >> .dfl_cftypes = files,
> >> @@ -585,6 +902,9 @@ void drmcg_device_early_init(struct drm_device *dev)
> >> dev->drmcg_props.bo_limits_total_allocated_default = S64_MAX;
> >> dev->drmcg_props.bo_limits_peak_allocated_default = S64_MAX;
> >>
> >> + dev->drmcg_props.lgpu_capacity = MAX_DRMCG_LGPU_CAPACITY;
> >> + bitmap_fill(dev->drmcg_props.lgpu_slots, MAX_DRMCG_LGPU_CAPACITY);
> >> +
> >> drmcg_update_cg_tree(dev);
> >> }
> >> EXPORT_SYMBOL(drmcg_device_early_init);
> >> --
> >> 2.25.0
> >>
> >> _______________________________________________
> >> dri-devel mailing list
> >> dri-devel@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply
* [PATCH AUTOSEL 5.5 493/542] KVM: PPC: Book3S HV: Release lock on page-out failure path
From: Sasha Levin @ 2020-02-14 15:48 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Bharata B Rao, Kamalesh Babulal, Paul Mackerras, Sasha Levin,
kvm-ppc, linuxppc-dev
In-Reply-To: <20200214154854.6746-1-sashal@kernel.org>
From: Bharata B Rao <bharata@linux.ibm.com>
[ Upstream commit e032e3b55b6f487e48c163c5dca74086f147a169 ]
When migrate_vma_setup() fails in kvmppc_svm_page_out(),
release kvm->arch.uvmem_lock before returning.
Fixes: ca9f4942670 ("KVM: PPC: Book3S HV: Support for running secure guests")
Signed-off-by: Bharata B Rao <bharata@linux.ibm.com>
Reviewed-by: Kamalesh Babulal <kamalesh@linux.vnet.ibm.com>
Signed-off-by: Paul Mackerras <paulus@ozlabs.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/powerpc/kvm/book3s_hv_uvmem.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/kvm/book3s_hv_uvmem.c b/arch/powerpc/kvm/book3s_hv_uvmem.c
index 2de264fc31563..5914fbfa5e0a7 100644
--- a/arch/powerpc/kvm/book3s_hv_uvmem.c
+++ b/arch/powerpc/kvm/book3s_hv_uvmem.c
@@ -543,7 +543,7 @@ kvmppc_svm_page_out(struct vm_area_struct *vma, unsigned long start,
ret = migrate_vma_setup(&mig);
if (ret)
- return ret;
+ goto out;
spage = migrate_pfn_to_page(*mig.src);
if (!spage || !(*mig.src & MIGRATE_PFN_MIGRATE))
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 5.5 496/542] char: hpet: Fix out-of-bounds read bug
From: Sasha Levin @ 2020-02-14 15:48 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Gustavo A. R. Silva, Tetsuo Handa, Eric Biggers,
Greg Kroah-Hartman, Sasha Levin
In-Reply-To: <20200214154854.6746-1-sashal@kernel.org>
From: "Gustavo A. R. Silva" <gustavo@embeddedor.com>
[ Upstream commit 98c49f1746ac44ccc164e914b9a44183fad09f51 ]
Currently, there is an out-of-bounds read on array hpetp->hp_dev
in the following for loop:
870 for (i = 0; i < hdp->hd_nirqs; i++)
871 hpetp->hp_dev[i].hd_hdwirq = hdp->hd_irq[i];
This is due to the recent change from one-element array to
flexible-array member in struct hpets:
104 struct hpets {
...
113 struct hpet_dev hp_dev[];
114 };
This change affected the total size of the dynamic memory
allocation, decreasing it by one time the size of struct hpet_dev.
Fix this by adjusting the allocation size when calling
struct_size().
Fixes: 987f028b8637c ("char: hpet: Use flexible-array member")
Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Acked-by: Eric Biggers <ebiggers@kernel.org>
Link: https://lore.kernel.org/r/20200129022613.GA24281@embeddedor.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/char/hpet.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/char/hpet.c b/drivers/char/hpet.c
index aed2c45f7968c..ed3b7dab678db 100644
--- a/drivers/char/hpet.c
+++ b/drivers/char/hpet.c
@@ -855,7 +855,7 @@ int hpet_alloc(struct hpet_data *hdp)
return 0;
}
- hpetp = kzalloc(struct_size(hpetp, hp_dev, hdp->hd_nirqs - 1),
+ hpetp = kzalloc(struct_size(hpetp, hp_dev, hdp->hd_nirqs),
GFP_KERNEL);
if (!hpetp)
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 5.5 498/542] trigger_next should increase position index
From: Sasha Levin @ 2020-02-14 15:48 UTC (permalink / raw)
To: linux-kernel, stable; +Cc: Vasily Averin, Steven Rostedt, Sasha Levin
In-Reply-To: <20200214154854.6746-1-sashal@kernel.org>
From: Vasily Averin <vvs@virtuozzo.com>
[ Upstream commit 6722b23e7a2ace078344064a9735fb73e554e9ef ]
if seq_file .next fuction does not change position index,
read after some lseek can generate unexpected output.
Without patch:
# dd bs=30 skip=1 if=/sys/kernel/tracing/events/sched/sched_switch/trigger
dd: /sys/kernel/tracing/events/sched/sched_switch/trigger: cannot skip to specified offset
n traceoff snapshot stacktrace enable_event disable_event enable_hist disable_hist hist
# Available triggers:
# traceon traceoff snapshot stacktrace enable_event disable_event enable_hist disable_hist hist
6+1 records in
6+1 records out
206 bytes copied, 0.00027916 s, 738 kB/s
Notice the printing of "# Available triggers:..." after the line.
With the patch:
# dd bs=30 skip=1 if=/sys/kernel/tracing/events/sched/sched_switch/trigger
dd: /sys/kernel/tracing/events/sched/sched_switch/trigger: cannot skip to specified offset
n traceoff snapshot stacktrace enable_event disable_event enable_hist disable_hist hist
2+1 records in
2+1 records out
88 bytes copied, 0.000526867 s, 167 kB/s
It only prints the end of the file, and does not restart.
Link: http://lkml.kernel.org/r/3c35ee24-dd3a-8119-9c19-552ed253388a@virtuozzo.com
https://bugzilla.kernel.org/show_bug.cgi?id=206283
Signed-off-by: Vasily Averin <vvs@virtuozzo.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
kernel/trace/trace_events_trigger.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/kernel/trace/trace_events_trigger.c b/kernel/trace/trace_events_trigger.c
index 40106fff06a48..287d77eae59b3 100644
--- a/kernel/trace/trace_events_trigger.c
+++ b/kernel/trace/trace_events_trigger.c
@@ -116,9 +116,10 @@ static void *trigger_next(struct seq_file *m, void *t, loff_t *pos)
{
struct trace_event_file *event_file = event_file_data(m->private);
- if (t == SHOW_AVAILABLE_TRIGGERS)
+ if (t == SHOW_AVAILABLE_TRIGGERS) {
+ (*pos)++;
return NULL;
-
+ }
return seq_list_next(t, &event_file->triggers, pos);
}
--
2.20.1
^ permalink raw reply related
* Re: [PATCH 09/11] drm, cgroup: Introduce lgpu as DRM cgroup resource
From: Jason Ekstrand @ 2020-02-14 17:48 UTC (permalink / raw)
To: Kenny Ho
Cc: juan.zuniga-anaya-5C7GfCeVMHo, Daniel Vetter, Kenny Ho,
Kuehling, Felix, jsparks-WVYJKLFxKCc, amd-gfx mailing list,
lkaplan-WVYJKLFxKCc, Alex Deucher, nirmoy.das-5C7GfCeVMHo,
Maling list - DRI developers, Greathouse, Joseph, Tejun Heo,
cgroups-u79uwXL29TY76Z2rM5mHXA, Christian König,
damon.mcdougall-5C7GfCeVMHo
In-Reply-To: <CAOWid-f62Uv=GZXX2V2BsQGM5A1JJG_qmyrOwd=KwZBx_sr-bg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Fri, Feb 14, 2020 at 11:08 AM Kenny Ho <y2kenny-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>
> Hi Jason,
>
> Thanks for the review.
>
> On Fri, Feb 14, 2020 at 11:44 AM Jason Ekstrand <jason-fQELhIk9awXprZlt/sZkLg@public.gmane.org> wrote:
> >
> > Pardon my ignorance but I'm a bit confused by this. What is a "logical GPU"? What are we subdividing? Are we carving up memory? Compute power? Both?
>
> The intention is compute but it is up to the individual drm driver to decide.
>
> > If it's carving up compute power, what's actually being carved up? Time? Execution units/waves/threads? Even if that's the case, what advantage does it give to have it in terms of a fixed set of lgpus where each cgroup gets to pick a fixed set. Does affinity matter that much? Why not just say how many waves the GPU supports and that they have to be allocated in chunks of 16 waves (pulling a number out of thin air) and let the cgroup specify how many waves it wants.
> >
> > Don't get me wrong here. I'm all for the notion of being able to use cgroups to carve up GPU compute resources. However, this sounds to me like the most AMD-specific solution possible. We (Intel) could probably do some sort of carving up as well but we'd likely want to do it with preemption and time-slicing rather than handing out specific EUs.
>
> This has been discussed in the RFC before
> (https://www.spinics.net/lists/cgroups/msg23469.html.) As mentioned
> before, the idea of a compute unit is hardly an AMD specific thing as
> it is in the OpenCL standard and part of the architecture of many
> different vendors. In addition, the interface presented here supports
> Intel's use case. What you described is what I considered as the
> "anonymous resources" view of the lgpu. What you/Intel can do, is to
> register your device to drmcg to have 100 lgpu and users can specify
> simply by count. So if they want to allocate 5% for a cgroup, they
> would set count=5. Per the documentation in this patch: "Some DRM
> devices may only support lgpu as anonymous resources. In such case,
> the significance of the position of the set bits in list will be
> ignored." What Intel does with the user expressed configuration of "5
> out of 100" is entirely up to Intel (time slice if you like, change to
> specific EUs later if you like, or make it driver configurable to
> support both if you like.)
Sure, there's an OpenCL thing. However, just because there's an
OpenCL thing doesn't mean that it's as standardized as it looks. :-(
In particular,
1. The OpenCL thing has a query first to ask the driver what kind of
carving up of the GPU is allowed
2. When clCreateSubdevices is called, the type of partitioning is
specified so they can specifically ask for devices grouped by shared
L2 cache, for instance.
3. Just because the API exists and everyone implements it doesn't
mean that everyone implements it usefully. From my reading of the
spec, it looks like the API is very much designed towards a CPU
implementation of OpenCL. The Intel OpenCL GPU compute drivers, for
instance, implement it as a total no-op and no real sub-dividing is
allowed.
That said, that doesn't necessarily mean that carving up units of
compute power is a bad plan. It's just unclear (as Daniel said on the
above referenced chain) what those units mean. Maybe it's ok if they
mean nothing or if their meaning is HW-specific?
> Regards,
> Kenny
>
> >
> > On Fri, Feb 14, 2020 at 9:57 AM Kenny Ho <Kenny.Ho-5C7GfCeVMHo@public.gmane.org> wrote:
> >>
> >> drm.lgpu
> >> A read-write nested-keyed file which exists on all cgroups.
> >> Each entry is keyed by the DRM device's major:minor.
> >>
> >> lgpu stands for logical GPU, it is an abstraction used to
> >> subdivide a physical DRM device for the purpose of resource
> >> management. This file stores user configuration while the
> >> drm.lgpu.effective reflects the actual allocation after
> >> considering the relationship between the cgroups and their
> >> configurations.
> >>
> >> The lgpu is a discrete quantity that is device specific (i.e.
> >> some DRM devices may have 64 lgpus while others may have 100
> >> lgpus.) The lgpu is a single quantity that can be allocated
> >> in three different ways denoted by the following nested keys.
> >>
> >> ===== ==============================================
> >> weight Allocate by proportion in relationship with
> >> active sibling cgroups
> >> count Allocate by amount statically, treat lgpu as
> >> anonymous resources
> >> list Allocate statically, treat lgpu as named
> >> resource
> >> ===== ==============================================
> >>
> >> For example:
> >> 226:0 weight=100 count=256 list=0-255
> >> 226:1 weight=100 count=4 list=0,2,4,6
> >> 226:2 weight=100 count=32 list=32-63
> >> 226:3 weight=100 count=0 list=
> >> 226:4 weight=500 count=0 list=
> >>
> >> lgpu is represented by a bitmap and uses the bitmap_parselist
> >> kernel function so the list key input format is a
> >> comma-separated list of decimal numbers and ranges.
> >>
> >> Consecutively set bits are shown as two hyphen-separated decimal
> >> numbers, the smallest and largest bit numbers set in the range.
> >> Optionally each range can be postfixed to denote that only parts
> >> of it should be set. The range will divided to groups of
> >> specific size.
> >> Syntax: range:used_size/group_size
> >> Example: 0-1023:2/256 ==> 0,1,256,257,512,513,768,769
> >>
> >> The count key is the hamming weight / hweight of the bitmap.
> >>
> >> Weight, count and list accept the max and default keywords.
> >>
> >> Some DRM devices may only support lgpu as anonymous resources.
> >> In such case, the significance of the position of the set bits
> >> in list will be ignored.
> >>
> >> The weight quantity is only in effect when static allocation
> >> is not used (by setting count=0) for this cgroup. The weight
> >> quantity distributes lgpus that are not statically allocated by
> >> the siblings. For example, given siblings cgroupA, cgroupB and
> >> cgroupC for a DRM device that has 64 lgpus, if cgroupA occupies
> >> 0-63, no lgpu is available to be distributed by weight.
> >> Similarly, if cgroupA has list=0-31 and cgroupB has list=16-63,
> >> cgroupC will be starved if it tries to allocate by weight.
> >>
> >> On the other hand, if cgroupA has weight=100 count=0, cgroupB
> >> has list=16-47, and cgroupC has weight=100 count=0, then 32
> >> lgpus are available to be distributed evenly between cgroupA
> >> and cgroupC. In drm.lgpu.effective, cgroupA will have
> >> list=0-15 and cgroupC will have list=48-63.
> >>
> >> This lgpu resource supports the 'allocation' and 'weight'
> >> resource distribution model.
> >>
> >> drm.lgpu.effective
> >> A read-only nested-keyed file which exists on all cgroups.
> >> Each entry is keyed by the DRM device's major:minor.
> >>
> >> lgpu stands for logical GPU, it is an abstraction used to
> >> subdivide a physical DRM device for the purpose of resource
> >> management. This file reflects the actual allocation after
> >> considering the relationship between the cgroups and their
> >> configurations in drm.lgpu.
> >>
> >> Change-Id: Idde0ef9a331fd67bb9c7eb8ef9978439e6452488
> >> Signed-off-by: Kenny Ho <Kenny.Ho-5C7GfCeVMHo@public.gmane.org>
> >> ---
> >> Documentation/admin-guide/cgroup-v2.rst | 80 ++++++
> >> include/drm/drm_cgroup.h | 3 +
> >> include/linux/cgroup_drm.h | 22 ++
> >> kernel/cgroup/drm.c | 324 +++++++++++++++++++++++-
> >> 4 files changed, 427 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/Documentation/admin-guide/cgroup-v2.rst b/Documentation/admin-guide/cgroup-v2.rst
> >> index ce5dc027366a..d8a41956e5c7 100644
> >> --- a/Documentation/admin-guide/cgroup-v2.rst
> >> +++ b/Documentation/admin-guide/cgroup-v2.rst
> >> @@ -2120,6 +2120,86 @@ DRM Interface Files
> >> Set largest allocation for /dev/dri/card1 to 4MB
> >> echo "226:1 4m" > drm.buffer.peak.max
> >>
> >> + drm.lgpu
> >> + A read-write nested-keyed file which exists on all cgroups.
> >> + Each entry is keyed by the DRM device's major:minor.
> >> +
> >> + lgpu stands for logical GPU, it is an abstraction used to
> >> + subdivide a physical DRM device for the purpose of resource
> >> + management. This file stores user configuration while the
> >> + drm.lgpu.effective reflects the actual allocation after
> >> + considering the relationship between the cgroups and their
> >> + configurations.
> >> +
> >> + The lgpu is a discrete quantity that is device specific (i.e.
> >> + some DRM devices may have 64 lgpus while others may have 100
> >> + lgpus.) The lgpu is a single quantity that can be allocated
> >> + in three different ways denoted by the following nested keys.
> >> +
> >> + ===== ==============================================
> >> + weight Allocate by proportion in relationship with
> >> + active sibling cgroups
> >> + count Allocate by amount statically, treat lgpu as
> >> + anonymous resources
> >> + list Allocate statically, treat lgpu as named
> >> + resource
> >> + ===== ==============================================
> >> +
> >> + For example:
> >> + 226:0 weight=100 count=256 list=0-255
> >> + 226:1 weight=100 count=4 list=0,2,4,6
> >> + 226:2 weight=100 count=32 list=32-63
> >> + 226:3 weight=100 count=0 list=
> >> + 226:4 weight=500 count=0 list=
> >> +
> >> + lgpu is represented by a bitmap and uses the bitmap_parselist
> >> + kernel function so the list key input format is a
> >> + comma-separated list of decimal numbers and ranges.
> >> +
> >> + Consecutively set bits are shown as two hyphen-separated decimal
> >> + numbers, the smallest and largest bit numbers set in the range.
> >> + Optionally each range can be postfixed to denote that only parts
> >> + of it should be set. The range will divided to groups of
> >> + specific size.
> >> + Syntax: range:used_size/group_size
> >> + Example: 0-1023:2/256 ==> 0,1,256,257,512,513,768,769
> >> +
> >> + The count key is the hamming weight / hweight of the bitmap.
> >> +
> >> + Weight, count and list accept the max and default keywords.
> >> +
> >> + Some DRM devices may only support lgpu as anonymous resources.
> >> + In such case, the significance of the position of the set bits
> >> + in list will be ignored.
> >> +
> >> + The weight quantity is only in effect when static allocation
> >> + is not used (by setting count=0) for this cgroup. The weight
> >> + quantity distributes lgpus that are not statically allocated by
> >> + the siblings. For example, given siblings cgroupA, cgroupB and
> >> + cgroupC for a DRM device that has 64 lgpus, if cgroupA occupies
> >> + 0-63, no lgpu is available to be distributed by weight.
> >> + Similarly, if cgroupA has list=0-31 and cgroupB has list=16-63,
> >> + cgroupC will be starved if it tries to allocate by weight.
> >> +
> >> + On the other hand, if cgroupA has weight=100 count=0, cgroupB
> >> + has list=16-47, and cgroupC has weight=100 count=0, then 32
> >> + lgpus are available to be distributed evenly between cgroupA
> >> + and cgroupC. In drm.lgpu.effective, cgroupA will have
> >> + list=0-15 and cgroupC will have list=48-63.
> >> +
> >> + This lgpu resource supports the 'allocation' and 'weight'
> >> + resource distribution model.
> >> +
> >> + drm.lgpu.effective
> >> + A read-only nested-keyed file which exists on all cgroups.
> >> + Each entry is keyed by the DRM device's major:minor.
> >> +
> >> + lgpu stands for logical GPU, it is an abstraction used to
> >> + subdivide a physical DRM device for the purpose of resource
> >> + management. This file reflects the actual allocation after
> >> + considering the relationship between the cgroups and their
> >> + configurations in drm.lgpu.
> >> +
> >> GEM Buffer Ownership
> >> ~~~~~~~~~~~~~~~~~~~~
> >>
> >> diff --git a/include/drm/drm_cgroup.h b/include/drm/drm_cgroup.h
> >> index 2b41d4d22e33..619a110cc748 100644
> >> --- a/include/drm/drm_cgroup.h
> >> +++ b/include/drm/drm_cgroup.h
> >> @@ -17,6 +17,9 @@ struct drmcg_props {
> >>
> >> s64 bo_limits_total_allocated_default;
> >> s64 bo_limits_peak_allocated_default;
> >> +
> >> + int lgpu_capacity;
> >> + DECLARE_BITMAP(lgpu_slots, MAX_DRMCG_LGPU_CAPACITY);
> >> };
> >>
> >> void drmcg_bind(struct drm_minor (*(*acq_dm)(unsigned int minor_id)),
> >> diff --git a/include/linux/cgroup_drm.h b/include/linux/cgroup_drm.h
> >> index eae400f3d9b4..bb09704e7f71 100644
> >> --- a/include/linux/cgroup_drm.h
> >> +++ b/include/linux/cgroup_drm.h
> >> @@ -11,10 +11,14 @@
> >> /* limit defined per the way drm_minor_alloc operates */
> >> #define MAX_DRM_DEV (64 * DRM_MINOR_RENDER)
> >>
> >> +#define MAX_DRMCG_LGPU_CAPACITY 256
> >> +
> >> enum drmcg_res_type {
> >> DRMCG_TYPE_BO_TOTAL,
> >> DRMCG_TYPE_BO_PEAK,
> >> DRMCG_TYPE_BO_COUNT,
> >> + DRMCG_TYPE_LGPU,
> >> + DRMCG_TYPE_LGPU_EFF,
> >> __DRMCG_TYPE_LAST,
> >> };
> >>
> >> @@ -32,6 +36,24 @@ struct drmcg_device_resource {
> >> s64 bo_limits_peak_allocated;
> >>
> >> s64 bo_stats_count_allocated;
> >> +
> >> + /**
> >> + * Logical GPU
> >> + *
> >> + * *_cfg are properties configured by users
> >> + * *_eff are the effective properties being applied to the hardware
> >> + * *_stg is used to calculate _eff before applying to _eff
> >> + * after considering the entire hierarchy
> >> + */
> >> + DECLARE_BITMAP(lgpu_stg, MAX_DRMCG_LGPU_CAPACITY);
> >> + /* user configurations */
> >> + s64 lgpu_weight_cfg;
> >> + DECLARE_BITMAP(lgpu_cfg, MAX_DRMCG_LGPU_CAPACITY);
> >> + /* effective lgpu for the cgroup after considering
> >> + * relationship with other cgroup
> >> + */
> >> + s64 lgpu_count_eff;
> >> + DECLARE_BITMAP(lgpu_eff, MAX_DRMCG_LGPU_CAPACITY);
> >> };
> >>
> >> /**
> >> diff --git a/kernel/cgroup/drm.c b/kernel/cgroup/drm.c
> >> index 5fcbbc13fa1c..a4e88a3704bb 100644
> >> --- a/kernel/cgroup/drm.c
> >> +++ b/kernel/cgroup/drm.c
> >> @@ -9,6 +9,7 @@
> >> #include <linux/seq_file.h>
> >> #include <linux/mutex.h>
> >> #include <linux/kernel.h>
> >> +#include <linux/bitmap.h>
> >> #include <linux/cgroup_drm.h>
> >> #include <drm/drm_file.h>
> >> #include <drm/drm_drv.h>
> >> @@ -41,6 +42,10 @@ enum drmcg_file_type {
> >> DRMCG_FTYPE_DEFAULT,
> >> };
> >>
> >> +#define LGPU_LIMITS_NAME_LIST "list"
> >> +#define LGPU_LIMITS_NAME_COUNT "count"
> >> +#define LGPU_LIMITS_NAME_WEIGHT "weight"
> >> +
> >> /**
> >> * drmcg_bind - Bind DRM subsystem to cgroup subsystem
> >> * @acq_dm: function pointer to the drm_minor_acquire function
> >> @@ -98,6 +103,13 @@ static inline int init_drmcg_single(struct drmcg *drmcg, struct drm_device *dev)
> >> ddr->bo_limits_peak_allocated =
> >> dev->drmcg_props.bo_limits_peak_allocated_default;
> >>
> >> + bitmap_copy(ddr->lgpu_cfg, dev->drmcg_props.lgpu_slots,
> >> + MAX_DRMCG_LGPU_CAPACITY);
> >> + bitmap_copy(ddr->lgpu_stg, dev->drmcg_props.lgpu_slots,
> >> + MAX_DRMCG_LGPU_CAPACITY);
> >> +
> >> + ddr->lgpu_weight_cfg = CGROUP_WEIGHT_DFL;
> >> +
> >> return 0;
> >> }
> >>
> >> @@ -121,6 +133,120 @@ static inline void drmcg_update_cg_tree(struct drm_device *dev)
> >> mutex_unlock(&cgroup_mutex);
> >> }
> >>
> >> +static void drmcg_calculate_effective_lgpu(struct drm_device *dev,
> >> + const unsigned long *free_static,
> >> + const unsigned long *free_weighted,
> >> + struct drmcg *parent_drmcg)
> >> +{
> >> + int capacity = dev->drmcg_props.lgpu_capacity;
> >> + DECLARE_BITMAP(lgpu_unused, MAX_DRMCG_LGPU_CAPACITY);
> >> + DECLARE_BITMAP(lgpu_by_weight, MAX_DRMCG_LGPU_CAPACITY);
> >> + struct drmcg_device_resource *parent_ddr;
> >> + struct drmcg_device_resource *ddr;
> >> + int minor = dev->primary->index;
> >> + struct cgroup_subsys_state *pos;
> >> + struct drmcg *child;
> >> + s64 weight_sum = 0;
> >> + s64 unused;
> >> +
> >> + parent_ddr = parent_drmcg->dev_resources[minor];
> >> +
> >> + if (bitmap_empty(parent_ddr->lgpu_cfg, capacity))
> >> + /* no static cfg, use weight for calculating the effective */
> >> + bitmap_copy(parent_ddr->lgpu_stg, free_weighted, capacity);
> >> + else
> >> + /* lgpu statically configured, use the overlap as effective */
> >> + bitmap_and(parent_ddr->lgpu_stg, free_static,
> >> + parent_ddr->lgpu_cfg, capacity);
> >> +
> >> + /* calculate lgpu available for distribution by weight for children */
> >> + bitmap_copy(lgpu_unused, parent_ddr->lgpu_stg, capacity);
> >> + css_for_each_child(pos, &parent_drmcg->css) {
> >> + child = css_to_drmcg(pos);
> >> + ddr = child->dev_resources[minor];
> >> +
> >> + if (bitmap_empty(ddr->lgpu_cfg, capacity))
> >> + /* no static allocation, participate in weight dist */
> >> + weight_sum += ddr->lgpu_weight_cfg;
> >> + else
> >> + /* take out statically allocated lgpu by siblings */
> >> + bitmap_andnot(lgpu_unused, lgpu_unused, ddr->lgpu_cfg,
> >> + capacity);
> >> + }
> >> +
> >> + unused = bitmap_weight(lgpu_unused, capacity);
> >> +
> >> + css_for_each_child(pos, &parent_drmcg->css) {
> >> + child = css_to_drmcg(pos);
> >> + ddr = child->dev_resources[minor];
> >> +
> >> + bitmap_zero(lgpu_by_weight, capacity);
> >> + /* no static allocation, participate in weight distribution */
> >> + if (bitmap_empty(ddr->lgpu_cfg, capacity)) {
> >> + int c;
> >> + int p = 0;
> >> +
> >> + for (c = ddr->lgpu_weight_cfg * unused / weight_sum;
> >> + c > 0; c--) {
> >> + p = find_next_bit(lgpu_unused, capacity, p);
> >> + if (p < capacity) {
> >> + clear_bit(p, lgpu_unused);
> >> + set_bit(p, lgpu_by_weight);
> >> + }
> >> + }
> >> +
> >> + }
> >> +
> >> + drmcg_calculate_effective_lgpu(dev, parent_ddr->lgpu_stg,
> >> + lgpu_by_weight, child);
> >> + }
> >> +}
> >> +
> >> +static void drmcg_apply_effective_lgpu(struct drm_device *dev)
> >> +{
> >> + int capacity = dev->drmcg_props.lgpu_capacity;
> >> + int minor = dev->primary->index;
> >> + struct drmcg_device_resource *ddr;
> >> + struct cgroup_subsys_state *pos;
> >> + struct drmcg *drmcg;
> >> +
> >> + if (root_drmcg == NULL) {
> >> + WARN_ON(root_drmcg == NULL);
> >> + return;
> >> + }
> >> +
> >> + rcu_read_lock();
> >> +
> >> + /* process the entire cgroup tree from root to simplify the algorithm */
> >> + drmcg_calculate_effective_lgpu(dev, dev->drmcg_props.lgpu_slots,
> >> + dev->drmcg_props.lgpu_slots, root_drmcg);
> >> +
> >> + /* apply changes to effective only if there is a change */
> >> + css_for_each_descendant_pre(pos, &root_drmcg->css) {
> >> + drmcg = css_to_drmcg(pos);
> >> + ddr = drmcg->dev_resources[minor];
> >> +
> >> + if (!bitmap_equal(ddr->lgpu_stg, ddr->lgpu_eff, capacity)) {
> >> + bitmap_copy(ddr->lgpu_eff, ddr->lgpu_stg, capacity);
> >> + ddr->lgpu_count_eff =
> >> + bitmap_weight(ddr->lgpu_eff, capacity);
> >> + }
> >> + }
> >> + rcu_read_unlock();
> >> +}
> >> +
> >> +static void drmcg_apply_effective(enum drmcg_res_type type,
> >> + struct drm_device *dev, struct drmcg *changed_drmcg)
> >> +{
> >> + switch (type) {
> >> + case DRMCG_TYPE_LGPU:
> >> + drmcg_apply_effective_lgpu(dev);
> >> + break;
> >> + default:
> >> + break;
> >> + }
> >> +}
> >> +
> >> /**
> >> * drmcg_register_dev - register a DRM device for usage in drm cgroup
> >> * @dev: DRM device
> >> @@ -143,7 +269,13 @@ void drmcg_register_dev(struct drm_device *dev)
> >> {
> >> dev->driver->drmcg_custom_init(dev, &dev->drmcg_props);
> >>
> >> + WARN_ON(dev->drmcg_props.lgpu_capacity !=
> >> + bitmap_weight(dev->drmcg_props.lgpu_slots,
> >> + MAX_DRMCG_LGPU_CAPACITY));
> >> +
> >> drmcg_update_cg_tree(dev);
> >> +
> >> + drmcg_apply_effective(DRMCG_TYPE_LGPU, dev, root_drmcg);
> >> }
> >> mutex_unlock(&drmcg_mutex);
> >> }
> >> @@ -297,7 +429,8 @@ static void drmcg_print_stats(struct drmcg_device_resource *ddr,
> >> }
> >>
> >> static void drmcg_print_limits(struct drmcg_device_resource *ddr,
> >> - struct seq_file *sf, enum drmcg_res_type type)
> >> + struct seq_file *sf, enum drmcg_res_type type,
> >> + struct drm_device *dev)
> >> {
> >> if (ddr == NULL) {
> >> seq_puts(sf, "\n");
> >> @@ -311,6 +444,25 @@ static void drmcg_print_limits(struct drmcg_device_resource *ddr,
> >> case DRMCG_TYPE_BO_PEAK:
> >> seq_printf(sf, "%lld\n", ddr->bo_limits_peak_allocated);
> >> break;
> >> + case DRMCG_TYPE_LGPU:
> >> + seq_printf(sf, "%s=%lld %s=%d %s=%*pbl\n",
> >> + LGPU_LIMITS_NAME_WEIGHT,
> >> + ddr->lgpu_weight_cfg,
> >> + LGPU_LIMITS_NAME_COUNT,
> >> + bitmap_weight(ddr->lgpu_cfg,
> >> + dev->drmcg_props.lgpu_capacity),
> >> + LGPU_LIMITS_NAME_LIST,
> >> + dev->drmcg_props.lgpu_capacity,
> >> + ddr->lgpu_cfg);
> >> + break;
> >> + case DRMCG_TYPE_LGPU_EFF:
> >> + seq_printf(sf, "%s=%lld %s=%*pbl\n",
> >> + LGPU_LIMITS_NAME_COUNT,
> >> + ddr->lgpu_count_eff,
> >> + LGPU_LIMITS_NAME_LIST,
> >> + dev->drmcg_props.lgpu_capacity,
> >> + ddr->lgpu_eff);
> >> + break;
> >> default:
> >> seq_puts(sf, "\n");
> >> break;
> >> @@ -329,6 +481,17 @@ static void drmcg_print_default(struct drmcg_props *props,
> >> seq_printf(sf, "%lld\n",
> >> props->bo_limits_peak_allocated_default);
> >> break;
> >> + case DRMCG_TYPE_LGPU:
> >> + seq_printf(sf, "%s=%d %s=%d %s=%*pbl\n",
> >> + LGPU_LIMITS_NAME_WEIGHT,
> >> + CGROUP_WEIGHT_DFL,
> >> + LGPU_LIMITS_NAME_COUNT,
> >> + bitmap_weight(props->lgpu_slots,
> >> + props->lgpu_capacity),
> >> + LGPU_LIMITS_NAME_LIST,
> >> + props->lgpu_capacity,
> >> + props->lgpu_slots);
> >> + break;
> >> default:
> >> seq_puts(sf, "\n");
> >> break;
> >> @@ -358,7 +521,7 @@ static int drmcg_seq_show_fn(int id, void *ptr, void *data)
> >> drmcg_print_stats(ddr, sf, type);
> >> break;
> >> case DRMCG_FTYPE_LIMIT:
> >> - drmcg_print_limits(ddr, sf, type);
> >> + drmcg_print_limits(ddr, sf, type, minor->dev);
> >> break;
> >> case DRMCG_FTYPE_DEFAULT:
> >> drmcg_print_default(&minor->dev->drmcg_props, sf, type);
> >> @@ -415,6 +578,115 @@ static int drmcg_process_limit_s64_val(char *sval, bool is_mem,
> >> return rc;
> >> }
> >>
> >> +static void drmcg_nested_limit_parse(struct kernfs_open_file *of,
> >> + struct drm_device *dev, char *attrs)
> >> +{
> >> + DECLARE_BITMAP(tmp_bitmap, MAX_DRMCG_LGPU_CAPACITY);
> >> + DECLARE_BITMAP(chk_bitmap, MAX_DRMCG_LGPU_CAPACITY);
> >> + enum drmcg_res_type type =
> >> + DRMCG_CTF_PRIV2RESTYPE(of_cft(of)->private);
> >> + struct drmcg *drmcg = css_to_drmcg(of_css(of));
> >> + struct drmcg_props *props = &dev->drmcg_props;
> >> + char *cft_name = of_cft(of)->name;
> >> + int minor = dev->primary->index;
> >> + char *nested = strstrip(attrs);
> >> + struct drmcg_device_resource *ddr =
> >> + drmcg->dev_resources[minor];
> >> + char *attr;
> >> + char sname[256];
> >> + char sval[256];
> >> + s64 val;
> >> + int rc;
> >> +
> >> + while (nested != NULL) {
> >> + attr = strsep(&nested, " ");
> >> +
> >> + if (sscanf(attr, "%255[^=]=%255[^=]", sname, sval) != 2)
> >> + continue;
> >> +
> >> + switch (type) {
> >> + case DRMCG_TYPE_LGPU:
> >> + if (strncmp(sname, LGPU_LIMITS_NAME_LIST, 256) &&
> >> + strncmp(sname, LGPU_LIMITS_NAME_COUNT, 256) &&
> >> + strncmp(sname, LGPU_LIMITS_NAME_WEIGHT, 256))
> >> + continue;
> >> +
> >> + if (strncmp(sname, LGPU_LIMITS_NAME_WEIGHT, 256) &&
> >> + (!strcmp("max", sval) ||
> >> + !strcmp("default", sval))) {
> >> + bitmap_copy(ddr->lgpu_cfg, props->lgpu_slots,
> >> + props->lgpu_capacity);
> >> +
> >> + continue;
> >> + }
> >> +
> >> + if (strncmp(sname, LGPU_LIMITS_NAME_WEIGHT, 256) == 0) {
> >> + rc = drmcg_process_limit_s64_val(sval,
> >> + false, CGROUP_WEIGHT_DFL,
> >> + CGROUP_WEIGHT_MAX, &val);
> >> +
> >> + if (rc || val < CGROUP_WEIGHT_MIN ||
> >> + val > CGROUP_WEIGHT_MAX) {
> >> + drmcg_pr_cft_err(drmcg, rc, cft_name,
> >> + minor);
> >> + continue;
> >> + }
> >> +
> >> + ddr->lgpu_weight_cfg = val;
> >> + continue;
> >> + }
> >> +
> >> + if (strncmp(sname, LGPU_LIMITS_NAME_COUNT, 256) == 0) {
> >> + rc = drmcg_process_limit_s64_val(sval,
> >> + false, props->lgpu_capacity,
> >> + props->lgpu_capacity, &val);
> >> +
> >> + if (rc || val < 0) {
> >> + drmcg_pr_cft_err(drmcg, rc, cft_name,
> >> + minor);
> >> + continue;
> >> + }
> >> +
> >> + bitmap_zero(tmp_bitmap,
> >> + MAX_DRMCG_LGPU_CAPACITY);
> >> + bitmap_set(tmp_bitmap, 0, val);
> >> + }
> >> +
> >> + if (strncmp(sname, LGPU_LIMITS_NAME_LIST, 256) == 0) {
> >> + rc = bitmap_parselist(sval, tmp_bitmap,
> >> + MAX_DRMCG_LGPU_CAPACITY);
> >> +
> >> + if (rc) {
> >> + drmcg_pr_cft_err(drmcg, rc, cft_name,
> >> + minor);
> >> + continue;
> >> + }
> >> +
> >> + bitmap_andnot(chk_bitmap, tmp_bitmap,
> >> + props->lgpu_slots,
> >> + MAX_DRMCG_LGPU_CAPACITY);
> >> +
> >> + /* user setting does not intersect with
> >> + * available lgpu */
> >> + if (!bitmap_empty(chk_bitmap,
> >> + MAX_DRMCG_LGPU_CAPACITY)) {
> >> + drmcg_pr_cft_err(drmcg, 0, cft_name,
> >> + minor);
> >> + continue;
> >> + }
> >> + }
> >> +
> >> + bitmap_copy(ddr->lgpu_cfg, tmp_bitmap,
> >> + props->lgpu_capacity);
> >> +
> >> + break; /* DRMCG_TYPE_LGPU */
> >> + default:
> >> + break;
> >> + } /* switch (type) */
> >> + }
> >> +}
> >> +
> >> +
> >> /**
> >> * drmcg_limit_write - parse cgroup interface files to obtain user config
> >> *
> >> @@ -499,9 +771,15 @@ static ssize_t drmcg_limit_write(struct kernfs_open_file *of, char *buf,
> >>
> >> ddr->bo_limits_peak_allocated = val;
> >> break;
> >> + case DRMCG_TYPE_LGPU:
> >> + drmcg_nested_limit_parse(of, dm->dev, sattr);
> >> + break;
> >> default:
> >> break;
> >> }
> >> +
> >> + drmcg_apply_effective(type, dm->dev, drmcg);
> >> +
> >> mutex_unlock(&dm->dev->drmcg_mutex);
> >>
> >> mutex_lock(&drmcg_mutex);
> >> @@ -560,12 +838,51 @@ struct cftype files[] = {
> >> .private = DRMCG_CTF_PRIV(DRMCG_TYPE_BO_COUNT,
> >> DRMCG_FTYPE_STATS),
> >> },
> >> + {
> >> + .name = "lgpu",
> >> + .seq_show = drmcg_seq_show,
> >> + .write = drmcg_limit_write,
> >> + .private = DRMCG_CTF_PRIV(DRMCG_TYPE_LGPU,
> >> + DRMCG_FTYPE_LIMIT),
> >> + },
> >> + {
> >> + .name = "lgpu.default",
> >> + .seq_show = drmcg_seq_show,
> >> + .flags = CFTYPE_ONLY_ON_ROOT,
> >> + .private = DRMCG_CTF_PRIV(DRMCG_TYPE_LGPU,
> >> + DRMCG_FTYPE_DEFAULT),
> >> + },
> >> + {
> >> + .name = "lgpu.effective",
> >> + .seq_show = drmcg_seq_show,
> >> + .private = DRMCG_CTF_PRIV(DRMCG_TYPE_LGPU_EFF,
> >> + DRMCG_FTYPE_LIMIT),
> >> + },
> >> { } /* terminate */
> >> };
> >>
> >> +static int drmcg_online_fn(int id, void *ptr, void *data)
> >> +{
> >> + struct drm_minor *minor = ptr;
> >> + struct drmcg *drmcg = data;
> >> +
> >> + if (minor->type != DRM_MINOR_PRIMARY)
> >> + return 0;
> >> +
> >> + drmcg_apply_effective(DRMCG_TYPE_LGPU, minor->dev, drmcg);
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static int drmcg_css_online(struct cgroup_subsys_state *css)
> >> +{
> >> + return drm_minor_for_each(&drmcg_online_fn, css_to_drmcg(css));
> >> +}
> >> +
> >> struct cgroup_subsys drm_cgrp_subsys = {
> >> .css_alloc = drmcg_css_alloc,
> >> .css_free = drmcg_css_free,
> >> + .css_online = drmcg_css_online,
> >> .early_init = false,
> >> .legacy_cftypes = files,
> >> .dfl_cftypes = files,
> >> @@ -585,6 +902,9 @@ void drmcg_device_early_init(struct drm_device *dev)
> >> dev->drmcg_props.bo_limits_total_allocated_default = S64_MAX;
> >> dev->drmcg_props.bo_limits_peak_allocated_default = S64_MAX;
> >>
> >> + dev->drmcg_props.lgpu_capacity = MAX_DRMCG_LGPU_CAPACITY;
> >> + bitmap_fill(dev->drmcg_props.lgpu_slots, MAX_DRMCG_LGPU_CAPACITY);
> >> +
> >> drmcg_update_cg_tree(dev);
> >> }
> >> EXPORT_SYMBOL(drmcg_device_early_init);
> >> --
> >> 2.25.0
> >>
> >> _______________________________________________
> >> dri-devel mailing list
> >> dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* [PATCH AUTOSEL 5.5 499/542] radeon: insert 10ms sleep in dce5_crtc_load_lut
From: Sasha Levin @ 2020-02-14 15:48 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Daniel Vetter, Alex Deucher, Michel Dänzer, Daniel Vetter,
Sasha Levin, amd-gfx, dri-devel
In-Reply-To: <20200214154854.6746-1-sashal@kernel.org>
From: Daniel Vetter <daniel.vetter@ffwll.ch>
[ Upstream commit ec3d65082d7dabad6fa8f66a8ef166f2d522d6b2 ]
Per at least one tester this is enough magic to recover the regression
introduced for some people (but not all) in
commit b8e2b0199cc377617dc238f5106352c06dcd3fa2
Author: Peter Rosin <peda@axentia.se>
Date: Tue Jul 4 12:36:57 2017 +0200
drm/fb-helper: factor out pseudo-palette
which for radeon had the side-effect of refactoring out a seemingly
redudant writing of the color palette.
10ms in a fairly slow modeset path feels like an acceptable form of
duct-tape, so maybe worth a shot and see what sticks.
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Michel Dänzer <michel.daenzer@amd.com>
References: https://bugzilla.kernel.org/show_bug.cgi?id=198123
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/gpu/drm/radeon/radeon_display.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/radeon/radeon_display.c b/drivers/gpu/drm/radeon/radeon_display.c
index 84d3d885b7a46..606972c4593aa 100644
--- a/drivers/gpu/drm/radeon/radeon_display.c
+++ b/drivers/gpu/drm/radeon/radeon_display.c
@@ -127,6 +127,8 @@ static void dce5_crtc_load_lut(struct drm_crtc *crtc)
DRM_DEBUG_KMS("%d\n", radeon_crtc->crtc_id);
+ msleep(10);
+
WREG32(NI_INPUT_CSC_CONTROL + radeon_crtc->crtc_offset,
(NI_INPUT_CSC_GRPH_MODE(NI_INPUT_CSC_BYPASS) |
NI_INPUT_CSC_OVL_MODE(NI_INPUT_CSC_BYPASS)));
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 5.5 497/542] ftrace: fpid_next() should increase position index
From: Sasha Levin @ 2020-02-14 15:48 UTC (permalink / raw)
To: linux-kernel, stable; +Cc: Vasily Averin, Steven Rostedt, Sasha Levin
In-Reply-To: <20200214154854.6746-1-sashal@kernel.org>
From: Vasily Averin <vvs@virtuozzo.com>
[ Upstream commit e4075e8bdffd93a9b6d6e1d52fabedceeca5a91b ]
if seq_file .next fuction does not change position index,
read after some lseek can generate unexpected output.
Without patch:
# dd bs=4 skip=1 if=/sys/kernel/tracing/set_ftrace_pid
dd: /sys/kernel/tracing/set_ftrace_pid: cannot skip to specified offset
id
no pid
2+1 records in
2+1 records out
10 bytes copied, 0.000213285 s, 46.9 kB/s
Notice the "id" followed by "no pid".
With the patch:
# dd bs=4 skip=1 if=/sys/kernel/tracing/set_ftrace_pid
dd: /sys/kernel/tracing/set_ftrace_pid: cannot skip to specified offset
id
0+1 records in
0+1 records out
3 bytes copied, 0.000202112 s, 14.8 kB/s
Notice that it only prints "id" and not the "no pid" afterward.
Link: http://lkml.kernel.org/r/4f87c6ad-f114-30bb-8506-c32274ce2992@virtuozzo.com
https://bugzilla.kernel.org/show_bug.cgi?id=206283
Signed-off-by: Vasily Averin <vvs@virtuozzo.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
kernel/trace/ftrace.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 3581bd96d6eb3..ddb47a0af854b 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -7038,9 +7038,10 @@ static void *fpid_next(struct seq_file *m, void *v, loff_t *pos)
struct trace_array *tr = m->private;
struct trace_pid_list *pid_list = rcu_dereference_sched(tr->function_pids);
- if (v == FTRACE_NO_PIDS)
+ if (v == FTRACE_NO_PIDS) {
+ (*pos)++;
return NULL;
-
+ }
return trace_pid_next(pid_list, v, pos);
}
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 5.5 502/542] powerpc: Do not consider weak unresolved symbol relocations as bad
From: Sasha Levin @ 2020-02-14 15:48 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Alexandre Ghiti, Stephen Rothwell, Michael Ellerman, Sasha Levin,
linuxppc-dev, netdev, bpf
In-Reply-To: <20200214154854.6746-1-sashal@kernel.org>
From: Alexandre Ghiti <alex@ghiti.fr>
[ Upstream commit 43e76cd368fbb67e767da5363ffeaa3989993c8c ]
Commit 8580ac9404f6 ("bpf: Process in-kernel BTF") introduced two weak
symbols that may be unresolved at link time which result in an absolute
relocation to 0. relocs_check.sh emits the following warning:
"WARNING: 2 bad relocations
c000000001a41478 R_PPC64_ADDR64 _binary__btf_vmlinux_bin_start
c000000001a41480 R_PPC64_ADDR64 _binary__btf_vmlinux_bin_end"
whereas those relocations are legitimate even for a relocatable kernel
compiled with -pie option.
relocs_check.sh already excluded some weak unresolved symbols explicitly:
remove those hardcoded symbols and add some logic that parses the symbols
using nm, retrieves all the weak unresolved symbols and excludes those from
the list of the potential bad relocations.
Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Signed-off-by: Alexandre Ghiti <alex@ghiti.fr>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/20200118170335.21440-1-alex@ghiti.fr
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/powerpc/Makefile.postlink | 4 ++--
arch/powerpc/tools/relocs_check.sh | 20 ++++++++++++--------
2 files changed, 14 insertions(+), 10 deletions(-)
diff --git a/arch/powerpc/Makefile.postlink b/arch/powerpc/Makefile.postlink
index 134f12f89b92b..2268396ff4bba 100644
--- a/arch/powerpc/Makefile.postlink
+++ b/arch/powerpc/Makefile.postlink
@@ -17,11 +17,11 @@ quiet_cmd_head_check = CHKHEAD $@
quiet_cmd_relocs_check = CHKREL $@
ifdef CONFIG_PPC_BOOK3S_64
cmd_relocs_check = \
- $(CONFIG_SHELL) $(srctree)/arch/powerpc/tools/relocs_check.sh "$(OBJDUMP)" "$@" ; \
+ $(CONFIG_SHELL) $(srctree)/arch/powerpc/tools/relocs_check.sh "$(OBJDUMP)" "$(NM)" "$@" ; \
$(BASH) $(srctree)/arch/powerpc/tools/unrel_branch_check.sh "$(OBJDUMP)" "$@"
else
cmd_relocs_check = \
- $(CONFIG_SHELL) $(srctree)/arch/powerpc/tools/relocs_check.sh "$(OBJDUMP)" "$@"
+ $(CONFIG_SHELL) $(srctree)/arch/powerpc/tools/relocs_check.sh "$(OBJDUMP)" "$(NM)" "$@"
endif
# `@true` prevents complaint when there is nothing to be done
diff --git a/arch/powerpc/tools/relocs_check.sh b/arch/powerpc/tools/relocs_check.sh
index 7b9fe0a567cf3..014e00e74d2b6 100755
--- a/arch/powerpc/tools/relocs_check.sh
+++ b/arch/powerpc/tools/relocs_check.sh
@@ -10,14 +10,21 @@
# based on relocs_check.pl
# Copyright © 2009 IBM Corporation
-if [ $# -lt 2 ]; then
- echo "$0 [path to objdump] [path to vmlinux]" 1>&2
+if [ $# -lt 3 ]; then
+ echo "$0 [path to objdump] [path to nm] [path to vmlinux]" 1>&2
exit 1
fi
-# Have Kbuild supply the path to objdump so we handle cross compilation.
+# Have Kbuild supply the path to objdump and nm so we handle cross compilation.
objdump="$1"
-vmlinux="$2"
+nm="$2"
+vmlinux="$3"
+
+# Remove from the bad relocations those that match an undefined weak symbol
+# which will result in an absolute relocation to 0.
+# Weak unresolved symbols are of that form in nm output:
+# " w _binary__btf_vmlinux_bin_end"
+undef_weak_symbols=$($nm "$vmlinux" | awk '$1 ~ /w/ { print $2 }')
bad_relocs=$(
$objdump -R "$vmlinux" |
@@ -26,8 +33,6 @@ $objdump -R "$vmlinux" |
# These relocations are okay
# On PPC64:
# R_PPC64_RELATIVE, R_PPC64_NONE
- # R_PPC64_ADDR64 mach_<name>
- # R_PPC64_ADDR64 __crc_<name>
# On PPC:
# R_PPC_RELATIVE, R_PPC_ADDR16_HI,
# R_PPC_ADDR16_HA,R_PPC_ADDR16_LO,
@@ -39,8 +44,7 @@ R_PPC_ADDR16_HI
R_PPC_ADDR16_HA
R_PPC_RELATIVE
R_PPC_NONE' |
- grep -E -v '\<R_PPC64_ADDR64[[:space:]]+mach_' |
- grep -E -v '\<R_PPC64_ADDR64[[:space:]]+__crc_'
+ ([ "$undef_weak_symbols" ] && grep -F -w -v "$undef_weak_symbols" || cat)
)
if [ -z "$bad_relocs" ]; then
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 5.5 503/542] btrfs: do not do delalloc reservation under page lock
From: Sasha Levin @ 2020-02-14 15:48 UTC (permalink / raw)
To: linux-kernel, stable; +Cc: Josef Bacik, David Sterba, Sasha Levin, linux-btrfs
In-Reply-To: <20200214154854.6746-1-sashal@kernel.org>
From: Josef Bacik <josef@toxicpanda.com>
[ Upstream commit f4b1363cae43fef7c86c993b7ca7fe7d546b3c68 ]
We ran into a deadlock in production with the fixup worker. The stack
traces were as follows:
Thread responsible for the writeout, waiting on the page lock
[<0>] io_schedule+0x12/0x40
[<0>] __lock_page+0x109/0x1e0
[<0>] extent_write_cache_pages+0x206/0x360
[<0>] extent_writepages+0x40/0x60
[<0>] do_writepages+0x31/0xb0
[<0>] __writeback_single_inode+0x3d/0x350
[<0>] writeback_sb_inodes+0x19d/0x3c0
[<0>] __writeback_inodes_wb+0x5d/0xb0
[<0>] wb_writeback+0x231/0x2c0
[<0>] wb_workfn+0x308/0x3c0
[<0>] process_one_work+0x1e0/0x390
[<0>] worker_thread+0x2b/0x3c0
[<0>] kthread+0x113/0x130
[<0>] ret_from_fork+0x35/0x40
[<0>] 0xffffffffffffffff
Thread of the fixup worker who is holding the page lock
[<0>] start_delalloc_inodes+0x241/0x2d0
[<0>] btrfs_start_delalloc_roots+0x179/0x230
[<0>] btrfs_alloc_data_chunk_ondemand+0x11b/0x2e0
[<0>] btrfs_check_data_free_space+0x53/0xa0
[<0>] btrfs_delalloc_reserve_space+0x20/0x70
[<0>] btrfs_writepage_fixup_worker+0x1fc/0x2a0
[<0>] normal_work_helper+0x11c/0x360
[<0>] process_one_work+0x1e0/0x390
[<0>] worker_thread+0x2b/0x3c0
[<0>] kthread+0x113/0x130
[<0>] ret_from_fork+0x35/0x40
[<0>] 0xffffffffffffffff
Thankfully the stars have to align just right to hit this. First you
have to end up in the fixup worker, which is tricky by itself (my
reproducer does DIO reads into a MMAP'ed region, so not a common
operation). Then you have to have less than a page size of free data
space and 0 unallocated space so you go down the "commit the transaction
to free up pinned space" path. This was accomplished by a random
balance that was running on the host. Then you get this deadlock.
I'm still in the process of trying to force the deadlock to happen on
demand, but I've hit other issues. I can still trigger the fixup worker
path itself so this patch has been tested in that regard, so the normal
case is fine.
Fixes: 87826df0ec36 ("btrfs: delalloc for page dirtied out-of-band in fixup worker")
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
fs/btrfs/inode.c | 76 ++++++++++++++++++++++++++++++++++++++----------
1 file changed, 60 insertions(+), 16 deletions(-)
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 27f2c554cac32..537b4c563f09c 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -2191,6 +2191,7 @@ int btrfs_set_extent_delalloc(struct inode *inode, u64 start, u64 end,
/* see btrfs_writepage_start_hook for details on why this is required */
struct btrfs_writepage_fixup {
struct page *page;
+ struct inode *inode;
struct btrfs_work work;
};
@@ -2205,9 +2206,20 @@ static void btrfs_writepage_fixup_worker(struct btrfs_work *work)
u64 page_start;
u64 page_end;
int ret = 0;
+ bool free_delalloc_space = true;
fixup = container_of(work, struct btrfs_writepage_fixup, work);
page = fixup->page;
+ inode = fixup->inode;
+ page_start = page_offset(page);
+ page_end = page_offset(page) + PAGE_SIZE - 1;
+
+ /*
+ * This is similar to page_mkwrite, we need to reserve the space before
+ * we take the page lock.
+ */
+ ret = btrfs_delalloc_reserve_space(inode, &data_reserved, page_start,
+ PAGE_SIZE);
again:
lock_page(page);
@@ -2216,25 +2228,48 @@ static void btrfs_writepage_fixup_worker(struct btrfs_work *work)
* page->mapping may go NULL, but it shouldn't be moved to a different
* address space.
*/
- if (!page->mapping || !PageDirty(page) || !PageChecked(page))
+ if (!page->mapping || !PageDirty(page) || !PageChecked(page)) {
+ /*
+ * Unfortunately this is a little tricky, either
+ *
+ * 1) We got here and our page had already been dealt with and
+ * we reserved our space, thus ret == 0, so we need to just
+ * drop our space reservation and bail. This can happen the
+ * first time we come into the fixup worker, or could happen
+ * while waiting for the ordered extent.
+ * 2) Our page was already dealt with, but we happened to get an
+ * ENOSPC above from the btrfs_delalloc_reserve_space. In
+ * this case we obviously don't have anything to release, but
+ * because the page was already dealt with we don't want to
+ * mark the page with an error, so make sure we're resetting
+ * ret to 0. This is why we have this check _before_ the ret
+ * check, because we do not want to have a surprise ENOSPC
+ * when the page was already properly dealt with.
+ */
+ if (!ret) {
+ btrfs_delalloc_release_extents(BTRFS_I(inode),
+ PAGE_SIZE);
+ btrfs_delalloc_release_space(inode, data_reserved,
+ page_start, PAGE_SIZE,
+ true);
+ }
+ ret = 0;
goto out_page;
+ }
/*
- * We keep the PageChecked() bit set until we're done with the
- * btrfs_start_ordered_extent() dance that we do below. That drops and
- * retakes the page lock, so we don't want new fixup workers queued for
- * this page during the churn.
+ * We can't mess with the page state unless it is locked, so now that
+ * it is locked bail if we failed to make our space reservation.
*/
- inode = page->mapping->host;
- page_start = page_offset(page);
- page_end = page_offset(page) + PAGE_SIZE - 1;
+ if (ret)
+ goto out_page;
lock_extent_bits(&BTRFS_I(inode)->io_tree, page_start, page_end,
&cached_state);
/* already ordered? We're done */
if (PagePrivate2(page))
- goto out;
+ goto out_reserved;
ordered = btrfs_lookup_ordered_range(BTRFS_I(inode), page_start,
PAGE_SIZE);
@@ -2247,11 +2282,6 @@ static void btrfs_writepage_fixup_worker(struct btrfs_work *work)
goto again;
}
- ret = btrfs_delalloc_reserve_space(inode, &data_reserved, page_start,
- PAGE_SIZE);
- if (ret)
- goto out;
-
ret = btrfs_set_extent_delalloc(inode, page_start, page_end, 0,
&cached_state);
if (ret)
@@ -2265,12 +2295,12 @@ static void btrfs_writepage_fixup_worker(struct btrfs_work *work)
* The page was dirty when we started, nothing should have cleaned it.
*/
BUG_ON(!PageDirty(page));
+ free_delalloc_space = false;
out_reserved:
btrfs_delalloc_release_extents(BTRFS_I(inode), PAGE_SIZE);
- if (ret)
+ if (free_delalloc_space)
btrfs_delalloc_release_space(inode, data_reserved, page_start,
PAGE_SIZE, true);
-out:
unlock_extent_cached(&BTRFS_I(inode)->io_tree, page_start, page_end,
&cached_state);
out_page:
@@ -2289,6 +2319,12 @@ static void btrfs_writepage_fixup_worker(struct btrfs_work *work)
put_page(page);
kfree(fixup);
extent_changeset_free(data_reserved);
+ /*
+ * As a precaution, do a delayed iput in case it would be the last iput
+ * that could need flushing space. Recursing back to fixup worker would
+ * deadlock.
+ */
+ btrfs_add_delayed_iput(inode);
}
/*
@@ -2326,10 +2362,18 @@ int btrfs_writepage_cow_fixup(struct page *page, u64 start, u64 end)
if (!fixup)
return -EAGAIN;
+ /*
+ * We are already holding a reference to this inode from
+ * write_cache_pages. We need to hold it because the space reservation
+ * takes place outside of the page lock, and we can't trust
+ * page->mapping outside of the page lock.
+ */
+ ihold(inode);
SetPageChecked(page);
get_page(page);
btrfs_init_work(&fixup->work, btrfs_writepage_fixup_worker, NULL, NULL);
fixup->page = page;
+ fixup->inode = inode;
btrfs_queue_work(fs_info->fixup_workers, &fixup->work);
return -EAGAIN;
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 5.5 507/542] lib/scatterlist.c: adjust indentation in __sg_alloc_table
From: Sasha Levin @ 2020-02-14 15:48 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Nathan Chancellor, Andrew Morton, Linus Torvalds, Sasha Levin,
clang-built-linux
In-Reply-To: <20200214154854.6746-1-sashal@kernel.org>
From: Nathan Chancellor <natechancellor@gmail.com>
[ Upstream commit 4e456fee215677584cafa7f67298a76917e89c64 ]
Clang warns:
../lib/scatterlist.c:314:5: warning: misleading indentation; statement
is not part of the previous 'if' [-Wmisleading-indentation]
return -ENOMEM;
^
../lib/scatterlist.c:311:4: note: previous statement is here
if (prv)
^
1 warning generated.
This warning occurs because there is a space before the tab on this
line. Remove it so that the indentation is consistent with the Linux
kernel coding style and clang no longer warns.
Link: http://lkml.kernel.org/r/20191218033606.11942-1-natechancellor@gmail.com
Link: https://github.com/ClangBuiltLinux/linux/issues/830
Fixes: edce6820a9fd ("scatterlist: prevent invalid free when alloc fails")
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
lib/scatterlist.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/scatterlist.c b/lib/scatterlist.c
index c2cf2c311b7db..5813072bc5895 100644
--- a/lib/scatterlist.c
+++ b/lib/scatterlist.c
@@ -311,7 +311,7 @@ int __sg_alloc_table(struct sg_table *table, unsigned int nents,
if (prv)
table->nents = ++table->orig_nents;
- return -ENOMEM;
+ return -ENOMEM;
}
sg_init_table(sg, alloc_size);
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 5.5 512/542] irqchip/gic-v3-its: Reference to its_invall_cmd descriptor when building INVALL
From: Sasha Levin @ 2020-02-14 15:48 UTC (permalink / raw)
To: linux-kernel, stable; +Cc: Zenghui Yu, Marc Zyngier, Sasha Levin
In-Reply-To: <20200214154854.6746-1-sashal@kernel.org>
From: Zenghui Yu <yuzenghui@huawei.com>
[ Upstream commit 107945227ac5d4c37911c7841b27c64b489ce9a9 ]
It looks like an obvious mistake to use its_mapc_cmd descriptor when
building the INVALL command block. It so far worked by luck because
both its_mapc_cmd.col and its_invall_cmd.col sit at the same offset of
the ITS command descriptor, but we should not rely on it.
Fixes: cc2d3216f53c ("irqchip: GICv3: ITS command queue")
Signed-off-by: Zenghui Yu <yuzenghui@huawei.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
Link: https://lore.kernel.org/r/20191202071021.1251-1-yuzenghui@huawei.com
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/irqchip/irq-gic-v3-its.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index b704214390c0f..50f89056c16bb 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -598,7 +598,7 @@ static struct its_collection *its_build_invall_cmd(struct its_node *its,
struct its_cmd_desc *desc)
{
its_encode_cmd(cmd, GITS_CMD_INVALL);
- its_encode_collection(cmd, desc->its_mapc_cmd.col->col_id);
+ its_encode_collection(cmd, desc->its_invall_cmd.col->col_id);
its_fixup_cmd(cmd);
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 5.5 514/542] nvmet: fix dsm failure when payload does not match sgl descriptor
From: Sasha Levin @ 2020-02-14 15:48 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Sagi Grimberg, Dakshaja Uppalapati, Christoph Hellwig,
Max Gurtovoy, Keith Busch, Sasha Levin, linux-nvme
In-Reply-To: <20200214154854.6746-1-sashal@kernel.org>
From: Sagi Grimberg <sagi@grimberg.me>
[ Upstream commit b716e6889c95f64ba32af492461f6cc9341f3f05 ]
The host is allowed to pass the controller an sgl describing a buffer
that is larger than the dsm payload itself, allow it when executing
dsm.
Reported-by: Dakshaja Uppalapati <dakshaja@chelsio.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>,
Reviewed-by: Max Gurtovoy <maxg@mellanox.com>
Signed-off-by: Sagi Grimberg <sagi@grimberg.me>
Signed-off-by: Keith Busch <kbusch@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/nvme/target/core.c | 11 +++++++++++
drivers/nvme/target/io-cmd-bdev.c | 2 +-
drivers/nvme/target/io-cmd-file.c | 2 +-
drivers/nvme/target/nvmet.h | 1 +
4 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
index 35810a0a8d212..461987f669c50 100644
--- a/drivers/nvme/target/core.c
+++ b/drivers/nvme/target/core.c
@@ -939,6 +939,17 @@ bool nvmet_check_data_len(struct nvmet_req *req, size_t data_len)
}
EXPORT_SYMBOL_GPL(nvmet_check_data_len);
+bool nvmet_check_data_len_lte(struct nvmet_req *req, size_t data_len)
+{
+ if (unlikely(data_len > req->transfer_len)) {
+ req->error_loc = offsetof(struct nvme_common_command, dptr);
+ nvmet_req_complete(req, NVME_SC_SGL_INVALID_DATA | NVME_SC_DNR);
+ return false;
+ }
+
+ return true;
+}
+
int nvmet_req_alloc_sgl(struct nvmet_req *req)
{
struct pci_dev *p2p_dev = NULL;
diff --git a/drivers/nvme/target/io-cmd-bdev.c b/drivers/nvme/target/io-cmd-bdev.c
index b6fca0e421ef1..ea0e596be15dc 100644
--- a/drivers/nvme/target/io-cmd-bdev.c
+++ b/drivers/nvme/target/io-cmd-bdev.c
@@ -280,7 +280,7 @@ static void nvmet_bdev_execute_discard(struct nvmet_req *req)
static void nvmet_bdev_execute_dsm(struct nvmet_req *req)
{
- if (!nvmet_check_data_len(req, nvmet_dsm_len(req)))
+ if (!nvmet_check_data_len_lte(req, nvmet_dsm_len(req)))
return;
switch (le32_to_cpu(req->cmd->dsm.attributes)) {
diff --git a/drivers/nvme/target/io-cmd-file.c b/drivers/nvme/target/io-cmd-file.c
index caebfce066056..cd5670b83118f 100644
--- a/drivers/nvme/target/io-cmd-file.c
+++ b/drivers/nvme/target/io-cmd-file.c
@@ -336,7 +336,7 @@ static void nvmet_file_dsm_work(struct work_struct *w)
static void nvmet_file_execute_dsm(struct nvmet_req *req)
{
- if (!nvmet_check_data_len(req, nvmet_dsm_len(req)))
+ if (!nvmet_check_data_len_lte(req, nvmet_dsm_len(req)))
return;
INIT_WORK(&req->f.work, nvmet_file_dsm_work);
schedule_work(&req->f.work);
diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
index 46df45e837c95..eda28b22a2c87 100644
--- a/drivers/nvme/target/nvmet.h
+++ b/drivers/nvme/target/nvmet.h
@@ -374,6 +374,7 @@ bool nvmet_req_init(struct nvmet_req *req, struct nvmet_cq *cq,
struct nvmet_sq *sq, const struct nvmet_fabrics_ops *ops);
void nvmet_req_uninit(struct nvmet_req *req);
bool nvmet_check_data_len(struct nvmet_req *req, size_t data_len);
+bool nvmet_check_data_len_lte(struct nvmet_req *req, size_t data_len);
void nvmet_req_complete(struct nvmet_req *req, u16 status);
int nvmet_req_alloc_sgl(struct nvmet_req *req);
void nvmet_req_free_sgl(struct nvmet_req *req);
--
2.20.1
^ permalink raw reply related
* Re: [PATCH v7 bpf-next RESEND 2/2] selftests/bpf: add bpf_read_branch_records() selftest
From: Andrii Nakryiko @ 2020-02-14 17:47 UTC (permalink / raw)
To: Daniel Xu
Cc: bpf, Alexei Starovoitov, Daniel Borkmann, Song Liu, Yonghong Song,
Andrii Nakryiko, open list, Kernel Team, Peter Ziljstra,
Ingo Molnar, Arnaldo Carvalho de Melo
In-Reply-To: <C0LP269G4WO4.1Q4M8CK4K92SU@dlxu-fedora-R90QNFJV>
On Thu, Feb 13, 2020 at 11:05 PM Daniel Xu <dxu@dxuuu.xyz> wrote:
>
> On Tue Feb 11, 2020 at 11:30 AM, Andrii Nakryiko wrote:
> > On Mon, Feb 10, 2020 at 12:09 PM Daniel Xu <dxu@dxuuu.xyz> wrote:
> [...]
>
> >
> > > + /* generate some branches on cpu 0 */
> > > + CPU_ZERO(&cpu_set);
> > > + CPU_SET(0, &cpu_set);
> > > + err = pthread_setaffinity_np(pthread_self(), sizeof(cpu_set), &cpu_set);
> > > + if (CHECK(err, "set_affinity", "cpu #0, err %d\n", err))
> > > + goto out_free_pb;
> > > + /* spin the loop for a while (random high number) */
> > > + for (i = 0; i < 1000000; ++i)
> > > + ++j;
> > > +
> >
> >
> > test_perf_branches__detach here?
>
> Yeah, good idea.
>
> [...]
>
> > > +struct fake_perf_branch_entry {
> > > + __u64 _a;
> > > + __u64 _b;
> > > + __u64 _c;
> > > +};
> > > +
> > > +struct output {
> > > + int required_size;
> > > + int written_stack;
> > > + int written_map;
> > > +};
> > > +
> > > +struct {
> > > + __uint(type, BPF_MAP_TYPE_PERF_EVENT_ARRAY);
> > > + __uint(key_size, sizeof(int));
> > > + __uint(value_size, sizeof(int));
> > > +} perf_buf_map SEC(".maps");
> > > +
> > > +typedef struct fake_perf_branch_entry fpbe_t[30];
> > > +
> > > +struct {
> > > + __uint(type, BPF_MAP_TYPE_ARRAY);
> > > + __uint(max_entries, 1);
> > > + __type(key, __u32);
> > > + __type(value, fpbe_t);
> > > +} scratch_map SEC(".maps");
> >
> >
> > Can you please use global variables instead of array
>
> If you mean `scratch_map`, sure.
>
> > and perf_event_array?
>
> Do you mean replace the perf ring buffer with global variables? I think
> that would limit the number of samples we validate in userspace to a fixed
> number. It might be better to validate as many as the system gives us.
>
> Let me know what you think. I might be overthinking it.
Yeah, I meant get rid of perf_buffer and just use global variables for
outputting data.
re: validating multiple samples in perf_buffer. Given you don't really
control how many samples you'll get and you check nothing specific
about any single sample, I think it's fine to just validate any. They
are not supposed to differ much, right? Checking that size is multiple
of perf_branch_entry size is pretty much the only thing we can check,
no?
>
> > Would make BPF side clearer and userspace simpler.
> > struct output member will just become variables.
>
>
> Thanks,
> Daniel
^ permalink raw reply
* [PATCH AUTOSEL 5.5 515/542] nvme-pci: remove nvmeq->tags
From: Sasha Levin @ 2020-02-14 15:48 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Christoph Hellwig, Edmund Nadolski, Keith Busch, Sasha Levin,
linux-nvme
In-Reply-To: <20200214154854.6746-1-sashal@kernel.org>
From: Christoph Hellwig <hch@lst.de>
[ Upstream commit cfa27356f835dc7755192e7b941d4f4851acbcc7 ]
There is no real need to have a pointer to the tagset in
struct nvme_queue, as we only need it in a single place, and that place
can derive the used tagset from the device and qid trivially. This
fixes a problem with stale pointer exposure when tagsets are reset,
and also shrinks the nvme_queue structure. It also matches what most
other transports have done since day 1.
Reported-by: Edmund Nadolski <edmund.nadolski@intel.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Keith Busch <kbusch@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/nvme/host/pci.c | 23 ++++++++---------------
1 file changed, 8 insertions(+), 15 deletions(-)
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 365a2ddbeaa76..da392b50f73e7 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -167,7 +167,6 @@ struct nvme_queue {
/* only used for poll queues: */
spinlock_t cq_poll_lock ____cacheline_aligned_in_smp;
volatile struct nvme_completion *cqes;
- struct blk_mq_tags **tags;
dma_addr_t sq_dma_addr;
dma_addr_t cq_dma_addr;
u32 __iomem *q_db;
@@ -376,29 +375,17 @@ static int nvme_admin_init_hctx(struct blk_mq_hw_ctx *hctx, void *data,
WARN_ON(hctx_idx != 0);
WARN_ON(dev->admin_tagset.tags[0] != hctx->tags);
- WARN_ON(nvmeq->tags);
hctx->driver_data = nvmeq;
- nvmeq->tags = &dev->admin_tagset.tags[0];
return 0;
}
-static void nvme_admin_exit_hctx(struct blk_mq_hw_ctx *hctx, unsigned int hctx_idx)
-{
- struct nvme_queue *nvmeq = hctx->driver_data;
-
- nvmeq->tags = NULL;
-}
-
static int nvme_init_hctx(struct blk_mq_hw_ctx *hctx, void *data,
unsigned int hctx_idx)
{
struct nvme_dev *dev = data;
struct nvme_queue *nvmeq = &dev->queues[hctx_idx + 1];
- if (!nvmeq->tags)
- nvmeq->tags = &dev->tagset.tags[hctx_idx];
-
WARN_ON(dev->tagset.tags[hctx_idx] != hctx->tags);
hctx->driver_data = nvmeq;
return 0;
@@ -948,6 +935,13 @@ static inline void nvme_ring_cq_doorbell(struct nvme_queue *nvmeq)
writel(head, nvmeq->q_db + nvmeq->dev->db_stride);
}
+static inline struct blk_mq_tags *nvme_queue_tagset(struct nvme_queue *nvmeq)
+{
+ if (!nvmeq->qid)
+ return nvmeq->dev->admin_tagset.tags[0];
+ return nvmeq->dev->tagset.tags[nvmeq->qid - 1];
+}
+
static inline void nvme_handle_cqe(struct nvme_queue *nvmeq, u16 idx)
{
volatile struct nvme_completion *cqe = &nvmeq->cqes[idx];
@@ -972,7 +966,7 @@ static inline void nvme_handle_cqe(struct nvme_queue *nvmeq, u16 idx)
return;
}
- req = blk_mq_tag_to_rq(*nvmeq->tags, cqe->command_id);
+ req = blk_mq_tag_to_rq(nvme_queue_tagset(nvmeq), cqe->command_id);
trace_nvme_sq(req, cqe->sq_head, nvmeq->sq_tail);
nvme_end_request(req, cqe->status, cqe->result);
}
@@ -1572,7 +1566,6 @@ static const struct blk_mq_ops nvme_mq_admin_ops = {
.queue_rq = nvme_queue_rq,
.complete = nvme_pci_complete_rq,
.init_hctx = nvme_admin_init_hctx,
- .exit_hctx = nvme_admin_exit_hctx,
.init_request = nvme_init_request,
.timeout = nvme_timeout,
};
--
2.20.1
^ permalink raw reply related
* Re: [Xen-devel] [OSSTEST PATCH V2] build: fix configuration of libvirt
From: Ian Jackson @ 2020-02-14 17:47 UTC (permalink / raw)
To: Jim Fehlig; +Cc: xen-devel@lists.xenproject.org
In-Reply-To: <20200214155933.7048-1-jfehlig@suse.com>
Jim Fehlig writes ("[OSSTEST PATCH V2] build: fix configuration of libvirt"):
> libvirt.git commit 2621d48f00 removed the last traces of gnulib, which
> also removed the '--no-git' option from autogen.sh. Unknown options are
> now passed to the configure script, which quickly fails with
>
> configure: error: unrecognized option: `--no-git'
>
> Remove the gnulib handling from ts-libvirt-build, including the '--no-git'
> option to autogen.sh. While at it remove configure options no longer
> supported by the libvirt configure script.
Harmf. Thanks for looking into this and trying to fix this mess.
I think there is a problem with your patch, which is that 2621d48f00
is recent enough that we might want still to be able to build with
earlier versions.
Is there an easy way to tell (by looking at the tree after checkout,
maybe) whether to do the old or the new thing ?
Your perl code looks good to me for what it is trying to do.
Thanks,
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply
* [PATCH AUTOSEL 5.5 516/542] iwlwifi: mvm: Fix thermal zone registration
From: Sasha Levin @ 2020-02-14 15:48 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Andrei Otcheretianski, Luca Coelho, Kalle Valo, Sasha Levin,
linux-wireless, netdev
In-Reply-To: <20200214154854.6746-1-sashal@kernel.org>
From: Andrei Otcheretianski <andrei.otcheretianski@intel.com>
[ Upstream commit baa6cf8450b72dcab11f37c47efce7c5b9b8ad0f ]
Use a unique name when registering a thermal zone. Otherwise, with
multiple NICS, we hit the following warning during the unregistration.
WARNING: CPU: 2 PID: 3525 at fs/sysfs/group.c:255
RIP: 0010:sysfs_remove_group+0x80/0x90
Call Trace:
dpm_sysfs_remove+0x57/0x60
device_del+0x5a/0x350
? sscanf+0x4e/0x70
device_unregister+0x1a/0x60
hwmon_device_unregister+0x4a/0xa0
thermal_remove_hwmon_sysfs+0x175/0x1d0
thermal_zone_device_unregister+0x188/0x1e0
iwl_mvm_thermal_exit+0xe7/0x100 [iwlmvm]
iwl_op_mode_mvm_stop+0x27/0x180 [iwlmvm]
_iwl_op_mode_stop.isra.3+0x2b/0x50 [iwlwifi]
iwl_opmode_deregister+0x90/0xa0 [iwlwifi]
__exit_compat+0x10/0x2c7 [iwlmvm]
__x64_sys_delete_module+0x13f/0x270
do_syscall_64+0x5a/0x110
entry_SYSCALL_64_after_hwframe+0x44/0xa9
Signed-off-by: Andrei Otcheretianski <andrei.otcheretianski@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/net/wireless/intel/iwlwifi/mvm/tt.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/tt.c b/drivers/net/wireless/intel/iwlwifi/mvm/tt.c
index b5a16f00bada9..fcad25ffd811f 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/tt.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/tt.c
@@ -734,7 +734,8 @@ static struct thermal_zone_device_ops tzone_ops = {
static void iwl_mvm_thermal_zone_register(struct iwl_mvm *mvm)
{
int i;
- char name[] = "iwlwifi";
+ char name[16];
+ static atomic_t counter = ATOMIC_INIT(0);
if (!iwl_mvm_is_tt_in_fw(mvm)) {
mvm->tz_device.tzone = NULL;
@@ -744,6 +745,7 @@ static void iwl_mvm_thermal_zone_register(struct iwl_mvm *mvm)
BUILD_BUG_ON(ARRAY_SIZE(name) >= THERMAL_NAME_LENGTH);
+ sprintf(name, "iwlwifi_%u", atomic_inc_return(&counter) & 0xFF);
mvm->tz_device.tzone = thermal_zone_device_register(name,
IWL_MAX_DTS_TRIPS,
IWL_WRITABLE_TRIPS_MSK,
--
2.20.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.