Hi all,
After merging the vfs tree, today's linux-next build (x86_64 allnoconfig)
failed like this:
fs/inode.c:1615:5: error: redefinition of 'bmap'
1615 | int bmap(struct inode *inode, sector_t *block)
| ^~~~
In file included from fs/inode.c:7:
include/linux/fs.h:2867:19: note: previous definition of 'bmap' was here
2867 | static inline int bmap(struct inode *inode, sector_t *block)
| ^~~~
Caused by commit
65a805fdd75f ("fibmap: Use bmap instead of ->bmap method in ioctl_fibmap")
I have added this patch for today:
From: Stephen Rothwell <[email protected]>
Date: Fri, 10 Jan 2020 17:53:19 +1100
Subject: [PATCH] fs: fix up for !CONFIG_BLOCK and bmap
Signed-off-by: Stephen Rothwell <[email protected]>
---
fs/inode.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/fs/inode.c b/fs/inode.c
index 9f894b25af2b..590f36daa006 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -1598,6 +1598,7 @@ void iput(struct inode *inode)
}
EXPORT_SYMBOL(iput);
+#ifdef CONFIG_BLOCK
/**
* bmap - find a block number in a file
* @inode: inode owning the block number being requested
@@ -1621,6 +1622,7 @@ int bmap(struct inode *inode, sector_t *block)
return 0;
}
EXPORT_SYMBOL(bmap);
+#endif
/*
* With relative atime, only update atime if the previous atime is
--
2.24.0
--
Cheers,
Stephen Rothwell
Meh...
On Fri, Jan 10, 2020 at 05:57:29PM +1100, Stephen Rothwell wrote:
> Hi all,
>
> After merging the vfs tree, today's linux-next build (x86_64 allnoconfig)
> failed like this:
>
> fs/inode.c:1615:5: error: redefinition of 'bmap'
> 1615 | int bmap(struct inode *inode, sector_t *block)
> | ^~~~
> In file included from fs/inode.c:7:
> include/linux/fs.h:2867:19: note: previous definition of 'bmap' was here
> 2867 | static inline int bmap(struct inode *inode, sector_t *block)
> | ^~~~
>
> Caused by commit
>
> 65a805fdd75f ("fibmap: Use bmap instead of ->bmap method in ioctl_fibmap")
>
> I have added this patch for today:
I had a fix to it, in one of my previous patches but I forgot to add it to this
version.
>
> From: Stephen Rothwell <[email protected]>
> Date: Fri, 10 Jan 2020 17:53:19 +1100
> Subject: [PATCH] fs: fix up for !CONFIG_BLOCK and bmap
>
> Signed-off-by: Stephen Rothwell <[email protected]>
> ---
> fs/inode.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/fs/inode.c b/fs/inode.c
> index 9f894b25af2b..590f36daa006 100644
> --- a/fs/inode.c
> +++ b/fs/inode.c
> @@ -1598,6 +1598,7 @@ void iput(struct inode *inode)
> }
> EXPORT_SYMBOL(iput);
>
> +#ifdef CONFIG_BLOCK
> /**
> * bmap - find a block number in a file
> * @inode: inode owning the block number being requested
> @@ -1621,6 +1622,7 @@ int bmap(struct inode *inode, sector_t *block)
> return 0;
> }
> EXPORT_SYMBOL(bmap);
> +#endif
The problem with this fix, is the fact bmap() could still be called for some
users even withouth CONFIG_BLOCK, so, it needs to have a dummy function to
return -EINVAL in case CONFIG_BLOCK is disabled.
I'll send a patch to fix it. Thanks for spotting it Stephen.
>
> /*
> * With relative atime, only update atime if the previous atime is
> --
> 2.24.0
>
> --
> Cheers,
> Stephen Rothwell
--
Carlos
On Fri, Jan 10, 2020 at 05:57:29PM +1100, Stephen Rothwell wrote:
> Hi all,
>
> After merging the vfs tree, today's linux-next build (x86_64 allnoconfig)
> failed like this:
>
> fs/inode.c:1615:5: error: redefinition of 'bmap'
> 1615 | int bmap(struct inode *inode, sector_t *block)
> | ^~~~
> In file included from fs/inode.c:7:
> include/linux/fs.h:2867:19: note: previous definition of 'bmap' was here
> 2867 | static inline int bmap(struct inode *inode, sector_t *block)
> | ^~~~
>
Oh, no, that's not the same issue I thought, and the patch applied does have the
dummy function.
/me grabs more coffee...
> ---
> fs/inode.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/fs/inode.c b/fs/inode.c
> index 9f894b25af2b..590f36daa006 100644
> --- a/fs/inode.c
> +++ b/fs/inode.c
> @@ -1598,6 +1598,7 @@ void iput(struct inode *inode)
> }
> EXPORT_SYMBOL(iput);
>
> +#ifdef CONFIG_BLOCK
> /**
> * bmap - find a block number in a file
> * @inode: inode owning the block number being requested
> @@ -1621,6 +1622,7 @@ int bmap(struct inode *inode, sector_t *block)
> return 0;
> }
> EXPORT_SYMBOL(bmap);
> +#endif
Eitherway, I am not 100% sure this is the right fix for this case, I remember
some bmap() users who didn't need CONFIG_BLOCK, so we may still need to export
it without CONFIG_BLOCK.
Can you please send me your configuration?
Thanks.
>
> /*
> * With relative atime, only update atime if the previous atime is
> --
> 2.24.0
>
> --
> Cheers,
> Stephen Rothwell
--
Carlos
Hi Carlos,
On Fri, 10 Jan 2020 12:03:53 +0100 Carlos Maiolino <[email protected]> wrote:
>
> Eitherway, I am not 100% sure this is the right fix for this case, I remember
> some bmap() users who didn't need CONFIG_BLOCK, so we may still need to export
> it without CONFIG_BLOCK.
> Can you please send me your configuration?
It was a x86_64 allnoconfig build.
--
Cheers,
Stephen Rothwell
On Sat, Jan 11, 2020 at 09:44:27AM +1100, Stephen Rothwell wrote:
> Hi Carlos,
>
> On Fri, 10 Jan 2020 12:03:53 +0100 Carlos Maiolino <[email protected]> wrote:
> >
> > Eitherway, I am not 100% sure this is the right fix for this case, I remember
> > some bmap() users who didn't need CONFIG_BLOCK, so we may still need to export
> > it without CONFIG_BLOCK.
> > Can you please send me your configuration?
>
> It was a x86_64 allnoconfig build.
Thanks for the info Stephen.
I think the correct way to fix this though, is to wrap the whole bmap(){}
definition in a #ifdef block, not only the EXPORT symbol, as, by my patches, we
redefine bmap() as an inline symbol if CONFIG_BLOCK is not set. So, something
like this:
diff --git a/fs/inode.c b/fs/inode.c
index 9f894b25af2b..21e58542801b 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -1612,6 +1612,8 @@ EXPORT_SYMBOL(iput);
* Returns -EINVAL in case of error, 0 otherwise. If mapping falls into a
* hole, returns 0 and *block is also set to 0.
*/
+
+#ifdef CONFIG_BLOCK
int bmap(struct inode *inode, sector_t *block)
{
if (!inode->i_mapping->a_ops->bmap)
@@ -1621,6 +1623,7 @@ int bmap(struct inode *inode, sector_t *block)
return 0;
}
EXPORT_SYMBOL(bmap);
+#endif
/*
* With relative atime, only update atime if the previous atime is
So, we preserve the original inline definition in include/fs.h (making bmap()
just returning -EINVAL). What do you think?
Viro, mind to share your opinion? I can send a 'Fixes:' patch.
Cheers
--
Carlos
Hi all,
On Fri, 10 Jan 2020 17:57:29 +1100 Stephen Rothwell <[email protected]> wrote:
>
> After merging the vfs tree, today's linux-next build (x86_64 allnoconfig)
> failed like this:
>
> fs/inode.c:1615:5: error: redefinition of 'bmap'
> 1615 | int bmap(struct inode *inode, sector_t *block)
> | ^~~~
> In file included from fs/inode.c:7:
> include/linux/fs.h:2867:19: note: previous definition of 'bmap' was here
> 2867 | static inline int bmap(struct inode *inode, sector_t *block)
> | ^~~~
>
> Caused by commit
>
> 65a805fdd75f ("fibmap: Use bmap instead of ->bmap method in ioctl_fibmap")
>
> I have added this patch for today:
>
> From: Stephen Rothwell <[email protected]>
> Date: Fri, 10 Jan 2020 17:53:19 +1100
> Subject: [PATCH] fs: fix up for !CONFIG_BLOCK and bmap
>
> Signed-off-by: Stephen Rothwell <[email protected]>
> ---
> fs/inode.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/fs/inode.c b/fs/inode.c
> index 9f894b25af2b..590f36daa006 100644
> --- a/fs/inode.c
> +++ b/fs/inode.c
> @@ -1598,6 +1598,7 @@ void iput(struct inode *inode)
> }
> EXPORT_SYMBOL(iput);
>
> +#ifdef CONFIG_BLOCK
> /**
> * bmap - find a block number in a file
> * @inode: inode owning the block number being requested
> @@ -1621,6 +1622,7 @@ int bmap(struct inode *inode, sector_t *block)
> return 0;
> }
> EXPORT_SYMBOL(bmap);
> +#endif
>
> /*
> * With relative atime, only update atime if the previous atime is
> --
> 2.24.0
I am still applying this patch each day ...
--
Cheers,
Stephen Rothwell
Hi all,
On Fri, 24 Jan 2020 13:41:24 +1100 Stephen Rothwell <[email protected]> wrote:
>
> On Fri, 10 Jan 2020 17:57:29 +1100 Stephen Rothwell <[email protected]> wrote:
> >
> > After merging the vfs tree, today's linux-next build (x86_64 allnoconfig)
> > failed like this:
> >
> > fs/inode.c:1615:5: error: redefinition of 'bmap'
> > 1615 | int bmap(struct inode *inode, sector_t *block)
> > | ^~~~
> > In file included from fs/inode.c:7:
> > include/linux/fs.h:2867:19: note: previous definition of 'bmap' was here
> > 2867 | static inline int bmap(struct inode *inode, sector_t *block)
> > | ^~~~
> >
> > Caused by commit
> >
> > 65a805fdd75f ("fibmap: Use bmap instead of ->bmap method in ioctl_fibmap")
> >
> > I have added this patch for today:
> >
> > From: Stephen Rothwell <[email protected]>
> > Date: Fri, 10 Jan 2020 17:53:19 +1100
> > Subject: [PATCH] fs: fix up for !CONFIG_BLOCK and bmap
> >
> > Signed-off-by: Stephen Rothwell <[email protected]>
> > ---
> > fs/inode.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/fs/inode.c b/fs/inode.c
> > index 9f894b25af2b..590f36daa006 100644
> > --- a/fs/inode.c
> > +++ b/fs/inode.c
> > @@ -1598,6 +1598,7 @@ void iput(struct inode *inode)
> > }
> > EXPORT_SYMBOL(iput);
> >
> > +#ifdef CONFIG_BLOCK
> > /**
> > * bmap - find a block number in a file
> > * @inode: inode owning the block number being requested
> > @@ -1621,6 +1622,7 @@ int bmap(struct inode *inode, sector_t *block)
> > return 0;
> > }
> > EXPORT_SYMBOL(bmap);
> > +#endif
> >
> > /*
> > * With relative atime, only update atime if the previous atime is
> > --
> > 2.24.0
>
> I am still applying this patch each day ...
Ping?
--
Cheers,
Stephen Rothwell