Hi Doug and Jason,
Mike and Ira have identified some fixes that they would like to see land in the
RC, these are serious problems and are marked as stable.
Changes since v1:
Update Lukasz' email address and fix subject tag.
---
Ira Weiny (1):
IB/hfi1: Fix SL array bounds check
Michael J. Ruhl (3):
IB/hfi1: Invalid user input can result in crash
IB/hfi1: Fix context recovery when PBC has an UnsupportedVL
IB/hfi1: Fix destroy_qp hang after a link down
drivers/infiniband/hw/hfi1/chip.c | 6 +++-
drivers/infiniband/hw/hfi1/pio.c | 51 ++++++++++++++++++++++++++------
drivers/infiniband/hw/hfi1/pio.h | 2 +
drivers/infiniband/hw/hfi1/user_sdma.c | 2 +
drivers/infiniband/hw/hfi1/verbs.c | 8 ++++-
5 files changed, 56 insertions(+), 13 deletions(-)
--
-Denny
Currently the way that we prevent userspace from performing new modesets
on MST connectors that have just been destroyed is rather broken.
There's nothing in the actual DRM DP MST topology helpers that checks
whether or not a connector still exists, instead each DRM driver does
this on it's own, usually by returning NULL from the best_encoder
callback which in turn, causes the atomic commit to fail.
However, this is wrong in a rather subtle way. If ->best_encoder()
returns NULL, this makes ALL modesets involving the connector fail. This
includes modesets from userspace that would shut off the CRTCs being
used by the connector. Since this results in blocking any changes to a
connector's DPMS prop, it has the sideaffect of preventing legacy
modesetting users from ever disabling a CRTC that was previously enabled
for use in an MST topology. An example of this, where X tries to
change the DPMS property of an MST connector that was just detached from
the system:
[ 2908.320131] [drm:drm_helper_probe_single_connector_modes [drm_kms_helper]] [CONNECTOR:82:DP-6]
[ 2908.320148] [drm:drm_helper_probe_single_connector_modes [drm_kms_helper]] [CONNECTOR:82:DP-6] status updated from connected to disconnected
[ 2908.320166] [drm:drm_helper_probe_single_connector_modes [drm_kms_helper]] [CONNECTOR:82:DP-6] disconnected
[ 2908.320193] [drm:drm_mode_object_put.part.2 [drm]] OBJ ID: 111 (1)
[ 2908.320230] [drm:drm_sysfs_hotplug_event [drm]] generating hotplug event
...
[ 2908.638539] [drm:drm_ioctl [drm]] pid=12928, dev=0xe201, auth=1, DRM_IOCTL_MODE_SETPROPERTY
[ 2908.638546] [drm:drm_atomic_state_init [drm]] Allocated atomic state 000000007155ba49
[ 2908.638553] [drm:drm_mode_object_get [drm]] OBJ ID: 114 (1)
[ 2908.638560] [drm:drm_mode_object_get [drm]] OBJ ID: 108 (1)
[ 2908.638568] [drm:drm_atomic_get_crtc_state [drm]] Added [CRTC:41:head-0] 0000000097a6396e state to 000000007155ba49
[ 2908.638575] [drm:drm_atomic_add_affected_connectors [drm]] Adding all current connectors for [CRTC:41:head-0] to 000000007155ba49
[ 2908.638582] [drm:drm_mode_object_get [drm]] OBJ ID: 82 (3)
[ 2908.638589] [drm:drm_mode_object_get [drm]] OBJ ID: 82 (4)
[ 2908.638596] [drm:drm_atomic_get_connector_state [drm]] Added [CONNECTOR:82:DP-6] 0000000087427144 state to 000000007155ba49
[ 2908.638603] [drm:drm_atomic_check_only [drm]] checking 000000007155ba49
[ 2908.638609] [drm:drm_atomic_helper_check_modeset [drm_kms_helper]] [CRTC:41:head-0] active changed
[ 2908.638613] [drm:drm_atomic_helper_check_modeset [drm_kms_helper]] Updating routing for [CONNECTOR:82:DP-6]
[ 2908.638616] [drm:drm_atomic_helper_check_modeset [drm_kms_helper]] No suitable encoder found for [CONNECTOR:82:DP-6]
[ 2908.638623] [drm:drm_atomic_check_only [drm]] atomic driver check for 000000007155ba49 failed: -22
[ 2908.638630] [drm:drm_atomic_state_default_clear [drm]] Clearing atomic state 000000007155ba49
[ 2908.638637] [drm:drm_mode_object_put.part.2 [drm]] OBJ ID: 82 (4)
[ 2908.638643] [drm:drm_mode_object_put.part.2 [drm]] OBJ ID: 82 (3)
[ 2908.638650] [drm:drm_mode_object_put.part.2 [drm]] OBJ ID: 114 (2)
[ 2908.638656] [drm:drm_mode_object_put.part.2 [drm]] OBJ ID: 108 (2)
[ 2908.638663] [drm:__drm_atomic_state_free [drm]] Freeing atomic state 000000007155ba49
[ 2908.638669] [drm:drm_mode_object_put.part.2 [drm]] OBJ ID: 82 (2)
[ 2908.638676] [drm:drm_ioctl [drm]] pid=12928, ret = -22
While this doesn't usually result in any errors that would be obvious to
the user, it does result in us leaving display resources on. This in
turn leads to unwanted sideaffects like inactive GPUs being left on
(usually from the resulting leaked runtime PM ref).
So, provide an easier way of doing this that doesn't require breaking
->best_encoder(): add a common drm_dp_mst_connector_atomic_check()
function that DRM drivers can call in order to have CRTC enabling
commits fail automatically if the MST port driving the connector no
longer exists. We'll also be able to expand upon this later as well once
we add MST fallback retraining support.
Changes since v1:
- Use list_for_each_entry_safe in drm_dp_mst_connector_still_exists() -
Julia Lawall
Signed-off-by: Lyude Paul <lyude(a)redhat.com>
Cc: Julia Lawall <julia.lawall(a)lip6.fr>
Cc: stable(a)vger.kernel.org
---
drivers/gpu/drm/drm_dp_mst_topology.c | 76 +++++++++++++++++++++++++++
include/drm/drm_dp_mst_helper.h | 3 ++
2 files changed, 79 insertions(+)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
index 7780567aa669..58b9554711c7 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -3129,6 +3129,82 @@ static const struct drm_private_state_funcs mst_state_funcs = {
.atomic_destroy_state = drm_dp_mst_destroy_state,
};
+static bool
+drm_dp_mst_connector_still_exists(struct drm_connector *connector,
+ struct drm_dp_mst_topology_mgr *mgr,
+ struct drm_dp_mst_branch *mstb)
+{
+ struct drm_dp_mst_port *port, *tmp;
+ bool exists = false;
+
+ mstb = drm_dp_get_validated_mstb_ref(mgr, mstb);
+ if (!mstb)
+ return false;
+
+ list_for_each_entry_safe(port, tmp, &mstb->ports, next) {
+ port = drm_dp_get_validated_port_ref(mgr, port);
+ if (!port)
+ continue;
+
+ exists = (port->connector == connector ||
+ (port->mstb &&
+ drm_dp_mst_connector_still_exists(connector, mgr,
+ port->mstb)));
+
+ drm_dp_put_port(port);
+ if (exists)
+ break;
+ }
+
+ drm_dp_put_mst_branch_device(mstb);
+ return exists;
+}
+
+/**
+ * drm_dp_mst_connector_atomic_check - Helper for validating a new atomic
+ * state on an MST connector
+ * @connector: drm connector
+ * @connector_state: the new atomic state of @connector
+ * @mgr: the MST topology mgr for @connector
+ *
+ * This function performs various atomic checks that apply to all drivers
+ * using the DRM DP MST helpers. This should be called by all drivers at the
+ * start of the atomic_check function for their MST connectors.
+ *
+ * Return 0 for success, or negative error code on failure.
+ */
+int
+drm_dp_mst_connector_atomic_check(struct drm_connector *connector,
+ struct drm_connector_state *connector_state,
+ struct drm_dp_mst_topology_mgr *mgr)
+{
+ struct drm_atomic_state *state = connector_state->state;
+ struct drm_crtc *crtc = connector_state->crtc;
+ struct drm_crtc_state *new_crtc_state;
+
+ if (!crtc)
+ return 0;
+
+ new_crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
+ if (!new_crtc_state)
+ return 0;
+
+ if (!drm_atomic_crtc_needs_modeset(new_crtc_state) ||
+ !new_crtc_state->active)
+ return 0;
+
+ /* Make sure that the port for this MST connector still exists */
+ if (!drm_dp_mst_connector_still_exists(connector, mgr,
+ mgr->mst_primary)) {
+ DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] has disappeared from the MST topology\n",
+ connector->base.id, connector->name);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_dp_mst_connector_atomic_check);
+
/**
* drm_atomic_get_mst_topology_state: get MST topology state
*
diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
index 7f78d26a0766..8e33c2c85d1e 100644
--- a/include/drm/drm_dp_mst_helper.h
+++ b/include/drm/drm_dp_mst_helper.h
@@ -625,6 +625,9 @@ void drm_dp_mst_topology_mgr_suspend(struct drm_dp_mst_topology_mgr *mgr);
int drm_dp_mst_topology_mgr_resume(struct drm_dp_mst_topology_mgr *mgr);
struct drm_dp_mst_topology_state *drm_atomic_get_mst_topology_state(struct drm_atomic_state *state,
struct drm_dp_mst_topology_mgr *mgr);
+int drm_dp_mst_connector_atomic_check(struct drm_connector *connector,
+ struct drm_connector_state *connector_state,
+ struct drm_dp_mst_topology_mgr *mgr);
int drm_dp_atomic_find_vcpi_slots(struct drm_atomic_state *state,
struct drm_dp_mst_topology_mgr *mgr,
struct drm_dp_mst_port *port, int pbn);
--
2.17.1
Currently the way that we prevent userspace from performing new modesets
on MST connectors that have just been destroyed is rather broken.
There's nothing in the actual DRM DP MST topology helpers that checks
whether or not a connector still exists, instead each DRM driver does
this on it's own, usually by returning NULL from the best_encoder
callback which in turn, causes the atomic commit to fail.
However, this is wrong in a rather subtle way. If ->best_encoder()
returns NULL, this makes ALL modesets involving the connector fail. This
includes modesets from userspace that would shut off the CRTCs being
used by the connector. Since this results in blocking any changes to a
connector's DPMS prop, it has the sideaffect of preventing legacy
modesetting users from ever disabling a CRTC that was previously enabled
for use in an MST topology. An example of this, where X tries to
change the DPMS property of an MST connector that was just detached from
the system:
[ 2908.320131] [drm:drm_helper_probe_single_connector_modes [drm_kms_helper]] [CONNECTOR:82:DP-6]
[ 2908.320148] [drm:drm_helper_probe_single_connector_modes [drm_kms_helper]] [CONNECTOR:82:DP-6] status updated from connected to disconnected
[ 2908.320166] [drm:drm_helper_probe_single_connector_modes [drm_kms_helper]] [CONNECTOR:82:DP-6] disconnected
[ 2908.320193] [drm:drm_mode_object_put.part.2 [drm]] OBJ ID: 111 (1)
[ 2908.320230] [drm:drm_sysfs_hotplug_event [drm]] generating hotplug event
...
[ 2908.638539] [drm:drm_ioctl [drm]] pid=12928, dev=0xe201, auth=1, DRM_IOCTL_MODE_SETPROPERTY
[ 2908.638546] [drm:drm_atomic_state_init [drm]] Allocated atomic state 000000007155ba49
[ 2908.638553] [drm:drm_mode_object_get [drm]] OBJ ID: 114 (1)
[ 2908.638560] [drm:drm_mode_object_get [drm]] OBJ ID: 108 (1)
[ 2908.638568] [drm:drm_atomic_get_crtc_state [drm]] Added [CRTC:41:head-0] 0000000097a6396e state to 000000007155ba49
[ 2908.638575] [drm:drm_atomic_add_affected_connectors [drm]] Adding all current connectors for [CRTC:41:head-0] to 000000007155ba49
[ 2908.638582] [drm:drm_mode_object_get [drm]] OBJ ID: 82 (3)
[ 2908.638589] [drm:drm_mode_object_get [drm]] OBJ ID: 82 (4)
[ 2908.638596] [drm:drm_atomic_get_connector_state [drm]] Added [CONNECTOR:82:DP-6] 0000000087427144 state to 000000007155ba49
[ 2908.638603] [drm:drm_atomic_check_only [drm]] checking 000000007155ba49
[ 2908.638609] [drm:drm_atomic_helper_check_modeset [drm_kms_helper]] [CRTC:41:head-0] active changed
[ 2908.638613] [drm:drm_atomic_helper_check_modeset [drm_kms_helper]] Updating routing for [CONNECTOR:82:DP-6]
[ 2908.638616] [drm:drm_atomic_helper_check_modeset [drm_kms_helper]] No suitable encoder found for [CONNECTOR:82:DP-6]
[ 2908.638623] [drm:drm_atomic_check_only [drm]] atomic driver check for 000000007155ba49 failed: -22
[ 2908.638630] [drm:drm_atomic_state_default_clear [drm]] Clearing atomic state 000000007155ba49
[ 2908.638637] [drm:drm_mode_object_put.part.2 [drm]] OBJ ID: 82 (4)
[ 2908.638643] [drm:drm_mode_object_put.part.2 [drm]] OBJ ID: 82 (3)
[ 2908.638650] [drm:drm_mode_object_put.part.2 [drm]] OBJ ID: 114 (2)
[ 2908.638656] [drm:drm_mode_object_put.part.2 [drm]] OBJ ID: 108 (2)
[ 2908.638663] [drm:__drm_atomic_state_free [drm]] Freeing atomic state 000000007155ba49
[ 2908.638669] [drm:drm_mode_object_put.part.2 [drm]] OBJ ID: 82 (2)
[ 2908.638676] [drm:drm_ioctl [drm]] pid=12928, ret = -22
While this doesn't usually result in any errors that would be obvious to
the user, it does result in us leaving display resources on. This in
turn leads to unwanted sideaffects like inactive GPUs being left on
(usually from the resulting leaked runtime PM ref).
So, provide an easier way of doing this that doesn't require breaking
->best_encoder(): add a common drm_dp_mst_connector_atomic_check()
function that DRM drivers can call in order to have CRTC enabling
commits fail automatically if the MST port driving the connector no
longer exists. We'll also be able to expand upon this later as well once
we add MST fallback retraining support.
Signed-off-by: Lyude Paul <lyude(a)redhat.com>
Cc: stable(a)vger.kernel.org
---
drivers/gpu/drm/drm_dp_mst_topology.c | 76 +++++++++++++++++++++++++++
include/drm/drm_dp_mst_helper.h | 3 ++
2 files changed, 79 insertions(+)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
index 7780567aa669..0162d4bf2549 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -3129,6 +3129,82 @@ static const struct drm_private_state_funcs mst_state_funcs = {
.atomic_destroy_state = drm_dp_mst_destroy_state,
};
+static bool
+drm_dp_mst_connector_still_exists(struct drm_connector *connector,
+ struct drm_dp_mst_topology_mgr *mgr,
+ struct drm_dp_mst_branch *mstb)
+{
+ struct drm_dp_mst_port *port;
+ bool exists = false;
+
+ mstb = drm_dp_get_validated_mstb_ref(mgr, mstb);
+ if (!mstb)
+ return false;
+
+ list_for_each_entry(port, &mstb->ports, next) {
+ port = drm_dp_get_validated_port_ref(mgr, port);
+ if (!port)
+ continue;
+
+ exists = (port->connector == connector ||
+ (port->mstb &&
+ drm_dp_mst_connector_still_exists(connector, mgr,
+ port->mstb)));
+
+ drm_dp_put_port(port);
+ if (exists)
+ break;
+ }
+
+ drm_dp_put_mst_branch_device(mstb);
+ return exists;
+}
+
+/**
+ * drm_dp_mst_connector_atomic_check - Helper for validating a new atomic
+ * state on an MST connector
+ * @connector: drm connector
+ * @connector_state: the new atomic state of @connector
+ * @mgr: the MST topology mgr for @connector
+ *
+ * This function performs various atomic checks that apply to all drivers
+ * using the DRM DP MST helpers. This should be called by all drivers at the
+ * start of the atomic_check function for their MST connectors.
+ *
+ * Return 0 for success, or negative error code on failure.
+ */
+int
+drm_dp_mst_connector_atomic_check(struct drm_connector *connector,
+ struct drm_connector_state *connector_state,
+ struct drm_dp_mst_topology_mgr *mgr)
+{
+ struct drm_atomic_state *state = connector_state->state;
+ struct drm_crtc *crtc = connector_state->crtc;
+ struct drm_crtc_state *new_crtc_state;
+
+ if (!crtc)
+ return 0;
+
+ new_crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
+ if (!new_crtc_state)
+ return 0;
+
+ if (!drm_atomic_crtc_needs_modeset(new_crtc_state) ||
+ !new_crtc_state->active)
+ return 0;
+
+ /* Make sure that the port for this MST connector still exists */
+ if (!drm_dp_mst_connector_still_exists(connector, mgr,
+ mgr->mst_primary)) {
+ DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] has disappeared from the MST topology\n",
+ connector->base.id, connector->name);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_dp_mst_connector_atomic_check);
+
/**
* drm_atomic_get_mst_topology_state: get MST topology state
*
diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
index 7f78d26a0766..8e33c2c85d1e 100644
--- a/include/drm/drm_dp_mst_helper.h
+++ b/include/drm/drm_dp_mst_helper.h
@@ -625,6 +625,9 @@ void drm_dp_mst_topology_mgr_suspend(struct drm_dp_mst_topology_mgr *mgr);
int drm_dp_mst_topology_mgr_resume(struct drm_dp_mst_topology_mgr *mgr);
struct drm_dp_mst_topology_state *drm_atomic_get_mst_topology_state(struct drm_atomic_state *state,
struct drm_dp_mst_topology_mgr *mgr);
+int drm_dp_mst_connector_atomic_check(struct drm_connector *connector,
+ struct drm_connector_state *connector_state,
+ struct drm_dp_mst_topology_mgr *mgr);
int drm_dp_atomic_find_vcpi_slots(struct drm_atomic_state *state,
struct drm_dp_mst_topology_mgr *mgr,
struct drm_dp_mst_port *port, int pbn);
--
2.17.1
On Thu, 20 Sep 2018, Evan Green wrote:
> On Thu, Aug 30, 2018 at 7:29 AM Thomas Gleixner <tglx(a)linutronix.de> wrote:
> >
> > On Thu, 30 Aug 2018, John Crispin wrote:
> >
> > > > Sry, that disturbing you all, but what are the conclusion here for 4.14.y?
> > > > - take Thomas's patch https://lore.kernel.org/patchwork/patch/969521/#1162900
> > > > - revert commit 2d898915ccf4838c04531c51a598469e921a5eb5
> > >
> > > Hi Frederic,
> > >
> > > I reported this very issue to tglx last night and he asked me to verify his
> > > proposed patch which i just did. I can confirm the the patch fixes the issue
> > > on 4.14.67 and Greg should add it to the stable queue please.
> > >
> > > Tested-by: John Crispin <john(a)phrozen.org>
> >
> > Let me whip up a proper patch with changelog.
> >
> Hi Thomas,
> Did this patch ever go anywhere? I believe it fixes the prints I'm
> seeing in our kernel, and I wondered if the official patch was
> somewhere.
It's out there:
https://lore.kernel.org/patchwork/patch/979451/
I assume that it has fallen through the stable cracks.
Thanks,
tglx
Hi Doug and Jason,
Mike and Ira have identified some fixes that they would like to see land in the
RC, these are serious problems and are marked as stable.
---
Ira Weiny (1):
IB/hfi1: Fix SL array bounds check
Michael J. Ruhl (3):
IB/hfi1: Invalid user input can result in crash
IB/hfi1: Fix context recovery when PBC has an UnsupportedVL
IB/hfi1: Fix destroy_qp hang after a link down
drivers/infiniband/hw/hfi1/chip.c | 6 +++-
drivers/infiniband/hw/hfi1/pio.c | 51 ++++++++++++++++++++++++++------
drivers/infiniband/hw/hfi1/pio.h | 2 +
drivers/infiniband/hw/hfi1/user_sdma.c | 2 +
drivers/infiniband/hw/hfi1/verbs.c | 8 ++++-
5 files changed, 56 insertions(+), 13 deletions(-)
--
-Denny
From: KJ Tsanaktsidis <ktsanaktsidis(a)zendesk.com>
Subject: fork: report pid exhaustion correctly
Make the clone and fork syscalls return EAGAIN when the limit on the
number of pids /proc/sys/kernel/pid_max is exceeded.
Currently, when the pid_max limit is exceeded, the kernel will return
ENOSPC from the fork and clone syscalls. This is contrary to the
documented behaviour, which explicitly calls out the pid_max case as one
where EAGAIN should be returned. It also leads to really confusing error
messages in userspace programs which will complain about a lack of disk
space when they fail to create processes/threads for this reason.
This error is being returned because alloc_pid() uses the idr api to find
a new pid; when there are none available, idr_alloc_cyclic() returns
-ENOSPC, and this is being propagated back to userspace.
This behaviour has been broken before, and was explicitly fixed in
commit 35f71bc0a09a ("fork: report pid reservation failure properly"),
so I think -EAGAIN is definitely the right thing to return in this case.
The current behaviour change dates from commit 95846ecf9dac ("pid:
replace pid bitmap implementation with IDR AIP") and was I believe
unintentional.
This patch has no impact on the case where allocating a pid fails because
the child reaper for the namespace is dead; that case will still return
-ENOMEM.
Link: http://lkml.kernel.org/r/20180903111016.46461-1-ktsanaktsidis@zendesk.com
Fixes: 95846ecf9dac ("pid: replace pid bitmap implementation with IDR AIP")
Signed-off-by: KJ Tsanaktsidis <ktsanaktsidis(a)zendesk.com>
Reviewed-by: Andrew Morton <akpm(a)linux-foundation.org>
Acked-by: Michal Hocko <mhocko(a)suse.com>
Cc: Gargi Sharma <gs051095(a)gmail.com>
Cc: Rik van Riel <riel(a)redhat.com>
Cc: Oleg Nesterov <oleg(a)redhat.com>
Cc: <stable(a)vger.kernel.org>
Signed-off-by: Andrew Morton <akpm(a)linux-foundation.org>
---
diff -puN kernel/pid.c~fork-report-pid-exhaustion-correctly kernel/pid.c
--- a/kernel/pid.c~fork-report-pid-exhaustion-correctly
+++ a/kernel/pid.c
@@ -195,7 +195,7 @@ struct pid *alloc_pid(struct pid_namespa
idr_preload_end();
if (nr < 0) {
- retval = nr;
+ retval = (nr == -ENOSPC) ? -EAGAIN : nr;
goto out_free;
}
_
From: KJ Tsanaktsidis <ktsanaktsidis(a)zendesk.com>
Subject: fork: report pid exhaustion correctly
Make the clone and fork syscalls return EAGAIN when the limit on the
number of pids /proc/sys/kernel/pid_max is exceeded.
Currently, when the pid_max limit is exceeded, the kernel will return
ENOSPC from the fork and clone syscalls. This is contrary to the
documented behaviour, which explicitly calls out the pid_max case as one
where EAGAIN should be returned. It also leads to really confusing error
messages in userspace programs which will complain about a lack of disk
space when they fail to create processes/threads for this reason.
This error is being returned because alloc_pid() uses the idr api to find
a new pid; when there are none available, idr_alloc_cyclic() returns
-ENOSPC, and this is being propagated back to userspace.
This behaviour has been broken before, and was explicitly fixed in
commit 35f71bc0a09a ("fork: report pid reservation failure properly"),
so I think -EAGAIN is definitely the right thing to return in this case.
The current behaviour change dates from commit 95846ecf9dac ("pid:
replace pid bitmap implementation with IDR AIP") and was I believe
unintentional.
This patch has no impact on the case where allocating a pid fails because
the child reaper for the namespace is dead; that case will still return
-ENOMEM.
Link: http://lkml.kernel.org/r/20180903111016.46461-1-ktsanaktsidis@zendesk.com
Fixes: 95846ecf9dac ("pid: replace pid bitmap implementation with IDR AIP")
Signed-off-by: KJ Tsanaktsidis <ktsanaktsidis(a)zendesk.com>
Reviewed-by: Andrew Morton <akpm(a)linux-foundation.org>
Acked-by: Michal Hocko <mhocko(a)suse.com>
Cc: Gargi Sharma <gs051095(a)gmail.com>
Cc: Rik van Riel <riel(a)redhat.com>
Cc: Oleg Nesterov <oleg(a)redhat.com>
Cc: <stable(a)vger.kernel.org>
Signed-off-by: Andrew Morton <akpm(a)linux-foundation.org>
---
diff -puN kernel/pid.c~fork-report-pid-exhaustion-correctly kernel/pid.c
--- a/kernel/pid.c~fork-report-pid-exhaustion-correctly
+++ a/kernel/pid.c
@@ -195,7 +195,7 @@ struct pid *alloc_pid(struct pid_namespa
idr_preload_end();
if (nr < 0) {
- retval = nr;
+ retval = (nr == -ENOSPC) ? -EAGAIN : nr;
goto out_free;
}
_