daddr_t is inconsistent and barely used

daddr_t is barely used in the kernel. 2.4.8.

fs/freevxfs/vxfs.h:   daddr_t                 vsi_oltext;     /* OLT extent */
fs/freevxfs/vxfs.h:   daddr_t                 vsi_oltsize;    /* OLT size */

fs/freevxfs/vxfs_bmap.c:static daddr_t
fs/freevxfs/vxfs_bmap.c: daddr_t pblock;
fs/freevxfs/vxfs_bmap.c:static daddr_t
fs/freevxfs/vxfs_bmap.c: daddr_t pblock = 0;
fs/freevxfs/vxfs_bmap.c:static daddr_t
fs/freevxfs/vxfs_bmap.c: daddr_t pblock = 0;
fs/freevxfs/vxfs_bmap.c:daddr_t
fs/freevxfs/vxfs_extern.h:extern daddr_t vxfs_bmap1(struct inode *, long);
fs/freevxfs/vxfs_olt.c:vxfs_oblock(daddr_t oblock, u_long bsize)

fs/freevxfs/vxfs_subr.c:      daddr_t                 pblock;
fs/freevxfs/vxfs_subr.c:      daddr_t                 pblock;
include/linux/genhd.h:        daddr_t s_start;                /* start sector no of partition */

include/linux/types.h:typedef __kernel_daddr_t daddr_t;
arch/parisc/hpux/sys_hpux.c: int32_t f_tfree; /* total free (daddr_t) */

The use of daddr_t in freevxfs may give different in core and disk layouts on different machines. Is that intended?.

Its definition is not consistent. Some 32 bit machines use int, some use long. Some 64 bit machines use int, some use long.

include/linux/types.h:typedef __kernel_daddr_t        daddr_t;
include/linux/types.h:        __kernel_daddr_t        f_tfree;
include/linux/mtio.h: __kernel_daddr_t mt_fileno;     /* number of current file on tape */
include/linux/mtio.h: __kernel_daddr_t mt_blkno;      /* current block number */
include/asm-i386/posix_types.h:typedef int            __kernel_daddr_t;
include/asm-mips/posix_types.h:typedef long           __kernel_daddr_t;
include/asm-alpha/posix_types.h:typedef int           __kernel_daddr_t;
include/asm-m68k/posix_types.h:typedef int            __kernel_daddr_t;

include/asm-sparc/posix_types.h:typedef long __kernel_daddr_t;
include/asm-ppc/posix_types.h:typedef int __kernel_daddr_t;
include/asm-sparc64/posix_types.h:typedef int __kernel_daddr_t;
include/asm-arm/posix_types.h:typedef int __kernel_daddr_t;

include/asm-sh/posix_types.h:typedef int              __kernel_daddr_t;
include/asm-ia64/posix_types.h:typedef int            __kernel_daddr_t;
include/asm-mips64/posix_types.h:typedef long         __kernel_daddr_t;

include/asm-s390/posix_types.h:typedef int __kernel_daddr_t;
include/asm-parisc/posix_types.h:typedef int __kernel_daddr_t;
include/asm-cris/posix_types.h:typedef int __kernel_daddr_t;
include/asm-s390x/posix_types.h:typedef int __kernel_daddr_t;

Do we still need daddr_t?

This question was raised when I saw patches for ia64 that replaced u32 with unsigned long because ia64 needs 64 bit. Shouldn't we be using a consistent type that holds kernel addresses as numbers? off_t and loff_t are not suitable. caddr_t is close but uses char *, sometimes you want just a number. What about defining kaddr_t?

No, it may not. Please double check.

I think so, in fact we really should use daddr_t for all incore disk addessing.

vaddr_t? That's consintant to virt_to_phys, virt_to_bus, etc.. and what SVR5 uses.

Christoph

Whip me.  Beat me.  Make me maintain AIX.

That is why I said "may". It seemed puzzling that freevxfs used vx_daddr_t for almost everything but daddr_t for a couple of fields. An inconsistency with no obvious reason.

vx_daddr_t is for disk structures, daddr_t for core.

...

This is asking for trouble. The disk structures aren't about to change. See include/linux/ext2_fs.h for a safe way to do the on-disk structure. For the in-core stuff, "unsigned long" is a perfectly fine data type -- and yes I know it gets wider with a 64-bit system.

To save you the trouble of looking up my example:

/*
* Structure of a blocks group descriptor
*/
struct ext2_group_desc
{

        __u32   bg_block_bitmap;                /* Blocks bitmap block */
        __u32   bg_inode_bitmap;                /* Inodes bitmap block */
        __u32   bg_inode_table;         /* Inodes table block */
        __u16   bg_free_blocks_count;   /* Free blocks count */
        __u16   bg_free_inodes_count;   /* Free inodes count */
        __u16   bg_used_dirs_count;     /* Directories count */
        __u16   bg_pad;
        __u32   bg_reserved[3];

};

Don't forget to add explicit padding as needed to give natural alignment.

Please take a look how vx_daddr_t is defined - just because I prefer descriptive names I am not stupid.

Christoph

Thanks for you help, I have never worked with structure layouts before..

Christoph

Of course it doesn't work. We've performed a software upgrade.