lkml.org 
[lkml]   [2006]   [Jun]   [23]   [last100]   RSS Feed
Views: [more markup]  [less markup]  [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
DateFri, 23 Jun 2006 03:04:47 -0700
FromAndrew Morton <>
SubjectRe: [patch 36/61] lock validator: special locking: serial
On Fri, 23 Jun 2006 11:49:41 +0200
Ingo Molnar <mingo@elte.hu> wrote:

> > * Andrew Morton <akpm@osdl.org> wrote:
> 
> > > +/*> > > + * lockdep: port->lock is initialized in two places, but we
> > > + *          want only one lock-type:> > > + */> > > +static struct lockdep_type_key port_lock_key;> > > +> > >  /**
> > >   *	uart_set_options - setup the serial console parameters
> > >   *	@port: pointer to the serial ports uart_port structure
> > > @@ -1869,7 +1875,7 @@ uart_set_options(struct uart_port *port,
> > >  	 * Ensure that the serial console lock is initialised
> > >  	 * early.
> > >  	 */
> > > -	spin_lock_init(&port->lock);
> > > +	spin_lock_init_key(&port->lock, &port_lock_key);
> > > 
> > >  	memset(&termios, 0, sizeof(struct termios));
> > > 
> > > @@ -2255,7 +2261,7 @@ int uart_add_one_port(struct uart_driver
> > >  	 * initialised.
> > >  	 */
> > >  	if (!(uart_console(port) && (port->cons->flags & CON_ENABLED)))
> > > -		spin_lock_init(&port->lock);
> > > +		spin_lock_init_key(&port->lock, &port_lock_key);
> > > 
> > >  	uart_configure_port(drv, state, port);
> > > > > > > Is there a cleaner way of doing this?> > > > Perhaps write a new helper function which initialises the spinlock, 
> > call that?  Rather than open-coding lockdep stuff?
> 
> yes, we can do that too - but that would have an effect to non-lockdep 
> kernels too.
> 
> Also, the initialization of the 'port' seems a bit twisted here, already 
> initialized and not-yet-initialized ports can be passed in to 
> uard_add_one_port(). So i did not want to touch the structure of the 
> code - hence the open-coded solution.
> 

btw, I was looking at this change:

diff -puN drivers/scsi/libata-core.c~lock-validator-locking-init-debugging-improvement drivers/scsi/libata-core.c
--- a/drivers/scsi/libata-core.c~lock-validator-locking-init-debugging-improvement
+++ a/drivers/scsi/libata-core.c
@@ -1003,6 +1003,7 @@ unsigned ata_exec_internal(struct ata_de
 	unsigned int err_mask;
 	int rc;
 
+	init_completion(&wait);
 	spin_lock_irqsave(ap->lock, flags);
 
 	/* no internal command while frozen */
That local was already initialised with DEFINE_WAIT().  Am surprised that
an init_wait() also was needed?

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

\
 
 \ /
  Last update: 2006-06-23 12:07    [from the cache]
©2003-2008