2013-07-30 00:30:37

by Murphy Zhou

[permalink] [raw]
Subject: [PATCH v2] staging/lustre: lloop depends on BLOCK

From: Xiong Zhou <[email protected]>

In the lustre client driver, lloop depends on BLOCK. Add an
option for this dependence. Last version of this patch makes
LUSTRE_FS depends on BLOCK.
Remove unnecessary jdb head files which depends on BLOCK.

Signed-off-by: Xiong Zhou <[email protected]>
---
drivers/staging/lustre/lustre/Kconfig | 7 ++++++-
drivers/staging/lustre/lustre/fld/fld_cache.c | 1 -
drivers/staging/lustre/lustre/fld/fld_request.c | 1 -
.../lustre/lustre/include/linux/lustre_compat25.h | 2 ++
drivers/staging/lustre/lustre/llite/Makefile | 2 +-
drivers/staging/lustre/lustre/lvfs/fsfilt.c | 1 -
6 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/lustre/lustre/Kconfig b/drivers/staging/lustre/lustre/Kconfig
index 0b45de0..4e898e4 100644
--- a/drivers/staging/lustre/lustre/Kconfig
+++ b/drivers/staging/lustre/lustre/Kconfig
@@ -1,6 +1,6 @@
config LUSTRE_FS
tristate "Lustre file system client support"
- depends on STAGING && INET && BLOCK && m
+ depends on INET && m
select LNET
select CRYPTO
select CRYPTO_CRC32
@@ -53,3 +53,8 @@ config LUSTRE_TRANSLATE_ERRNOS
bool
depends on LUSTRE_FS && !X86
default true
+
+config LUSTRE_LLITE_LLOOP
+ bool "Lustre virtual block device"
+ depends on LUSTRE_FS && BLOCK
+ default m
diff --git a/drivers/staging/lustre/lustre/fld/fld_cache.c b/drivers/staging/lustre/lustre/fld/fld_cache.c
index 347f2ae..8410107 100644
--- a/drivers/staging/lustre/lustre/fld/fld_cache.c
+++ b/drivers/staging/lustre/lustre/fld/fld_cache.c
@@ -45,7 +45,6 @@

# include <linux/libcfs/libcfs.h>
# include <linux/module.h>
-# include <linux/jbd.h>
# include <asm/div64.h>

#include <obd.h>
diff --git a/drivers/staging/lustre/lustre/fld/fld_request.c b/drivers/staging/lustre/lustre/fld/fld_request.c
index c99b945..049322a 100644
--- a/drivers/staging/lustre/lustre/fld/fld_request.c
+++ b/drivers/staging/lustre/lustre/fld/fld_request.c
@@ -44,7 +44,6 @@

# include <linux/libcfs/libcfs.h>
# include <linux/module.h>
-# include <linux/jbd.h>
# include <asm/div64.h>

