Illumos #3742

3742 zfs comments need cleaner, more consistent style
Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Reviewed by: George Wilson <george.wilson@delphix.com>
Reviewed by: Eric Schrock <eric.schrock@delphix.com>
Approved by: Christopher Siden <christopher.siden@delphix.com>

References:
  https://www.illumos.org/issues/3742
  illumos/illumos-gate@f717074149

Ported-by: Richard Yao <ryao@gentoo.org>
Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Issue #1775

Porting notes:

1. The change to zfs_vfsops.c was dropped because it involves
   zfs_mount_label_policy, which does not exist in the Linux port.

include/sys/ddt.h

@@ -63,16 +63,15 @@ enum ddt_class {
  */
 typedef struct ddt_key {
 	zio_cksum_t	ddk_cksum;	/* 256-bit block checksum */
-	uint64_t	ddk_prop;	/* LSIZE, PSIZE, compression */
+	/*
+	 * Encoded with logical & physical size, and compression, as follows:
+	 *   +-------+-------+-------+-------+-------+-------+-------+-------+
+	 *   |   0   |   0   |   0   | comp  |     PSIZE     |     LSIZE     |
+	 *   +-------+-------+-------+-------+-------+-------+-------+-------+
+	 */
+	uint64_t	ddk_prop;
 } ddt_key_t;
 
-/*
- * ddk_prop layout:
- *
- *	+-------+-------+-------+-------+-------+-------+-------+-------+
- *	|   0	|   0	|   0	| comp	|     PSIZE	|     LSIZE	|
- *	+-------+-------+-------+-------+-------+-------+-------+-------+
- */
 #define	DDK_GET_LSIZE(ddk)	\
 	BF64_GET_SB((ddk)->ddk_prop, 0, 16, SPA_MINBLOCKSHIFT, 1)
 #define	DDK_SET_LSIZE(ddk, x)	\

include/sys/dnode.h

@@ -145,9 +145,8 @@ typedef struct dnode_phys {
 
 typedef struct dnode {
 	/*
-	 * dn_struct_rwlock protects the structure of the dnode,
-	 * including the number of levels of indirection (dn_nlevels),
-	 * dn_maxblkid, and dn_next_*
+	 * Protects the structure of the dnode, including the number of levels
+	 * of indirection (dn_nlevels), dn_maxblkid, and dn_next_*
 	 */
 	krwlock_t dn_struct_rwlock;
 

include/sys/dsl_pool.h

@@ -112,6 +112,7 @@ typedef struct dsl_pool {
 
 	/*
 	 * Protects administrative changes (properties, namespace)
+	 *
 	 * It is only held for write in syncing context.  Therefore
 	 * syncing context does not need to ever have it for read, since
 	 * nobody else could possibly have it for write.

include/sys/sa_impl.h

@@ -150,6 +150,7 @@ struct sa_os {
 
 /*
  * header for all bonus and spill buffers.
+ *
  * The header has a fixed portion with a variable number
  * of "lengths" depending on the number of variable sized
  * attribues which are determined by the "layout number"
@@ -158,29 +159,27 @@ struct sa_os {
 #define	SA_MAGIC	0x2F505A  /* ZFS SA */
 typedef struct sa_hdr_phys {
 	uint32_t sa_magic;
-	uint16_t sa_layout_info;  /* Encoded with hdrsize and layout number */
+	/*
+	 * Encoded with hdrsize and layout number as follows:
+	 * 16      10       0
+	 * +--------+-------+
+	 * | hdrsz  |layout |
+	 * +--------+-------+
+	 *
+	 * Bits 0-10 are the layout number
+	 * Bits 11-16 are the size of the header.
+	 * The hdrsize is the number * 8
+	 *
+	 * For example.
+	 * hdrsz of 1 ==> 8 byte header
+	 *          2 ==> 16 byte header
+	 *
+	 */
+	uint16_t sa_layout_info;
 	uint16_t sa_lengths[1];	/* optional sizes for variable length attrs */
 	/* ... Data follows the lengths.  */
 } sa_hdr_phys_t;
 
-/*
- * sa_hdr_phys -> sa_layout_info
- *
- * 16      10       0
- * +--------+-------+
- * | hdrsz  |layout |
- * +--------+-------+
- *
- * Bits 0-10 are the layout number
- * Bits 11-16 are the size of the header.
- * The hdrsize is the number * 8
- *
- * For example.
- * hdrsz of 1 ==> 8 byte header
- *          2 ==> 16 byte header
- *
- */
-
 #define	SA_HDR_LAYOUT_NUM(hdr) BF32_GET(hdr->sa_layout_info, 0, 10)
 #define	SA_HDR_SIZE(hdr) BF32_GET_SB(hdr->sa_layout_info, 10, 6, 3, 0)
 #define	SA_HDR_LAYOUT_INFO_ENCODE(x, num, size) \

include/sys/spa_impl.h

@@ -239,7 +239,7 @@ struct spa {
 	spa_stats_t	spa_stats;		/* assorted spa statistics */
 
 	/*
-	 * spa_refcnt & spa_config_lock must be the last elements
+	 * spa_refcount & spa_config_lock must be the last elements
 	 * because refcount_t changes size based on compilation options.
 	 * In order for the MDB module to function correctly, the other
 	 * fields must remain in the same location.

include/sys/space_map.h

@@ -94,7 +94,6 @@ struct space_map_ops {
  *   63  62    60 59        50 49                               0
  *
  *
- *
  * non-debug entry
  *
  *    1               47                   1           15

include/sys/unique.h

@@ -26,8 +26,6 @@
 #ifndef	_SYS_UNIQUE_H
 #define	_SYS_UNIQUE_H
 
-
-
 #include <sys/zfs_context.h>
 
 #ifdef	__cplusplus
@@ -42,7 +40,7 @@ void unique_fini(void);
 
 /*
  * Return a new unique value (which will not be uniquified against until
- * it is unique_insert()-ed.
+ * it is unique_insert()-ed).
  */
 uint64_t unique_create(void);
 

include/sys/vdev_impl.h

@@ -254,12 +254,13 @@ typedef struct vdev_label {
 #define	VDD_METASLAB	0x01
 #define	VDD_DTL		0x02
 
+/* Offset of embedded boot loader region on each label */
+#define	VDEV_BOOT_OFFSET	(2 * sizeof (vdev_label_t))
 /*
- * Size and offset of embedded boot loader region on each label.
+ * Size of embedded boot loader region on each label.
  * The total size of the first two labels plus the boot area is 4MB.
  */
-#define	VDEV_BOOT_OFFSET	(2 * sizeof (vdev_label_t))
-#define	VDEV_BOOT_SIZE		(7ULL << 19)			/* 3.5M	*/
+#define	VDEV_BOOT_SIZE		(7ULL << 19)			/* 3.5M */
 
 /*
  * Size of label regions at the start and end of each leaf device.
@@ -326,8 +327,9 @@ extern uint64_t vdev_get_min_asize(vdev_t *vd);
 extern void vdev_set_min_asize(vdev_t *vd);
 
 /*
- * zdb uses this tunable, so it must be declared here to make lint happy.
+ * Global variables
  */
+/* zdb uses this tunable, so it must be declared here to make lint happy. */
 extern int zfs_vdev_cache_size;
 
 #ifdef	__cplusplus

include/sys/zap.h

@@ -86,18 +86,22 @@ extern "C" {
 #endif
 
 /*
- * The matchtype specifies which entry will be accessed.
- * MT_EXACT: only find an exact match (non-normalized)
- * MT_FIRST: find the "first" normalized (case and Unicode
- *     form) match; the designated "first" match will not change as long
- *     as the set of entries with this normalization doesn't change
- * MT_BEST: if there is an exact match, find that, otherwise find the
- *     first normalized match
+ * Specifies matching criteria for ZAP lookups.
  */
 typedef enum matchtype
 {
+	/* Only find an exact match (non-normalized) */
 	MT_EXACT,
+	/*
+	 * If there is an exact match, find that, otherwise find the
+	 * first normalized match.
+	 */
 	MT_BEST,
+	/*
+	 * Find the "first" normalized (case and Unicode form) match;
+	 * the designated "first" match will not change as long as the
+	 * set of entries with this normalization doesn't change.
+	 */
 	MT_FIRST
 } matchtype_t;
 
@@ -174,16 +178,21 @@ int zap_destroy(objset_t *ds, uint64_t zapobj, dmu_tx_t *tx);
  * call will fail and return EINVAL.
  *
  * If 'integer_size' is equal to or larger than the attribute's integer
- * size, the call will succeed and return 0.  * When converting to a
- * larger integer size, the integers will be treated as unsigned (ie. no
- * sign-extension will be performed).
+ * size, the call will succeed and return 0.
+ *
+ * When converting to a larger integer size, the integers will be treated as
+ * unsigned (ie. no sign-extension will be performed).
  *
  * 'num_integers' is the length (in integers) of 'buf'.
  *
  * If the attribute is longer than the buffer, as many integers as will
  * fit will be transferred to 'buf'.  If the entire attribute was not
  * transferred, the call will return EOVERFLOW.
- *
+ */
+int zap_lookup(objset_t *ds, uint64_t zapobj, const char *name,
+    uint64_t integer_size, uint64_t num_integers, void *buf);
+
+/*
  * If rn_len is nonzero, realname will be set to the name of the found
  * entry (which may be different from the requested name if matchtype is
  * not MT_EXACT).
@@ -191,8 +200,6 @@ int zap_destroy(objset_t *ds, uint64_t zapobj, dmu_tx_t *tx);
  * If normalization_conflictp is not NULL, it will be set if there is
  * another name with the same case/unicode normalized form.
  */
-int zap_lookup(objset_t *ds, uint64_t zapobj, const char *name,
-    uint64_t integer_size, uint64_t num_integers, void *buf);
 int zap_lookup_norm(objset_t *ds, uint64_t zapobj, const char *name,
     uint64_t integer_size, uint64_t num_integers, void *buf,
     matchtype_t mt, char *realname, int rn_len,

include/sys/zap_leaf.h

@@ -101,6 +101,7 @@ typedef enum zap_chunk_type {
  */
 typedef struct zap_leaf_phys {
 	struct zap_leaf_header {
+		/* Public to ZAP */
 		uint64_t lh_block_type;		/* ZBT_LEAF */
 		uint64_t lh_pad1;
 		uint64_t lh_prefix;		/* hash prefix of this leaf */
@@ -109,8 +110,7 @@ typedef struct zap_leaf_phys {
 		uint16_t lh_nentries;		/* number of entries */
 		uint16_t lh_prefix_len;		/* num bits used to id this */
 
-/* above is accessable to zap, below is zap_leaf private */
-
+		/* Private to zap_leaf */
 		uint16_t lh_freelist;		/* chunk head of free list */
 		uint8_t lh_flags;		/* ZLF_* flags */
 		uint8_t lh_pad2[11];
@@ -161,13 +161,13 @@ typedef struct zap_leaf {
 
 
 typedef struct zap_entry_handle {
-	/* below is set by zap_leaf.c and is public to zap.c */
+	/* Set by zap_leaf and public to ZAP */
 	uint64_t zeh_num_integers;
 	uint64_t zeh_hash;
 	uint32_t zeh_cd;
 	uint8_t zeh_integer_size;
 
-	/* below is private to zap_leaf.c */
+	/* Private to zap_leaf */
 	uint16_t zeh_fakechunk;
 	uint16_t *zeh_chunkp;
 	zap_leaf_t *zeh_leaf;
@@ -202,7 +202,7 @@ extern int zap_entry_read_name(struct zap *zap, const zap_entry_handle_t *zeh,
 /*
  * Replace the value of an existing entry.
  *
- * zap_entry_update may fail if it runs out of space (ENOSPC).
+ * May fail if it runs out of space (ENOSPC).
  */
 extern int zap_entry_update(zap_entry_handle_t *zeh,
     uint8_t integer_size, uint64_t num_integers, const void *buf);
@@ -221,10 +221,7 @@ extern int zap_entry_create(zap_leaf_t *l, struct zap_name *zn, uint32_t cd,
     uint8_t integer_size, uint64_t num_integers, const void *buf,
     zap_entry_handle_t *zeh);
 
-/*
- * Return true if there are additional entries with the same normalized
- * form.
- */
+/* Determine whether there is another entry with the same normalized form. */
 extern boolean_t zap_entry_normalization_conflict(zap_entry_handle_t *zeh,
     struct zap_name *zn, const char *name, struct zap *zap);
 

include/sys/zfs_acl.h

@@ -47,7 +47,8 @@ struct znode_phys;
 #define	ZFS_ACL_VERSION		ZFS_ACL_VERSION_FUID
 
 /*
- * ZFS ACLs are store in various forms.
+ * ZFS ACLs (Access Control Lists) are stored in various forms.
+ *
  * Files created with ACL version ZFS_ACL_VERSION_INITIAL
  * will all be created with fixed length ACEs of type
  * zfs_oldace_t.
@@ -137,8 +138,8 @@ typedef struct acl_ops {
 	size_t		(*ace_size)(void *acep); /* how big is this ace */
 	size_t		(*ace_abstract_size)(void); /* sizeof abstract entry */
 	int		(*ace_mask_off)(void); /* off of access mask in ace */
+	/* ptr to data if any */
 	int		(*ace_data)(void *acep, void **datap);
-			    /* ptr to data if any */
 } acl_ops_t;
 
 /*

include/sys/zfs_rlock.h

@@ -26,8 +26,6 @@
 #ifndef	_SYS_FS_ZFS_RLOCK_H
 #define	_SYS_FS_ZFS_RLOCK_H
 
-
-
 #ifdef	__cplusplus
 extern "C" {
 #endif
@@ -58,16 +56,14 @@ typedef struct rl {
 } rl_t;
 
 /*
- * Lock a range (offset, length) as either shared (READER)
- * or exclusive (WRITER or APPEND). APPEND is a special type that
- * is converted to WRITER that specified to lock from the start of the
- * end of file.  zfs_range_lock() returns the range lock structure.
+ * Lock a range (offset, length) as either shared (RL_READER)
+ * or exclusive (RL_WRITER or RL_APPEND).  RL_APPEND is a special type that
+ * is converted to RL_WRITER that specified to lock from the start of the
+ * end of file.  Returns the range lock structure.
  */
 rl_t *zfs_range_lock(znode_t *zp, uint64_t off, uint64_t len, rl_type_t type);
 
-/*
- * Unlock range and destroy range lock structure.
- */
+/* Unlock range and destroy range lock structure. */
 void zfs_range_unlock(rl_t *rl);
 
 /*
@@ -77,7 +73,8 @@ void zfs_range_unlock(rl_t *rl);
 void zfs_range_reduce(rl_t *rl, uint64_t off, uint64_t len);
 
 /*
- * AVL comparison function used to compare range locks
+ * AVL comparison function used to order range locks
+ * Locks are ordered on the start offset of the range.
  */
 int zfs_range_compare(const void *arg1, const void *arg2);
 

include/sys/zfs_znode.h

@@ -139,8 +139,9 @@ extern "C" {
 
 #define	ZFS_MAX_BLOCKSIZE	(SPA_MAXBLOCKSIZE)
 
-/* Path component length */
 /*
+ * Path component length
+ *
  * The generic fs code uses MAXNAMELEN to represent
  * what the largest component length is.  Unfortunately,
  * this length includes the terminating NULL.  ZFS needs
@@ -248,11 +249,7 @@ typedef struct znode {
 
 #define	S_ISDEV(mode)	(S_ISCHR(mode) || S_ISBLK(mode) || S_ISFIFO(mode))
 
-/*
- * ZFS_ENTER() is called on entry to each ZFS inode and vfs operation.
- * ZFS_EXIT() must be called before exitting the vop.
- * ZFS_VERIFY_ZP() verifies the znode is valid.
- */
+/* Called on entry to each ZFS vnode and vfs operation  */
 #define	ZFS_ENTER(zsb) \
 	{ \
 		rrw_enter_read(&(zsb)->z_teardown_lock, FTAG); \
@@ -262,12 +259,14 @@ typedef struct znode {
 		} \
 	}
 
+/* Must be called before exiting the vop */
 #define	ZFS_EXIT(zsb) \
 	{ \
 		rrw_exit(&(zsb)->z_teardown_lock, FTAG); \
 		tsd_exit(); \
 	}
 
+/* Verifies the znode is valid */
 #define	ZFS_VERIFY_ZP(zp) \
 	if ((zp)->z_sa_hdl == NULL) { \
 		ZFS_EXIT(ZTOZSB(zp)); \
@@ -289,15 +288,14 @@ typedef struct znode {
 #define	ZFS_OBJ_HOLD_OWNED(zsb, obj_num) \
 	mutex_owned(ZFS_OBJ_MUTEX((zsb), (obj_num)))
 
-/*
- * Macros to encode/decode ZFS stored time values from/to struct timespec
- */
+/* Encode ZFS stored time values from a struct timespec */
 #define	ZFS_TIME_ENCODE(tp, stmp)		\
 {						\
 	(stmp)[0] = (uint64_t)(tp)->tv_sec;	\
 	(stmp)[1] = (uint64_t)(tp)->tv_nsec;	\
 }
 
+/* Decode ZFS stored time values to a struct timespec */
 #define	ZFS_TIME_DECODE(tp, stmp)		\
 {						\
 	(tp)->tv_sec = (time_t)(stmp)[0];		\

include/sys/zil.h

@@ -242,6 +242,12 @@ typedef struct {
  * information needed for replaying the create.  If the
  * file doesn't have any actual ACEs then the lr_aclcnt
  * would be zero.
+ *
+ * After lr_acl_flags, there are a lr_acl_bytes number of variable sized ace's.
+ * If create is also setting xvattr's, then acl data follows xvattr.
+ * If ACE FUIDs are needed then they will follow the xvattr_t.  Following
+ * the FUIDs will be the domain table information.  The FUIDs for the owner
+ * and group will be in lr_create.  Name follows ACL data.
  */
 typedef struct {
 	lr_create_t	lr_create;	/* common create portion */
@@ -250,13 +256,6 @@ typedef struct {
 	uint64_t	lr_fuidcnt;	/* number of real fuids */
 	uint64_t	lr_acl_bytes;	/* number of bytes in ACL */
 	uint64_t	lr_acl_flags;	/* ACL flags */
-	/* lr_acl_bytes number of variable sized ace's follows */
-	/* if create is also setting xvattr's, then acl data follows xvattr */
-	/* if ACE FUIDs are needed then they will follow the xvattr_t */
-	/* Following the FUIDs will be the domain table information. */
-	/* The FUIDs for the owner and group will be in the lr_create */
-	/* portion of the record. */
-	/* name follows ACL data */
 } lr_acl_create_t;
 
 typedef struct {

include/sys/zio_compress.h

@@ -33,11 +33,10 @@
 extern "C" {
 #endif
 
-/*
- * Common signature for all zio compress/decompress functions.
- */
+/* Common signature for all zio compress functions. */
 typedef size_t zio_compress_func_t(void *src, void *dst,
     size_t s_len, size_t d_len, int);
+/* Common signature for all zio decompress functions. */
 typedef int zio_decompress_func_t(void *src, void *dst,
     size_t s_len, size_t d_len, int);