Hi Benjamin,
After merging the aio tree, today's linux-next build (powerpc
ppc64_defconfig) failed like this:
In file included from include/linux/kernel.h:13:0,
from fs/aio.c:13:
fs/aio.c: In function 'aio_free_ring':
fs/aio.c:188:32: error: 'struct dentry' has no member named 'd_count'
aio_ring_file->f_path.dentry->d_count,
^
include/linux/printk.h:246:38: note: in definition of macro 'pr_debug'
no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
^
Caused by commit 36bc08cc0170 ("s/aio: Add support to aio ring pages
migration") interacting with commit 98474236f72e ("vfs: make the dentry
cache use the lockref infrastructure") from Linus' tree.
I applied the following (probably suboptimal) fix and can carry it as
necessary.
From: Stephen Rothwell <[email protected]>
Date: Fri, 30 Aug 2013 17:49:10 +1000
Subject: [PATCH] aio: fixup for lockref changes
Signed-off-by: Stephen Rothwell <[email protected]>
---
fs/aio.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/aio.c b/fs/aio.c
index 9f783e3..6d00cd9 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -185,7 +185,7 @@ static void aio_free_ring(struct kioctx *ctx)
truncate_setsize(aio_ring_file->f_inode, 0);
pr_debug("pid(%d) i_nlink=%u d_count=%d d_unhashed=%d i_count=%d\n",
current->pid, aio_ring_file->f_inode->i_nlink,
- aio_ring_file->f_path.dentry->d_count,
+ aio_ring_file->f_path.dentry->d_lockref.count,
d_unhashed(aio_ring_file->f_path.dentry),
atomic_read(&aio_ring_file->f_inode->i_count));
fput(aio_ring_file);
--
1.8.4.rc3
--
Cheers,
Stephen Rothwell [email protected]
Hi Stephen,
On Fri, Aug 30, 2013 at 05:55:09PM +1000, Stephen Rothwell wrote:
> Hi Benjamin,
>
> After merging the aio tree, today's linux-next build (powerpc
> ppc64_defconfig) failed like this:
>
> In file included from include/linux/kernel.h:13:0,
> from fs/aio.c:13:
> fs/aio.c: In function 'aio_free_ring':
> fs/aio.c:188:32: error: 'struct dentry' has no member named 'd_count'
> aio_ring_file->f_path.dentry->d_count,
> ^
> include/linux/printk.h:246:38: note: in definition of macro 'pr_debug'
> no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
Ah, this is unnecessary debugging code that got changed. I've committed
the following to my aio-next tree to just delete the code since it is not
required.
-ben
--
"Thought is the essence of where you are now."
commit 79bd1bcf1ab22ea723da7d5854a9e72a350ecbf8
Author: Benjamin LaHaise <[email protected]>
Date: Fri Aug 30 10:22:04 2013 -0400
aio: remove unnecessary debugging from aio_free_ring()
The commit 36bc08cc0170 ("fs/aio: Add support to aio ring pages migration")
added some debugging code that is not required and resulted in a build error
when 98474236f72e ("vfs: make the dentry cache use the lockref infrastructure")
was added to the tree. The code is not required, so just delete it.
Signed-off-by: Benjamin LaHaise <[email protected]>
diff --git a/fs/aio.c b/fs/aio.c
index c3f005d..d0defcb 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -183,11 +183,6 @@ static void aio_free_ring(struct kioctx *ctx)
if (aio_ring_file) {
truncate_setsize(aio_ring_file->f_inode, 0);
- pr_debug("pid(%d) i_nlink=%u d_count=%d d_unhashed=%d i_count=%d\n",
- current->pid, aio_ring_file->f_inode->i_nlink,
- aio_ring_file->f_path.dentry->d_count,
- d_unhashed(aio_ring_file->f_path.dentry),
- atomic_read(&aio_ring_file->f_inode->i_count));
fput(aio_ring_file);
ctx->aio_ring_file = NULL;
}
On Fri, Aug 30, 2013 at 12:55 AM, Stephen Rothwell <[email protected]> wrote:
> - aio_ring_file->f_path.dentry->d_count,
> + aio_ring_file->f_path.dentry->d_lockref.count,
This is wrong. If you really want the dentry count (which I doubt, I
don't see why this code would care _even_ just for a debug printout),
you should use
d_count(aio_ring_file->f_dentry)
instead these days. That will get the count from the right place,
regardless of any lockref issues or anything else (and f_dentry may be
the "old" way to get the dentry, but it's still supported and unlikely
to go away. No point in writing out "f_path.dentry" unless you *want*
to be aware of the fact that f_path has both a dentry and a mnt member
- but most people really don't care about the mnt information).
Linus
On Fri, Aug 30, 2013 at 10:38:25AM -0700, Linus Torvalds wrote:
> On Fri, Aug 30, 2013 at 12:55 AM, Stephen Rothwell <[email protected]> wrote:
> > - aio_ring_file->f_path.dentry->d_count,
> > + aio_ring_file->f_path.dentry->d_lockref.count,
>
> This is wrong. If you really want the dentry count (which I doubt, I
> don't see why this code would care _even_ just for a debug printout),
> you should use
Agreed -- it was just debugging code from initial development of the code,
so I just ended up deleting it instead.
-ben
--
"Thought is the essence of where you are now."
Hi Ben,
On Sat, 16 Jan 2016 09:55:15 +1100 Stephen Rothwell <[email protected]> wrote:
>
> On Fri, 15 Jan 2016 10:18:21 -0500 Benjamin LaHaise <[email protected]> wrote:
> >
> > On Fri, Jan 15, 2016 at 01:25:31AM -0800, Christoph Hellwig wrote:
> > > On Fri, Jan 15, 2016 at 08:23:16PM +1100, Stephen Rothwell wrote:
> > > > Via the aio tree (git://git.kvack.org/~bcrl/aio-next.git#master) added
> > > > in July 2013 at Ben's request. The code was added to the aio tree in
> > > > Jan 12 (my time), but has never been in a published linux-next tree due
> > > > to the above build problem (I back out to the previous days version of
> > > > the aio tree).
> > >
> > > Well, it's code Ben posted a few days ago, which to say it mildly is
> > > rather controversial. It's cetainly not 4.5 material.
> >
> > It still needs the exposure.
>
> If it is not destined for v4.5, then it should not (yet) be in
> linux-next. It should wait until after v4.5-rc1 is released (the merge
> window closes). I would also argue that if the functionality itself is
> still under active review (and I haven't competely followed the
> discussion so I don't know where that is up to, but Christoph, at
> least, seems not completely convinced), then it should also not yet be
> in linux-next.
OK, so at this point (just to get rid of the build failure I have done this:
I have reset the aio tree head to commit
b47275df9e1c ("aio: add support for aio poll via aio thread helper")
and then cherry-picked the following commits on top:
fb2e69217129 ("aio: Fix compile error due to unexpected use of cmpxchg()")
0964acffc614 ("aio: revert addition of io_send_sig() in generic_write_checks")
> > As for the build failure, it's a bug in the arch __get_user() implementation
> > that needs to be fixed. __get_user() should really be able to handle 64 bit
> > types.
>
> Yeah, it is a bit weird.
Well, you need to negotiate that with the affected architectures.
--
Cheers,
Stephen Rothwell
On Mon, Mar 14, 2016 at 03:49:15PM +1100, Stephen Rothwell wrote:
> Hi Ben,
...
> OK, so at this point (just to get rid of the build failure I have done this:
...
> Well, you need to negotiate that with the affected architectures.
I put in a patch to use get_user() for now since the 32 bit architectures
don't seem to have any plans for fixing this issue in a predictable
timeframe. There shouldn't be any build failures now -- I've checked ia64,
i386 and x86_64. The merge conflict looks trivial, so I won't touch
those pieces for the time being.
-ben
--
"Thought is the essence of where you are now."
Hi Ben,
On Mon, 14 Mar 2016 13:08:07 -0400 Benjamin LaHaise <[email protected]> wrote:
>
> I put in a patch to use get_user() for now since the 32 bit architectures
> don't seem to have any plans for fixing this issue in a predictable
> timeframe. There shouldn't be any build failures now -- I've checked ia64,
> i386 and x86_64. The merge conflict looks trivial, so I won't touch
> those pieces for the time being.
OK, thanks for the heads up.
--
Cheers,
Stephen Rothwell