lkml.org 
[lkml]   [2010]   [Mar]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [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.


    \
     
     \ /
      Last update: 2010-03-01 23:17    [W:4.186 / U:0.164 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site