Re: [PATCH] kernel:irq:manage: request threaded irq with a specified priority
Daniel & tglx, Points taken and thanks a lot for such detailed explanations. BR Song On 2021/4/16 下午5:09, Thomas Gleixner wrote: On Fri, Apr 16 2021 at 12:57, chensong wrote: On 2021/4/13 下午4:39, Thomas Gleixner wrote: It breaks because the system designer failed to assign proper priorities to the irq threads int_a, int_b and to the user space process task_a. yes, it's designers' responsibility to assign proper priorities, but kernel is also obliged to provide interfaces for those designers. The interface exists. sched_setscheduler(2) chrt can help designers in this case, however, the truth is lot of customers are not familiar with it. The truth is that real-time systems need to be carefully designed and parametrized. And that's only possible when _all_ of the system components and their constraints are known. Trying to solve it at the device driver level of a single device is impossible and a guarantee for fail. If the customer does not know how to do it, then I really have to ask why he's trying to use a real-time system at all. There is no real-time system which magically tunes itself by pulling the overall system constraints out of thin air. what's more, chrt can also apply to userspace rt task, but userspace also has sched_setscheduler to assgin proper priority inside code like cyclictest, why can't driver writers have another choice? There is a very simple reason: The driver writer cannot know about the requirements of the complete system which is composed of kernel, drivers and user space applications, unless the driver writer is fully aware of the overall system design and constraints. How is that supposed to work on a general purpose kernel which is utilized for a gazillion of different use cases which all have different expectations? It simply cannot work because default A will only work for usecase A and be completely wrong for all others. Further, what if irq handlear thread has to run on the expected priority at the very beginning? This patch helps. There is no such thing as the expected priority of an interrupt thread which can be applied upfront. There are ~5400 instances of request*irq() in the kernel source and there is no way to make priority decisions for them which work for every RT system out there. The kernel sets a default and the system designer, admin, user has to take care of tuning it to match the expectations and constraints of his particular application scenario. The kernel provides an userspace interface to do that. That interface might be a bit awkward to use, but there are tools out there which help with that, and if at all we can think about providing a better and easier to use interface for this. Trying to solve that at the kernel level is putting the cart before the horse. Thanks, tglx
Re: [PATCH] kernel:irq:manage: request threaded irq with a specified priority
On 2021/4/13 下午4:39, Thomas Gleixner wrote: On Tue, Apr 13 2021 at 14:19, Song Chen wrote: In general, irq handler thread will be assigned a default priority which is MAX_RT_PRIO/2, as a result, no one can preempt others. Here is the case I found in a real project, an interrupt int_a is coming, wakes up its handler handler_a and handler_a wakes up a userspace RT process task_a. However, if another irq handler handler_b which has nothing to do with any RT tasks is running when int_a is coming, handler_a can't preempt handler_b, as a result, task_a can't be waken up immediately as expected until handler_b gives up cpu voluntarily. In this case, determinism breaks. It breaks because the system designer failed to assign proper priorities to the irq threads int_a, int_b and to the user space process task_a. yes, it's designers' responsibility to assign proper priorities, but kernel is also obliged to provide interfaces for those designers. chrt can help designers in this case, however, the truth is lot of customers are not familiar with it. what's more, chrt can also apply to userspace rt task, but userspace also has sched_setscheduler to assgin proper priority inside code like cyclictest, why can't driver writers have another choice? Further, what if irq handlear thread has to run on the expected priority at the very beginning? This patch helps. BR Song That's not solvable at the kernel level. Thanks, tglx
[PATCH] staging: comedi: remove warnings of comedi_lrange
Checkpatch.pl reports "warning: struct comedi_lrange should
normally be const" in some places, which are supposed to
be removed.
Signed-off-by: chensong
---
drivers/staging/comedi/drivers/das16.c | 4 ++--
drivers/staging/comedi/drivers/jr3_pci.c | 4 ++--
drivers/staging/comedi/drivers/ni_670x.c | 2 +-
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/comedi/drivers/das16.c
b/drivers/staging/comedi/drivers/das16.c
index 4ac2622..40bfd84 100644
--- a/drivers/staging/comedi/drivers/das16.c
+++ b/drivers/staging/comedi/drivers/das16.c
@@ -958,7 +958,7 @@ static const struct comedi_lrange *das16_ai_range(struct
comedi_device *dev,
/* get any user-defined input range */
if (pg_type == das16_pg_none && (min || max)) {
- struct comedi_lrange *lrange;
+ const struct comedi_lrange *lrange;
struct comedi_krange *krange;
/* allocate single-range range table */
@@ -992,7 +992,7 @@ static const struct comedi_lrange *das16_ao_range(struct
comedi_device *dev,
/* get any user-defined output range */
if (min || max) {
- struct comedi_lrange *lrange;
+ const struct comedi_lrange *lrange;
struct comedi_krange *krange;
/* allocate single-range range table */
diff --git a/drivers/staging/comedi/drivers/jr3_pci.c
b/drivers/staging/comedi/drivers/jr3_pci.c
index 7a02c4f..c35db0b 100644
--- a/drivers/staging/comedi/drivers/jr3_pci.c
+++ b/drivers/staging/comedi/drivers/jr3_pci.c
@@ -91,8 +91,8 @@ struct jr3_pci_dev_private {
};
union jr3_pci_single_range {
- struct comedi_lrange l;
- char _reserved[offsetof(struct comedi_lrange, range[1])];
+ const struct comedi_lrange l;
+ char _reserved[offsetof(const struct comedi_lrange, range[1])];
};
enum jr3_pci_poll_state {
diff --git a/drivers/staging/comedi/drivers/ni_670x.c
b/drivers/staging/comedi/drivers/ni_670x.c
index c197e47..66485ec 100644
--- a/drivers/staging/comedi/drivers/ni_670x.c
+++ b/drivers/staging/comedi/drivers/ni_670x.c
@@ -200,7 +200,7 @@ static int ni_670x_auto_attach(struct comedi_device *dev,
const struct comedi_lrange **range_table_list;
range_table_list = kmalloc_array(32,
-sizeof(struct comedi_lrange *),
+sizeof(const struct
comedi_lrange *),
GFP_KERNEL);
if (!range_table_list)
return -ENOMEM;
--
2.7.4
[PATCH] staging: comedi: clean up debugging code in #if 0 or 1
There are a log of "#if 0" or "#if 1" in comedi driver which
cause warning when running checkpatch.pl, they are supposed to
be cleaned up before release.
Signed-off-by: chensong
---
drivers/staging/comedi/drivers/cb_pcidas64.c | 95 --
drivers/staging/comedi/drivers/dt2801.c| 29
drivers/staging/comedi/drivers/ni_atmio16d.c | 9 ---
drivers/staging/comedi/drivers/ni_mio_common.c | 37 +-
drivers/staging/comedi/drivers/ni_mio_cs.c | 10 ---
drivers/staging/comedi/drivers/ni_pcidio.c | 5 --
drivers/staging/comedi/drivers/ni_pcimio.c | 48 -
drivers/staging/comedi/drivers/s526.c | 49 -
drivers/staging/comedi/drivers/s626.c | 45
9 files changed, 1 insertion(+), 326 deletions(-)
diff --git a/drivers/staging/comedi/drivers/cb_pcidas64.c
b/drivers/staging/comedi/drivers/cb_pcidas64.c
index fa987bb..2d74ec9 100644
--- a/drivers/staging/comedi/drivers/cb_pcidas64.c
+++ b/drivers/staging/comedi/drivers/cb_pcidas64.c
@@ -998,101 +998,6 @@ static const struct pcidas64_board pcidas64_boards[] = {
.ai_fifo= &ai_fifo_4020,
.has_8255 = 1,
},
-#if 0
- /* The device id for these boards is unknown */
-
- [BOARD_PCIDAS6402_16_JR] = {
- .name = "pci-das6402/16/jr",
- .ai_se_chans= 64,
- .ai_bits= 16,
- .ai_speed = 5000,
- .ao_nchan = 0,
- .ao_scan_speed = 1,
- .layout = LAYOUT_64XX,
- .ai_range_table = &ai_ranges_64xx,
- .ai_range_code = ai_range_code_64xx,
- .ai_fifo= ai_fifo_64xx,
- .has_8255 = 1,
- },
- [BOARD_PCIDAS64_M1_16_JR] = {
- .name = "pci-das64/m1/16/jr",
- .ai_se_chans= 64,
- .ai_bits= 16,
- .ai_speed = 1000,
- .ao_nchan = 0,
- .ao_scan_speed = 1,
- .layout = LAYOUT_64XX,
- .ai_range_table = &ai_ranges_64_mx,
- .ai_range_code = ai_range_code_64_mx,
- .ai_fifo= ai_fifo_64xx,
- .has_8255 = 1,
- },
- [BOARD_PCIDAS64_M2_16_JR] = {
- .name = "pci-das64/m2/16/jr",
- .ai_se_chans= 64,
- .ai_bits= 16,
- .ai_speed = 500,
- .ao_nchan = 0,
- .ao_scan_speed = 1,
- .layout = LAYOUT_64XX,
- .ai_range_table = &ai_ranges_64_mx,
- .ai_range_code = ai_range_code_64_mx,
- .ai_fifo= ai_fifo_64xx,
- .has_8255 = 1,
- },
- [BOARD_PCIDAS64_M3_16_JR] = {
- .name = "pci-das64/m3/16/jr",
- .ai_se_chans= 64,
- .ai_bits= 16,
- .ai_speed = 333,
- .ao_nchan = 0,
- .ao_scan_speed = 1,
- .layout = LAYOUT_64XX,
- .ai_range_table = &ai_ranges_64_mx,
- .ai_range_code = ai_range_code_64_mx,
- .ai_fifo= ai_fifo_64xx,
- .has_8255 = 1,
- },
- [BOARD_PCIDAS64_M1_14] = {
- .name = "pci-das64/m1/14",
- .ai_se_chans= 64,
- .ai_bits= 14,
- .ai_speed = 1000,
- .ao_nchan = 2,
- .ao_scan_speed = 1,
- .layout = LAYOUT_64XX,
- .ai_range_table = &ai_ranges_64_mx,
- .ai_range_code = ai_range_code_64_mx,
- .ai_fifo= ai_fifo_64xx,
- .has_8255 = 1,
- },
- [BOARD_PCIDAS64_M2_14] = {
- .name = "pci-das64/m2/14",
- .ai_se_chans= 64,
- .ai_bits= 14,
- .ai_speed = 500,
- .ao_nchan = 2,
- .ao_scan_speed = 1,
- .layout = LAYOUT_64XX,
- .ai_range_table = &ai_ranges_64_mx,
- .ai_range_code = ai_range_code_64_mx,
- .ai_fifo= ai_fifo_64xx,
- .has_8255 = 1,
- },
- [BOARD_PCIDAS64_M3_14] = {
- .name = "pci-das64/m3/14",
- .ai_se_chans= 64,
- .ai_bits= 14,
- .ai_speed = 333,
- .ao_nchan = 2,
- .ao_scan_speed = 1,
- .layout = LAYOUT_64XX,
- .ai_range_table = &ai_ranges_64_mx,
-
[PATCH] staging: comedi: correct spelling mistakes of I/O port base address
"base" was double input in comment line "I/O port base address", remove one of them. Signed-off-by: chensong --- drivers/staging/comedi/drivers/dt2815.c | 2 +- drivers/staging/comedi/drivers/dt2817.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/comedi/drivers/dt2815.c b/drivers/staging/comedi/drivers/dt2815.c index 5906f32..2be2406 100644 --- a/drivers/staging/comedi/drivers/dt2815.c +++ b/drivers/staging/comedi/drivers/dt2815.c @@ -17,7 +17,7 @@ * contrary, please update. * * Configuration options: - * [0] - I/O port base base address + * [0] - I/O port base address * [1] - IRQ (unused) * [2] - Voltage unipolar/bipolar configuration * 0 == unipolar 5V (0V -- +5V) diff --git a/drivers/staging/comedi/drivers/dt2817.c b/drivers/staging/comedi/drivers/dt2817.c index 7c1463e..a173394 100644 --- a/drivers/staging/comedi/drivers/dt2817.c +++ b/drivers/staging/comedi/drivers/dt2817.c @@ -21,7 +21,7 @@ * with 32 channels, configurable in groups of 8. * * Configuration options: - * [0] - I/O port base base address + * [0] - I/O port base address */ #include -- 2.7.4
