[PATCH 0/2] btrfs: basic header cleanups

[PATCH 0/2] btrfs: basic header cleanups

[Qu Wenruo]

While reading headers, clangd would do a lot of extra checks, from the
very basic like including the header itself, to missing type definition
inside the header's include chain.

There are 2 very basic fixes can be done immediately:

- Do not do recursive include

- Do not include rwlock_types.h
  As it already mentioned to include spinlock_types.h instead.

Qu Wenruo (2):
  btrfs: cleanup recursive include of the same header
  btrfs: do not directly rwlock_types.h

 fs/btrfs/btrfs_inode.h | 1 -
 fs/btrfs/extent_map.h  | 3 +--
 fs/btrfs/fs.h          | 1 -
 fs/btrfs/locking.h     | 1 -
 fs/btrfs/lru_cache.h   | 1 -
 5 files changed, 1 insertion(+), 6 deletions(-)

-- 
2.45.1

[PATCH 1/2] btrfs: cleanup recursive include of the same header

[Qu Wenruo]

We have several headers that are including themselves, triggering clangd
warnings.

Just remove such unnecessary include.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/btrfs_inode.h | 1 -
 fs/btrfs/extent_map.h  | 1 -
 fs/btrfs/fs.h          | 1 -
 fs/btrfs/locking.h     | 1 -
 fs/btrfs/lru_cache.h   | 1 -
 5 files changed, 5 deletions(-)

diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h
index 97ce56a60672..08828e1ea1f7 100644
--- a/fs/btrfs/btrfs_inode.h
+++ b/fs/btrfs/btrfs_inode.h
@@ -19,7 +19,6 @@
 #include <uapi/linux/btrfs_tree.h>
 #include <trace/events/btrfs.h>
 #include "block-rsv.h"
-#include "btrfs_inode.h"
 #include "extent_map.h"
 #include "extent_io.h"
 #include "extent-io-tree.h"
diff --git a/fs/btrfs/extent_map.h b/fs/btrfs/extent_map.h
index 2bcf7149b44c..d3d1e5b7528d 100644
--- a/fs/btrfs/extent_map.h
+++ b/fs/btrfs/extent_map.h
@@ -9,7 +9,6 @@
 #include <linux/list.h>
 #include <linux/refcount.h>
 #include "misc.h"
-#include "extent_map.h"
 #include "compression.h"
 
 struct btrfs_inode;
diff --git a/fs/btrfs/fs.h b/fs/btrfs/fs.h
index 89f0650631cd..e6b1903f6c32 100644
--- a/fs/btrfs/fs.h
+++ b/fs/btrfs/fs.h
@@ -29,7 +29,6 @@
 #include "extent-io-tree.h"
 #include "async-thread.h"
 #include "block-rsv.h"
-#include "fs.h"
 
 struct inode;
 struct super_block;
diff --git a/fs/btrfs/locking.h b/fs/btrfs/locking.h
index 1bc8e6738879..3c15c75e0582 100644
--- a/fs/btrfs/locking.h
+++ b/fs/btrfs/locking.h
@@ -11,7 +11,6 @@
 #include <linux/lockdep.h>
 #include <linux/percpu_counter.h>
 #include "extent_io.h"
-#include "locking.h"
 
 struct extent_buffer;
 struct btrfs_path;
diff --git a/fs/btrfs/lru_cache.h b/fs/btrfs/lru_cache.h
index e32906ab6faa..07f1bb1c6aa3 100644
--- a/fs/btrfs/lru_cache.h
+++ b/fs/btrfs/lru_cache.h
@@ -6,7 +6,6 @@
 #include <linux/types.h>
 #include <linux/maple_tree.h>
 #include <linux/list.h>
