LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Pete Zaitcev <zaitcev@redhat.com>
To: vitalivanov@gmail.com
Cc: Oliver Neukum <oliver@neukum.org>,
	linux-usb-devel@lists.sourceforge.net, greg@kroah.com,
	linux-kernel@vger.kernel.org, netwiz@crc.id.au,
	zaitcev@redhat.com
Subject: Re: USB: FIx locks and urb->status in adutux
Date: Mon, 29 Oct 2007 21:24:33 -0700	[thread overview]
Message-ID: <20071029212433.c9837c4b.zaitcev@redhat.com> (raw)
In-Reply-To: <1193681097.20985.31.camel@dell1.softservecom.com>

On Mon, 29 Oct 2007 20:04:57 +0200, Vitaliy Ivanov <vitalivanov@gmail.com> wrote:

> Finally had a time on my weekend to perform tests and fix Pete's patch a little. Now it seems to be correct.

Great!

> 1. One device per user space application. Old driver allowed many users application to work with same device which can lead to IO mess.

OK. This trick was popular in UNIX. Personally I think it's in a bad
taste, because good applications still need to verify if only one
instance is running, and threfore can use application level locking.
But if you are gunning for the maintenership I'm not going to argue
your style. The busy lock-out certainly works better than "/dev/cua" :-)

However, this looks wrong:

> +			 dev->interrupt_in_endpoint->bInterval);
> +	dev->read_urb_finished = 0;
> +	retval = usb_submit_urb(dev->interrupt_in_urb, GFP_KERNEL);
> +	/* we ignore failure */
> +	/* end of fixup for first read */
> +
> +	/* initialize out direction */
> +	dev->out_urb_finished = 1;

The finished flag is only set when URB is not in use anymore. Did you
observe an anomaly with my code? Any hangs? If so, I assure you this
is not the fix. As it's written, even if we ignore the failure (e.g.
do not pass it to userland), we sill have to maintain the correct
flag state.

-- Pete

  reply	other threads:[~2007-10-30  4:24 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-10-23  3:34 Pete Zaitcev
2007-10-23  9:38 ` [linux-usb-devel] " Oliver Neukum
2007-10-23 21:38   ` Pete Zaitcev
2007-10-24 14:04     ` Oliver Neukum
2007-10-24  1:53   ` Pete Zaitcev
2007-10-24 14:09     ` Vitaliy Ivanov
2007-10-25  3:20       ` Pete Zaitcev
2007-10-25  3:25       ` Pete Zaitcev
     [not found]         ` <200710251403.48688.oliver@neukum.org>
2007-10-25 16:38           ` Pete Zaitcev
2007-10-29 18:04         ` Vitaliy Ivanov
2007-10-30  4:24           ` Pete Zaitcev [this message]
2007-10-30 13:09             ` Vitaliy Ivanov
2007-10-30 21:54               ` Pete Zaitcev
2007-10-31 11:54                 ` Vitaliy Ivanov
2007-10-31 22:01                   ` Pete Zaitcev
2007-11-01  9:06                     ` Vitaliy Ivanov
2007-11-01 17:28                       ` Pete Zaitcev
2007-10-24 14:49     ` Oliver Neukum
2007-10-24 21:25       ` Greg KH
2007-10-26  9:57         ` Vitaliy Ivanov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20071029212433.c9837c4b.zaitcev@redhat.com \
    --to=zaitcev@redhat.com \
    --cc=greg@kroah.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb-devel@lists.sourceforge.net \
    --cc=netwiz@crc.id.au \
    --cc=oliver@neukum.org \
    --cc=vitalivanov@gmail.com \
    --subject='Re: USB: FIx locks and urb->status in adutux' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).