Messages in this thread | | | Date | Mon, 1 Mar 2010 14:13:35 -0800 | From | Andrew Morton <> | Subject | Re: [PATCH] msdos: add support for large disks |
| |
On Wed, 24 Feb 2010 19:17:47 -0800 "Daniel Taylor" <Daniel.Taylor@wdc.com> wrote:
> In order to use disks larger than 2TiB on Windows XP, it is necessary to use > 4096-byte logical sectors in an MBR. > > Although the kernel storage and functions called from msdos.c used > "sector_t" internally, msdos.c still used u32 variables, which results in > the ability to handle XP-compatible large disks. > > This patch changes the internal variables to "sector_t". >
Please Cc OGAWA Hirofumi <hirofumi@mail.parknet.co.jp> and myself on this work.
The patch was really badly wordwrapped. Please fix up your email client for next time.
> --- > diff --git a/fs/partitions/msdos.c b/fs/partitions/msdos.c > index 0028d2e..fa9c77d 100644 > --- a/fs/partitions/msdos.c > +++ b/fs/partitions/msdos.c > @@ -104,12 +104,12 @@ static int aix_magic_present(unsigned char *p, struct > block_device *bdev) > > static void > parse_extended(struct parsed_partitions *state, struct block_device *bdev, > - u32 first_sector, u32 first_size) > + sector_t first_sector, sector_t first_size) > { > struct partition *p; > Sector sect; > unsigned char *data; > - u32 this_sector, this_size; > + sector_t this_sector, this_size; > int sector_size = bdev_logical_block_size(bdev) / 512; > int loopct = 0; /* number of links followed > without finding a data partition */ > @@ -145,15 +145,16 @@ parse_extended(struct parsed_partitions *state, struct > block_device *bdev, > * First process the data partition(s) > */ > for (i=0; i<4; i++, p++) { > - u32 offs, size, next; > + sector_t offs, size, next; > if (!NR_SECTS(p) || is_extended_partition(p)) > continue; > > /* Check the 3rd and 4th entries - > these sometimes contain random garbage */ > - offs = START_SECT(p)*sector_size; > - size = NR_SECTS(p)*sector_size; > + offs = > (sector_t)(START_SECT(p))*(sector_t)(sector_size); > + size = > (sector_t)(NR_SECTS(p))*(sector_t)(sector_size);
The patch would be cleaner if START_SECT() and NR_SECTS() were returning the correct type. I suggest converting these to regular C functions, renaming them to start_sect() and nr_sects() (or something more descriptive if you like) and making them return a sector_t.
Making local variable `sector_size' have type sector_t would eliminate the other typecast and is a logical enough thing to do, I think.
> next = this_sector + offs; > + > if (i >= 2) { > if (offs + size > this_size) > continue; > > ... > > @@ -263,14 +264,14 @@ parse_bsd(struct parsed_partitions *state, struct > block_device *bdev, > if (le16_to_cpu(l->d_npartitions) < max_partitions) > max_partitions = le16_to_cpu(l->d_npartitions); > for (p = l->d_partitions; p - l->d_partitions < max_partitions; p++) > { > - u32 bsd_start, bsd_size; > + sector_t bsd_start, bsd_size; > > if (state->next == state->limit) > break; > if (p->p_fstype == BSD_FS_UNUSED) > continue; > - bsd_start = le32_to_cpu(p->p_offset); > - bsd_size = le32_to_cpu(p->p_size); > + bsd_start = (sector_t)(le32_to_cpu(p->p_offset)); > + bsd_size = (sector_t)(le32_to_cpu(p->p_size));
I don't see why these two casts are needed? The compiler will take care of assigning a u32 to a sector_t. Perhaps this applies to other places in the patch.
> if (offset == bsd_start && size == bsd_size) > /* full parent partition, we have it already */ > continue; > > ... > > @@ -483,8 +484,8 @@ int msdos_partition(struct parsed_partitions *state, > struct block_device *bdev) > > state->next = 5; > for (slot = 1 ; slot <= 4 ; slot++, p++) { > - u32 start = START_SECT(p)*sector_size; > - u32 size = NR_SECTS(p)*sector_size; > + sector_t start = > (sector_t)(START_SECT(p))*(sector_t)(sector_size); > + sector_t size = > (sector_t)(NR_SECTS(p))*(sector_t)(sector_size); > if (!size) > continue; > if (is_extended_partition(p)) { > @@ -521,8 +522,8 @@ int msdos_partition(struct parsed_partitions *state, > struct block_device *bdev) > > if (!subtypes[n].parse) > continue; > - subtypes[n].parse(state, bdev, START_SECT(p)*sector_size, > - NR_SECTS(p)*sector_size, > slot); > + subtypes[n].parse(state, bdev, > (sector_t)(START_SECT(p))*(sector_t)(sector_size), > + > (sector_t)(NR_SECTS(p))*(sector_t)(sector_size), slot); > } > put_dev_sector(sect); > return 1;
Again, we can avoid some/all of this casting by more appropriate selection of types.
| |