* [PATCH] xfsprogs: move stubbed-out kernel functions out of xfs_shared.h
@ 2021-11-04 2:21 Eric Sandeen
2021-11-04 2:28 ` Darrick J. Wong
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Eric Sandeen @ 2021-11-04 2:21 UTC (permalink / raw)
To: xfs; +Cc: Darrick J. Wong
Move kernel stubs out of libxfs/xfs_shared.h, which is kernel
libxfs code and should not have userspace shims in it.
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
diff --git a/include/libxfs.h b/include/libxfs.h
index 24424d0e..64b44af8 100644
--- a/include/libxfs.h
+++ b/include/libxfs.h
@@ -11,6 +11,7 @@
#include "platform_defs.h"
#include "xfs.h"
+#include "stubs.h"
#include "list.h"
#include "hlist.h"
#include "cache.h"
diff --git a/include/stubs.h b/include/stubs.h
new file mode 100644
index 00000000..41eaa0c4
--- /dev/null
+++ b/include/stubs.h
@@ -0,0 +1,28 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * Stub out unimplemented and unneeded kernel structures etc
+ */
+#ifndef STUBS_H
+#define STUBS_H
+
+struct rb_root {
+};
+
+#define RB_ROOT (struct rb_root) { }
+
+typedef struct wait_queue_head {
+} wait_queue_head_t;
+
+#define init_waitqueue_head(wqh) do { } while(0)
+
+struct rhashtable {
+};
+
+struct delayed_work {
+};
+
+#define INIT_DELAYED_WORK(work, func) do { } while(0)
+#define cancel_delayed_work_sync(work) do { } while(0)
+
+#endif
diff --git a/libxfs/libxfs_priv.h b/libxfs/libxfs_priv.h
index 15bae1ff..32271c66 100644
--- a/libxfs/libxfs_priv.h
+++ b/libxfs/libxfs_priv.h
@@ -41,6 +41,7 @@
#include "platform_defs.h"
#include "xfs.h"
+#include "stubs.h"
#include "list.h"
#include "hlist.h"
#include "cache.h"
diff --git a/libxfs/xfs_shared.h b/libxfs/xfs_shared.h
index bafee48c..25c4cab5 100644
--- a/libxfs/xfs_shared.h
+++ b/libxfs/xfs_shared.h
@@ -180,24 +180,4 @@ struct xfs_ino_geometry {
};
-/* Faked up kernel bits */
-struct rb_root {
-};
-
-#define RB_ROOT (struct rb_root) { }
-
-typedef struct wait_queue_head {
-} wait_queue_head_t;
-
-#define init_waitqueue_head(wqh) do { } while(0)
-
-struct rhashtable {
-};
-
-struct delayed_work {
-};
-
-#define INIT_DELAYED_WORK(work, func) do { } while(0)
-#define cancel_delayed_work_sync(work) do { } while(0)
-
#endif /* __XFS_SHARED_H__ */
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] xfsprogs: move stubbed-out kernel functions out of xfs_shared.h
2021-11-04 2:21 [PATCH] xfsprogs: move stubbed-out kernel functions out of xfs_shared.h Eric Sandeen
@ 2021-11-04 2:28 ` Darrick J. Wong
2021-11-04 2:55 ` Eric Sandeen
2021-11-04 2:59 ` [PATCH V2] " Eric Sandeen
2021-11-04 17:15 ` [PATCH V3 RFC] xfsprogs: remove stubbed-out kernel functions out from xfs_shared.h Eric Sandeen
2 siblings, 1 reply; 11+ messages in thread
From: Darrick J. Wong @ 2021-11-04 2:28 UTC (permalink / raw)
To: Eric Sandeen; +Cc: xfs
On Wed, Nov 03, 2021 at 09:21:35PM -0500, Eric Sandeen wrote:
> Move kernel stubs out of libxfs/xfs_shared.h, which is kernel
> libxfs code and should not have userspace shims in it.
>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
>
> diff --git a/include/libxfs.h b/include/libxfs.h
> index 24424d0e..64b44af8 100644
> --- a/include/libxfs.h
> +++ b/include/libxfs.h
> @@ -11,6 +11,7 @@
> #include "platform_defs.h"
> #include "xfs.h"
> +#include "stubs.h"
> #include "list.h"
> #include "hlist.h"
> #include "cache.h"
> diff --git a/include/stubs.h b/include/stubs.h
> new file mode 100644
> index 00000000..41eaa0c4
> --- /dev/null
> +++ b/include/stubs.h
> @@ -0,0 +1,28 @@
> +// SPDX-License-Identifier: GPL-2.0
This needs a C-style (not C++-style) comment for SPDX compliance.
(I still don't get why the committee who came up with SPDX required C++
style comments for C code...)
> +
Needs a copyright statement too.
> +/*
> + * Stub out unimplemented and unneeded kernel structures etc
> + */
> +#ifndef STUBS_H
> +#define STUBS_H
> +
> +struct rb_root {
> +};
> +
> +#define RB_ROOT (struct rb_root) { }
Space after 'T' and before '('.
--D
> +
> +typedef struct wait_queue_head {
> +} wait_queue_head_t;
> +
> +#define init_waitqueue_head(wqh) do { } while(0)
> +
> +struct rhashtable {
> +};
> +
> +struct delayed_work {
> +};
> +
> +#define INIT_DELAYED_WORK(work, func) do { } while(0)
> +#define cancel_delayed_work_sync(work) do { } while(0)
> +
> +#endif
> diff --git a/libxfs/libxfs_priv.h b/libxfs/libxfs_priv.h
> index 15bae1ff..32271c66 100644
> --- a/libxfs/libxfs_priv.h
> +++ b/libxfs/libxfs_priv.h
> @@ -41,6 +41,7 @@
> #include "platform_defs.h"
> #include "xfs.h"
> +#include "stubs.h"
> #include "list.h"
> #include "hlist.h"
> #include "cache.h"
> diff --git a/libxfs/xfs_shared.h b/libxfs/xfs_shared.h
> index bafee48c..25c4cab5 100644
> --- a/libxfs/xfs_shared.h
> +++ b/libxfs/xfs_shared.h
> @@ -180,24 +180,4 @@ struct xfs_ino_geometry {
> };
> -/* Faked up kernel bits */
> -struct rb_root {
> -};
> -
> -#define RB_ROOT (struct rb_root) { }
> -
> -typedef struct wait_queue_head {
> -} wait_queue_head_t;
> -
> -#define init_waitqueue_head(wqh) do { } while(0)
> -
> -struct rhashtable {
> -};
> -
> -struct delayed_work {
> -};
> -
> -#define INIT_DELAYED_WORK(work, func) do { } while(0)
> -#define cancel_delayed_work_sync(work) do { } while(0)
> -
> #endif /* __XFS_SHARED_H__ */
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] xfsprogs: move stubbed-out kernel functions out of xfs_shared.h
2021-11-04 2:28 ` Darrick J. Wong
@ 2021-11-04 2:55 ` Eric Sandeen
0 siblings, 0 replies; 11+ messages in thread
From: Eric Sandeen @ 2021-11-04 2:55 UTC (permalink / raw)
To: Darrick J. Wong, Eric Sandeen; +Cc: xfs
On 11/3/21 9:28 PM, Darrick J. Wong wrote:
> On Wed, Nov 03, 2021 at 09:21:35PM -0500, Eric Sandeen wrote:
>> Move kernel stubs out of libxfs/xfs_shared.h, which is kernel
>> libxfs code and should not have userspace shims in it.
>>
>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>> ---
>>
>> diff --git a/include/libxfs.h b/include/libxfs.h
>> index 24424d0e..64b44af8 100644
>> --- a/include/libxfs.h
>> +++ b/include/libxfs.h
>> @@ -11,6 +11,7 @@
>> #include "platform_defs.h"
>> #include "xfs.h"
>> +#include "stubs.h"
>> #include "list.h"
>> #include "hlist.h"
>> #include "cache.h"
>> diff --git a/include/stubs.h b/include/stubs.h
>> new file mode 100644
>> index 00000000..41eaa0c4
>> --- /dev/null
>> +++ b/include/stubs.h
>> @@ -0,0 +1,28 @@
>> +// SPDX-License-Identifier: GPL-2.0
>
> This needs a C-style (not C++-style) comment for SPDX compliance.
>
> (I still don't get why the committee who came up with SPDX required C++
> style comments for C code...)
>
>> +
>
> Needs a copyright statement too.
well that's what I get for cargo-culting bitops.h. :/
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH V2] xfsprogs: move stubbed-out kernel functions out of xfs_shared.h
2021-11-04 2:21 [PATCH] xfsprogs: move stubbed-out kernel functions out of xfs_shared.h Eric Sandeen
2021-11-04 2:28 ` Darrick J. Wong
@ 2021-11-04 2:59 ` Eric Sandeen
2021-11-04 3:14 ` Darrick J. Wong
2021-11-04 17:15 ` [PATCH V3 RFC] xfsprogs: remove stubbed-out kernel functions out from xfs_shared.h Eric Sandeen
2 siblings, 1 reply; 11+ messages in thread
From: Eric Sandeen @ 2021-11-04 2:59 UTC (permalink / raw)
To: xfs; +Cc: Darrick J. Wong
Move kernel stubs out of libxfs/xfs_shared.h, which is kernel
libxfs code and should not have userspace shims in it.
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
V2: fix spdx and copyright
diff --git a/include/libxfs.h b/include/libxfs.h
index 24424d0e..64b44af8 100644
--- a/include/libxfs.h
+++ b/include/libxfs.h
@@ -11,6 +11,7 @@
#include "platform_defs.h"
#include "xfs.h"
+#include "stubs.h"
#include "list.h"
#include "hlist.h"
#include "cache.h"
diff --git a/include/stubs.h b/include/stubs.h
new file mode 100644
index 00000000..d80e8de0
--- /dev/null
+++ b/include/stubs.h
@@ -0,0 +1,29 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2021 Red Hat, Inc.
+ * All Rights Reserved.
+ */
+#ifndef STUBS_H
+#define STUBS_H
+
+/* Stub out unimplemented and unneeded kernel functions */
+struct rb_root {
+};
+
+#define RB_ROOT (struct rb_root) { }
+
+typedef struct wait_queue_head {
+} wait_queue_head_t;
+
+#define init_waitqueue_head(wqh) do { } while(0)
+
+struct rhashtable {
+};
+
+struct delayed_work {
+};
+
+#define INIT_DELAYED_WORK(work, func) do { } while(0)
+#define cancel_delayed_work_sync(work) do { } while(0)
+
+#endif
diff --git a/libxfs/libxfs_priv.h b/libxfs/libxfs_priv.h
index 15bae1ff..32271c66 100644
--- a/libxfs/libxfs_priv.h
+++ b/libxfs/libxfs_priv.h
@@ -41,6 +41,7 @@
#include "platform_defs.h"
#include "xfs.h"
+#include "stubs.h"
#include "list.h"
#include "hlist.h"
#include "cache.h"
diff --git a/libxfs/xfs_shared.h b/libxfs/xfs_shared.h
index bafee48c..25c4cab5 100644
--- a/libxfs/xfs_shared.h
+++ b/libxfs/xfs_shared.h
@@ -180,24 +180,4 @@ struct xfs_ino_geometry {
};
-/* Faked up kernel bits */
-struct rb_root {
-};
-
-#define RB_ROOT (struct rb_root) { }
-
-typedef struct wait_queue_head {
-} wait_queue_head_t;
-
-#define init_waitqueue_head(wqh) do { } while(0)
-
-struct rhashtable {
-};
-
-struct delayed_work {
-};
-
-#define INIT_DELAYED_WORK(work, func) do { } while(0)
-#define cancel_delayed_work_sync(work) do { } while(0)
-
#endif /* __XFS_SHARED_H__ */
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH V2] xfsprogs: move stubbed-out kernel functions out of xfs_shared.h
2021-11-04 2:59 ` [PATCH V2] " Eric Sandeen
@ 2021-11-04 3:14 ` Darrick J. Wong
2021-11-04 3:33 ` Eric Sandeen
0 siblings, 1 reply; 11+ messages in thread
From: Darrick J. Wong @ 2021-11-04 3:14 UTC (permalink / raw)
To: sandeen; +Cc: xfs
On Wed, Nov 03, 2021 at 09:59:57PM -0500, Eric Sandeen wrote:
> Move kernel stubs out of libxfs/xfs_shared.h, which is kernel
> libxfs code and should not have userspace shims in it.
>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
>
> V2: fix spdx and copyright
>
> diff --git a/include/libxfs.h b/include/libxfs.h
> index 24424d0e..64b44af8 100644
> --- a/include/libxfs.h
> +++ b/include/libxfs.h
> @@ -11,6 +11,7 @@
> #include "platform_defs.h"
> #include "xfs.h"
> +#include "stubs.h"
> #include "list.h"
> #include "hlist.h"
> #include "cache.h"
> diff --git a/include/stubs.h b/include/stubs.h
> new file mode 100644
> index 00000000..d80e8de0
> --- /dev/null
> +++ b/include/stubs.h
> @@ -0,0 +1,29 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2021 Red Hat, Inc.
> + * All Rights Reserved.
> + */
> +#ifndef STUBS_H
> +#define STUBS_H
> +
> +/* Stub out unimplemented and unneeded kernel functions */
> +struct rb_root {
> +};
> +
> +#define RB_ROOT (struct rb_root) { }
Please to remove ^ this unnecessary space.
> +
> +typedef struct wait_queue_head {
> +} wait_queue_head_t;
> +
> +#define init_waitqueue_head(wqh) do { } while(0)
> +
> +struct rhashtable {
> +};
> +
> +struct delayed_work {
> +};
> +
> +#define INIT_DELAYED_WORK(work, func) do { } while(0)
> +#define cancel_delayed_work_sync(work) do { } while(0)
> +
> +#endif
This probably ought to be '#endif /* STUBS_H */' just to keep it clear
which #ifdef it goes with.
With those two things fixed,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
--D
> diff --git a/libxfs/libxfs_priv.h b/libxfs/libxfs_priv.h
> index 15bae1ff..32271c66 100644
> --- a/libxfs/libxfs_priv.h
> +++ b/libxfs/libxfs_priv.h
> @@ -41,6 +41,7 @@
> #include "platform_defs.h"
> #include "xfs.h"
> +#include "stubs.h"
> #include "list.h"
> #include "hlist.h"
> #include "cache.h"
> diff --git a/libxfs/xfs_shared.h b/libxfs/xfs_shared.h
> index bafee48c..25c4cab5 100644
> --- a/libxfs/xfs_shared.h
> +++ b/libxfs/xfs_shared.h
> @@ -180,24 +180,4 @@ struct xfs_ino_geometry {
> };
> -/* Faked up kernel bits */
> -struct rb_root {
> -};
> -
> -#define RB_ROOT (struct rb_root) { }
> -
> -typedef struct wait_queue_head {
> -} wait_queue_head_t;
> -
> -#define init_waitqueue_head(wqh) do { } while(0)
> -
> -struct rhashtable {
> -};
> -
> -struct delayed_work {
> -};
> -
> -#define INIT_DELAYED_WORK(work, func) do { } while(0)
> -#define cancel_delayed_work_sync(work) do { } while(0)
> -
> #endif /* __XFS_SHARED_H__ */
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH V2] xfsprogs: move stubbed-out kernel functions out of xfs_shared.h
2021-11-04 3:14 ` Darrick J. Wong
@ 2021-11-04 3:33 ` Eric Sandeen
0 siblings, 0 replies; 11+ messages in thread
From: Eric Sandeen @ 2021-11-04 3:33 UTC (permalink / raw)
To: Darrick J. Wong, sandeen; +Cc: xfs
On 11/3/21 10:14 PM, Darrick J. Wong wrote:
> On Wed, Nov 03, 2021 at 09:59:57PM -0500, Eric Sandeen wrote:
>> Move kernel stubs out of libxfs/xfs_shared.h, which is kernel
>> libxfs code and should not have userspace shims in it.
>>
>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>> ---
>>
>> V2: fix spdx and copyright
>>
>> diff --git a/include/libxfs.h b/include/libxfs.h
>> index 24424d0e..64b44af8 100644
>> --- a/include/libxfs.h
>> +++ b/include/libxfs.h
>> @@ -11,6 +11,7 @@
>> #include "platform_defs.h"
>> #include "xfs.h"
>> +#include "stubs.h"
>> #include "list.h"
>> #include "hlist.h"
>> #include "cache.h"
>> diff --git a/include/stubs.h b/include/stubs.h
>> new file mode 100644
>> index 00000000..d80e8de0
>> --- /dev/null
>> +++ b/include/stubs.h
>> @@ -0,0 +1,29 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + * Copyright (c) 2021 Red Hat, Inc.
>> + * All Rights Reserved.
>> + */
>> +#ifndef STUBS_H
>> +#define STUBS_H
>> +
>> +/* Stub out unimplemented and unneeded kernel functions */
>> +struct rb_root {
>> +};
>> +
>> +#define RB_ROOT (struct rb_root) { }
>
> Please to remove ^ this unnecessary space.
>
>> +
>> +typedef struct wait_queue_head {
>> +} wait_queue_head_t;
>> +
>> +#define init_waitqueue_head(wqh) do { } while(0)
>> +
>> +struct rhashtable {
>> +};
>> +
>> +struct delayed_work {
>> +};
>> +
>> +#define INIT_DELAYED_WORK(work, func) do { } while(0)
>> +#define cancel_delayed_work_sync(work) do { } while(0)
>> +
>> +#endif
>
> This probably ought to be '#endif /* STUBS_H */' just to keep it clear
> which #ifdef it goes with.
Yup I spotted and added that already. I'm so rusty.
> With those two things fixed,
> Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Thanks,
-Eric
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH V3 RFC] xfsprogs: remove stubbed-out kernel functions out from xfs_shared.h
2021-11-04 2:21 [PATCH] xfsprogs: move stubbed-out kernel functions out of xfs_shared.h Eric Sandeen
2021-11-04 2:28 ` Darrick J. Wong
2021-11-04 2:59 ` [PATCH V2] " Eric Sandeen
@ 2021-11-04 17:15 ` Eric Sandeen
2021-11-04 19:08 ` Darrick J. Wong
2021-11-04 22:38 ` Dave Chinner
2 siblings, 2 replies; 11+ messages in thread
From: Eric Sandeen @ 2021-11-04 17:15 UTC (permalink / raw)
To: xfs; +Cc: Darrick J. Wong, Dave Chinner
Remove these kernel stubs by #ifdeffing code instead.
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
Dave preferred #ifdefs over stubs, and this is what I came up with.
Honestly, I think this is worse, and will lead to more libxfs-sync pain
unless we're willing to scatter #ifdefs around the kernel code as well,
but I figured I'd put this out there for discussion.
diff --git a/libxfs/xfs_ag.c b/libxfs/xfs_ag.c
index 9eda6eba..c01986f7 100644
--- a/libxfs/xfs_ag.c
+++ b/libxfs/xfs_ag.c
@@ -170,7 +170,9 @@ __xfs_free_perag(
{
struct xfs_perag *pag = container_of(head, struct xfs_perag, rcu_head);
+#ifdef __KERNEL__
ASSERT(!delayed_work_pending(&pag->pag_blockgc_work));
+#endif /* __KERNEL__ */
ASSERT(atomic_read(&pag->pag_ref) == 0);
kmem_free(pag);
}
@@ -192,9 +194,11 @@ xfs_free_perag(
ASSERT(pag);
ASSERT(atomic_read(&pag->pag_ref) == 0);
+#ifdef __KERNEL__
cancel_delayed_work_sync(&pag->pag_blockgc_work);
xfs_iunlink_destroy(pag);
xfs_buf_hash_destroy(pag);
+#endif /* __KERNEL__ */
call_rcu(&pag->rcu_head, __xfs_free_perag);
}
@@ -246,6 +250,7 @@ xfs_initialize_perag(
spin_unlock(&mp->m_perag_lock);
radix_tree_preload_end();
+#ifdef __KERNEL__
/* Place kernel structure only init below this point. */
spin_lock_init(&pag->pag_ici_lock);
spin_lock_init(&pag->pagb_lock);
@@ -267,6 +272,7 @@ xfs_initialize_perag(
/* first new pag is fully initialized */
if (first_initialised == NULLAGNUMBER)
first_initialised = index;
+#endif /* __KERNEL__ */
}
index = xfs_set_inode_alloc(mp, agcount);
@@ -277,10 +283,12 @@ xfs_initialize_perag(
mp->m_ag_prealloc_blocks = xfs_prealloc_blocks(mp);
return 0;
+#ifdef __KERNEL__
out_hash_destroy:
xfs_buf_hash_destroy(pag);
out_remove_pag:
radix_tree_delete(&mp->m_perag_tree, index);
+#endif /* __KERNEL__ */
out_free_pag:
kmem_free(pag);
out_unwind_new_pags:
diff --git a/libxfs/xfs_ag.h b/libxfs/xfs_ag.h
index 4c6f9045..dda1303e 100644
--- a/libxfs/xfs_ag.h
+++ b/libxfs/xfs_ag.h
@@ -64,6 +64,9 @@ struct xfs_perag {
/* Blocks reserved for the reverse mapping btree. */
struct xfs_ag_resv pag_rmapbt_resv;
+ /* for rcu-safe freeing */
+ struct rcu_head rcu_head;
+
/* -- kernel only structures below this line -- */
/*
@@ -75,6 +78,7 @@ struct xfs_perag {
spinlock_t pag_state_lock;
spinlock_t pagb_lock; /* lock for pagb_tree */
+#ifdef __KERNEL__
struct rb_root pagb_tree; /* ordered tree of busy extents */
unsigned int pagb_gen; /* generation count for pagb_tree */
wait_queue_head_t pagb_wait; /* woken when pagb_gen changes */
@@ -90,9 +94,6 @@ struct xfs_perag {
spinlock_t pag_buf_lock; /* lock for pag_buf_hash */
struct rhashtable pag_buf_hash;
- /* for rcu-safe freeing */
- struct rcu_head rcu_head;
-
/* background prealloc block trimming */
struct delayed_work pag_blockgc_work;
@@ -102,6 +103,7 @@ struct xfs_perag {
* or have some other means to control concurrency.
*/
struct rhashtable pagi_unlinked_hash;
+#endif /* __KERNEL__ */
};
int xfs_initialize_perag(struct xfs_mount *mp, xfs_agnumber_t agcount,
diff --git a/libxfs/xfs_shared.h b/libxfs/xfs_shared.h
index bafee48c..25c4cab5 100644
--- a/libxfs/xfs_shared.h
+++ b/libxfs/xfs_shared.h
@@ -180,24 +180,4 @@ struct xfs_ino_geometry {
};
-/* Faked up kernel bits */
-struct rb_root {
-};
-
-#define RB_ROOT (struct rb_root) { }
-
-typedef struct wait_queue_head {
-} wait_queue_head_t;
-
-#define init_waitqueue_head(wqh) do { } while(0)
-
-struct rhashtable {
-};
-
-struct delayed_work {
-};
-
-#define INIT_DELAYED_WORK(work, func) do { } while(0)
-#define cancel_delayed_work_sync(work) do { } while(0)
-
#endif /* __XFS_SHARED_H__ */
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH V3 RFC] xfsprogs: remove stubbed-out kernel functions out from xfs_shared.h
2021-11-04 17:15 ` [PATCH V3 RFC] xfsprogs: remove stubbed-out kernel functions out from xfs_shared.h Eric Sandeen
@ 2021-11-04 19:08 ` Darrick J. Wong
2021-11-04 22:38 ` Dave Chinner
1 sibling, 0 replies; 11+ messages in thread
From: Darrick J. Wong @ 2021-11-04 19:08 UTC (permalink / raw)
To: sandeen; +Cc: xfs, Dave Chinner
On Thu, Nov 04, 2021 at 12:15:04PM -0500, Eric Sandeen wrote:
> Remove these kernel stubs by #ifdeffing code instead.
>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
>
> Dave preferred #ifdefs over stubs, and this is what I came up with.
>
> Honestly, I think this is worse, and will lead to more libxfs-sync pain
> unless we're willing to scatter #ifdefs around the kernel code as well,
> but I figured I'd put this out there for discussion.
Yuck. Now I wish I'd pushed back harder on the patch author (Dave) to
provide the xfsprogs version of this, or whatever fixes are needed to
libxfs-diff to deuglify whatever the result was, rather than let this
fall to the maintainer (Eric). :/
--D
>
> diff --git a/libxfs/xfs_ag.c b/libxfs/xfs_ag.c
> index 9eda6eba..c01986f7 100644
> --- a/libxfs/xfs_ag.c
> +++ b/libxfs/xfs_ag.c
> @@ -170,7 +170,9 @@ __xfs_free_perag(
> {
> struct xfs_perag *pag = container_of(head, struct xfs_perag, rcu_head);
> +#ifdef __KERNEL__
> ASSERT(!delayed_work_pending(&pag->pag_blockgc_work));
> +#endif /* __KERNEL__ */
> ASSERT(atomic_read(&pag->pag_ref) == 0);
> kmem_free(pag);
> }
> @@ -192,9 +194,11 @@ xfs_free_perag(
> ASSERT(pag);
> ASSERT(atomic_read(&pag->pag_ref) == 0);
> +#ifdef __KERNEL__
> cancel_delayed_work_sync(&pag->pag_blockgc_work);
> xfs_iunlink_destroy(pag);
> xfs_buf_hash_destroy(pag);
> +#endif /* __KERNEL__ */
> call_rcu(&pag->rcu_head, __xfs_free_perag);
> }
> @@ -246,6 +250,7 @@ xfs_initialize_perag(
> spin_unlock(&mp->m_perag_lock);
> radix_tree_preload_end();
> +#ifdef __KERNEL__
> /* Place kernel structure only init below this point. */
> spin_lock_init(&pag->pag_ici_lock);
> spin_lock_init(&pag->pagb_lock);
> @@ -267,6 +272,7 @@ xfs_initialize_perag(
> /* first new pag is fully initialized */
> if (first_initialised == NULLAGNUMBER)
> first_initialised = index;
> +#endif /* __KERNEL__ */
> }
> index = xfs_set_inode_alloc(mp, agcount);
> @@ -277,10 +283,12 @@ xfs_initialize_perag(
> mp->m_ag_prealloc_blocks = xfs_prealloc_blocks(mp);
> return 0;
> +#ifdef __KERNEL__
> out_hash_destroy:
> xfs_buf_hash_destroy(pag);
> out_remove_pag:
> radix_tree_delete(&mp->m_perag_tree, index);
> +#endif /* __KERNEL__ */
> out_free_pag:
> kmem_free(pag);
> out_unwind_new_pags:
> diff --git a/libxfs/xfs_ag.h b/libxfs/xfs_ag.h
> index 4c6f9045..dda1303e 100644
> --- a/libxfs/xfs_ag.h
> +++ b/libxfs/xfs_ag.h
> @@ -64,6 +64,9 @@ struct xfs_perag {
> /* Blocks reserved for the reverse mapping btree. */
> struct xfs_ag_resv pag_rmapbt_resv;
> + /* for rcu-safe freeing */
> + struct rcu_head rcu_head;
> +
> /* -- kernel only structures below this line -- */
> /*
> @@ -75,6 +78,7 @@ struct xfs_perag {
> spinlock_t pag_state_lock;
> spinlock_t pagb_lock; /* lock for pagb_tree */
> +#ifdef __KERNEL__
> struct rb_root pagb_tree; /* ordered tree of busy extents */
> unsigned int pagb_gen; /* generation count for pagb_tree */
> wait_queue_head_t pagb_wait; /* woken when pagb_gen changes */
> @@ -90,9 +94,6 @@ struct xfs_perag {
> spinlock_t pag_buf_lock; /* lock for pag_buf_hash */
> struct rhashtable pag_buf_hash;
> - /* for rcu-safe freeing */
> - struct rcu_head rcu_head;
> -
> /* background prealloc block trimming */
> struct delayed_work pag_blockgc_work;
> @@ -102,6 +103,7 @@ struct xfs_perag {
> * or have some other means to control concurrency.
> */
> struct rhashtable pagi_unlinked_hash;
> +#endif /* __KERNEL__ */
> };
> int xfs_initialize_perag(struct xfs_mount *mp, xfs_agnumber_t agcount,
> diff --git a/libxfs/xfs_shared.h b/libxfs/xfs_shared.h
> index bafee48c..25c4cab5 100644
> --- a/libxfs/xfs_shared.h
> +++ b/libxfs/xfs_shared.h
> @@ -180,24 +180,4 @@ struct xfs_ino_geometry {
> };
> -/* Faked up kernel bits */
> -struct rb_root {
> -};
> -
> -#define RB_ROOT (struct rb_root) { }
> -
> -typedef struct wait_queue_head {
> -} wait_queue_head_t;
> -
> -#define init_waitqueue_head(wqh) do { } while(0)
> -
> -struct rhashtable {
> -};
> -
> -struct delayed_work {
> -};
> -
> -#define INIT_DELAYED_WORK(work, func) do { } while(0)
> -#define cancel_delayed_work_sync(work) do { } while(0)
> -
> #endif /* __XFS_SHARED_H__ */
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH V3 RFC] xfsprogs: remove stubbed-out kernel functions out from xfs_shared.h
2021-11-04 17:15 ` [PATCH V3 RFC] xfsprogs: remove stubbed-out kernel functions out from xfs_shared.h Eric Sandeen
2021-11-04 19:08 ` Darrick J. Wong
@ 2021-11-04 22:38 ` Dave Chinner
2021-11-05 16:40 ` Eric Sandeen
1 sibling, 1 reply; 11+ messages in thread
From: Dave Chinner @ 2021-11-04 22:38 UTC (permalink / raw)
To: sandeen; +Cc: xfs, Darrick J. Wong
On Thu, Nov 04, 2021 at 12:15:04PM -0500, Eric Sandeen wrote:
> Remove these kernel stubs by #ifdeffing code instead.
>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
>
> Dave preferred #ifdefs over stubs, and this is what I came up with.
>
> Honestly, I think this is worse, and will lead to more libxfs-sync pain
> unless we're willing to scatter #ifdefs around the kernel code as well,
> but I figured I'd put this out there for discussion.
>
> diff --git a/libxfs/xfs_ag.c b/libxfs/xfs_ag.c
> index 9eda6eba..c01986f7 100644
> --- a/libxfs/xfs_ag.c
> +++ b/libxfs/xfs_ag.c
> @@ -170,7 +170,9 @@ __xfs_free_perag(
> {
> struct xfs_perag *pag = container_of(head, struct xfs_perag, rcu_head);
> +#ifdef __KERNEL__
> ASSERT(!delayed_work_pending(&pag->pag_blockgc_work));
> +#endif /* __KERNEL__ */
> ASSERT(atomic_read(&pag->pag_ref) == 0);
> kmem_free(pag);
> }
> @@ -192,9 +194,11 @@ xfs_free_perag(
> ASSERT(pag);
> ASSERT(atomic_read(&pag->pag_ref) == 0);
> +#ifdef __KERNEL__
> cancel_delayed_work_sync(&pag->pag_blockgc_work);
> xfs_iunlink_destroy(pag);
> xfs_buf_hash_destroy(pag);
> +#endif /* __KERNEL__ */
> call_rcu(&pag->rcu_head, __xfs_free_perag);
> }
These can be stubbed in libxfs_priv.h as we do with all other kernel
functions we don't use:
#define delayed_work_pending(a) ((void)0)
#define cancel_delayed_work_sync(a) ((void)0)
#define xfs_iunlink_destroy(a) ((void)0)
#define xfs_buf_hash_destroy(a) ((void)0)
That is the normal way we avoid needing these ifdef KERNEL clauses
in the libxfs C code.
> @@ -246,6 +250,7 @@ xfs_initialize_perag(
> spin_unlock(&mp->m_perag_lock);
> radix_tree_preload_end();
> +#ifdef __KERNEL__
> /* Place kernel structure only init below this point. */
> spin_lock_init(&pag->pag_ici_lock);
> spin_lock_init(&pag->pagb_lock);
> @@ -267,6 +272,7 @@ xfs_initialize_perag(
> /* first new pag is fully initialized */
> if (first_initialised == NULLAGNUMBER)
> first_initialised = index;
> +#endif /* __KERNEL__ */
> }
endif is in the wrong place - it needs to be before the
first_initialised checks because that is necessary for error
unwinding.
> index = xfs_set_inode_alloc(mp, agcount);
> @@ -277,10 +283,12 @@ xfs_initialize_perag(
> mp->m_ag_prealloc_blocks = xfs_prealloc_blocks(mp);
> return 0;
> +#ifdef __KERNEL__
> out_hash_destroy:
> xfs_buf_hash_destroy(pag);
> out_remove_pag:
> radix_tree_delete(&mp->m_perag_tree, index);
> +#endif /* __KERNEL__ */
> out_free_pag:
> kmem_free(pag);
> out_unwind_new_pags:
Again, stubbing out the functions like so:
#define xfs_buf_hash_init(a) ((void)0)
#define xfs_unlink_init(a) ((void)0)
means that the conditional init code doesn't need to be ifdef'd out
and so the error unwinding doesn't need to be ifdef'd out, either.
And, FWIW, you missed the xfs_buf_hash_destroy/xfs_iunlink_destroy
calls in the unwinding code....
> diff --git a/libxfs/xfs_ag.h b/libxfs/xfs_ag.h
> index 4c6f9045..dda1303e 100644
> --- a/libxfs/xfs_ag.h
> +++ b/libxfs/xfs_ag.h
> @@ -64,6 +64,9 @@ struct xfs_perag {
> /* Blocks reserved for the reverse mapping btree. */
> struct xfs_ag_resv pag_rmapbt_resv;
> + /* for rcu-safe freeing */
> + struct rcu_head rcu_head;
> +
> /* -- kernel only structures below this line -- */
> /*
Moving the rcu_head needs to be done in a separate patch, as that
needs to be done on the kernel side, too. When this change went into
the kernel, we didn't have userspace RCU so it was considered a
kernel only structure....
With those changes, we end up with some new stubs in libxfs_priv.h
and two places where we need #ifdef __KERNEL__ in xfs_ag.[ch]. Most
of the mess in this patch goes away....
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH V3 RFC] xfsprogs: remove stubbed-out kernel functions out from xfs_shared.h
2021-11-04 22:38 ` Dave Chinner
@ 2021-11-05 16:40 ` Eric Sandeen
2021-11-07 22:58 ` Dave Chinner
0 siblings, 1 reply; 11+ messages in thread
From: Eric Sandeen @ 2021-11-05 16:40 UTC (permalink / raw)
To: Dave Chinner, sandeen; +Cc: xfs, Darrick J. Wong
On 11/4/21 5:38 PM, Dave Chinner wrote:
> With those changes, we end up with some new stubs in libxfs_priv.h
> and two places where we need #ifdef __KERNEL__ in xfs_ag.[ch]. Most
> of the mess in this patch goes away....
>
> Cheers,
>
> Dave.
Ok.
I will split this up into the right patch granularity, but is this the
endpoint you're looking for? One #ifdef in each of xfs_ag.[ch], two total.
The delayed work init/cancel assymmetry is a little odd, but I'll
get over it.
diff --git a/libxfs/libxfs_priv.h b/libxfs/libxfs_priv.h
index 15bae1ff..2ca3b9b2 100644
--- a/libxfs/libxfs_priv.h
+++ b/libxfs/libxfs_priv.h
@@ -673,6 +673,9 @@ static inline void xfs_iunlink_destroy(struct xfs_perag *pag) { }
xfs_agnumber_t xfs_set_inode_alloc(struct xfs_mount *mp,
xfs_agnumber_t agcount);
+/* Faked up kernel bits */
+#define cancel_delayed_work_sync(work) do { } while(0)
+
/* Keep static checkers quiet about nonstatic functions by exporting */
int xfs_rtbuf_get(struct xfs_mount *mp, struct xfs_trans *tp,
xfs_rtblock_t block, int issum, struct xfs_buf **bpp);
diff --git a/libxfs/xfs_ag.c b/libxfs/xfs_ag.c
index 9eda6eba..149f9857 100644
--- a/libxfs/xfs_ag.c
+++ b/libxfs/xfs_ag.c
@@ -246,6 +246,7 @@ xfs_initialize_perag(
spin_unlock(&mp->m_perag_lock);
radix_tree_preload_end();
+#ifdef __KERNEL__
/* Place kernel structure only init below this point. */
spin_lock_init(&pag->pag_ici_lock);
spin_lock_init(&pag->pagb_lock);
@@ -255,6 +256,7 @@ xfs_initialize_perag(
init_waitqueue_head(&pag->pagb_wait);
pag->pagb_count = 0;
pag->pagb_tree = RB_ROOT;
+#endif /* __KERNEL_ */
error = xfs_buf_hash_init(pag);
if (error)
diff --git a/libxfs/xfs_ag.h b/libxfs/xfs_ag.h
index 4c6f9045..ef04a537 100644
--- a/libxfs/xfs_ag.h
+++ b/libxfs/xfs_ag.h
@@ -64,8 +64,11 @@ struct xfs_perag {
/* Blocks reserved for the reverse mapping btree. */
struct xfs_ag_resv pag_rmapbt_resv;
- /* -- kernel only structures below this line -- */
+ /* for rcu-safe freeing */
+ struct rcu_head rcu_head;
+#ifdef __KERNEL__
+ /* -- kernel only structures below this line -- */
/*
* Bitsets of per-ag metadata that have been checked and/or are sick.
* Callers should hold pag_state_lock before accessing this field.
@@ -90,9 +93,6 @@ struct xfs_perag {
spinlock_t pag_buf_lock; /* lock for pag_buf_hash */
struct rhashtable pag_buf_hash;
- /* for rcu-safe freeing */
- struct rcu_head rcu_head;
-
/* background prealloc block trimming */
struct delayed_work pag_blockgc_work;
@@ -102,6 +102,7 @@ struct xfs_perag {
* or have some other means to control concurrency.
*/
struct rhashtable pagi_unlinked_hash;
+#endif /* __KERNEL__ */
};
int xfs_initialize_perag(struct xfs_mount *mp, xfs_agnumber_t agcount,
diff --git a/libxfs/xfs_shared.h b/libxfs/xfs_shared.h
index bafee48c..25c4cab5 100644
--- a/libxfs/xfs_shared.h
+++ b/libxfs/xfs_shared.h
@@ -180,24 +180,4 @@ struct xfs_ino_geometry {
};
-/* Faked up kernel bits */
-struct rb_root {
-};
-
-#define RB_ROOT (struct rb_root) { }
-
-typedef struct wait_queue_head {
-} wait_queue_head_t;
-
-#define init_waitqueue_head(wqh) do { } while(0)
-
-struct rhashtable {
-};
-
-struct delayed_work {
-};
-
-#define INIT_DELAYED_WORK(work, func) do { } while(0)
-#define cancel_delayed_work_sync(work) do { } while(0)
-
#endif /* __XFS_SHARED_H__ */
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH V3 RFC] xfsprogs: remove stubbed-out kernel functions out from xfs_shared.h
2021-11-05 16:40 ` Eric Sandeen
@ 2021-11-07 22:58 ` Dave Chinner
0 siblings, 0 replies; 11+ messages in thread
From: Dave Chinner @ 2021-11-07 22:58 UTC (permalink / raw)
To: Eric Sandeen; +Cc: sandeen, xfs, Darrick J. Wong
On Fri, Nov 05, 2021 at 11:40:57AM -0500, Eric Sandeen wrote:
> On 11/4/21 5:38 PM, Dave Chinner wrote:
>
> > With those changes, we end up with some new stubs in libxfs_priv.h
> > and two places where we need #ifdef __KERNEL__ in xfs_ag.[ch]. Most
> > of the mess in this patch goes away....
> >
> > Cheers,
> >
> > Dave.
> Ok.
>
> I will split this up into the right patch granularity, but is this the
> endpoint you're looking for? One #ifdef in each of xfs_ag.[ch], two total.
Yup.
> The delayed work init/cancel assymmetry is a little odd, but I'll
> get over it.
>
> diff --git a/libxfs/libxfs_priv.h b/libxfs/libxfs_priv.h
> index 15bae1ff..2ca3b9b2 100644
> --- a/libxfs/libxfs_priv.h
> +++ b/libxfs/libxfs_priv.h
> @@ -673,6 +673,9 @@ static inline void xfs_iunlink_destroy(struct xfs_perag *pag) { }
> xfs_agnumber_t xfs_set_inode_alloc(struct xfs_mount *mp,
> xfs_agnumber_t agcount);
> +/* Faked up kernel bits */
> +#define cancel_delayed_work_sync(work) do { } while(0)
Comment is completely redundant. libxfs_priv.h is entirely for
"faked up kernel bits".
I'd also put this up near the top of the file near the definition of
struct iomap, not place it randomly in amongst a bunch of XFS
definitions.
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2021-11-07 22:58 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-04 2:21 [PATCH] xfsprogs: move stubbed-out kernel functions out of xfs_shared.h Eric Sandeen
2021-11-04 2:28 ` Darrick J. Wong
2021-11-04 2:55 ` Eric Sandeen
2021-11-04 2:59 ` [PATCH V2] " Eric Sandeen
2021-11-04 3:14 ` Darrick J. Wong
2021-11-04 3:33 ` Eric Sandeen
2021-11-04 17:15 ` [PATCH V3 RFC] xfsprogs: remove stubbed-out kernel functions out from xfs_shared.h Eric Sandeen
2021-11-04 19:08 ` Darrick J. Wong
2021-11-04 22:38 ` Dave Chinner
2021-11-05 16:40 ` Eric Sandeen
2021-11-07 22:58 ` Dave Chinner
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.