Re: [PATCH 3/5] tty: handle VT specific compat ioctls in vt driver

[Arnd Bergmann <arnd@arndb.de>]

On Friday 07 August 2009, Frederic Weisbecker wrote:

> On Thu, Aug 06, 2009 at 03:09:28PM +0200, Arnd Bergmann wrote:
> > The VT specific compat_ioctl handlers are the only ones
> > in common code that require the BKL. Moving them into
> > the vt driver lets us remove the BKL from the other handlers
> > and cleans up the code.
> 
> 
> Why does it require the bkl?
> 

All the VT ioctls are currently called under the BKL. I did not try
to find out why that is but simply kept that state. All other
compat ioctl do not interact with device driver state at all,
so they obviously do not need the BKL.

> > +
> > +long vt_compat_ioctl(struct tty_struct *tty, struct file * file,
> > +	     unsigned int cmd, unsigned long arg)
> > +{
> > +	struct vc_data *vc = tty->driver_data;
> > +	struct console_font_op op;	/* used in multiple places here */
> > +	struct kbd_struct *kbd;
> > +	unsigned int console;
> > +	void __user *up = (void __user *)arg;
> > +	int perm;
> > +	int ret = 0;
> > +
> > +	console = vc->vc_num;
> > +
> > +	lock_kernel();
> 
> 
> 
> It would be really nice to add a comment here that explain what it
> is protecting.
> I would like to work on removing the bkl from tty, and such nude lock_kernel()
> don't help much to work in this area.
> This is more than ever a FUD lock, nothing about its role in tty in the Lockronomicon,
> and even not in the comments :-)

This function is a straight copy from vt_ioctl, with the data structures replaced,
and calling it vt_ioctl where they are identical.

You are right that this needs more code comments to make that obvious.

	Arnd <><