Re: sysctl diskinit tailq foreach

[Alexander Bluhm]

On Tue, Dec 21, 2021 at 04:03:22PM +0100, Alexander Bluhm wrote:
> I would like to use TAILQ_FOREACH to traverse the disk list.
> Code is easier to read.

Merged to -current.

ok?

bluhm

Index: kern/kern_sysctl.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/kern/kern_sysctl.c,v
retrieving revision 1.397
diff -u -p -r1.397 kern_sysctl.c
--- kern/kern_sysctl.c  22 Dec 2021 22:20:13 -  1.397
+++ kern/kern_sysctl.c  22 Dec 2021 23:51:03 -
@@ -121,7 +121,6 @@
 extern struct forkstat forkstat;
 extern struct nchstats nchstats;
 extern int nselcoll, fscale;
-extern struct disklist_head disklist;
 extern fixpt_t ccpu;
 extern long numvnodes;
 extern int allowdt;
@@ -2132,12 +2131,12 @@ sysctl_diskinit(int update, struct proc 
struct diskstats *sdk;
struct disk *dk;
const char *duid;
-   int i, changed = 0;
+   int error, changed = 0;
 
KERNEL_ASSERT_LOCKED();
 
-   if ((i = rw_enter(_disklock, RW_WRITE|RW_INTR)) != 0)
-   return i;
+   if ((error = rw_enter(_disklock, RW_WRITE|RW_INTR)) != 0)
+   return error;
 
/* Run in a loop, disks may change while malloc sleeps. */
while (disk_change) {
@@ -2145,8 +2144,8 @@ sysctl_diskinit(int update, struct proc 
 
disk_change = 0;
 
-   for (dk = TAILQ_FIRST(), tlen = 0; dk;
-   dk = TAILQ_NEXT(dk, dk_link)) {
+   tlen = 0;
+   TAILQ_FOREACH(dk, , dk_link) {
if (dk->dk_name)
tlen += strlen(dk->dk_name);
tlen += 18; /* label uid + separators */
@@ -2173,8 +2172,9 @@ sysctl_diskinit(int update, struct proc 
if (changed) {
int l;
 
-   for (dk = TAILQ_FIRST(), i = 0, l = 0; dk;
-   dk = TAILQ_NEXT(dk, dk_link), i++) {
+   l = 0;
+   sdk = diskstats;
+   TAILQ_FOREACH(dk, , dk_link) {
duid = NULL;
if (dk->dk_label && !duid_iszero(dk->dk_label->d_uid))
duid = duid_format(dk->dk_label->d_uid);
@@ -2182,7 +2182,6 @@ sysctl_diskinit(int update, struct proc 
dk->dk_name ? dk->dk_name : "",
duid ? duid : "");
l += strlen(disknames + l);
-   sdk = diskstats + i;
strlcpy(sdk->ds_name, dk->dk_name,
sizeof(sdk->ds_name));
mtx_enter(>dk_mtx);
@@ -2196,6 +2195,7 @@ sysctl_diskinit(int update, struct proc 
sdk->ds_timestamp = dk->dk_timestamp;
sdk->ds_time = dk->dk_time;
mtx_leave(>dk_mtx);
+   sdk++;
}
 
/* Eliminate trailing comma */
@@ -2203,9 +2203,8 @@ sysctl_diskinit(int update, struct proc 
disknames[l - 1] = '\0';
} else if (update) {
/* Just update, number of drives hasn't changed */
-   for (dk = TAILQ_FIRST(), i = 0; dk;
-   dk = TAILQ_NEXT(dk, dk_link), i++) {
-   sdk = diskstats + i;
+   sdk = diskstats;
+   TAILQ_FOREACH(dk, , dk_link) {
strlcpy(sdk->ds_name, dk->dk_name,
sizeof(sdk->ds_name));
mtx_enter(>dk_mtx);
@@ -2219,6 +2218,7 @@ sysctl_diskinit(int update, struct proc 
sdk->ds_timestamp = dk->dk_timestamp;
sdk->ds_time = dk->dk_time;
mtx_leave(>dk_mtx);
+   sdk++;
}
}
rw_exit_write(_disklock);

sysctl diskinit tailq foreach

[Alexander Bluhm]

Hi,

I would like to use TAILQ_FOREACH to traverse the disk list.
Code is easier to read.

ok?

bluhm

Index: kern/kern_sysctl.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/kern/kern_sysctl.c,v
retrieving revision 1.396
diff -u -p -r1.396 kern_sysctl.c
--- kern/kern_sysctl.c  30 Oct 2021 23:24:48 -  1.396
+++ kern/kern_sysctl.c  21 Dec 2021 14:23:02 -
@@ -121,7 +121,6 @@
 extern struct forkstat forkstat;
 extern struct nchstats nchstats;
 extern int nselcoll, fscale;
-extern struct disklist_head disklist;
 extern fixpt_t ccpu;
 extern long numvnodes;
 extern int allowdt;
@@ -2132,14 +2131,14 @@ sysctl_diskinit(int update, struct proc 
struct diskstats *sdk;
struct disk *dk;
const char *duid;
-   int i, tlen, l;
+   int error, tlen, l;
 
-   if ((i = rw_enter(_disklock, RW_WRITE|RW_INTR)) != 0)
-   return i;
+   if ((error = rw_enter(_disklock, RW_WRITE|RW_INTR)) != 0)
+   return error;
 
if (disk_change) {
-   for (dk = TAILQ_FIRST(), tlen = 0; dk;
-   dk = TAILQ_NEXT(dk, dk_link)) {
+   tlen = 0;
+   TAILQ_FOREACH(dk, , dk_link) {
if (dk->dk_name)
tlen += strlen(dk->dk_name);
tlen += 18; /* label uid + separators */
@@ -2159,8 +2158,9 @@ sysctl_diskinit(int update, struct proc 
disknameslen = tlen;
disknames[0] = '\0';
 
-   for (dk = TAILQ_FIRST(), i = 0, l = 0; dk;
-   dk = TAILQ_NEXT(dk, dk_link), i++) {
+   l = 0;
+   sdk = diskstats;
+   TAILQ_FOREACH(dk, , dk_link) {
duid = NULL;
if (dk->dk_label && !duid_iszero(dk->dk_label->d_uid))
duid = duid_format(dk->dk_label->d_uid);
@@ -2168,7 +2168,6 @@ sysctl_diskinit(int update, struct proc 
dk->dk_name ? dk->dk_name : "",
duid ? duid : "");
l += strlen(disknames + l);
-   sdk = diskstats + i;
strlcpy(sdk->ds_name, dk->dk_name,
sizeof(sdk->ds_name));
mtx_enter(>dk_mtx);
@@ -2182,6 +2181,7 @@ sysctl_diskinit(int update, struct proc 
sdk->ds_timestamp = dk->dk_timestamp;
sdk->ds_time = dk->dk_time;
mtx_leave(>dk_mtx);
+   sdk++;
}
 
/* Eliminate trailing comma */
@@ -2190,9 +2190,8 @@ sysctl_diskinit(int update, struct proc 
disk_change = 0;
} else if (update) {
/* Just update, number of drives hasn't changed */
-   for (dk = TAILQ_FIRST(), i = 0; dk;
-   dk = TAILQ_NEXT(dk, dk_link), i++) {
-   sdk = diskstats + i;
+   sdk = diskstats;
+   TAILQ_FOREACH(dk, , dk_link) {
strlcpy(sdk->ds_name, dk->dk_name,
sizeof(sdk->ds_name));
mtx_enter(>dk_mtx);
@@ -2206,6 +2205,7 @@ sysctl_diskinit(int update, struct proc 
sdk->ds_timestamp = dk->dk_timestamp;
sdk->ds_time = dk->dk_time;
mtx_leave(>dk_mtx);
+   sdk++;
}
}
rw_exit_write(_disklock);