* [PATCH 01/14] staging: gasket: fix check_and_invoke_callback log param
2018-07-21 13:34 [PATCH 00/14] staging: gasket: more assorted cleanups Todd Poynor
@ 2018-07-21 13:34 ` Todd Poynor
2018-07-21 13:34 ` [PATCH 02/14] staging: gasket: remove duplicate call to retrieve device callback Todd Poynor
` (13 subsequent siblings)
14 siblings, 0 replies; 17+ messages in thread
From: Todd Poynor @ 2018-07-21 13:34 UTC (permalink / raw)
To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
Cc: Zhongze Hu, Dmitry Torokhov, Guenter Roeck, devel, linux-kernel,
Todd Poynor
From: Todd Poynor <toddpoynor@google.com>
The message should be passed the callback function pointer, not
the pointer to the gasket device.
Signed-off-by: Zhongze Hu <frankhu@chromium.org>
Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
drivers/staging/gasket/gasket_core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c
index 40e46ca5228c8..2cd232230845c 100644
--- a/drivers/staging/gasket/gasket_core.c
+++ b/drivers/staging/gasket/gasket_core.c
@@ -205,7 +205,7 @@ static inline int check_and_invoke_callback(
{
int ret = 0;
- gasket_nodev_error("check_and_invoke_callback %p", gasket_dev);
+ gasket_nodev_error("check_and_invoke_callback %p", cb_function);
if (cb_function) {
mutex_lock(&gasket_dev->mutex);
ret = cb_function(gasket_dev);
--
2.18.0.233.g985f88cf7e-goog
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 02/14] staging: gasket: remove duplicate call to retrieve device callback
2018-07-21 13:34 [PATCH 00/14] staging: gasket: more assorted cleanups Todd Poynor
2018-07-21 13:34 ` [PATCH 01/14] staging: gasket: fix check_and_invoke_callback log param Todd Poynor
@ 2018-07-21 13:34 ` Todd Poynor
2018-07-21 13:34 ` [PATCH 03/14] staging: gasket: gasket_handle_ioctl fix ioctl exit trace param Todd Poynor
` (12 subsequent siblings)
14 siblings, 0 replies; 17+ messages in thread
From: Todd Poynor @ 2018-07-21 13:34 UTC (permalink / raw)
To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
Cc: Zhongze Hu, Dmitry Torokhov, Guenter Roeck, devel, linux-kernel,
Todd Poynor
From: Todd Poynor <toddpoynor@google.com>
gasket_handle_ioctl() calls gasket_get_ioctl_permissions_cb() twice;
simplify the code and avoid duplicated work by fetching the callback
pointer only once.
Reported-by: Dmitry Torokhov <dtor@chromium.org>
Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
drivers/staging/gasket/gasket_ioctl.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/gasket/gasket_ioctl.c b/drivers/staging/gasket/gasket_ioctl.c
index 2e2c9b997093b..dbe9fdef0c268 100644
--- a/drivers/staging/gasket/gasket_ioctl.c
+++ b/drivers/staging/gasket/gasket_ioctl.c
@@ -55,14 +55,15 @@ long gasket_handle_ioctl(struct file *filp, uint cmd, void __user *argp)
{
struct gasket_dev *gasket_dev;
unsigned long arg = (unsigned long)argp;
+ gasket_ioctl_permissions_cb_t ioctl_permissions_cb;
int retval;
gasket_dev = (struct gasket_dev *)filp->private_data;
trace_gasket_ioctl_entry(gasket_dev->dev_info.name, cmd);
- if (gasket_get_ioctl_permissions_cb(gasket_dev)) {
- retval = gasket_get_ioctl_permissions_cb(gasket_dev)(
- filp, cmd, argp);
+ ioctl_permissions_cb = gasket_get_ioctl_permissions_cb(gasket_dev);
+ if (ioctl_permissions_cb) {
+ retval = ioctl_permissions_cb(filp, cmd, argp);
if (retval < 0) {
trace_gasket_ioctl_exit(-EPERM);
return retval;
--
2.18.0.233.g985f88cf7e-goog
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 03/14] staging: gasket: gasket_handle_ioctl fix ioctl exit trace param
2018-07-21 13:34 [PATCH 00/14] staging: gasket: more assorted cleanups Todd Poynor
2018-07-21 13:34 ` [PATCH 01/14] staging: gasket: fix check_and_invoke_callback log param Todd Poynor
2018-07-21 13:34 ` [PATCH 02/14] staging: gasket: remove duplicate call to retrieve device callback Todd Poynor
@ 2018-07-21 13:34 ` Todd Poynor
2018-07-21 13:34 ` [PATCH 04/14] staging: gasket: avoid copy to user on error in coherent alloc config Todd Poynor
` (11 subsequent siblings)
14 siblings, 0 replies; 17+ messages in thread
From: Todd Poynor @ 2018-07-21 13:34 UTC (permalink / raw)
To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
Cc: Zhongze Hu, Dmitry Torokhov, Guenter Roeck, devel, linux-kernel,
Todd Poynor
From: Todd Poynor <toddpoynor@google.com>
Pass the return value from the device ioctl permissions callback to the
tracepoint when the callback returns an error.
Reported-by: Dmitry Torokhov <dtor@chromium.org>
Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
drivers/staging/gasket/gasket_ioctl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/gasket/gasket_ioctl.c b/drivers/staging/gasket/gasket_ioctl.c
index dbe9fdef0c268..1b164ac7a0496 100644
--- a/drivers/staging/gasket/gasket_ioctl.c
+++ b/drivers/staging/gasket/gasket_ioctl.c
@@ -65,7 +65,7 @@ long gasket_handle_ioctl(struct file *filp, uint cmd, void __user *argp)
if (ioctl_permissions_cb) {
retval = ioctl_permissions_cb(filp, cmd, argp);
if (retval < 0) {
- trace_gasket_ioctl_exit(-EPERM);
+ trace_gasket_ioctl_exit(retval);
return retval;
} else if (retval == 0) {
trace_gasket_ioctl_exit(-EPERM);
--
2.18.0.233.g985f88cf7e-goog
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 04/14] staging: gasket: avoid copy to user on error in coherent alloc config
2018-07-21 13:34 [PATCH 00/14] staging: gasket: more assorted cleanups Todd Poynor
` (2 preceding siblings ...)
2018-07-21 13:34 ` [PATCH 03/14] staging: gasket: gasket_handle_ioctl fix ioctl exit trace param Todd Poynor
@ 2018-07-21 13:34 ` Todd Poynor
2018-07-21 13:34 ` [PATCH 05/14] staging: gasket: print mmap starting address as unsigned long Todd Poynor
` (10 subsequent siblings)
14 siblings, 0 replies; 17+ messages in thread
From: Todd Poynor @ 2018-07-21 13:34 UTC (permalink / raw)
To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
Cc: Zhongze Hu, Dmitry Torokhov, Guenter Roeck, devel, linux-kernel,
Todd Poynor
From: Todd Poynor <toddpoynor@google.com>
gasket_config_coherent_allocator() on error return the error to caller
without copying a possibly-update DMA address back to userspace.
Reported-by: Dmitry Torokhov <dtor@chromium.org>
Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
drivers/staging/gasket/gasket_ioctl.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/gasket/gasket_ioctl.c b/drivers/staging/gasket/gasket_ioctl.c
index 1b164ac7a0496..8cf094b90cdb0 100644
--- a/drivers/staging/gasket/gasket_ioctl.c
+++ b/drivers/staging/gasket/gasket_ioctl.c
@@ -441,8 +441,10 @@ static int gasket_config_coherent_allocator(
gasket_dev, ibuf.size, &ibuf.dma_address,
ibuf.page_table_index);
}
+ if (ret)
+ return ret;
if (copy_to_user(argp, &ibuf, sizeof(ibuf)))
return -EFAULT;
- return ret;
+ return 0;
}
--
2.18.0.233.g985f88cf7e-goog
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 05/14] staging: gasket: print mmap starting address as unsigned long
2018-07-21 13:34 [PATCH 00/14] staging: gasket: more assorted cleanups Todd Poynor
` (3 preceding siblings ...)
2018-07-21 13:34 ` [PATCH 04/14] staging: gasket: avoid copy to user on error in coherent alloc config Todd Poynor
@ 2018-07-21 13:34 ` Todd Poynor
2018-07-21 13:34 ` [PATCH 06/14] staging: gasket: remove unnecessary NULL checks on calls from VFS Todd Poynor
` (9 subsequent siblings)
14 siblings, 0 replies; 17+ messages in thread
From: Todd Poynor @ 2018-07-21 13:34 UTC (permalink / raw)
To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
Cc: Zhongze Hu, Dmitry Torokhov, Guenter Roeck, devel, linux-kernel,
Todd Poynor
From: Todd Poynor <toddpoynor@google.com>
Page alignment error log should print the offending value as an unsigned
long, not as a kernel pointer.
Reported-by: Guenter Roeck <groeck@chromium.org>
Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
drivers/staging/gasket/gasket_core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c
index 2cd232230845c..11ab049854493 100644
--- a/drivers/staging/gasket/gasket_core.c
+++ b/drivers/staging/gasket/gasket_core.c
@@ -1596,8 +1596,8 @@ static int gasket_mmap(struct file *filp, struct vm_area_struct *vma)
if (vma->vm_start & ~PAGE_MASK) {
gasket_log_error(
- gasket_dev, "Base address not page-aligned: 0x%p\n",
- (void *)vma->vm_start);
+ gasket_dev, "Base address not page-aligned: 0x%lx\n",
+ vma->vm_start);
trace_gasket_mmap_exit(-EINVAL);
return -EINVAL;
}
--
2.18.0.233.g985f88cf7e-goog
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 06/14] staging: gasket: remove unnecessary NULL checks on calls from VFS
2018-07-21 13:34 [PATCH 00/14] staging: gasket: more assorted cleanups Todd Poynor
` (4 preceding siblings ...)
2018-07-21 13:34 ` [PATCH 05/14] staging: gasket: print mmap starting address as unsigned long Todd Poynor
@ 2018-07-21 13:34 ` Todd Poynor
2018-07-21 13:35 ` [PATCH 07/14] staging: gasket: gasket_get_device drop check for NULL pci_dev Todd Poynor
` (8 subsequent siblings)
14 siblings, 0 replies; 17+ messages in thread
From: Todd Poynor @ 2018-07-21 13:34 UTC (permalink / raw)
To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
Cc: Zhongze Hu, Dmitry Torokhov, Guenter Roeck, devel, linux-kernel,
Todd Poynor
From: Todd Poynor <toddpoynor@google.com>
Remove unneeded checks for NULL pointers in struct file pointers passed
from the VFS layer or the private_data that must have been properly set
at file open time.
Reported-by: Guenter Roeck <groeck@chromium.org>
Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
drivers/staging/gasket/gasket_core.c | 15 ---------------
1 file changed, 15 deletions(-)
diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c
index 11ab049854493..e82f8ce39c9fd 100644
--- a/drivers/staging/gasket/gasket_core.c
+++ b/drivers/staging/gasket/gasket_core.c
@@ -1587,11 +1587,6 @@ static int gasket_mmap(struct file *filp, struct vm_area_struct *vma)
int num_map_regions = 0;
enum do_map_region_status map_status;
- if (!gasket_dev) {
- gasket_nodev_error("Unable to retrieve device data");
- trace_gasket_mmap_exit(-EINVAL);
- return -EINVAL;
- }
driver_desc = gasket_dev->internal_desc->driver_desc;
if (vma->vm_start & ~PAGE_MASK) {
@@ -1785,17 +1780,7 @@ static long gasket_ioctl(struct file *filp, uint cmd, ulong arg)
void __user *argp = (void __user *)arg;
char path[256];
- if (!filp)
- return -ENODEV;
-
gasket_dev = (struct gasket_dev *)filp->private_data;
- if (!gasket_dev) {
- gasket_nodev_error(
- "Unable to find Gasket structure for file %s",
- d_path(&filp->f_path, path, 256));
- return -ENODEV;
- }
-
driver_desc = gasket_dev->internal_desc->driver_desc;
if (!driver_desc) {
gasket_log_error(
--
2.18.0.233.g985f88cf7e-goog
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 07/14] staging: gasket: gasket_get_device drop check for NULL pci_dev
2018-07-21 13:34 [PATCH 00/14] staging: gasket: more assorted cleanups Todd Poynor
` (5 preceding siblings ...)
2018-07-21 13:34 ` [PATCH 06/14] staging: gasket: remove unnecessary NULL checks on calls from VFS Todd Poynor
@ 2018-07-21 13:35 ` Todd Poynor
2018-07-21 13:35 ` [PATCH 08/14] staging: gasket: apex return error on sysfs show of missing attribute Todd Poynor
` (7 subsequent siblings)
14 siblings, 0 replies; 17+ messages in thread
From: Todd Poynor @ 2018-07-21 13:35 UTC (permalink / raw)
To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
Cc: Zhongze Hu, Dmitry Torokhov, Guenter Roeck, devel, linux-kernel,
Todd Poynor
From: Todd Poynor <toddpoynor@google.com>
The pci_dev field of a struct gasket_dev can never be NULL, there's no
need to check for this in gasket_get_device().
Reported-by: Guenter Roeck <groeck@chromium.org>
Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
drivers/staging/gasket/gasket_core.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c
index e82f8ce39c9fd..8265d543623d7 100644
--- a/drivers/staging/gasket/gasket_core.c
+++ b/drivers/staging/gasket/gasket_core.c
@@ -2026,9 +2026,7 @@ const struct gasket_driver_desc *gasket_get_driver_desc(struct gasket_dev *dev)
*/
struct device *gasket_get_device(struct gasket_dev *dev)
{
- if (dev->pci_dev)
- return &dev->pci_dev->dev;
- return NULL;
+ return &dev->pci_dev->dev;
}
/**
--
2.18.0.233.g985f88cf7e-goog
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 08/14] staging: gasket: apex return error on sysfs show of missing attribute
2018-07-21 13:34 [PATCH 00/14] staging: gasket: more assorted cleanups Todd Poynor
` (6 preceding siblings ...)
2018-07-21 13:35 ` [PATCH 07/14] staging: gasket: gasket_get_device drop check for NULL pci_dev Todd Poynor
@ 2018-07-21 13:35 ` Todd Poynor
2018-07-21 13:35 ` [PATCH 09/14] staging: gasket: core: convert various logs to debug level Todd Poynor
` (6 subsequent siblings)
14 siblings, 0 replies; 17+ messages in thread
From: Todd Poynor @ 2018-07-21 13:35 UTC (permalink / raw)
To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
Cc: Zhongze Hu, Dmitry Torokhov, Guenter Roeck, devel, linux-kernel,
Todd Poynor
From: Todd Poynor <toddpoynor@google.com>
Apex sysfs show function return -ENODEV if the attribute is not present,
rather than silently failing from the standpoint of the userspace
accessor.
Reported-by: Guenter Roeck <groeck@chromium.org>
Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
drivers/staging/gasket/apex_driver.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/gasket/apex_driver.c b/drivers/staging/gasket/apex_driver.c
index 3e76c4db5db2e..1c6f73b5a2a9e 100644
--- a/drivers/staging/gasket/apex_driver.c
+++ b/drivers/staging/gasket/apex_driver.c
@@ -710,14 +710,14 @@ static ssize_t sysfs_show(
gasket_dev = gasket_sysfs_get_device_data(device);
if (!gasket_dev) {
gasket_nodev_error("No Apex device sysfs mapping found");
- return 0;
+ return -ENODEV;
}
gasket_attr = gasket_sysfs_get_attr(device, attr);
if (!gasket_attr) {
gasket_nodev_error("No Apex device sysfs attr data found");
gasket_sysfs_put_device_data(device, gasket_dev);
- return 0;
+ return -ENODEV;
}
type = (enum sysfs_attribute_type)gasket_sysfs_get_attr(device, attr);
--
2.18.0.233.g985f88cf7e-goog
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 09/14] staging: gasket: core: convert various logs to debug level
2018-07-21 13:34 [PATCH 00/14] staging: gasket: more assorted cleanups Todd Poynor
` (7 preceding siblings ...)
2018-07-21 13:35 ` [PATCH 08/14] staging: gasket: apex return error on sysfs show of missing attribute Todd Poynor
@ 2018-07-21 13:35 ` Todd Poynor
2018-07-21 13:35 ` [PATCH 10/14] staging: gasket: interrupts: " Todd Poynor
` (5 subsequent siblings)
14 siblings, 0 replies; 17+ messages in thread
From: Todd Poynor @ 2018-07-21 13:35 UTC (permalink / raw)
To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
Cc: Zhongze Hu, Dmitry Torokhov, Guenter Roeck, devel, linux-kernel,
Todd Poynor
From: Todd Poynor <toddpoynor@google.com>
Debugging information is improperly logged at non-debug log level in a
number of places, and some logs regarding error conditions may be
generated too frequently, such that these could cause performance
problems and/or obscure other logs. Convert these to debug log level.
Signed-off-by: Zhongze Hu <frankhu@chromium.org>
Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
drivers/staging/gasket/gasket_core.c | 38 +++++++++++++++-------------
1 file changed, 20 insertions(+), 18 deletions(-)
diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c
index 8265d543623d7..1d04fd0990e45 100644
--- a/drivers/staging/gasket/gasket_core.c
+++ b/drivers/staging/gasket/gasket_core.c
@@ -205,7 +205,8 @@ static inline int check_and_invoke_callback(
{
int ret = 0;
- gasket_nodev_error("check_and_invoke_callback %p", cb_function);
+ gasket_log_debug(gasket_dev, "check_and_invoke_callback %p",
+ cb_function);
if (cb_function) {
mutex_lock(&gasket_dev->mutex);
ret = cb_function(gasket_dev);
@@ -227,7 +228,7 @@ static inline int gasket_check_and_invoke_callback_nolock(
int ret = 0;
if (cb_function) {
- gasket_log_info(
+ gasket_log_debug(
gasket_dev, "Invoking device-specific callback.");
ret = cb_function(gasket_dev);
}
@@ -1177,7 +1178,7 @@ static int gasket_release(struct inode *inode, struct file *file)
if (file->f_mode & FMODE_WRITE) {
ownership->write_open_count--;
if (ownership->write_open_count == 0) {
- gasket_log_info(gasket_dev, "Device is now free");
+ gasket_log_debug(gasket_dev, "Device is now free");
ownership->is_owned = 0;
ownership->owner = 0;
@@ -1198,7 +1199,7 @@ static int gasket_release(struct inode *inode, struct file *file)
}
}
- gasket_log_info(
+ gasket_log_debug(
gasket_dev, "New open count (owning tgid %u): %d",
ownership->owner, ownership->write_open_count);
mutex_unlock(&gasket_dev->mutex);
@@ -1225,7 +1226,7 @@ static bool gasket_mmap_has_permissions(
/* Never allow non-sysadmins to access to a dead device. */
if (gasket_dev->status != GASKET_STATUS_ALIVE) {
- gasket_log_info(gasket_dev, "Device is dead.");
+ gasket_log_debug(gasket_dev, "Device is dead.");
return false;
}
@@ -1233,7 +1234,7 @@ static bool gasket_mmap_has_permissions(
requested_permissions =
(vma->vm_flags & (VM_WRITE | VM_READ | VM_EXEC));
if (requested_permissions & ~(bar_permissions)) {
- gasket_log_info(
+ gasket_log_debug(
gasket_dev,
"Attempting to map a region with requested permissions "
"0x%x, but region has permissions 0x%x.",
@@ -1244,7 +1245,7 @@ static bool gasket_mmap_has_permissions(
/* Do not allow a non-owner to write. */
if ((vma->vm_flags & VM_WRITE) &&
!gasket_owned_by_current_tgid(&gasket_dev->dev_info)) {
- gasket_log_info(
+ gasket_log_debug(
gasket_dev,
"Attempting to mmap a region for write without owning "
"device.");
@@ -1736,15 +1737,16 @@ static int gasket_get_hw_status(struct gasket_dev *gasket_dev)
status = gasket_check_and_invoke_callback_nolock(
gasket_dev, driver_desc->device_status_cb);
if (status != GASKET_STATUS_ALIVE) {
- gasket_log_info(gasket_dev, "Hardware reported status %d.",
- status);
+ gasket_log_debug(gasket_dev, "Hardware reported status %d.",
+ status);
return status;
}
status = gasket_interrupt_system_status(gasket_dev);
if (status != GASKET_STATUS_ALIVE) {
- gasket_log_info(gasket_dev,
- "Interrupt system reported status %d.", status);
+ gasket_log_debug(gasket_dev,
+ "Interrupt system reported status %d.",
+ status);
return status;
}
@@ -1752,7 +1754,7 @@ static int gasket_get_hw_status(struct gasket_dev *gasket_dev)
status = gasket_page_table_system_status(
gasket_dev->page_table[i]);
if (status != GASKET_STATUS_ALIVE) {
- gasket_log_info(
+ gasket_log_debug(
gasket_dev, "Page table %d reported status %d.",
i, status);
return status;
@@ -1783,7 +1785,7 @@ static long gasket_ioctl(struct file *filp, uint cmd, ulong arg)
gasket_dev = (struct gasket_dev *)filp->private_data;
driver_desc = gasket_dev->internal_desc->driver_desc;
if (!driver_desc) {
- gasket_log_error(
+ gasket_log_debug(
gasket_dev,
"Unable to find device descriptor for file %s",
d_path(&filp->f_path, path, 256));
@@ -1831,7 +1833,7 @@ int gasket_reset_nolock(struct gasket_dev *gasket_dev, uint reset_type)
/* Perform a device reset of the requested type. */
ret = driver_desc->device_reset_cb(gasket_dev, reset_type);
if (ret) {
- gasket_log_error(
+ gasket_log_debug(
gasket_dev, "Device reset cb returned %d.", ret);
return ret;
}
@@ -1842,7 +1844,7 @@ int gasket_reset_nolock(struct gasket_dev *gasket_dev, uint reset_type)
ret = gasket_interrupt_reinit(gasket_dev);
if (ret) {
- gasket_log_error(
+ gasket_log_debug(
gasket_dev, "Unable to reinit interrupts: %d.", ret);
return ret;
}
@@ -1850,7 +1852,7 @@ int gasket_reset_nolock(struct gasket_dev *gasket_dev, uint reset_type)
/* Get current device health. */
gasket_dev->status = gasket_get_hw_status(gasket_dev);
if (gasket_dev->status == GASKET_STATUS_DEAD) {
- gasket_log_error(gasket_dev, "Device reported as dead.");
+ gasket_log_debug(gasket_dev, "Device reported as dead.");
return -EINVAL;
}
@@ -2002,7 +2004,7 @@ static ssize_t gasket_sysfs_data_show(
}
break;
default:
- gasket_log_error(
+ gasket_log_debug(
gasket_dev, "Unknown attribute: %s", attr->attr.name);
ret = 0;
break;
@@ -2056,7 +2058,7 @@ int gasket_wait_with_reschedule(
msleep(delay_ms);
retries++;
}
- gasket_log_error(gasket_dev, "%s timeout: reg %llx timeout (%llu ms)",
+ gasket_log_debug(gasket_dev, "%s timeout: reg %llx timeout (%llu ms)",
__func__, offset, max_retries * delay_ms);
return -ETIMEDOUT;
}
--
2.18.0.233.g985f88cf7e-goog
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 10/14] staging: gasket: interrupts: convert various logs to debug level
2018-07-21 13:34 [PATCH 00/14] staging: gasket: more assorted cleanups Todd Poynor
` (8 preceding siblings ...)
2018-07-21 13:35 ` [PATCH 09/14] staging: gasket: core: convert various logs to debug level Todd Poynor
@ 2018-07-21 13:35 ` Todd Poynor
2018-07-21 13:35 ` [PATCH 11/14] staging: gasket: ioctl common: " Todd Poynor
` (4 subsequent siblings)
14 siblings, 0 replies; 17+ messages in thread
From: Todd Poynor @ 2018-07-21 13:35 UTC (permalink / raw)
To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
Cc: Zhongze Hu, Dmitry Torokhov, Guenter Roeck, devel, linux-kernel,
Todd Poynor
From: Todd Poynor <toddpoynor@google.com>
Debugging information is improperly logged at non-debug log level in a
number of places, and some logs regarding error conditions may be
generated too frequently, such that these could cause performance
problems and/or obscure other logs. Convert these to debug log level.
Signed-off-by: Zhongze Hu <frankhu@chromium.org>
Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
drivers/staging/gasket/gasket_interrupt.c | 24 +++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/drivers/staging/gasket/gasket_interrupt.c b/drivers/staging/gasket/gasket_interrupt.c
index d096ce73cc8b3..2b8c26d065336 100644
--- a/drivers/staging/gasket/gasket_interrupt.c
+++ b/drivers/staging/gasket/gasket_interrupt.c
@@ -287,7 +287,7 @@ int gasket_interrupt_reinit(struct gasket_dev *gasket_dev)
int ret;
if (!gasket_dev->interrupt_data) {
- gasket_log_error(
+ gasket_log_debug(
gasket_dev,
"Attempted to reinit uninitialized interrupt data.");
return -EINVAL;
@@ -305,7 +305,7 @@ int gasket_interrupt_reinit(struct gasket_dev *gasket_dev)
case PCI_MSI:
case PLATFORM_WIRE:
default:
- gasket_nodev_error(
+ gasket_nodev_debug(
"Cannot handle unsupported interrupt type %d.",
gasket_dev->interrupt_data->type);
ret = -EINVAL;
@@ -351,7 +351,7 @@ static void gasket_interrupt_setup(struct gasket_dev *gasket_dev)
gasket_dev->interrupt_data;
if (!interrupt_data) {
- gasket_log_error(
+ gasket_log_debug(
gasket_dev, "Interrupt data is not initialized.");
return;
}
@@ -365,7 +365,7 @@ static void gasket_interrupt_setup(struct gasket_dev *gasket_dev)
}
if (interrupt_data->type != PCI_MSIX) {
- gasket_nodev_error(
+ gasket_nodev_debug(
"Cannot handle unsupported interrupt type %d.",
interrupt_data->type);
return;
@@ -403,7 +403,7 @@ static void gasket_interrupt_setup(struct gasket_dev *gasket_dev)
pack_shift = 3 * interrupt_data->pack_width;
break;
default:
- gasket_nodev_error(
+ gasket_nodev_debug(
"Found interrupt description with "
"unknown enum %d.",
interrupt_data->interrupts[i].packing);
@@ -445,7 +445,7 @@ void gasket_interrupt_cleanup(struct gasket_dev *gasket_dev)
case PCI_MSI:
case PLATFORM_WIRE:
default:
- gasket_nodev_error(
+ gasket_nodev_debug(
"Cannot handle unsupported interrupt type %d.",
interrupt_data->type);
};
@@ -460,18 +460,18 @@ void gasket_interrupt_cleanup(struct gasket_dev *gasket_dev)
int gasket_interrupt_system_status(struct gasket_dev *gasket_dev)
{
if (!gasket_dev->interrupt_data) {
- gasket_nodev_info("Interrupt data is null.");
+ gasket_nodev_debug("Interrupt data is null.");
return GASKET_STATUS_DEAD;
}
if (!gasket_dev->interrupt_data->msix_configured) {
- gasket_nodev_info("Interrupt not initialized.");
+ gasket_nodev_debug("Interrupt not initialized.");
return GASKET_STATUS_LAMED;
}
if (gasket_dev->interrupt_data->num_configured !=
gasket_dev->interrupt_data->num_interrupts) {
- gasket_nodev_info("Not all interrupts were configured.");
+ gasket_nodev_debug("Not all interrupts were configured.");
return GASKET_STATUS_LAMED;
}
@@ -516,14 +516,14 @@ static ssize_t interrupt_sysfs_show(
gasket_dev = gasket_sysfs_get_device_data(device);
if (!gasket_dev) {
- gasket_nodev_error(
+ gasket_nodev_debug(
"No sysfs mapping found for device 0x%p", device);
return 0;
}
gasket_attr = gasket_sysfs_get_attr(device, attr);
if (!gasket_attr) {
- gasket_nodev_error(
+ gasket_nodev_debug(
"No sysfs attr data found for device 0x%p", device);
gasket_sysfs_put_device_data(device, gasket_dev);
return 0;
@@ -545,7 +545,7 @@ static ssize_t interrupt_sysfs_show(
ret = total_written;
break;
default:
- gasket_log_error(
+ gasket_log_debug(
gasket_dev, "Unknown attribute: %s", attr->attr.name);
ret = 0;
break;
--
2.18.0.233.g985f88cf7e-goog
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 11/14] staging: gasket: ioctl common: convert various logs to debug level
2018-07-21 13:34 [PATCH 00/14] staging: gasket: more assorted cleanups Todd Poynor
` (9 preceding siblings ...)
2018-07-21 13:35 ` [PATCH 10/14] staging: gasket: interrupts: " Todd Poynor
@ 2018-07-21 13:35 ` Todd Poynor
2018-07-21 13:35 ` [PATCH 12/14] staging: gasket: page table: " Todd Poynor
` (3 subsequent siblings)
14 siblings, 0 replies; 17+ messages in thread
From: Todd Poynor @ 2018-07-21 13:35 UTC (permalink / raw)
To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
Cc: Zhongze Hu, Dmitry Torokhov, Guenter Roeck, devel, linux-kernel,
Todd Poynor
From: Todd Poynor <toddpoynor@google.com>
Debugging information is improperly logged at non-debug log level in a
number of places, and some logs regarding error conditions may be
generated too frequently, such that these could cause performance
problems and/or obscure other logs. Convert these to debug log level.
Signed-off-by: Zhongze Hu <frankhu@chromium.org>
Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
drivers/staging/gasket/gasket_core.c | 2 +-
drivers/staging/gasket/gasket_ioctl.c | 6 +++---
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c
index 1d04fd0990e45..732218773c3c6 100644
--- a/drivers/staging/gasket/gasket_core.c
+++ b/drivers/staging/gasket/gasket_core.c
@@ -1801,7 +1801,7 @@ static long gasket_ioctl(struct file *filp, uint cmd, ulong arg)
if (driver_desc->ioctl_handler_cb)
return driver_desc->ioctl_handler_cb(filp, cmd, argp);
- gasket_log_error(
+ gasket_log_debug(
gasket_dev, "Received unknown ioctl 0x%x", cmd);
return -EINVAL;
}
diff --git a/drivers/staging/gasket/gasket_ioctl.c b/drivers/staging/gasket/gasket_ioctl.c
index 8cf094b90cdb0..63e139ab7ff89 100644
--- a/drivers/staging/gasket/gasket_ioctl.c
+++ b/drivers/staging/gasket/gasket_ioctl.c
@@ -73,7 +73,7 @@ long gasket_handle_ioctl(struct file *filp, uint cmd, void __user *argp)
}
} else if (!gasket_ioctl_check_permissions(filp, cmd)) {
trace_gasket_ioctl_exit(-EPERM);
- gasket_log_error(gasket_dev, "ioctl cmd=%x noperm.", cmd);
+ gasket_log_debug(gasket_dev, "ioctl cmd=%x noperm.", cmd);
return -EPERM;
}
@@ -132,7 +132,7 @@ long gasket_handle_ioctl(struct file *filp, uint cmd, void __user *argp)
* the arg.
*/
trace_gasket_ioctl_integer_data(arg);
- gasket_log_warn(
+ gasket_log_debug(
gasket_dev,
"Unknown ioctl cmd=0x%x not caught by "
"gasket_is_supported_ioctl",
@@ -329,7 +329,7 @@ static int gasket_partition_page_table(
gasket_dev->page_table[ibuf.page_table_index]);
if (ibuf.size > max_page_table_size) {
- gasket_log_error(
+ gasket_log_debug(
gasket_dev,
"Partition request 0x%llx too large, max is 0x%x.",
ibuf.size, max_page_table_size);
--
2.18.0.233.g985f88cf7e-goog
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 12/14] staging: gasket: page table: convert various logs to debug level
2018-07-21 13:34 [PATCH 00/14] staging: gasket: more assorted cleanups Todd Poynor
` (10 preceding siblings ...)
2018-07-21 13:35 ` [PATCH 11/14] staging: gasket: ioctl common: " Todd Poynor
@ 2018-07-21 13:35 ` Todd Poynor
2018-07-21 13:35 ` [PATCH 13/14] staging: gasket: page table: remove unnecessary logs Todd Poynor
` (2 subsequent siblings)
14 siblings, 0 replies; 17+ messages in thread
From: Todd Poynor @ 2018-07-21 13:35 UTC (permalink / raw)
To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
Cc: Zhongze Hu, Dmitry Torokhov, Guenter Roeck, devel, linux-kernel,
Todd Poynor
From: Todd Poynor <toddpoynor@google.com>
Debugging information is improperly logged at non-debug log level in a
number of places, and some logs regarding error conditions may be
generated too frequently, such that these could cause performance
problems and/or obscure other logs. Convert these to debug log level.
Signed-off-by: Zhongze Hu <frankhu@chromium.org>
Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
drivers/staging/gasket/gasket_page_table.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/gasket/gasket_page_table.c b/drivers/staging/gasket/gasket_page_table.c
index 9f8116112e0ac..f0c4884cb4bc6 100644
--- a/drivers/staging/gasket/gasket_page_table.c
+++ b/drivers/staging/gasket/gasket_page_table.c
@@ -306,7 +306,7 @@ int gasket_page_table_init(
* hardware register that contains the page table size.
*/
if (total_entries == ULONG_MAX) {
- gasket_nodev_error(
+ gasket_nodev_debug(
"Error reading page table size. "
"Initializing page table with size 0.");
total_entries = 0;
@@ -323,7 +323,7 @@ int gasket_page_table_init(
*ppg_tbl = kzalloc(sizeof(**ppg_tbl), GFP_KERNEL);
if (!*ppg_tbl) {
- gasket_nodev_error("No memory for page table.");
+ gasket_nodev_debug("No memory for page table.");
return -ENOMEM;
}
@@ -332,7 +332,7 @@ int gasket_page_table_init(
if (bytes != 0) {
pg_tbl->entries = vmalloc(bytes);
if (!pg_tbl->entries) {
- gasket_nodev_error(
+ gasket_nodev_debug(
"No memory for address translation metadata.");
kfree(pg_tbl);
*ppg_tbl = NULL;
@@ -658,7 +658,7 @@ int gasket_page_table_system_status(struct gasket_page_table *page_table)
}
if (gasket_page_table_num_entries(page_table) == 0) {
- gasket_nodev_error("Page table size is 0.");
+ gasket_nodev_debug("Page table size is 0.");
return GASKET_STATUS_LAMED;
}
@@ -903,7 +903,7 @@ static int gasket_perform_mapping(
(unsigned long long)ptes[i].dma_addr);
if (ptes[i].dma_addr == -1) {
- gasket_nodev_error(
+ gasket_nodev_debug(
"%s i %d"
" -> fail to map page %llx "
"[pfn %p ohys %p]\n",
@@ -1612,7 +1612,7 @@ int gasket_set_user_virt(
*/
pg_tbl = gasket_dev->page_table[0];
if (!pg_tbl) {
- gasket_nodev_error(
+ gasket_nodev_debug(
"%s: invalid page table index", __func__);
return 0;
}
--
2.18.0.233.g985f88cf7e-goog
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 13/14] staging: gasket: page table: remove unnecessary logs
2018-07-21 13:34 [PATCH 00/14] staging: gasket: more assorted cleanups Todd Poynor
` (11 preceding siblings ...)
2018-07-21 13:35 ` [PATCH 12/14] staging: gasket: page table: " Todd Poynor
@ 2018-07-21 13:35 ` Todd Poynor
2018-07-21 13:35 ` [PATCH 14/14] staging: gasket: apex: convert various logs to debug level Todd Poynor
2018-07-21 13:45 ` [PATCH 00/14] staging: gasket: more assorted cleanups Greg Kroah-Hartman
14 siblings, 0 replies; 17+ messages in thread
From: Todd Poynor @ 2018-07-21 13:35 UTC (permalink / raw)
To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
Cc: Zhongze Hu, Dmitry Torokhov, Guenter Roeck, devel, linux-kernel,
Todd Poynor
From: Todd Poynor <toddpoynor@google.com>
Some error logs in page table handling code could only be hit in
cases of programming errors not expected in the current code base, and
aren't likely to be useful on their own. Remove these.
Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
drivers/staging/gasket/gasket_page_table.c | 23 +++++-----------------
1 file changed, 5 insertions(+), 18 deletions(-)
diff --git a/drivers/staging/gasket/gasket_page_table.c b/drivers/staging/gasket/gasket_page_table.c
index f0c4884cb4bc6..4f2ff77006586 100644
--- a/drivers/staging/gasket/gasket_page_table.c
+++ b/drivers/staging/gasket/gasket_page_table.c
@@ -605,10 +605,8 @@ EXPORT_SYMBOL(gasket_page_table_is_dev_addr_bad);
/* See gasket_page_table.h for description. */
uint gasket_page_table_max_size(struct gasket_page_table *page_table)
{
- if (!page_table) {
- gasket_nodev_error("Passed a null page table.");
+ if (!page_table)
return 0;
- }
return page_table->config.total_entries;
}
EXPORT_SYMBOL(gasket_page_table_max_size);
@@ -616,11 +614,8 @@ EXPORT_SYMBOL(gasket_page_table_max_size);
/* See gasket_page_table.h for description. */
uint gasket_page_table_num_entries(struct gasket_page_table *pg_tbl)
{
- if (!pg_tbl) {
- gasket_nodev_error("Passed a null page table.");
+ if (!pg_tbl)
return 0;
- }
-
return pg_tbl->num_simple_entries + pg_tbl->num_extended_entries;
}
EXPORT_SYMBOL(gasket_page_table_num_entries);
@@ -628,11 +623,8 @@ EXPORT_SYMBOL(gasket_page_table_num_entries);
/* See gasket_page_table.h for description. */
uint gasket_page_table_num_simple_entries(struct gasket_page_table *pg_tbl)
{
- if (!pg_tbl) {
- gasket_nodev_error("Passed a null page table.");
+ if (!pg_tbl)
return 0;
- }
-
return pg_tbl->num_simple_entries;
}
EXPORT_SYMBOL(gasket_page_table_num_simple_entries);
@@ -640,11 +632,8 @@ EXPORT_SYMBOL(gasket_page_table_num_simple_entries);
/* See gasket_page_table.h for description. */
uint gasket_page_table_num_active_pages(struct gasket_page_table *pg_tbl)
{
- if (!pg_tbl) {
- gasket_nodev_error("Passed a null page table.");
+ if (!pg_tbl)
return 0;
- }
-
return pg_tbl->num_active_pages;
}
EXPORT_SYMBOL(gasket_page_table_num_active_pages);
@@ -652,10 +641,8 @@ EXPORT_SYMBOL(gasket_page_table_num_active_pages);
/* See gasket_page_table.h */
int gasket_page_table_system_status(struct gasket_page_table *page_table)
{
- if (!page_table) {
- gasket_nodev_error("Passed a null page table.");
+ if (!page_table)
return GASKET_STATUS_LAMED;
- }
if (gasket_page_table_num_entries(page_table) == 0) {
gasket_nodev_debug("Page table size is 0.");
--
2.18.0.233.g985f88cf7e-goog
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 14/14] staging: gasket: apex: convert various logs to debug level
2018-07-21 13:34 [PATCH 00/14] staging: gasket: more assorted cleanups Todd Poynor
` (12 preceding siblings ...)
2018-07-21 13:35 ` [PATCH 13/14] staging: gasket: page table: remove unnecessary logs Todd Poynor
@ 2018-07-21 13:35 ` Todd Poynor
2018-07-21 13:45 ` [PATCH 00/14] staging: gasket: more assorted cleanups Greg Kroah-Hartman
14 siblings, 0 replies; 17+ messages in thread
From: Todd Poynor @ 2018-07-21 13:35 UTC (permalink / raw)
To: Rob Springer, John Joseph, Ben Chan, Greg Kroah-Hartman
Cc: Zhongze Hu, Dmitry Torokhov, Guenter Roeck, devel, linux-kernel,
Todd Poynor
From: Todd Poynor <toddpoynor@google.com>
Debugging information is improperly logged at non-debug log level in a
number of places, and some logs regarding error conditions may be
generated too frequently, such that these could cause performance
problems and/or obscure other logs. Convert these to debug log level.
Signed-off-by: Zhongze Hu <frankhu@chromium.org>
Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
drivers/staging/gasket/apex_driver.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/gasket/apex_driver.c b/drivers/staging/gasket/apex_driver.c
index 1c6f73b5a2a9e..6396b18b246a5 100644
--- a/drivers/staging/gasket/apex_driver.c
+++ b/drivers/staging/gasket/apex_driver.c
@@ -420,7 +420,7 @@ static int apex_device_cleanup(struct gasket_dev *gasket_dev)
gasket_dev, APEX_BAR_INDEX,
APEX_BAR2_REG_SCALAR_CORE_ERROR_STATUS);
- gasket_log_info(
+ gasket_log_debug(
gasket_dev,
"%s 0x%p hib_error 0x%llx scalar_error "
"0x%llx.",
@@ -589,7 +589,7 @@ static int apex_quit_reset(struct gasket_dev *gasket_dev, uint type)
SCU3_RG_PWR_STATE_OVR_BIT_OFFSET);
val1 = gasket_dev_read_32(
gasket_dev, APEX_BAR_INDEX, APEX_BAR2_REG_SCU_3);
- gasket_log_error(
+ gasket_log_debug(
gasket_dev, "Disallow HW clock gating 0x%x -> 0x%x",
val0, val1);
} else {
@@ -602,7 +602,7 @@ static int apex_quit_reset(struct gasket_dev *gasket_dev, uint type)
SCU3_RG_PWR_STATE_OVR_BIT_OFFSET);
val1 = gasket_dev_read_32(
gasket_dev, APEX_BAR_INDEX, APEX_BAR2_REG_SCU_3);
- gasket_log_error(
+ gasket_log_debug(
gasket_dev, "Allow HW clock gating 0x%x -> 0x%x", val0,
val1);
}
@@ -668,7 +668,7 @@ static long apex_clock_gating(struct gasket_dev *gasket_dev,
if (copy_from_user(&ibuf, argp, sizeof(ibuf)))
return -EFAULT;
- gasket_log_error(gasket_dev, "%s %llu", __func__, ibuf.enable);
+ gasket_log_debug(gasket_dev, "%s %llu", __func__, ibuf.enable);
if (ibuf.enable) {
/* Quiesce AXI, gate GCB clock. */
@@ -738,7 +738,7 @@ static ssize_t sysfs_show(
gasket_dev->page_table[0]));
break;
default:
- gasket_log_error(
+ gasket_log_debug(
gasket_dev, "Unknown attribute: %s", attr->attr.name);
ret = 0;
break;
--
2.18.0.233.g985f88cf7e-goog
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 00/14] staging: gasket: more assorted cleanups
2018-07-21 13:34 [PATCH 00/14] staging: gasket: more assorted cleanups Todd Poynor
` (13 preceding siblings ...)
2018-07-21 13:35 ` [PATCH 14/14] staging: gasket: apex: convert various logs to debug level Todd Poynor
@ 2018-07-21 13:45 ` Greg Kroah-Hartman
14 siblings, 0 replies; 17+ messages in thread
From: Greg Kroah-Hartman @ 2018-07-21 13:45 UTC (permalink / raw)
To: Todd Poynor
Cc: Rob Springer, John Joseph, Ben Chan, devel, Zhongze Hu,
linux-kernel, Guenter Roeck, Todd Poynor, Dmitry Torokhov
On Sat, Jul 21, 2018 at 06:34:53AM -0700, Todd Poynor wrote:
> From: Todd Poynor <toddpoynor@google.com>
>
> More fixups arising from the chromium review of the gasket and apex
> drivers.
That looks better, I guess you forgot to delete the old patch series
from your directory :)
greg k-h
^ permalink raw reply [flat|nested] 17+ messages in thread