Re: Bug in "ide-pci.c"

2000-10-07 Thread David Woodhouse

On Fri, 6 Oct 2000, Andre Hedrick wrote:

> void go_take_a_dump (float load)
> {
>   if (pull_down_pants() && purge_bowles() && wipe_anus() && pull_up_pants())
>  flush(load);
>   else
>  wear(load);
> }

But here you make another classic mistake. Consider the case where
purge_bowels() fails (-EWOULDBLOCK?). 

In that case, you don't actually execute the subsequent two procedure
calls, which I'm sure we all agree is a suboptimal state of affairs.

-- 
dwmw2


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Bug in ide-pci.c

2000-10-07 Thread David Woodhouse

On Fri, 6 Oct 2000, Andre Hedrick wrote:

 void go_take_a_dump (float load)
 {
   if (pull_down_pants()  purge_bowles()  wipe_anus()  pull_up_pants())
  flush(load);
   else
  wear(load);
 }

But here you make another classic mistake. Consider the case where
purge_bowels() fails (-EWOULDBLOCK?). 

In that case, you don't actually execute the subsequent two procedure
calls, which I'm sure we all agree is a suboptimal state of affairs.

-- 
dwmw2


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Bug in "ide-pci.c"

2000-10-06 Thread Sean Estabrooks

Andre,
(Thanks for calling my bluff) && (I was wrong).  

  Sean


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Bug in "ide-pci.c"

2000-10-06 Thread Tom Leete

Sean Estabrooks wrote:
> 
> Hi Andre,
> 
>   The if() logic must then rely on implementation specific compiler
> details and not have any optimizations which break this code.   While it may
> "WORK" it isn't particularly reliable code.
> 
>   Sean

Nope, the logical ops are sequence points required by
standard to lazy evaluate left to right. For ||, that means
they operate in order till they find a true.

Tom
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Bug in "ide-pci.c"

2000-10-06 Thread Andre Hedrick

On Fri, 6 Oct 2000, Sean Estabrooks wrote:

> Hi Andre,
> 
>   The if() logic must then rely on implementation specific compiler
> details and not have any optimizations which break this code.   While it may
> "WORK" it isn't particularly reliable code.

If that is the case and it is proven then there is more code than mine
that will fail.  Also if the compiler is allowed in optimizations to break
specific test order, then it is a bad compiler.

Order of operation is critical in writing code.

In my world of ATA, you dork the order of a command and you get crap on
your drive.  So since there is not crap on the drive in general, I am
accounting for stupid programmer errors in the past, I call your bluff.

If you issue the command register first the the rest of the register will
execute on the next command register.  This is doing thing bass-ackwards.

void go_take_a_dump (float load)
{
  if (pull_down_pants() && purge_bowles() && wipe_anus() && pull_up_pants())
 flush(load);
  else
 wear(load);
}

You do this out of order and this is more descriptive.

Now if you are complaining about logical operators in the test and how
these get optimized then you point is noted.

Since I was an astronomer and +/- one AU (93,000,000 miles) is critical,
feel free to check the rules for order.  Then you can call my bluff.

Cheers,

Andre Hedrick
The Linux ATA/IDE guy

FYI, I have a new baby-girl and thus the diaper-humor!

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Bug in "ide-pci.c"

2000-10-06 Thread Sean Estabrooks

Hi Andre,

  The if() logic must then rely on implementation specific compiler
details and not have any optimizations which break this code.   While it may
"WORK" it isn't particularly reliable code.

  Sean


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Bug in "ide-pci.c"

2000-10-06 Thread Jeff V. Merkey


Andre,

BTW, how did your testing of the speed=4 problem with ide-scsi turn
out.  We are still seeing the speed=2 problem on 2.4.0-pre9.  I cannot
get the drive to burn clean unless the speed setting is cranked down to
speed=2.

Jeff 

