Re: [PATCH] eventfs: Create dentries and inodes at dir open

2024-01-16 Thread Steven Rostedt 
On Tue, 16 Jan 2024 10:53:36 -0800
Linus Torvalds  wrote:

> Let's at least *try* to move towards a better and simpler world, in other
> words.

OK, but I did just finish the below. I'll save it for another time if the
single inode becomes an issue.

I cropped it to just 31 bits, so I'm not sure how much that would still
expose kernel addresses.

But as you prefer a single inode number, I'll submit that instead, and
store this as one of my many git branches that I hoard.

This patch would give me:

~# ls -i /sys/kernel/tracing/events
2024739183 9p   801480813 iocost281269778 qdisc
 401505785 alarmtimer   641794539 iomap1206040657 ras
1930290274 avc 1239754069 iommu 275816503 raw_syscalls
1108006611 block138722947 io_uring 1758073266 rcu
1528091656 bpf_test_run 694421747 ipi  1384703124 regmap
 557364529 bpf_trace  2765888 irq30507018 regulator
1351362737 cfg80211 369905250 irq_matrix   2078862821 resctrl
 886445085 cgroup  1115008967 irq_vectors   324090967 rpm
 796209754 clk 1448778784 jbd2 1141318882 rseq
 478739129 compaction99410253 kmem 1274783780 rtc
1714397712 cpuhp   2001779594 ksm  1409072807 sched
 720701943 csd   51728677 kyber 347239934 scsi
1824588347 dev 1507974437 libata   1768671172 sd
1640041299 devfreq 1421146927 lock 1167562598 signal
 121399632 dma_fence956825721 mac802114116590 skb
 975908383 drm  738868061 maple_tree   1435501164 smbus
1227060804 e1000e_trace 969175760 mce  1664441095 sock
1770307058 enable  1225375766 mdio 1634697993 spi
1372107864 error_report 744198394 migrate   556841757 swiotlb
1356665351 exceptions   602669807 mmap  400337480 syscalls
[..]

~# ls -i /sys/kernel/tracing/events/sched
1906992193 enable1210369853 sched_process_wait
 592505447 filter1443020725 sched_stat_blocked
1742488280 sched_kthread_stop1213185672 sched_stat_iowait
1674576234 sched_kthread_stop_ret1908510325 sched_stat_runtime
1999376743 sched_kthread_work_execute_end1570203600 sched_stat_sleep
1041533096 sched_kthread_work_execute_start   608113040 sched_stat_wait
 166824308 sched_kthread_work_queue_work 2033295833 sched_stick_numa
 492462616 sched_migrate_task1617500395 sched_swap_numa
1786868196 sched_move_numa   1865216679 sched_switch
 691687388 sched_pi_setprio  1465729401 sched_wait_task
1610977146 sched_process_exec 969941227 
sched_wake_idle_without_ipi
 610128037 sched_process_exit  84398030 sched_wakeup
2139284699 sched_process_fork 750220489 sched_wakeup_new
 169172804 sched_process_free1024064201 sched_waking

-- Steve

diff --git a/fs/tracefs/event_inode.c b/fs/tracefs/event_inode.c
index a0598eb3e26e..57448bf4acb4 100644
--- a/fs/tracefs/event_inode.c
+++ b/fs/tracefs/event_inode.c
@@ -19,6 +19,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -32,6 +33,9 @@
  */
 static DEFINE_MUTEX(eventfs_mutex);
 
+/* Used for making inode numbers */
+static siphash_key_t inode_key;
+
 /*
  * The eventfs_inode (ei) itself is protected by SRCU. It is released from
  * its parent's list and will have is_freed set (under eventfs_mutex).
@@ -57,6 +61,28 @@ static int eventfs_dir_open(struct inode *inode, struct file 
*file);
 static int eventfs_iterate(struct file *file, struct dir_context *ctx);
 static int eventfs_dir_release(struct inode *inode, struct file *file);
 
+/* Copied from scripts/kconfig/symbol.c */
+static unsigned strhash(const char *s)
+{
+   /* fnv32 hash */
+   unsigned hash = 2166136261U;
+   for (; *s; s++)
+   hash = (hash ^ *s) * 0x01000193;
+   return hash;
+}
+
+/* Just try to make something consistent and unique */
+static int eventfs_inode_ino(struct eventfs_inode *ei, const char *name)
+{
+   unsigned long sip = (unsigned long)ei;
+
+   sip += strhash(name);
+   sip = siphash_1u32((int)sip, _key);
+
+   /* keep it positive */
+   return sip & ((1U << 31) - 1);
+}
+
 static void update_attr(struct eventfs_attr *attr, struct iattr *iattr)
 {
unsigned int ia_valid = iattr->ia_valid;
@@ -491,6 +517,8 @@ create_file_dentry(struct eventfs_inode *ei, int idx,
mutex_unlock(_mutex);
 
dentry = create_file(name, mode, attr, parent, data, fops);
+   if (!IS_ERR_OR_NULL(dentry))
+   dentry->d_inode->i_ino = eventfs_inode_ino(ei, name);
 
mutex_lock(_mutex);
 
@@ -585,6 +613,8 @@ create_dir_dentry(struct eventfs_inode *pei, struct 
eventfs_inode *ei,
mutex_unlock(_mutex);
 
dentry = create_dir(ei, parent);
+   if (!IS_ERR_OR_NULL(dentry))
+

Re: [PATCH] eventfs: Create dentries and inodes at dir open

2024-01-16 Thread Steven Rostedt 
On Tue, 16 Jan 2024 10:21:49 -0800
Linus Torvalds  wrote:

> Here's a clue: just fix your inode numbers.
> 
> I can think of many ways to do it. Here's a couple:
> 
>  - use a fixed inode number for all inodes. It's fine. Really. You might
> confuse some programs that still do getpwd() the legacy way, but hey,
> nobody cares
> 
>  - just put the inode number in the same data structure everything else is
>
> 
>  - make the inode number be a hash of the address of your data structure.
> That's actually the closest to a traditional "real" inode number, which is
> just an index to some on-disk thing
> 
> I'm sure there are other *trivial* solutions.
> 
> None of this is an excuse to misuse sentries.
> 
> Try the first one - one single inode number - first. You shouldn't be doing
> iget() anyway, so why do you care so deeply about a number that makes no
> sense and nobody should care about?

It was me being paranoid that using the same inode number would break user
space. If that is not a concern, then I'm happy to just make it either the
same, or maybe just hash the ei and name that it is associated with.

If I do not fully understand how something is used, I try hard to make it
act the same as it does for other use cases. That is, I did all this to
keep inodes unique and consistent because I did not know if it would break
something if I didn't.

Removing that requirement does make it much easier to implement readdir.

I think I'll do the hashing, just because I'm still paranoid that something
might still break if they are all the same.

-- Steve