#include <obd.h>
diff --git a/drivers/staging/lustre/lustre/include/linux/lustre_compat25.h b/drivers/staging/lustre/lustre/include/linux/lustre_compat25.h
index 426533b..c0156e3 100644
--- a/drivers/staging/lustre/lustre/include/linux/lustre_compat25.h
+++ b/drivers/staging/lustre/lustre/include/linux/lustre_compat25.h
@@ -111,12 +111,14 @@ static inline void ll_set_fs_pwd(struct fs_struct *fs, struct vfsmount *mnt,
#define TREE_READ_LOCK_IRQ(mapping) spin_lock_irq(&(mapping)->tree_lock)
#define TREE_READ_UNLOCK_IRQ(mapping) spin_unlock_irq(&(mapping)->tree_lock)

+#ifdef CONFIG_BLOCK
static inline
int ll_unregister_blkdev(unsigned int dev, const char *name)
{
unregister_blkdev(dev, name);
return 0;
}
+#endif

#define ll_invalidate_bdev(a,b) invalidate_bdev((a))

diff --git a/drivers/staging/lustre/lustre/llite/Makefile b/drivers/staging/lustre/lustre/llite/Makefile
index dff0c04..f493e07 100644
--- a/drivers/staging/lustre/lustre/llite/Makefile
+++ b/drivers/staging/lustre/lustre/llite/Makefile
@@ -1,5 +1,5 @@
obj-$(CONFIG_LUSTRE_FS) += lustre.o
-obj-$(CONFIG_LUSTRE_FS) += llite_lloop.o
+obj-$(CONFIG_LUSTRE_LLITE_LLOOP) += llite_lloop.o
lustre-y := dcache.o dir.o file.o llite_close.o llite_lib.o llite_nfs.o \
rw.o lproc_llite.o namei.o symlink.o llite_mmap.o \
xattr.o remote_perm.o llite_rmtacl.o llite_capa.o \
diff --git a/drivers/staging/lustre/lustre/lvfs/fsfilt.c b/drivers/staging/lustre/lustre/lvfs/fsfilt.c
index 064445c..ce71f80 100644
--- a/drivers/staging/lustre/lustre/lvfs/fsfilt.c
+++ b/drivers/staging/lustre/lustre/lvfs/fsfilt.c
@@ -35,7 +35,6 @@
#define DEBUG_SUBSYSTEM S_FILTER

#include <linux/fs.h>
-#include <linux/jbd.h>
#include <linux/module.h>
#include <linux/kmod.h>
#include <linux/slab.h>


2013-07-30 07:45:04

by Peng Tao

[permalink] [raw]
Subject: Re: [PATCH v2] staging/lustre: lloop depends on BLOCK

On Tue, Jul 30, 2013 at 8:29 AM, Xiong Zhou <[email protected]> wrote:
> From: Xiong Zhou <[email protected]>
>
> In the lustre client driver, lloop depends on BLOCK. Add an
> option for this dependence. Last version of this patch makes
> LUSTRE_FS depends on BLOCK.
> Remove unnecessary jdb head files which depends on BLOCK.
>
Thanks for the patch. One minor comment below. Other than that, you can add
Reviewed-by: Peng Tao <[email protected]>

> Signed-off-by: Xiong Zhou <[email protected]>
> ---
> drivers/staging/lustre/lustre/Kconfig | 7 ++++++-
> drivers/staging/lustre/lustre/fld/fld_cache.c | 1 -
> drivers/staging/lustre/lustre/fld/fld_request.c | 1 -
> .../lustre/lustre/include/linux/lustre_compat25.h | 2 ++
> drivers/staging/lustre/lustre/llite/Makefile | 2 +-
> drivers/staging/lustre/lustre/lvfs/fsfilt.c | 1 -
> 6 files changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/staging/lustre/lustre/Kconfig b/drivers/staging/lustre/lustre/Kconfig
> index 0b45de0..4e898e4 100644
> --- a/drivers/staging/lustre/lustre/Kconfig
> +++ b/drivers/staging/lustre/lustre/Kconfig
> @@ -1,6 +1,6 @@
> config LUSTRE_FS
> tristate "Lustre file system client support"
> - depends on STAGING && INET && BLOCK && m
> + depends on INET && m
> select LNET
> select CRYPTO
> select CRYPTO_CRC32
> @@ -53,3 +53,8 @@ config LUSTRE_TRANSLATE_ERRNOS
> bool
> depends on LUSTRE_FS && !X86
> default true
> +
> +config LUSTRE_LLITE_LLOOP
> + bool "Lustre virtual block device"
> + depends on LUSTRE_FS && BLOCK
> + default m
> diff --git a/drivers/staging/lustre/lustre/fld/fld_cache.c b/drivers/staging/lustre/lustre/fld/fld_cache.c
> index 347f2ae..8410107 100644
> --- a/drivers/staging/lustre/lustre/fld/fld_cache.c
> +++ b/drivers/staging/lustre/lustre/fld/fld_cache.c
> @@ -45,7 +45,6 @@
>
> # include <linux/libcfs/libcfs.h>
> # include <linux/module.h>
> -# include <linux/jbd.h>
> # include <asm/div64.h>
>
> #include <obd.h>
> diff --git a/drivers/staging/lustre/lustre/fld/fld_request.c b/drivers/staging/lustre/lustre/fld/fld_request.c
> index c99b945..049322a 100644
> --- a/drivers/staging/lustre/lustre/fld/fld_request.c
> +++ b/drivers/staging/lustre/lustre/fld/fld_request.c
> @@ -44,7 +44,6 @@
>
> # include <linux/libcfs/libcfs.h>
> # include <linux/module.h>
> -# include <linux/jbd.h>
> # include <asm/div64.h>
>
> #include <obd.h>
> diff --git a/drivers/staging/lustre/lustre/include/linux/lustre_compat25.h b/drivers/staging/lustre/lustre/include/linux/lustre_compat25.h
> index 426533b..c0156e3 100644
> --- a/drivers/staging/lustre/lustre/include/linux/lustre_compat25.h
> +++ b/drivers/staging/lustre/lustre/include/linux/lustre_compat25.h
> @@ -111,12 +111,14 @@ static inline void ll_set_fs_pwd(struct fs_struct *fs, struct vfsmount *mnt,
> #define TREE_READ_LOCK_IRQ(mapping) spin_lock_irq(&(mapping)->tree_lock)
> #define TREE_READ_UNLOCK_IRQ(mapping) spin_unlock_irq(&(mapping)->tree_lock)
>
> +#ifdef CONFIG_BLOCK
> static inline
> int ll_unregister_blkdev(unsigned int dev, const char *name)
It is better to remove the wrapper and just call unregister_blkdev in
lloop.c. The wrapper exists to support old kernels in Lustre master
but doesn't make sense in upstream kernel.

Thanks,
Tao

> {
> unregister_blkdev(dev, name);
> return 0;
> }
> +#endif
>
> #define ll_invalidate_bdev(a,b) invalidate_bdev((a))
>
> diff --git a/drivers/staging/lustre/lustre/llite/Makefile b/drivers/staging/lustre/lustre/llite/Makefile
> index dff0c04..f493e07 100644
> --- a/drivers/staging/lustre/lustre/llite/Makefile
> +++ b/drivers/staging/lustre/lustre/llite/Makefile
> @@ -1,5 +1,5 @@
> obj-$(CONFIG_LUSTRE_FS) += lustre.o
> -obj-$(CONFIG_LUSTRE_FS) += llite_lloop.o
> +obj-$(CONFIG_LUSTRE_LLITE_LLOOP) += llite_lloop.o
> lustre-y := dcache.o dir.o file.o llite_close.o llite_lib.o llite_nfs.o \
> rw.o lproc_llite.o namei.o symlink.o llite_mmap.o \
> xattr.o remote_perm.o llite_rmtacl.o llite_capa.o \
> diff --git a/drivers/staging/lustre/lustre/lvfs/fsfilt.c b/drivers/staging/lustre/lustre/lvfs/fsfilt.c
> index 064445c..ce71f80 100644
> --- a/drivers/staging/lustre/lustre/lvfs/fsfilt.c
> +++ b/drivers/staging/lustre/lustre/lvfs/fsfilt.c
> @@ -35,7 +35,6 @@
> #define DEBUG_SUBSYSTEM S_FILTER
>
> #include <linux/fs.h>
> -#include <linux/jbd.h>
> #include <linux/module.h>
> #include <linux/kmod.h>
> #include <linux/slab.h>

2013-07-31 02:30:59

by Murphy Zhou

[permalink] [raw]
Subject: Re: [PATCH v2] staging/lustre: lloop depends on BLOCK



On Tue, 30 Jul 2013, Peng Tao wrote:

> On Tue, Jul 30, 2013 at 8:29 AM, Xiong Zhou <[email protected]> wrote:
> > From: Xiong Zhou <[email protected]>
> >
> > In the lustre client driver, lloop depends on BLOCK. Add an
> > option for this dependence. Last version of this patch makes
> > LUSTRE_FS depends on BLOCK.
> > Remove unnecessary jdb head files which depends on BLOCK.
> >
> Thanks for the patch. One minor comment below. Other than that, you can add
> Reviewed-by: Peng Tao <[email protected]>
>
> > Signed-off-by: Xiong Zhou <[email protected]>
> > ---
> > drivers/staging/lustre/lustre/Kconfig | 7 ++++++-
> > drivers/staging/lustre/lustre/fld/fld_cache.c | 1 -
> > drivers/staging/lustre/lustre/fld/fld_request.c | 1 -
> > .../lustre/lustre/include/linux/lustre_compat25.h | 2 ++
> > drivers/staging/lustre/lustre/llite/Makefile | 2 +-
> > drivers/staging/lustre/lustre/lvfs/fsfilt.c | 1 -
> > 6 files changed, 9 insertions(+), 5 deletions(-)
> >
> > #define TREE_READ_UNLOCK_IRQ(mapping) spin_unlock_irq(&(mapping)->tree_lock)
> >
> > +#ifdef CONFIG_BLOCK
> > static inline
> > int ll_unregister_blkdev(unsigned int dev, const char *name)
> It is better to remove the wrapper and just call unregister_blkdev in
> lloop.c. The wrapper exists to support old kernels in Lustre master
> but doesn't make sense in upstream kernel.
>
> Thanks,
> Tao
>

Good comment, Thanks for the review.



From: Xiong Zhou <[email protected]>

First version of this patch makes LUSTRE_FS depends on BLOCK.
Second version makes only lloop depends on BLOCK with a config
option for this dependence, and remove unnecessary jdb header
files which depends on BLOCK.
This version removes the wrapper ll_unregister_blkdev which
depends on BLOCK in header and just call unregister_blkdev in
lloop.c based on Peng Tao's comment.

Signed-off-by: Xiong Zhou <[email protected]>
Reviewed-by: Peng Tao <[email protected]>
---
drivers/staging/lustre/lustre/Kconfig | 7 ++++++-
drivers/staging/lustre/lustre/fld/fld_cache.c | 1 -
drivers/staging/lustre/lustre/fld/fld_request.c | 1 -
.../lustre/lustre/include/linux/lustre_compat25.h | 7 -------
drivers/staging/lustre/lustre/llite/Makefile | 2 +-
drivers/staging/lustre/lustre/llite/lloop.c | 6 ++----
drivers/staging/lustre/lustre/lvfs/fsfilt.c | 1 -
7 files changed, 9 insertions(+), 16 deletions(-)

diff --git a/drivers/staging/lustre/lustre/Kconfig b/drivers/staging/lustre/lustre/Kconfig
index 0b45de0..4e898e4 100644
--- a/drivers/staging/lustre/lustre/Kconfig
+++ b/drivers/staging/lustre/lustre/Kconfig
@@ -1,6 +1,6 @@
config LUSTRE_FS
tristate "Lustre file system client support"
- depends on STAGING && INET && BLOCK && m
+ depends on INET && m
select LNET
select CRYPTO
select CRYPTO_CRC32
@@ -53,3 +53,8 @@ config LUSTRE_TRANSLATE_ERRNOS
bool
depends on LUSTRE_FS && !X86
default true
+
+config LUSTRE_LLITE_LLOOP
+ bool "Lustre virtual block device"
+ depends on LUSTRE_FS && BLOCK
+ default m
diff --git a/drivers/staging/lustre/lustre/fld/fld_cache.c b/drivers/staging/lustre/lustre/fld/fld_cache.c
index 347f2ae..8410107 100644
--- a/drivers/staging/lustre/lustre/fld/fld_cache.c
+++ b/drivers/staging/lustre/lustre/fld/fld_cache.c
@@ -45,7 +45,6 @@

# include <linux/libcfs/libcfs.h>
# include <linux/module.h>
-# include <linux/jbd.h>
# include <asm/div64.h>

#include <obd.h>
diff --git a/drivers/staging/lustre/lustre/fld/fld_request.c b/drivers/staging/lustre/lustre/fld/fld_request.c
index c99b945..049322a 100644
--- a/drivers/staging/lustre/lustre/fld/fld_request.c
+++ b/drivers/staging/lustre/lustre/fld/fld_request.c
@@ -44,7 +44,6 @@

# include <linux/libcfs/libcfs.h>
# include <linux/module.h>
-# include <linux/jbd.h>
# include <asm/div64.h>

#include <obd.h>
diff --git a/drivers/staging/lustre/lustre/include/linux/lustre_compat25.h b/drivers/staging/lustre/lustre/include/linux/lustre_compat25.h
index 426533b..018e604 100644
--- a/drivers/staging/lustre/lustre/include/linux/lustre_compat25.h
+++ b/drivers/staging/lustre/lustre/include/linux/lustre_compat25.h
@@ -111,13 +111,6 @@ static inline void ll_set_fs_pwd(struct fs_struct *fs, struct vfsmount *mnt,
#define TREE_READ_LOCK_IRQ(mapping) spin_lock_irq(&(mapping)->tree_lock)
#define TREE_READ_UNLOCK_IRQ(mapping) spin_unlock_irq(&(mapping)->tree_lock)

-static inline
-int ll_unregister_blkdev(unsigned int dev, const char *name)
-{
- unregister_blkdev(dev, name);
- return 0;
-}
-
#define ll_invalidate_bdev(a,b) invalidate_bdev((a))

#ifndef FS_HAS_FIEMAP
diff --git a/drivers/staging/lustre/lustre/llite/Makefile b/drivers/staging/lustre/lustre/llite/Makefile
index dff0c04..f493e07 100644
--- a/drivers/staging/lustre/lustre/llite/Makefile
+++ b/drivers/staging/lustre/lustre/llite/Makefile
@@ -1,5 +1,5 @@
obj-$(CONFIG_LUSTRE_FS) += lustre.o
-obj-$(CONFIG_LUSTRE_FS) += llite_lloop.o
+obj-$(CONFIG_LUSTRE_LLITE_LLOOP) += llite_lloop.o
lustre-y := dcache.o dir.o file.o llite_close.o llite_lib.o llite_nfs.o \
rw.o lproc_llite.o namei.o symlink.o llite_mmap.o \
xattr.o remote_perm.o llite_rmtacl.o llite_capa.o \
diff --git a/drivers/staging/lustre/lustre/llite/lloop.c b/drivers/staging/lustre/lustre/llite/lloop.c
index e063efc..ae3dc9d 100644
--- a/drivers/staging/lustre/lustre/llite/lloop.c
+++ b/drivers/staging/lustre/lustre/llite/lloop.c
@@ -848,10 +848,8 @@ static void lloop_exit(void)
blk_cleanup_queue(loop_dev[i].lo_queue);
put_disk(disks[i]);
}
- if (ll_unregister_blkdev(lloop_major, "lloop"))
- CWARN("lloop: cannot unregister blkdev\n");
- else
- CDEBUG(D_CONFIG, "unregistered lloop major %d\n", lloop_major);
+
+ unregister_blkdev(lloop_major, "lloop");

OBD_FREE(disks, max_loop * sizeof(*disks));
OBD_FREE(loop_dev, max_loop * sizeof(*loop_dev));
diff --git a/drivers/staging/lustre/lustre/lvfs/fsfilt.c b/drivers/staging/lustre/lustre/lvfs/fsfilt.c
index 064445c..ce71f80 100644
--- a/drivers/staging/lustre/lustre/lvfs/fsfilt.c
+++ b/drivers/staging/lustre/lustre/lvfs/fsfilt.c
@@ -35,7 +35,6 @@
#define DEBUG_SUBSYSTEM S_FILTER

#include <linux/fs.h>
-#include <linux/jbd.h>

2013-08-01 07:26:08

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH v2] staging/lustre: lloop depends on BLOCK

On Wed, Jul 31, 2013 at 10:30:41AM +0800, Xiong Zhou wrote:
> From: Xiong Zhou <[email protected]>
>
> First version of this patch makes LUSTRE_FS depends on BLOCK. Second
> version makes only lloop depends on BLOCK with a config option for this
> dependence, and remove unnecessary jdb header files which depends on
> BLOCK.
>
> This version removes the wrapper ll_unregister_blkdev which depends on
> BLOCK in header and just call unregister_blkdev in lloop.c based on Peng
> Tao's comment.

This isn't all needed in the patch changelog info, just say what it
does.

Also, it doesn't apply for some reason, care to refresh this against my
latest tree and resend?

thanks,

greg k-h

2013-08-01 08:45:48

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH v2] staging/lustre: lloop depends on BLOCK

