[PATCH vhost 7/7] vdpa/mlx5: Keep notifiers during suspend but ignore
Unregistering notifiers is a costly operation. Instead of removing
the notifiers during device suspend and adding them back at resume,
simply ignore the call when the device is suspended.
At resume time call queue_link_work() to make sure that the device state
is propagated in case there were changes.
For 1 vDPA device x 32 VQs (16 VQPs) attached to a large VM (256 GB RAM,
32 CPUs x 2 threads per core), the device suspend time is reduced from
~13 ms to ~2.5 ms.
Signed-off-by: Dragos Tatulea
Reviewed-by: Tariq Toukan
---
drivers/vdpa/mlx5/net/mlx5_vnet.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c
b/drivers/vdpa/mlx5/net/mlx5_vnet.c
index 87d355aba380..af96e49697d0 100644
--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
@@ -2934,6 +2934,9 @@ static int event_handler(struct notifier_block *nb,
unsigned long event, void *p
struct mlx5_eqe *eqe = param;
int ret = NOTIFY_DONE;
+ if (ndev->mvdev.suspended)
+ return NOTIFY_DONE;
+
if (event == MLX5_EVENT_TYPE_PORT_CHANGE) {
switch (eqe->sub_type) {
case MLX5_PORT_CHANGE_SUBTYPE_DOWN:
@@ -3679,7 +3682,6 @@ static int mlx5_vdpa_suspend(struct vdpa_device *vdev)
mlx5_vdpa_info(mvdev, "suspending device\n");
down_write(>reslock);
- unregister_link_notifier(ndev);
err = suspend_vqs(ndev, 0, ndev->cur_num_vqs);
mlx5_vdpa_cvq_suspend(mvdev);
mvdev->suspended = true;
@@ -3701,7 +3703,7 @@ static int mlx5_vdpa_resume(struct vdpa_device *vdev)
down_write(>reslock);
mvdev->suspended = false;
err = resume_vqs(ndev, 0, ndev->cur_num_vqs);
- register_link_notifier(ndev);
+ queue_link_work(ndev);
up_write(>reslock);
return err;
--
2.45.2
Re: [PATCH V2] net: qrtr: ns: Ignore ENODEV failures in ns
Hello: This patch was applied to netdev/net-next.git (main) by Paolo Abeni : On Wed, 12 Jun 2024 12:01:56 +0530 you wrote: > From: Chris Lew > > Ignore the ENODEV failures returned by kernel_sendmsg(). These errors > indicate that either the local port has been closed or the remote has > gone down. Neither of these scenarios are fatal and will eventually be > handled through packets that are later queued on the control port. > > [...] Here is the summary with links: - [V2] net: qrtr: ns: Ignore ENODEV failures in ns https://git.kernel.org/netdev/net-next/c/404dbd26322f You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html
[PATCH V2] net: qrtr: ns: Ignore ENODEV failures in ns
From: Chris Lew
Ignore the ENODEV failures returned by kernel_sendmsg(). These errors
indicate that either the local port has been closed or the remote has
gone down. Neither of these scenarios are fatal and will eventually be
handled through packets that are later queued on the control port.
Signed-off-by: Chris Lew
Signed-off-by: Sarannya Sasikumar
Reviewed-by: Simon Horman
---
Changes from previous revision:
Changed return type of service_announce_del from int to void.
Fixed alignment issues.
net/qrtr/ns.c | 17 ++---
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/net/qrtr/ns.c b/net/qrtr/ns.c
index 654a3cc0d347..3de9350cbf30 100644
--- a/net/qrtr/ns.c
+++ b/net/qrtr/ns.c
@@ -132,8 +132,8 @@ static int service_announce_new(struct sockaddr_qrtr *dest,
return kernel_sendmsg(qrtr_ns.sock, , , 1, sizeof(pkt));
}
-static int service_announce_del(struct sockaddr_qrtr *dest,
- struct qrtr_server *srv)
+static void service_announce_del(struct sockaddr_qrtr *dest,
+struct qrtr_server *srv)
{
struct qrtr_ctrl_pkt pkt;
struct msghdr msg = { };
@@ -157,10 +157,10 @@ static int service_announce_del(struct sockaddr_qrtr
*dest,
msg.msg_namelen = sizeof(*dest);
ret = kernel_sendmsg(qrtr_ns.sock, , , 1, sizeof(pkt));
- if (ret < 0)
+ if (ret < 0 && ret != -ENODEV)
pr_err("failed to announce del service\n");
- return ret;
+ return;
}
static void lookup_notify(struct sockaddr_qrtr *to, struct qrtr_server *srv,
@@ -188,7 +188,7 @@ static void lookup_notify(struct sockaddr_qrtr *to, struct
qrtr_server *srv,
msg.msg_namelen = sizeof(*to);
ret = kernel_sendmsg(qrtr_ns.sock, , , 1, sizeof(pkt));
- if (ret < 0)
+ if (ret < 0 && ret != -ENODEV)
pr_err("failed to send lookup notification\n");
}
@@ -207,6 +207,9 @@ static int announce_servers(struct sockaddr_qrtr *sq)
xa_for_each(>servers, index, srv) {
ret = service_announce_new(sq, srv);
if (ret < 0) {
+ if (ret == -ENODEV)
+ continue;
+
pr_err("failed to announce new service\n");
return ret;
}
@@ -369,7 +372,7 @@ static int ctrl_cmd_bye(struct sockaddr_qrtr *from)
msg.msg_namelen = sizeof(sq);
ret = kernel_sendmsg(qrtr_ns.sock, , , 1, sizeof(pkt));
- if (ret < 0) {
+ if (ret < 0 && ret != -ENODEV) {
pr_err("failed to send bye cmd\n");
return ret;
}
@@ -443,7 +446,7 @@ static int ctrl_cmd_del_client(struct sockaddr_qrtr *from,
msg.msg_namelen = sizeof(sq);
ret = kernel_sendmsg(qrtr_ns.sock, , , 1, sizeof(pkt));
- if (ret < 0) {
+ if (ret < 0 && ret != -ENODEV) {
pr_err("failed to send del client cmd\n");
return ret;
}
--
2.25.1
Re: [PATCH V2] net: qrtr: ns: Ignore ENODEV failures in ns
On Thu, 30 May 2024 16:06:17 +0530 Sarannya S wrote: > -static int service_announce_del(struct sockaddr_qrtr *dest, > +static void service_announce_del(struct sockaddr_qrtr *dest, > struct qrtr_server *srv) nit: please realign the continuation line
[PATCH V2] net: qrtr: ns: Ignore ENODEV failures in ns
From: Chris Lew
Ignore the ENODEV failures returned by kernel_sendmsg(). These errors
indicate that either the local port has been closed or the remote has
gone down. Neither of these scenarios are fatal and will eventually be
handled through packets that are later queued on the control port.
Signed-off-by: Chris Lew
Signed-off-by: Sarannya Sasikumar
Reviewed-by: Simon Horman
---
Changes from previous revision:
Changed return type of service_announce_del from int to void.
net/qrtr/ns.c | 15 +--
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/net/qrtr/ns.c b/net/qrtr/ns.c
index 654a3cc0d347..e821101e7a4b 100644
--- a/net/qrtr/ns.c
+++ b/net/qrtr/ns.c
@@ -132,7 +132,7 @@ static int service_announce_new(struct sockaddr_qrtr *dest,
return kernel_sendmsg(qrtr_ns.sock, , , 1, sizeof(pkt));
}
-static int service_announce_del(struct sockaddr_qrtr *dest,
+static void service_announce_del(struct sockaddr_qrtr *dest,
struct qrtr_server *srv)
{
struct qrtr_ctrl_pkt pkt;
@@ -157,10 +157,10 @@ static int service_announce_del(struct sockaddr_qrtr
*dest,
msg.msg_namelen = sizeof(*dest);
ret = kernel_sendmsg(qrtr_ns.sock, , , 1, sizeof(pkt));
- if (ret < 0)
+ if (ret < 0 && ret != -ENODEV)
pr_err("failed to announce del service\n");
- return ret;
+ return;
}
static void lookup_notify(struct sockaddr_qrtr *to, struct qrtr_server *srv,
@@ -188,7 +188,7 @@ static void lookup_notify(struct sockaddr_qrtr *to, struct
qrtr_server *srv,
msg.msg_namelen = sizeof(*to);
ret = kernel_sendmsg(qrtr_ns.sock, , , 1, sizeof(pkt));
- if (ret < 0)
+ if (ret < 0 && ret != -ENODEV)
pr_err("failed to send lookup notification\n");
}
@@ -207,6 +207,9 @@ static int announce_servers(struct sockaddr_qrtr *sq)
xa_for_each(>servers, index, srv) {
ret = service_announce_new(sq, srv);
if (ret < 0) {
+ if (ret == -ENODEV)
+ continue;
+
pr_err("failed to announce new service\n");
return ret;
}
@@ -369,7 +372,7 @@ static int ctrl_cmd_bye(struct sockaddr_qrtr *from)
msg.msg_namelen = sizeof(sq);
ret = kernel_sendmsg(qrtr_ns.sock, , , 1, sizeof(pkt));
- if (ret < 0) {
+ if (ret < 0 && ret != -ENODEV) {
pr_err("failed to send bye cmd\n");
return ret;
}
@@ -443,7 +446,7 @@ static int ctrl_cmd_del_client(struct sockaddr_qrtr *from,
msg.msg_namelen = sizeof(sq);
ret = kernel_sendmsg(qrtr_ns.sock, , , 1, sizeof(pkt));
- if (ret < 0) {
+ if (ret < 0 && ret != -ENODEV) {
pr_err("failed to send del client cmd\n");
return ret;
}
--
2.25.1
Re: [PATCH] [v4] module: don't ignore sysfs_create_link() failures
On Mon, Apr 08, 2024 at 09:00:06AM -0700, Luis Chamberlain wrote:
> On Mon, Apr 08, 2024 at 10:05:58AM +0200, Arnd Bergmann wrote:
> > From: Arnd Bergmann
> >
> > The sysfs_create_link() return code is marked as __must_check, but the
> > module_add_driver() function tries hard to not care, by assigning the
> > return code to a variable. When building with 'make W=1', gcc still
> > warns because this variable is only assigned but not used:
> >
> > drivers/base/module.c: In function 'module_add_driver':
> > drivers/base/module.c:36:6: warning: variable 'no_warn' set but not used
> > [-Wunused-but-set-variable]
> >
> > Rework the code to properly unwind and return the error code to the
> > caller. My reading of the original code was that it tries to
> > not fail when the links already exist, so keep ignoring -EEXIST
> > errors.
> >
> > Fixes: e17e0f51aeea ("Driver core: show drivers in /sys/module/")
> > Reviewed-by: Greg Kroah-Hartman
>
> Reviewed-by: Luis Chamberlain
Oh right, I should apply this, sorry about that, will go do that now...
Re: [PATCH] [v4] module: don't ignore sysfs_create_link() failures
On Mon, Apr 08, 2024 at 10:05:58AM +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann
>
> The sysfs_create_link() return code is marked as __must_check, but the
> module_add_driver() function tries hard to not care, by assigning the
> return code to a variable. When building with 'make W=1', gcc still
> warns because this variable is only assigned but not used:
>
> drivers/base/module.c: In function 'module_add_driver':
> drivers/base/module.c:36:6: warning: variable 'no_warn' set but not used
> [-Wunused-but-set-variable]
>
> Rework the code to properly unwind and return the error code to the
> caller. My reading of the original code was that it tries to
> not fail when the links already exist, so keep ignoring -EEXIST
> errors.
>
> Fixes: e17e0f51aeea ("Driver core: show drivers in /sys/module/")
> Reviewed-by: Greg Kroah-Hartman
Reviewed-by: Luis Chamberlain
Luis
[PATCH] [v4] module: don't ignore sysfs_create_link() failures
From: Arnd Bergmann
The sysfs_create_link() return code is marked as __must_check, but the
module_add_driver() function tries hard to not care, by assigning the
return code to a variable. When building with 'make W=1', gcc still
warns because this variable is only assigned but not used:
drivers/base/module.c: In function 'module_add_driver':
drivers/base/module.c:36:6: warning: variable 'no_warn' set but not used
[-Wunused-but-set-variable]
Rework the code to properly unwind and return the error code to the
caller. My reading of the original code was that it tries to
not fail when the links already exist, so keep ignoring -EEXIST
errors.
Fixes: e17e0f51aeea ("Driver core: show drivers in /sys/module/")
Reviewed-by: Greg Kroah-Hartman
See-also: 4a7fb6363f2d ("add __must_check to device management code")
Signed-off-by: Arnd Bergmann
---
Luis, can you merge this through the modules-next tree?
Cc: Luis Chamberlain
Cc: linux-modu...@vger.kernel.org
Cc: "Rafael J. Wysocki"
---
v4: minor style changes, based on feedback from Andy Shevchenko
v3: make error handling stricter, add unwinding,
fix build fail with CONFIG_MODULES=n
v2: rework to actually handle the error. I have not tested the
error handling beyond build testing, so please review carefully.
---
drivers/base/base.h | 9 ++---
drivers/base/bus.c| 9 -
drivers/base/module.c | 42 +++---
3 files changed, 45 insertions(+), 15 deletions(-)
diff --git a/drivers/base/base.h b/drivers/base/base.h
index 0738ccad08b2..db4f910e8e36 100644
--- a/drivers/base/base.h
+++ b/drivers/base/base.h
@@ -192,11 +192,14 @@ extern struct kset *devices_kset;
void devices_kset_move_last(struct device *dev);
#if defined(CONFIG_MODULES) && defined(CONFIG_SYSFS)
-void module_add_driver(struct module *mod, struct device_driver *drv);
+int module_add_driver(struct module *mod, struct device_driver *drv);
void module_remove_driver(struct device_driver *drv);
#else
-static inline void module_add_driver(struct module *mod,
-struct device_driver *drv) { }
+static inline int module_add_driver(struct module *mod,
+ struct device_driver *drv)
+{
+ return 0;
+}
static inline void module_remove_driver(struct device_driver *drv) { }
#endif
diff --git a/drivers/base/bus.c b/drivers/base/bus.c
index daee55c9b2d9..ffea0728b8b2 100644
--- a/drivers/base/bus.c
+++ b/drivers/base/bus.c
@@ -674,7 +674,12 @@ int bus_add_driver(struct device_driver *drv)
if (error)
goto out_del_list;
}
- module_add_driver(drv->owner, drv);
+ error = module_add_driver(drv->owner, drv);
+ if (error) {
+ printk(KERN_ERR "%s: failed to create module links for %s\n",
+ __func__, drv->name);
+ goto out_detach;
+ }
error = driver_create_file(drv, _attr_uevent);
if (error) {
@@ -699,6 +704,8 @@ int bus_add_driver(struct device_driver *drv)
return 0;
+out_detach:
+ driver_detach(drv);
out_del_list:
klist_del(>knode_bus);
out_unregister:
diff --git a/drivers/base/module.c b/drivers/base/module.c
index 46ad4d636731..a1b55da07127 100644
--- a/drivers/base/module.c
+++ b/drivers/base/module.c
@@ -30,14 +30,14 @@ static void module_create_drivers_dir(struct module_kobject
*mk)
mutex_unlock(_dir_mutex);
}
-void module_add_driver(struct module *mod, struct device_driver *drv)
+int module_add_driver(struct module *mod, struct device_driver *drv)
{
char *driver_name;
- int no_warn;
struct module_kobject *mk = NULL;
+ int ret;
if (!drv)
- return;
+ return 0;
if (mod)
mk = >mkobj;
@@ -56,17 +56,37 @@ void module_add_driver(struct module *mod, struct
device_driver *drv)
}
if (!mk)
- return;
+ return 0;
+
+ ret = sysfs_create_link(>p->kobj, >kobj, "module");
+ if (ret)
+ return ret;
- /* Don't check return codes; these calls are idempotent */
- no_warn = sysfs_create_link(>p->kobj, >kobj, "module");
driver_name = make_driver_name(drv);
- if (driver_name) {
- module_create_drivers_dir(mk);
- no_warn = sysfs_create_link(mk->drivers_dir, >p->kobj,
- driver_name);
- kfree(driver_name);
+ if (!driver_name) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ module_create_drivers_dir(mk);
+ if (!mk->drivers_dir) {
+ ret = -EINVAL;
+ goto out;
}
+
+ ret = sysfs_create_link(mk->drivers_dir, >p->kobj, driver_name);
+ if (ret)
+ goto out;
+
+ kfree(driver_name);
+
+ return 0;
+out:
+ sysfs_remove_link(>p->kobj, "module");
+
Re: [PATCH] [v3] module: don't ignore sysfs_create_link() failures
On Tue, Mar 26, 2024, at 16:29, Andy Shevchenko wrote:
> On Tue, Mar 26, 2024 at 03:57:18PM +0100, Arnd Bergmann wrote:
>> From: Arnd Bergmann
>>
>> The sysfs_create_link() return code is marked as __must_check, but the
>> module_add_driver() function tries hard to not care, by assigning the
>> return code to a variable. When building with 'make W=1', gcc still
>> warns because this variable is only assigned but not used:
>>
>> drivers/base/module.c: In function 'module_add_driver':
>> drivers/base/module.c:36:6: warning: variable 'no_warn' set but not used
>> [-Wunused-but-set-variable]
>>
>> Rework the code to properly unwind and return the error code to the
>> caller. My reading of the original code was that it tries to
>> not fail when the links already exist, so keep ignoring -EEXIST
>> errors.
>
>> Cc: Luis Chamberlain
>> Cc: linux-modu...@vger.kernel.org
>> Cc: Greg Kroah-Hartman
>> Cc: "Rafael J. Wysocki"
>
> Wondering if you can move these to be after --- to avoid polluting commit
> message. This will have the same effect and be archived on lore. But on
> pros side it will unload the commit message(s) from unneeded noise.
Done
>
>> +error = module_add_driver(drv->owner, drv);
>> +if (error) {
>> +printk(KERN_ERR "%s: failed to create module links for %s\n",
>> +__func__, drv->name);
>
> What's wrong with pr_err()? Even if it's not a style used, in a new pieces of
> code this can be improved beforehand. So, we will reduce a technical debt, and
> not adding to it.
I think that would be more confusing, and would rather keep the
style consistent. There is no practical difference here.
>> +int module_add_driver(struct module *mod, struct device_driver *drv)
>> {
>> char *driver_name;
>> -int no_warn;
>> +int ret;
>
> I would move it...
>
>> struct module_kobject *mk = NULL;
>
> ...to be here.
Done
Arnd
Re: [PATCH] [v3] module: don't ignore sysfs_create_link() failures
On Tue, Mar 26, 2024 at 03:57:18PM +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann
>
> The sysfs_create_link() return code is marked as __must_check, but the
> module_add_driver() function tries hard to not care, by assigning the
> return code to a variable. When building with 'make W=1', gcc still
> warns because this variable is only assigned but not used:
>
> drivers/base/module.c: In function 'module_add_driver':
> drivers/base/module.c:36:6: warning: variable 'no_warn' set but not used
> [-Wunused-but-set-variable]
>
> Rework the code to properly unwind and return the error code to the
> caller. My reading of the original code was that it tries to
> not fail when the links already exist, so keep ignoring -EEXIST
> errors.
>
> Cc: Luis Chamberlain
> Cc: linux-modu...@vger.kernel.org
> Cc: Greg Kroah-Hartman
> Cc: "Rafael J. Wysocki"
> Fixes: e17e0f51aeea ("Driver core: show drivers in /sys/module/")
> See-also: 4a7fb6363f2d ("add __must_check to device management code")
> Signed-off-by: Arnd Bergmann
> ---
> v3: make error handling stricter, add unwinding,
> fix build fail with CONFIG_MODULES=n
> v2: rework to actually handle the error. I have not tested the
> error handling beyond build testing, so please review carefully.
> ---
> drivers/base/base.h | 9 ++---
> drivers/base/bus.c| 9 -
> drivers/base/module.c | 42 +++---
> 3 files changed, 45 insertions(+), 15 deletions(-)
Reviewed-by: Greg Kroah-Hartman
Re: [PATCH] [v3] module: don't ignore sysfs_create_link() failures
On Tue, Mar 26, 2024 at 03:57:18PM +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann
>
> The sysfs_create_link() return code is marked as __must_check, but the
> module_add_driver() function tries hard to not care, by assigning the
> return code to a variable. When building with 'make W=1', gcc still
> warns because this variable is only assigned but not used:
>
> drivers/base/module.c: In function 'module_add_driver':
> drivers/base/module.c:36:6: warning: variable 'no_warn' set but not used
> [-Wunused-but-set-variable]
>
> Rework the code to properly unwind and return the error code to the
> caller. My reading of the original code was that it tries to
> not fail when the links already exist, so keep ignoring -EEXIST
> errors.
> Cc: Luis Chamberlain
> Cc: linux-modu...@vger.kernel.org
> Cc: Greg Kroah-Hartman
> Cc: "Rafael J. Wysocki"
Wondering if you can move these to be after --- to avoid polluting commit
message. This will have the same effect and be archived on lore. But on
pros side it will unload the commit message(s) from unneeded noise.
...
> + error = module_add_driver(drv->owner, drv);
> + if (error) {
> + printk(KERN_ERR "%s: failed to create module links for %s\n",
> + __func__, drv->name);
What's wrong with pr_err()? Even if it's not a style used, in a new pieces of
code this can be improved beforehand. So, we will reduce a technical debt, and
not adding to it.
> + goto out_detach;
> + }
...
> +int module_add_driver(struct module *mod, struct device_driver *drv)
> {
> char *driver_name;
> - int no_warn;
> + int ret;
I would move it...
> struct module_kobject *mk = NULL;
...to be here.
--
With Best Regards,
Andy Shevchenko
[PATCH] [v3] module: don't ignore sysfs_create_link() failures
From: Arnd Bergmann
The sysfs_create_link() return code is marked as __must_check, but the
module_add_driver() function tries hard to not care, by assigning the
return code to a variable. When building with 'make W=1', gcc still
warns because this variable is only assigned but not used:
drivers/base/module.c: In function 'module_add_driver':
drivers/base/module.c:36:6: warning: variable 'no_warn' set but not used
[-Wunused-but-set-variable]
Rework the code to properly unwind and return the error code to the
caller. My reading of the original code was that it tries to
not fail when the links already exist, so keep ignoring -EEXIST
errors.
Cc: Luis Chamberlain
Cc: linux-modu...@vger.kernel.org
Cc: Greg Kroah-Hartman
Cc: "Rafael J. Wysocki"
Fixes: e17e0f51aeea ("Driver core: show drivers in /sys/module/")
See-also: 4a7fb6363f2d ("add __must_check to device management code")
Signed-off-by: Arnd Bergmann
---
v3: make error handling stricter, add unwinding,
fix build fail with CONFIG_MODULES=n
v2: rework to actually handle the error. I have not tested the
error handling beyond build testing, so please review carefully.
---
drivers/base/base.h | 9 ++---
drivers/base/bus.c| 9 -
drivers/base/module.c | 42 +++---
3 files changed, 45 insertions(+), 15 deletions(-)
diff --git a/drivers/base/base.h b/drivers/base/base.h
index 0738ccad08b2..db4f910e8e36 100644
--- a/drivers/base/base.h
+++ b/drivers/base/base.h
@@ -192,11 +192,14 @@ extern struct kset *devices_kset;
void devices_kset_move_last(struct device *dev);
#if defined(CONFIG_MODULES) && defined(CONFIG_SYSFS)
-void module_add_driver(struct module *mod, struct device_driver *drv);
+int module_add_driver(struct module *mod, struct device_driver *drv);
void module_remove_driver(struct device_driver *drv);
#else
-static inline void module_add_driver(struct module *mod,
-struct device_driver *drv) { }
+static inline int module_add_driver(struct module *mod,
+ struct device_driver *drv)
+{
+ return 0;
+}
static inline void module_remove_driver(struct device_driver *drv) { }
#endif
diff --git a/drivers/base/bus.c b/drivers/base/bus.c
index daee55c9b2d9..ffea0728b8b2 100644
--- a/drivers/base/bus.c
+++ b/drivers/base/bus.c
@@ -674,7 +674,12 @@ int bus_add_driver(struct device_driver *drv)
if (error)
goto out_del_list;
}
- module_add_driver(drv->owner, drv);
+ error = module_add_driver(drv->owner, drv);
+ if (error) {
+ printk(KERN_ERR "%s: failed to create module links for %s\n",
+ __func__, drv->name);
+ goto out_detach;
+ }
error = driver_create_file(drv, _attr_uevent);
if (error) {
@@ -699,6 +704,8 @@ int bus_add_driver(struct device_driver *drv)
return 0;
+out_detach:
+ driver_detach(drv);
out_del_list:
klist_del(>knode_bus);
out_unregister:
diff --git a/drivers/base/module.c b/drivers/base/module.c
index 46ad4d636731..d16b5c8e5473 100644
--- a/drivers/base/module.c
+++ b/drivers/base/module.c
@@ -30,14 +30,14 @@ static void module_create_drivers_dir(struct module_kobject
*mk)
mutex_unlock(_dir_mutex);
}
-void module_add_driver(struct module *mod, struct device_driver *drv)
+int module_add_driver(struct module *mod, struct device_driver *drv)
{
char *driver_name;
- int no_warn;
+ int ret;
struct module_kobject *mk = NULL;
if (!drv)
- return;
+ return 0;
if (mod)
mk = >mkobj;
@@ -56,17 +56,37 @@ void module_add_driver(struct module *mod, struct
device_driver *drv)
}
if (!mk)
- return;
+ return 0;
+
+ ret = sysfs_create_link(>p->kobj, >kobj, "module");
+ if (ret)
+ return ret;
- /* Don't check return codes; these calls are idempotent */
- no_warn = sysfs_create_link(>p->kobj, >kobj, "module");
driver_name = make_driver_name(drv);
- if (driver_name) {
- module_create_drivers_dir(mk);
- no_warn = sysfs_create_link(mk->drivers_dir, >p->kobj,
- driver_name);
- kfree(driver_name);
+ if (!driver_name) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ module_create_drivers_dir(mk);
+ if (!mk->drivers_dir) {
+ ret = -EINVAL;
+ goto out;
}
+
+ ret = sysfs_create_link(mk->drivers_dir, >p->kobj, driver_name);
+ if (ret)
+ goto out;
+
+ kfree(driver_name);
+
+ return 0;
+out:
+ sysfs_remove_link(>p->kobj, "module");
+ sysfs_remove_link(mk->drivers_dir, driver_name);
+ kfree(driver_name);
+
+ return ret;
}
void
Re: [PATCH] [v2] module: don't ignore sysfs_create_link() failures
On Sat, Mar 23, 2024, at 17:50, Greg Kroah-Hartman wrote:
> On Fri, Mar 22, 2024 at 06:39:11PM +0100, Arnd Bergmann wrote:
>> diff --git a/drivers/base/bus.c b/drivers/base/bus.c
>> index daee55c9b2d9..7ef75b60d331 100644
>> --- a/drivers/base/bus.c
>> +++ b/drivers/base/bus.c
>> @@ -674,7 +674,12 @@ int bus_add_driver(struct device_driver *drv)
>> if (error)
>> goto out_del_list;
>> }
>> -module_add_driver(drv->owner, drv);
>> +error = module_add_driver(drv->owner, drv);
>> +if (error) {
>> +printk(KERN_ERR "%s: failed to create module links for %s\n",
>> +__func__, drv->name);
>> +goto out_del_list;
>
> Don't we need to walk back the driver_attach() call here if this fails?
Yes, fixed now. There are still some other calls right after
it that print an error but don't cause bus_add_driver() to fail
though. We may want to add similar unwinding there, but that
feels like it should be a separate patch.
>>
>> if (!mk)
>> -return;
>> +return 0;
>> +
>> +ret = sysfs_create_link(>p->kobj, >kobj, "module");
>> +if (ret && ret != -EEXIST)
>
> Why would EEXIST happen here? How can this be called twice?
>
My impression was that the lack of error handling and the
comment was ab out a case where that might happen
intentionally. I've removed it now as I couldn't find any
evidence that this is really needed. I suppose we would
find out in testing if we do.
Arnd
Re: [PATCH] [v2] module: don't ignore sysfs_create_link() failures
On Fri, Mar 22, 2024 at 06:39:11PM +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann
>
> The sysfs_create_link() return code is marked as __must_check, but the
> module_add_driver() function tries hard to not care, by assigning the
> return code to a variable. When building with 'make W=1', gcc still
> warns because this variable is only assigned but not used:
>
> drivers/base/module.c: In function 'module_add_driver':
> drivers/base/module.c:36:6: warning: variable 'no_warn' set but not used
> [-Wunused-but-set-variable]
>
> Rework the code to properly unwind and return the error code to the
> caller. My reading of the original code was that it tries to
> not fail when the links already exist, so keep ignoring -EEXIST
> errors.
>
> Cc: Luis Chamberlain
> Cc: linux-modu...@vger.kernel.org
> Cc: Greg Kroah-Hartman
> Cc: "Rafael J. Wysocki"
> Fixes: e17e0f51aeea ("Driver core: show drivers in /sys/module/")
> See-also: 4a7fb6363f2d ("add __must_check to device management code")
> Signed-off-by: Arnd Bergmann
> ---
> v2: rework to actually handle the error. I have not tested the
> error handling beyond build testing, so please review carefully.
> ---
> drivers/base/base.h | 2 +-
> drivers/base/bus.c| 7 ++-
> drivers/base/module.c | 42 +++---
> 3 files changed, 38 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/base/base.h b/drivers/base/base.h
> index 0738ccad08b2..0e04bfe02943 100644
> --- a/drivers/base/base.h
> +++ b/drivers/base/base.h
> @@ -192,7 +192,7 @@ extern struct kset *devices_kset;
> void devices_kset_move_last(struct device *dev);
>
> #if defined(CONFIG_MODULES) && defined(CONFIG_SYSFS)
> -void module_add_driver(struct module *mod, struct device_driver *drv);
> +int module_add_driver(struct module *mod, struct device_driver *drv);
> void module_remove_driver(struct device_driver *drv);
> #else
> static inline void module_add_driver(struct module *mod,
> diff --git a/drivers/base/bus.c b/drivers/base/bus.c
> index daee55c9b2d9..7ef75b60d331 100644
> --- a/drivers/base/bus.c
> +++ b/drivers/base/bus.c
> @@ -674,7 +674,12 @@ int bus_add_driver(struct device_driver *drv)
> if (error)
> goto out_del_list;
> }
> - module_add_driver(drv->owner, drv);
> + error = module_add_driver(drv->owner, drv);
> + if (error) {
> + printk(KERN_ERR "%s: failed to create module links for %s\n",
> + __func__, drv->name);
> + goto out_del_list;
Don't we need to walk back the driver_attach() call here if this fails?
> + }
>
> error = driver_create_file(drv, _attr_uevent);
> if (error) {
> diff --git a/drivers/base/module.c b/drivers/base/module.c
> index 46ad4d636731..61282eaed670 100644
> --- a/drivers/base/module.c
> +++ b/drivers/base/module.c
> @@ -30,14 +30,14 @@ static void module_create_drivers_dir(struct
> module_kobject *mk)
> mutex_unlock(_dir_mutex);
> }
>
> -void module_add_driver(struct module *mod, struct device_driver *drv)
> +int module_add_driver(struct module *mod, struct device_driver *drv)
> {
> char *driver_name;
> - int no_warn;
> + int ret;
> struct module_kobject *mk = NULL;
>
> if (!drv)
> - return;
> + return 0;
>
> if (mod)
> mk = >mkobj;
> @@ -56,17 +56,37 @@ void module_add_driver(struct module *mod, struct
> device_driver *drv)
> }
>
> if (!mk)
> - return;
> + return 0;
> +
> + ret = sysfs_create_link(>p->kobj, >kobj, "module");
> + if (ret && ret != -EEXIST)
Why would EEXIST happen here? How can this be called twice?
> + return ret;
>
> - /* Don't check return codes; these calls are idempotent */
> - no_warn = sysfs_create_link(>p->kobj, >kobj, "module");
> driver_name = make_driver_name(drv);
> - if (driver_name) {
> - module_create_drivers_dir(mk);
> - no_warn = sysfs_create_link(mk->drivers_dir, >p->kobj,
> - driver_name);
> - kfree(driver_name);
> + if (!driver_name) {
> + ret = -ENOMEM;
> + goto out;
> + }
> +
> + module_create_drivers_dir(mk);
> + if (!mk->drivers_dir) {
> + ret = -EINVAL;
> + goto out;
> }
> +
> + ret = sysfs_create_link(mk->drivers_dir, >p->kobj, driver_name);
> + if (ret && ret != -EEXIST)
> + goto out;
Same EEXIST question here.
thanks,
greg k-h
[PATCH] [v2] module: don't ignore sysfs_create_link() failures
From: Arnd Bergmann
The sysfs_create_link() return code is marked as __must_check, but the
module_add_driver() function tries hard to not care, by assigning the
return code to a variable. When building with 'make W=1', gcc still
warns because this variable is only assigned but not used:
drivers/base/module.c: In function 'module_add_driver':
drivers/base/module.c:36:6: warning: variable 'no_warn' set but not used
[-Wunused-but-set-variable]
Rework the code to properly unwind and return the error code to the
caller. My reading of the original code was that it tries to
not fail when the links already exist, so keep ignoring -EEXIST
errors.
Cc: Luis Chamberlain
Cc: linux-modu...@vger.kernel.org
Cc: Greg Kroah-Hartman
Cc: "Rafael J. Wysocki"
Fixes: e17e0f51aeea ("Driver core: show drivers in /sys/module/")
See-also: 4a7fb6363f2d ("add __must_check to device management code")
Signed-off-by: Arnd Bergmann
---
v2: rework to actually handle the error. I have not tested the
error handling beyond build testing, so please review carefully.
---
drivers/base/base.h | 2 +-
drivers/base/bus.c| 7 ++-
drivers/base/module.c | 42 +++---
3 files changed, 38 insertions(+), 13 deletions(-)
diff --git a/drivers/base/base.h b/drivers/base/base.h
index 0738ccad08b2..0e04bfe02943 100644
--- a/drivers/base/base.h
+++ b/drivers/base/base.h
@@ -192,7 +192,7 @@ extern struct kset *devices_kset;
void devices_kset_move_last(struct device *dev);
#if defined(CONFIG_MODULES) && defined(CONFIG_SYSFS)
-void module_add_driver(struct module *mod, struct device_driver *drv);
+int module_add_driver(struct module *mod, struct device_driver *drv);
void module_remove_driver(struct device_driver *drv);
#else
static inline void module_add_driver(struct module *mod,
diff --git a/drivers/base/bus.c b/drivers/base/bus.c
index daee55c9b2d9..7ef75b60d331 100644
--- a/drivers/base/bus.c
+++ b/drivers/base/bus.c
@@ -674,7 +674,12 @@ int bus_add_driver(struct device_driver *drv)
if (error)
goto out_del_list;
}
- module_add_driver(drv->owner, drv);
+ error = module_add_driver(drv->owner, drv);
+ if (error) {
+ printk(KERN_ERR "%s: failed to create module links for %s\n",
+ __func__, drv->name);
+ goto out_del_list;
+ }
error = driver_create_file(drv, _attr_uevent);
if (error) {
diff --git a/drivers/base/module.c b/drivers/base/module.c
index 46ad4d636731..61282eaed670 100644
--- a/drivers/base/module.c
+++ b/drivers/base/module.c
@@ -30,14 +30,14 @@ static void module_create_drivers_dir(struct module_kobject
*mk)
mutex_unlock(_dir_mutex);
}
-void module_add_driver(struct module *mod, struct device_driver *drv)
+int module_add_driver(struct module *mod, struct device_driver *drv)
{
char *driver_name;
- int no_warn;
+ int ret;
struct module_kobject *mk = NULL;
if (!drv)
- return;
+ return 0;
if (mod)
mk = >mkobj;
@@ -56,17 +56,37 @@ void module_add_driver(struct module *mod, struct
device_driver *drv)
}
if (!mk)
- return;
+ return 0;
+
+ ret = sysfs_create_link(>p->kobj, >kobj, "module");
+ if (ret && ret != -EEXIST)
+ return ret;
- /* Don't check return codes; these calls are idempotent */
- no_warn = sysfs_create_link(>p->kobj, >kobj, "module");
driver_name = make_driver_name(drv);
- if (driver_name) {
- module_create_drivers_dir(mk);
- no_warn = sysfs_create_link(mk->drivers_dir, >p->kobj,
- driver_name);
- kfree(driver_name);
+ if (!driver_name) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ module_create_drivers_dir(mk);
+ if (!mk->drivers_dir) {
+ ret = -EINVAL;
+ goto out;
}
+
+ ret = sysfs_create_link(mk->drivers_dir, >p->kobj, driver_name);
+ if (ret && ret != -EEXIST)
+ goto out;
+
+ kfree(driver_name);
+
+ return 0;
+out:
+ sysfs_remove_link(>p->kobj, "module");
+ sysfs_remove_link(mk->drivers_dir, driver_name);
+ kfree(driver_name);
+
+ return ret;
}
void module_remove_driver(struct device_driver *drv)
--
2.39.2
Re: [PATCH 2/2] tracing: Ignore -Wstring-compare with diagnostic macros
On Tue, 19 Mar 2024 17:30:41 -0700
Justin Stitt wrote:
> > diff --git a/include/trace/stages/stage6_event_callback.h
> > b/include/trace/stages/stage6_event_callback.h
> > index 83da83a0c14f..56a4eea5a48e 100644
> > --- a/include/trace/stages/stage6_event_callback.h
> > +++ b/include/trace/stages/stage6_event_callback.h
> > @@ -35,9 +35,14 @@
> > do {\
> > char *__str__ = __get_str(dst); \
> > int __len__ = __get_dynamic_array_len(dst) - 1; \
> > + __diag_push(); \
> > + __diag_ignore(clang, 11, "-Wstring-compare",\
> > + "__builtin_constant_p() ensures strcmp()" \
> > + "will be used for string literals"); \
> > WARN_ON_ONCE(__builtin_constant_p(src) ?\
> > strcmp((src), __data_offsets.dst##_ptr_) : \
> > (src) != __data_offsets.dst##_ptr_); \
>
> What exactly is the point of the literal string comparison? Why
> doesn't strcmp do the trick?
This is done in the hotpath, and is only for debugging. The string passed
into __string() should be the same as the string passed into __assign_str().
But this is moot as I ended up always using strcmp() even if it will slow
down the recording of the event.
Next merge window the src parameter (along with the strcmp() checks) are
going away.
-- Steve
>
> > + __diag_pop(); \
> > memcpy(__str__, __data_offsets.dst##_ptr_ ? : \
> >EVENT_NULL_STR, __len__);\
> > __str__[__len__] = '\0';\
> >
> > --
> > 2.44.0
> >
Re: [PATCH 2/2] tracing: Ignore -Wstring-compare with diagnostic macros
On Tue, Mar 19, 2024 at 9:08 AM Nathan Chancellor wrote:
>
> Commit b1afefa62ca9 ("tracing: Use strcmp() in __assign_str() WARN_ON()
> check") addressed a clang warning, -Wstring-compare, with the use of
> __builtin_constant_p() to dispatch to strcmp() if the source string is a
> string literal and a direct comparison if not. Unfortunately, even with
> this change, the warning is still present because __builtin_constant_p()
> is not evaluated at this stage of the pipeline, so clang still thinks
> the else branch could occur for this situation:
>
> include/trace/events/sunrpc.h:705:4: error: result of comparison against a
> string literal is unspecified (use an explicit string comparison function
> instead) [-Werror,-Wstring-compare]
> ...
> include/trace/stages/stage6_event_callback.h:40:15: note: expanded from
> macro '__assign_str'
> 40 | (src) != __data_offsets.dst##_ptr_);
> \
> |^
> ...
>
> Use the compiler diagnostic macros to disable this warning around the
> WARN_ON_ONCE() expression since a string comparison function, strcmp(),
> will always be used for the comparison of string literals.
>
> Fixes: b1afefa62ca9 ("tracing: Use strcmp() in __assign_str() WARN_ON()
> check")
> Reported-by: Linux Kernel Functional Testing
> Closes:
> https://lore.kernel.org/all/CA+G9fYs=otkazs6g1p1ewadfr0qoe6lgovsohqkxmfxoteo...@mail.gmail.com/
> Signed-off-by: Nathan Chancellor
> ---
> include/trace/stages/stage6_event_callback.h | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/include/trace/stages/stage6_event_callback.h
> b/include/trace/stages/stage6_event_callback.h
> index 83da83a0c14f..56a4eea5a48e 100644
> --- a/include/trace/stages/stage6_event_callback.h
> +++ b/include/trace/stages/stage6_event_callback.h
> @@ -35,9 +35,14 @@
> do {\
> char *__str__ = __get_str(dst); \
> int __len__ = __get_dynamic_array_len(dst) - 1; \
> + __diag_push(); \
> + __diag_ignore(clang, 11, "-Wstring-compare",\
> + "__builtin_constant_p() ensures strcmp()" \
> + "will be used for string literals"); \
> WARN_ON_ONCE(__builtin_constant_p(src) ?\
> strcmp((src), __data_offsets.dst##_ptr_) : \
> (src) != __data_offsets.dst##_ptr_); \
What exactly is the point of the literal string comparison? Why
doesn't strcmp do the trick?
> + __diag_pop(); \
> memcpy(__str__, __data_offsets.dst##_ptr_ ? : \
>EVENT_NULL_STR, __len__);\
> __str__[__len__] = '\0';\
>
> --
> 2.44.0
>
[PATCH 2/2] tracing: Ignore -Wstring-compare with diagnostic macros
Commit b1afefa62ca9 ("tracing: Use strcmp() in __assign_str() WARN_ON()
check") addressed a clang warning, -Wstring-compare, with the use of
__builtin_constant_p() to dispatch to strcmp() if the source string is a
string literal and a direct comparison if not. Unfortunately, even with
this change, the warning is still present because __builtin_constant_p()
is not evaluated at this stage of the pipeline, so clang still thinks
the else branch could occur for this situation:
include/trace/events/sunrpc.h:705:4: error: result of comparison against a
string literal is unspecified (use an explicit string comparison function
instead) [-Werror,-Wstring-compare]
...
include/trace/stages/stage6_event_callback.h:40:15: note: expanded from macro
'__assign_str'
40 | (src) != __data_offsets.dst##_ptr_);
\
|^
...
Use the compiler diagnostic macros to disable this warning around the
WARN_ON_ONCE() expression since a string comparison function, strcmp(),
will always be used for the comparison of string literals.
Fixes: b1afefa62ca9 ("tracing: Use strcmp() in __assign_str() WARN_ON() check")
Reported-by: Linux Kernel Functional Testing
Closes:
https://lore.kernel.org/all/CA+G9fYs=otkazs6g1p1ewadfr0qoe6lgovsohqkxmfxoteo...@mail.gmail.com/
Signed-off-by: Nathan Chancellor
---
include/trace/stages/stage6_event_callback.h | 5 +
1 file changed, 5 insertions(+)
diff --git a/include/trace/stages/stage6_event_callback.h
b/include/trace/stages/stage6_event_callback.h
index 83da83a0c14f..56a4eea5a48e 100644
--- a/include/trace/stages/stage6_event_callback.h
+++ b/include/trace/stages/stage6_event_callback.h
@@ -35,9 +35,14 @@
do {\
char *__str__ = __get_str(dst); \
int __len__ = __get_dynamic_array_len(dst) - 1; \
+ __diag_push(); \
+ __diag_ignore(clang, 11, "-Wstring-compare",\
+ "__builtin_constant_p() ensures strcmp()" \
+ "will be used for string literals"); \
WARN_ON_ONCE(__builtin_constant_p(src) ?\
strcmp((src), __data_offsets.dst##_ptr_) : \
(src) != __data_offsets.dst##_ptr_); \
+ __diag_pop(); \
memcpy(__str__, __data_offsets.dst##_ptr_ ? : \
EVENT_NULL_STR, __len__);\
__str__[__len__] = '\0';\
--
2.44.0
Re: [PATCH V1] net: qrtr: ns: Ignore ENODEV failures in ns
On Tue, Dec 26, 2023 at 04:20:03PM -0800, Chris Lew wrote:
>
>
> On 12/23/2023 5:56 AM, Simon Horman wrote:
> > [Dropped bjorn.anders...@kernel.org, as the correct address seems
> > to be anders...@kernel.org, which is already in the CC list.
> > kernel.org rejected sending this email without that update.]
> >
> > On Thu, Dec 21, 2023 at 03:36:50PM +0530, Sarannya S wrote:
> > > From: Chris Lew
> > >
> > > Ignore the ENODEV failures returned by kernel_sendmsg(). These errors
> > > indicate that either the local port has been closed or the remote has
> > > gone down. Neither of these scenarios are fatal and will eventually be
> > > handled through packets that are later queued on the control port.
> > >
> > > Signed-off-by: Chris Lew
> > > Signed-off-by: Sarannya Sasikumar
> > > ---
> > > net/qrtr/ns.c | 11 +++
> > > 1 file changed, 7 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/net/qrtr/ns.c b/net/qrtr/ns.c
> > > index abb0c70..8234339 100644
> > > --- a/net/qrtr/ns.c
> > > +++ b/net/qrtr/ns.c
> > > @@ -157,7 +157,7 @@ static int service_announce_del(struct sockaddr_qrtr
> > > *dest,
> > > msg.msg_namelen = sizeof(*dest);
> > > ret = kernel_sendmsg(qrtr_ns.sock, , , 1, sizeof(pkt));
> > > - if (ret < 0)
> > > + if (ret < 0 && ret != -ENODEV)
> > > pr_err("failed to announce del service\n");
> > > return ret;
> >
> > Hi,
> >
> > The caller of service_announce_del() ignores it's return value.
> > So the only action on error is the pr_err() call above, and so
> > with this patch -ENODEV is indeed ignored.
> >
> > However, I wonder if it would make things clearer to the reader (me?)
> > if the return type of service_announce_del was updated void. Because
> > as things stand -ENODEV may be returned, which implies something might
> > handle that, even though it doe not.
> >
> > The above notwithstanding, this change looks good to me.
> >
> > Reviewed-by: Simon Horman
> >
> > ...
>
> Hi Simon, thanks for the review and suggestion. We weren't sure whether we
> should change the function prototype on these patches on the chance that
> there will be something that listens and handles this in the future. I think
> it's a good idea to change it to void and we can change it back if there is
> such a usecase in the future.
Hi Chris,
yes, I think that would be a good approach.
Re: [PATCH V1] net: qrtr: ns: Ignore ENODEV failures in ns
On 12/23/2023 5:56 AM, Simon Horman wrote:
[Dropped bjorn.anders...@kernel.org, as the correct address seems
to be anders...@kernel.org, which is already in the CC list.
kernel.org rejected sending this email without that update.]
On Thu, Dec 21, 2023 at 03:36:50PM +0530, Sarannya S wrote:
From: Chris Lew
Ignore the ENODEV failures returned by kernel_sendmsg(). These errors
indicate that either the local port has been closed or the remote has
gone down. Neither of these scenarios are fatal and will eventually be
handled through packets that are later queued on the control port.
Signed-off-by: Chris Lew
Signed-off-by: Sarannya Sasikumar
---
net/qrtr/ns.c | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/net/qrtr/ns.c b/net/qrtr/ns.c
index abb0c70..8234339 100644
--- a/net/qrtr/ns.c
+++ b/net/qrtr/ns.c
@@ -157,7 +157,7 @@ static int service_announce_del(struct sockaddr_qrtr *dest,
msg.msg_namelen = sizeof(*dest);
ret = kernel_sendmsg(qrtr_ns.sock, , , 1, sizeof(pkt));
- if (ret < 0)
+ if (ret < 0 && ret != -ENODEV)
pr_err("failed to announce del service\n");
return ret;
Hi,
The caller of service_announce_del() ignores it's return value.
So the only action on error is the pr_err() call above, and so
with this patch -ENODEV is indeed ignored.
However, I wonder if it would make things clearer to the reader (me?)
if the return type of service_announce_del was updated void. Because
as things stand -ENODEV may be returned, which implies something might
handle that, even though it doe not.
The above notwithstanding, this change looks good to me.
Reviewed-by: Simon Horman
...
Hi Simon, thanks for the review and suggestion. We weren't sure whether
we should change the function prototype on these patches on the chance
that there will be something that listens and handles this in the
future. I think it's a good idea to change it to void and we can change
it back if there is such a usecase in the future.
Re: [PATCH V1] net: qrtr: ns: Ignore ENODEV failures in ns
[Dropped bjorn.anders...@kernel.org, as the correct address seems
to be anders...@kernel.org, which is already in the CC list.
kernel.org rejected sending this email without that update.]
On Thu, Dec 21, 2023 at 03:36:50PM +0530, Sarannya S wrote:
> From: Chris Lew
>
> Ignore the ENODEV failures returned by kernel_sendmsg(). These errors
> indicate that either the local port has been closed or the remote has
> gone down. Neither of these scenarios are fatal and will eventually be
> handled through packets that are later queued on the control port.
>
> Signed-off-by: Chris Lew
> Signed-off-by: Sarannya Sasikumar
> ---
> net/qrtr/ns.c | 11 +++
> 1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/net/qrtr/ns.c b/net/qrtr/ns.c
> index abb0c70..8234339 100644
> --- a/net/qrtr/ns.c
> +++ b/net/qrtr/ns.c
> @@ -157,7 +157,7 @@ static int service_announce_del(struct sockaddr_qrtr
> *dest,
> msg.msg_namelen = sizeof(*dest);
>
> ret = kernel_sendmsg(qrtr_ns.sock, , , 1, sizeof(pkt));
> - if (ret < 0)
> + if (ret < 0 && ret != -ENODEV)
> pr_err("failed to announce del service\n");
>
> return ret;
Hi,
The caller of service_announce_del() ignores it's return value.
So the only action on error is the pr_err() call above, and so
with this patch -ENODEV is indeed ignored.
However, I wonder if it would make things clearer to the reader (me?)
if the return type of service_announce_del was updated void. Because
as things stand -ENODEV may be returned, which implies something might
handle that, even though it doe not.
The above notwithstanding, this change looks good to me.
Reviewed-by: Simon Horman
...
[PATCH V1] net: qrtr: ns: Ignore ENODEV failures in ns
From: Chris Lew
Ignore the ENODEV failures returned by kernel_sendmsg(). These errors
indicate that either the local port has been closed or the remote has
gone down. Neither of these scenarios are fatal and will eventually be
handled through packets that are later queued on the control port.
Signed-off-by: Chris Lew
Signed-off-by: Sarannya Sasikumar
---
net/qrtr/ns.c | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/net/qrtr/ns.c b/net/qrtr/ns.c
index abb0c70..8234339 100644
--- a/net/qrtr/ns.c
+++ b/net/qrtr/ns.c
@@ -157,7 +157,7 @@ static int service_announce_del(struct sockaddr_qrtr *dest,
msg.msg_namelen = sizeof(*dest);
ret = kernel_sendmsg(qrtr_ns.sock, , , 1, sizeof(pkt));
- if (ret < 0)
+ if (ret < 0 && ret != -ENODEV)
pr_err("failed to announce del service\n");
return ret;
@@ -188,7 +188,7 @@ static void lookup_notify(struct sockaddr_qrtr *to, struct
qrtr_server *srv,
msg.msg_namelen = sizeof(*to);
ret = kernel_sendmsg(qrtr_ns.sock, , , 1, sizeof(pkt));
- if (ret < 0)
+ if (ret < 0 && ret != -ENODEV)
pr_err("failed to send lookup notification\n");
}
@@ -207,6 +207,9 @@ static int announce_servers(struct sockaddr_qrtr *sq)
xa_for_each(>servers, index, srv) {
ret = service_announce_new(sq, srv);
if (ret < 0) {
+ if (ret == -ENODEV)
+ continue;
+
pr_err("failed to announce new service\n");
return ret;
}
@@ -369,7 +372,7 @@ static int ctrl_cmd_bye(struct sockaddr_qrtr *from)
msg.msg_namelen = sizeof(sq);
ret = kernel_sendmsg(qrtr_ns.sock, , , 1, sizeof(pkt));
- if (ret < 0) {
+ if (ret < 0 && ret != -ENODEV) {
pr_err("failed to send bye cmd\n");
return ret;
}
@@ -443,7 +446,7 @@ static int ctrl_cmd_del_client(struct sockaddr_qrtr *from,
msg.msg_namelen = sizeof(sq);
ret = kernel_sendmsg(qrtr_ns.sock, , , 1, sizeof(pkt));
- if (ret < 0) {
+ if (ret < 0 && ret != -ENODEV) {
pr_err("failed to send del client cmd\n");
return ret;
}
--
2.7.4
[PATCH V1 1/1] net: qrtr: ns: Ignore ENODEV failures in ns
From: Chris Lew
Ignore the ENODEV failures returned by kernel_sendmsg(). These errors
indicate that either the local port has been closed or the remote has
gone down. Neither of these scenarios are fatal and will eventually be
handled through packets that are later queued on the control port.
Signed-off-by: Chris Lew
Signed-off-by: Sarannya Sasikumar
---
net/qrtr/ns.c | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/net/qrtr/ns.c b/net/qrtr/ns.c
index abb0c70..8234339 100644
--- a/net/qrtr/ns.c
+++ b/net/qrtr/ns.c
@@ -157,7 +157,7 @@ static int service_announce_del(struct sockaddr_qrtr *dest,
msg.msg_namelen = sizeof(*dest);
ret = kernel_sendmsg(qrtr_ns.sock, , , 1, sizeof(pkt));
- if (ret < 0)
+ if (ret < 0 && ret != -ENODEV)
pr_err("failed to announce del service\n");
return ret;
@@ -188,7 +188,7 @@ static void lookup_notify(struct sockaddr_qrtr *to, struct
qrtr_server *srv,
msg.msg_namelen = sizeof(*to);
ret = kernel_sendmsg(qrtr_ns.sock, , , 1, sizeof(pkt));
- if (ret < 0)
+ if (ret < 0 && ret != -ENODEV)
pr_err("failed to send lookup notification\n");
}
@@ -207,6 +207,9 @@ static int announce_servers(struct sockaddr_qrtr *sq)
xa_for_each(>servers, index, srv) {
ret = service_announce_new(sq, srv);
if (ret < 0) {
+ if (ret == -ENODEV)
+ continue;
+
pr_err("failed to announce new service\n");
return ret;
}
@@ -369,7 +372,7 @@ static int ctrl_cmd_bye(struct sockaddr_qrtr *from)
msg.msg_namelen = sizeof(sq);
ret = kernel_sendmsg(qrtr_ns.sock, , , 1, sizeof(pkt));
- if (ret < 0) {
+ if (ret < 0 && ret != -ENODEV) {
pr_err("failed to send bye cmd\n");
return ret;
}
@@ -443,7 +446,7 @@ static int ctrl_cmd_del_client(struct sockaddr_qrtr *from,
msg.msg_namelen = sizeof(sq);
ret = kernel_sendmsg(qrtr_ns.sock, , , 1, sizeof(pkt));
- if (ret < 0) {
+ if (ret < 0 && ret != -ENODEV) {
pr_err("failed to send del client cmd\n");
return ret;
}
--
2.7.4
[PATCH 3/3] module: Don't ignore errors from set_memory_XX()
set_memory_ro(), set_memory_nx(), set_memory_x() and other helps
can fail an return an error. In that case the memory might not be
protected as expected and the module loading has to be aborted to
avoid security issues.
Check return value of all calls to set_memory_XX() and handle
error if any.
Signed-off-by: Christophe Leroy
---
kernel/module/internal.h | 6 ++---
kernel/module/main.c | 18 ++
kernel/module/strict_rwx.c | 48 ++
3 files changed, 50 insertions(+), 22 deletions(-)
diff --git a/kernel/module/internal.h b/kernel/module/internal.h
index 4f1b98f011da..2ebece8a789f 100644
--- a/kernel/module/internal.h
+++ b/kernel/module/internal.h
@@ -322,9 +322,9 @@ static inline struct module *mod_find(unsigned long addr,
struct mod_tree_root *
}
#endif /* CONFIG_MODULES_TREE_LOOKUP */
-void module_enable_rodata_ro(const struct module *mod, bool after_init);
-void module_enable_data_nx(const struct module *mod);
-void module_enable_text_rox(const struct module *mod);
+int module_enable_rodata_ro(const struct module *mod, bool after_init);
+int module_enable_data_nx(const struct module *mod);
+int module_enable_text_rox(const struct module *mod);
int module_enforce_rwx_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
char *secstrings, struct module *mod);
diff --git a/kernel/module/main.c b/kernel/module/main.c
index 64662e55e275..cfe197455d64 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -2568,7 +2568,9 @@ static noinline int do_init_module(struct module *mod)
/* Switch to core kallsyms now init is done: kallsyms may be walking! */
rcu_assign_pointer(mod->kallsyms, >core_kallsyms);
#endif
- module_enable_rodata_ro(mod, true);
+ ret = module_enable_rodata_ro(mod, true);
+ if (ret)
+ goto fail_mutex_unlock;
mod_tree_remove_init(mod);
module_arch_freeing_init(mod);
for_class_mod_mem_type(type, init) {
@@ -2606,6 +2608,8 @@ static noinline int do_init_module(struct module *mod)
return 0;
+fail_mutex_unlock:
+ mutex_unlock(_mutex);
fail_free_freeinit:
kfree(freeinit);
fail:
@@ -2733,9 +2737,15 @@ static int complete_formation(struct module *mod, struct
load_info *info)
module_bug_finalize(info->hdr, info->sechdrs, mod);
module_cfi_finalize(info->hdr, info->sechdrs, mod);
- module_enable_rodata_ro(mod, false);
- module_enable_data_nx(mod);
- module_enable_text_rox(mod);
+ err = module_enable_rodata_ro(mod, false);
+ if (err)
+ goto out;
+ err = module_enable_data_nx(mod);
+ if (err)
+ goto out;
+ err = module_enable_text_rox(mod);
+ if (err)
+ goto out;
/*
* Mark state as coming so strong_try_module_get() ignores us,
diff --git a/kernel/module/strict_rwx.c b/kernel/module/strict_rwx.c
index 9b2d58a8d59d..a14df9655dbe 100644
--- a/kernel/module/strict_rwx.c
+++ b/kernel/module/strict_rwx.c
@@ -11,13 +11,13 @@
#include
#include "internal.h"
-static void module_set_memory(const struct module *mod, enum mod_mem_type type,
+static int module_set_memory(const struct module *mod, enum mod_mem_type type,
int (*set_memory)(unsigned long start, int
num_pages))
{
const struct module_memory *mod_mem = >mem[type];
set_vm_flush_reset_perms(mod_mem->base);
- set_memory((unsigned long)mod_mem->base, mod_mem->size >> PAGE_SHIFT);
+ return set_memory((unsigned long)mod_mem->base, mod_mem->size >>
PAGE_SHIFT);
}
/*
@@ -26,39 +26,57 @@ static void module_set_memory(const struct module *mod,
enum mod_mem_type type,
* CONFIG_STRICT_MODULE_RWX because they are needed regardless of whether we
* are strict.
*/
-void module_enable_text_rox(const struct module *mod)
+int module_enable_text_rox(const struct module *mod)
{
for_class_mod_mem_type(type, text) {
+ int ret;
+
if (IS_ENABLED(CONFIG_STRICT_MODULE_RWX))
- module_set_memory(mod, type, set_memory_rox);
+ ret = module_set_memory(mod, type, set_memory_rox);
else
- module_set_memory(mod, type, set_memory_x);
+ ret = module_set_memory(mod, type, set_memory_x);
+ if (ret)
+ return ret;
}
+ return 0;
}
-void module_enable_rodata_ro(const struct module *mod, bool after_init)
+int module_enable_rodata_ro(const struct module *mod, bool after_init)
{
+ int ret;
+
if (!IS_ENABLED(CONFIG_STRICT_MODULE_RWX))
- return;
+ return 0;
#ifdef CONFIG_STRICT_MODULE_RWX
if (!rodata_enabled)
- return;
+ return 0;
#endif
- module_set_memory(mod, MOD_RODATA, set_memory_ro);
- module_set_memory(mod,
[PATCH 3/3] habanalabs: ignore f/w status error
In case firmware has a bug and erroneously reports a status error
(e.g. device unusable) during boot, allow the user to tell the driver
to continue the boot regardless of the error status.
This will be done via kernel parameter which exposes a mask. The
user that loads the driver can decide exactly which status error to
ignore and which to take into account. The bitmask is according to
defines in hl_boot_if.h
Signed-off-by: Oded Gabbay
---
drivers/misc/habanalabs/common/firmware_if.c| 3 ++-
drivers/misc/habanalabs/common/habanalabs.h | 7 +++
drivers/misc/habanalabs/common/habanalabs_drv.c | 7 +++
3 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/drivers/misc/habanalabs/common/firmware_if.c
b/drivers/misc/habanalabs/common/firmware_if.c
index 377a7ca886fe..0713b2c12d54 100644
--- a/drivers/misc/habanalabs/common/firmware_if.c
+++ b/drivers/misc/habanalabs/common/firmware_if.c
@@ -400,7 +400,8 @@ static int fw_read_errors(struct hl_device *hdev, u32
boot_err0_reg,
err_exists = true;
}
- if (err_exists)
+ if (err_exists && ((err_val & ~CPU_BOOT_ERR0_ENABLED) &
+ lower_32_bits(hdev->boot_error_status_mask)))
return -EIO;
return 0;
diff --git a/drivers/misc/habanalabs/common/habanalabs.h
b/drivers/misc/habanalabs/common/habanalabs.h
index 91291a8e201e..6579f8767abd 100644
--- a/drivers/misc/habanalabs/common/habanalabs.h
+++ b/drivers/misc/habanalabs/common/habanalabs.h
@@ -1962,6 +1962,12 @@ struct hl_mmu_funcs {
* @clock_gating_mask: is clock gating enabled. bitmask that represents the
* different engines. See debugfs-driver-habanalabs for
* details.
+ * @boot_error_status_mask: contains a mask of the device boot error status.
+ * Each bit represents a different error, according to
+ * the defines in hl_boot_if.h. If the bit is cleared,
+ * the error will be ignored by the driver during
+ * device initialization. Mainly used to debug and
+ * workaround firmware bugs
* @in_reset: is device in reset flow.
* @curr_pll_profile: current PLL profile.
* @card_type: Various ASICs have several card types. This indicates the card
@@ -2077,6 +2083,7 @@ struct hl_device {
u64 timeout_jiffies;
u64 max_power;
u64 clock_gating_mask;
+ u64 boot_error_status_mask;
atomic_tin_reset;
enum hl_pll_frequency curr_pll_profile;
enum cpucp_card_types card_type;
diff --git a/drivers/misc/habanalabs/common/habanalabs_drv.c
b/drivers/misc/habanalabs/common/habanalabs_drv.c
index 7135f1e03864..64d1530db985 100644
--- a/drivers/misc/habanalabs/common/habanalabs_drv.c
+++ b/drivers/misc/habanalabs/common/habanalabs_drv.c
@@ -30,6 +30,7 @@ static DEFINE_MUTEX(hl_devs_idr_lock);
static int timeout_locked = 30;
static int reset_on_lockup = 1;
static int memory_scrub = 1;
+static ulong boot_error_status_mask = ULONG_MAX;
module_param(timeout_locked, int, 0444);
MODULE_PARM_DESC(timeout_locked,
@@ -43,6 +44,10 @@ module_param(memory_scrub, int, 0444);
MODULE_PARM_DESC(memory_scrub,
"Scrub device memory in various states (0 = no, 1 = yes, default yes)");
+module_param(boot_error_status_mask, ulong, 0444);
+MODULE_PARM_DESC(boot_error_status_mask,
+ "Mask of the error status during device CPU boot (If bitX is cleared
then error X is masked. Default all 1's)");
+
#define PCI_VENDOR_ID_HABANALABS 0x1da3
#define PCI_IDS_GOYA 0x0001
@@ -319,6 +324,8 @@ int create_hdev(struct hl_device **dev, struct pci_dev
*pdev,
hdev->major = hl_major;
hdev->reset_on_lockup = reset_on_lockup;
hdev->memory_scrub = memory_scrub;
+ hdev->boot_error_status_mask = boot_error_status_mask;
+
hdev->pldm = 0;
set_driver_behavior_per_device(hdev);
--
2.25.1
[PATCH 12/15] x86, module: Ignore __typeid__ relocations
Ignore the __typeid__ relocations generated with CONFIG_CFI_CLANG
when loading modules.
Signed-off-by: Sami Tolvanen
---
arch/x86/kernel/module.c | 4
1 file changed, 4 insertions(+)
diff --git a/arch/x86/kernel/module.c b/arch/x86/kernel/module.c
index 5e9a34b5bd74..c4aeba237eef 100644
--- a/arch/x86/kernel/module.c
+++ b/arch/x86/kernel/module.c
@@ -197,6 +197,10 @@ static int __apply_relocate_add(Elf64_Shdr *sechdrs,
val -= (u64)loc;
write(loc, , 8);
break;
+ case R_X86_64_8:
+ if (!strncmp(strtab + sym->st_name, "__typeid__", 10))
+ break;
+ fallthrough;
default:
pr_err("%s: Unknown rela relocation: %llu\n",
me->name, ELF64_R_TYPE(rel[i].r_info));
--
2.31.1.368.gbe11c130af-goog
[PATCH 11/15] x86, relocs: Ignore __typeid__ relocations
From: Kees Cook
The __typeid__* symbols aren't actually relocations, so they can be
ignored during relocation generation.
Signed-off-by: Kees Cook
Signed-off-by: Sami Tolvanen
---
arch/x86/tools/relocs.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/arch/x86/tools/relocs.c b/arch/x86/tools/relocs.c
index 04c5a44b9682..78516ccea0c8 100644
--- a/arch/x86/tools/relocs.c
+++ b/arch/x86/tools/relocs.c
@@ -48,6 +48,7 @@ static const char * const sym_regex_kernel[S_NSYMTYPES] = {
"^(xen_irq_disable_direct_reloc$|"
"xen_save_fl_direct_reloc$|"
"VDSO|"
+ "__typeid__|"
"__crc_)",
/*
@@ -808,6 +809,12 @@ static int do_reloc64(struct section *sec, Elf_Rel *rel,
ElfW(Sym) *sym,
symname);
break;
+ case R_X86_64_8:
+ if (!shn_abs || !is_reloc(S_ABS, symname))
+ die("Non-whitelisted %s relocation: %s\n",
+ rel_type(r_type), symname);
+ break;
+
case R_X86_64_32:
case R_X86_64_32S:
case R_X86_64_64:
--
2.31.1.368.gbe11c130af-goog
[PATCH 5.11 20/23] block: dont ignore REQ_NOWAIT for direct IO
From: Pavel Begunkov [ Upstream commit f8b78caf21d5bc3fcfc40c18898f9d52ed1451a5 ] If IOCB_NOWAIT is set on submission, then that needs to get propagated to REQ_NOWAIT on the block side. Otherwise we completely lose this information, and any issuer of IOCB_NOWAIT IO will potentially end up blocking on eg request allocation on the storage side. Signed-off-by: Pavel Begunkov Signed-off-by: Jens Axboe Signed-off-by: Sasha Levin --- fs/block_dev.c | 4 1 file changed, 4 insertions(+) diff --git a/fs/block_dev.c b/fs/block_dev.c index 85500e2400cf..b988f78ad4b7 100644 --- a/fs/block_dev.c +++ b/fs/block_dev.c @@ -276,6 +276,8 @@ __blkdev_direct_IO_simple(struct kiocb *iocb, struct iov_iter *iter, bio.bi_opf = dio_bio_write_op(iocb); task_io_account_write(ret); } + if (iocb->ki_flags & IOCB_NOWAIT) + bio.bi_opf |= REQ_NOWAIT; if (iocb->ki_flags & IOCB_HIPRI) bio_set_polled(, iocb); @@ -429,6 +431,8 @@ __blkdev_direct_IO(struct kiocb *iocb, struct iov_iter *iter, int nr_pages) bio->bi_opf = dio_bio_write_op(iocb); task_io_account_write(bio->bi_iter.bi_size); } + if (iocb->ki_flags & IOCB_NOWAIT) + bio->bi_opf |= REQ_NOWAIT; dio->size += bio->bi_iter.bi_size; pos += bio->bi_iter.bi_size; -- 2.30.2
[PATCH 5.10 20/25] block: dont ignore REQ_NOWAIT for direct IO
From: Pavel Begunkov [ Upstream commit f8b78caf21d5bc3fcfc40c18898f9d52ed1451a5 ] If IOCB_NOWAIT is set on submission, then that needs to get propagated to REQ_NOWAIT on the block side. Otherwise we completely lose this information, and any issuer of IOCB_NOWAIT IO will potentially end up blocking on eg request allocation on the storage side. Signed-off-by: Pavel Begunkov Signed-off-by: Jens Axboe Signed-off-by: Sasha Levin --- fs/block_dev.c | 4 1 file changed, 4 insertions(+) diff --git a/fs/block_dev.c b/fs/block_dev.c index 6516051807b8..718533f0fb90 100644 --- a/fs/block_dev.c +++ b/fs/block_dev.c @@ -280,6 +280,8 @@ __blkdev_direct_IO_simple(struct kiocb *iocb, struct iov_iter *iter, bio.bi_opf = dio_bio_write_op(iocb); task_io_account_write(ret); } + if (iocb->ki_flags & IOCB_NOWAIT) + bio.bi_opf |= REQ_NOWAIT; if (iocb->ki_flags & IOCB_HIPRI) bio_set_polled(, iocb); @@ -433,6 +435,8 @@ __blkdev_direct_IO(struct kiocb *iocb, struct iov_iter *iter, int nr_pages) bio->bi_opf = dio_bio_write_op(iocb); task_io_account_write(bio->bi_iter.bi_size); } + if (iocb->ki_flags & IOCB_NOWAIT) + bio->bi_opf |= REQ_NOWAIT; dio->size += bio->bi_iter.bi_size; pos += bio->bi_iter.bi_size; -- 2.30.2
[PATCH 5.4 12/18] block: dont ignore REQ_NOWAIT for direct IO
From: Pavel Begunkov [ Upstream commit f8b78caf21d5bc3fcfc40c18898f9d52ed1451a5 ] If IOCB_NOWAIT is set on submission, then that needs to get propagated to REQ_NOWAIT on the block side. Otherwise we completely lose this information, and any issuer of IOCB_NOWAIT IO will potentially end up blocking on eg request allocation on the storage side. Signed-off-by: Pavel Begunkov Signed-off-by: Jens Axboe Signed-off-by: Sasha Levin --- fs/block_dev.c | 4 1 file changed, 4 insertions(+) diff --git a/fs/block_dev.c b/fs/block_dev.c index 79272cdbe827..bd93563477a4 100644 --- a/fs/block_dev.c +++ b/fs/block_dev.c @@ -246,6 +246,8 @@ __blkdev_direct_IO_simple(struct kiocb *iocb, struct iov_iter *iter, bio.bi_opf = dio_bio_write_op(iocb); task_io_account_write(ret); } + if (iocb->ki_flags & IOCB_NOWAIT) + bio.bi_opf |= REQ_NOWAIT; if (iocb->ki_flags & IOCB_HIPRI) bio_set_polled(, iocb); @@ -399,6 +401,8 @@ __blkdev_direct_IO(struct kiocb *iocb, struct iov_iter *iter, int nr_pages) bio->bi_opf = dio_bio_write_op(iocb); task_io_account_write(bio->bi_iter.bi_size); } + if (iocb->ki_flags & IOCB_NOWAIT) + bio->bi_opf |= REQ_NOWAIT; dio->size += bio->bi_iter.bi_size; pos += bio->bi_iter.bi_size; -- 2.30.2
Re: [PATCH 12/12] USB: cdc-acm: add more Maxlinear/Exar models to ignore list
On Mon, Apr 12, 2021 at 11:55:57AM +0200, Johan Hovold wrote: > From: Mauro Carvalho Chehab > > Now that the xr_serial got support for other models, add their USB IDs > as well. > > The Maxlinear/Exar USB UARTs can be used in either ACM mode using the > cdc-acm driver or in "custom driver" mode in which further features such > as hardware and software flow control, GPIO control and in-band > line-status reporting are available. > > In ACM mode the device always enables RTS/CTS flow control, something > which could prevent transmission in case the CTS input isn't wired up > correctly. > > Ensure that cdc_acm will not bind to these devices if the custom > USB-serial driver is enabled. > > Signed-off-by: Mauro Carvalho Chehab > Link: > https://lore.kernel.org/r/5155887a764cbc11f8da0217fe08a24a77d120b4.1616571453.git.mchehab+hua...@kernel.org > [ johan: rewrite commit message, clean up entries ] > Cc: Oliver Neukum > Signed-off-by: Johan Hovold Reviewed-by: Greg Kroah-Hartman
[PATCH 12/12] USB: cdc-acm: add more Maxlinear/Exar models to ignore list
From: Mauro Carvalho Chehab
Now that the xr_serial got support for other models, add their USB IDs
as well.
The Maxlinear/Exar USB UARTs can be used in either ACM mode using the
cdc-acm driver or in "custom driver" mode in which further features such
as hardware and software flow control, GPIO control and in-band
line-status reporting are available.
In ACM mode the device always enables RTS/CTS flow control, something
which could prevent transmission in case the CTS input isn't wired up
correctly.
Ensure that cdc_acm will not bind to these devices if the custom
USB-serial driver is enabled.
Signed-off-by: Mauro Carvalho Chehab
Link:
https://lore.kernel.org/r/5155887a764cbc11f8da0217fe08a24a77d120b4.1616571453.git.mchehab+hua...@kernel.org
[ johan: rewrite commit message, clean up entries ]
Cc: Oliver Neukum
Signed-off-by: Johan Hovold
---
drivers/usb/class/cdc-acm.c | 14 +++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index 337ffced9c40..434539b13a70 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -1903,9 +1903,17 @@ static const struct usb_device_id acm_ids[] = {
#endif
#if IS_ENABLED(CONFIG_USB_SERIAL_XR)
- { USB_DEVICE(0x04e2, 0x1410), /* Ignore XR21V141X USB to Serial
converter */
- .driver_info = IGNORE_DEVICE,
- },
+ { USB_DEVICE(0x04e2, 0x1400), .driver_info = IGNORE_DEVICE },
+ { USB_DEVICE(0x04e2, 0x1401), .driver_info = IGNORE_DEVICE },
+ { USB_DEVICE(0x04e2, 0x1402), .driver_info = IGNORE_DEVICE },
+ { USB_DEVICE(0x04e2, 0x1403), .driver_info = IGNORE_DEVICE },
+ { USB_DEVICE(0x04e2, 0x1410), .driver_info = IGNORE_DEVICE },
+ { USB_DEVICE(0x04e2, 0x1411), .driver_info = IGNORE_DEVICE },
+ { USB_DEVICE(0x04e2, 0x1412), .driver_info = IGNORE_DEVICE },
+ { USB_DEVICE(0x04e2, 0x1414), .driver_info = IGNORE_DEVICE },
+ { USB_DEVICE(0x04e2, 0x1420), .driver_info = IGNORE_DEVICE },
+ { USB_DEVICE(0x04e2, 0x1422), .driver_info = IGNORE_DEVICE },
+ { USB_DEVICE(0x04e2, 0x1424), .driver_info = IGNORE_DEVICE },
#endif
/*Samsung phone in firmware update mode */
--
2.26.3
[PATCH 5.11 139/210] nfp: flower: ignore duplicate merge hints from FW
From: Yinjun Zhang
[ Upstream commit 2ea538dbee1c79f6f6c24a6f2f82986e4b7ccb78 ]
A merge hint message needs some time to process before the merged
flow actually reaches the firmware, during which we may get duplicate
merge hints if there're more than one packet that hit the pre-merged
flow. And processing duplicate merge hints will cost extra host_ctx's
which are a limited resource.
Avoid the duplicate merge by using hash table to store the sub_flows
to be merged.
Fixes: 8af56f40e53b ("nfp: flower: offload merge flows")
Signed-off-by: Yinjun Zhang
Signed-off-by: Louis Peens
Signed-off-by: Simon Horman
Signed-off-by: David S. Miller
Signed-off-by: Sasha Levin
---
.../net/ethernet/netronome/nfp/flower/main.h | 8
.../ethernet/netronome/nfp/flower/metadata.c | 16 ++-
.../ethernet/netronome/nfp/flower/offload.c | 48 ++-
3 files changed, 69 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/netronome/nfp/flower/main.h
b/drivers/net/ethernet/netronome/nfp/flower/main.h
index caf12eec9945..56833a41f3d2 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/main.h
+++ b/drivers/net/ethernet/netronome/nfp/flower/main.h
@@ -190,6 +190,7 @@ struct nfp_fl_internal_ports {
* @qos_rate_limiters: Current active qos rate limiters
* @qos_stats_lock:Lock on qos stats updates
* @pre_tun_rule_cnt: Number of pre-tunnel rules offloaded
+ * @merge_table: Hash table to store merged flows
*/
struct nfp_flower_priv {
struct nfp_app *app;
@@ -223,6 +224,7 @@ struct nfp_flower_priv {
unsigned int qos_rate_limiters;
spinlock_t qos_stats_lock; /* Protect the qos stats */
int pre_tun_rule_cnt;
+ struct rhashtable merge_table;
};
/**
@@ -350,6 +352,12 @@ struct nfp_fl_payload_link {
};
extern const struct rhashtable_params nfp_flower_table_params;
+extern const struct rhashtable_params merge_table_params;
+
+struct nfp_merge_info {
+ u64 parent_ctx;
+ struct rhash_head ht_node;
+};
struct nfp_fl_stats_frame {
__be32 stats_con_id;
diff --git a/drivers/net/ethernet/netronome/nfp/flower/metadata.c
b/drivers/net/ethernet/netronome/nfp/flower/metadata.c
index aa06fcb38f8b..327bb56b3ef5 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/metadata.c
+++ b/drivers/net/ethernet/netronome/nfp/flower/metadata.c
@@ -490,6 +490,12 @@ const struct rhashtable_params nfp_flower_table_params = {
.automatic_shrinking= true,
};
+const struct rhashtable_params merge_table_params = {
+ .key_offset = offsetof(struct nfp_merge_info, parent_ctx),
+ .head_offset= offsetof(struct nfp_merge_info, ht_node),
+ .key_len= sizeof(u64),
+};
+
int nfp_flower_metadata_init(struct nfp_app *app, u64 host_ctx_count,
unsigned int host_num_mems)
{
@@ -506,6 +512,10 @@ int nfp_flower_metadata_init(struct nfp_app *app, u64
host_ctx_count,
if (err)
goto err_free_flow_table;
+ err = rhashtable_init(>merge_table, _table_params);
+ if (err)
+ goto err_free_stats_ctx_table;
+
get_random_bytes(>mask_id_seed, sizeof(priv->mask_id_seed));
/* Init ring buffer and unallocated mask_ids. */
@@ -513,7 +523,7 @@ int nfp_flower_metadata_init(struct nfp_app *app, u64
host_ctx_count,
kmalloc_array(NFP_FLOWER_MASK_ENTRY_RS,
NFP_FLOWER_MASK_ELEMENT_RS, GFP_KERNEL);
if (!priv->mask_ids.mask_id_free_list.buf)
- goto err_free_stats_ctx_table;
+ goto err_free_merge_table;
priv->mask_ids.init_unallocated = NFP_FLOWER_MASK_ENTRY_RS - 1;
@@ -550,6 +560,8 @@ err_free_last_used:
kfree(priv->mask_ids.last_used);
err_free_mask_id:
kfree(priv->mask_ids.mask_id_free_list.buf);
+err_free_merge_table:
+ rhashtable_destroy(>merge_table);
err_free_stats_ctx_table:
rhashtable_destroy(>stats_ctx_table);
err_free_flow_table:
@@ -568,6 +580,8 @@ void nfp_flower_metadata_cleanup(struct nfp_app *app)
nfp_check_rhashtable_empty, NULL);
rhashtable_free_and_destroy(>stats_ctx_table,
nfp_check_rhashtable_empty, NULL);
+ rhashtable_free_and_destroy(>merge_table,
+ nfp_check_rhashtable_empty, NULL);
kvfree(priv->stats);
kfree(priv->mask_ids.mask_id_free_list.buf);
kfree(priv->mask_ids.last_used);
diff --git a/drivers/net/ethernet/netronome/nfp/flower/offload.c
b/drivers/net/ethernet/netronome/nfp/flower/offload.c
index d72225d64a75..e95969c462e4 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/offload.c
+++ b/drivers/net/ethernet/netronome/nfp/flower/offload.c
@@ -1009,6 +1009,8 @@ int nfp_flower_merge_offloaded_flows(struct nfp_app *app,
struct netlink_ext_ack *extack = NULL;
struct nfp_fl_payload *merge_flow;
struct nfp_fl_key_ls
[PATCH 5.10 127/188] nfp: flower: ignore duplicate merge hints from FW
From: Yinjun Zhang
[ Upstream commit 2ea538dbee1c79f6f6c24a6f2f82986e4b7ccb78 ]
A merge hint message needs some time to process before the merged
flow actually reaches the firmware, during which we may get duplicate
merge hints if there're more than one packet that hit the pre-merged
flow. And processing duplicate merge hints will cost extra host_ctx's
which are a limited resource.
Avoid the duplicate merge by using hash table to store the sub_flows
to be merged.
Fixes: 8af56f40e53b ("nfp: flower: offload merge flows")
Signed-off-by: Yinjun Zhang
Signed-off-by: Louis Peens
Signed-off-by: Simon Horman
Signed-off-by: David S. Miller
Signed-off-by: Sasha Levin
---
.../net/ethernet/netronome/nfp/flower/main.h | 8
.../ethernet/netronome/nfp/flower/metadata.c | 16 ++-
.../ethernet/netronome/nfp/flower/offload.c | 48 ++-
3 files changed, 69 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/netronome/nfp/flower/main.h
b/drivers/net/ethernet/netronome/nfp/flower/main.h
index caf12eec9945..56833a41f3d2 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/main.h
+++ b/drivers/net/ethernet/netronome/nfp/flower/main.h
@@ -190,6 +190,7 @@ struct nfp_fl_internal_ports {
* @qos_rate_limiters: Current active qos rate limiters
* @qos_stats_lock:Lock on qos stats updates
* @pre_tun_rule_cnt: Number of pre-tunnel rules offloaded
+ * @merge_table: Hash table to store merged flows
*/
struct nfp_flower_priv {
struct nfp_app *app;
@@ -223,6 +224,7 @@ struct nfp_flower_priv {
unsigned int qos_rate_limiters;
spinlock_t qos_stats_lock; /* Protect the qos stats */
int pre_tun_rule_cnt;
+ struct rhashtable merge_table;
};
/**
@@ -350,6 +352,12 @@ struct nfp_fl_payload_link {
};
extern const struct rhashtable_params nfp_flower_table_params;
+extern const struct rhashtable_params merge_table_params;
+
+struct nfp_merge_info {
+ u64 parent_ctx;
+ struct rhash_head ht_node;
+};
struct nfp_fl_stats_frame {
__be32 stats_con_id;
diff --git a/drivers/net/ethernet/netronome/nfp/flower/metadata.c
b/drivers/net/ethernet/netronome/nfp/flower/metadata.c
index aa06fcb38f8b..327bb56b3ef5 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/metadata.c
+++ b/drivers/net/ethernet/netronome/nfp/flower/metadata.c
@@ -490,6 +490,12 @@ const struct rhashtable_params nfp_flower_table_params = {
.automatic_shrinking= true,
};
+const struct rhashtable_params merge_table_params = {
+ .key_offset = offsetof(struct nfp_merge_info, parent_ctx),
+ .head_offset= offsetof(struct nfp_merge_info, ht_node),
+ .key_len= sizeof(u64),
+};
+
int nfp_flower_metadata_init(struct nfp_app *app, u64 host_ctx_count,
unsigned int host_num_mems)
{
@@ -506,6 +512,10 @@ int nfp_flower_metadata_init(struct nfp_app *app, u64
host_ctx_count,
if (err)
goto err_free_flow_table;
+ err = rhashtable_init(>merge_table, _table_params);
+ if (err)
+ goto err_free_stats_ctx_table;
+
get_random_bytes(>mask_id_seed, sizeof(priv->mask_id_seed));
/* Init ring buffer and unallocated mask_ids. */
@@ -513,7 +523,7 @@ int nfp_flower_metadata_init(struct nfp_app *app, u64
host_ctx_count,
kmalloc_array(NFP_FLOWER_MASK_ENTRY_RS,
NFP_FLOWER_MASK_ELEMENT_RS, GFP_KERNEL);
if (!priv->mask_ids.mask_id_free_list.buf)
- goto err_free_stats_ctx_table;
+ goto err_free_merge_table;
priv->mask_ids.init_unallocated = NFP_FLOWER_MASK_ENTRY_RS - 1;
@@ -550,6 +560,8 @@ err_free_last_used:
kfree(priv->mask_ids.last_used);
err_free_mask_id:
kfree(priv->mask_ids.mask_id_free_list.buf);
+err_free_merge_table:
+ rhashtable_destroy(>merge_table);
err_free_stats_ctx_table:
rhashtable_destroy(>stats_ctx_table);
err_free_flow_table:
@@ -568,6 +580,8 @@ void nfp_flower_metadata_cleanup(struct nfp_app *app)
nfp_check_rhashtable_empty, NULL);
rhashtable_free_and_destroy(>stats_ctx_table,
nfp_check_rhashtable_empty, NULL);
+ rhashtable_free_and_destroy(>merge_table,
+ nfp_check_rhashtable_empty, NULL);
kvfree(priv->stats);
kfree(priv->mask_ids.mask_id_free_list.buf);
kfree(priv->mask_ids.last_used);
diff --git a/drivers/net/ethernet/netronome/nfp/flower/offload.c
b/drivers/net/ethernet/netronome/nfp/flower/offload.c
index d72225d64a75..e95969c462e4 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/offload.c
+++ b/drivers/net/ethernet/netronome/nfp/flower/offload.c
@@ -1009,6 +1009,8 @@ int nfp_flower_merge_offloaded_flows(struct nfp_app *app,
struct netlink_ext_ack *extack = NULL;
struct nfp_fl_payload *merge_flow;
struct nfp_fl_key_ls
[PATCH 5.4 066/111] nfp: flower: ignore duplicate merge hints from FW
From: Yinjun Zhang
[ Upstream commit 2ea538dbee1c79f6f6c24a6f2f82986e4b7ccb78 ]
A merge hint message needs some time to process before the merged
flow actually reaches the firmware, during which we may get duplicate
merge hints if there're more than one packet that hit the pre-merged
flow. And processing duplicate merge hints will cost extra host_ctx's
which are a limited resource.
Avoid the duplicate merge by using hash table to store the sub_flows
to be merged.
Fixes: 8af56f40e53b ("nfp: flower: offload merge flows")
Signed-off-by: Yinjun Zhang
Signed-off-by: Louis Peens
Signed-off-by: Simon Horman
Signed-off-by: David S. Miller
Signed-off-by: Sasha Levin
---
.../net/ethernet/netronome/nfp/flower/main.h | 8
.../ethernet/netronome/nfp/flower/metadata.c | 16 ++-
.../ethernet/netronome/nfp/flower/offload.c | 48 ++-
3 files changed, 69 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/netronome/nfp/flower/main.h
b/drivers/net/ethernet/netronome/nfp/flower/main.h
index 31d94592a7c0..2d99533ad3e0 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/main.h
+++ b/drivers/net/ethernet/netronome/nfp/flower/main.h
@@ -164,6 +164,7 @@ struct nfp_fl_internal_ports {
* @qos_rate_limiters: Current active qos rate limiters
* @qos_stats_lock:Lock on qos stats updates
* @pre_tun_rule_cnt: Number of pre-tunnel rules offloaded
+ * @merge_table: Hash table to store merged flows
*/
struct nfp_flower_priv {
struct nfp_app *app;
@@ -196,6 +197,7 @@ struct nfp_flower_priv {
unsigned int qos_rate_limiters;
spinlock_t qos_stats_lock; /* Protect the qos stats */
int pre_tun_rule_cnt;
+ struct rhashtable merge_table;
};
/**
@@ -310,6 +312,12 @@ struct nfp_fl_payload_link {
};
extern const struct rhashtable_params nfp_flower_table_params;
+extern const struct rhashtable_params merge_table_params;
+
+struct nfp_merge_info {
+ u64 parent_ctx;
+ struct rhash_head ht_node;
+};
struct nfp_fl_stats_frame {
__be32 stats_con_id;
diff --git a/drivers/net/ethernet/netronome/nfp/flower/metadata.c
b/drivers/net/ethernet/netronome/nfp/flower/metadata.c
index aa06fcb38f8b..327bb56b3ef5 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/metadata.c
+++ b/drivers/net/ethernet/netronome/nfp/flower/metadata.c
@@ -490,6 +490,12 @@ const struct rhashtable_params nfp_flower_table_params = {
.automatic_shrinking= true,
};
+const struct rhashtable_params merge_table_params = {
+ .key_offset = offsetof(struct nfp_merge_info, parent_ctx),
+ .head_offset= offsetof(struct nfp_merge_info, ht_node),
+ .key_len= sizeof(u64),
+};
+
int nfp_flower_metadata_init(struct nfp_app *app, u64 host_ctx_count,
unsigned int host_num_mems)
{
@@ -506,6 +512,10 @@ int nfp_flower_metadata_init(struct nfp_app *app, u64
host_ctx_count,
if (err)
goto err_free_flow_table;
+ err = rhashtable_init(>merge_table, _table_params);
+ if (err)
+ goto err_free_stats_ctx_table;
+
get_random_bytes(>mask_id_seed, sizeof(priv->mask_id_seed));
/* Init ring buffer and unallocated mask_ids. */
@@ -513,7 +523,7 @@ int nfp_flower_metadata_init(struct nfp_app *app, u64
host_ctx_count,
kmalloc_array(NFP_FLOWER_MASK_ENTRY_RS,
NFP_FLOWER_MASK_ELEMENT_RS, GFP_KERNEL);
if (!priv->mask_ids.mask_id_free_list.buf)
- goto err_free_stats_ctx_table;
+ goto err_free_merge_table;
priv->mask_ids.init_unallocated = NFP_FLOWER_MASK_ENTRY_RS - 1;
@@ -550,6 +560,8 @@ err_free_last_used:
kfree(priv->mask_ids.last_used);
err_free_mask_id:
kfree(priv->mask_ids.mask_id_free_list.buf);
+err_free_merge_table:
+ rhashtable_destroy(>merge_table);
err_free_stats_ctx_table:
rhashtable_destroy(>stats_ctx_table);
err_free_flow_table:
@@ -568,6 +580,8 @@ void nfp_flower_metadata_cleanup(struct nfp_app *app)
nfp_check_rhashtable_empty, NULL);
rhashtable_free_and_destroy(>stats_ctx_table,
nfp_check_rhashtable_empty, NULL);
+ rhashtable_free_and_destroy(>merge_table,
+ nfp_check_rhashtable_empty, NULL);
kvfree(priv->stats);
kfree(priv->mask_ids.mask_id_free_list.buf);
kfree(priv->mask_ids.last_used);
diff --git a/drivers/net/ethernet/netronome/nfp/flower/offload.c
b/drivers/net/ethernet/netronome/nfp/flower/offload.c
index 4dd3f8a5a9b8..f57e7f337012 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/offload.c
+++ b/drivers/net/ethernet/netronome/nfp/flower/offload.c
@@ -923,6 +923,8 @@ int nfp_flower_merge_offloaded_flows(struct nfp_app *app,
struct netlink_ext_ack *extack = NULL;
struct nfp_fl_payload *merge_flow;
struct nfp_fl_key_ls
[tip: sched/core] sched/fair: Ignore percpu threads for imbalance pulls
The following commit has been merged into the sched/core branch of tip:
Commit-ID: 9bcb959d05eeb564dfc9cac13a59843a4fb2edf2
Gitweb:
https://git.kernel.org/tip/9bcb959d05eeb564dfc9cac13a59843a4fb2edf2
Author:Lingutla Chandrasekhar
AuthorDate:Wed, 07 Apr 2021 23:06:26 +01:00
Committer: Peter Zijlstra
CommitterDate: Fri, 09 Apr 2021 18:02:20 +02:00
sched/fair: Ignore percpu threads for imbalance pulls
During load balance, LBF_SOME_PINNED will be set if any candidate task
cannot be detached due to CPU affinity constraints. This can result in
setting env->sd->parent->sgc->group_imbalance, which can lead to a group
being classified as group_imbalanced (rather than any of the other, lower
group_type) when balancing at a higher level.
In workloads involving a single task per CPU, LBF_SOME_PINNED can often be
set due to per-CPU kthreads being the only other runnable tasks on any
given rq. This results in changing the group classification during
load-balance at higher levels when in reality there is nothing that can be
done for this affinity constraint: per-CPU kthreads, as the name implies,
don't get to move around (modulo hotplug shenanigans).
It's not as clear for userspace tasks - a task could be in an N-CPU cpuset
with N-1 offline CPUs, making it an "accidental" per-CPU task rather than
an intended one. KTHREAD_IS_PER_CPU gives us an indisputable signal which
we can leverage here to not set LBF_SOME_PINNED.
Note that the aforementioned classification to group_imbalance (when
nothing can be done) is especially problematic on big.LITTLE systems, which
have a topology the likes of:
DIE [ ]
MC [][]
0 1 2 3
L L B B
arch_scale_cpu_capacity(L) < arch_scale_cpu_capacity(B)
Here, setting LBF_SOME_PINNED due to a per-CPU kthread when balancing at MC
level on CPUs [0-1] will subsequently prevent CPUs [2-3] from classifying
the [0-1] group as group_misfit_task when balancing at DIE level. Thus, if
CPUs [0-1] are running CPU-bound (misfit) tasks, ill-timed per-CPU kthreads
can significantly delay the upgmigration of said misfit tasks. Systems
relying on ASYM_PACKING are likely to face similar issues.
Signed-off-by: Lingutla Chandrasekhar
[Use kthread_is_per_cpu() rather than p->nr_cpus_allowed]
[Reword changelog]
Signed-off-by: Valentin Schneider
Signed-off-by: Peter Zijlstra (Intel)
Reviewed-by: Dietmar Eggemann
Reviewed-by: Vincent Guittot
Link:
https://lkml.kernel.org/r/20210407220628.3798191-2-valentin.schnei...@arm.com
---
kernel/sched/fair.c | 4
1 file changed, 4 insertions(+)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index bc34e35..1ad929b 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -7598,6 +7598,10 @@ int can_migrate_task(struct task_struct *p, struct
lb_env *env)
if (throttled_lb_pair(task_group(p), env->src_cpu, env->dst_cpu))
return 0;
+ /* Disregard pcpu kthreads; they are where they need to be. */
+ if ((p->flags & PF_KTHREAD) && kthread_is_per_cpu(p))
+ return 0;
+
if (!cpumask_test_cpu(env->dst_cpu, p->cpus_ptr)) {
int cpu;
[tip: sched/core] sched/fair: Ignore percpu threads for imbalance pulls
The following commit has been merged into the sched/core branch of tip:
Commit-ID: 8d25d10a4f5a5d87c062838358ab5b3ed7eaa131
Gitweb:
https://git.kernel.org/tip/8d25d10a4f5a5d87c062838358ab5b3ed7eaa131
Author:Lingutla Chandrasekhar
AuthorDate:Wed, 07 Apr 2021 23:06:26 +01:00
Committer: Peter Zijlstra
CommitterDate: Fri, 09 Apr 2021 13:52:10 +02:00
sched/fair: Ignore percpu threads for imbalance pulls
During load balance, LBF_SOME_PINNED will be set if any candidate task
cannot be detached due to CPU affinity constraints. This can result in
setting env->sd->parent->sgc->group_imbalance, which can lead to a group
being classified as group_imbalanced (rather than any of the other, lower
group_type) when balancing at a higher level.
In workloads involving a single task per CPU, LBF_SOME_PINNED can often be
set due to per-CPU kthreads being the only other runnable tasks on any
given rq. This results in changing the group classification during
load-balance at higher levels when in reality there is nothing that can be
done for this affinity constraint: per-CPU kthreads, as the name implies,
don't get to move around (modulo hotplug shenanigans).
It's not as clear for userspace tasks - a task could be in an N-CPU cpuset
with N-1 offline CPUs, making it an "accidental" per-CPU task rather than
an intended one. KTHREAD_IS_PER_CPU gives us an indisputable signal which
we can leverage here to not set LBF_SOME_PINNED.
Note that the aforementioned classification to group_imbalance (when
nothing can be done) is especially problematic on big.LITTLE systems, which
have a topology the likes of:
DIE [ ]
MC [][]
0 1 2 3
L L B B
arch_scale_cpu_capacity(L) < arch_scale_cpu_capacity(B)
Here, setting LBF_SOME_PINNED due to a per-CPU kthread when balancing at MC
level on CPUs [0-1] will subsequently prevent CPUs [2-3] from classifying
the [0-1] group as group_misfit_task when balancing at DIE level. Thus, if
CPUs [0-1] are running CPU-bound (misfit) tasks, ill-timed per-CPU kthreads
can significantly delay the upgmigration of said misfit tasks. Systems
relying on ASYM_PACKING are likely to face similar issues.
Signed-off-by: Lingutla Chandrasekhar
[Use kthread_is_per_cpu() rather than p->nr_cpus_allowed]
[Reword changelog]
Signed-off-by: Valentin Schneider
Signed-off-by: Peter Zijlstra (Intel)
Reviewed-by: Dietmar Eggemann
Reviewed-by: Vincent Guittot
Link:
https://lkml.kernel.org/r/20210407220628.3798191-2-valentin.schnei...@arm.com
---
kernel/sched/fair.c | 4
1 file changed, 4 insertions(+)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index d0bd861..d10e33d 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -7598,6 +7598,10 @@ int can_migrate_task(struct task_struct *p, struct
lb_env *env)
if (throttled_lb_pair(task_group(p), env->src_cpu, env->dst_cpu))
return 0;
+ /* Disregard pcpu kthreads; they are where they need to be. */
+ if ((p->flags & PF_KTHREAD) && kthread_is_per_cpu(p))
+ return 0;
+
if (!cpumask_test_cpu(env->dst_cpu, p->cpus_ptr)) {
int cpu;
[tip: sched/core] sched/fair: Ignore percpu threads for imbalance pulls
The following commit has been merged into the sched/core branch of tip:
Commit-ID: 29b628b521119c0dfe151da302e11018cb32db4f
Gitweb:
https://git.kernel.org/tip/29b628b521119c0dfe151da302e11018cb32db4f
Author:Lingutla Chandrasekhar
AuthorDate:Wed, 07 Apr 2021 23:06:26 +01:00
Committer: Peter Zijlstra
CommitterDate: Thu, 08 Apr 2021 23:09:44 +02:00
sched/fair: Ignore percpu threads for imbalance pulls
During load balance, LBF_SOME_PINNED will be set if any candidate task
cannot be detached due to CPU affinity constraints. This can result in
setting env->sd->parent->sgc->group_imbalance, which can lead to a group
being classified as group_imbalanced (rather than any of the other, lower
group_type) when balancing at a higher level.
In workloads involving a single task per CPU, LBF_SOME_PINNED can often be
set due to per-CPU kthreads being the only other runnable tasks on any
given rq. This results in changing the group classification during
load-balance at higher levels when in reality there is nothing that can be
done for this affinity constraint: per-CPU kthreads, as the name implies,
don't get to move around (modulo hotplug shenanigans).
It's not as clear for userspace tasks - a task could be in an N-CPU cpuset
with N-1 offline CPUs, making it an "accidental" per-CPU task rather than
an intended one. KTHREAD_IS_PER_CPU gives us an indisputable signal which
we can leverage here to not set LBF_SOME_PINNED.
Note that the aforementioned classification to group_imbalance (when
nothing can be done) is especially problematic on big.LITTLE systems, which
have a topology the likes of:
DIE [ ]
MC [][]
0 1 2 3
L L B B
arch_scale_cpu_capacity(L) < arch_scale_cpu_capacity(B)
Here, setting LBF_SOME_PINNED due to a per-CPU kthread when balancing at MC
level on CPUs [0-1] will subsequently prevent CPUs [2-3] from classifying
the [0-1] group as group_misfit_task when balancing at DIE level. Thus, if
CPUs [0-1] are running CPU-bound (misfit) tasks, ill-timed per-CPU kthreads
can significantly delay the upgmigration of said misfit tasks. Systems
relying on ASYM_PACKING are likely to face similar issues.
[Use kthread_is_per_cpu() rather than p->nr_cpus_allowed]
[Reword changelog]
Signed-off-by: Valentin Schneider
Signed-off-by: Lingutla Chandrasekhar
Signed-off-by: Peter Zijlstra (Intel)
Reviewed-by: Dietmar Eggemann
Reviewed-by: Vincent Guittot
Link:
https://lkml.kernel.org/r/20210407220628.3798191-2-valentin.schnei...@arm.com
---
kernel/sched/fair.c | 4
1 file changed, 4 insertions(+)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index d0bd861..d10e33d 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -7598,6 +7598,10 @@ int can_migrate_task(struct task_struct *p, struct
lb_env *env)
if (throttled_lb_pair(task_group(p), env->src_cpu, env->dst_cpu))
return 0;
+ /* Disregard pcpu kthreads; they are where they need to be. */
+ if ((p->flags & PF_KTHREAD) && kthread_is_per_cpu(p))
+ return 0;
+
if (!cpumask_test_cpu(env->dst_cpu, p->cpus_ptr)) {
int cpu;
[PATCH 5.11 34/45] cifs: Silently ignore unknown oplock break handle
From: Vincent Whitchurch [ Upstream commit 219481a8f90ec3a5eed9638fb35609e4b1aeece7 ] Make SMB2 not print out an error when an oplock break is received for an unknown handle, similar to SMB1. The debug message which is printed for these unknown handles may also be misleading, so fix that too. The SMB2 lease break path is not affected by this patch. Without this, a program which writes to a file from one thread, and opens, reads, and writes the same file from another thread triggers the below errors several times a minute when run against a Samba server configured with "smb2 leases = no". CIFS: VFS: \\192.168.0.1 No task to wake, unknown frame received! NumMids 2 : 424d53fe 0040 0012 .SMB@... 0010: 0001 0020: 0030: Signed-off-by: Vincent Whitchurch Reviewed-by: Tom Talpey Reviewed-by: Paulo Alcantara (SUSE) Signed-off-by: Steve French Signed-off-by: Sasha Levin --- fs/cifs/smb2misc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/cifs/smb2misc.c b/fs/cifs/smb2misc.c index d9073b569e17..53fb751bf210 100644 --- a/fs/cifs/smb2misc.c +++ b/fs/cifs/smb2misc.c @@ -754,8 +754,8 @@ smb2_is_valid_oplock_break(char *buffer, struct TCP_Server_Info *server) } } spin_unlock(_tcp_ses_lock); - cifs_dbg(FYI, "Can not process oplock break for non-existent connection\n"); - return false; + cifs_dbg(FYI, "No file id matched, oplock break ignored\n"); + return true; } void -- 2.30.2
[PATCH 5.11 26/45] platform/x86: intel_pmc_core: Ignore GBE LTR on Tiger Lake platforms
From: David E. Box
[ Upstream commit d1635448f1105e549b4041aab930dbc6945fc635 ]
Due to a HW limitation, the Latency Tolerance Reporting (LTR) value
programmed in the Tiger Lake GBE controller is not large enough to allow
the platform to enter Package C10, which in turn prevents the platform from
achieving its low power target during suspend-to-idle. Ignore the GBE LTR
value on Tiger Lake. LTR ignore functionality is currently performed solely
by a debugfs write call. Split out the LTR code into its own function that
can be called by both the debugfs writer and by this work around.
Signed-off-by: David E. Box
Reviewed-by: Sasha Neftin
Cc: intel-wired-...@lists.osuosl.org
Reviewed-by: Rajneesh Bhardwaj
Link:
https://lore.kernel.org/r/20210319201844.3305399-2-david.e@linux.intel.com
Signed-off-by: Hans de Goede
Signed-off-by: Sasha Levin
---
drivers/platform/x86/intel_pmc_core.c | 50 +++
1 file changed, 35 insertions(+), 15 deletions(-)
diff --git a/drivers/platform/x86/intel_pmc_core.c
b/drivers/platform/x86/intel_pmc_core.c
index ee2f757515b0..b5888aeb4bcf 100644
--- a/drivers/platform/x86/intel_pmc_core.c
+++ b/drivers/platform/x86/intel_pmc_core.c
@@ -863,34 +863,45 @@ static int pmc_core_pll_show(struct seq_file *s, void
*unused)
}
DEFINE_SHOW_ATTRIBUTE(pmc_core_pll);
-static ssize_t pmc_core_ltr_ignore_write(struct file *file,
-const char __user *userbuf,
-size_t count, loff_t *ppos)
+static int pmc_core_send_ltr_ignore(u32 value)
{
struct pmc_dev *pmcdev =
const struct pmc_reg_map *map = pmcdev->map;
- u32 val, buf_size, fd;
- int err;
-
- buf_size = count < 64 ? count : 64;
-
- err = kstrtou32_from_user(userbuf, buf_size, 10, );
- if (err)
- return err;
+ u32 reg;
+ int err = 0;
mutex_lock(>lock);
- if (val > map->ltr_ignore_max) {
+ if (value > map->ltr_ignore_max) {
err = -EINVAL;
goto out_unlock;
}
- fd = pmc_core_reg_read(pmcdev, map->ltr_ignore_offset);
- fd |= (1U << val);
- pmc_core_reg_write(pmcdev, map->ltr_ignore_offset, fd);
+ reg = pmc_core_reg_read(pmcdev, map->ltr_ignore_offset);
+ reg |= BIT(value);
+ pmc_core_reg_write(pmcdev, map->ltr_ignore_offset, reg);
out_unlock:
mutex_unlock(>lock);
+
+ return err;
+}
+
+static ssize_t pmc_core_ltr_ignore_write(struct file *file,
+const char __user *userbuf,
+size_t count, loff_t *ppos)
+{
+ u32 buf_size, value;
+ int err;
+
+ buf_size = min_t(u32, count, 64);
+
+ err = kstrtou32_from_user(userbuf, buf_size, 10, );
+ if (err)
+ return err;
+
+ err = pmc_core_send_ltr_ignore(value);
+
return err == 0 ? count : err;
}
@@ -1244,6 +1255,15 @@ static int pmc_core_probe(struct platform_device *pdev)
pmcdev->pmc_xram_read_bit = pmc_core_check_read_lock_bit();
dmi_check_system(pmc_core_dmi_table);
+ /*
+* On TGL, due to a hardware limitation, the GBE LTR blocks PC10 when
+ * a cable is attached. Tell the PMC to ignore it.
+*/
+ if (pmcdev->map == _reg_map) {
+ dev_dbg(>dev, "ignoring GBE LTR\n");
+ pmc_core_send_ltr_ignore(3);
+ }
+
pmc_core_dbgfs_register(pmcdev);
device_initialized = true;
--
2.30.2
[PATCH 5.10 29/41] cifs: Silently ignore unknown oplock break handle
From: Vincent Whitchurch [ Upstream commit 219481a8f90ec3a5eed9638fb35609e4b1aeece7 ] Make SMB2 not print out an error when an oplock break is received for an unknown handle, similar to SMB1. The debug message which is printed for these unknown handles may also be misleading, so fix that too. The SMB2 lease break path is not affected by this patch. Without this, a program which writes to a file from one thread, and opens, reads, and writes the same file from another thread triggers the below errors several times a minute when run against a Samba server configured with "smb2 leases = no". CIFS: VFS: \\192.168.0.1 No task to wake, unknown frame received! NumMids 2 : 424d53fe 0040 0012 .SMB@... 0010: 0001 0020: 0030: Signed-off-by: Vincent Whitchurch Reviewed-by: Tom Talpey Reviewed-by: Paulo Alcantara (SUSE) Signed-off-by: Steve French Signed-off-by: Sasha Levin --- fs/cifs/smb2misc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/cifs/smb2misc.c b/fs/cifs/smb2misc.c index db22d686c61f..be3df90bb2bc 100644 --- a/fs/cifs/smb2misc.c +++ b/fs/cifs/smb2misc.c @@ -745,8 +745,8 @@ smb2_is_valid_oplock_break(char *buffer, struct TCP_Server_Info *server) } } spin_unlock(_tcp_ses_lock); - cifs_dbg(FYI, "Can not process oplock break for non-existent connection\n"); - return false; + cifs_dbg(FYI, "No file id matched, oplock break ignored\n"); + return true; } void -- 2.30.2
[PATCH 5.10 22/41] platform/x86: intel_pmc_core: Ignore GBE LTR on Tiger Lake platforms
From: David E. Box
[ Upstream commit d1635448f1105e549b4041aab930dbc6945fc635 ]
Due to a HW limitation, the Latency Tolerance Reporting (LTR) value
programmed in the Tiger Lake GBE controller is not large enough to allow
the platform to enter Package C10, which in turn prevents the platform from
achieving its low power target during suspend-to-idle. Ignore the GBE LTR
value on Tiger Lake. LTR ignore functionality is currently performed solely
by a debugfs write call. Split out the LTR code into its own function that
can be called by both the debugfs writer and by this work around.
Signed-off-by: David E. Box
Reviewed-by: Sasha Neftin
Cc: intel-wired-...@lists.osuosl.org
Reviewed-by: Rajneesh Bhardwaj
Link:
https://lore.kernel.org/r/20210319201844.3305399-2-david.e@linux.intel.com
Signed-off-by: Hans de Goede
Signed-off-by: Sasha Levin
---
drivers/platform/x86/intel_pmc_core.c | 50 +++
1 file changed, 35 insertions(+), 15 deletions(-)
diff --git a/drivers/platform/x86/intel_pmc_core.c
b/drivers/platform/x86/intel_pmc_core.c
index 3e5fe66333f1..e06b36e87a33 100644
--- a/drivers/platform/x86/intel_pmc_core.c
+++ b/drivers/platform/x86/intel_pmc_core.c
@@ -863,34 +863,45 @@ static int pmc_core_pll_show(struct seq_file *s, void
*unused)
}
DEFINE_SHOW_ATTRIBUTE(pmc_core_pll);
-static ssize_t pmc_core_ltr_ignore_write(struct file *file,
-const char __user *userbuf,
-size_t count, loff_t *ppos)
+static int pmc_core_send_ltr_ignore(u32 value)
{
struct pmc_dev *pmcdev =
const struct pmc_reg_map *map = pmcdev->map;
- u32 val, buf_size, fd;
- int err;
-
- buf_size = count < 64 ? count : 64;
-
- err = kstrtou32_from_user(userbuf, buf_size, 10, );
- if (err)
- return err;
+ u32 reg;
+ int err = 0;
mutex_lock(>lock);
- if (val > map->ltr_ignore_max) {
+ if (value > map->ltr_ignore_max) {
err = -EINVAL;
goto out_unlock;
}
- fd = pmc_core_reg_read(pmcdev, map->ltr_ignore_offset);
- fd |= (1U << val);
- pmc_core_reg_write(pmcdev, map->ltr_ignore_offset, fd);
+ reg = pmc_core_reg_read(pmcdev, map->ltr_ignore_offset);
+ reg |= BIT(value);
+ pmc_core_reg_write(pmcdev, map->ltr_ignore_offset, reg);
out_unlock:
mutex_unlock(>lock);
+
+ return err;
+}
+
+static ssize_t pmc_core_ltr_ignore_write(struct file *file,
+const char __user *userbuf,
+size_t count, loff_t *ppos)
+{
+ u32 buf_size, value;
+ int err;
+
+ buf_size = min_t(u32, count, 64);
+
+ err = kstrtou32_from_user(userbuf, buf_size, 10, );
+ if (err)
+ return err;
+
+ err = pmc_core_send_ltr_ignore(value);
+
return err == 0 ? count : err;
}
@@ -1244,6 +1255,15 @@ static int pmc_core_probe(struct platform_device *pdev)
pmcdev->pmc_xram_read_bit = pmc_core_check_read_lock_bit();
dmi_check_system(pmc_core_dmi_table);
+ /*
+* On TGL, due to a hardware limitation, the GBE LTR blocks PC10 when
+ * a cable is attached. Tell the PMC to ignore it.
+*/
+ if (pmcdev->map == _reg_map) {
+ dev_dbg(>dev, "ignoring GBE LTR\n");
+ pmc_core_send_ltr_ignore(3);
+ }
+
pmc_core_dbgfs_register(pmcdev);
device_initialized = true;
--
2.30.2
[PATCH 5.4 18/23] cifs: Silently ignore unknown oplock break handle
From: Vincent Whitchurch [ Upstream commit 219481a8f90ec3a5eed9638fb35609e4b1aeece7 ] Make SMB2 not print out an error when an oplock break is received for an unknown handle, similar to SMB1. The debug message which is printed for these unknown handles may also be misleading, so fix that too. The SMB2 lease break path is not affected by this patch. Without this, a program which writes to a file from one thread, and opens, reads, and writes the same file from another thread triggers the below errors several times a minute when run against a Samba server configured with "smb2 leases = no". CIFS: VFS: \\192.168.0.1 No task to wake, unknown frame received! NumMids 2 : 424d53fe 0040 0012 .SMB@... 0010: 0001 0020: 0030: Signed-off-by: Vincent Whitchurch Reviewed-by: Tom Talpey Reviewed-by: Paulo Alcantara (SUSE) Signed-off-by: Steve French Signed-off-by: Sasha Levin --- fs/cifs/smb2misc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/cifs/smb2misc.c b/fs/cifs/smb2misc.c index 7d875a47d022..7177720e822e 100644 --- a/fs/cifs/smb2misc.c +++ b/fs/cifs/smb2misc.c @@ -738,8 +738,8 @@ smb2_is_valid_oplock_break(char *buffer, struct TCP_Server_Info *server) } } spin_unlock(_tcp_ses_lock); - cifs_dbg(FYI, "Can not process oplock break for non-existent connection\n"); - return false; + cifs_dbg(FYI, "No file id matched, oplock break ignored\n"); + return true; } void -- 2.30.2
[PATCH 4.19 14/18] cifs: Silently ignore unknown oplock break handle
From: Vincent Whitchurch [ Upstream commit 219481a8f90ec3a5eed9638fb35609e4b1aeece7 ] Make SMB2 not print out an error when an oplock break is received for an unknown handle, similar to SMB1. The debug message which is printed for these unknown handles may also be misleading, so fix that too. The SMB2 lease break path is not affected by this patch. Without this, a program which writes to a file from one thread, and opens, reads, and writes the same file from another thread triggers the below errors several times a minute when run against a Samba server configured with "smb2 leases = no". CIFS: VFS: \\192.168.0.1 No task to wake, unknown frame received! NumMids 2 : 424d53fe 0040 0012 .SMB@... 0010: 0001 0020: 0030: Signed-off-by: Vincent Whitchurch Reviewed-by: Tom Talpey Reviewed-by: Paulo Alcantara (SUSE) Signed-off-by: Steve French Signed-off-by: Sasha Levin --- fs/cifs/smb2misc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/cifs/smb2misc.c b/fs/cifs/smb2misc.c index 7d875a47d022..7177720e822e 100644 --- a/fs/cifs/smb2misc.c +++ b/fs/cifs/smb2misc.c @@ -738,8 +738,8 @@ smb2_is_valid_oplock_break(char *buffer, struct TCP_Server_Info *server) } } spin_unlock(_tcp_ses_lock); - cifs_dbg(FYI, "Can not process oplock break for non-existent connection\n"); - return false; + cifs_dbg(FYI, "No file id matched, oplock break ignored\n"); + return true; } void -- 2.30.2
[PATCH 4.14 11/14] cifs: Silently ignore unknown oplock break handle
From: Vincent Whitchurch [ Upstream commit 219481a8f90ec3a5eed9638fb35609e4b1aeece7 ] Make SMB2 not print out an error when an oplock break is received for an unknown handle, similar to SMB1. The debug message which is printed for these unknown handles may also be misleading, so fix that too. The SMB2 lease break path is not affected by this patch. Without this, a program which writes to a file from one thread, and opens, reads, and writes the same file from another thread triggers the below errors several times a minute when run against a Samba server configured with "smb2 leases = no". CIFS: VFS: \\192.168.0.1 No task to wake, unknown frame received! NumMids 2 : 424d53fe 0040 0012 .SMB@... 0010: 0001 0020: 0030: Signed-off-by: Vincent Whitchurch Reviewed-by: Tom Talpey Reviewed-by: Paulo Alcantara (SUSE) Signed-off-by: Steve French Signed-off-by: Sasha Levin --- fs/cifs/smb2misc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/cifs/smb2misc.c b/fs/cifs/smb2misc.c index 0c6e5450ff76..80339e318294 100644 --- a/fs/cifs/smb2misc.c +++ b/fs/cifs/smb2misc.c @@ -656,8 +656,8 @@ smb2_is_valid_oplock_break(char *buffer, struct TCP_Server_Info *server) } } spin_unlock(_tcp_ses_lock); - cifs_dbg(FYI, "Can not process oplock break for non-existent connection\n"); - return false; + cifs_dbg(FYI, "No file id matched, oplock break ignored\n"); + return true; } void -- 2.30.2
[PATCH 4.9 09/13] cifs: Silently ignore unknown oplock break handle
From: Vincent Whitchurch [ Upstream commit 219481a8f90ec3a5eed9638fb35609e4b1aeece7 ] Make SMB2 not print out an error when an oplock break is received for an unknown handle, similar to SMB1. The debug message which is printed for these unknown handles may also be misleading, so fix that too. The SMB2 lease break path is not affected by this patch. Without this, a program which writes to a file from one thread, and opens, reads, and writes the same file from another thread triggers the below errors several times a minute when run against a Samba server configured with "smb2 leases = no". CIFS: VFS: \\192.168.0.1 No task to wake, unknown frame received! NumMids 2 : 424d53fe 0040 0012 .SMB@... 0010: 0001 0020: 0030: Signed-off-by: Vincent Whitchurch Reviewed-by: Tom Talpey Reviewed-by: Paulo Alcantara (SUSE) Signed-off-by: Steve French Signed-off-by: Sasha Levin --- fs/cifs/smb2misc.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) --- a/fs/cifs/smb2misc.c +++ b/fs/cifs/smb2misc.c @@ -651,8 +651,8 @@ smb2_is_valid_oplock_break(char *buffer, } } spin_unlock(_tcp_ses_lock); - cifs_dbg(FYI, "Can not process oplock break for non-existent connection\n"); - return false; + cifs_dbg(FYI, "No file id matched, oplock break ignored\n"); + return true; } void
[PATCH 4.4 07/20] cifs: Silently ignore unknown oplock break handle
From: Vincent Whitchurch [ Upstream commit 219481a8f90ec3a5eed9638fb35609e4b1aeece7 ] Make SMB2 not print out an error when an oplock break is received for an unknown handle, similar to SMB1. The debug message which is printed for these unknown handles may also be misleading, so fix that too. The SMB2 lease break path is not affected by this patch. Without this, a program which writes to a file from one thread, and opens, reads, and writes the same file from another thread triggers the below errors several times a minute when run against a Samba server configured with "smb2 leases = no". CIFS: VFS: \\192.168.0.1 No task to wake, unknown frame received! NumMids 2 : 424d53fe 0040 0012 .SMB@... 0010: 0001 0020: 0030: Signed-off-by: Vincent Whitchurch Reviewed-by: Tom Talpey Reviewed-by: Paulo Alcantara (SUSE) Signed-off-by: Steve French Signed-off-by: Sasha Levin --- fs/cifs/smb2misc.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) --- a/fs/cifs/smb2misc.c +++ b/fs/cifs/smb2misc.c @@ -633,8 +633,8 @@ smb2_is_valid_oplock_break(char *buffer, } } spin_unlock(_tcp_ses_lock); - cifs_dbg(FYI, "Can not process oplock break for non-existent connection\n"); - return false; + cifs_dbg(FYI, "No file id matched, oplock break ignored\n"); + return true; } void
[PATCH v5 1/3] sched/fair: Ignore percpu threads for imbalance pulls
From: Lingutla Chandrasekhar
During load balance, LBF_SOME_PINNED will be set if any candidate task
cannot be detached due to CPU affinity constraints. This can result in
setting env->sd->parent->sgc->group_imbalance, which can lead to a group
being classified as group_imbalanced (rather than any of the other, lower
group_type) when balancing at a higher level.
In workloads involving a single task per CPU, LBF_SOME_PINNED can often be
set due to per-CPU kthreads being the only other runnable tasks on any
given rq. This results in changing the group classification during
load-balance at higher levels when in reality there is nothing that can be
done for this affinity constraint: per-CPU kthreads, as the name implies,
don't get to move around (modulo hotplug shenanigans).
It's not as clear for userspace tasks - a task could be in an N-CPU cpuset
with N-1 offline CPUs, making it an "accidental" per-CPU task rather than
an intended one. KTHREAD_IS_PER_CPU gives us an indisputable signal which
we can leverage here to not set LBF_SOME_PINNED.
Note that the aforementioned classification to group_imbalance (when
nothing can be done) is especially problematic on big.LITTLE systems, which
have a topology the likes of:
DIE [ ]
MC [][]
0 1 2 3
L L B B
arch_scale_cpu_capacity(L) < arch_scale_cpu_capacity(B)
Here, setting LBF_SOME_PINNED due to a per-CPU kthread when balancing at MC
level on CPUs [0-1] will subsequently prevent CPUs [2-3] from classifying
the [0-1] group as group_misfit_task when balancing at DIE level. Thus, if
CPUs [0-1] are running CPU-bound (misfit) tasks, ill-timed per-CPU kthreads
can significantly delay the upgmigration of said misfit tasks. Systems
relying on ASYM_PACKING are likely to face similar issues.
Signed-off-by: Lingutla Chandrasekhar
[Use kthread_is_per_cpu() rather than p->nr_cpus_allowed]
[Reword changelog]
Signed-off-by: Valentin Schneider
Reviewed-by: Vincent Guittot
Reviewed-by: Dietmar Eggemann
---
kernel/sched/fair.c | 4
1 file changed, 4 insertions(+)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 6d73bdbb2d40..04d5e14fa261 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -7567,6 +7567,10 @@ int can_migrate_task(struct task_struct *p, struct
lb_env *env)
if (throttled_lb_pair(task_group(p), env->src_cpu, env->dst_cpu))
return 0;
+ /* Disregard pcpu kthreads; they are where they need to be. */
+ if ((p->flags & PF_KTHREAD) && kthread_is_per_cpu(p))
+ return 0;
+
if (!cpumask_test_cpu(env->dst_cpu, p->cpus_ptr)) {
int cpu;
--
2.25.1
[PATCH v5 06/12] evm: Ignore INTEGRITY_NOLABEL/INTEGRITY_NOXATTRS if conditions are safe
When a file is being created, LSMs can set the initial label with the
inode_init_security hook. If no HMAC key is loaded, the new file will have
LSM xattrs but not the HMAC. It is also possible that the file remains
without protected xattrs after creation if no active LSM provided it.
Unfortunately, EVM will deny any further metadata operation on new files,
as evm_protect_xattr() will always return the INTEGRITY_NOLABEL error, or
INTEGRITY_NOXATTRS if no protected xattrs exist. This would limit the
usability of EVM when only a public key is loaded, as commands such as cp
or tar with the option to preserve xattrs won't work.
This patch ignores these errors when they won't be an issue, if no HMAC key
is loaded and cannot be loaded in the future (which can be enforced by
setting the EVM_SETUP_COMPLETE initialization flag).
Signed-off-by: Roberto Sassu
---
security/integrity/evm/evm_main.c | 23 ++-
1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/security/integrity/evm/evm_main.c
b/security/integrity/evm/evm_main.c
index 998818283fda..6556e8c22da9 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -90,6 +90,24 @@ static bool evm_key_loaded(void)
return (bool)(evm_initialized & EVM_KEY_MASK);
}
+/*
+ * Ignoring INTEGRITY_NOLABEL/INTEGRITY_NOXATTRS is safe if no HMAC key
+ * is loaded and the EVM_SETUP_COMPLETE initialization flag is set.
+ */
+static bool evm_ignore_error_safe(enum integrity_status evm_status)
+{
+ if (evm_initialized & EVM_INIT_HMAC)
+ return false;
+
+ if (!(evm_initialized & EVM_SETUP_COMPLETE))
+ return false;
+
+ if (evm_status != INTEGRITY_NOLABEL && evm_status != INTEGRITY_NOXATTRS)
+ return false;
+
+ return true;
+}
+
static int evm_find_protected_xattrs(struct dentry *dentry)
{
struct inode *inode = d_backing_inode(dentry);
@@ -354,6 +372,8 @@ static int evm_protect_xattr(struct dentry *dentry, const
char *xattr_name,
-EPERM, 0);
}
out:
+ if (evm_ignore_error_safe(evm_status))
+ return 0;
if (evm_status != INTEGRITY_PASS)
integrity_audit_msg(AUDIT_INTEGRITY_METADATA,
d_backing_inode(dentry),
dentry->d_name.name, "appraise_metadata",
@@ -515,7 +535,8 @@ int evm_inode_setattr(struct dentry *dentry, struct iattr
*attr)
return 0;
evm_status = evm_verify_current_integrity(dentry);
if ((evm_status == INTEGRITY_PASS) ||
- (evm_status == INTEGRITY_NOXATTRS))
+ (evm_status == INTEGRITY_NOXATTRS) ||
+ (evm_ignore_error_safe(evm_status)))
return 0;
integrity_audit_msg(AUDIT_INTEGRITY_METADATA, d_backing_inode(dentry),
dentry->d_name.name, "appraise_metadata",
--
2.26.2
[PATCH 22/24] USB: serial: ftdi_sio: ignore baud_base changes
The TIOCSSERIAL error handling is inconsistent at best, but drivers tend
to ignore requests to change parameters which cannot be changed rather
than return an error.
The FTDI driver ignores change requests for all immutable parameters but
baud_base so return success also in this case for consistency.
Signed-off-by: Johan Hovold
---
drivers/usb/serial/ftdi_sio.c | 4
1 file changed, 4 deletions(-)
diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
index 16d3e50487e6..3fd7875200b9 100644
--- a/drivers/usb/serial/ftdi_sio.c
+++ b/drivers/usb/serial/ftdi_sio.c
@@ -1509,10 +1509,6 @@ static int set_serial_info(struct tty_struct *tty,
goto check_and_exit;
}
- if (ss->baud_base != priv->baud_base) {
- mutex_unlock(>cfg_lock);
- return -EINVAL;
- }
/* Make the changes - these are privileged changes! */
--
2.26.3
Re: [PATCH v4 1/3] sched/fair: Ignore percpu threads for imbalance pulls
On 06/04/21 17:35, Dietmar Eggemann wrote: > On 01/04/2021 21:30, Valentin Schneider wrote: >> From: Lingutla Chandrasekhar >> >> During load balance, LBF_SOME_PINNED will bet set if any candidate task > > nitpick; s/bet/be ? > Yes indeed... > [...] > > Reviewed-by: Dietmar Eggemann
Re: [PATCH v4 1/3] sched/fair: Ignore percpu threads for imbalance pulls
On 01/04/2021 21:30, Valentin Schneider wrote: > From: Lingutla Chandrasekhar > > During load balance, LBF_SOME_PINNED will bet set if any candidate task nitpick; s/bet/be ? [...] Reviewed-by: Dietmar Eggemann
[PATCH AUTOSEL 5.4 13/13] block: don't ignore REQ_NOWAIT for direct IO
From: Pavel Begunkov [ Upstream commit f8b78caf21d5bc3fcfc40c18898f9d52ed1451a5 ] If IOCB_NOWAIT is set on submission, then that needs to get propagated to REQ_NOWAIT on the block side. Otherwise we completely lose this information, and any issuer of IOCB_NOWAIT IO will potentially end up blocking on eg request allocation on the storage side. Signed-off-by: Pavel Begunkov Signed-off-by: Jens Axboe Signed-off-by: Sasha Levin --- fs/block_dev.c | 4 1 file changed, 4 insertions(+) diff --git a/fs/block_dev.c b/fs/block_dev.c index 79272cdbe827..bd93563477a4 100644 --- a/fs/block_dev.c +++ b/fs/block_dev.c @@ -246,6 +246,8 @@ __blkdev_direct_IO_simple(struct kiocb *iocb, struct iov_iter *iter, bio.bi_opf = dio_bio_write_op(iocb); task_io_account_write(ret); } + if (iocb->ki_flags & IOCB_NOWAIT) + bio.bi_opf |= REQ_NOWAIT; if (iocb->ki_flags & IOCB_HIPRI) bio_set_polled(, iocb); @@ -399,6 +401,8 @@ __blkdev_direct_IO(struct kiocb *iocb, struct iov_iter *iter, int nr_pages) bio->bi_opf = dio_bio_write_op(iocb); task_io_account_write(bio->bi_iter.bi_size); } + if (iocb->ki_flags & IOCB_NOWAIT) + bio->bi_opf |= REQ_NOWAIT; dio->size += bio->bi_iter.bi_size; pos += bio->bi_iter.bi_size; -- 2.30.2
[PATCH AUTOSEL 5.10 22/22] block: don't ignore REQ_NOWAIT for direct IO
From: Pavel Begunkov [ Upstream commit f8b78caf21d5bc3fcfc40c18898f9d52ed1451a5 ] If IOCB_NOWAIT is set on submission, then that needs to get propagated to REQ_NOWAIT on the block side. Otherwise we completely lose this information, and any issuer of IOCB_NOWAIT IO will potentially end up blocking on eg request allocation on the storage side. Signed-off-by: Pavel Begunkov Signed-off-by: Jens Axboe Signed-off-by: Sasha Levin --- fs/block_dev.c | 4 1 file changed, 4 insertions(+) diff --git a/fs/block_dev.c b/fs/block_dev.c index fe201b757baa..1b6a34fd1fef 100644 --- a/fs/block_dev.c +++ b/fs/block_dev.c @@ -280,6 +280,8 @@ __blkdev_direct_IO_simple(struct kiocb *iocb, struct iov_iter *iter, bio.bi_opf = dio_bio_write_op(iocb); task_io_account_write(ret); } + if (iocb->ki_flags & IOCB_NOWAIT) + bio.bi_opf |= REQ_NOWAIT; if (iocb->ki_flags & IOCB_HIPRI) bio_set_polled(, iocb); @@ -433,6 +435,8 @@ __blkdev_direct_IO(struct kiocb *iocb, struct iov_iter *iter, int nr_pages) bio->bi_opf = dio_bio_write_op(iocb); task_io_account_write(bio->bi_iter.bi_size); } + if (iocb->ki_flags & IOCB_NOWAIT) + bio->bi_opf |= REQ_NOWAIT; dio->size += bio->bi_iter.bi_size; pos += bio->bi_iter.bi_size; -- 2.30.2
[PATCH AUTOSEL 5.11 22/22] block: don't ignore REQ_NOWAIT for direct IO
From: Pavel Begunkov [ Upstream commit f8b78caf21d5bc3fcfc40c18898f9d52ed1451a5 ] If IOCB_NOWAIT is set on submission, then that needs to get propagated to REQ_NOWAIT on the block side. Otherwise we completely lose this information, and any issuer of IOCB_NOWAIT IO will potentially end up blocking on eg request allocation on the storage side. Signed-off-by: Pavel Begunkov Signed-off-by: Jens Axboe Signed-off-by: Sasha Levin --- fs/block_dev.c | 4 1 file changed, 4 insertions(+) diff --git a/fs/block_dev.c b/fs/block_dev.c index c33151020bcd..06b422fa67a4 100644 --- a/fs/block_dev.c +++ b/fs/block_dev.c @@ -276,6 +276,8 @@ __blkdev_direct_IO_simple(struct kiocb *iocb, struct iov_iter *iter, bio.bi_opf = dio_bio_write_op(iocb); task_io_account_write(ret); } + if (iocb->ki_flags & IOCB_NOWAIT) + bio.bi_opf |= REQ_NOWAIT; if (iocb->ki_flags & IOCB_HIPRI) bio_set_polled(, iocb); @@ -429,6 +431,8 @@ __blkdev_direct_IO(struct kiocb *iocb, struct iov_iter *iter, int nr_pages) bio->bi_opf = dio_bio_write_op(iocb); task_io_account_write(bio->bi_iter.bi_size); } + if (iocb->ki_flags & IOCB_NOWAIT) + bio->bi_opf |= REQ_NOWAIT; dio->size += bio->bi_iter.bi_size; pos += bio->bi_iter.bi_size; -- 2.30.2
[PATCH 5.11 131/152] USB: quirks: ignore remote wake-up on Fibocom L850-GL LTE modem
From: Vincent Palatin
commit 0bd860493f81eb2a46173f6f5e44cc38331c8dbd upstream.
This LTE modem (M.2 card) has a bug in its power management:
there is some kind of race condition for U3 wake-up between the host and
the device. The modem firmware sometimes crashes/locks when both events
happen at the same time and the modem fully drops off the USB bus (and
sometimes re-enumerates, sometimes just gets stuck until the next
reboot).
Tested with the modem wired to the XHCI controller on an AMD 3015Ce
platform. Without the patch, the modem dropped of the USB bus 5 times in
3 days. With the quirk, it stayed connected for a week while the
'runtime_suspended_time' counter incremented as excepted.
Signed-off-by: Vincent Palatin
Link: https://lore.kernel.org/r/20210319124802.2315195-1-vpala...@chromium.org
Cc: stable
Signed-off-by: Greg Kroah-Hartman
---
drivers/usb/core/quirks.c |4
1 file changed, 4 insertions(+)
--- a/drivers/usb/core/quirks.c
+++ b/drivers/usb/core/quirks.c
@@ -498,6 +498,10 @@ static const struct usb_device_id usb_qu
/* DJI CineSSD */
{ USB_DEVICE(0x2ca3, 0x0031), .driver_info = USB_QUIRK_NO_LPM },
+ /* Fibocom L850-GL LTE Modem */
+ { USB_DEVICE(0x2cb7, 0x0007), .driver_info =
+ USB_QUIRK_IGNORE_REMOTE_WAKEUP },
+
/* INTEL VALUE SSD */
{ USB_DEVICE(0x8086, 0xf1a5), .driver_info = USB_QUIRK_RESET_RESUME },
[PATCH 5.10 106/126] USB: quirks: ignore remote wake-up on Fibocom L850-GL LTE modem
From: Vincent Palatin
commit 0bd860493f81eb2a46173f6f5e44cc38331c8dbd upstream.
This LTE modem (M.2 card) has a bug in its power management:
there is some kind of race condition for U3 wake-up between the host and
the device. The modem firmware sometimes crashes/locks when both events
happen at the same time and the modem fully drops off the USB bus (and
sometimes re-enumerates, sometimes just gets stuck until the next
reboot).
Tested with the modem wired to the XHCI controller on an AMD 3015Ce
platform. Without the patch, the modem dropped of the USB bus 5 times in
3 days. With the quirk, it stayed connected for a week while the
'runtime_suspended_time' counter incremented as excepted.
Signed-off-by: Vincent Palatin
Link: https://lore.kernel.org/r/20210319124802.2315195-1-vpala...@chromium.org
Cc: stable
Signed-off-by: Greg Kroah-Hartman
---
drivers/usb/core/quirks.c |4
1 file changed, 4 insertions(+)
--- a/drivers/usb/core/quirks.c
+++ b/drivers/usb/core/quirks.c
@@ -498,6 +498,10 @@ static const struct usb_device_id usb_qu
/* DJI CineSSD */
{ USB_DEVICE(0x2ca3, 0x0031), .driver_info = USB_QUIRK_NO_LPM },
+ /* Fibocom L850-GL LTE Modem */
+ { USB_DEVICE(0x2cb7, 0x0007), .driver_info =
+ USB_QUIRK_IGNORE_REMOTE_WAKEUP },
+
/* INTEL VALUE SSD */
{ USB_DEVICE(0x8086, 0xf1a5), .driver_info = USB_QUIRK_RESET_RESUME },
[PATCH 5.4 61/74] USB: quirks: ignore remote wake-up on Fibocom L850-GL LTE modem
From: Vincent Palatin
commit 0bd860493f81eb2a46173f6f5e44cc38331c8dbd upstream.
This LTE modem (M.2 card) has a bug in its power management:
there is some kind of race condition for U3 wake-up between the host and
the device. The modem firmware sometimes crashes/locks when both events
happen at the same time and the modem fully drops off the USB bus (and
sometimes re-enumerates, sometimes just gets stuck until the next
reboot).
Tested with the modem wired to the XHCI controller on an AMD 3015Ce
platform. Without the patch, the modem dropped of the USB bus 5 times in
3 days. With the quirk, it stayed connected for a week while the
'runtime_suspended_time' counter incremented as excepted.
Signed-off-by: Vincent Palatin
Link: https://lore.kernel.org/r/20210319124802.2315195-1-vpala...@chromium.org
Cc: stable
Signed-off-by: Greg Kroah-Hartman
---
drivers/usb/core/quirks.c |4
1 file changed, 4 insertions(+)
--- a/drivers/usb/core/quirks.c
+++ b/drivers/usb/core/quirks.c
@@ -498,6 +498,10 @@ static const struct usb_device_id usb_qu
/* DJI CineSSD */
{ USB_DEVICE(0x2ca3, 0x0031), .driver_info = USB_QUIRK_NO_LPM },
+ /* Fibocom L850-GL LTE Modem */
+ { USB_DEVICE(0x2cb7, 0x0007), .driver_info =
+ USB_QUIRK_IGNORE_REMOTE_WAKEUP },
+
/* INTEL VALUE SSD */
{ USB_DEVICE(0x8086, 0xf1a5), .driver_info = USB_QUIRK_RESET_RESUME },
[PATCH 4.19 44/56] USB: quirks: ignore remote wake-up on Fibocom L850-GL LTE modem
From: Vincent Palatin
commit 0bd860493f81eb2a46173f6f5e44cc38331c8dbd upstream.
This LTE modem (M.2 card) has a bug in its power management:
there is some kind of race condition for U3 wake-up between the host and
the device. The modem firmware sometimes crashes/locks when both events
happen at the same time and the modem fully drops off the USB bus (and
sometimes re-enumerates, sometimes just gets stuck until the next
reboot).
Tested with the modem wired to the XHCI controller on an AMD 3015Ce
platform. Without the patch, the modem dropped of the USB bus 5 times in
3 days. With the quirk, it stayed connected for a week while the
'runtime_suspended_time' counter incremented as excepted.
Signed-off-by: Vincent Palatin
Link: https://lore.kernel.org/r/20210319124802.2315195-1-vpala...@chromium.org
Cc: stable
Signed-off-by: Greg Kroah-Hartman
---
drivers/usb/core/quirks.c |4
1 file changed, 4 insertions(+)
--- a/drivers/usb/core/quirks.c
+++ b/drivers/usb/core/quirks.c
@@ -498,6 +498,10 @@ static const struct usb_device_id usb_qu
/* DJI CineSSD */
{ USB_DEVICE(0x2ca3, 0x0031), .driver_info = USB_QUIRK_NO_LPM },
+ /* Fibocom L850-GL LTE Modem */
+ { USB_DEVICE(0x2cb7, 0x0007), .driver_info =
+ USB_QUIRK_IGNORE_REMOTE_WAKEUP },
+
/* INTEL VALUE SSD */
{ USB_DEVICE(0x8086, 0xf1a5), .driver_info = USB_QUIRK_RESET_RESUME },
[PATCH 4.14 42/52] USB: quirks: ignore remote wake-up on Fibocom L850-GL LTE modem
From: Vincent Palatin
commit 0bd860493f81eb2a46173f6f5e44cc38331c8dbd upstream.
This LTE modem (M.2 card) has a bug in its power management:
there is some kind of race condition for U3 wake-up between the host and
the device. The modem firmware sometimes crashes/locks when both events
happen at the same time and the modem fully drops off the USB bus (and
sometimes re-enumerates, sometimes just gets stuck until the next
reboot).
Tested with the modem wired to the XHCI controller on an AMD 3015Ce
platform. Without the patch, the modem dropped of the USB bus 5 times in
3 days. With the quirk, it stayed connected for a week while the
'runtime_suspended_time' counter incremented as excepted.
Signed-off-by: Vincent Palatin
Link: https://lore.kernel.org/r/20210319124802.2315195-1-vpala...@chromium.org
Cc: stable
Signed-off-by: Greg Kroah-Hartman
---
drivers/usb/core/quirks.c |4
1 file changed, 4 insertions(+)
--- a/drivers/usb/core/quirks.c
+++ b/drivers/usb/core/quirks.c
@@ -321,6 +321,10 @@ static const struct usb_device_id usb_qu
/* DJI CineSSD */
{ USB_DEVICE(0x2ca3, 0x0031), .driver_info = USB_QUIRK_NO_LPM },
+ /* Fibocom L850-GL LTE Modem */
+ { USB_DEVICE(0x2cb7, 0x0007), .driver_info =
+ USB_QUIRK_IGNORE_REMOTE_WAKEUP },
+
/* INTEL VALUE SSD */
{ USB_DEVICE(0x8086, 0xf1a5), .driver_info = USB_QUIRK_RESET_RESUME },
[PATCH 4.9 27/35] USB: quirks: ignore remote wake-up on Fibocom L850-GL LTE modem
From: Vincent Palatin
commit 0bd860493f81eb2a46173f6f5e44cc38331c8dbd upstream.
This LTE modem (M.2 card) has a bug in its power management:
there is some kind of race condition for U3 wake-up between the host and
the device. The modem firmware sometimes crashes/locks when both events
happen at the same time and the modem fully drops off the USB bus (and
sometimes re-enumerates, sometimes just gets stuck until the next
reboot).
Tested with the modem wired to the XHCI controller on an AMD 3015Ce
platform. Without the patch, the modem dropped of the USB bus 5 times in
3 days. With the quirk, it stayed connected for a week while the
'runtime_suspended_time' counter incremented as excepted.
Signed-off-by: Vincent Palatin
Link: https://lore.kernel.org/r/20210319124802.2315195-1-vpala...@chromium.org
Cc: stable
Signed-off-by: Greg Kroah-Hartman
---
drivers/usb/core/quirks.c |4
1 file changed, 4 insertions(+)
--- a/drivers/usb/core/quirks.c
+++ b/drivers/usb/core/quirks.c
@@ -321,6 +321,10 @@ static const struct usb_device_id usb_qu
/* DJI CineSSD */
{ USB_DEVICE(0x2ca3, 0x0031), .driver_info = USB_QUIRK_NO_LPM },
+ /* Fibocom L850-GL LTE Modem */
+ { USB_DEVICE(0x2cb7, 0x0007), .driver_info =
+ USB_QUIRK_IGNORE_REMOTE_WAKEUP },
+
/* INTEL VALUE SSD */
{ USB_DEVICE(0x8086, 0xf1a5), .driver_info = USB_QUIRK_RESET_RESUME },
[PATCH 4.4 23/28] USB: quirks: ignore remote wake-up on Fibocom L850-GL LTE modem
From: Vincent Palatin
commit 0bd860493f81eb2a46173f6f5e44cc38331c8dbd upstream.
This LTE modem (M.2 card) has a bug in its power management:
there is some kind of race condition for U3 wake-up between the host and
the device. The modem firmware sometimes crashes/locks when both events
happen at the same time and the modem fully drops off the USB bus (and
sometimes re-enumerates, sometimes just gets stuck until the next
reboot).
Tested with the modem wired to the XHCI controller on an AMD 3015Ce
platform. Without the patch, the modem dropped of the USB bus 5 times in
3 days. With the quirk, it stayed connected for a week while the
'runtime_suspended_time' counter incremented as excepted.
Signed-off-by: Vincent Palatin
Link: https://lore.kernel.org/r/20210319124802.2315195-1-vpala...@chromium.org
Cc: stable
Signed-off-by: Greg Kroah-Hartman
---
drivers/usb/core/quirks.c |4
1 file changed, 4 insertions(+)
--- a/drivers/usb/core/quirks.c
+++ b/drivers/usb/core/quirks.c
@@ -321,6 +321,10 @@ static const struct usb_device_id usb_qu
/* DJI CineSSD */
{ USB_DEVICE(0x2ca3, 0x0031), .driver_info = USB_QUIRK_NO_LPM },
+ /* Fibocom L850-GL LTE Modem */
+ { USB_DEVICE(0x2cb7, 0x0007), .driver_info =
+ USB_QUIRK_IGNORE_REMOTE_WAKEUP },
+
/* INTEL VALUE SSD */
{ USB_DEVICE(0x8086, 0xf1a5), .driver_info = USB_QUIRK_RESET_RESUME },
Re: [PATCH] block: don't ignore REQ_NOWAIT for direct IO
On 11/20/20 10:10 AM, Pavel Begunkov wrote: > io_uring's direct nowait requests end up waiting on io_schedule() in > sbitmap, that's seems to be so because blkdev_direct_IO() fails to > propagate IOCB_NOWAIT to a bio and hence to blk-mq. Thanks, applied. This slipped through the cracks, and I didn't notice until I went and directly tested some of this... iomap suffers from the same issue, fwiw. -- Jens Axboe
Re: [PATCH] block: don't ignore REQ_NOWAIT for direct IO
On 20/11/2020 19:13, Jens Axboe wrote: > On 11/20/20 10:10 AM, Pavel Begunkov wrote: >> io_uring's direct nowait requests end up waiting on io_schedule() in >> sbitmap, that's seems to be so because blkdev_direct_IO() fails to >> propagate IOCB_NOWAIT to a bio and hence to blk-mq. >> >> Signed-off-by: Pavel Begunkov >> --- >> fs/block_dev.c | 4 >> 1 file changed, 4 insertions(+) >> >> diff --git a/fs/block_dev.c b/fs/block_dev.c >> index 9e84b1928b94..e7e860c78d93 100644 >> --- a/fs/block_dev.c >> +++ b/fs/block_dev.c >> @@ -263,6 +263,8 @@ __blkdev_direct_IO_simple(struct kiocb *iocb, struct >> iov_iter *iter, >> bio.bi_opf = dio_bio_write_op(iocb); >> task_io_account_write(ret); >> } >> +if (iocb->ki_flags & IOCB_NOWAIT) >> +bio.bi_opf |= REQ_NOWAIT; >> if (iocb->ki_flags & IOCB_HIPRI) >> bio_set_polled(, iocb); > > Was thinking this wasn't needed, but I guess that users could do sync && > NOWAIT > and get -EAGAIN if using preadv2/pwritev2. > >> @@ -416,6 +418,8 @@ __blkdev_direct_IO(struct kiocb *iocb, struct iov_iter >> *iter, int nr_pages) >> bio->bi_opf = dio_bio_write_op(iocb); >> task_io_account_write(bio->bi_iter.bi_size); >> } >> +if (iocb->ki_flags & IOCB_NOWAIT) >> +bio->bi_opf |= REQ_NOWAIT; >> >> dio->size += bio->bi_iter.bi_size; >> pos += bio->bi_iter.bi_size; > > Looks fine to me, we definitely should not be waiting on tags for IOCB_NOWAIT > IO. Will run some shakedown and test for 5.11. > up -- Pavel Begunkov
Re: [PATCH v4 1/3] sched/fair: Ignore percpu threads for imbalance pulls
On Thu, 1 Apr 2021 at 21:30, Valentin Schneider
wrote:
>
> From: Lingutla Chandrasekhar
>
> During load balance, LBF_SOME_PINNED will bet set if any candidate task
> cannot be detached due to CPU affinity constraints. This can result in
> setting env->sd->parent->sgc->group_imbalance, which can lead to a group
> being classified as group_imbalanced (rather than any of the other, lower
> group_type) when balancing at a higher level.
>
> In workloads involving a single task per CPU, LBF_SOME_PINNED can often be
> set due to per-CPU kthreads being the only other runnable tasks on any
> given rq. This results in changing the group classification during
> load-balance at higher levels when in reality there is nothing that can be
> done for this affinity constraint: per-CPU kthreads, as the name implies,
> don't get to move around (modulo hotplug shenanigans).
>
> It's not as clear for userspace tasks - a task could be in an N-CPU cpuset
> with N-1 offline CPUs, making it an "accidental" per-CPU task rather than
> an intended one. KTHREAD_IS_PER_CPU gives us an indisputable signal which
> we can leverage here to not set LBF_SOME_PINNED.
>
> Note that the aforementioned classification to group_imbalance (when
> nothing can be done) is especially problematic on big.LITTLE systems, which
> have a topology the likes of:
>
> DIE [ ]
> MC [][]
>0 1 2 3
>L L B B
>
> arch_scale_cpu_capacity(L) < arch_scale_cpu_capacity(B)
>
> Here, setting LBF_SOME_PINNED due to a per-CPU kthread when balancing at MC
> level on CPUs [0-1] will subsequently prevent CPUs [2-3] from classifying
> the [0-1] group as group_misfit_task when balancing at DIE level. Thus, if
> CPUs [0-1] are running CPU-bound (misfit) tasks, ill-timed per-CPU kthreads
> can significantly delay the upgmigration of said misfit tasks. Systems
> relying on ASYM_PACKING are likely to face similar issues.
>
> Signed-off-by: Lingutla Chandrasekhar
> [Use kthread_is_per_cpu() rather than p->nr_cpus_allowed]
> [Reword changelog]
> Signed-off-by: Valentin Schneider
Reviewed-by: Vincent Guittot
> ---
> kernel/sched/fair.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 6d73bdbb2d40..04d5e14fa261 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -7567,6 +7567,10 @@ int can_migrate_task(struct task_struct *p, struct
> lb_env *env)
> if (throttled_lb_pair(task_group(p), env->src_cpu, env->dst_cpu))
> return 0;
>
> + /* Disregard pcpu kthreads; they are where they need to be. */
> + if ((p->flags & PF_KTHREAD) && kthread_is_per_cpu(p))
> + return 0;
> +
> if (!cpumask_test_cpu(env->dst_cpu, p->cpus_ptr)) {
> int cpu;
>
> --
> 2.25.1
>
[PATCH v4 1/3] sched/fair: Ignore percpu threads for imbalance pulls
From: Lingutla Chandrasekhar
During load balance, LBF_SOME_PINNED will bet set if any candidate task
cannot be detached due to CPU affinity constraints. This can result in
setting env->sd->parent->sgc->group_imbalance, which can lead to a group
being classified as group_imbalanced (rather than any of the other, lower
group_type) when balancing at a higher level.
In workloads involving a single task per CPU, LBF_SOME_PINNED can often be
set due to per-CPU kthreads being the only other runnable tasks on any
given rq. This results in changing the group classification during
load-balance at higher levels when in reality there is nothing that can be
done for this affinity constraint: per-CPU kthreads, as the name implies,
don't get to move around (modulo hotplug shenanigans).
It's not as clear for userspace tasks - a task could be in an N-CPU cpuset
with N-1 offline CPUs, making it an "accidental" per-CPU task rather than
an intended one. KTHREAD_IS_PER_CPU gives us an indisputable signal which
we can leverage here to not set LBF_SOME_PINNED.
Note that the aforementioned classification to group_imbalance (when
nothing can be done) is especially problematic on big.LITTLE systems, which
have a topology the likes of:
DIE [ ]
MC [][]
0 1 2 3
L L B B
arch_scale_cpu_capacity(L) < arch_scale_cpu_capacity(B)
Here, setting LBF_SOME_PINNED due to a per-CPU kthread when balancing at MC
level on CPUs [0-1] will subsequently prevent CPUs [2-3] from classifying
the [0-1] group as group_misfit_task when balancing at DIE level. Thus, if
CPUs [0-1] are running CPU-bound (misfit) tasks, ill-timed per-CPU kthreads
can significantly delay the upgmigration of said misfit tasks. Systems
relying on ASYM_PACKING are likely to face similar issues.
Signed-off-by: Lingutla Chandrasekhar
[Use kthread_is_per_cpu() rather than p->nr_cpus_allowed]
[Reword changelog]
Signed-off-by: Valentin Schneider
---
kernel/sched/fair.c | 4
1 file changed, 4 insertions(+)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 6d73bdbb2d40..04d5e14fa261 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -7567,6 +7567,10 @@ int can_migrate_task(struct task_struct *p, struct
lb_env *env)
if (throttled_lb_pair(task_group(p), env->src_cpu, env->dst_cpu))
return 0;
+ /* Disregard pcpu kthreads; they are where they need to be. */
+ if ((p->flags & PF_KTHREAD) && kthread_is_per_cpu(p))
+ return 0;
+
if (!cpumask_test_cpu(env->dst_cpu, p->cpus_ptr)) {
int cpu;
--
2.25.1
Re: [PATCH v2 7/7] USB: cdc-acm: add other non-standard xr_serial models to ignore list
On Wed, Mar 24, 2021 at 08:41:11AM +0100, Mauro Carvalho Chehab wrote:
> Now that the xr_serial got support for other models, add their
> USB IDs as well, as those devices won't work with the standard
> CDC driver.
As far as I understand these devices should work also with the standard
class driver, but the problem is that your development board is broken
in that CTS isn't wired up properly so that TX is stalled (I even
verified that in the schematics).
Sure, there are other features available in custom-driver mode, but
still funny (sad?) if all that would really have been needed was a tiny
bit of led. :)
I kept this commit but updated the commit message and cleaned up the
entries below somewhat.
> Signed-off-by: Mauro Carvalho Chehab
> ---
> drivers/usb/class/cdc-acm.c | 17 ++---
> 1 file changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
> index 39ddb5585ded..839b80093478 100644
> --- a/drivers/usb/class/cdc-acm.c
> +++ b/drivers/usb/class/cdc-acm.c
> @@ -1902,9 +1902,20 @@ static const struct usb_device_id acm_ids[] = {
> #endif
>
> #if IS_ENABLED(CONFIG_USB_SERIAL_XR)
> - { USB_DEVICE(0x04e2, 0x1410), /* Ignore XR21V141X USB to Serial
> converter */
> - .driver_info = IGNORE_DEVICE,
> - },
> + /* Ignore MaxLinear/Exar USB UARTs and USB UART bridges */
> + { USB_DEVICE(0x04e2, 0x1400), .driver_info = IGNORE_DEVICE,},
> + { USB_DEVICE(0x04e2, 0x1401), .driver_info = IGNORE_DEVICE,},
> + { USB_DEVICE(0x04e2, 0x1402), .driver_info = IGNORE_DEVICE,},
> + { USB_DEVICE(0x04e2, 0x1403), .driver_info = IGNORE_DEVICE,},
> +
> + { USB_DEVICE(0x04e2, 0x1410), .driver_info = IGNORE_DEVICE,},
> + { USB_DEVICE(0x04e2, 0x1411), .driver_info = IGNORE_DEVICE,},
> + { USB_DEVICE(0x04e2, 0x1412), .driver_info = IGNORE_DEVICE,},
> + { USB_DEVICE(0x04e2, 0x1414), .driver_info = IGNORE_DEVICE,},
> +
> + { USB_DEVICE(0x04e2, 0x1420), .driver_info = IGNORE_DEVICE,},
> + { USB_DEVICE(0x04e2, 0x1422), .driver_info = IGNORE_DEVICE,},
> + { USB_DEVICE(0x04e2, 0x1424), .driver_info = IGNORE_DEVICE,},
> #endif
>
> /*Samsung phone in firmware update mode */
Johan
Re: [PATCH] mm: page_alloc: ignore init_on_free=1 for page alloc
On Tue, Mar 30, 2021 at 12:07 AM Vlastimil Babka wrote: > > On 3/30/21 12:00 AM, Andrey Konovalov wrote: > > On Mon, Mar 29, 2021 at 2:10 PM Vlastimil Babka wrote: > >> > >> > commit 855a9c4018f3219db8be7e4b9a65ab22aebfde82 > >> > Author: Andrey Konovalov > >> > Date: Thu Mar 18 17:01:40 2021 +1100 > >> > > >> > kasan, mm: integrate page_alloc init with HW_TAGS > >> > >> But the mmotm patch/-next commit also changes post_alloc_hook() > >> > >> Before the patch it was: > >> kernel_unpoison_pages(page, 1 << order); > >> ... > >> kernel_init_free_pages(page, 1 << order); > >> > >> Now it is (for !kasan_has_integrated_init()): > >> > >> kernel_init_free_pages(page, 1 << order); > >> > >> kernel_unpoison_pages(page, 1 << order); > >> > >> That has to be wrong, because we init the page with zeroes and then call > >> kernel_unpoison_pages() which checks for the 0xaa pattern. Andrey? > > > > It's similar to free_pages_prepare(): kernel_unpoison_pages() and > > want_init_on_alloc() are exclusive, so the order shouldn't matter. Am > > I missing something? > > Yeah, when the allocation has __GFP_ZERO, want_init_on_alloc() will return > true > even with the static branches disabled. Ah, I see. I'll post a fix soon. Thank you!
Re: [PATCH v3] mm: page_alloc: ignore init_on_free=1 for debug_pagealloc=1
On 30.03.21 00:25, Sergei Trofimovich wrote:
On !ARCH_SUPPORTS_DEBUG_PAGEALLOC (like ia64) debug_pagealloc=1
implies page_poison=on:
if (page_poisoning_enabled() ||
(!IS_ENABLED(CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC) &&
debug_pagealloc_enabled()))
static_branch_enable(&_page_poisoning_enabled);
page_poison=on needs to override init_on_free=1.
Before the change it did not work as expected for the following case:
- have PAGE_POISONING=y
- have page_poison unset
- have !ARCH_SUPPORTS_DEBUG_PAGEALLOC arch (like ia64)
- have init_on_free=1
- have debug_pagealloc=1
That way we get both keys enabled:
- static_branch_enable(_on_free);
- static_branch_enable(&_page_poisoning_enabled);
which leads to poisoned pages returned for __GFP_ZERO pages.
After the change we execute only:
- static_branch_enable(&_page_poisoning_enabled);
and ignore init_on_free=1.
Acked-by: Vlastimil Babka
Fixes: 8db26a3d4735 ("mm, page_poison: use static key more efficiently")
Cc:
CC: Andrew Morton
CC: linux...@kvack.org
CC: David Hildenbrand
CC: Andrey Konovalov
Link: https://lkml.org/lkml/2021/3/26/443
Signed-off-by: Sergei Trofimovich
---
Change since v2:
- Added 'Fixes:' and 'CC: stable@' suggested by Vlastimil and David
- Renamed local variable to 'page_poisoning_requested' for
consistency suggested by David
- Simplified initialization of page_poisoning_requested suggested
by David
- Added 'Acked-by: Vlastimil'
mm/page_alloc.c | 30 +-
1 file changed, 17 insertions(+), 13 deletions(-)
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index cfc72873961d..4bb3cdfc47f8 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -764,32 +764,36 @@ static inline void clear_page_guard(struct zone *zone,
struct page *page,
*/
void init_mem_debugging_and_hardening(void)
{
+ bool page_poisoning_requested = false;
+
+#ifdef CONFIG_PAGE_POISONING
+ /*
+* Page poisoning is debug page alloc for some arches. If
+* either of those options are enabled, enable poisoning.
+*/
+ if (page_poisoning_enabled() ||
+(!IS_ENABLED(CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC) &&
+ debug_pagealloc_enabled())) {
+ static_branch_enable(&_page_poisoning_enabled);
+ page_poisoning_requested = true;
+ }
+#endif
+
if (_init_on_alloc_enabled_early) {
- if (page_poisoning_enabled())
+ if (page_poisoning_requested)
pr_info("mem auto-init: CONFIG_PAGE_POISONING is on, "
"will take precedence over init_on_alloc\n");
else
static_branch_enable(_on_alloc);
}
if (_init_on_free_enabled_early) {
- if (page_poisoning_enabled())
+ if (page_poisoning_requested)
pr_info("mem auto-init: CONFIG_PAGE_POISONING is on, "
"will take precedence over init_on_free\n");
else
static_branch_enable(_on_free);
}
-#ifdef CONFIG_PAGE_POISONING
- /*
-* Page poisoning is debug page alloc for some arches. If
-* either of those options are enabled, enable poisoning.
-*/
- if (page_poisoning_enabled() ||
-(!IS_ENABLED(CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC) &&
- debug_pagealloc_enabled()))
- static_branch_enable(&_page_poisoning_enabled);
-#endif
-
#ifdef CONFIG_DEBUG_PAGEALLOC
if (!debug_pagealloc_enabled())
return;
Thanks!
Reviewed-by: David Hildenbrand
--
Thanks,
David / dhildenb
[PATCH AUTOSEL 4.9 10/10] cifs: Silently ignore unknown oplock break handle
From: Vincent Whitchurch [ Upstream commit 219481a8f90ec3a5eed9638fb35609e4b1aeece7 ] Make SMB2 not print out an error when an oplock break is received for an unknown handle, similar to SMB1. The debug message which is printed for these unknown handles may also be misleading, so fix that too. The SMB2 lease break path is not affected by this patch. Without this, a program which writes to a file from one thread, and opens, reads, and writes the same file from another thread triggers the below errors several times a minute when run against a Samba server configured with "smb2 leases = no". CIFS: VFS: \\192.168.0.1 No task to wake, unknown frame received! NumMids 2 : 424d53fe 0040 0012 .SMB@... 0010: 0001 0020: 0030: Signed-off-by: Vincent Whitchurch Reviewed-by: Tom Talpey Reviewed-by: Paulo Alcantara (SUSE) Signed-off-by: Steve French Signed-off-by: Sasha Levin --- fs/cifs/smb2misc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/cifs/smb2misc.c b/fs/cifs/smb2misc.c index bddb2d7b3982..075b285bbd3e 100644 --- a/fs/cifs/smb2misc.c +++ b/fs/cifs/smb2misc.c @@ -651,8 +651,8 @@ smb2_is_valid_oplock_break(char *buffer, struct TCP_Server_Info *server) } } spin_unlock(_tcp_ses_lock); - cifs_dbg(FYI, "Can not process oplock break for non-existent connection\n"); - return false; + cifs_dbg(FYI, "No file id matched, oplock break ignored\n"); + return true; } void -- 2.30.1
[PATCH AUTOSEL 4.4 8/8] cifs: Silently ignore unknown oplock break handle
From: Vincent Whitchurch [ Upstream commit 219481a8f90ec3a5eed9638fb35609e4b1aeece7 ] Make SMB2 not print out an error when an oplock break is received for an unknown handle, similar to SMB1. The debug message which is printed for these unknown handles may also be misleading, so fix that too. The SMB2 lease break path is not affected by this patch. Without this, a program which writes to a file from one thread, and opens, reads, and writes the same file from another thread triggers the below errors several times a minute when run against a Samba server configured with "smb2 leases = no". CIFS: VFS: \\192.168.0.1 No task to wake, unknown frame received! NumMids 2 : 424d53fe 0040 0012 .SMB@... 0010: 0001 0020: 0030: Signed-off-by: Vincent Whitchurch Reviewed-by: Tom Talpey Reviewed-by: Paulo Alcantara (SUSE) Signed-off-by: Steve French Signed-off-by: Sasha Levin --- fs/cifs/smb2misc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/cifs/smb2misc.c b/fs/cifs/smb2misc.c index 44198b9a5315..19baeb4ca511 100644 --- a/fs/cifs/smb2misc.c +++ b/fs/cifs/smb2misc.c @@ -633,8 +633,8 @@ smb2_is_valid_oplock_break(char *buffer, struct TCP_Server_Info *server) } } spin_unlock(_tcp_ses_lock); - cifs_dbg(FYI, "Can not process oplock break for non-existent connection\n"); - return false; + cifs_dbg(FYI, "No file id matched, oplock break ignored\n"); + return true; } void -- 2.30.1
[PATCH AUTOSEL 4.14 12/12] cifs: Silently ignore unknown oplock break handle
From: Vincent Whitchurch [ Upstream commit 219481a8f90ec3a5eed9638fb35609e4b1aeece7 ] Make SMB2 not print out an error when an oplock break is received for an unknown handle, similar to SMB1. The debug message which is printed for these unknown handles may also be misleading, so fix that too. The SMB2 lease break path is not affected by this patch. Without this, a program which writes to a file from one thread, and opens, reads, and writes the same file from another thread triggers the below errors several times a minute when run against a Samba server configured with "smb2 leases = no". CIFS: VFS: \\192.168.0.1 No task to wake, unknown frame received! NumMids 2 : 424d53fe 0040 0012 .SMB@... 0010: 0001 0020: 0030: Signed-off-by: Vincent Whitchurch Reviewed-by: Tom Talpey Reviewed-by: Paulo Alcantara (SUSE) Signed-off-by: Steve French Signed-off-by: Sasha Levin --- fs/cifs/smb2misc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/cifs/smb2misc.c b/fs/cifs/smb2misc.c index 0c6e5450ff76..80339e318294 100644 --- a/fs/cifs/smb2misc.c +++ b/fs/cifs/smb2misc.c @@ -656,8 +656,8 @@ smb2_is_valid_oplock_break(char *buffer, struct TCP_Server_Info *server) } } spin_unlock(_tcp_ses_lock); - cifs_dbg(FYI, "Can not process oplock break for non-existent connection\n"); - return false; + cifs_dbg(FYI, "No file id matched, oplock break ignored\n"); + return true; } void -- 2.30.1
[PATCH v3] mm: page_alloc: ignore init_on_free=1 for debug_pagealloc=1
On !ARCH_SUPPORTS_DEBUG_PAGEALLOC (like ia64) debug_pagealloc=1
implies page_poison=on:
if (page_poisoning_enabled() ||
(!IS_ENABLED(CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC) &&
debug_pagealloc_enabled()))
static_branch_enable(&_page_poisoning_enabled);
page_poison=on needs to override init_on_free=1.
Before the change it did not work as expected for the following case:
- have PAGE_POISONING=y
- have page_poison unset
- have !ARCH_SUPPORTS_DEBUG_PAGEALLOC arch (like ia64)
- have init_on_free=1
- have debug_pagealloc=1
That way we get both keys enabled:
- static_branch_enable(_on_free);
- static_branch_enable(&_page_poisoning_enabled);
which leads to poisoned pages returned for __GFP_ZERO pages.
After the change we execute only:
- static_branch_enable(&_page_poisoning_enabled);
and ignore init_on_free=1.
Acked-by: Vlastimil Babka
Fixes: 8db26a3d4735 ("mm, page_poison: use static key more efficiently")
Cc:
CC: Andrew Morton
CC: linux...@kvack.org
CC: David Hildenbrand
CC: Andrey Konovalov
Link: https://lkml.org/lkml/2021/3/26/443
Signed-off-by: Sergei Trofimovich
---
Change since v2:
- Added 'Fixes:' and 'CC: stable@' suggested by Vlastimil and David
- Renamed local variable to 'page_poisoning_requested' for
consistency suggested by David
- Simplified initialization of page_poisoning_requested suggested
by David
- Added 'Acked-by: Vlastimil'
mm/page_alloc.c | 30 +-
1 file changed, 17 insertions(+), 13 deletions(-)
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index cfc72873961d..4bb3cdfc47f8 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -764,32 +764,36 @@ static inline void clear_page_guard(struct zone *zone,
struct page *page,
*/
void init_mem_debugging_and_hardening(void)
{
+ bool page_poisoning_requested = false;
+
+#ifdef CONFIG_PAGE_POISONING
+ /*
+* Page poisoning is debug page alloc for some arches. If
+* either of those options are enabled, enable poisoning.
+*/
+ if (page_poisoning_enabled() ||
+(!IS_ENABLED(CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC) &&
+ debug_pagealloc_enabled())) {
+ static_branch_enable(&_page_poisoning_enabled);
+ page_poisoning_requested = true;
+ }
+#endif
+
if (_init_on_alloc_enabled_early) {
- if (page_poisoning_enabled())
+ if (page_poisoning_requested)
pr_info("mem auto-init: CONFIG_PAGE_POISONING is on, "
"will take precedence over init_on_alloc\n");
else
static_branch_enable(_on_alloc);
}
if (_init_on_free_enabled_early) {
- if (page_poisoning_enabled())
+ if (page_poisoning_requested)
pr_info("mem auto-init: CONFIG_PAGE_POISONING is on, "
"will take precedence over init_on_free\n");
else
static_branch_enable(_on_free);
}
-#ifdef CONFIG_PAGE_POISONING
- /*
-* Page poisoning is debug page alloc for some arches. If
-* either of those options are enabled, enable poisoning.
-*/
- if (page_poisoning_enabled() ||
-(!IS_ENABLED(CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC) &&
- debug_pagealloc_enabled()))
- static_branch_enable(&_page_poisoning_enabled);
-#endif
-
#ifdef CONFIG_DEBUG_PAGEALLOC
if (!debug_pagealloc_enabled())
return;
--
2.31.1
[PATCH AUTOSEL 4.19 15/15] cifs: Silently ignore unknown oplock break handle
From: Vincent Whitchurch [ Upstream commit 219481a8f90ec3a5eed9638fb35609e4b1aeece7 ] Make SMB2 not print out an error when an oplock break is received for an unknown handle, similar to SMB1. The debug message which is printed for these unknown handles may also be misleading, so fix that too. The SMB2 lease break path is not affected by this patch. Without this, a program which writes to a file from one thread, and opens, reads, and writes the same file from another thread triggers the below errors several times a minute when run against a Samba server configured with "smb2 leases = no". CIFS: VFS: \\192.168.0.1 No task to wake, unknown frame received! NumMids 2 : 424d53fe 0040 0012 .SMB@... 0010: 0001 0020: 0030: Signed-off-by: Vincent Whitchurch Reviewed-by: Tom Talpey Reviewed-by: Paulo Alcantara (SUSE) Signed-off-by: Steve French Signed-off-by: Sasha Levin --- fs/cifs/smb2misc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/cifs/smb2misc.c b/fs/cifs/smb2misc.c index 7d875a47d022..7177720e822e 100644 --- a/fs/cifs/smb2misc.c +++ b/fs/cifs/smb2misc.c @@ -738,8 +738,8 @@ smb2_is_valid_oplock_break(char *buffer, struct TCP_Server_Info *server) } } spin_unlock(_tcp_ses_lock); - cifs_dbg(FYI, "Can not process oplock break for non-existent connection\n"); - return false; + cifs_dbg(FYI, "No file id matched, oplock break ignored\n"); + return true; } void -- 2.30.1
[PATCH AUTOSEL 5.4 19/19] cifs: Silently ignore unknown oplock break handle
From: Vincent Whitchurch [ Upstream commit 219481a8f90ec3a5eed9638fb35609e4b1aeece7 ] Make SMB2 not print out an error when an oplock break is received for an unknown handle, similar to SMB1. The debug message which is printed for these unknown handles may also be misleading, so fix that too. The SMB2 lease break path is not affected by this patch. Without this, a program which writes to a file from one thread, and opens, reads, and writes the same file from another thread triggers the below errors several times a minute when run against a Samba server configured with "smb2 leases = no". CIFS: VFS: \\192.168.0.1 No task to wake, unknown frame received! NumMids 2 : 424d53fe 0040 0012 .SMB@... 0010: 0001 0020: 0030: Signed-off-by: Vincent Whitchurch Reviewed-by: Tom Talpey Reviewed-by: Paulo Alcantara (SUSE) Signed-off-by: Steve French Signed-off-by: Sasha Levin --- fs/cifs/smb2misc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/cifs/smb2misc.c b/fs/cifs/smb2misc.c index 7d875a47d022..7177720e822e 100644 --- a/fs/cifs/smb2misc.c +++ b/fs/cifs/smb2misc.c @@ -738,8 +738,8 @@ smb2_is_valid_oplock_break(char *buffer, struct TCP_Server_Info *server) } } spin_unlock(_tcp_ses_lock); - cifs_dbg(FYI, "Can not process oplock break for non-existent connection\n"); - return false; + cifs_dbg(FYI, "No file id matched, oplock break ignored\n"); + return true; } void -- 2.30.1
[PATCH AUTOSEL 5.11 36/38] cifs: Silently ignore unknown oplock break handle
From: Vincent Whitchurch [ Upstream commit 219481a8f90ec3a5eed9638fb35609e4b1aeece7 ] Make SMB2 not print out an error when an oplock break is received for an unknown handle, similar to SMB1. The debug message which is printed for these unknown handles may also be misleading, so fix that too. The SMB2 lease break path is not affected by this patch. Without this, a program which writes to a file from one thread, and opens, reads, and writes the same file from another thread triggers the below errors several times a minute when run against a Samba server configured with "smb2 leases = no". CIFS: VFS: \\192.168.0.1 No task to wake, unknown frame received! NumMids 2 : 424d53fe 0040 0012 .SMB@... 0010: 0001 0020: 0030: Signed-off-by: Vincent Whitchurch Reviewed-by: Tom Talpey Reviewed-by: Paulo Alcantara (SUSE) Signed-off-by: Steve French Signed-off-by: Sasha Levin --- fs/cifs/smb2misc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/cifs/smb2misc.c b/fs/cifs/smb2misc.c index d9073b569e17..53fb751bf210 100644 --- a/fs/cifs/smb2misc.c +++ b/fs/cifs/smb2misc.c @@ -754,8 +754,8 @@ smb2_is_valid_oplock_break(char *buffer, struct TCP_Server_Info *server) } } spin_unlock(_tcp_ses_lock); - cifs_dbg(FYI, "Can not process oplock break for non-existent connection\n"); - return false; + cifs_dbg(FYI, "No file id matched, oplock break ignored\n"); + return true; } void -- 2.30.1
[PATCH AUTOSEL 5.10 31/33] cifs: Silently ignore unknown oplock break handle
From: Vincent Whitchurch [ Upstream commit 219481a8f90ec3a5eed9638fb35609e4b1aeece7 ] Make SMB2 not print out an error when an oplock break is received for an unknown handle, similar to SMB1. The debug message which is printed for these unknown handles may also be misleading, so fix that too. The SMB2 lease break path is not affected by this patch. Without this, a program which writes to a file from one thread, and opens, reads, and writes the same file from another thread triggers the below errors several times a minute when run against a Samba server configured with "smb2 leases = no". CIFS: VFS: \\192.168.0.1 No task to wake, unknown frame received! NumMids 2 : 424d53fe 0040 0012 .SMB@... 0010: 0001 0020: 0030: Signed-off-by: Vincent Whitchurch Reviewed-by: Tom Talpey Reviewed-by: Paulo Alcantara (SUSE) Signed-off-by: Steve French Signed-off-by: Sasha Levin --- fs/cifs/smb2misc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/cifs/smb2misc.c b/fs/cifs/smb2misc.c index db22d686c61f..be3df90bb2bc 100644 --- a/fs/cifs/smb2misc.c +++ b/fs/cifs/smb2misc.c @@ -745,8 +745,8 @@ smb2_is_valid_oplock_break(char *buffer, struct TCP_Server_Info *server) } } spin_unlock(_tcp_ses_lock); - cifs_dbg(FYI, "Can not process oplock break for non-existent connection\n"); - return false; + cifs_dbg(FYI, "No file id matched, oplock break ignored\n"); + return true; } void -- 2.30.1
[PATCH AUTOSEL 5.10 24/33] platform/x86: intel_pmc_core: Ignore GBE LTR on Tiger Lake platforms
From: "David E. Box"
[ Upstream commit d1635448f1105e549b4041aab930dbc6945fc635 ]
Due to a HW limitation, the Latency Tolerance Reporting (LTR) value
programmed in the Tiger Lake GBE controller is not large enough to allow
the platform to enter Package C10, which in turn prevents the platform from
achieving its low power target during suspend-to-idle. Ignore the GBE LTR
value on Tiger Lake. LTR ignore functionality is currently performed solely
by a debugfs write call. Split out the LTR code into its own function that
can be called by both the debugfs writer and by this work around.
Signed-off-by: David E. Box
Reviewed-by: Sasha Neftin
Cc: intel-wired-...@lists.osuosl.org
Reviewed-by: Rajneesh Bhardwaj
Link:
https://lore.kernel.org/r/20210319201844.3305399-2-david.e@linux.intel.com
Signed-off-by: Hans de Goede
Signed-off-by: Sasha Levin
---
drivers/platform/x86/intel_pmc_core.c | 50 +++
1 file changed, 35 insertions(+), 15 deletions(-)
diff --git a/drivers/platform/x86/intel_pmc_core.c
b/drivers/platform/x86/intel_pmc_core.c
index 3e5fe66333f1..e06b36e87a33 100644
--- a/drivers/platform/x86/intel_pmc_core.c
+++ b/drivers/platform/x86/intel_pmc_core.c
@@ -863,34 +863,45 @@ static int pmc_core_pll_show(struct seq_file *s, void
*unused)
}
DEFINE_SHOW_ATTRIBUTE(pmc_core_pll);
-static ssize_t pmc_core_ltr_ignore_write(struct file *file,
-const char __user *userbuf,
-size_t count, loff_t *ppos)
+static int pmc_core_send_ltr_ignore(u32 value)
{
struct pmc_dev *pmcdev =
const struct pmc_reg_map *map = pmcdev->map;
- u32 val, buf_size, fd;
- int err;
-
- buf_size = count < 64 ? count : 64;
-
- err = kstrtou32_from_user(userbuf, buf_size, 10, );
- if (err)
- return err;
+ u32 reg;
+ int err = 0;
mutex_lock(>lock);
- if (val > map->ltr_ignore_max) {
+ if (value > map->ltr_ignore_max) {
err = -EINVAL;
goto out_unlock;
}
- fd = pmc_core_reg_read(pmcdev, map->ltr_ignore_offset);
- fd |= (1U << val);
- pmc_core_reg_write(pmcdev, map->ltr_ignore_offset, fd);
+ reg = pmc_core_reg_read(pmcdev, map->ltr_ignore_offset);
+ reg |= BIT(value);
+ pmc_core_reg_write(pmcdev, map->ltr_ignore_offset, reg);
out_unlock:
mutex_unlock(>lock);
+
+ return err;
+}
+
+static ssize_t pmc_core_ltr_ignore_write(struct file *file,
+const char __user *userbuf,
+size_t count, loff_t *ppos)
+{
+ u32 buf_size, value;
+ int err;
+
+ buf_size = min_t(u32, count, 64);
+
+ err = kstrtou32_from_user(userbuf, buf_size, 10, );
+ if (err)
+ return err;
+
+ err = pmc_core_send_ltr_ignore(value);
+
return err == 0 ? count : err;
}
@@ -1244,6 +1255,15 @@ static int pmc_core_probe(struct platform_device *pdev)
pmcdev->pmc_xram_read_bit = pmc_core_check_read_lock_bit();
dmi_check_system(pmc_core_dmi_table);
+ /*
+* On TGL, due to a hardware limitation, the GBE LTR blocks PC10 when
+* a cable is attached. Tell the PMC to ignore it.
+*/
+ if (pmcdev->map == _reg_map) {
+ dev_dbg(>dev, "ignoring GBE LTR\n");
+ pmc_core_send_ltr_ignore(3);
+ }
+
pmc_core_dbgfs_register(pmcdev);
device_initialized = true;
--
2.30.1
[PATCH AUTOSEL 5.11 28/38] platform/x86: intel_pmc_core: Ignore GBE LTR on Tiger Lake platforms
From: "David E. Box"
[ Upstream commit d1635448f1105e549b4041aab930dbc6945fc635 ]
Due to a HW limitation, the Latency Tolerance Reporting (LTR) value
programmed in the Tiger Lake GBE controller is not large enough to allow
the platform to enter Package C10, which in turn prevents the platform from
achieving its low power target during suspend-to-idle. Ignore the GBE LTR
value on Tiger Lake. LTR ignore functionality is currently performed solely
by a debugfs write call. Split out the LTR code into its own function that
can be called by both the debugfs writer and by this work around.
Signed-off-by: David E. Box
Reviewed-by: Sasha Neftin
Cc: intel-wired-...@lists.osuosl.org
Reviewed-by: Rajneesh Bhardwaj
Link:
https://lore.kernel.org/r/20210319201844.3305399-2-david.e@linux.intel.com
Signed-off-by: Hans de Goede
Signed-off-by: Sasha Levin
---
drivers/platform/x86/intel_pmc_core.c | 50 +++
1 file changed, 35 insertions(+), 15 deletions(-)
diff --git a/drivers/platform/x86/intel_pmc_core.c
b/drivers/platform/x86/intel_pmc_core.c
index ee2f757515b0..b5888aeb4bcf 100644
--- a/drivers/platform/x86/intel_pmc_core.c
+++ b/drivers/platform/x86/intel_pmc_core.c
@@ -863,34 +863,45 @@ static int pmc_core_pll_show(struct seq_file *s, void
*unused)
}
DEFINE_SHOW_ATTRIBUTE(pmc_core_pll);
-static ssize_t pmc_core_ltr_ignore_write(struct file *file,
-const char __user *userbuf,
-size_t count, loff_t *ppos)
+static int pmc_core_send_ltr_ignore(u32 value)
{
struct pmc_dev *pmcdev =
const struct pmc_reg_map *map = pmcdev->map;
- u32 val, buf_size, fd;
- int err;
-
- buf_size = count < 64 ? count : 64;
-
- err = kstrtou32_from_user(userbuf, buf_size, 10, );
- if (err)
- return err;
+ u32 reg;
+ int err = 0;
mutex_lock(>lock);
- if (val > map->ltr_ignore_max) {
+ if (value > map->ltr_ignore_max) {
err = -EINVAL;
goto out_unlock;
}
- fd = pmc_core_reg_read(pmcdev, map->ltr_ignore_offset);
- fd |= (1U << val);
- pmc_core_reg_write(pmcdev, map->ltr_ignore_offset, fd);
+ reg = pmc_core_reg_read(pmcdev, map->ltr_ignore_offset);
+ reg |= BIT(value);
+ pmc_core_reg_write(pmcdev, map->ltr_ignore_offset, reg);
out_unlock:
mutex_unlock(>lock);
+
+ return err;
+}
+
+static ssize_t pmc_core_ltr_ignore_write(struct file *file,
+const char __user *userbuf,
+size_t count, loff_t *ppos)
+{
+ u32 buf_size, value;
+ int err;
+
+ buf_size = min_t(u32, count, 64);
+
+ err = kstrtou32_from_user(userbuf, buf_size, 10, );
+ if (err)
+ return err;
+
+ err = pmc_core_send_ltr_ignore(value);
+
return err == 0 ? count : err;
}
@@ -1244,6 +1255,15 @@ static int pmc_core_probe(struct platform_device *pdev)
pmcdev->pmc_xram_read_bit = pmc_core_check_read_lock_bit();
dmi_check_system(pmc_core_dmi_table);
+ /*
+* On TGL, due to a hardware limitation, the GBE LTR blocks PC10 when
+* a cable is attached. Tell the PMC to ignore it.
+*/
+ if (pmcdev->map == _reg_map) {
+ dev_dbg(>dev, "ignoring GBE LTR\n");
+ pmc_core_send_ltr_ignore(3);
+ }
+
pmc_core_dbgfs_register(pmcdev);
device_initialized = true;
--
2.30.1
Re: [PATCH] mm: page_alloc: ignore init_on_free=1 for page alloc
On 3/30/21 12:00 AM, Andrey Konovalov wrote: > On Mon, Mar 29, 2021 at 2:10 PM Vlastimil Babka wrote: >> >> > commit 855a9c4018f3219db8be7e4b9a65ab22aebfde82 >> > Author: Andrey Konovalov >> > Date: Thu Mar 18 17:01:40 2021 +1100 >> > >> > kasan, mm: integrate page_alloc init with HW_TAGS >> >> But the mmotm patch/-next commit also changes post_alloc_hook() >> >> Before the patch it was: >> kernel_unpoison_pages(page, 1 << order); >> ... >> kernel_init_free_pages(page, 1 << order); >> >> Now it is (for !kasan_has_integrated_init()): >> >> kernel_init_free_pages(page, 1 << order); >> >> kernel_unpoison_pages(page, 1 << order); >> >> That has to be wrong, because we init the page with zeroes and then call >> kernel_unpoison_pages() which checks for the 0xaa pattern. Andrey? > > It's similar to free_pages_prepare(): kernel_unpoison_pages() and > want_init_on_alloc() are exclusive, so the order shouldn't matter. Am > I missing something? Yeah, when the allocation has __GFP_ZERO, want_init_on_alloc() will return true even with the static branches disabled.
Re: [PATCH] mm: page_alloc: ignore init_on_free=1 for page alloc
On Mon, Mar 29, 2021 at 2:10 PM Vlastimil Babka wrote: > > > commit 855a9c4018f3219db8be7e4b9a65ab22aebfde82 > > Author: Andrey Konovalov > > Date: Thu Mar 18 17:01:40 2021 +1100 > > > > kasan, mm: integrate page_alloc init with HW_TAGS > > But the mmotm patch/-next commit also changes post_alloc_hook() > > Before the patch it was: > kernel_unpoison_pages(page, 1 << order); > ... > kernel_init_free_pages(page, 1 << order); > > Now it is (for !kasan_has_integrated_init()): > > kernel_init_free_pages(page, 1 << order); > > kernel_unpoison_pages(page, 1 << order); > > That has to be wrong, because we init the page with zeroes and then call > kernel_unpoison_pages() which checks for the 0xaa pattern. Andrey? It's similar to free_pages_prepare(): kernel_unpoison_pages() and want_init_on_alloc() are exclusive, so the order shouldn't matter. Am I missing something?
Re: [PATCH] mm: page_alloc: ignore init_on_free=1 for page alloc
On 3/26/21 2:48 PM, David Hildenbrand wrote: > On 26.03.21 12:26, Sergei Trofimovich wrote: >> init_on_free=1 does not guarantee that free pages contain only zero bytes. >> >> Some examples: >> 1. page_poison=on takes presedence over init_on_alloc=1 / ini_on_free=1 > > s/ini_on_free/init_on_free/ > >> 2. free_pages_prepare() always poisons pages: >> >> if (want_init_on_free()) >> kernel_init_free_pages(page, 1 << order); >> kernel_poison_pages(page, 1 << order > > In next/master, it's the other way around already. And that should be OK as the order should not matter, as long as they are indeed exclusive. They should be after Sergei's v2 fix. As long as kasan_free_nondeferred_pages() which follows doesn't do anything unexpected to poisoned pages (I haven't check). > commit 855a9c4018f3219db8be7e4b9a65ab22aebfde82 > Author: Andrey Konovalov > Date: Thu Mar 18 17:01:40 2021 +1100 > > kasan, mm: integrate page_alloc init with HW_TAGS But the mmotm patch/-next commit also changes post_alloc_hook() Before the patch it was: kernel_unpoison_pages(page, 1 << order); ... kernel_init_free_pages(page, 1 << order); Now it is (for !kasan_has_integrated_init()): kernel_init_free_pages(page, 1 << order); kernel_unpoison_pages(page, 1 << order); That has to be wrong, because we init the page with zeroes and then call kernel_unpoison_pages() which checks for the 0xaa pattern. Andrey? >> >> I observed use of poisoned pages as the crash on ia64 booted with >> init_on_free=1 init_on_alloc=1 (CONFIG_PAGE_POISONING=y config). >> There pmd page contained 0x poison pages and led to early crash. >> >> The change drops the assumption that init_on_free=1 guarantees free >> pages to contain zeros. >> >> Alternative would be to make interaction between runtime poisoning and >> sanitizing options and build-time debug flags like CONFIG_PAGE_POISONING >> more coherent. I took the simpler path. >> > > I thought latest work be Vlastimil tried to tackle that. To me, it feels like > page_poison=on and init_on_free=1 should bail out and disable one of both > things. Having both at the same time doesn't sound helpful. > >> Tested the fix on rx3600. > > Fixes: ? > >
Re: [PATCH v2] mm: page_alloc: ignore init_on_free=1 for debug_pagealloc=1
On 3/27/21 7:21 PM, Sergei Trofimovich wrote:
> On !ARCH_SUPPORTS_DEBUG_PAGEALLOC (like ia64) debug_pagealloc=1
> implies page_poison=on:
>
> if (page_poisoning_enabled() ||
> (!IS_ENABLED(CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC) &&
> debug_pagealloc_enabled()))
> static_branch_enable(&_page_poisoning_enabled);
>
> page_poison=on needs to init_on_free=1.
>
> Before the change id happened too late for the following case:
> - have PAGE_POISONING=y
> - have page_poison unset
> - have !ARCH_SUPPORTS_DEBUG_PAGEALLOC arch (like ia64)
> - have init_on_free=1
> - have debug_pagealloc=1
>
> That way we get both keys enabled:
> - static_branch_enable(_on_free);
> - static_branch_enable(&_page_poisoning_enabled);
>
> which leads to poisoned pages returned for __GFP_ZERO pages.
Good catch, thanks for finding the root cause!
> After the change we execute only:
> - static_branch_enable(&_page_poisoning_enabled);
> and ignore init_on_free=1.
> CC: Vlastimil Babka
> CC: Andrew Morton
> CC: linux...@kvack.org
> CC: David Hildenbrand
> CC: Andrey Konovalov
> Link: https://lkml.org/lkml/2021/3/26/443
> Signed-off-by: Sergei Trofimovich
Acked-by: Vlastimil Babka
8db26a3d4735 ("mm, page_poison: use static key more efficiently")
Cc:
> ---
> mm/page_alloc.c | 30 +-
> 1 file changed, 17 insertions(+), 13 deletions(-)
>
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index d57d9b4f7089..10a8a1d28c11 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -764,32 +764,36 @@ static inline void clear_page_guard(struct zone *zone,
> struct page *page,
> */
> void init_mem_debugging_and_hardening(void)
> {
> + bool page_poison_requested = page_poisoning_enabled();
> +
> +#ifdef CONFIG_PAGE_POISONING
> + /*
> + * Page poisoning is debug page alloc for some arches. If
> + * either of those options are enabled, enable poisoning.
> + */
> + if (page_poisoning_enabled() ||
> + (!IS_ENABLED(CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC) &&
> + debug_pagealloc_enabled())) {
> + static_branch_enable(&_page_poisoning_enabled);
> + page_poison_requested = true;
> + }
> +#endif
> +
> if (_init_on_alloc_enabled_early) {
> - if (page_poisoning_enabled())
> + if (page_poison_requested)
> pr_info("mem auto-init: CONFIG_PAGE_POISONING is on, "
> "will take precedence over init_on_alloc\n");
> else
> static_branch_enable(_on_alloc);
> }
> if (_init_on_free_enabled_early) {
> - if (page_poisoning_enabled())
> + if (page_poison_requested)
> pr_info("mem auto-init: CONFIG_PAGE_POISONING is on, "
> "will take precedence over init_on_free\n");
> else
> static_branch_enable(_on_free);
> }
>
> -#ifdef CONFIG_PAGE_POISONING
> - /*
> - * Page poisoning is debug page alloc for some arches. If
> - * either of those options are enabled, enable poisoning.
> - */
> - if (page_poisoning_enabled() ||
> - (!IS_ENABLED(CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC) &&
> - debug_pagealloc_enabled()))
> - static_branch_enable(&_page_poisoning_enabled);
> -#endif
> -
> #ifdef CONFIG_DEBUG_PAGEALLOC
> if (!debug_pagealloc_enabled())
> return;
>
Re: [PATCH v2] mm: page_alloc: ignore init_on_free=1 for debug_pagealloc=1
On 27.03.21 19:21, Sergei Trofimovich wrote:
On !ARCH_SUPPORTS_DEBUG_PAGEALLOC (like ia64) debug_pagealloc=1
implies page_poison=on:
if (page_poisoning_enabled() ||
(!IS_ENABLED(CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC) &&
debug_pagealloc_enabled()))
static_branch_enable(&_page_poisoning_enabled);
page_poison=on needs to init_on_free=1.
Before the change id happened too late for the following case:
- have PAGE_POISONING=y
- have page_poison unset
- have !ARCH_SUPPORTS_DEBUG_PAGEALLOC arch (like ia64)
- have init_on_free=1
- have debug_pagealloc=1
That way we get both keys enabled:
- static_branch_enable(_on_free);
- static_branch_enable(&_page_poisoning_enabled);
which leads to poisoned pages returned for __GFP_ZERO pages.
After the change we execute only:
- static_branch_enable(&_page_poisoning_enabled);
and ignore init_on_free=1.
CC: Vlastimil Babka
CC: Andrew Morton
CC: linux...@kvack.org
CC: David Hildenbrand
CC: Andrey Konovalov
Link: https://lkml.org/lkml/2021/3/26/443
Again, Fixes: tag? IOW, which commit initially broke it.
Signed-off-by: Sergei Trofimovich
---
mm/page_alloc.c | 30 +-
1 file changed, 17 insertions(+), 13 deletions(-)
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index d57d9b4f7089..10a8a1d28c11 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -764,32 +764,36 @@ static inline void clear_page_guard(struct zone *zone,
struct page *page,
*/
void init_mem_debugging_and_hardening(void)
{
+ bool page_poison_requested = page_poisoning_enabled();
s/page_poison_requested/page_poisoning_requested/
And I wonder if you should just initialize to "false" here.
Without CONFIG_PAGE_POISONING, page_poisoning_enabled() will always
return false, so it seems unnecessary.
+
+#ifdef CONFIG_PAGE_POISONING
+ /*
+* Page poisoning is debug page alloc for some arches. If
+* either of those options are enabled, enable poisoning.
+*/
+ if (page_poisoning_enabled() ||
+(!IS_ENABLED(CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC) &&
+ debug_pagealloc_enabled())) {
+ static_branch_enable(&_page_poisoning_enabled);
+ page_poison_requested = true;
+ }
+#endif
Apart from that, looks good.
--
Thanks,
David / dhildenb
[PATCH 5.11 009/254] ALSA: hda: ignore invalid NHLT table
From: Mark Pearson
[ Upstream commit a14a6219996ee6f6e858d83b11affc7907633687 ]
On some Lenovo systems if the microphone is disabled in the BIOS
only the NHLT table header is created, with no data. This means
the endpoints field is not correctly set to zero - leading to an
unintialised variable and hence invalid descriptors are parsed
leading to page faults.
The Lenovo firmware team is addressing this, but adding a check
preventing invalid tables being parsed is worthwhile.
Tested on a Lenovo T14.
Tested-by: Philipp Leskovitz
Reported-by: Philipp Leskovitz
Signed-off-by: Mark Pearson
Reviewed-by: Pierre-Louis Bossart
Link: https://lore.kernel.org/r/20210302141003.7342-1-markpear...@lenovo.com
Signed-off-by: Takashi Iwai
Signed-off-by: Sasha Levin
---
sound/hda/intel-nhlt.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/sound/hda/intel-nhlt.c b/sound/hda/intel-nhlt.c
index d053beccfaec..e2237239d922 100644
--- a/sound/hda/intel-nhlt.c
+++ b/sound/hda/intel-nhlt.c
@@ -39,6 +39,11 @@ int intel_nhlt_get_dmic_geo(struct device *dev, struct
nhlt_acpi_table *nhlt)
if (!nhlt)
return 0;
+ if (nhlt->header.length <= sizeof(struct acpi_table_header)) {
+ dev_warn(dev, "Invalid DMIC description table\n");
+ return 0;
+ }
+
for (j = 0, epnt = nhlt->desc; j < nhlt->endpoint_count; j++,
epnt = (struct nhlt_endpoint *)((u8 *)epnt + epnt->length)) {
--
2.30.1
[PATCH 5.10 010/221] ALSA: hda: ignore invalid NHLT table
From: Mark Pearson
[ Upstream commit a14a6219996ee6f6e858d83b11affc7907633687 ]
On some Lenovo systems if the microphone is disabled in the BIOS
only the NHLT table header is created, with no data. This means
the endpoints field is not correctly set to zero - leading to an
unintialised variable and hence invalid descriptors are parsed
leading to page faults.
The Lenovo firmware team is addressing this, but adding a check
preventing invalid tables being parsed is worthwhile.
Tested on a Lenovo T14.
Tested-by: Philipp Leskovitz
Reported-by: Philipp Leskovitz
Signed-off-by: Mark Pearson
Reviewed-by: Pierre-Louis Bossart
Link: https://lore.kernel.org/r/20210302141003.7342-1-markpear...@lenovo.com
Signed-off-by: Takashi Iwai
Signed-off-by: Sasha Levin
---
sound/hda/intel-nhlt.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/sound/hda/intel-nhlt.c b/sound/hda/intel-nhlt.c
index d053beccfaec..e2237239d922 100644
--- a/sound/hda/intel-nhlt.c
+++ b/sound/hda/intel-nhlt.c
@@ -39,6 +39,11 @@ int intel_nhlt_get_dmic_geo(struct device *dev, struct
nhlt_acpi_table *nhlt)
if (!nhlt)
return 0;
+ if (nhlt->header.length <= sizeof(struct acpi_table_header)) {
+ dev_warn(dev, "Invalid DMIC description table\n");
+ return 0;
+ }
+
for (j = 0, epnt = nhlt->desc; j < nhlt->endpoint_count; j++,
epnt = (struct nhlt_endpoint *)((u8 *)epnt + epnt->length)) {
--
2.30.1
[PATCH 5.4 007/111] ALSA: hda: ignore invalid NHLT table
From: Mark Pearson
[ Upstream commit a14a6219996ee6f6e858d83b11affc7907633687 ]
On some Lenovo systems if the microphone is disabled in the BIOS
only the NHLT table header is created, with no data. This means
the endpoints field is not correctly set to zero - leading to an
unintialised variable and hence invalid descriptors are parsed
leading to page faults.
The Lenovo firmware team is addressing this, but adding a check
preventing invalid tables being parsed is worthwhile.
Tested on a Lenovo T14.
Tested-by: Philipp Leskovitz
Reported-by: Philipp Leskovitz
Signed-off-by: Mark Pearson
Reviewed-by: Pierre-Louis Bossart
Link: https://lore.kernel.org/r/20210302141003.7342-1-markpear...@lenovo.com
Signed-off-by: Takashi Iwai
Signed-off-by: Sasha Levin
---
sound/hda/intel-nhlt.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/sound/hda/intel-nhlt.c b/sound/hda/intel-nhlt.c
index baeda6c9716a..6ed80a4cba01 100644
--- a/sound/hda/intel-nhlt.c
+++ b/sound/hda/intel-nhlt.c
@@ -72,6 +72,11 @@ int intel_nhlt_get_dmic_geo(struct device *dev, struct
nhlt_acpi_table *nhlt)
if (!nhlt)
return 0;
+ if (nhlt->header.length <= sizeof(struct acpi_table_header)) {
+ dev_warn(dev, "Invalid DMIC description table\n");
+ return 0;
+ }
+
for (j = 0, epnt = nhlt->desc; j < nhlt->endpoint_count; j++,
epnt = (struct nhlt_endpoint *)((u8 *)epnt + epnt->length)) {
--
2.30.1
[PATCH v2] mm: page_alloc: ignore init_on_free=1 for debug_pagealloc=1
On !ARCH_SUPPORTS_DEBUG_PAGEALLOC (like ia64) debug_pagealloc=1
implies page_poison=on:
if (page_poisoning_enabled() ||
(!IS_ENABLED(CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC) &&
debug_pagealloc_enabled()))
static_branch_enable(&_page_poisoning_enabled);
page_poison=on needs to init_on_free=1.
Before the change id happened too late for the following case:
- have PAGE_POISONING=y
- have page_poison unset
- have !ARCH_SUPPORTS_DEBUG_PAGEALLOC arch (like ia64)
- have init_on_free=1
- have debug_pagealloc=1
That way we get both keys enabled:
- static_branch_enable(_on_free);
- static_branch_enable(&_page_poisoning_enabled);
which leads to poisoned pages returned for __GFP_ZERO pages.
After the change we execute only:
- static_branch_enable(&_page_poisoning_enabled);
and ignore init_on_free=1.
CC: Vlastimil Babka
CC: Andrew Morton
CC: linux...@kvack.org
CC: David Hildenbrand
CC: Andrey Konovalov
Link: https://lkml.org/lkml/2021/3/26/443
Signed-off-by: Sergei Trofimovich
---
mm/page_alloc.c | 30 +-
1 file changed, 17 insertions(+), 13 deletions(-)
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index d57d9b4f7089..10a8a1d28c11 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -764,32 +764,36 @@ static inline void clear_page_guard(struct zone *zone,
struct page *page,
*/
void init_mem_debugging_and_hardening(void)
{
+ bool page_poison_requested = page_poisoning_enabled();
+
+#ifdef CONFIG_PAGE_POISONING
+ /*
+* Page poisoning is debug page alloc for some arches. If
+* either of those options are enabled, enable poisoning.
+*/
+ if (page_poisoning_enabled() ||
+(!IS_ENABLED(CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC) &&
+ debug_pagealloc_enabled())) {
+ static_branch_enable(&_page_poisoning_enabled);
+ page_poison_requested = true;
+ }
+#endif
+
if (_init_on_alloc_enabled_early) {
- if (page_poisoning_enabled())
+ if (page_poison_requested)
pr_info("mem auto-init: CONFIG_PAGE_POISONING is on, "
"will take precedence over init_on_alloc\n");
else
static_branch_enable(_on_alloc);
}
if (_init_on_free_enabled_early) {
- if (page_poisoning_enabled())
+ if (page_poison_requested)
pr_info("mem auto-init: CONFIG_PAGE_POISONING is on, "
"will take precedence over init_on_free\n");
else
static_branch_enable(_on_free);
}
-#ifdef CONFIG_PAGE_POISONING
- /*
-* Page poisoning is debug page alloc for some arches. If
-* either of those options are enabled, enable poisoning.
-*/
- if (page_poisoning_enabled() ||
-(!IS_ENABLED(CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC) &&
- debug_pagealloc_enabled()))
- static_branch_enable(&_page_poisoning_enabled);
-#endif
-
#ifdef CONFIG_DEBUG_PAGEALLOC
if (!debug_pagealloc_enabled())
return;
--
2.31.0
Re: [PATCH] mm: page_alloc: ignore init_on_free=1 for page alloc
On Fri, 26 Mar 2021 17:25:22 +
Sergei Trofimovich wrote:
> On Fri, 26 Mar 2021 15:17:00 +0100
> Vlastimil Babka wrote:
>
> > On 3/26/21 12:26 PM, Sergei Trofimovich wrote:
> > > init_on_free=1 does not guarantee that free pages contain only zero bytes.
> > >
> > > Some examples:
> > > 1. page_poison=on takes presedence over init_on_alloc=1 / ini_on_free=1
> > >
> >
> > Yes, and it spits out a message that you enabled both and poisoning takes
> > precedence. It was that way even before my changes IIRC, but not
> > consistent.
>
> Yeah. I probably should not have included this case as page_poison=on actually
> made my machine boot just fine. My main focus was to understand why I an
> seeing
> the crash on kernel with init_on_alloc=1 init_on_free=1 and most debugging
> options
> on.
>
> My apologies! I'll try to find where this extra poisoning comes from.
>
> Making a step back and explaining my setup:
>
> Initially it's an ia64 box that manages to consistently corrupt memory
> on socket free; https://lkml.org/lkml/2021/2/23/653
>
> To get better understanding where corruption comes from I enabled
> A Lot of VM, pagealloc and slab debugging options. Full config:
>
>
> https://dev.gentoo.org/~slyfox/configs/guppy-config-5.12.0-rc4-00016-g427684abc9fd-dirty
>
> I boot machine as:
>
> [0.00] Kernel command line:
> BOOT_IMAGE=/boot/vmlinuz-5.12.0-rc4-00016-g427684abc9fd-dirty root=/dev/sda3
> ro slab_nomerge memblock=debug debug_pagealloc=1 hardened_usercopy=1
> page_owner=on page_poison=0 init_on_alloc=1 init_on_free=1
> debug_guardpage_minorder=0
>
> My boot log:
>
>
> https://dev.gentoo.org/~slyfox/bugs/ia64-boot-bug/2021-03-26-init_on_alloc-fail
>
> Caveats in reading boot log:
> - kernel crashes too early: stack unwinder does not have working
> kmalloc() yet
> - kernel crashes in MCE handler: normally it should not. It's an
> unrelated bug
> that makes backtrace useless. I'll try to fix it later, but it will not
> be fast.
> - I added a bunch of printk()s around the crash.
>
> The important pernel boot failure part is:
> [0.00] put_kernel_page: pmd=e001
> [0.00] pmd:(ptrval):
>
I added WARN_ON_ONCE(1) to __kernel_poison_pages() to get the idea where
poisoning comes from and got it at:
[0.00] [ cut here ]
[0.00] WARNING: CPU: 0 PID: 0 at mm/page_poison.c:40
__kernel_poison_pages+0x1a0/0x1c0
[0.00] Modules linked in:
[0.00] CPU: 0 PID: 0 Comm: swapper Not tainted
5.12.0-rc4-00016-g427684abc9fd-dirty #196
Call Trace:
[0.00] [] show_stack+0x90/0xc0
[0.00] [] dump_stack+0x150/0x1c0
[0.00] [] __warn+0x180/0x220
[0.00] [] warn_slowpath_fmt+0xc0/0x100
[0.00] [] __kernel_poison_pages+0x1a0/0x1c0
[0.00] [] __free_pages_ok+0x2a0/0x10c0
[0.00] [] __free_pages_core+0x2d0/0x480
[0.00] [] memblock_free_pages+0x30/0x50
[0.00] [] memblock_free_all+0x280/0x3c0
[0.00] [] mem_init+0x70/0x2d0
[0.00] [] start_kernel+0x670/0xc20
[0.00] [] start_ap+0x760/0x780
[0.00] ---[ end trace ]---
I think I found where page_poison=on get enabled at
init_mem_debugging_and_hardening():
void init_mem_debugging_and_hardening(void)
{
if (_init_on_alloc_enabled_early) {
if (page_poisoning_enabled())
pr_info("mem auto-init: CONFIG_PAGE_POISONING is on, "
"will take precedence over init_on_alloc\n");
else
static_branch_enable(_on_alloc);
}
if (_init_on_free_enabled_early) {
if (page_poisoning_enabled())
pr_info("mem auto-init: CONFIG_PAGE_POISONING is on, "
"will take precedence over init_on_free\n");
else
static_branch_enable(_on_free);
}
#ifdef CONFIG_PAGE_POISONING
/*
* Page poisoning is debug page alloc for some arches. If
* either of those options are enabled, enable poisoning.
*/
if (page_poisoning_enabled() ||
(!IS_ENABLED(CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC) &&
debug_pagealloc_enabled()))
static_branch_enable(&_page_poisoning_enabled); // <- HERE
#endif
...
}
If I follow the code correctly to trigger the problem one needs to:
- have PAGE_POISONING=y
- have page_poison=off set (or just unset)
- have arch without ARCH_SUPPORTS_DEBUG_PAGEALLOC (ia64 is one of
such arches)
- have init_on_free=1
- have debug_pagealloc=1
That way we get both executed:
- static_branch_enable(_on_free);
- static_branch_enable(&_page_poisoning_enabled);
Sounds plausible? I'll send another version
Re: [PATCH] mm: page_alloc: ignore init_on_free=1 for page alloc
On Fri, 26 Mar 2021 15:17:00 +0100
Vlastimil Babka wrote:
> On 3/26/21 12:26 PM, Sergei Trofimovich wrote:
> > init_on_free=1 does not guarantee that free pages contain only zero bytes.
> >
> > Some examples:
> > 1. page_poison=on takes presedence over init_on_alloc=1 / ini_on_free=1
>
> Yes, and it spits out a message that you enabled both and poisoning takes
> precedence. It was that way even before my changes IIRC, but not consistent.
Yeah. I probably should not have included this case as page_poison=on actually
made my machine boot just fine. My main focus was to understand why I an seeing
the crash on kernel with init_on_alloc=1 init_on_free=1 and most debugging
options
on.
My apologies! I'll try to find where this extra poisoning comes from.
Making a step back and explaining my setup:
Initially it's an ia64 box that manages to consistently corrupt memory
on socket free; https://lkml.org/lkml/2021/2/23/653
To get better understanding where corruption comes from I enabled
A Lot of VM, pagealloc and slab debugging options. Full config:
https://dev.gentoo.org/~slyfox/configs/guppy-config-5.12.0-rc4-00016-g427684abc9fd-dirty
I boot machine as:
[0.00] Kernel command line:
BOOT_IMAGE=/boot/vmlinuz-5.12.0-rc4-00016-g427684abc9fd-dirty root=/dev/sda3 ro
slab_nomerge memblock=debug debug_pagealloc=1 hardened_usercopy=1 page_owner=on
page_poison=0 init_on_alloc=1 init_on_free=1 debug_guardpage_minorder=0
My boot log:
https://dev.gentoo.org/~slyfox/bugs/ia64-boot-bug/2021-03-26-init_on_alloc-fail
Caveats in reading boot log:
- kernel crashes too early: stack unwinder does not have working kmalloc()
yet
- kernel crashes in MCE handler: normally it should not. It's an unrelated
bug
that makes backtrace useless. I'll try to fix it later, but it will not
be fast.
- I added a bunch of printk()s around the crash.
The important pernel boot failure part is:
[0.00] put_kernel_page: pmd=e001
[0.00] pmd:(ptrval):
Note 1: I do not really enable page_poison at runtime and was misleading you
in previous emails. (I initially assumed kernel_poison_pages() poisons pages
unconditionally but you all explained it does not). Something else manages to
poison my pmd(s?).
Note 2: I have many other debugging options enabled that might trigger
poisoning.
> > 2. free_pages_prepare() always poisons pages:
> >
> >if (want_init_on_free())
> >kernel_init_free_pages(page, 1 << order);
> >kernel_poison_pages(page, 1 << order
>
> kernel_poison_pages() includes a test if poisoning is enabled. And in that
> case
> want_init_on_free() shouldn't be. see init_mem_debugging_and_hardening()
I completely missed that! Thank you! Will try to trace real cause of poisoning.
> > I observed use of poisoned pages as the crash on ia64 booted with
> > init_on_free=1 init_on_alloc=1 (CONFIG_PAGE_POISONING=y config).
> > There pmd page contained 0x poison pages and led to early crash.
>
> Hm but that looks lika a sign that ia64 pmd allocation should use __GFP_ZERO
> and
> doesn't. It shouldn't rely on init_on_alloc or init_on_free being enabled.
ia64 does use __GFP_ZERO (I even tried to add it manually to pmd_alloc_one()
before I realized all _PGTABLEs imply __GFP_ZERO).
I'll provide the call chain I arrived at for completeness:
- [ia64 boots]
- mem_init() (defined at arch/ia64/mm/init.c)
-> setup_gate() (defined at arch/ia64/mm/init.c)
-> put_kernel_page() (defined at arch/ia64/mm/init.c)
-> [NOTE: from now on it's all generic code, not ia64-speficic]
-> pmd_alloc() (defined at include/linux/mm.h)
-> __pmd_alloc() (defined at mm/memory.c)
-> [under #ifndef __PAGETABLE_PMD_FOLDED] pmd_alloc_one() (defined at
include/asm-generic/pgalloc.h)
-> pmd_alloc_one() [defined at include/asm-generic/pgalloc.h):
static inline pmd_t *pmd_alloc_one(struct mm_struct *mm, unsigned long addr)
{
struct page *page;
gfp_t gfp = GFP_PGTABLE_USER;
if (mm == _mm)
gfp = GFP_PGTABLE_KERNEL;
page = alloc_pages(gfp, 0);
if (!page)
return NULL;
if (!pgtable_pmd_page_ctor(page)) {
__free_pages(page, 0);
return NULL;
}
return (pmd_t *)page_address(page);
}
In our case it is a GFP_PGTABLE_KERNEL with __GFP_ZERO and result is
poisoned page instead of zeroed page.
If I interpret the above correctly it means that something (probably
memalloc_free_pages() ?) puts initial free pages as poisoned and later
alloc_pages() assumes they are memset()-zero. But I don't see why.
> > The change drops the assumption that init_on_free=1 guarantees free
> > pages to contain zeros.
>
> The change assumes that page_poison=on also leaves want_init_on_free()
>
Re: [PATCH] mm: page_alloc: ignore init_on_free=1 for page alloc
On Fri, 26 Mar 2021 16:00:34 +0100 Andrey Konovalov wrote: > On Fri, Mar 26, 2021 at 2:49 PM David Hildenbrand wrote: > > > > > I observed use of poisoned pages as the crash on ia64 booted with > > > init_on_free=1 init_on_alloc=1 (CONFIG_PAGE_POISONING=y config). > > > There pmd page contained 0x poison pages and led to early crash. > > > > > > The change drops the assumption that init_on_free=1 guarantees free > > > pages to contain zeros. > > > > > > Alternative would be to make interaction between runtime poisoning and > > > sanitizing options and build-time debug flags like CONFIG_PAGE_POISONING > > > more coherent. I took the simpler path. > > > > > > > I thought latest work be Vlastimil tried to tackle that. To me, it feels > > like page_poison=on and init_on_free=1 should bail out and disable one > > of both things. Having both at the same time doesn't sound helpful. > > This is exactly how it works, see init_mem_debugging_and_hardening(). > > Sergei, could you elaborate more on what kind of crash this patch is > trying to fix? Where does it happen and why? Yeah, I see I misinterpreted page_poison=on handling and misled you all. Something else poisons a page when it should have not. I'll answer in more detail to Vlastimil's email upthread and will provide more detail of the unexpected poisoning I see. -- Sergei
Re: [PATCH] mm: page_alloc: ignore init_on_free=1 for page alloc
On Fri, Mar 26, 2021 at 2:49 PM David Hildenbrand wrote: > > > I observed use of poisoned pages as the crash on ia64 booted with > > init_on_free=1 init_on_alloc=1 (CONFIG_PAGE_POISONING=y config). > > There pmd page contained 0x poison pages and led to early crash. > > > > The change drops the assumption that init_on_free=1 guarantees free > > pages to contain zeros. > > > > Alternative would be to make interaction between runtime poisoning and > > sanitizing options and build-time debug flags like CONFIG_PAGE_POISONING > > more coherent. I took the simpler path. > > > > I thought latest work be Vlastimil tried to tackle that. To me, it feels > like page_poison=on and init_on_free=1 should bail out and disable one > of both things. Having both at the same time doesn't sound helpful. This is exactly how it works, see init_mem_debugging_and_hardening(). Sergei, could you elaborate more on what kind of crash this patch is trying to fix? Where does it happen and why?
Re: [PATCH] mm: page_alloc: ignore init_on_free=1 for page alloc
On 3/26/21 12:26 PM, Sergei Trofimovich wrote: > init_on_free=1 does not guarantee that free pages contain only zero bytes. > > Some examples: > 1. page_poison=on takes presedence over init_on_alloc=1 / ini_on_free=1 Yes, and it spits out a message that you enabled both and poisoning takes precedence. It was that way even before my changes IIRC, but not consistent. > 2. free_pages_prepare() always poisons pages: > >if (want_init_on_free()) >kernel_init_free_pages(page, 1 << order); >kernel_poison_pages(page, 1 << order kernel_poison_pages() includes a test if poisoning is enabled. And in that case want_init_on_free() shouldn't be. see init_mem_debugging_and_hardening() > > I observed use of poisoned pages as the crash on ia64 booted with > init_on_free=1 init_on_alloc=1 (CONFIG_PAGE_POISONING=y config). > There pmd page contained 0x poison pages and led to early crash. Hm but that looks lika a sign that ia64 pmd allocation should use __GFP_ZERO and doesn't. It shouldn't rely on init_on_alloc or init_on_free being enabled. > The change drops the assumption that init_on_free=1 guarantees free > pages to contain zeros. The change assumes that page_poison=on also leaves want_init_on_free() enabled, but it doesn't. > Alternative would be to make interaction between runtime poisoning and > sanitizing options and build-time debug flags like CONFIG_PAGE_POISONING > more coherent. I took the simpler path. So that was done in 5.11 and the decisions can be seen in init_mem_debugging_and_hardening(). There might be of course a bug, or later changes broke something. Which was the version that you observed a bug? > Tested the fix on rx3600. > > CC: Andrew Morton > CC: linux...@kvack.org > Signed-off-by: Sergei Trofimovich > --- > mm/page_alloc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > index cfc72873961d..d57d9b4f7089 100644 > --- a/mm/page_alloc.c > +++ b/mm/page_alloc.c > @@ -2301,7 +2301,7 @@ inline void post_alloc_hook(struct page *page, unsigned > int order, > kernel_unpoison_pages(page, 1 << order); > set_page_owner(page, order, gfp_flags); > > - if (!want_init_on_free() && want_init_on_alloc(gfp_flags)) > + if (want_init_on_alloc(gfp_flags)) > kernel_init_free_pages(page, 1 << order); > } > >
Re: [PATCH] mm: page_alloc: ignore init_on_free=1 for page alloc
On 26.03.21 12:26, Sergei Trofimovich wrote: init_on_free=1 does not guarantee that free pages contain only zero bytes. Some examples: 1. page_poison=on takes presedence over init_on_alloc=1 / ini_on_free=1 s/ini_on_free/init_on_free/ 2. free_pages_prepare() always poisons pages: if (want_init_on_free()) kernel_init_free_pages(page, 1 << order); kernel_poison_pages(page, 1 << order In next/master, it's the other way around already. commit 855a9c4018f3219db8be7e4b9a65ab22aebfde82 Author: Andrey Konovalov Date: Thu Mar 18 17:01:40 2021 +1100 kasan, mm: integrate page_alloc init with HW_TAGS I observed use of poisoned pages as the crash on ia64 booted with init_on_free=1 init_on_alloc=1 (CONFIG_PAGE_POISONING=y config). There pmd page contained 0x poison pages and led to early crash. The change drops the assumption that init_on_free=1 guarantees free pages to contain zeros. Alternative would be to make interaction between runtime poisoning and sanitizing options and build-time debug flags like CONFIG_PAGE_POISONING more coherent. I took the simpler path. I thought latest work be Vlastimil tried to tackle that. To me, it feels like page_poison=on and init_on_free=1 should bail out and disable one of both things. Having both at the same time doesn't sound helpful. Tested the fix on rx3600. Fixes: ? -- Thanks, David / dhildenb
[PATCH] mm: page_alloc: ignore init_on_free=1 for page alloc
init_on_free=1 does not guarantee that free pages contain only zero bytes. Some examples: 1. page_poison=on takes presedence over init_on_alloc=1 / ini_on_free=1 2. free_pages_prepare() always poisons pages: if (want_init_on_free()) kernel_init_free_pages(page, 1 << order); kernel_poison_pages(page, 1 << order I observed use of poisoned pages as the crash on ia64 booted with init_on_free=1 init_on_alloc=1 (CONFIG_PAGE_POISONING=y config). There pmd page contained 0x poison pages and led to early crash. The change drops the assumption that init_on_free=1 guarantees free pages to contain zeros. Alternative would be to make interaction between runtime poisoning and sanitizing options and build-time debug flags like CONFIG_PAGE_POISONING more coherent. I took the simpler path. Tested the fix on rx3600. CC: Andrew Morton CC: linux...@kvack.org Signed-off-by: Sergei Trofimovich --- mm/page_alloc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mm/page_alloc.c b/mm/page_alloc.c index cfc72873961d..d57d9b4f7089 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -2301,7 +2301,7 @@ inline void post_alloc_hook(struct page *page, unsigned int order, kernel_unpoison_pages(page, 1 << order); set_page_owner(page, order, gfp_flags); - if (!want_init_on_free() && want_init_on_alloc(gfp_flags)) + if (want_init_on_alloc(gfp_flags)) kernel_init_free_pages(page, 1 << order); } -- 2.31.0
[PATCH 3/3] checkpatch: Ignore labels when checking indentation
Goto labels are commonly written in the leftmost column (sometimes with
one space in front), regardless of indentation level. Sometimes they're
on a line of their own, but sometimes the same line is shared with a
normal code statement that then starts at the expected indentation
level. When checking indentation, we should check where that normal
piece of code starts, not where the label starts (there's a separate
INDENTED_LABEL test to check the label itself). Therefore, the
line_stats() function that is used to get indentation level should treat
goto labels like whitespace. The SUSPICIOUS_CODE_INDENT test also needs
to explicitly ignore labels to make sure it doesn't get confused by
them.
Signed-off-by: Julius Werner
---
scripts/checkpatch.pl | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index c1dfc0107be41d..d89367a59e7d37 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1396,8 +1396,12 @@ sub copy_spacing {
sub line_stats {
my ($line) = @_;
- # Drop the diff line leader and expand tabs
+ # Drop the diff line leader
$line =~ s/^.//;
+
+ # Treat labels like whitespace when counting indentation
+ $line =~ s/^( ?$Ident:)/" " x length($1)/e;
+
$line = expand_tabs($line);
# Pick the indent from the front of the line.
@@ -4197,6 +4201,9 @@ sub process {
# Remove any comments
$s_next =~ s/$;//g;
+ # Remove any leading labels
+ $s_next =~ s/\n( ?$Ident:)/"\n" . " " x length($1)/eg;
+
# Skip this check for in case next statement starts
with 'else'
if ($s_next !~ /\s*\belse\b/) {
--
2.29.2
Re: [PATCH v3] cifs: Silently ignore unknown oplock break handle
Vincent Whitchurch writes: > Make SMB2 not print out an error when an oplock break is received for an > unknown handle, similar to SMB1. The debug message which is printed for > these unknown handles may also be misleading, so fix that too. > > The SMB2 lease break path is not affected by this patch. > > Without this, a program which writes to a file from one thread, and > opens, reads, and writes the same file from another thread triggers the > below errors several times a minute when run against a Samba server > configured with "smb2 leases = no". > > CIFS: VFS: \\192.168.0.1 No task to wake, unknown frame received! NumMids 2 > : 424d53fe 0040 0012 .SMB@... > 0010: 0001 > 0020: > 0030: > > Signed-off-by: Vincent Whitchurch > --- > > Notes: > v3: > - Change debug print to Tom Talpey's suggestion > > v2: > - Drop change to lease break > - Rewrite commit message > > fs/cifs/smb2misc.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) Reviewed-by: Paulo Alcantara (SUSE)
Re: [PATCH v3] cifs: Silently ignore unknown oplock break handle
merged into cifs-2.6.git for-next On Fri, Mar 19, 2021 at 9:06 AM Tom Talpey via samba-technical wrote: > > LGTM feel free to add > > Reviewed-By: Tom Talpey > > On 3/19/2021 9:57 AM, Vincent Whitchurch wrote: > > Make SMB2 not print out an error when an oplock break is received for an > > unknown handle, similar to SMB1. The debug message which is printed for > > these unknown handles may also be misleading, so fix that too. > > > > The SMB2 lease break path is not affected by this patch. > > > > Without this, a program which writes to a file from one thread, and > > opens, reads, and writes the same file from another thread triggers the > > below errors several times a minute when run against a Samba server > > configured with "smb2 leases = no". > > > > CIFS: VFS: \\192.168.0.1 No task to wake, unknown frame received! NumMids > > 2 > > : 424d53fe 0040 0012 .SMB@... > > 0010: 0001 > > 0020: > > 0030: > > > > Signed-off-by: Vincent Whitchurch > > --- > > > > Notes: > > v3: > > - Change debug print to Tom Talpey's suggestion > > > > v2: > > - Drop change to lease break > > - Rewrite commit message > > > > fs/cifs/smb2misc.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/fs/cifs/smb2misc.c b/fs/cifs/smb2misc.c > > index 60d4bd1eae2b..76cd05b8d53b 100644 > > --- a/fs/cifs/smb2misc.c > > +++ b/fs/cifs/smb2misc.c > > @@ -754,8 +754,8 @@ smb2_is_valid_oplock_break(char *buffer, struct > > TCP_Server_Info *server) > > } > > } > > spin_unlock(_tcp_ses_lock); > > - cifs_dbg(FYI, "Can not process oplock break for non-existent > > connection\n"); > > - return false; > > + cifs_dbg(FYI, "No file id matched, oplock break ignored\n"); > > + return true; > > } > > > > void > > > -- Thanks, Steve
[PATCH v2 6/7] scripts: get_abi: ignore code blocks for cross-references
The script should not generate cross-references inside
literal blocks.
Signed-off-by: Mauro Carvalho Chehab
---
scripts/get_abi.pl | 19 +++
1 file changed, 19 insertions(+)
diff --git a/scripts/get_abi.pl b/scripts/get_abi.pl
index e5d1da492c1e..d7aa82094296 100755
--- a/scripts/get_abi.pl
+++ b/scripts/get_abi.pl
@@ -382,8 +382,27 @@ sub output_rest {
# Enrich text by creating cross-references
my $new_desc = "";
+ my $init_indent = -1;
+ my $literal_indent = -1;
+
open(my $fh, "+<", \$desc);
while (my $d = <$fh>) {
+ my $indent = $d =~ m/^(\s+)/;
+ my $spaces = length($indent);
+ $init_indent = $indent if ($init_indent
< 0);
+ if ($literal_indent >= 0) {
+ if ($spaces > $literal_indent) {
+ $new_desc .= $d;
+ next;
+ } else {
+ $literal_indent = -1;
+ }
+ } else {
+ if ($d =~ /()::$/ && !($d =~
/^\s*\.\./)) {
+ $literal_indent =
$spaces;
+ }
+ }
+
$d =~
s,Documentation/(?!devicetree)(\S+)\.rst,:doc:`/$1`,g;
my @matches = $d =~
m,Documentation/ABI/([\w\/\-]+),g;
--
2.30.2
Re: [PATCH 1/2] media: v4l2-core: ignore native time32 ioctls on 64-bit
On 21/03/2021 09:50, Hans Verkuil wrote:
> Hi Arnd,
>> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c
>> b/drivers/media/v4l2-core/v4l2-subdev.c
>> index 336133dbc759..9f5573d3b857 100644
>> --- a/drivers/media/v4l2-core/v4l2-subdev.c
>> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
>> @@ -428,7 +428,7 @@ static long subdev_do_ioctl(struct file *file, unsigned
>> int cmd, void *arg)
>>
>> return v4l2_event_dequeue(vfh, arg, file->f_flags & O_NONBLOCK);
>>
>> -#ifdef CONFIG_COMPAT_32BIT_TIME
>> +#if !defined(CONFIG_64BIT) && defined(CONFIG_COMPAT_32BIT_TIME)
>> case VIDIOC_DQEVENT_TIME32: {
>> struct v4l2_event_time32 *ev32 = arg;
>> struct v4l2_event ev = { };
>>
>
> This chunk doesn't apply since there is no '#ifdef CONFIG_COMPAT_32BIT_TIME'
> in
> either the mainline kernel or the media_tree master branch.
>
> Are we missing a patch for v4l2-subdev.c?
Ping!
Hans
>
> Regards,
>
> Hans
>
