Promise IDE patches

Promise IDE patches

[Jan Niehusmann]

Hi!

Two weeks ago, I tried two patches against 2.4.21 regarding LBA48
support. One limits the size of a drive to 137GB if LBA48 is not
available. Without this patch, severe data corruption is possible.

http://gondor.com/linux/patch-limit48-2.4.21

The other one is making LBA48 support work with pdc 20265 controllers. 

http://gondor.com/linux/patch-pdc-lba48-2.4.22

I think they should be candidates for inclusion in 2.4.23, as well as
a fix for hdparm -I (and other commands going directly to the drive) on
(some?) promise controllers:

http://marc.theaimsgroup.com/?l=linux-kernel&m=104250818527780&w=2

Jan

Re: Promise IDE patches

[Bartlomiej Zolnierkiewicz]

Thanks Jan.

Marcelo can you apply these patches?

On Wednesday 27 of August 2003 00:31, Jan Niehusmann wrote:
> Hi!
>
> Two weeks ago, I tried two patches against 2.4.21 regarding LBA48
> support. One limits the size of a drive to 137GB if LBA48 is not
> available. Without this patch, severe data corruption is possible.
>
> http://gondor.com/linux/patch-limit48-2.4.21
>
> The other one is making LBA48 support work with pdc 20265 controllers.
>
> http://gondor.com/linux/patch-pdc-lba48-2.4.22
>
> I think they should be candidates for inclusion in 2.4.23, as well as
> a fix for hdparm -I (and other commands going directly to the drive) on
> (some?) promise controllers:
>
> http://marc.theaimsgroup.com/?l=linux-kernel&m=104250818527780&w=2
>
> Jan

Re: Promise IDE patches

[Erik Andersen]

> Thanks Jan.
> 
> Marcelo can you apply these patches?
> 
> On Wednesday 27 of August 2003 00:31, Jan Niehusmann wrote:
> > Hi!
> >
> > Two weeks ago, I tried two patches against 2.4.21 regarding
> > LBA48
> > support. One limits the size of a drive to 137GB if LBA48 is
> > not
> > available. Without this patch, severe data corruption is
> > possible.
> >
> > http://gondor.com/linux/patch-limit48-2.4.21

My recent IDE patch also fixes this problem....

http://marc.theaimsgroup.com/?l=linux-kernel&m=106159060721871&w=2

 -Erik

--
Erik B. Andersen             http://codepoet-consulting.com/
--This message was written using 73% post-consumer electrons--

Re: Promise IDE patches

[Alan Cox]

On Maw, 2003-08-26 at 23:54, Bartlomiej Zolnierkiewicz wrote:
> Thanks Jan.
> 
> Marcelo can you apply these patches?

The last one looks wrong to me. At the least it needs to check
which pdc202xx chip first

Re: Promise IDE patches

[Alan Cox]

On Maw, 2003-08-26 at 23:54, Bartlomiej Zolnierkiewicz wrote:
> Thanks Jan.
> 
> Marcelo can you apply these patches?

The first one of these is wrong too. It'll stop some people from
being able to access existing file systems. This has to be fixed
properly (in both 2.4 and 2.6) to distinguish between 
"Can PIO LBA48" "Can DMA LBA48" and "no LBA48". The latter being
basically non existant.

If you fail to do this then existing PIO LBA48 setups will just die
on boot.

Re: Promise IDE patches

[Jan Niehusmann]

On Wed, Aug 27, 2003 at 03:59:52PM +0100, Alan Cox wrote:
> If you fail to do this then existing PIO LBA48 setups will just die
> on boot.

But 'die on boot' is much better than 'silently currupt data', don't you
think? 
Still, a proper fix would work in all cases... 

What do you think about a check in __ide_do_rw_disk? calling
lba_28_rw_disk or chs_rw_disk when the block address doesn't fit in
28bit is surely wrong and should return an error. 

This will lead to read or write errors if LBA48 doesn't work, but
it will prevent silent data corruption.

Jan

Re: Promise IDE patches

[Bartlomiej Zolnierkiewicz]

On Wednesday 27 of August 2003 16:59, Alan Cox wrote:
> On Maw, 2003-08-26 at 23:54, Bartlomiej Zolnierkiewicz wrote:
> > Thanks Jan.
> >
> > Marcelo can you apply these patches?
>
> The first one of these is wrong too. It'll stop some people from
> being able to access existing file systems. This has to be fixed
> properly (in both 2.4 and 2.6) to distinguish between
> "Can PIO LBA48" "Can DMA LBA48" and "no LBA48". The latter being
> basically non existant.
>
> If you fail to do this then existing PIO LBA48 setups will just die
> on boot.