On Tue, Jul 30, 2013 at 08:29:51AM +0800, Xiong Zhou wrote:
> From: Xiong Zhou <[email protected]>
>
> In the lustre client driver, lloop depends on BLOCK. Add an
> option for this dependence. Last version of this patch makes
> LUSTRE_FS depends on BLOCK.
> Remove unnecessary jdb head files which depends on BLOCK.

The driver should be removed, a filesystem has no business bringing
its own loop driver.

2013-08-01 19:57:32

by Dilger, Andreas

[permalink] [raw]
Subject: Re: [PATCH v2] staging/lustre: lloop depends on BLOCK

On 2013/08/01 2:45 AM, "Christoph Hellwig" <[email protected]> wrote:

>On Tue, Jul 30, 2013 at 08:29:51AM +0800, Xiong Zhou wrote:
>> From: Xiong Zhou <[email protected]>
>>
>> In the lustre client driver, lloop depends on BLOCK. Add an
>> option for this dependence. Last version of this patch makes
>> LUSTRE_FS depends on BLOCK.
>> Remove unnecessary jdb head files which depends on BLOCK.
>
>The driver should be removed, a filesystem has no business bringing
>its own loop driver.

It provides significant performance improvement for network IO on Lustre.
It bypasses DLM locking in Lustre and the VFS layer on the client, copying
in the loop driver, and page-by-page IO submission in the normal IO path.

