On Fri, 2009-11-20 at 14:57 +0100, Jan Damborsky wrote:
> Hi Alex,
>
> please see my comments below.
>
> BTW, thank you very much for addressing
> cstyle issues.
>
> Jan
>
>
> boot_archive_archive.py
> -----------------------
>
> 356 TI_ATTR_DC_RAMDISK_INODES_NUM: TI_DC_RAMDISK_INODES_DEFAULT,
>
> Ramdisk is created with 2048 bytes/inode, but looking at bug Report,
> it suggests to use optimized value 16384 instead. Why is default used ?
>
I would still leave this for boot_archive as it because free space
calculation is not done yet, but yes, we can use this number right now -
free space will be more.
> ti_api.h
> --------
>
> 102 #define TI_DC_RAMDISK_INODES_DEFAULT 2048
>
> Default is hardcoded - I might recommend not to define it at all and
> instead take advantage of the fact that newfs(1M) calculates default value
> if none is explicitly specified - please also see below.
>
>
> 296 /* uint_32_t - # of inodes to be created */
> 297 #define TI_ATTR_DC_RAMDISK_INODES_NUM "ti_dc_ramdisk_inodes_num"
>
> Since we are actually tuning number of bytes per inode, not number
> of inodes, I might recommend to change the comment and name of the
> attribute to reflect this - e.g.
>
> 296 /* uint_32_t - # of bytes per inode */
> 297 #define TI_ATTR_DC_RAMDISK_BYTES_PER_INODE
> "ti_dc_ramdisk_bytes_per_inode"
>
>
> ti_dcm.c
> --------
>
> TI_ATTR_DC_RAMDISK_INODES_NUM is mandatory attribute and TI fails to create
> ramdisk if it is not specified.
>
> Since it is optional parameter for newfs(1M), I might recommend to make
> it optional for TI as well and let newfs(1M) work out the default nbpi
> if this attribute is not provided - e.g. something like
>
> 158 if (nvlist_lookup_uint32(attrs, TI_ATTR_DC_RAMDISK_INODES_NUM,
> 159 &ramdisk_nbpi) != 0) {
> 160 ls_write_dbg_message(TIDC, LS_DBGLVL_ERR,
> 161 "Number of bytes per inode not provided\n");
> 162 return (TI_E_INVALID_RAMDISK_ATTR);
> 163 }
> ->
> 158 if (nvlist_lookup_uint32(attrs, TI_ATTR_DC_RAMDISK_INODES_NUM,
> 159 &ramdisk_nbpi) != 0) {
> 160 ls_write_dbg_message(TIDC, LS_DBGLVL_INFO,
> 161 "Number of bytes per inode not provided, newfs(1M)"
> 162 " will use the default value\n");
> 163 ramdisk_nbpi = 0;
> 164 }
>
> ...
>
> 211 (void) snprintf(cmd, sizeof (cmd), "/usr/sbin/newfs -m 0 -o space
> "
> 212 "-i %d %s 0</dev/null", ramdisk_nbpi, pseudod);
> ->
> 211 if (ramdisk_nbpi != 0)
> 212 (void) snprintf(cmd, sizeof (cmd), "/usr/sbin/newfs -m 0 -o
> space "
> 213 "-i %d %s 0</dev/null", ramdisk_nbpi, pseudod);
> 214 else
> 215 (void) snprintf(cmd, sizeof (cmd), "/usr/sbin/newfs -m 0 -o
> space "
> 216 "%s 0</dev/null", pseudod);
>
Agree. Thanks Jan!
--
::alhazred
