Re: [PATCH] uio: Allow to take irq bottom-half into irq_handler with additional dt-binding

From: Greg KH
Date: Thu Dec 07 2017 - 04:16:11 EST



A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

A: No.
Q: Should I include quotations after my reply?

http://daringfireball.net/2007/07/on_top

On Wed, Dec 06, 2017 at 05:59:01PM +0100, Andrey Zhizhikin wrote:
> Hello Greg,
>
> Thanks a lot for your prompt reply!
>
> First of, this is my first patch submission to the Kernel, so thanks a
> lot for your additional guidelines here regarding missing pieces.
> Please don't judge me hard here. :)

No worries, everyone has to start somewhere :)

> I would add new DT bindings to Documentation and contact DT
> maintainers to have a new binding discussed. However, I was not able
> to find any dt-binding documentation for uio drivers in the kernel,
> presumably I would have to create a new entry there...

The uio-phys driver does use DT bindings, so perhaps look at how those
are defined.

> As for the win against latency and running the patch against the
> system which has all IRQ in threaded mode:
>
> I've actually originated this patch based on the PREEMPT_RT kernel
> configuration, where all IRQs are threaded. I have ARM-based system
> running around 20 genirq UIO instances, and was demoting 2 of those
> from threaded to non-threaded IRQ handlers without any issues recorded
> to all the IRQ handlers. This patch actually is aimed exactly with the
> logic that if new property is not found - then system behavior is not
> amended, and all IRQs stays threaded. If needed, then a developer can
> enable this property for it's node, but then he should be well-aware
> of what this property implications are.
>
> In average, using ftrace and kernelshark to analyze I observed the
> gain of 20-30 usec per chain: irq_handler_entry -> irq/XX-uio -> <User
> space IRQ part> so I would say the gain is not very significant for
> average user-space task. However IMHO, there are several hidden
> benefits here with having this modification, namely:
> - It eliminates few re-scheduling operations, making INT ACK code more
> robust and relieves the pressure from the scheduler when HW interrupt
> for this IRQ is signaled at a high-enough frequency;
> - It makes top and bottom half to be executed back-to-back with IRQ
> OFF, making operation pseudo-atomic;
> - Above gain might be significant when average latency times for the
> systems are comparable.

Ok, that all seems like a good thing to have the ability to do here, you
should mention it in the changelog text when you redo this patch.

thanks,

greg k-h