Cheers, Andreas
--
Andreas Dilger

Lustre Software Architect
Intel High Performance Data Division

2013-08-02 02:20:49

by Murphy Zhou

[permalink] [raw]
Subject: Re: [PATCH v2] staging/lustre: lloop depends on BLOCK



On Wed, 31 Jul 2013, Greg Kroah-Hartman wrote:

> On Wed, Jul 31, 2013 at 10:30:41AM +0800, Xiong Zhou wrote:
> > From: Xiong Zhou <[email protected]>
> >
> > First version of this patch makes LUSTRE_FS depends on BLOCK. Second
> > version makes only lloop depends on BLOCK with a config option for this
> > dependence, and remove unnecessary jdb header files which depends on
> > BLOCK.
> >
> > This version removes the wrapper ll_unregister_blkdev which depends on
> > BLOCK in header and just call unregister_blkdev in lloop.c based on Peng
> > Tao's comment.
>
> This isn't all needed in the patch changelog info, just say what it
> does.
>
> Also, it doesn't apply for some reason, care to refresh this against my
> latest tree and resend?
>
> thanks,
>
> greg k-h
>

OK, this patch is based on the staging-next branch of your staging tree.


From: Xiong Zhou <[email protected]>

Add a config option for llite/lloop in lustre driver, making it
depends on BLOCK to fix this better:
drivers/staging/lustre/lustre/fid/../include/linux/lustre_compat25.h:117:2:
error: implicit declaration of function ?unregister_blkdev?

