[PATCH] staging: wlan-ng: collect return status without variable
err and result variables are declared to collect return status
of prism2_domibset_uint32.
Check return status in if loop and return directly.
Rearragne code such that we can avoid declaring these variables.
Signed-off-by: Hariprasad Kelam
---
drivers/staging/wlan-ng/cfg80211.c | 17 ++---
1 file changed, 6 insertions(+), 11 deletions(-)
diff --git a/drivers/staging/wlan-ng/cfg80211.c
b/drivers/staging/wlan-ng/cfg80211.c
index 8a862f7..5dad5ac 100644
--- a/drivers/staging/wlan-ng/cfg80211.c
+++ b/drivers/staging/wlan-ng/cfg80211.c
@@ -231,17 +231,12 @@ static int prism2_set_default_key(struct wiphy *wiphy,
struct net_device *dev,
{
struct wlandevice *wlandev = dev->ml_priv;
- int err = 0;
- int result = 0;
-
- result = prism2_domibset_uint32(wlandev,
- DIDMIB_DOT11SMT_PRIVACYTABLE_WEPDEFAULTKEYID,
- key_index);
-
- if (result)
- err = -EFAULT;
-
- return err;
+ if (prism2_domibset_uint32(wlandev,
+ DIDMIB_DOT11SMT_PRIVACYTABLE_WEPDEFAULTKEYID,
+ key_index))
+ return -EFAULT;
+ else
+ return 0;
}
static int prism2_get_station(struct wiphy *wiphy, struct net_device *dev,
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 3/7] lib/hexdump.c: Optionally suppress lines of repeated bytes
On Wed, 2019-05-08 at 17:58 -0700, Randy Dunlap wrote: > On 5/8/19 12:01 AM, Alastair D'Silva wrote: > > From: Alastair D'Silva > > > > Some buffers may only be partially filled with useful data, while > > the rest > > is padded (typically with 0x00 or 0xff). > > > > This patch introduces a flag to allow the supression of lines of > > repeated > > bytes, which are replaced with '** Skipped %u bytes of value 0x%x > > **' > > > > An inline wrapper function is provided for backwards compatibility > > with > > existing code, which maintains the original behaviour. > > > > Signed-off-by: Alastair D'Silva > > --- > > include/linux/printk.h | 25 +--- > > lib/hexdump.c | 91 > > -- > > 2 files changed, 99 insertions(+), 17 deletions(-) > > > > Hi, > Did you do "make htmldocs" or something similar on this? > > > diff --git a/lib/hexdump.c b/lib/hexdump.c > > index 3943507bc0e9..d61a1e4f19fa 100644 > > --- a/lib/hexdump.c > > +++ b/lib/hexdump.c > > @@ -212,8 +212,44 @@ int hex_dump_to_buffer(const void *buf, size_t > > len, int rowsize, int groupsize, > > EXPORT_SYMBOL(hex_dump_to_buffer); > > > > #ifdef CONFIG_PRINTK > > + > > +/** > > + * Check if a buffer contains only a single byte value > > + * @buf: pointer to the buffer > > + * @len: the size of the buffer in bytes > > + * @val: outputs the value if if the bytes are identical > > Does this work without a function name? > Documentation/doc-guide/kernel-doc.rst says the general format is: > > /** >* function_name() - Brief description of function. >* @arg1: Describe the first argument. >* @arg2: Describe the second argument. >*One can provide multiple line descriptions >*for arguments. >* > > > + */ > > /** > > - * print_hex_dump - print a text hex dump to syslog for a binary > > blob of data > > + * print_hex_dump_ext: dump a binary blob of data to syslog in > > hexadecimal > > Also not in the general documented format. > Thanks Randy, I'll address these. -- Alastair D'Silva mob: 0423 762 819 skype: alastair_dsilva Twitter: @EvilDeece blog: http://alastair.d-silva.org ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFC] simple_lmk: Introduce Simple Low Memory Killer for Android
On Thu, May 09, 2019 at 05:56:46PM +0200, Oleg Nesterov wrote: > Impossible ;) I bet lockdep should report the deadlock as soon as > find_victims() > calls find_lock_task_mm() when you already have a locked victim. I hope you're not a betting man ;) With the following configured: CONFIG_DEBUG_RT_MUTEXES=y CONFIG_DEBUG_SPINLOCK=y # CONFIG_DEBUG_SPINLOCK_BITE_ON_BUG is not set CONFIG_DEBUG_SPINLOCK_PANIC_ON_BUG=y CONFIG_DEBUG_MUTEXES=y CONFIG_DEBUG_WW_MUTEX_SLOWPATH=y CONFIG_DEBUG_LOCK_ALLOC=y CONFIG_PROVE_LOCKING=y CONFIG_LOCKDEP=y CONFIG_LOCK_STAT=y CONFIG_DEBUG_LOCKDEP=y CONFIG_DEBUG_ATOMIC_SLEEP=y # CONFIG_DEBUG_LOCKING_API_SELFTESTS is not set # CONFIG_LOCK_TORTURE_TEST is not set And a printk added in vtsk_is_duplicate() to print when a duplicate is detected, and my phone's memory cut in half to make simple_lmk do something, this is what I observed: taimen:/ # dmesg | grep lockdep [0.00] \x09RCU lockdep checking is enabled. taimen:/ # dmesg | grep simple_lmk [ 23.211091] simple_lmk: Killing android.carrier with adj 906 to free 37420 kiB [ 23.211160] simple_lmk: Killing oadcastreceiver with adj 906 to free 36784 kiB [ 23.248457] simple_lmk: Killing .apps.translate with adj 904 to free 45884 kiB [ 23.248720] simple_lmk: Killing ndroid.settings with adj 904 to free 42868 kiB [ 23.313417] simple_lmk: DUPLICATE VTSK! [ 23.313440] simple_lmk: Killing ndroid.keychain with adj 906 to free 33680 kiB [ 23.313513] simple_lmk: Killing com.whatsapp with adj 904 to free 51436 kiB [ 34.646695] simple_lmk: DUPLICATE VTSK! [ 34.646717] simple_lmk: Killing ndroid.apps.gcs with adj 906 to free 37956 kiB [ 34.646792] simple_lmk: Killing droid.apps.maps with adj 904 to free 63600 kiB taimen:/ # dmesg | grep lockdep [0.00] \x09RCU lockdep checking is enabled. taimen:/ # > As for > https://github.com/kerneltoast/android_kernel_google_wahoo/commit/afc8c9bf2dbde95941253c168d1adb64cfa2e3ad > Well, > > mmdrop(mm); > simple_lmk_mm_freed(mm); > > looks racy because mmdrop(mm) can free this mm_struct. Yes, > simple_lmk_mm_freed() > does not dereference this pointer, but the same memory can be re-allocated as > another ->mm for the new task which can be found by find_victims(), and _in > theory_ > this all can happen in between, so the "victims[i].mm == mm" can be false > positive. > > And this also means that simple_lmk_mm_freed() should clear victims[i].mm when > it detects "victims[i].mm == mm", otherwise we have the same theoretical race, > victims_to_kill is only cleared when the last victim goes away. Fair point. Putting simple_lmk_mm_freed(mm) right before mmdrop(mm), and sprinkling in a cmpxchg in simple_lmk_mm_freed() should fix that up. > Another nit... you can drop tasklist_lock right after the 1st "find_victims" > loop. True! > And it seems that you do not really need to walk the "victims" array twice > after that, > you can do everything in a single loop, but this is cosmetic. Won't this result in potentially holding the task lock way longer than necessary for multiple tasks that aren't going to be killed? Thanks, Sultan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH v2] kpc_i2c: Remove unused file
>-Original Message- >From: Dan Carpenter > >Add Staging: to the subject. Added to my notes so I don't forget it next time. >[PATCH v2] Staging: kpc_i2c: Remove unused file fileops.c > >On Thu, May 09, 2019 at 01:38:27PM +, Matt Sickler wrote: >> The whole file was wrapped in an #if 0. I'm guessing it was a >> leftover file from when we were first developing the driver and we just >forgot about it. >> >> V2: Forgot the signed-off-by line on the first patch. > >Put this after the --- cut off line > >> >> Signed-off-by: Matt Sickler >> --- > ^^^ > >Here. Noted. I just looked up a "v2" patch in the mailing list archive to see what that looks like. I'll try to do that next time. > >There is something else wrong with the patch and it's corrupted a bit or >something. Please read the first paragraph of Documentation/process/email- >clients.rst Ugh. I'm about to throw Outlook in the trash and just use my personal email account. I know most subsystem maintainers don't accept pull requests but Daktronics does have a github account that I could push my changes to and use git-request-pull to ask Greg to pull from. Greg, would that be an acceptable solution? If not, I can continue struggle-bussing with Outlook. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFC] simple_lmk: Introduce Simple Low Memory Killer for Android
On 05/07, Sultan Alsawaf wrote: > > On Tue, May 07, 2019 at 05:31:54PM +0200, Oleg Nesterov wrote: > > > Did you test this patch with lockdep enabled? > > > > If I read the patch correctly, lockdep should complain. vtsk_is_duplicate() > > ensures that we do not take the same ->alloc_lock twice or more, but lockdep > > can't know this. > > Yeah, lockdep is fine with this, at least on 4.4. Impossible ;) I bet lockdep should report the deadlock as soon as find_victims() calls find_lock_task_mm() when you already have a locked victim. Nevermind, I guess this code won't run with lockdep enabled... As for https://github.com/kerneltoast/android_kernel_google_wahoo/commit/afc8c9bf2dbde95941253c168d1adb64cfa2e3ad Well, mmdrop(mm); simple_lmk_mm_freed(mm); looks racy because mmdrop(mm) can free this mm_struct. Yes, simple_lmk_mm_freed() does not dereference this pointer, but the same memory can be re-allocated as another ->mm for the new task which can be found by find_victims(), and _in theory_ this all can happen in between, so the "victims[i].mm == mm" can be false positive. And this also means that simple_lmk_mm_freed() should clear victims[i].mm when it detects "victims[i].mm == mm", otherwise we have the same theoretical race, victims_to_kill is only cleared when the last victim goes away. Another nit... you can drop tasklist_lock right after the 1st "find_victims" loop. And it seems that you do not really need to walk the "victims" array twice after that, you can do everything in a single loop, but this is cosmetic. Oleg. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH v2] kpc_i2c: Remove unused file
>-Original Message- >From: Dan Carpenter > >On Thu, May 09, 2019 at 02:47:50PM +, Matt Sickler wrote: > >A few people/subsystems (DRM) put the change log in the commit message >but that's pretty weird and I don't know if they do it on purpose or >they're just not aware how to do it properly... :P > >When it's under the --- then it isn't stored in the permanent git log. Good to know the "why" behind that process! >> Ugh. I'm about to throw Outlook in the trash and just use >> my personal email account. >> I know most subsystem maintainers don't accept pull requests >> but Daktronics does have a github account that I could push >> my changes to and use git-request-pull to ask Greg to pull >> from. Greg, would that be an acceptable solution? If not, >> I can continue struggle-bussing with Outlook. > >You can't just use git send-email? Outlook is going to give you >headaches forever. I use mutt, but one subsystem only accept patches >from git send-email so I have to add a fake "X-Mailer: git-send-email" >header to my patches... I guess I have not tried. Daktronics uses Outlook and Office 365 and lots of "security" junk like Okta 2FA. I had assumed that mutt wouldn't work because of those settings. Our IT is very much windows-only, so they probably wouldn't make any exceptions for me to allow mutt to work. I'll give it a try though. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] kpc_i2c: Remove unused file
On Thu, May 09, 2019 at 02:47:50PM +, Matt Sickler wrote: > >> --- > > ^^^ > > > >Here. > > Noted. I just looked up a "v2" patch in the mailing list > archive to see what that looks like. I'll try to do that > next time. > A few people/subsystems (DRM) put the change log in the commit message but that's pretty weird and I don't know if they do it on purpose or they're just not aware how to do it properly... :P When it's under the --- then it isn't stored in the permanent git log. > > > >There is something else wrong with the patch and it's corrupted a bit or > >something. Please read the first paragraph of Documentation/process/email- > >clients.rst > > Ugh. I'm about to throw Outlook in the trash and just use > my personal email account. > I know most subsystem maintainers don't accept pull requests > but Daktronics does have a github account that I could push > my changes to and use git-request-pull to ask Greg to pull > from. Greg, would that be an acceptable solution? If not, > I can continue struggle-bussing with Outlook. You can't just use git send-email? Outlook is going to give you headaches forever. I use mutt, but one subsystem only accept patches from git send-email so I have to add a fake "X-Mailer: git-send-email" header to my patches... regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3 4/4] staging: vchiq: stop explicitly comparing with zero to catch errors
On Thu, May 09, 2019 at 04:31:36PM +0200, Nicolas Saenz Julienne wrote: > The vchiq code tends to follow a coding pattern that's not accepted as > per the Linux kernel coding style > > We have this: > if (expression != 0) > > We want this: > if (expression) > > We make an exception if the expression refers to a size, in which case > it's accepted for the sake of clarity. It's not really Linux kernel style, it's just my style... I wouldn't have complained if the original code were consistent one way or the other. But since I was encouraging you to pick a style and use it consistently, then I'm always going to advocate my style... :P Anyway, thanks! Looks good to me. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 4/4] staging: vchiq: stop explicitly comparing with zero to catch errors
The vchiq code tends to follow a coding pattern that's not accepted as
per the Linux kernel coding style
We have this:
if (expression != 0)
We want this:
if (expression)
We make an exception if the expression refers to a size, in which case
it's accepted for the sake of clarity.
Signed-off-by: Nicolas Saenz Julienne
---
.../bcm2835-camera/bcm2835-camera.c | 11 ++--
.../interface/vchiq_arm/vchiq_2835_arm.c | 2 +-
.../interface/vchiq_arm/vchiq_arm.c | 66 ---
.../interface/vchiq_arm/vchiq_connected.c | 4 +-
.../interface/vchiq_arm/vchiq_core.c | 28
.../interface/vchiq_arm/vchiq_debugfs.c | 4 +-
6 files changed, 52 insertions(+), 63 deletions(-)
diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
index 68f08dc18da9..57f79c153277 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
@@ -327,7 +327,7 @@ static void buffer_cb(struct vchiq_mmal_instance *instance,
"%s: status:%d, buf:%p, length:%lu, flags %u, pts %lld\n",
__func__, status, buf, length, mmal_flags, pts);
- if (status != 0) {
+ if (status) {
/* error in transfer */
if (buf) {
/* there was a buffer with the error so return it */
@@ -359,8 +359,7 @@ static void buffer_cb(struct vchiq_mmal_instance *instance,
}
} else {
if (dev->capture.frame_count) {
- if (dev->capture.vc_start_timestamp != -1 &&
- pts != 0) {
+ if (dev->capture.vc_start_timestamp != -1 && pts) {
ktime_t timestamp;
s64 runtime_us = pts -
dev->capture.vc_start_timestamp;
@@ -826,7 +825,7 @@ static int vidioc_enum_input(struct file *file, void *priv,
struct v4l2_input *inp)
{
/* only a single camera input */
- if (inp->index != 0)
+ if (inp->index)
return -EINVAL;
inp->type = V4L2_INPUT_TYPE_CAMERA;
@@ -842,7 +841,7 @@ static int vidioc_g_input(struct file *file, void *priv,
unsigned int *i)
static int vidioc_s_input(struct file *file, void *priv, unsigned int i)
{
- if (i != 0)
+ if (i)
return -EINVAL;
return 0;
@@ -1281,7 +1280,7 @@ static int vidioc_s_fmt_vid_cap(struct file *file, void
*priv,
}
ret = mmal_setup_components(dev, f);
- if (ret != 0) {
+ if (ret) {
v4l2_err(&dev->v4l2_dev,
"%s: failed to setup mmal components: %d\n",
__func__, ret);
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
index 49d3b39b1059..cb588c0b9364 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
@@ -514,7 +514,7 @@ create_pagelist(char __user *buf, size_t count, unsigned
short type)
(g_cache_line_size - 1 {
char *fragments;
- if (down_interruptible(&g_free_fragments_sema) != 0) {
+ if (down_interruptible(&g_free_fragments_sema)) {
cleanup_pagelistinfo(pagelistinfo);
return NULL;
}
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index 62d8f599e765..9264a07cf160 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -238,7 +238,7 @@ VCHIQ_STATUS_T vchiq_shutdown(VCHIQ_INSTANCE_T instance)
vchiq_log_trace(vchiq_core_log_level,
"%s(%p) called", __func__, instance);
- if (mutex_lock_killable(&state->mutex) != 0)
+ if (mutex_lock_killable(&state->mutex))
return VCHIQ_RETRY;
/* Remove all services */
@@ -280,7 +280,7 @@ VCHIQ_STATUS_T vchiq_connect(VCHIQ_INSTANCE_T instance)
vchiq_log_trace(vchiq_core_log_level,
"%s(%p) called", __func__, instance);
- if (mutex_lock_killable(&state->mutex) != 0) {
+ if (mutex_lock_killable(&state->mutex)) {
vchiq_log_trace(vchiq_core_log_level,
"%s: call to mutex_lock failed", __func__);
status = VCHIQ_RETRY;
@@ -645,8 +645,7 @@ service_callback(VCHIQ_REASON_T reason, struct vchiq_header
*header,
DEBUG_TRACE(SERVICE_CALLBACK_LINE);
if (wait_for_completion_interruptible(
-
[PATCH v3 2/4] staging: vchiq: revert "switch to wait_for_completion_killable"
The killable version of wait_for_completion() is meant to be used on
situations where it should not fail at all costs, but still have the
convenience of being able to kill it if really necessary. VCHIQ doesn't
fit this criteria, as it's mainly used as an interface to V4L2 and ALSA
devices.
Fixes: a772f116702e ("staging: vchiq: switch to wait_for_completion_killable")
Signed-off-by: Nicolas Saenz Julienne
---
.../interface/vchiq_arm/vchiq_arm.c | 21 ++-
.../interface/vchiq_arm/vchiq_core.c | 21 ++-
.../interface/vchiq_arm/vchiq_util.c | 6 +++---
3 files changed, 25 insertions(+), 23 deletions(-)
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index ab7d6a0ce94c..62d8f599e765 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -532,7 +532,8 @@ add_completion(VCHIQ_INSTANCE_T instance, VCHIQ_REASON_T
reason,
vchiq_log_trace(vchiq_arm_log_level,
"%s - completion queue full", __func__);
DEBUG_COUNT(COMPLETION_QUEUE_FULL_COUNT);
- if (wait_for_completion_killable(&instance->remove_event)) {
+ if (wait_for_completion_interruptible(
+ &instance->remove_event)) {
vchiq_log_info(vchiq_arm_log_level,
"service_callback interrupted");
return VCHIQ_RETRY;
@@ -643,7 +644,7 @@ service_callback(VCHIQ_REASON_T reason, struct vchiq_header
*header,
}
DEBUG_TRACE(SERVICE_CALLBACK_LINE);
- if (wait_for_completion_killable(
+ if (wait_for_completion_interruptible(
&user_service->remove_event)
!= 0) {
vchiq_log_info(vchiq_arm_log_level,
@@ -978,7 +979,7 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned
long arg)
has been closed until the client library calls the
CLOSE_DELIVERED ioctl, signalling close_event. */
if (user_service->close_pending &&
- wait_for_completion_killable(
+ wait_for_completion_interruptible(
&user_service->close_event))
status = VCHIQ_RETRY;
break;
@@ -1154,7 +1155,7 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned
long arg)
DEBUG_TRACE(AWAIT_COMPLETION_LINE);
mutex_unlock(&instance->completion_mutex);
- rc = wait_for_completion_killable(
+ rc = wait_for_completion_interruptible(
&instance->insert_event);
mutex_lock(&instance->completion_mutex);
if (rc != 0) {
@@ -1324,7 +1325,7 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned
long arg)
do {
spin_unlock(&msg_queue_spinlock);
DEBUG_TRACE(DEQUEUE_MESSAGE_LINE);
- if (wait_for_completion_killable(
+ if (wait_for_completion_interruptible(
&user_service->insert_event)) {
vchiq_log_info(vchiq_arm_log_level,
"DEQUEUE_MESSAGE interrupted");
@@ -2328,7 +2329,7 @@ vchiq_keepalive_thread_func(void *v)
while (1) {
long rc = 0, uc = 0;
- if (wait_for_completion_killable(&arm_state->ka_evt)
+ if (wait_for_completion_interruptible(&arm_state->ka_evt)
!= 0) {
vchiq_log_error(vchiq_susp_log_level,
"%s interrupted", __func__);
@@ -2579,7 +2580,7 @@ block_resume(struct vchiq_arm_state *arm_state)
write_unlock_bh(&arm_state->susp_res_lock);
vchiq_log_info(vchiq_susp_log_level, "%s wait for previously "
"blocked clients", __func__);
- if (wait_for_completion_killable_timeout(
+ if (wait_for_completion_interruptible_timeout(
&arm_state->blocked_blocker, timeout_val)
<= 0) {
vchiq_log_error(vchiq_susp_log_level, "%s wait for "
@@ -2605,7 +2606,7 @@ block_resume(struct vchiq_arm_state *arm_state)
write_unlock_bh(&arm_state->susp_res_lock);
vchiq_log_info(vchiq_susp_log_level, "%s wait for resume",
_
[PATCH v3 3/4] staging: vchiq: make wait events interruptible
The killable version of wait_event() is meant to be used on situations
where it should not fail at all costs, but still have the convenience of
being able to kill it if really necessary. Wait events in VCHIQ doesn't
fit this criteria, as it's mainly used as an interface to V4L2 and ALSA
devices.
Fixes: 852b2876a8a8 ("staging: vchiq: rework remove_event handling")
Signed-off-by: Nicolas Saenz Julienne
---
.../vc04_services/interface/vchiq_arm/vchiq_core.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
index c65cf1e6f910..44bfa890e0e5 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -395,13 +395,21 @@ remote_event_create(wait_queue_head_t *wq, struct
remote_event *event)
init_waitqueue_head(wq);
}
+/*
+ * All the event waiting routines in VCHIQ used a custom semaphore
+ * implementation that filtered most signals. This achieved a behaviour similar
+ * to the "killable" family of functions. While cleaning up this code all the
+ * routines where switched to the "interruptible" family of functions, as the
+ * former was deemed unjustified and the use "killable" set all VCHIQ's
+ * threads in D state.
+ */
static inline int
remote_event_wait(wait_queue_head_t *wq, struct remote_event *event)
{
if (!event->fired) {
event->armed = 1;
dsb(sy);
- if (wait_event_killable(*wq, event->fired)) {
+ if (wait_event_interruptible(*wq, event->fired)) {
event->armed = 0;
return 0;
}
--
2.21.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 0/4] staging: vchiq: use interruptible waits
Hi, this series tries to address an issue that came up in Raspbian's kernel tree [1] and upstream distros [2][3]. We adopted some changes that moved wait calls from a custom implementation to the more standard killable family of functions. Users complained that all the VCHIQ threads showed up in D state (which is the expected behaviour). The custom implementation we deleted tried to mimic the killable family of functions, yet accepted more signals than the later; SIGKILL | SIGINT | SIGQUIT | SIGTRAP | SIGSTOP | SIGCONT for the custom implementation as opposed to plain old SIGKILL. Raspbian maintainers decided roll back some of those changes and leave the wait functions as interruptible. Hence creating some divergence between both trees. One could argue that not liking having the threads stuck in D state is not really a software issue. It's more a cosmetic thing that can scare people when they look at "uptime". On the other hand, if we are ever to unstage this driver, we'd really need a proper justification for using the killable family of functions. Which I think it's not really clear at the moment. As Raspbian's kernel has been working for a while with interruptible waits I propose we follow through. If needed we can always go back to killable. But at least we'll have a proper understanding on the actual needs. In the end the driver is in staging, and the potential for errors small. The first 3 commits fix the issue, and should probably get in as soon as possible, the last commit is just cosmetic and can wait until 5.3. Regards, Nicolas [1] https://github.com/raspberrypi/linux/issues/2881 [2] https://archlinuxarm.org/forum/viewtopic.php?f=65&t=13485 [3] https://lists.fedoraproject.org/archives/list/a...@lists.fedoraproject.org/message/GBXGJ7DOV5CQQXFPOZCXTRD6W4BEPT4Q/ -- Changes since v2: - Cleaned up revert commit message - Rebase & merge conflict resolutions - Add code cleanup suggested by Dan Carpenter Changes since v1: - Proplery format revert commits - Add code comment to remind of this issue - Add Fixes tags Nicolas Saenz Julienne (4): staging: vchiq_2835_arm: revert "quit using custom down_interruptible()" staging: vchiq: revert "switch to wait_for_completion_killable" staging: vchiq: make wait events interruptible staging: vchiq: stop explicitly comparing with zero to catch errors .../bcm2835-camera/bcm2835-camera.c | 11 ++- .../interface/vchiq_arm/vchiq_2835_arm.c | 2 +- .../interface/vchiq_arm/vchiq_arm.c | 85 +-- .../interface/vchiq_arm/vchiq_connected.c | 4 +- .../interface/vchiq_arm/vchiq_core.c | 53 +++- .../interface/vchiq_arm/vchiq_debugfs.c | 4 +- .../interface/vchiq_arm/vchiq_util.c | 6 +- 7 files changed, 82 insertions(+), 83 deletions(-) -- 2.21.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 1/4] staging: vchiq_2835_arm: revert "quit using custom down_interruptible()"
The killable version of down() is meant to be used on situations where
it should not fail at all costs, but still have the convenience of being
able to kill it if really necessary. VCHIQ doesn't fit this criteria, as
it's mainly used as an interface to V4L2 and ALSA devices.
Fixes: ff5979ad8636 ("staging: vchiq_2835_arm: quit using custom
down_interruptible()")
Signed-off-by: Nicolas Saenz Julienne
---
.../staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
index a9a22917ecdb..49d3b39b1059 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
@@ -514,7 +514,7 @@ create_pagelist(char __user *buf, size_t count, unsigned
short type)
(g_cache_line_size - 1 {
char *fragments;
- if (down_killable(&g_free_fragments_sema)) {
+ if (down_interruptible(&g_free_fragments_sema) != 0) {
cleanup_pagelistinfo(pagelistinfo);
return NULL;
}
--
2.21.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] kpc_i2c: Remove unused file
Add Staging: to the subject. [PATCH v2] Staging: kpc_i2c: Remove unused file fileops.c On Thu, May 09, 2019 at 01:38:27PM +, Matt Sickler wrote: > The whole file was wrapped in an #if 0. I'm guessing it was a leftover file > from when we were first developing the driver and we just forgot about it. > > V2: Forgot the signed-off-by line on the first patch. Put this after the --- cut off line > > Signed-off-by: Matt Sickler > --- ^^^ Here. There is something else wrong with the patch and it's corrupted a bit or something. Please read the first paragraph of Documentation/process/email-clients.rst regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2] kpc_i2c: Remove unused file
The whole file was wrapped in an #if 0. I'm guessing it was a leftover file
from when we were first developing the driver and we just forgot about it.
V2: Forgot the signed-off-by line on the first patch.
Signed-off-by: Matt Sickler
---
drivers/staging/kpc2000/kpc_i2c/Makefile | 2 +-
drivers/staging/kpc2000/kpc_i2c/fileops.c | 181 --
2 files changed, 1 insertion(+), 182 deletions(-) delete mode 100644
drivers/staging/kpc2000/kpc_i2c/fileops.c
diff --git a/drivers/staging/kpc2000/kpc_i2c/Makefile
b/drivers/staging/kpc2000/kpc_i2c/Makefile
index 73ec07ac7d39..63a6ce4b8e03 100644
--- a/drivers/staging/kpc2000/kpc_i2c/Makefile
+++ b/drivers/staging/kpc2000/kpc_i2c/Makefile
@@ -1,4 +1,4 @@
# SPDX-License-Identifier: GPL-2.0
obj-m := kpc2000_i2c.o
-kpc2000_i2c-objs := i2c_driver.o fileops.o
+kpc2000_i2c-objs := i2c_driver.o
diff --git a/drivers/staging/kpc2000/kpc_i2c/fileops.c
b/drivers/staging/kpc2000/kpc_i2c/fileops.c
deleted file mode 100644
index e749c0994491..
--- a/drivers/staging/kpc2000/kpc_i2c/fileops.c
+++ /dev/null
@@ -1,181 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0+
-#if 0
-#include
-#include
-#include /* printk() */
-#include /* kmalloc() */
-#include /* everything... */
-#include/* error codes */
-#include/* size_t */
-#include
-#include/* copy_*_user */
-
-#include "i2c_driver.h"
-
-int i2c_cdev_open(struct inode *inode, struct file *filp) -{
- struct i2c_device *lddev;
-
- if(NULL == inode) {
-//printk(KERN_WARNING " i2c_cdev_open: inode is a NULL pointer\n");
-DBG_PRINT(KERN_WARNING, "i2c_cdev_open: inode is a NULL pointer\n");
-return -EINVAL;
- }
- if(NULL == filp) {
-//printk(KERN_WARNING " i2c_cdev_open: filp is a NULL pointer\n");
-DBG_PRINT(KERN_WARNING, "i2c_cdev_open: filp is a NULL pointer\n");
-return -EINVAL;
- }
-
- lddev = container_of(inode->i_cdev, struct i2c_device, cdev);
- //printk(KERN_DEBUG " i2c_cdev_open(filp = [%p], lddev = [%p])\n",
filp, lddev);
- DBG_PRINT(KERN_DEBUG, "i2c_cdev_open(filp = [%p], lddev = [%p])\n", filp,
lddev);
-
- filp->private_data = lddev; /* so other methods can access it */
-
- return 0;/* success */
-}
-
-int i2c_cdev_close(struct inode *inode, struct file *filp) -{
- struct i2c_device *lddev;
-
- if(NULL == inode) {
-//printk(KERN_WARNING " i2c_cdev_close: inode is a NULL
pointer\n");
-DBG_PRINT(KERN_WARNING, "i2c_cdev_close: inode is a NULL pointer\n");
-return -EINVAL;
- }
- if(NULL == filp) {
-//printk(KERN_WARNING " i2c_cdev_close: filp is a NULL pointer\n");
-DBG_PRINT(KERN_WARNING, "i2c_cdev_close: filp is a NULL pointer\n");
-return -EINVAL;
- }
-
- lddev = filp->private_data;
- //printk(KERN_DEBUG " i2c_cdev_close(filp = [%p], lddev = [%p])\n",
filp, lddev);
- DBG_PRINT(KERN_DEBUG, "i2c_cdev_close(filp = [%p], lddev = [%p])\n", filp,
lddev);
-
- return 0;
-}
-
-ssize_t i2c_cdev_read(struct file *filp, char __user *buf, size_t count,
loff_t *f_pos) -{
- size_t copy;
- ssize_t ret = 0;
- int err = 0;
- u64 read_val;
- char tmp_buf[48] = { 0 };
- struct i2c_device *lddev = filp->private_data;
-
- if(NULL == filp) {
-//printk(KERN_WARNING " i2c_cdev_read: filp is a NULL pointer\n");
-DBG_PRINT(KERN_WARNING, "i2c_cdev_read: filp is a NULL pointer\n");
-return -EINVAL;
- }
- if(NULL == buf) {
-//printk(KERN_WARNING " i2c_cdev_read: buf is a NULL pointer\n");
-DBG_PRINT(KERN_WARNING, "i2c_cdev_read: buf is a NULL pointer\n");
-return -EINVAL;
- }
- if(NULL == f_pos) {
-//printk(KERN_WARNING " i2c_cdev_read: f_pos is a NULL pointer\n");
-DBG_PRINT(KERN_WARNING, "i2c_cdev_read: f_pos is a NULL pointer\n");
-return -EINVAL;
- }
-
- if(count < sizeof(tmp_buf)) {
-//printk(KERN_INFO " i2c_cdev_read: buffer is too small (count =
%d, should be at least %d bytes)\n", (int)count, (int)sizeof(tmp_buf));
-DBG_PRINT(KERN_INFO, "i2c_cdev_read: buffer is too small (count = %d,
should be at least %d bytes)\n", (int)count, (int)sizeof(tmp_buf));
-return -EINVAL;
- }
- if(((*f_pos * 8) + lddev->pldev->resource[0].start) >
lddev->pldev->resource[0].end) {
-//printk(KERN_INFO " i2c_cdev_read: bad read addr %016llx\n",
(*f_pos * 8) + lddev->pldev->resource[0].start);
-DBG_PRINT(KERN_INFO, "i2c_cdev_read: bad read addr %016llx\n", (*f_pos *
8) + lddev->pldev->resource[0].start);
-//printk(KERN_INFO " i2c_cdev_read: addr end %016llx\n",
lddev->pldev->resource[0].end);
-DBG_PRINT(KERN_INFO, "i2c_cdev_read: addr end %016llx\n",
lddev->pldev->resource[0].end);
-//printk(KERN_INFO " i2c_cdev_read: EOF reached\n");
-DBG_PRINT(KERN_INFO, "i2c_cdev_read: EOF reached\n");
-return 0;
- }
-
- down_read(&lddev->rw_sem);
-
- read_val = *(lddev->regs + *f_pos);
- copy = clamp_t(size_t, count, 1, sizeof(tmp_buf));
- copy = scnprintf(tmp_buf, copy, "reg: 0x%x val: 0x%llx\n", (unsigned