-#include "lru_cache.h"
 
 /*
  * A cache entry. This is meant to be embedded in a structure of a user of
-- 
2.45.1

Re: [PATCH 1/2] btrfs: cleanup recursive include of the same header

[Filipe Manana]

On Tue, May 28, 2024 at 6:34 AM Qu Wenruo <wqu@suse.com> wrote:
>
> We have several headers that are including themselves, triggering clangd
> warnings.
>
> Just remove such unnecessary include.
>
> Signed-off-by: Qu Wenruo <wqu@suse.com>

Maybe add a

Fixes: 602035d7fecf ("btrfs: add forward declarations and headers, part 2")

Not sure how many people are using clangd or if it causes warnings
with other tools.

Anyway:

Reviewed-by: Filipe Manana <fdmanana@suse.com>

Thanks.

> ---
>  fs/btrfs/btrfs_inode.h | 1 -
>  fs/btrfs/extent_map.h  | 1 -
>  fs/btrfs/fs.h          | 1 -
>  fs/btrfs/locking.h     | 1 -
>  fs/btrfs/lru_cache.h   | 1 -
>  5 files changed, 5 deletions(-)
>
> diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h
> index 97ce56a60672..08828e1ea1f7 100644
> --- a/fs/btrfs/btrfs_inode.h
> +++ b/fs/btrfs/btrfs_inode.h
> @@ -19,7 +19,6 @@
>  #include <uapi/linux/btrfs_tree.h>
>  #include <trace/events/btrfs.h>
>  #include "block-rsv.h"
> -#include "btrfs_inode.h"
>  #include "extent_map.h"
>  #include "extent_io.h"
>  #include "extent-io-tree.h"
> diff --git a/fs/btrfs/extent_map.h b/fs/btrfs/extent_map.h
> index 2bcf7149b44c..d3d1e5b7528d 100644
> --- a/fs/btrfs/extent_map.h
> +++ b/fs/btrfs/extent_map.h
> @@ -9,7 +9,6 @@
>  #include <linux/list.h>
>  #include <linux/refcount.h>
>  #include "misc.h"
> -#include "extent_map.h"
>  #include "compression.h"
>
>  struct btrfs_inode;
> diff --git a/fs/btrfs/fs.h b/fs/btrfs/fs.h
> index 89f0650631cd..e6b1903f6c32 100644
> --- a/fs/btrfs/fs.h
> +++ b/fs/btrfs/fs.h
> @@ -29,7 +29,6 @@
>  #include "extent-io-tree.h"
>  #include "async-thread.h"
>  #include "block-rsv.h"
> -#include "fs.h"
>
>  struct inode;
>  struct super_block;
> diff --git a/fs/btrfs/locking.h b/fs/btrfs/locking.h
> index 1bc8e6738879..3c15c75e0582 100644
> --- a/fs/btrfs/locking.h
> +++ b/fs/btrfs/locking.h
> @@ -11,7 +11,6 @@
>  #include <linux/lockdep.h>
>  #include <linux/percpu_counter.h>
>  #include "extent_io.h"
> -#include "locking.h"
>
>  struct extent_buffer;
>  struct btrfs_path;
> diff --git a/fs/btrfs/lru_cache.h b/fs/btrfs/lru_cache.h
> index e32906ab6faa..07f1bb1c6aa3 100644
> --- a/fs/btrfs/lru_cache.h
> +++ b/fs/btrfs/lru_cache.h
> @@ -6,7 +6,6 @@
>  #include <linux/types.h>
>  #include <linux/maple_tree.h>
>  #include <linux/list.h>
> -#include "lru_cache.h"
>
>  /*
>   * A cache entry. This is meant to be embedded in a structure of a user of
> --
> 2.45.1
>
>

Re: [PATCH 1/2] btrfs: cleanup recursive include of the same header

[David Sterba]

On Tue, May 28, 2024 at 11:20:25AM +0100, Filipe Manana wrote:
> On Tue, May 28, 2024 at 6:34 AM Qu Wenruo <wqu@suse.com> wrote:
> >
> > We have several headers that are including themselves, triggering clangd
> > warnings.
> >
> > Just remove such unnecessary include.
> >
> > Signed-off-by: Qu Wenruo <wqu@suse.com>
> 
> Maybe add a
> 
> Fixes: 602035d7fecf ("btrfs: add forward declarations and headers, part 2")

Adding Fixes: to tags could trigger the stable workflow and the patches
could be picked for backport although it's not necessary. Mentioning the
patch that introduced the problme in changelog text should be sufficient
in such cases.

[PATCH 2/2] btrfs: do not directly rwlock_types.h

[Qu Wenruo]

There is already an error inside that header:

 #if !defined(__LINUX_SPINLOCK_TYPES_H)
 # error "Do not include directly, include spinlock_types.h"
 #endif

Thankfully it never get triggered as some other headers have already
included spinlock_types.h.

However clangd would still do a proper warning on that if only
extent_map.h is opened.
Fix it by using spinlock_types.h instead.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/extent_map.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/btrfs/extent_map.h b/fs/btrfs/extent_map.h
index d3d1e5b7528d..5154a8f1d26c 100644
--- a/fs/btrfs/extent_map.h
+++ b/fs/btrfs/extent_map.h
@@ -4,7 +4,7 @@
 #define BTRFS_EXTENT_MAP_H
 
 #include <linux/compiler_types.h>
-#include <linux/rwlock_types.h>
+#include <linux/spinlock_types.h>
 #include <linux/rbtree.h>
 #include <linux/list.h>
 #include <linux/refcount.h>
-- 
2.45.1

Re: [PATCH 2/2] btrfs: do not directly rwlock_types.h

[Filipe Manana]

On Tue, May 28, 2024 at 6:34 AM Qu Wenruo <wqu@suse.com> wrote:
>
> There is already an error inside that header:
>
>  #if !defined(__LINUX_SPINLOCK_TYPES_H)
>  # error "Do not include directly, include spinlock_types.h"
>  #endif
>
> Thankfully it never get triggered as some other headers have already
> included spinlock_types.h.
>
> However clangd would still do a proper warning on that if only
> extent_map.h is opened.
> Fix it by using spinlock_types.h instead.
>
> Signed-off-by: Qu Wenruo <wqu@suse.com>

Looks good, but the subject is missing a word, it should be:

   btrfs: do not directly include rwlock_types.h

Anyway, that can be updated when adding the patch to for-next, so:

Reviewed-by: Filipe Manana <fdmanana@suse.com>

Thanks.

> ---
>  fs/btrfs/extent_map.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/btrfs/extent_map.h b/fs/btrfs/extent_map.h
> index d3d1e5b7528d..5154a8f1d26c 100644
> --- a/fs/btrfs/extent_map.h
> +++ b/fs/btrfs/extent_map.h
> @@ -4,7 +4,7 @@
>  #define BTRFS_EXTENT_MAP_H
>
>  #include <linux/compiler_types.h>
> -#include <linux/rwlock_types.h>
> +#include <linux/spinlock_types.h>
>  #include <linux/rbtree.h>
>  #include <linux/list.h>
>  #include <linux/refcount.h>
> --
> 2.45.1
>
>

Re: [PATCH 0/2] btrfs: basic header cleanups

[David Sterba]

On Tue, May 28, 2024 at 03:03:46PM +0930, Qu Wenruo wrote:
> While reading headers, clangd would do a lot of extra checks, from the
> very basic like including the header itself, to missing type definition
> inside the header's include chain.
> 
> There are 2 very basic fixes can be done immediately:
> 
> - Do not do recursive include
> 
> - Do not include rwlock_types.h
>   As it already mentioned to include spinlock_types.h instead.
> 
> Qu Wenruo (2):
>   btrfs: cleanup recursive include of the same header
>   btrfs: do not directly rwlock_types.h

Reviewed-by: David Sterba <dsterba@suse.com>