Bartlomiej Zolnierkiewicz wrote:
On 8/3/05, Mark Bellon <[email protected]> wrote:
Bartlomiej Zolnierkiewicz wrote:
Hi,
The topic was discussed to death on linux-kernel.
Mark, you need to fix your applications and stop using /proc/ide/hd*/geometry
or/and HDIO_GET_GEO ioctl (which BTW your patch also affects).
Fixing the applications I can understand but the patch still seems
necessary, to me, so the
HDIO_GET_GEO returns "rational" values.
I tested HDIO_GET_GEO and it returns the same broken values as go into
/proc (no surprises) without my patch.
Please explain.
Let me explain more below. Thanks for the response as now I can
understand better the issue (which was what I was fishing for).
If a drive is in LBA mode (28 or 48 bit) the existing code doesn't
always "fix up" the geometry properly for some value returns. It only
tries with 48 bit mode and it fails there for some values. My patch
forces a complete geometry and appears (to me) to preserve the side
efefcts of the existing code.
Am I missing something?
diff -Naur linux-2.6.13-rc3-git9-orig/drivers/ide/ide-disk.c
linux-2.6.13-rc3-git9/drivers/ide/ide-disk.c
--- linux-2.6.13-rc3-git9-orig/drivers/ide/ide-disk.c 2005-08-01
13:48:21.000000000 -0700
+++ linux-2.6.13-rc3-git9/drivers/ide/ide-disk.c 2005-08-02
12:14:43.000000000 -0700
@@ -884,10 +884,17 @@
ide_add_setting(drive, "max_failures", SETTING_RW, -1, -1, TYPE_INT, 0, 65535, 1, 1, &drive->max_failures, NULL);
}
+static uint32_t do_div64_32 (__u64 n, uint32_t d)
+{
+ do_div(n, d);
+
+ return n;
+}
+
static void idedisk_setup (ide_drive_t *drive)
{
struct hd_driveid *id = drive->id;
- unsigned long long capacity;
+ __u64 capacity;
int barrier;
idedisk_add_settings(drive);
@@ -949,27 +956,32 @@
*/
capacity = idedisk_capacity (drive);
if (!drive->forced_geom) {
+ uint32_t cylsz, cyl;
- if (idedisk_supports_lba48(drive->id)) {
- /* compatibility */
- drive->bios_sect = 63;
- drive->bios_head = 255;
+ /*
+ * In LBA mode the geometry isn't used to talk to the drive
+ * so always create a "rational" geometry from the capacity.
+ */
+ if (drive->select.b.lba) {
why are you forcing drive->bios_sect and drive->bios_head values for LBA28 drives?
Any LBA 28 drive can return fixed geometry (regardless of size) just as
well as an LBA 48 drive can. I've got drives all over the place that do
just that. In any event there are machines which do not have have an
BIOS, like a PPC target, and the BIOS values can be "fixed up".
The ioctl returns the BIOS values and these are totally wrong on some
targets. Comments?
+ drive->bios_sect = drive->sect = 63;
+ drive->bios_head = drive->head = 255;
changing drive->sect and drive->head is just wrong,
these are values reported by the device - do not touch them
If drive->cyl, drive-head amd drive->sect are to be left alone in all
cases I have no problem.
+
+ cylsz = drive->bios_sect * drive->bios_head;
+ cyl = do_div64_32(capacity, cylsz);
division by zero now is possible (yes, 0/0/0 is possible)
Then the original code is prefered.
+
+ /* "fake out" works up to ~500 GB */
+ cyl = (cyl > 65535) ? 65535 : cyl;
+ drive->bios_cyl = drive->cyl = cyl;
drive->cyl shouldn't be changed
Same comment as above. Only change BIOS values.
}
if (drive->bios_sect && drive->bios_head) {
- unsigned int cap0 = capacity; /* truncate to 32 bits */
- unsigned int cylsz, cyl;
+ cylsz = drive->bios_sect * drive->bios_head;
+ cyl = do_div64_32(capacity, cylsz);
- if (cap0 != capacity)
- drive->bios_cyl = 65535;
cap0 != capacity => set the max possible drive->bios_cyl,
no need for adjusting it
- else {
cap0 == capacity => no need for do_div(), only lower 32-bits are used,
adjust drive->bios_cyl
- cylsz = drive->bios_sect * drive->bios_head;
- cyl = cap0 / cylsz;
- if (cyl > 65535)
- cyl = 65535;
- if (cyl > drive->bios_cyl)
- drive->bios_cyl = cyl;
- }
+ if (cyl > 65535)
+ cyl = 65535;
+ if (cyl > drive->bios_cyl)
+ drive->bios_cyl = cyl;
}
}
printk(KERN_INFO "%s: %llu sectors (%llu MB)",
I understand clearly now...
mark
mark
Bartlomiej
On 8/3/05, Andre Hedrick <[email protected]> wrote:
Did you read ATA-1 through ATA-7 to understand all the variations?
On Tue, 2 Aug 2005, Mark Bellon wrote:
The ATA specification tells large disk drives to return C/H/S data of
16383/16/63 regardless of their actual size (other variations on this
return include 15 heads and/or 4092 cylinders). Unfortunately these CHS
data confuse the existing IDE code and cause it to report invalid
geometries in /proc when the disk runs in LBA mode.
The invalid geometries can cause failures in the partitioning tools;
partitioning may be impossible or illogical size limitations occur. This
also leads to various forms of human confusion.
I attach a patch that fixes this problem while strongly attempting to
not break any existing side effects and await any comments.
mark
Signed-off-by: Mark Bellon <[email protected]>
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
| |
 |