Alan, its not true.  Please check with ide-disk.c.
If hwif->addressing is 1 then probe_lba_addressing() wont set
drive->addressing to 1 and therefore you wont get LBA48
(both PIO and DMA) because of drive->addressing check
insided __ide_do_rw_disk().

Its not complete fix but it doesnt break anything and stops fs corruption.

--bartlomiej

Re: Promise IDE patches

[Jan Niehusmann]

On Wed, Aug 27, 2003 at 05:12:27PM +0200, Jan Niehusmann wrote:
> What do you think about a check in __ide_do_rw_disk? calling
> lba_28_rw_disk or chs_rw_disk when the block address doesn't fit in
> 28bit is surely wrong and should return an error. 

Sorry - I didn't notice that this straight forward distinction is only
possible in the CONFIG_IDE_TASKFILE_IO case. 

Jan

Re: Promise IDE patches

[Bartlomiej Zolnierkiewicz]

On Wednesday 27 of August 2003 16:58, Alan Cox wrote:
> On Maw, 2003-08-26 at 23:54, Bartlomiej Zolnierkiewicz wrote:
> > Thanks Jan.
> >
> > Marcelo can you apply these patches?
>
> The last one looks wrong to me. At the least it needs to check
> which pdc202xx chip first

Yep, you are right.
[ I guess that PIO forced is 20246 specific, but we don't know. ]

--bartlomiej

Re: Promise IDE patches

[Alan Cox]

On Mer, 2003-08-27 at 16:12, Jan Niehusmann wrote:
> On Wed, Aug 27, 2003 at 03:59:52PM +0100, Alan Cox wrote:
> > If you fail to do this then existing PIO LBA48 setups will just die
> > on boot.
> 
> But 'die on boot' is much better than 'silently currupt data', don't you
> think? 
> Still, a proper fix would work in all cases... 

You'd prevent people from using valid working file systems in ways
that don't corrupt.

I think the logic we need is something like

	if drive_is_lba48 && controller_doesnt_do_lba48
		clip_capacity

as a variant of the current change. That covers the "no way" case. For
the others we need to check capacity requires lba48 && !dma_lba48
somewhere, probably ide_dma_check. That would make us drop out of DMA
for unsafe setups in a way that doesnt cause crashes or suprises and
still let everyone access the data properly.

Re: Promise IDE patches

[Jan Niehusmann]

[-- Attachment #1: Type: text/plain, Size: 1893 bytes --]

Hi Bartolomeij, hi Marcelo!

Two months ago, I had problems with a >137GB drive on a promise
controller. These where fixed by a few patches, and the computer runs
fine since then with a patched 2.4.22 kernel.

Unfortunately, as far as I can see, none of the patches got applied to
2.4.23-pre7. I know that Alan had a few objections against the patches,
and they should be addressed, but as things are now, plain 2.4.23-pre7
would lead to severe data corruption if I used it on my computer.

For reference, here is the complete mail where Bartolomeij asked Marcelo
to apply the patches:

On Wed, Aug 27, 2003 at 12:54:27AM +0200, Bartlomiej Zolnierkiewicz wrote:
> Marcelo can you apply these patches?
> 
> On Wednesday 27 of August 2003 00:31, Jan Niehusmann wrote:
> > Hi!
> >
> > Two weeks ago, I tried two patches against 2.4.21 regarding LBA48
> > support. One limits the size of a drive to 137GB if LBA48 is not
> > available. Without this patch, severe data corruption is possible.
> >
> > http://gondor.com/linux/patch-limit48-2.4.21
> >
> > The other one is making LBA48 support work with pdc 20265 controllers.
> >
> > http://gondor.com/linux/patch-pdc-lba48-2.4.22
> >
> > I think they should be candidates for inclusion in 2.4.23, as well as
> > a fix for hdparm -I (and other commands going directly to the drive) on
> > (some?) promise controllers:
> >
> > http://marc.theaimsgroup.com/?l=linux-kernel&m=104250818527780&w=2
> >
> > Jan

Alan mentioned problems with patches 1 and 3, but AFAIK nobody had
problems with patch 2. Consult the archives for details, it's all been
on linux-kernel.

Even though I could easily fix 2.4.23 for myself, I'd really like to
have the official kernel work without local patches. And I guess an
increasing number of people is using drives >137GB, so it can easily hit
other people as well.

Jan


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]