linux-kernel - Re: [PATCH]: Atmel Serial Console interrupt handler splitup

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <3efb10970712172332o32b6dab5l33ecac57d18f49d4@mail.gmail.com>
Date:	Tue, 18 Dec 2007 08:32:57 +0100
From:	"Remy Bohmer" <linux@...mer.net>
To:	"Haavard Skinnemoen" <hskinnemoen@...el.com>
Cc:	"Andrew Victor" <linux@...im.org.za>,
	RT <linux-rt-users@...r.kernel.org>,
	"Steven Rostedt" <rostedt@...dmis.org>,
	"ARM Linux Mailing List" <linux-arm-kernel@...ts.arm.linux.org.uk>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH]: Atmel Serial Console interrupt handler splitup

Hello Haavard,

> > Yep. All calls that block on a Mutex somehow on Preempt-RT. (such as
> > spinlocks, wakeup_interruptible() and many of its friends.)
> Right. Looks like the DMA patch call these functions from irq context
> too...I guess it'll need the same treatment?

That is correct. DMA code does do that, but the DMA code is not used
for DBGU, and _only_ the interrupt handler of DBGU runs in
IRQF_NODELAY context (because it is part of the System-IRQ and thus
shared with the timer-irq). The DMA code always runs in IRQ-thread
context, where it is safe to block on a mutex.
So, it is not mandatory to change that also. (It may be nice to do it anyway)

> > > > +struct atmel_uart_char {
> > > > +     unsigned int status;
> > > > +     unsigned int overrun;
> > > > +     unsigned int ch;
> > > > +     unsigned int flg;
> > > > +};
> > >
> > > Hmm. 16 bytes per char is a bit excessive, isn't it? How about
> > >
> > > struct atmel_uart_char {
> > >         u16     ch;
> > >         u16     status;
> > > };
> > >
> > > where ch is the received character (up to 9 bits) and status is the
> > > lowest 16 bits of the status register?
> > I used the NODELAY patch for the 8250 serial port as reference, it
> > contains similar code, and I tried to make both look the same.
> Ok, I see. But...
> > > > +#define ATMEL_SERIAL_RINGSIZE 1024
> This means that the buffer ends up at 16K. Since the buffer itself is
> kept in a struct along with a few other pieces of data, we end up
> kmalloc()ing 32KB of memory...

Oops... 32kB on X86 is not much, relatively, on ARM it is somewhat different.
We can also decrease the total RingSize, 128 would be good enough also.
I can look at it later this week.

> I'm a bit tempted to just go ahead and do the changes we agree on and
> post the result. Then I'll see if I can make the DMA stuff behave. I've
> got a different driver lying around which is DMA-only and has a few
> problems on its own.

Beware that, according to several older discussions, DBGU cannot make
use of DMA... (If I understood it correctly, it was because the buffer
first have to be full, before an interupt is generated. That would be
very annoying while typing 1 character at the time ;-)

> Also, I think the DMA patch needs to be more integrated with your
> "interrupt handler splitup" patch. No point in keeping two RX buffers
> around, and the DMA code needs to obey the same rules as the rest of
> the driver if it's going to work in -rt.

As mentioned, not necessarily...

> And I'd really like to have working DMA support on avr32 before
> christmas ;-)

Me too ;-)


Kind Regards,

Remy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/