Greg KH wrote:
>> In the original patch I sent, this was inlined. Greg KH made the
>> change and added the exported function for other drivers to call. The
>> original patch used the same methodology that the uart drivers used
>> with the serial headers.
>
> Ah, sorry, I thought it would be helpful for other drivers to be able to
> call this. Inline vs. non-inline shouldn't be the issue here, it's an
> overall "let's not touch every individual byte" issue :)
>
>> I agree that these function calls don't belong in the fast path unless
>> the device is acting as a system console, which is the only place a
>> sysrq should be processed in the first place. The handle break
>> function was already changed in a later patch in the series to only
>> fire its function calls if the device is acting as a system console.
>>
>> Would you find the revised patch acceptable Alan, where the
>> functions are only called when the device is registered as a system
>> console? This also restores the multi-byte behavior when not a system
>> console which is definitely seems appropriate.
>>
>> Specifically we are talking about the section:
>>
>> if (port->console) {
>> for (i = 0; i < urb->actual_length; i++, ch++) {
>> if (!usb_serial_handle_sysrq_char(port, *ch))
>> tty_insert_flip_char(tty, *ch, TTY_NORMAL);
>> }
>> tty_flip_buffer_push(tty);
>> } else if (urb->actual_length) {
>> i = tty_buffer_request_room(tty, urb->actual_length);
>> if (i) {
>> tty_insert_flip_string(tty, urb->transfer_buffer, i);
>> tty_flip_buffer_push(tty);
>> }
>> }
>>
>>
>> Also should I submit a new patch which converts the
>> usb_serial_handle_sysrq_char() back to an inline?
>
> No, I think other drivers still need to call this, right?
>
Yes other drivers could need to call the sysrq checks directly, but it
is fine to convert it back to a static inline, there are no functions
called there which are not already exported in the kernel's export
function table (so we can still build external usb serial kernel
modules).
The original looked something like the following and was directly
derived from the serial uart equivalent.
static inline int
usb_serial_handle_sysrq_char(struct usb_serial_port *port, unsigned int ch)
{
#ifdef CONFIG_SYSRQ
if (port->sysrq) {
if (ch && time_before(jiffies, port->sysrq)) {
handle_sysrq(ch, tty_port_tty_get(&port->port));
port->sysrq = 0;
return 1;
}
port->sysrq = 0;
}
#endif /* CONFIG_SYSRQ */
return 0;
}
Alan correctly pointed out that we don't even want to look at each
byte in the first place, if it is not a console device, so we
definitely need that proposed change. The in-lining is a further
optimization. If this is what we want, I will generate a new patch.
Jason.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
