Re: [PATCH 1/2] NILFS2: add omitted comments for structures in nilfs2_fs.h

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,
On Mon, 16 Jul 2012 23:51:45 +0400, Vyacheslav Dubeyko wrote:
> Hi,
> 
> This patch adds omitted comments for structures in nilfs2_fs.h.
> 
> With the best regards,
> Vyacheslav Dubeyko.

Very helpful patch, thank you.

The patch seems mostly ok, but I have a few comments.  Please consider
reflecting the comments in-line below.

Regards,
Ryusuke Konishi

> --
> From: Vyacheslav Dubeyko <slava@xxxxxxxxxxx>
> Subject: [PATCH 1/2] NILFS2: add omitted comments for structures in nilfs2_fs.h
> 
> This patch adds omitted comments for structures in nilfs2_fs.h.
> 
> Signed-off-by: Vyacheslav Dubeyko <slava@xxxxxxxxxxx>
> ---
> diff --git a/include/linux/nilfs2_fs.h b/include/linux/nilfs2_fs.h
> index 89bd4a4..d7ef08e 100644
> --- a/include/linux/nilfs2_fs.h
> +++ b/include/linux/nilfs2_fs.h
> @@ -293,7 +293,7 @@ struct nilfs_dir_entry {
>  	__le64	inode;			/* Inode number */
>  	__le16	rec_len;		/* Directory entry length */
>  	__u8	name_len;		/* Name length */
> -	__u8	file_type;
> +	__u8	file_type;		/* Dir entry type (file, dir, special) */
>  	char	name[NILFS_NAME_LEN];	/* File name */
>  	char    pad;
>  };
> @@ -395,7 +395,7 @@ union nilfs_binfo {
>  };
>  
>  /**
> - * struct nilfs_segment_summary - segment summary
> + * struct nilfs_segment_summary - segment summary header
>   * @ss_datasum: checksum of data
>   * @ss_sumsum: checksum of segment summary
>   * @ss_magic: magic number
> @@ -683,9 +683,9 @@ struct nilfs_sufile_header {
>  
>  /**
>   * nilfs_suinfo - segment usage information
> - * @sui_lastmod:
> - * @sui_nblocks:
> - * @sui_flags:
> + * @sui_lastmod: timestamp of last modification
> + * @sui_nblocks: number of live blocks in segment

"live blocks" is confusing because it doesn't represent live/dead
state of blocks.  To be precise, it means "written" or "in-use"
blocks:

 * @sui_nblocks: number of written blocks in segment


> + * @sui_flags: segment usage flags
>   */
>  struct nilfs_suinfo {
>  	__u64 sui_lastmod;
> @@ -716,9 +716,10 @@ enum {
>  };
>  
>  /**
> - * struct nilfs_cpmode -
> - * @cc_cno:
> - * @cc_mode:
> + * struct nilfs_cpmode - change checkpoint mode structure
> + * @cm_cno: checkpoint number
> + * @cm_mode: mode of checkpoint
> + * @cm_pad: padding
>   */
>  struct nilfs_cpmode {
>  	__u64 cm_cno;
> @@ -728,11 +729,11 @@ struct nilfs_cpmode {
>  
>  /**
>   * struct nilfs_argv - argument vector
> - * @v_base:
> - * @v_nmembs:
> - * @v_size:
> - * @v_flags:
> - * @v_index:
> + * @v_base: pointer on data array from userspace
> + * @v_nmembs: number of members in data array
> + * @v_size: size of data array in bytes
> + * @v_flags: flags
> + * @v_index: index (index in array or checkpoint number)

@v_index is used to pass a start number of the target data items (e.g.
start checkpoint number or start segment number).  It's not like
@v_index gives an index of the data array.

 * @v_index: start number of target data items

>   */
>  struct nilfs_argv {
>  	__u64 v_base;
> @@ -743,9 +744,9 @@ struct nilfs_argv {
>  };
>  
>  /**
> - * struct nilfs_period -
> - * @p_start:
> - * @p_end:
> + * struct nilfs_period - period of checkpoint numbers
> + * @p_start: start checkpoint number
> + * @p_end: end checkpoint number

We often confuse these two numbers.  Start checkpoint numbers are
inclusive in nilfs, however end checkpoint numbers are not.

The following notes would be helpful.

 * @p_start: start checkpoint number (inclusive)
 * @p_end: end checkpoint number (exclusive)


>   */
>  struct nilfs_period {
>  	__u64 p_start;
> @@ -753,7 +754,7 @@ struct nilfs_period {
>  };
>  
>  /**
> - * struct nilfs_cpstat -
> + * struct nilfs_cpstat - checkpoint statistics
>   * @cs_cno: checkpoint number
>   * @cs_ncps: number of checkpoints
>   * @cs_nsss: number of snapshots
> @@ -765,7 +766,7 @@ struct nilfs_cpstat {
>  };
>  
>  /**
> - * struct nilfs_sustat -
> + * struct nilfs_sustat - segment usage statistics
>   * @ss_nsegs: number of segments
>   * @ss_ncleansegs: number of clean segments
>   * @ss_ndirtysegs: number of dirty segments
> @@ -784,10 +785,10 @@ struct nilfs_sustat {
>  
>  /**
>   * struct nilfs_vinfo - virtual block number information
> - * @vi_vblocknr:
> - * @vi_start:
> - * @vi_end:
> - * @vi_blocknr:
> + * @vi_vblocknr: virtual block number
> + * @vi_start: start checkpoint number
> + * @vi_end: end checkpoint number

ditto.

> + * @vi_blocknr: disk block number
>   */
>  struct nilfs_vinfo {
>  	__u64 vi_vblocknr;
> @@ -797,7 +798,15 @@ struct nilfs_vinfo {
>  };
>  
>  /**
> - * struct nilfs_vdesc -
> + * struct nilfs_vdesc - descriptor of virtual block number
> + * @vd_ino: inode number
> + * @vd_cno: checkpoint number
> + * @vd_vblocknr: virtual block number
> + * @vd_period: period of checkpoint numbers
> + * @vd_blocknr: disk block number
> + * @vd_offset: logical block offset inside a file
> + * @vd_flags: flags (data or node block)
> + * @vd_pad: padding
>   */
>  struct nilfs_vdesc {
>  	__u64 vd_ino;
> @@ -811,7 +820,13 @@ struct nilfs_vdesc {
>  };
>  
>  /**
> - * struct nilfs_bdesc -
> + * struct nilfs_bdesc - descriptor of disk block number
> + * @bd_ino: inode number
> + * @bd_oblocknr: disk block address (for skipping dead blocks)
> + * @bd_blocknr: disk block address
> + * @bd_offset: logical block offset inside a file
> + * @bd_level: level in the b-tree organization
> + * @bd_pad: padding
>   */
>  struct nilfs_bdesc {
>  	__u64 bd_ino;
> --
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Filesystem Development]     [Linux BTRFS]     [Linux CIFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux SCSI]

  Powered by Linux