3.15 regression: wrong cgroup magic

3.15 regression: wrong cgroup magic

[Andy Lutomirski]

Sorry I didn't notice this earlier.  Linux 3.15 breaks my production
system :(  The cause appears to be:

commit 2bd59d48ebfb3df41ee56938946ca0dd30887312
Author: Tejun Heo <tj@kernel.org>
Date:   Tue Feb 11 11:52:49 2014 -0500

    cgroup: convert to kernfs

In particular, this piece:

-       sb->s_magic = CGROUP_SUPER_MAGIC;

The result is that cgroup shows up with the wrong magic number, so my
code goes "oh crap, cgroupfs isn't mounted" and fails.

I can change my code to hack around this, but I can imagine other
things getting tripped up.  Is there still time to fix this?

--Andy

Re: 3.15 regression: wrong cgroup magic

[Linus Torvalds]

On Mon, Jun 2, 2014 at 5:56 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>
> In particular, this piece:
>
> -       sb->s_magic = CGROUP_SUPER_MAGIC;
>
> The result is that cgroup shows up with the wrong magic number, so my
> code goes "oh crap, cgroupfs isn't mounted" and fails.
>
> I can change my code to hack around this, but I can imagine other
> things getting tripped up.  Is there still time to fix this?

Sure. Send me a tested patch. I'm assuming it's going to look something like

  --- a/kernel/cgroup.c
  +++ b/kernel/cgroup.c
  @@ -54,6 +54,7 @@
   #include <linux/vmalloc.h> /* TODO: replace with more sophisticated array */
   #include <linux/kthread.h>
   #include <linux/delay.h>
  +#include <linux/magic.h>

   #include <linux/atomic.h>

  @@ -1607,6 +1608,8 @@ out_unlock:
          dentry = kernfs_mount(fs_type, flags, root->kf_root, &new_sb);
          if (IS_ERR(dentry) || !new_sb)
                  cgroup_put(&root->cgrp);
  +       else
  +               dentry->d_sb->s_magic = CGROUP_SUPER_MAGIC;
          return dentry;
   }

but somebody definitely needs to test it.

               Linus

Re: 3.15 regression: wrong cgroup magic

[Li Zefan]

Cc: Greg
Cc: Jianyu Zhan

On 2014/6/3 8:56, Andy Lutomirski wrote:
> Sorry I didn't notice this earlier.  Linux 3.15 breaks my production

But 3.15 hasn't been released. :)

