From: Chris Wright on
* Greg KH (greg(a)kroah.com) wrote:
> On Tue, May 18, 2010 at 04:44:09PM +1000, Stephen Rothwell wrote:
> > Hi Greg,
> >
> > After merging the driver-core tree, today's linux-next build (powerpc
> > ppc64_defconfig) failed like this:
> >
> > cc1: warnings being treated as errors
> > In file included from include/linux/kobject.h:21,
> > from include/linux/device.h:17,
> > from arch/powerpc/lib/devres.c:10:
> > include/linux/sysfs.h:97: error: 'struct file' declared inside parameter list
> > include/linux/sysfs.h:97: error: its scope is only this definition or declaration, which is probably not what you want
> > include/linux/sysfs.h:99: error: 'struct file' declared inside parameter list
> > include/linux/sysfs.h:101: error: 'struct file' declared inside parameter list
> >
> > and many more (arch/powerpc is built with -Werror (as do some other
> > architectures)) and lots of similar warnings ...
> >
> > Caused by commit f8e898186196a22756b50b908ecd92123265f8a2 ("sysfs: add
> > struct file* to bin_attr callbacks"). See Rule 1 in
> > Documentation/SubmitChecklist. The header file probably just needs
> > "struct file;" added in the right place.
> >
> > I have reverted that commit for today (and commit
> > 44e425ab9f887ec6d3a7a4481f3b0c99f120de19 ("pci: check caps from sysfs
> > file open to read device dependent config space") that depends on it).
>
> Ick.
>
> Chris, care to send a patch to resolve this?

Yeah, I'll see where the header is missing.

thanks,
-chris
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Greg KH on
On Tue, May 18, 2010 at 04:44:09PM +1000, Stephen Rothwell wrote:
> Hi Greg,
>
> After merging the driver-core tree, today's linux-next build (powerpc
> ppc64_defconfig) failed like this:
>
> cc1: warnings being treated as errors
> In file included from include/linux/kobject.h:21,
> from include/linux/device.h:17,
> from arch/powerpc/lib/devres.c:10:
> include/linux/sysfs.h:97: error: 'struct file' declared inside parameter list
> include/linux/sysfs.h:97: error: its scope is only this definition or declaration, which is probably not what you want
> include/linux/sysfs.h:99: error: 'struct file' declared inside parameter list
> include/linux/sysfs.h:101: error: 'struct file' declared inside parameter list
>
> and many more (arch/powerpc is built with -Werror (as do some other
> architectures)) and lots of similar warnings ...
>
> Caused by commit f8e898186196a22756b50b908ecd92123265f8a2 ("sysfs: add
> struct file* to bin_attr callbacks"). See Rule 1 in
> Documentation/SubmitChecklist. The header file probably just needs
> "struct file;" added in the right place.
>
> I have reverted that commit for today (and commit
> 44e425ab9f887ec6d3a7a4481f3b0c99f120de19 ("pci: check caps from sysfs
> file open to read device dependent config space") that depends on it).

Ick.

Chris, care to send a patch to resolve this?

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Al Viro on
On Tue, May 18, 2010 at 03:35:10AM -0700, Eric W. Biederman wrote:
> Thanks.
>
> I will cook up a proper incremental patch after I get some sleep. Stephen
> it appears those two lines you have commented out are actually unnecessary.
>
> We have
> deactivate_super
> kill_sb aka sysfs_kill_sb
> kill_anon_super
> generic_shutdown_super
> sb_lock
> list_del(sb->s_instances)
> sb_unlock
> kfree(info)
>
> Nothing generic stomps on s_fs_info.
>
> Which means that if I find a superblock on sb->s_instances sb->s_fs_info
> still points to a valid sysfs_super_info.

Except that sb_lock is going away next cycle. There are very few users left
outside of fs/super.c and I'd much prefer it to become static.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Eric W. Biederman on

Greg this fixes the conflict with the vfs tree we see in linux-next.

Al I will be happy to work with you to rework this hunk of code in the
next cycle so you can kill sb_lock. It is just too late in the cycle
to be making more than the minimal change necessary.

Eric

From: Eric W. Biederman <ebiederm(a)aristanetworks.com>
Date: Tue, 18 May 2010 12:24:26 -0700
Subject: [PATCH] sysfs: Remove usage of S_BIAS to avoid merge conflict with the vfs tree

In Al's latest vfs tree the code is reworked and S_BIAS has been removed.

It turns out that checking to see if a super block is in the
middle of an unmount in sysfs_exit_ns is unnecessary because we
remove the super_block from the s_supers/s_instances list before
struct sysfs_super_info pointed to by sb->s_fs_info is freed.

For now just delete the unnecessary check to see if a superblock is in the
middle of an unmount, it isn't necessary with or without Al's changes
and it just causes a needless conflict.

Reported-by: Stephen Rothwell <sfr(a)canb.auug.org.au>
Signed-off-by: Eric W. Biederman <ebiederm(a)aristanetworks.com>
---
fs/sysfs/mount.c | 11 ++++++++---
1 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/fs/sysfs/mount.c b/fs/sysfs/mount.c
index bbba090..74f0529 100644
--- a/fs/sysfs/mount.c
+++ b/fs/sysfs/mount.c
@@ -144,6 +144,9 @@ static void sysfs_kill_sb(struct super_block *sb)
{
struct sysfs_super_info *info = sysfs_info(sb);

+ /* Remove the superblock from fs_supers/s_instances
+ * so we can't find it, before freeing sysfs_super_info.
+ */
kill_anon_super(sb);
kfree(info);
}
@@ -162,9 +165,11 @@ void sysfs_exit_ns(enum kobj_ns_type type, const void *ns)
spin_lock(&sb_lock);
list_for_each_entry(sb, &sysfs_fs_type.fs_supers, s_instances) {
struct sysfs_super_info *info = sysfs_info(sb);
- /* Ignore superblocks that are in the process of unmounting */
- if (sb->s_count <= S_BIAS)
- continue;
+ /*
+ * If we see a superblock on the fs_supers/s_instances
+ * list the unmount has not completed and sb->s_fs_info
+ * points to a valid struct sysfs_super_info.
+ */
/* Ignore superblocks with the wrong ns */
if (info->ns[type] != ns)
continue;
--
1.6.6.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Chris Wright on
* Greg KH (greg(a)kroah.com) wrote:
> On Tue, May 18, 2010 at 04:44:09PM +1000, Stephen Rothwell wrote:
> > Hi Greg,
> >
> > After merging the driver-core tree, today's linux-next build (powerpc
> > ppc64_defconfig) failed like this:
> >
> > cc1: warnings being treated as errors
> > In file included from include/linux/kobject.h:21,
> > from include/linux/device.h:17,
> > from arch/powerpc/lib/devres.c:10:
> > include/linux/sysfs.h:97: error: 'struct file' declared inside parameter list
> > include/linux/sysfs.h:97: error: its scope is only this definition or declaration, which is probably not what you want
> > include/linux/sysfs.h:99: error: 'struct file' declared inside parameter list
> > include/linux/sysfs.h:101: error: 'struct file' declared inside parameter list
> >
> > and many more (arch/powerpc is built with -Werror (as do some other
> > architectures)) and lots of similar warnings ...
> >
> > Caused by commit f8e898186196a22756b50b908ecd92123265f8a2 ("sysfs: add
> > struct file* to bin_attr callbacks"). See Rule 1 in
> > Documentation/SubmitChecklist. The header file probably just needs
> > "struct file;" added in the right place.
> >
> > I have reverted that commit for today (and commit
> > 44e425ab9f887ec6d3a7a4481f3b0c99f120de19 ("pci: check caps from sysfs
> > file open to read device dependent config space") that depends on it).
>
> Ick.
>
> Chris, care to send a patch to resolve this?

Would you prefer incremental to fold in, or respin? It's just this
one-liner fwd declaration as Stephen mentioned.

thanks,
-chris
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/