On Wed, Mar 15, 2023 at 06:08:19AM +0000, Seymour, Shane M wrote: > The following patch implements host state statistics via sysfs. The intent > is to allow user space to see the state changes and be able to report when > a host changes state. The files do not separate out the time spent into > each state but only into three: Why does userspace care about these things at all? What tool needs them and what can userspace do with the information? > > $ ll /sys/class/scsi_host/host0/stats > total 0 > -r--r--r--. 1 root root 4096 Mar 13 22:43 state_first_change_time > -r--r--r--. 1 root root 4096 Mar 13 22:43 state_last_change_time > -r--r--r--. 1 root root 4096 Mar 13 22:43 state_other_count > -r--r--r--. 1 root root 4096 Mar 13 22:43 state_other_ns > -r--r--r--. 1 root root 4096 Mar 13 22:43 state_recovery_count > -r--r--r--. 1 root root 4096 Mar 13 22:43 state_recovery_ns > -r--r--r--. 1 root root 4096 Mar 13 22:43 state_running_count > -r--r--r--. 1 root root 4096 Mar 13 22:43 state_running_ns > > They are running, recovery and other. The running state is SHOST_CREATED > and SHOST_RUNNING. The recovery state is SHOST_RECOVERY, > SHOST_CANCEL_RECOVERY, and SHOST_DEL_RECOVERY. Any other state gets > accounted for in other. > > The current state is not accounted for in the information read. Because > of that you must read: > > 1. The last_change_time for that host > 2. the current state of a host and the uptime > 3. each of the above *count and *ns files > 4. Re-read the last_change_time > 5. Compare the two last_change_time values read and if different try again. > 6. The total time read from the *ns files is subtracted from the uptime and > that time is then allocated to the current state time. > > The first change time is to determine when the host was created so programs > can determine if it was newly created or not. > > A (GPLv2) program called hostmond will be released in a few months that > will monitor these interfaces and report (local host only via syslog(3C)) > when hosts change state. We kind of need to see this before the kernel changes can be accepted for obvious reasons, what is preventing that from happening now? > +static ssize_t state_first_change_time_show(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + struct Scsi_Host *shost = class_to_shost(dev); > + > + return scnprintf(buf, PAGE_SIZE, "%lld", > + ktime_to_ns(shost->stats->state_first_change_time)); Please always use sysfs_emit() instead of the crazy scnprintf() for sysfs entries. > +struct scsi_host_stats { > + ktime_t state_running_ns; > + ktime_t state_recovery_ns; > + ktime_t state_other_ns; > + ktime_t state_first_change_time; > + ktime_t state_last_change_time; > + uint32_t state_running_count; > + uint32_t state_recovery_count; > + uint32_t state_other_count; u32 is a kernel type, not uint32_t please, but I don't know what the scsi layer is used to. thanks, greg k-h