Also, remove the wrapper ll_unregister_blkdev which depends on BLOCK in
the header and just call unregister_blkdev in lloop.c based on Peng Tao's
comment. Drop the redundant dependency on STAGING for LUSTRE_FS, remove
some unnecessary jdb header files which depends on BLOCK btw.

Signed-off-by: Xiong Zhou <[email protected]>
Reviewed-by: Peng Tao <[email protected]>
---
drivers/staging/lustre/lustre/Kconfig | 7 ++++++-
drivers/staging/lustre/lustre/fld/fld_cache.c | 1 -
drivers/staging/lustre/lustre/fld/fld_request.c | 1 -
.../lustre/lustre/include/linux/lustre_compat25.h | 7 -------
drivers/staging/lustre/lustre/llite/Makefile | 2 +-
drivers/staging/lustre/lustre/llite/lloop.c | 6 ++----
drivers/staging/lustre/lustre/lvfs/fsfilt.c | 1 -
7 files changed, 9 insertions(+), 16 deletions(-)

diff --git a/drivers/staging/lustre/lustre/Kconfig b/drivers/staging/lustre/lustre/Kconfig
index 0b45de0..4e898e4 100644
--- a/drivers/staging/lustre/lustre/Kconfig
+++ b/drivers/staging/lustre/lustre/Kconfig
@@ -1,6 +1,6 @@
config LUSTRE_FS
tristate "Lustre file system client support"
- depends on STAGING && INET && BLOCK && m
+ depends on INET && m
select LNET
select CRYPTO
select CRYPTO_CRC32
@@ -53,3 +53,8 @@ config LUSTRE_TRANSLATE_ERRNOS
bool
depends on LUSTRE_FS && !X86
default true
+
+config LUSTRE_LLITE_LLOOP
+ bool "Lustre virtual block device"
+ depends on LUSTRE_FS && BLOCK
+ default m
diff --git a/drivers/staging/lustre/lustre/fld/fld_cache.c b/drivers/staging/lustre/lustre/fld/fld_cache.c
index 347f2ae..8410107 100644
--- a/drivers/staging/lustre/lustre/fld/fld_cache.c
+++ b/drivers/staging/lustre/lustre/fld/fld_cache.c
@@ -45,7 +45,6 @@

