The rule of thumb is to include the headers we are the direct user of.
In particular, if we need an atomic API, we include <linux/atomic.h>.
On the other hand we should not use headers for no reason. In particular,
if we are not doing any IRQ job, why is the <linux/irq.h> included?
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Since The length of the message buffer is BUF_LEN. When writing the
BUF_LEN length of the string it will overwrite the last character
(usually it is '\0' from the initialization). And, because the read
operation uses the character in the message buffer ('\0') to stop the
read loop. It will cause the read operation will read out of the
message buffer when the length parameter of read() is not lower than
or equal to BUF_LEN. So add one more byte space to avoid this problem.
After forking, Each file descriptor in the child refers to the same
open file description as the parent. So when calling open() before
fork(), the child can access the device file without checking by
exclusive access in device_open(). It may cause race conditions
in device_ioctl().
Because of that, it is unnecessary to check the multiple access
in device_open(). It just needs check in device_ioctl(), since
read(), write(), seek() system call are atomic [1][2].
Related discussion:
- https://github.com/sysprog21/lkmpg/issues/148
[1] https://lore.kernel.org/lkml/53022DB1.4070805@gmail.com/
[2] https://www.kernel.org/doc/html/latest/filesystems/files.htmlClose#148
Since Linux v3.14, the read, write and seek operations of "struct file" are
guaranteed for thread safety [1][2]. This patch added an explanation.
Here are the potential problems:
chardev.c:
- Move the "msg_ptr" pointer into the read function to remove unnecessary usage.
- List the clear states of "already_open" by using mnemonic enumeration.
chardev2.c:
- The "buffer" in the write function is user space data. It cannot use in the
kernel space.
- Reduce the redundant type transformation.
- List the states of "already_open". Same as chardev.c.
[1] https://lore.kernel.org/lkml/20140303210359.26624.qmail@science.horizon.com/T/#u
[2] 9c225f2655
In file {chardev,chardev2,sleep}.c, the variable to determine
the exclusive access was of integer type, which led to race
condition.
This patch rewrote the above with atomic CAS respectively
to eliminate the race.
Close#93
chardev2.c demonstrates the ioctl operation with static major
number MAJOR_NUM, but there also exists "Major," the dynamic
one, which results in registration and deregistration on different
device. Once the module remove, it cannot insert again:
$ sudo insmod chardev2.ko
$ sudo rmmod chardev2
$ cat /proc/devices
Character devices:
...
100 char_dev
$ sudo insmod chardev2.ko
insmod: ERROR: could not insert module chardev2.ko: Device or resource busy
This patch removed the use of dynamic major number.