> system :(  The cause appears to be:
> 
> commit 2bd59d48ebfb3df41ee56938946ca0dd30887312
> Author: Tejun Heo <tj@kernel.org>
> Date:   Tue Feb 11 11:52:49 2014 -0500
> 
>     cgroup: convert to kernfs
> 
> In particular, this piece:
> 
> -       sb->s_magic = CGROUP_SUPER_MAGIC;
> 
> The result is that cgroup shows up with the wrong magic number, so my
> code goes "oh crap, cgroupfs isn't mounted" and fails.
> 
> I can change my code to hack around this, but I can imagine other
> things getting tripped up.  Is there still time to fix this?
> 

This should be fixed by "kernfs: move the last knowledge of sysfs out from kernfs".

It's in driver-core-next.

https://git.kernel.org/cgit/linux/kernel/git/gregkh/driver-core.git/commit/?h=driver-core-next&id=26fc9cd200ec839e0b3095e05ae018f27314e7aa

Re: 3.15 regression: wrong cgroup magic

[Tejun Heo]

On Tue, Jun 03, 2014 at 09:18:25AM +0800, Li Zefan wrote:
> > commit 2bd59d48ebfb3df41ee56938946ca0dd30887312
> > Author: Tejun Heo <tj@kernel.org>
> > Date:   Tue Feb 11 11:52:49 2014 -0500
> > 
> >     cgroup: convert to kernfs
> > 
> > In particular, this piece:
> > 
> > -       sb->s_magic = CGROUP_SUPER_MAGIC;
> > 
> > The result is that cgroup shows up with the wrong magic number, so my
> > code goes "oh crap, cgroupfs isn't mounted" and fails.
> > 
> > I can change my code to hack around this, but I can imagine other
> > things getting tripped up.  Is there still time to fix this?
> > 
> 
> This should be fixed by "kernfs: move the last knowledge of sysfs out from kernfs".
> 
> It's in driver-core-next.
> 
> https://git.kernel.org/cgit/linux/kernel/git/gregkh/driver-core.git/commit/?h=driver-core-next&id=26fc9cd200ec839e0b3095e05ae018f27314e7aa

Right, I was writing about the same patch with a nagging sense of
deja-vu.  I should have noticed that this must go through
driver-core-linus not -next.  Sorry about that.

Linus, can you please cherry-pick the commit?

Thanks.

-- 
tejun

Re: 3.15 regression: wrong cgroup magic

[Linus Torvalds]

On Mon, Jun 2, 2014 at 6:22 PM, Tejun Heo <tj@kernel.org> wrote:
>
> Linus, can you please cherry-pick the commit?

I'd much rather see it go through the proper channels than go ahead
and cherry-pick from some branch that hasn't even been sent to me yet.
The whole "you have to send things to me for me to take them" policy
is not new, I don't want to start taking stuff that the
authors/maintainers haven't actively sent my way.

That said, I suspect that Greg didn't expect this to actually matter
(the commit message certainly doesn't make it sound like anything that
people would notice), so the reason it is in -next is likely that
nobody thought it was a regression.

Of course, Greg could just send it to me for my next branch (since the
merge window for 3.16 is already open) and tell me that it's also
stable material for 3.15. At _that_ point I'll happily cherry-pick it
intpo master...

                Linus

Re: 3.15 regression: wrong cgroup magic

[Greg Kroah-Hartman]

On Mon, Jun 02, 2014 at 06:30:20PM -0700, Linus Torvalds wrote:
> On Mon, Jun 2, 2014 at 6:22 PM, Tejun Heo <tj@kernel.org> wrote:
> >
> > Linus, can you please cherry-pick the commit?
> 
> I'd much rather see it go through the proper channels than go ahead
> and cherry-pick from some branch that hasn't even been sent to me yet.
> The whole "you have to send things to me for me to take them" policy
> is not new, I don't want to start taking stuff that the
> authors/maintainers haven't actively sent my way.
> 
> That said, I suspect that Greg didn't expect this to actually matter
> (the commit message certainly doesn't make it sound like anything that
> people would notice), so the reason it is in -next is likely that
> nobody thought it was a regression.

Yes, I did not realize it at all, otherwise I would have sent it to you
earlier.

> Of course, Greg could just send it to me for my next branch (since the
> merge window for 3.16 is already open) and tell me that it's also
> stable material for 3.15. At _that_ point I'll happily cherry-pick it
> intpo master...

Ok, I'll go make up a pull request tonight for that.

thanks,

greg k-h

Re: 3.15 regression: wrong cgroup magic

[Linus Torvalds]

On Mon, Jun 2, 2014 at 6:30 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> Of course, Greg could just send it to me for my next branch (since the
> merge window for 3.16 is already open) and tell me that it's also
> stable material for 3.15. At _that_ point I'll happily cherry-pick it
> into master...

Ok, so that happened.

Tejun, just to be anal, can you please test and verify current -git
(preferably both 'master' and 'next') with your app that cared about
the f_type information for statfs()..

             Linus

Re: 3.15 regression: wrong cgroup magic

[Andy Lutomirski]

On Tue, Jun 3, 2014 at 10:23 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Mon, Jun 2, 2014 at 6:30 PM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
>>
>> Of course, Greg could just send it to me for my next branch (since the
>> merge window for 3.16 is already open) and tell me that it's also
>> stable material for 3.15. At _that_ point I'll happily cherry-pick it
>> into master...
>
> Ok, so that happened.
>
> Tejun, just to be anal, can you please test and verify current -git
> (preferably both 'master' and 'next') with your app that cared about
> the f_type information for statfs()..

Did you mean me?

My code seems to work now, and strace says:

fstatfs(6, {f_type=0x27e0eb, f_bsize=4096, f_blocks=0, f_bfree=0, f_bavail=0, f_
files=0, f_ffree=0, f_fsid={0, 0}, f_namelen=255, f_frsize=4096}) = 0

which is consistent with:

#define CGROUP_SUPER_MAGIC      0x27e0eb

This is on master.  I'll test next eventually.

Thanks everyone!

--Andy

Re: 3.15 regression: wrong cgroup magic

[Linus Torvalds]

On Tue, Jun 3, 2014 at 10:48 AM, Andy Lutomirski <luto@amacapital.net> wrote:
>
> Did you mean me?

I did. Lost track of the people involved. Oops.

Thanks,
     Linus

Re: 3.15 regression: wrong cgroup magic

[Tejun Heo]

On Tue, Jun 03, 2014 at 10:23:03AM -0700, Linus Torvalds wrote:
> Tejun, just to be anal, can you please test and verify current -git
> (preferably both 'master' and 'next') with your app that cared about
> the f_type information for statfs()..

It was Andy.  Andy, can you please verify the latest -git fixes your
setup?  FWIW, I just verified the reported magic numbers and
everything looks kosher to me.

 # ./fstype / /sys /sys/fs/cgroup/memory/
 f_type=0x0000ef53 /
 f_type=0x62656572 /sys
 f_type=0x0027e0eb /sys/fs/cgroup/memory/

Thanks.

-- 
tejun

Re: 3.15 regression: wrong cgroup magic

[Tejun Heo]

On Tue, Jun 03, 2014 at 01:52:51PM -0400, Tejun Heo wrote:
> On Tue, Jun 03, 2014 at 10:23:03AM -0700, Linus Torvalds wrote:
> > Tejun, just to be anal, can you please test and verify current -git
> > (preferably both 'master' and 'next') with your app that cared about
> > the f_type information for statfs()..
> 
> It was Andy.  Andy, can you please verify the latest -git fixes your
> setup?  FWIW, I just verified the reported magic numbers and
> everything looks kosher to me.

Oops, Andy beat me to it. :)

-- 
tejun