# include <linux/libcfs/libcfs.h>
# include <linux/module.h>
-# include <linux/jbd.h>
# include <asm/div64.h>

#include <obd.h>
diff --git a/drivers/staging/lustre/lustre/fld/fld_request.c b/drivers/staging/lustre/lustre/fld/fld_request.c
index c99b945..049322a 100644
--- a/drivers/staging/lustre/lustre/fld/fld_request.c
+++ b/drivers/staging/lustre/lustre/fld/fld_request.c
@@ -44,7 +44,6 @@

# include <linux/libcfs/libcfs.h>
# include <linux/module.h>
-# include <linux/jbd.h>
# include <asm/div64.h>

#include <obd.h>
diff --git a/drivers/staging/lustre/lustre/include/linux/lustre_compat25.h b/drivers/staging/lustre/lustre/include/linux/lustre_compat25.h
index 426533b..018e604 100644
--- a/drivers/staging/lustre/lustre/include/linux/lustre_compat25.h
+++ b/drivers/staging/lustre/lustre/include/linux/lustre_compat25.h
@@ -111,13 +111,6 @@ static inline void ll_set_fs_pwd(struct fs_struct *fs, struct vfsmount *mnt,
#define TREE_READ_LOCK_IRQ(mapping) spin_lock_irq(&(mapping)->tree_lock)
#define TREE_READ_UNLOCK_IRQ(mapping) spin_unlock_irq(&(mapping)->tree_lock)

-static inline
-int ll_unregister_blkdev(unsigned int dev, const char *name)
-{
- unregister_blkdev(dev, name);
- return 0;
-}
-
#define ll_invalidate_bdev(a,b) invalidate_bdev((a))

#ifndef FS_HAS_FIEMAP
diff --git a/drivers/staging/lustre/lustre/llite/Makefile b/drivers/staging/lustre/lustre/llite/Makefile
index dff0c04..f493e07 100644
--- a/drivers/staging/lustre/lustre/llite/Makefile
+++ b/drivers/staging/lustre/lustre/llite/Makefile
@@ -1,5 +1,5 @@
obj-$(CONFIG_LUSTRE_FS) += lustre.o
-obj-$(CONFIG_LUSTRE_FS) += llite_lloop.o
+obj-$(CONFIG_LUSTRE_LLITE_LLOOP) += llite_lloop.o
lustre-y := dcache.o dir.o file.o llite_close.o llite_lib.o llite_nfs.o \
rw.o lproc_llite.o namei.o symlink.o llite_mmap.o \
xattr.o remote_perm.o llite_rmtacl.o llite_capa.o \
diff --git a/drivers/staging/lustre/lustre/llite/lloop.c b/drivers/staging/lustre/lustre/llite/lloop.c
index e063efc..ae3dc9d 100644
--- a/drivers/staging/lustre/lustre/llite/lloop.c
+++ b/drivers/staging/lustre/lustre/llite/lloop.c
@@ -848,10 +848,8 @@ static void lloop_exit(void)
blk_cleanup_queue(loop_dev[i].lo_queue);
put_disk(disks[i]);
}
- if (ll_unregister_blkdev(lloop_major, "lloop"))
- CWARN("lloop: cannot unregister blkdev\n");
- else
- CDEBUG(D_CONFIG, "unregistered lloop major %d\n", lloop_major);
+
+ unregister_blkdev(lloop_major, "lloop");