Andre Hedrick wrote:
> 
> On Fri, 6 Oct 2000, Sean Estabrooks wrote:
> 
> > ide-pci.c bug:
> >
> > ide_setup_pci_baseregs() may inappropriately report device as not capable of full 
>native PCI:
> >
> > //  BUGGY LINE: 
> > if (pci_read_config_byte(dev, PCI_CLASS_PROG, ) || (progif & 5) != 5) {
> > //  =   TWO CONDITIONS USING PROGIF
> >if ((progif & 0xa) != 0xa) {
> >   printk("%s: device not capable of full native PCI mode\n", name);
> >   return 1;
> >}
> >
> >  ...
> >
> > In the first line of code above there is no guarantee that the first condition 
>will be executed
> > before the second.  As progif is set to 0 before this block of code, the second 
>test will always
> > be true if it is executed prior to the first.
> 
> if () are serial in test.
> 
> >
> >
> >
> 
> Andre Hedrick
> The Linux ATA/IDE guy
> 
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [EMAIL PROTECTED]
> Please read the FAQ at http://www.tux.org/lkml/
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Bug in "ide-pci.c"

2000-10-06 Thread Andre Hedrick

On Fri, 6 Oct 2000, Sean Estabrooks wrote:

> ide-pci.c bug:
> 
> ide_setup_pci_baseregs() may inappropriately report device as not capable of full 
>native PCI:
> 
> //  BUGGY LINE: 
> if (pci_read_config_byte(dev, PCI_CLASS_PROG, ) || (progif & 5) != 5) {
> //  =   TWO CONDITIONS USING PROGIF 
>if ((progif & 0xa) != 0xa) {
>   printk("%s: device not capable of full native PCI mode\n", name);
>   return 1;
>}
> 
>  ...
> 
> In the first line of code above there is no guarantee that the first condition 
>will be executed
> before the second.  As progif is set to 0 before this block of code, the second test 
>will always
> be true if it is executed prior to the first.

if () are serial in test.

> 
> 
> 

Andre Hedrick
The Linux ATA/IDE guy

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Bug in ide-pci.c

2000-10-06 Thread Andre Hedrick

On Fri, 6 Oct 2000, Sean Estabrooks wrote:

 ide-pci.c bug:
 
 ide_setup_pci_baseregs() may inappropriately report device as not capable of full 
native PCI:
 
 //  BUGGY LINE: 
 if (pci_read_config_byte(dev, PCI_CLASS_PROG, progif) || (progif  5) != 5) {
 //  =   TWO CONDITIONS USING PROGIF 
if ((progif  0xa) != 0xa) {
   printk("%s: device not capable of full native PCI mode\n", name);
   return 1;
}
 
  ...
 
 In the first line of code above there is no guarantee that the first condition 
will be executed
 before the second.  As progif is set to 0 before this block of code, the second test 
will always
 be true if it is executed prior to the first.

if () are serial in test.

 
 
 

Andre Hedrick
The Linux ATA/IDE guy

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Bug in ide-pci.c

2000-10-06 Thread Jeff V. Merkey


Andre,

BTW, how did your testing of the speed=4 problem with ide-scsi turn
out.  We are still seeing the speed=2 problem on 2.4.0-pre9.  I cannot
get the drive to burn clean unless the speed setting is cranked down to
speed=2.

Jeff 

Andre Hedrick wrote:
 
 On Fri, 6 Oct 2000, Sean Estabrooks wrote:
 
  ide-pci.c bug:
 
  ide_setup_pci_baseregs() may inappropriately report device as not capable of full 
native PCI:
 
  //  BUGGY LINE: 
  if (pci_read_config_byte(dev, PCI_CLASS_PROG, progif) || (progif  5) != 5) {
  //  =   TWO CONDITIONS USING PROGIF
 if ((progif  0xa) != 0xa) {
printk("%s: device not capable of full native PCI mode\n", name);
return 1;
 }
 
   ...
 
  In the first line of code above there is no guarantee that the first condition 
will be executed
  before the second.  As progif is set to 0 before this block of code, the second 
test will always
  be true if it is executed prior to the first.
 
 if () are serial in test.
 
 
 
 
 
 Andre Hedrick
 The Linux ATA/IDE guy
 
 -
 To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
 the body of a message to [EMAIL PROTECTED]
 Please read the FAQ at http://www.tux.org/lkml/
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Bug in ide-pci.c

2000-10-06 Thread Sean Estabrooks

Hi Andre,

  The if() logic must then rely on implementation specific compiler
details and not have any optimizations which break this code.   While it may
"WORK" it isn't particularly reliable code.

  Sean


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Bug in ide-pci.c

2000-10-06 Thread Andre Hedrick

On Fri, 6 Oct 2000, Sean Estabrooks wrote:

 Hi Andre,
 
   The if() logic must then rely on implementation specific compiler
 details and not have any optimizations which break this code.   While it may
 "WORK" it isn't particularly reliable code.

If that is the case and it is proven then there is more code than mine
that will fail.  Also if the compiler is allowed in optimizations to break
specific test order, then it is a bad compiler.

Order of operation is critical in writing code.

In my world of ATA, you dork the order of a command and you get crap on
your drive.  So since there is not crap on the drive in general, I am
accounting for stupid programmer errors in the past, I call your bluff.

If you issue the command register first the the rest of the register will
execute on the next command register.  This is doing thing bass-ackwards.

void go_take_a_dump (float load)
{
  if (pull_down_pants()  purge_bowles()  wipe_anus()  pull_up_pants())
 flush(load);
  else
 wear(load);
}

You do this out of order and this is more descriptive.

Now if you are complaining about logical operators in the test and how
these get optimized then you point is noted.

Since I was an astronomer and +/- one AU (93,000,000 miles) is critical,
feel free to check the rules for order.  Then you can call my bluff.

Cheers,

Andre Hedrick
The Linux ATA/IDE guy

FYI, I have a new baby-girl and thus the diaper-humor!

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Bug in ide-pci.c

2000-10-06 Thread Tom Leete

Sean Estabrooks wrote:
 
 Hi Andre,
 
   The if() logic must then rely on implementation specific compiler
 details and not have any optimizations which break this code.   While it may
 "WORK" it isn't particularly reliable code.
 
   Sean

Nope, the logical ops are sequence points required by
standard to lazy evaluate left to right. For ||, that means
they operate in order till they find a true.

Tom
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Bug in ide-pci.c

2000-10-06 Thread Sean Estabrooks

Andre,
(Thanks for calling my bluff)  (I was wrong).  

  Sean


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/