(timeout) error during cd fixation

[Alexander Best]

thanks a lot. with the patch you attached the CDRIOCFIXATE-error completely
disappeared. i tested it burning a cd using maximum speed settings.

i guess this entirely solves the issues described in
http://www.freebsd.org/cgi/query-pr.cgi?pr=95979 so the PR should be closed.

can you commit the changes in ata-queue.c to svn/head? it would also be nice
if the patch would make it into 7.1-RELEASE.

cheers.
alex

Jaakko Heinonen schrieb am 2008-12-17:

> Hi,

> On 2008-12-14, Alexander Best wrote:
> > as far as i remember this issue (ioctl(CDRIOCFIXATE): Input/output
> > error) has
> > always been present in burncd (since RELENG4 oder RELENG5).

> > i found a PR in feedback state dealing with this problem
> > (http://www.freebsd.org/cgi/query-pr.cgi?pr=95979&cat=bin). the PR
> > includes a
> > patch which i applied. however that didn't solve the issue.

> > the problem seems to be the timeout variable used in acd_fixate()
> > in
> > sys/dev/ata/atapi-cd.c. it get's set to:

> > int timeout = 5*60*2;

> > changing timeout to a higher value doesn't make much of a
> > difference though,
> > because it gets altered a bit further down in the src.

> It's divided by burning speed. It seems that the approximation
> doesn't
> work very well at higher burning speeds.

> > that's the reason the
> > patch didn't fix the issue for me. this is the code where timout
> > gets altered
> > again:

> >     /* some drives just return ready, wait for the expected fixate
> >     time */
> >     if ((error = acd_test_ready(dev)) != EBUSY) {
> >         timeout = timeout / (cdp->cap.cur_write_speed / 177);
> >         pause("acdfix", timeout * hz / 2);
> >         return acd_test_ready(dev);
> >     }

> > it seems my drive doesn't return EBUSY, although it's still busy.
> > that's why

> I think I've found the bug now. Looks like ata_atapicmd() can never
> really return EBUSY which is expected in acd_fixate(). When
> acd_test_ready() (ata_atapicmd()) reports correctly the busy state
> there's no need for use the expected wait time method.

> I will take a closer look when I have more time. In the mean time
> could
> you try this patch?

> %%%
> Index: sys/dev/ata/ata-queue.c
> ===================================================================
> --- sys/dev/ata/ata-queue.c     (revision 186213)
> +++ sys/dev/ata/ata-queue.c     (working copy)
> @@ -434,7 +434,8 @@ ata_completed(void *context, int dummy)
>                 printf("\n");
>         }

> -       if ((request->u.atapi.sense.key & ATA_SENSE_KEY_MASK ?
> +       if (!request->result &&
> +            (request->u.atapi.sense.key & ATA_SENSE_KEY_MASK ?
>              request->u.atapi.sense.key & ATA_SENSE_KEY_MASK :
>              request->error))
>             request->result = EIO;
> %%%

> > dd: /dev/acd0: Input/output error

> This is unrelated and AFAIK expected.