OBD_FREE(disks, max_loop * sizeof(*disks));
OBD_FREE(loop_dev, max_loop * sizeof(*loop_dev));
diff --git a/drivers/staging/lustre/lustre/lvfs/fsfilt.c b/drivers/staging/lustre/lustre/lvfs/fsfilt.c
index 064445c..ce71f80 100644
--- a/drivers/staging/lustre/lustre/lvfs/fsfilt.c
+++ b/drivers/staging/lustre/lustre/lvfs/fsfilt.c
@@ -35,7 +35,6 @@
#define DEBUG_SUBSYSTEM S_FILTER

#include <linux/fs.h>
-#include <linux/jbd.h>
#include <linux/module.h>
#include <linux/kmod.h>
#include <linux/slab.h>

2013-08-02 03:09:58

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH v2] staging/lustre: lloop depends on BLOCK

On Fri, Aug 02, 2013 at 10:20:28AM +0800, Xiong Zhou wrote:
>
>
> On Wed, 31 Jul 2013, Greg Kroah-Hartman wrote:
>
> > On Wed, Jul 31, 2013 at 10:30:41AM +0800, Xiong Zhou wrote:
> > > From: Xiong Zhou <[email protected]>
> > >
> > > First version of this patch makes LUSTRE_FS depends on BLOCK. Second
> > > version makes only lloop depends on BLOCK with a config option for this
> > > dependence, and remove unnecessary jdb header files which depends on
> > > BLOCK.
> > >
> > > This version removes the wrapper ll_unregister_blkdev which depends on
> > > BLOCK in header and just call unregister_blkdev in lloop.c based on Peng
> > > Tao's comment.
> >
> > This isn't all needed in the patch changelog info, just say what it
> > does.
> >
> > Also, it doesn't apply for some reason, care to refresh this against my
> > latest tree and resend?
> >
> > thanks,
> >
> > greg k-h
> >
>
> OK, this patch is based on the staging-next branch of your staging tree.
>
>
> From: Xiong Zhou <[email protected]>
>
> Add a config option for llite/lloop in lustre driver, making it
> depends on BLOCK to fix this better:
> drivers/staging/lustre/lustre/fid/../include/linux/lustre_compat25.h:117:2:
> error: implicit declaration of function ‘unregister_blkdev’
>
> Also, remove the wrapper ll_unregister_blkdev which depends on BLOCK in
> the header and just call unregister_blkdev in lloop.c based on Peng Tao's
> comment. Drop the redundant dependency on STAGING for LUSTRE_FS, remove
> some unnecessary jdb header files which depends on BLOCK btw.
>
> Signed-off-by: Xiong Zhou <[email protected]>
> Reviewed-by: Peng Tao <[email protected]>

