Re: [PATCH] tty races

[Andrew Morton]

Jason Baron <jbaron@xxxxxxxxxx> wrote:
>
> > 
> > I don't see anywhere which takes lock_kernel() on the tty_open() path.
> > 
> 
> fs/char_dev.c:chrdev_open():        
> 
> 	if (filp->f_op->open) {
>                 lock_kernel();
>                 ret = filp->f_op->open(inode,filp);
>                 unlock_kernel();
>         }
> 

hm, we're still doing that.

> > 
> > We want to move away from lock_kernel()-based locking.
> > 
> 
> I completely agree, but unfortunately lock_kernel() is currently used 
> extensively throughout the tty layer. 

Well no - it's being migrated over to use tty_sem.  We shouldn't start
heading in the reverse direction.  Plus your patch reverts part of
http://linux.bkbits.net:8080/linux-2.5/diffs/drivers/char/tty_io.c@xxxxx?nav=index.html|src/|src/drivers|src/drivers/char|hist/drivers/char/tty_io.c
in ways which might be unsafe.

> lock_kernel() is used extensively throughout the tty layer. We can 
> re-write the locking for the layer, but I'd like to see this bug fix in 
> 2.6.12, if that isn't done in time.

Sorry, but AFAICT all you have done is to advocate for the existing patch
without having attempted to fix this problem with tty_sem.  Please try to
come up with a tty_sem-based fix.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/