Re: [PATCH v2] staging: pi433: add comment to rx_lock mutex definition
From: Greg KH <gregkh@linuxfoundation.org>
Date: 2021-12-30 10:56:39
Also in:
lkml
On Thu, Dec 23, 2021 at 10:56:15AM +1300, Paulo Miguel Almeida wrote:
quoted hunk ↗ jump to hunk
Checkpatch reports: CHECK: struct mutex definition without comment. Fix this by documenting what rx_mutex struct is used for in pi433 driver. Signed-off-by: Paulo Miguel Almeida <redacted> --- v2: ellaborate on reasons why the mutex lock is used in the driver (Req: Greg k-h) v1: https://lore.kernel.org/lkml/20211222093626.GA13332@localhost.localdomain/ (local) --- drivers/staging/pi433/pi433_if.c | 11 +++++++++++ 1 file changed, 11 insertions(+)diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c index 29bd37669059..1cd3d5f2df2a 100644 --- a/drivers/staging/pi433/pi433_if.c +++ b/drivers/staging/pi433/pi433_if.c@@ -92,6 +92,17 @@ struct pi433_device { u32 rx_bytes_to_drop; u32 rx_bytes_dropped; unsigned int rx_position; + /* + * rx_lock is used to avoid race-conditions that can be triggered from userspace. + * + * For instance, if a program in userspace is reading the char device + * allocated in this module then another program won't be able to change RX + * configuration of the RF69 hardware module via ioctl and vice versa. + * + * utilization summary: + * - pi433_read: blocks are read until rx read something (up to the buffer size) + * - pi433_ioctl: during pending read request, change of config not allowed + */
This is nice, but way too specific, and will quickly get out-of-date. How about something simple like: /* Protects all rx_* variable accesses */ thanks, greg k-h