Re: [PATCH 2/8] fs: extract simple_pin/release_fs to separate files
On 4/21/20 1:19 PM, Frederic Barrat wrote: diff --git a/drivers/misc/cxl/Kconfig b/drivers/misc/cxl/Kconfig index 39eec9031487..a62795079d9c 100644 --- a/drivers/misc/cxl/Kconfig +++ b/drivers/misc/cxl/Kconfig @@ -19,6 +19,7 @@ config CXL select CXL_BASE select CXL_AFU_DRIVER_OPS select CXL_LIB + select SIMPLEFS default m help Select this option to enable driver support for IBM Coherent diff --git a/drivers/misc/cxl/api.c b/drivers/misc/cxl/api.c index b493de962153..0b8f8de7475a 100644 --- a/drivers/misc/cxl/api.c +++ b/drivers/misc/cxl/api.c @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include diff --git a/drivers/misc/ocxl/Kconfig b/drivers/misc/ocxl/Kconfig index 2d2266c1439e..ddd9245fff3d 100644 --- a/drivers/misc/ocxl/Kconfig +++ b/drivers/misc/ocxl/Kconfig @@ -12,6 +12,7 @@ config OCXL depends on PPC_POWERNV && PCI && EEH select OCXL_BASE select HOTPLUG_PCI_POWERNV + select SIMPLEFS It's not clear to me the Kconfig updated is needed for the ocxl driver. I think it's only needed for the cxl driver. I am going to get rid of the separate simplefs.c file and related Kconfig entry and put everything in fs/libfs.c, so this file (together with many others touched in this patch) won't be modified in v2. Thanks, Emanuele
Re: [PATCH 2/8] fs: extract simple_pin/release_fs to separate files
diff --git a/drivers/misc/cxl/Kconfig b/drivers/misc/cxl/Kconfig index 39eec9031487..a62795079d9c 100644 --- a/drivers/misc/cxl/Kconfig +++ b/drivers/misc/cxl/Kconfig @@ -19,6 +19,7 @@ config CXL select CXL_BASE select CXL_AFU_DRIVER_OPS select CXL_LIB + select SIMPLEFS default m help Select this option to enable driver support for IBM Coherent diff --git a/drivers/misc/cxl/api.c b/drivers/misc/cxl/api.c index b493de962153..0b8f8de7475a 100644 --- a/drivers/misc/cxl/api.c +++ b/drivers/misc/cxl/api.c @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include diff --git a/drivers/misc/ocxl/Kconfig b/drivers/misc/ocxl/Kconfig index 2d2266c1439e..ddd9245fff3d 100644 --- a/drivers/misc/ocxl/Kconfig +++ b/drivers/misc/ocxl/Kconfig @@ -12,6 +12,7 @@ config OCXL depends on PPC_POWERNV && PCI && EEH select OCXL_BASE select HOTPLUG_PCI_POWERNV + select SIMPLEFS It's not clear to me the Kconfig updated is needed for the ocxl driver. I think it's only needed for the cxl driver. Fred
Re: [PATCH 2/8] fs: extract simple_pin/release_fs to separate files
On Tue, Apr 14, 2020 at 02:42:56PM +0200, Emanuele Giuseppe Esposito wrote: > We will augment this family of functions with inode management. To avoid > littering include/linux/fs.h and fs/libfs.c, move them to a separate header, > with a Kconfig symbol to enable them. If there are no functional changes, indicating that on the commit log will make the review much easier. > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug > index d1398cef3b18..fc38a6f0fc11 100644 > --- a/lib/Kconfig.debug > +++ b/lib/Kconfig.debug > @@ -288,12 +288,16 @@ config STRIP_ASM_SYMS > > config READABLE_ASM > bool "Generate readable assembler code" > - depends on DEBUG_KERNEL > - help > - Disable some compiler optimizations that tend to generate human > unreadable > - assembler output. This may make the kernel slightly slower, but it > helps > - to keep kernel developers who have to stare a lot at assembler > listings > - sane. > +depends on DEBUG_KERNEL > +help > + Disable some compiler optimizations that tend to generate human > unreadable > + assembler output. This may make the kernel slightly slower, but it > helps > + to keep kernel developers who have to stare a lot at assembler listings > + sane. > + This minor change above should just be a separate patch. Its just noise otherwise. > +config DEBUG_FS > + bool "Debug Filesystem" > + select SIMPLEFS I'm at a loss reviewing this, my lib/Kconfig.debug already has a config DEBUG_FS. But above I see it is being added for the very first time. I'm sure there is some odd conditional which is obscuring this, can this be explained in the commit log? Luis
Re: [PATCH 2/8] fs: extract simple_pin/release_fs to separate files
On Tue, Apr 14, 2020 at 02:42:56PM +0200, Emanuele Giuseppe Esposito wrote: > We will augment this family of functions with inode management. To avoid > littering include/linux/fs.h and fs/libfs.c, move them to a separate header, > with a Kconfig symbol to enable them. > > Signed-off-by: Emanuele Giuseppe Esposito You have a lot of people on cc:, this is going to be hard for everyone to review... > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug > index d1398cef3b18..fc38a6f0fc11 100644 > --- a/lib/Kconfig.debug > +++ b/lib/Kconfig.debug > @@ -288,12 +288,16 @@ config STRIP_ASM_SYMS > > config READABLE_ASM > bool "Generate readable assembler code" > - depends on DEBUG_KERNEL > - help > - Disable some compiler optimizations that tend to generate human > unreadable > - assembler output. This may make the kernel slightly slower, but it > helps > - to keep kernel developers who have to stare a lot at assembler > listings > - sane. > +depends on DEBUG_KERNEL > +help > + Disable some compiler optimizations that tend to generate human > unreadable > + assembler output. This may make the kernel slightly slower, but it > helps > + to keep kernel developers who have to stare a lot at assembler listings > + sane. > + Why did you loose the indentation here and add trailing whitespace? > +config DEBUG_FS > + bool "Debug Filesystem" > + select SIMPLEFS > We already have a DEBUG_FS config option in this file, why another one? And what happened to the help text? I think you need to rework your patch series to do smaller things on each step, which would make it reviewable much easier, and prevent mistakes like this one. thanks, greg k-h
[PATCH 2/8] fs: extract simple_pin/release_fs to separate files
We will augment this family of functions with inode management. To avoid littering include/linux/fs.h and fs/libfs.c, move them to a separate header, with a Kconfig symbol to enable them. Signed-off-by: Emanuele Giuseppe Esposito --- drivers/gpu/drm/Kconfig | 1 + drivers/gpu/drm/drm_drv.c | 2 +- drivers/misc/cxl/Kconfig| 1 + drivers/misc/cxl/api.c | 1 + drivers/misc/ocxl/Kconfig | 1 + drivers/scsi/cxlflash/ocxl_hw.c | 1 + fs/Kconfig | 3 +++ fs/Kconfig.binfmt | 1 + fs/Makefile | 1 + fs/binfmt_misc.c| 2 +- fs/configfs/Kconfig | 1 + fs/configfs/mount.c | 2 +- fs/debugfs/inode.c | 2 +- fs/libfs.c | 36 +-- fs/simplefs.c | 38 + fs/tracefs/inode.c | 2 +- include/linux/fs.h | 2 -- include/linux/simplefs.h| 10 + lib/Kconfig.debug | 16 -- security/Kconfig| 1 + security/apparmor/apparmorfs.c | 3 ++- security/inode.c| 2 +- 22 files changed, 79 insertions(+), 50 deletions(-) create mode 100644 fs/simplefs.c create mode 100644 include/linux/simplefs.h diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index 43594978958e..a6fe933de9da 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -14,6 +14,7 @@ menuconfig DRM select I2C select I2C_ALGOBIT select DMA_SHARED_BUFFER + select SIMPLEFS select SYNC_FILE help Kernel-level support for the Direct Rendering Infrastructure (DRI) diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index 7b1a628d1f6e..187a61091b5c 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -27,7 +27,7 @@ */ #include -#include +#include #include #include #include diff --git a/drivers/misc/cxl/Kconfig b/drivers/misc/cxl/Kconfig index 39eec9031487..a62795079d9c 100644 --- a/drivers/misc/cxl/Kconfig +++ b/drivers/misc/cxl/Kconfig @@ -19,6 +19,7 @@ config CXL select CXL_BASE select CXL_AFU_DRIVER_OPS select CXL_LIB + select SIMPLEFS default m help Select this option to enable driver support for IBM Coherent diff --git a/drivers/misc/cxl/api.c b/drivers/misc/cxl/api.c index b493de962153..0b8f8de7475a 100644 --- a/drivers/misc/cxl/api.c +++ b/drivers/misc/cxl/api.c @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include diff --git a/drivers/misc/ocxl/Kconfig b/drivers/misc/ocxl/Kconfig index 2d2266c1439e..ddd9245fff3d 100644 --- a/drivers/misc/ocxl/Kconfig +++ b/drivers/misc/ocxl/Kconfig @@ -12,6 +12,7 @@ config OCXL depends on PPC_POWERNV && PCI && EEH select OCXL_BASE select HOTPLUG_PCI_POWERNV + select SIMPLEFS default m help Select this option to enable the ocxl driver for Open diff --git a/drivers/scsi/cxlflash/ocxl_hw.c b/drivers/scsi/cxlflash/ocxl_hw.c index 7018cd802569..429f55651090 100644 --- a/drivers/scsi/cxlflash/ocxl_hw.c +++ b/drivers/scsi/cxlflash/ocxl_hw.c @@ -12,6 +12,7 @@ #include #include #include +#include #include #include #include diff --git a/fs/Kconfig b/fs/Kconfig index f08fbbfafd9a..a6795ae312a2 100644 --- a/fs/Kconfig +++ b/fs/Kconfig @@ -218,6 +218,9 @@ config HUGETLB_PAGE config MEMFD_CREATE def_bool TMPFS || HUGETLBFS +config SIMPLEFS + bool + config ARCH_HAS_GIGANTIC_PAGE bool diff --git a/fs/Kconfig.binfmt b/fs/Kconfig.binfmt index 62dc4f577ba1..af7b765de4d3 100644 --- a/fs/Kconfig.binfmt +++ b/fs/Kconfig.binfmt @@ -176,6 +176,7 @@ config BINFMT_EM86 config BINFMT_MISC tristate "Kernel support for MISC binaries" + select SIMPLEFS ---help--- If you say Y here, it will be possible to plug wrapper-driven binary formats into the kernel. You will like this especially when you use diff --git a/fs/Makefile b/fs/Makefile index 2ce5112b02c8..c5c665984b9e 100644 --- a/fs/Makefile +++ b/fs/Makefile @@ -26,6 +26,7 @@ obj-$(CONFIG_PROC_FS) += proc_namespace.o obj-y += notify/ obj-$(CONFIG_EPOLL)+= eventpoll.o obj-y += anon_inodes.o +obj-$(CONFIG_SIMPLEFS) += simplefs.o obj-$(CONFIG_SIGNALFD) += signalfd.o obj-$(CONFIG_TIMERFD) += timerfd.o obj-$(CONFIG_EVENTFD) += eventfd.o diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c index cdb45829354d..c764110f5f0b 100644 --- a/fs/binfmt_misc.c +++ b/fs/binfmt_misc.c @@ -25,7 +25,7 @@ #include #include #include -#include +#include #include #include "internal.h" diff --git a/fs/configfs/Kconfig b/fs/configfs/Kconfig index 272b64456999..3b461e4e3989 100644 --- a/fs/configfs/Kconfig +++