Re: [PATCH 10/12] bcache: move closures to lib/
On Mon, Jun 10, 2019 at 03:14:18PM -0400, Kent Overstreet wrote: > Prep work for bcachefs - being a fork of bcache it also uses closures NAK. This obsfucation needs to go away from bcache and not actually be spread further, especially not as an API with multiple users which will make it even harder to get rid of it.
Re: [PATCH 10/12] bcache: move closures to lib/
On Thu, Jun 13, 2019 at 12:28:41AM -0700, Christoph Hellwig wrote: > On Mon, Jun 10, 2019 at 03:14:18PM -0400, Kent Overstreet wrote: > > Prep work for bcachefs - being a fork of bcache it also uses closures > > NAK. This obsfucation needs to go away from bcache and not actually be > spread further, especially not as an API with multiple users which will > make it even harder to get rid of it. Christoph, you've made it plenty clear how much you dislike closures in the past but "I don't like it" is not remotely the kind of objection that is appropriate or useful for technical discussions, and that's pretty much all you've ever given. If you really think that code should be gotten rid of, then maybe you should actually _look at what they do that's not covered by other kernel infrastructure_ and figure out something better. Otherwise... no one else seems to care all that much about closures, and you're not actually giving any technical feedback, so I'm not sure what you expect.
Re: [PATCH 10/12] bcache: move closures to lib/
On 2019/6/11 3:14 上午, Kent Overstreet wrote:
> Prep work for bcachefs - being a fork of bcache it also uses closures
>
> Signed-off-by: Kent Overstreet
Acked-by: Coly Li
Thanks.
Coly Li
> ---
> drivers/md/bcache/Kconfig | 10 +--
> drivers/md/bcache/Makefile| 6 ++--
> drivers/md/bcache/bcache.h| 2 +-
> drivers/md/bcache/super.c | 1 -
> drivers/md/bcache/util.h | 3 +-
> .../md/bcache => include/linux}/closure.h | 17 ++-
> lib/Kconfig | 3 ++
> lib/Kconfig.debug | 9 ++
> lib/Makefile | 2 ++
> {drivers/md/bcache => lib}/closure.c | 28 ++-
> 10 files changed, 37 insertions(+), 44 deletions(-)
> rename {drivers/md/bcache => include/linux}/closure.h (97%)
> rename {drivers/md/bcache => lib}/closure.c (89%)
>
> diff --git a/drivers/md/bcache/Kconfig b/drivers/md/bcache/Kconfig
> index f6e0a8b3a6..3dd1d48987 100644
> --- a/drivers/md/bcache/Kconfig
> +++ b/drivers/md/bcache/Kconfig
> @@ -2,6 +2,7 @@
> config BCACHE
> tristate "Block device as cache"
> select CRC64
> + select CLOSURES
> help
> Allows a block device to be used as cache for other devices; uses
> a btree for indexing and the layout is optimized for SSDs.
> @@ -16,12 +17,3 @@ config BCACHE_DEBUG
>
> Enables extra debugging tools, allows expensive runtime checks to be
> turned on.
> -
> -config BCACHE_CLOSURES_DEBUG
> - bool "Debug closures"
> - depends on BCACHE
> - select DEBUG_FS
> - help
> - Keeps all active closures in a linked list and provides a debugfs
> - interface to list them, which makes it possible to see asynchronous
> - operations that get stuck.
> diff --git a/drivers/md/bcache/Makefile b/drivers/md/bcache/Makefile
> index d26b351958..2b790fb813 100644
> --- a/drivers/md/bcache/Makefile
> +++ b/drivers/md/bcache/Makefile
> @@ -2,8 +2,8 @@
>
> obj-$(CONFIG_BCACHE) += bcache.o
>
> -bcache-y := alloc.o bset.o btree.o closure.o debug.o extents.o\
> - io.o journal.o movinggc.o request.o stats.o super.o sysfs.o trace.o\
> - util.o writeback.o
> +bcache-y := alloc.o bset.o btree.o debug.o extents.o io.o\
> + journal.o movinggc.o request.o stats.o super.o sysfs.o trace.o util.o\
> + writeback.o
>
> CFLAGS_request.o += -Iblock
> diff --git a/drivers/md/bcache/bcache.h b/drivers/md/bcache/bcache.h
> index fdf75352e1..ced9f1526c 100644
> --- a/drivers/md/bcache/bcache.h
> +++ b/drivers/md/bcache/bcache.h
> @@ -180,6 +180,7 @@
>
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -192,7 +193,6 @@
>
> #include "bset.h"
> #include "util.h"
> -#include "closure.h"
>
> struct bucket {
> atomic_tpin;
> diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
> index a697a3a923..da6803f280 100644
> --- a/drivers/md/bcache/super.c
> +++ b/drivers/md/bcache/super.c
> @@ -2487,7 +2487,6 @@ static int __init bcache_init(void)
> goto err;
>
> bch_debug_init();
> - closure_debug_init();
>
> return 0;
> err:
> diff --git a/drivers/md/bcache/util.h b/drivers/md/bcache/util.h
> index 00aab6abcf..8a75100c0b 100644
> --- a/drivers/md/bcache/util.h
> +++ b/drivers/md/bcache/util.h
> @@ -4,6 +4,7 @@
> #define _BCACHE_UTIL_H
>
> #include
> +#include
> #include
> #include
> #include
> @@ -13,8 +14,6 @@
> #include
> #include
>
> -#include "closure.h"
> -
> #define PAGE_SECTORS (PAGE_SIZE / 512)
>
> struct closure;
> diff --git a/drivers/md/bcache/closure.h b/include/linux/closure.h
> similarity index 97%
> rename from drivers/md/bcache/closure.h
> rename to include/linux/closure.h
> index 376c5e659c..308e38028c 100644
> --- a/drivers/md/bcache/closure.h
> +++ b/include/linux/closure.h
> @@ -155,7 +155,7 @@ struct closure {
>
> atomic_tremaining;
>
> -#ifdef CONFIG_BCACHE_CLOSURES_DEBUG
> +#ifdef CONFIG_DEBUG_CLOSURES
> #define CLOSURE_MAGIC_DEAD 0xc054dead
> #define CLOSURE_MAGIC_ALIVE 0xc054a11e
>
> @@ -184,15 +184,13 @@ static inline void closure_sync(struct closure *cl)
> __closure_sync(cl);
> }
>
> -#ifdef CONFIG_BCACHE_CLOSURES_DEBUG
> +#ifdef CONFIG_DEBUG_CLOSURES
>
> -void closure_debug_init(void);
> void closure_debug_create(struct closure *cl);
> void closure_debug_destroy(struct closure *cl);
>
> #else
>
> -static inline void closure_debug_init(void) {}
> static inline void closure_debug_create(struct closure *cl) {}
> static inline void closure_debug_destroy(struct closure *cl) {}
>
> @@ -200,21 +198,21 @@ static inline void closure_debug_destroy(struct closure
> *cl) {}
>
> static inline void closure_set_ip(struct closure *cl)
> {
> -#ifdef CONFIG_BCACHE_CLOSURES_DEBUG
>
[PATCH 10/12] bcache: move closures to lib/
Prep work for bcachefs - being a fork of bcache it also uses closures
Signed-off-by: Kent Overstreet
---
drivers/md/bcache/Kconfig | 10 +--
drivers/md/bcache/Makefile| 6 ++--
drivers/md/bcache/bcache.h| 2 +-
drivers/md/bcache/super.c | 1 -
drivers/md/bcache/util.h | 3 +-
.../md/bcache => include/linux}/closure.h | 17 ++-
lib/Kconfig | 3 ++
lib/Kconfig.debug | 9 ++
lib/Makefile | 2 ++
{drivers/md/bcache => lib}/closure.c | 28 ++-
10 files changed, 37 insertions(+), 44 deletions(-)
rename {drivers/md/bcache => include/linux}/closure.h (97%)
rename {drivers/md/bcache => lib}/closure.c (89%)
diff --git a/drivers/md/bcache/Kconfig b/drivers/md/bcache/Kconfig
index f6e0a8b3a6..3dd1d48987 100644
--- a/drivers/md/bcache/Kconfig
+++ b/drivers/md/bcache/Kconfig
@@ -2,6 +2,7 @@
config BCACHE
tristate "Block device as cache"
select CRC64
+ select CLOSURES
help
Allows a block device to be used as cache for other devices; uses
a btree for indexing and the layout is optimized for SSDs.
@@ -16,12 +17,3 @@ config BCACHE_DEBUG
Enables extra debugging tools, allows expensive runtime checks to be
turned on.
-
-config BCACHE_CLOSURES_DEBUG
- bool "Debug closures"
- depends on BCACHE
- select DEBUG_FS
- help
- Keeps all active closures in a linked list and provides a debugfs
- interface to list them, which makes it possible to see asynchronous
- operations that get stuck.
diff --git a/drivers/md/bcache/Makefile b/drivers/md/bcache/Makefile
index d26b351958..2b790fb813 100644
--- a/drivers/md/bcache/Makefile
+++ b/drivers/md/bcache/Makefile
@@ -2,8 +2,8 @@
obj-$(CONFIG_BCACHE) += bcache.o
-bcache-y := alloc.o bset.o btree.o closure.o debug.o extents.o\
- io.o journal.o movinggc.o request.o stats.o super.o sysfs.o trace.o\
- util.o writeback.o
+bcache-y := alloc.o bset.o btree.o debug.o extents.o io.o\
+ journal.o movinggc.o request.o stats.o super.o sysfs.o trace.o util.o\
+ writeback.o
CFLAGS_request.o += -Iblock
diff --git a/drivers/md/bcache/bcache.h b/drivers/md/bcache/bcache.h
index fdf75352e1..ced9f1526c 100644
--- a/drivers/md/bcache/bcache.h
+++ b/drivers/md/bcache/bcache.h
@@ -180,6 +180,7 @@
#include
#include
+#include
#include
#include
#include
@@ -192,7 +193,6 @@
#include "bset.h"
#include "util.h"
-#include "closure.h"
struct bucket {
atomic_tpin;
diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
index a697a3a923..da6803f280 100644
--- a/drivers/md/bcache/super.c
+++ b/drivers/md/bcache/super.c
@@ -2487,7 +2487,6 @@ static int __init bcache_init(void)
goto err;
bch_debug_init();
- closure_debug_init();
return 0;
err:
diff --git a/drivers/md/bcache/util.h b/drivers/md/bcache/util.h
index 00aab6abcf..8a75100c0b 100644
--- a/drivers/md/bcache/util.h
+++ b/drivers/md/bcache/util.h
@@ -4,6 +4,7 @@
#define _BCACHE_UTIL_H
#include
+#include
#include
#include
#include
@@ -13,8 +14,6 @@
#include
#include
-#include "closure.h"
-
#define PAGE_SECTORS (PAGE_SIZE / 512)
struct closure;
diff --git a/drivers/md/bcache/closure.h b/include/linux/closure.h
similarity index 97%
rename from drivers/md/bcache/closure.h
rename to include/linux/closure.h
index 376c5e659c..308e38028c 100644
--- a/drivers/md/bcache/closure.h
+++ b/include/linux/closure.h
@@ -155,7 +155,7 @@ struct closure {
atomic_tremaining;
-#ifdef CONFIG_BCACHE_CLOSURES_DEBUG
+#ifdef CONFIG_DEBUG_CLOSURES
#define CLOSURE_MAGIC_DEAD 0xc054dead
#define CLOSURE_MAGIC_ALIVE0xc054a11e
@@ -184,15 +184,13 @@ static inline void closure_sync(struct closure *cl)
__closure_sync(cl);
}
-#ifdef CONFIG_BCACHE_CLOSURES_DEBUG
+#ifdef CONFIG_DEBUG_CLOSURES
-void closure_debug_init(void);
void closure_debug_create(struct closure *cl);
void closure_debug_destroy(struct closure *cl);
#else
-static inline void closure_debug_init(void) {}
static inline void closure_debug_create(struct closure *cl) {}
static inline void closure_debug_destroy(struct closure *cl) {}
@@ -200,21 +198,21 @@ static inline void closure_debug_destroy(struct closure
*cl) {}
static inline void closure_set_ip(struct closure *cl)
{
-#ifdef CONFIG_BCACHE_CLOSURES_DEBUG
+#ifdef CONFIG_DEBUG_CLOSURES
cl->ip = _THIS_IP_;
#endif
}
static inline void closure_set_ret_ip(struct closure *cl)
{
-#ifdef CONFIG_BCACHE_CLOSURES_DEBUG
+#ifdef CONFIG_DEBUG_CLOSURES
cl->ip = _RET_IP_;
#endif
}
static inline void closure_set_waiting(struct closure *cl, unsigned long f)
{
-#ifdef
