On 2021-01-05 18:27, Greg KH wrote:
Oops, forgot the big problem that I noticed:
On Mon, Jan 04, 2021 at 07:15:51PM -0800, Can Guo wrote:
+static ssize_t monitor_show(struct device *dev, struct
device_attribute *attr,
+ char *buf)
+{
+ struct ufs_hba *hba = dev_get_drvdata(dev);
+ struct ufs_qcom_host *host = ufshcd_get_variant(hba);
+ struct ufs_qcom_perf_monitor *mon = &host->monitor;
+ unsigned long nr_sec_rd, nr_sec_wr, busy_us_rd, busy_us_wr;
+ unsigned long lat_max_rd, lat_min_rd, lat_sum_rd, lat_avg_rd,
nr_req_rd;
+ unsigned long lat_max_wr, lat_min_wr, lat_sum_wr, lat_avg_wr,
nr_req_wr;
+ bool is_enabled;
+
+ /*
+ * Don't lock the host lock since user needs to cat the entry very
+ * frequently during performance test, otherwise it may impact the
+ * performance.
+ */
+ is_enabled = mon->enabled;
+ if (!is_enabled)
+ goto print_usage;
+
+ nr_sec_rd = mon->nr_sec_rw[READ];
+ nr_sec_wr = mon->nr_sec_rw[WRITE];
+ busy_us_rd = ktime_to_us(mon->total_busy[READ]);
+ busy_us_wr = ktime_to_us(mon->total_busy[WRITE]);
+
+ nr_req_rd = mon->nr_req[READ];
+ lat_max_rd = ktime_to_us(mon->lat_max[READ]);
+ lat_min_rd = ktime_to_us(mon->lat_min[READ]);
+ lat_sum_rd = ktime_to_us(mon->lat_sum[READ]);
+ lat_avg_rd = lat_sum_rd / nr_req_rd;
+
+ nr_req_wr = mon->nr_req[WRITE];
+ lat_max_wr = ktime_to_us(mon->lat_max[WRITE]);
+ lat_min_wr = ktime_to_us(mon->lat_min[WRITE]);
+ lat_sum_wr = ktime_to_us(mon->lat_sum[WRITE]);
+ lat_avg_wr = lat_sum_wr / nr_req_wr;
+
+ return scnprintf(buf, PAGE_SIZE, "Read %lu %s %lu us, %lu %s max %lu
| min %lu | avg %lu | sum %lu\nWrite %lu %s %lu us, %lu %s max %lu |
min %lu | avg %lu | sum %lu\n",
+ nr_sec_rd, "sectors (in 512 bytes) in ", busy_us_rd,
+ nr_req_rd, "read reqs completed, latencies in us: ",
+ lat_max_rd, lat_min_rd, lat_avg_rd, lat_sum_rd,
+ nr_sec_wr, "sectors (in 512 bytes) in ", busy_us_wr,
+ nr_req_wr, "write reqs completed, latencies in us: ",
+ lat_max_wr, lat_min_wr, lat_avg_wr, lat_sum_wr);
sysfs is one-value-per-file, not
throw-everything-in-one-file-and-hope-userspace-can-parse-it.
This is not acceptable at all. Why not just use debugfs for stats like
this?
Also, use sysfs_emit() for any new sysfs files please.
thanks,
greg k-h
Hi Greg,
Thanks for the comments, I will rework the change to make it right.
Regards,
Can Guo.