Please resend this so I don't have to edit the body of the email, and
the Subject: line as well.

Also, you have a bunch of trailing whitespace in your changelog
comments, are you sure you want/need that?

thanks,

greg k-h

2013-08-02 10:49:37

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH v2] staging/lustre: lloop depends on BLOCK

On Thu, Aug 01, 2013 at 07:57:22PM +0000, Dilger, Andreas wrote:
> It provides significant performance improvement for network IO on Lustre.
> It bypasses DLM locking in Lustre and the VFS layer on the client, copying
> in the loop driver, and page-by-page IO submission in the normal IO path.

Part of being upstream is improving existing drivers instead of copy and
pasting them. Please take a Look at Shaggys in-kernel direct I/O and
loop improvements and submit any incremental improvements ontop of that
one.

2013-08-07 07:45:42

by Dilger, Andreas

[permalink] [raw]
Subject: Re: [PATCH v2] staging/lustre: lloop depends on BLOCK

On 2013/08/02 4:49 AM, "Christoph Hellwig" <[email protected]> wrote:
>On Thu, Aug 01, 2013 at 07:57:22PM +0000, Dilger, Andreas wrote:
>> It provides significant performance improvement for network IO on
>>Lustre.
>> It bypasses DLM locking in Lustre and the VFS layer on the client,
>>copying
>> in the loop driver, and page-by-page IO submission in the normal IO
>>path.
>
>Part of being upstream is improving existing drivers instead of copy and
>pasting them. Please take a Look at Shaggys in-kernel direct I/O and
>loop improvements and submit any incremental improvements ontop of that
>one.

The problem still remains that the kernel loop driver eventually depends on
a local block device for the pages/bios to be written. The Lustre lloop
driver bypasses the VFS and block layer to generate RPCs from the submitted
pages to RDMA over the network without a data copy.

I wouldn't think that anyone would want a Lustre-specific RPC engine in the
standard loop.c file, nor would it be practical due to symbol dependencies.

I could imagine being able to register do_bio_lustrebacked() as a BIO
submission
routine instead of do_bio_filebacked(), along with some private data to
link
the loop file to the underlying storage (in Lustre's case an object layout
and
a preallocated I for generating the RPC).

How to register this from userspace compared to a normal file-backed loop
might be a bit tricky. Lustre uses its own ioctls to register/deregister
the
device, though I could imagine some kind of per-file(system) method table
for
specifying the underlying bio submission routine.

In any case, rewriting the lloop/loop driver to merge this functionality
is not
high on the priority list compared to other Lustre code that needs to be
cleaned
up before it can be merged into the main kernel tree. Can we leave this
code
as-is for now, and decide whether to rewrite or remove it when we are
closer to
having the rest of the code cleaned up?

Cheers, Andreas
--
Andreas Dilger

Lustre Software Architect
Intel High Performance Data Division

2013-08-08 12:17:55

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH v2] staging/lustre: lloop depends on BLOCK

On Wed, Aug 07, 2013 at 07:45:17AM +0000, Dilger, Andreas wrote:
> The problem still remains that the kernel loop driver eventually depends on
> a local block device for the pages/bios to be written. The Lustre lloop
> driver bypasses the VFS and block layer to generate RPCs from the submitted
> pages to RDMA over the network without a data copy.

No, it doesn't. It still consumes bios just like the regular loop
driver.

Besides missing all kinds of fixes from years of kernel development the
only difference is that it takes a lustre-specific shortcut into the
direct I/O code instead of going through the pagecache.

The patch series I've pointed you to does exactly that in a generic way
and thus superceeds the lloop driver fully.

In case my previous reference was a bit to vague the series starts at:

[PATCH V8 00/33] loop: Issue O_DIRECT aio using bio_vec

please take a look and make sure to review it in case you see any
shortcomings.