small design problem with hso driver

small design problem with hso driver

[Denis Joseph Barrow]

Hi,
I'm getting a fairly consistently reproducable problem
with the 3g modem hso driver. The most reproducable
bug I have.

It happens when I plug out the usb modem & ctrl-c
out of an ioctl call with the modem connected in paralell over ppp
with wvdial 

Basically whats happening is the hso_serial_close
is being called more times than the hso_serial_open &
I don't know why.

I can put in a kludge to cure the symptom which would prevent the crash
but it would be nice to know why this might be
happening.


-- 
best regards,
D.J. Barrow
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: small design problem with hso driver

[Greg KH]

On Wed, Oct 08, 2008 at 03:52:38PM +0200, Denis Joseph Barrow wrote:
> Hi,
> I'm getting a fairly consistently reproducable problem
> with the 3g modem hso driver. The most reproducable
> bug I have.
> 
> It happens when I plug out the usb modem & ctrl-c
> out of an ioctl call with the modem connected in paralell over ppp
> with wvdial 
> 
> Basically whats happening is the hso_serial_close
> is being called more times than the hso_serial_open &
> I don't know why.

Care to put a trace or printk() to determine who is doing each open and
close?

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: small design problem with hso driver

[Denis Joseph Barrow]

Hi,
I've found something suspicous but am not sure if
it's my problem. It looks like it when I'm
doing a gdb debug session but I can't believe
code so wrong can get into the kernel
& it didn't cause problems before.

Have a look at /drivers/char/tty_io.c

tty_open

around line 2234 this calls
tty->ops->open which calls hso_serial_open
& if this returns an error i.e. -ENODEV
release_dev is called which calls in tty_open
which calls tty->ops_close around line 2003 in tty_io.c.

Surely if the driver returns an error code on hso_serial_open
hso_serial_close shouldn't be called
or have I a complete misunderstanding
of char driver semantics.

Greg KH wrote:
> On Wed, Oct 08, 2008 at 03:52:38PM +0200, Denis Joseph Barrow wrote:
>> Hi,
>> I'm getting a fairly consistently reproducable problem
>> with the 3g modem hso driver. The most reproducable
>> bug I have.
>>
>> It happens when I plug out the usb modem & ctrl-c
>> out of an ioctl call with the modem connected in paralell over ppp
>> with wvdial 
>>
>> Basically whats happening is the hso_serial_close
>> is being called more times than the hso_serial_open &
>> I don't know why.
> 
> Care to put a trace or printk() to determine who is doing each open and
> close?
> 
> thanks,
> 
> greg k-h


-- 
best regards,
D.J. Barrow
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: small design problem with hso driver

[Alan Cox]

> Surely if the driver returns an error code on hso_serial_open
> hso_serial_close shouldn't be called
> or have I a complete misunderstanding
> of char driver semantics.

Generally yes that would be sane. The tty layer has always called the
close method of drivers on a failed open and the drivers are all written
assuming that.

It would be nice to change but there are far bigger fish to fry.

Alan
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: small design problem with hso driver

[Denis Joseph Barrow]

Hi Alan,
No they are not.
Have a look at.
/drivers/char/nozomi.c
admittedly it's written by some of the same guys.
/* Called when the userspace process opens the tty, /dev/noz*.  */
static int ntty_open(struct tty_struct *tty, struct file *file)
{
	struct port *port = get_port_by_tty(tty);
	struct nozomi *dc = get_dc_by_tty(tty);
	unsigned long flags;

	if (!port || !dc || dc->state != NOZOMI_STATE_READY)
		return -ENODEV;

	if (mutex_lock_interruptible(&port->tty_sem))
		return -ERESTARTSYS;

	port->tty_open_count++;
	dc->open_ttys++;

	/* Enable interrupt downlink for channel */
	if (port->tty_open_count == 1) {
		tty->low_latency = 1;
		tty->driver_data = port;
		port->tty = tty;
		DBG1("open: %d", port->token_dl);
		spin_lock_irqsave(&dc->spin_mutex, flags);
		dc->last_ier = dc->last_ier | port->token_dl;
		writew(dc->last_ier, dc->reg_ier);
		spin_unlock_irqrestore(&dc->spin_mutex, flags);
	}

	mutex_unlock(&port->tty_sem);

	return 0;
}

/* Called when the userspace process close the tty, /dev/noz*. */
static void ntty_close(struct tty_struct *tty, struct file *file)
{
	struct nozomi *dc = get_dc_by_tty(tty);
	struct port *port = tty->driver_data;
	unsigned long flags;

	if (!dc || !port)
		return;

	if (mutex_lock_interruptible(&port->tty_sem))
		return;

	if (!port->tty_open_count)
		goto exit;

	dc->open_ttys--;
	port->tty_open_count--;

	if (port->tty_open_count == 0) {
		DBG1("close: %d", port->token_dl);
		spin_lock_irqsave(&dc->spin_mutex, flags);
		dc->last_ier &= ~(port->token_dl);
		writew(dc->last_ier, dc->reg_ier);
		spin_unlock_irqrestore(&dc->spin_mutex, flags);
	}

exit:
	mutex_unlock(&port->tty_sem);
}


Alan Cox wrote:
>> Surely if the driver returns an error code on hso_serial_open
>> hso_serial_close shouldn't be called
>> or have I a complete misunderstanding
>> of char driver semantics.
> 
> Generally yes that would be sane. The tty layer has always called the
> close method of drivers on a failed open and the drivers are all written
> assuming that.
> 
> It would be nice to change but there are far bigger fish to fry.
> 
> Alan


-- 
best regards,
D.J. Barrow
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: small design problem with hso driver

[Alan Cox]

On Thu, 09 Oct 2008 11:56:38 +0200
Denis Joseph Barrow <D.Barow-x9gZzRpC1QbQT0dZR+AlfA@public.gmane.org> wrote:

> Hi Alan,
> No they are not.
> Have a look at.
> /drivers/char/nozomi.c
> admittedly it's written by some of the same guys.

Nozomi still has lots and lots of bugs left. Compared to the original its
now a work of art and much good stuff has been done but it is not by any
means 'done'.

File a bug with the nozomi author
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html