Is it a good idea to export certain internal structure declaration to make bcc/ebpf happier?

* Is it a good idea to export certain internal structure declaration to make bcc/ebpf happier?
@ 2019-04-02  7:41 Qu Wenruo
  2019-04-02  7:59 ` Nikolay Borisov
  0 siblings, 1 reply; 6+ messages in thread
From: Qu Wenruo @ 2019-04-02  7:41 UTC (permalink / raw)
  To: linux-btrfs, iovisor-dev


[-- Attachment #1.1: Type: text/plain, Size: 5654 bytes --]

Hi,

I'm recently working on crafting a small bcc based script to show how
btrfs tree block concurrency.

The prototype is here:
https://gist.github.com/adam900710/63db6cdc7d26c4744a53dfc7755fafda

However it has some problems, and cannot be used directly on latest kernel.

- No way to pass btrfs headers into eBPF code.
  This is because the declaration of extent_buffer is in
  `fs/btrfs/extent_io.h`.
  No in the common kernel headers search path.

- eBPF program can't access btrfs_header_owner() helper.
  Even we make eBPF code understand the headers, we can't
  access a lot of kernel functions, like btrfs_header_owner().

So I'm wondering is it possible for us to export extent_buffer
declaration just for possible eBPF usage.

Or is bcc suit strong enough to handle such case?

The diff would be something like this to make above bcc script work:

diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index a3e3e95c632e..ba7628434ac9 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -13,6 +13,7 @@
 #include <linux/pagevec.h>
 #include <linux/prefetch.h>
 #include <linux/cleancache.h>
+#include <linux/btrfs.h>
 #include "extent_io.h"
 #include "extent_map.h"
 #include "ctree.h"
diff --git a/fs/btrfs/extent_io.h b/fs/btrfs/extent_io.h
index c4ec104ac157..5bc0df2c1c64 100644
--- a/fs/btrfs/extent_io.h
+++ b/fs/btrfs/extent_io.h
@@ -146,48 +146,6 @@ struct extent_state {
 #endif
 };

-#define INLINE_EXTENT_BUFFER_PAGES 16
-#define MAX_INLINE_EXTENT_BUFFER_SIZE (INLINE_EXTENT_BUFFER_PAGES *
PAGE_SIZE)
-struct extent_buffer {
-       u64 start;
-       unsigned long len;
-       unsigned long bflags;
-       struct btrfs_fs_info *fs_info;
-       spinlock_t refs_lock;
-       atomic_t refs;
-       atomic_t io_pages;
-       int read_mirror;
-       struct rcu_head rcu_head;
-       pid_t lock_owner;
-
-       atomic_t blocking_writers;
-       atomic_t blocking_readers;
-       bool lock_nested;
-       /* >= 0 if eb belongs to a log tree, -1 otherwise */
-       short log_index;
-
-       /* protects write locks */
-       rwlock_t lock;
-
-       /* readers use lock_wq while they wait for the write
-        * lock holders to unlock
-        */
-       wait_queue_head_t write_lock_wq;
-
-       /* writers use read_lock_wq while they wait for readers
-        * to unlock
-        */
-       wait_queue_head_t read_lock_wq;
-       struct page *pages[INLINE_EXTENT_BUFFER_PAGES];
-#ifdef CONFIG_BTRFS_DEBUG
-       atomic_t spinning_writers;
-       atomic_t spinning_readers;
-       atomic_t read_locks;
-       atomic_t write_locks;
-       struct list_head leak_list;
-#endif
-};
-
 /*
  * Structure to record how many bytes and which ranges are set/cleared
  */
diff --git a/fs/btrfs/locking.c b/fs/btrfs/locking.c
index 6df03ba36026..5cf075d02b1b 100644
--- a/fs/btrfs/locking.c
+++ b/fs/btrfs/locking.c
@@ -174,6 +174,8 @@ void btrfs_tree_read_lock(struct extent_buffer *eb)
                BUG_ON(eb->lock_nested);
                eb->lock_nested = true;
                read_unlock(&eb->lock);
+               if (IS_ENABLED(CONFIG_BTRFS_DEBUG))
+                       eb->owner = btrfs_header_owner(eb);
                return;
        }
        if (atomic_read(&eb->blocking_writers)) {
@@ -184,6 +186,8 @@ void btrfs_tree_read_lock(struct extent_buffer *eb)
        }
        btrfs_assert_tree_read_locks_get(eb);
        btrfs_assert_spinning_readers_get(eb);
+       if (IS_ENABLED(CONFIG_BTRFS_DEBUG))
+               eb->owner = btrfs_header_owner(eb);
 }

 /*
@@ -312,6 +316,8 @@ void btrfs_tree_lock(struct extent_buffer *eb)
        btrfs_assert_spinning_writers_get(eb);
        btrfs_assert_tree_write_locks_get(eb);
        eb->lock_owner = current->pid;
+       if (IS_ENABLED(CONFIG_BTRFS_DEBUG))
+               eb->owner = btrfs_header_owner(eb);
 }

 /*
diff --git a/include/uapi/linux/btrfs.h b/include/uapi/linux/btrfs.h
index c195896d478f..3aeaffdadabb 100644
--- a/include/uapi/linux/btrfs.h
+++ b/include/uapi/linux/btrfs.h
@@ -944,4 +944,47 @@ enum btrfs_err_code {
 #define BTRFS_IOC_INO_LOOKUP_USER _IOWR(BTRFS_IOCTL_MAGIC, 62, \
                                struct btrfs_ioctl_ino_lookup_user_args)

+#define INLINE_EXTENT_BUFFER_PAGES 16
+#define MAX_INLINE_EXTENT_BUFFER_SIZE (INLINE_EXTENT_BUFFER_PAGES *
PAGE_SIZE)
+struct extent_buffer {
+       u64 start;
+       unsigned long len;
+       unsigned long bflags;
+       struct btrfs_fs_info *fs_info;
+       spinlock_t refs_lock;
+       atomic_t refs;
+       atomic_t io_pages;
+       int read_mirror;
+       struct rcu_head rcu_head;
+       pid_t lock_owner;
+
+       atomic_t blocking_writers;
+       atomic_t blocking_readers;
+       bool lock_nested;
+       /* >= 0 if eb belongs to a log tree, -1 otherwise */
+       short log_index;
+
+       /* protects write locks */
+       rwlock_t lock;
+
+       /* readers use lock_wq while they wait for the write
+        * lock holders to unlock
+        */
+       wait_queue_head_t write_lock_wq;
+
+       /* writers use read_lock_wq while they wait for readers
+        * to unlock
+        */
+       wait_queue_head_t read_lock_wq;
+       struct page *pages[INLINE_EXTENT_BUFFER_PAGES];
+#ifdef CONFIG_BTRFS_DEBUG
+       atomic_t spinning_writers;
+       atomic_t spinning_readers;
+       atomic_t read_locks;
+       atomic_t write_locks;
+       u64 owner;
+       struct list_head leak_list;
+#endif
+};
+
 #endif /* _UAPI_LINUX_BTRFS_H */

Thanks,
Qu


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply related	[flat|nested] 6+ messages in thread