Home
Reading
Searching
Subscribe
Sponsors
Statistics
Posting
Contact
Spam
Lists
Links
About
Hosting
Filtering
Features Download
Marketing
Archives
FAQ
Blog
 
Gmane
From: Dan Dennedy <dan <at> dennedy.org>
Subject: Re: ISO Send Problems - Possible Kernel Compatibility
Newsgroups: gmane.linux.kernel.firewire.user
Date: Sunday 24th September 2006 07:17:01 UTC (over 11 years ago)
On Friday 22 September 2006 10:34, Robert Hailey wrote:
> On Feb 21, 2006, at 6:40 PM, Stefan Richter wrote:
> > Robert Hailey wrote:
> >> On Feb 21, 2006, at 2:37 PM, Stefan Richter wrote:
> >>> So the regression happened either in 2.6.13 or 2.6.14. Could you
> >>> please test 2.6.13?
> >>
> >> I just tested 2.6.13 and found that it has my same problem.
> >
> > Thanks. It's a pity; I think there were a few more relevant changes
> > from 2.6.12 to 2.6.13 than from 2.6.13 to 2.6.14. What's more, the
> > change history before 2.6.13 is not as easily accessible as after
> > 2.6.13. Anyway, this information should enable us to narrow the bug
> > further down.
> > --
> > Stefan Richter
> > -=====-=-==- --=- =-==-
> > http://arcgraph.de/sr/
>
> Is there a bug number for this? I have tracked down the difference
> between 2.6.12 and 2.6.13 that presented this problem. Line numbers are
> from the latest stable (2.6.17.13).
>
> ---- Stack trace ----
> raw1394_loop_iterate
> 	RAW_ISO_ACTIVITY
> 		_raw1394_iso_iterate
> 			_raw1394_iso_xmit_queue_packets
> 				ioctl(RAW1394_IOC_ISO_XMIT_PACKETS);
> -------kernel---------------------------------
> 				ioctl(RAW1394_IOC_ISO_XMIT_PACKETS)
> 					raw1394_iso_send_packets
>
>
> ---- drivers/ieee1394/raw1394.c:2595 ----
> 	if (upackets.n_packets >= fi->iso_handle->buf_packets)
> 		return -EINVAL;
>
> 	if (upackets.n_packets >= hpsb_iso_n_ready(fi->iso_handle))
> 		return -EAGAIN;
> --------
>
> Unless I am missing something, this looks like a simple off-by-one
> error. The ">" was changed to a ">=" in 2.6.13 along with the addition
> of another constraint also using ">=" potentially incorrectly. Changing
> these to "greater-than" allows the same code to run successfully. I
> imagine the userland/kernel interaction like this...
>
> --> user("I would like to buffer 200 packets")
> <-- kernel(0): OK.
>
> --> user("here is the first 200 packets")
> <-- kernel(EINVAL): NO, you can't buffer that many, only 199

Yes, that is correct due to an implementation detail of rawiso, you can not

use all of the buffers at the same time. I am sorry we had to expose 
userspace to this implementation detail.

There was a problem and discussion here that lead to those changes:
http://marc2.theaimsgroup.com/?l=linux1394-devel&m=111903200810298&w=2

> Question 1: is there an allocated buffer somewhere that this was
> overflowing, and that is why this was changed?

The DMA descriptor chain always needs to indicate a stop on one buffer so
it 
can stay in sync with subsequently submitted buffers. If you try to submit 
all buffers, then it wraps around to the first buffer, telling DMA to stop 
immediately.

> Question 2: has newer versions of libraw compensated for this by adding
> a "+1" somewhere?

Yes, the libraw1394 change came prior to the kernel changes:
http://marc2.theaimsgroup.com/?l=linux1394-devel&m=111947481207342&w=2

The kernel changes were put in place to safeguard against DMA not running
when 
not using libraw1394 and directly using kernel interface.

> Question 3: what about the "buf_packets - 1"s that where added to
> hpsb_iso_xmit_start in the same patch?
> ---- drivers/ieee1394/raw1394.c:2595 ----
> 	if (prebuffer < 0)
> 		prebuffer = iso->buf_packets - 1;
> 	else if (prebuffer == 0)
> 		prebuffer = 1;
>
> 	if (prebuffer >= iso->buf_packets)
> 		prebuffer = iso->buf_packets - 1;
> ------------

To initialize these to sane values for the same reason.

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share
your
opinions on IT & business topics through brief surveys -- and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
 
CD: 3ms