Re: Error returns not handled correctly by sysfs.c:subsys_attr_store()

2008-01-07 Thread Andrew Patterson
On Thu, 2008-01-03 at 17:17 -0700, Andrew Patterson wrote:
> On Fri, 2008-01-04 at 09:07 +0900, Tejun Heo wrote:
> > Hello,
> > 
> > Andrew Patterson wrote:
> > > It looks like this is a shell issue.  After looking through the sysfs
> > > code, I realized that this problem seems to be driven from user-land.
> > > So I performed some experiments:
> > > 
> > >  1. Wrote a simple program that just used write(2) to write to the
> > > sysfs entry. This works fine.
> > >  2. Used /bin/echo instead of the built-in echo command.  This too
> > > works fine.
> > >  3. Tried several shells.  Zsh and Bash both fail.  Csh works fine.
> > > 
> > > I then ran strace on the following shell-script:
> > > 
> > > #!/bin/bash
> > > 
> > > echo x > allow_restart
> > > echo y > allow_restart
> > > echo z > allow_restart
> > > 
> > > and got:
> > > 
> > > # strace -e trace=write ~/tmp/tester.sh 
> > > write(1, "x\n", 2)  = -1 EINVAL (Invalid argument)
> > > write(1, "x\n", 2)  = -1 EINVAL (Invalid argument)
> > > write(2, "/home/andrew/tmp/tester.sh: line"..., 
> > > 72/home/andrew/tmp/tester.sh: line 4: echo: write error: Invalid argument
> > > ) = 72
> > > write(1, "x\ny\n", 4)   = -1 EINVAL (Invalid argument)
> > > write(1, "x\ny\n", 4)   = -1 EINVAL (Invalid argument)
> > > write(2, "/home/andrew/tmp/tester.sh: line"..., 
> > > 72/home/andrew/tmp/tester.sh: line 5: echo: write error: Invalid argument
> > > ) = 72
> > > write(1, "x\ny\nz\n", 6)= -1 EINVAL (Invalid argument)
> > > write(1, "x\ny\nz\n", 6)= -1 EINVAL (Invalid argument)
> > > write(2, "/home/andrew/tmp/tester.sh: line"..., 
> > > 72/home/andrew/tmp/tester.sh: line 6: echo: write error: Invalid argument
> > > ) = 72
> > > write(1, "x\ny\nz\n", 6x
> > > y
> > > z
> > > )= 6
> > > Process 3800 detached
> > 
> > Eeee That's scary.  Which distro are you using and what does
> > 'bash --version' say?
> 
> IA64 Debian lenny.  
> 
> # bash --version
> GNU bash, version 3.1.17(1)-release (ia64-unknown-linux-gnu)
> 
> # zsh --version 
> zsh 4.3.4 (ia64-unknown-linux-gnu)
> 
> # csh --version
> tcsh 6.14.00 (Astron) 2005-03-25 (ia64-unknown-linux) options
> wide,nls,dl,al,kan,rh,nd,color,filec
> 
> I suppose I should try this an ia32 box again, and perhaps with some
> other distros.  I am not sure what the kernel can do about this, but it
> might be nice to report it to the shell maintainers.
> 

This looks like it might be the culprit. 

http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=459643

The fact that it works on SLES10 lends further evidence to glibc being
the problem.

-- 
Andrew Patterson
Hewlett-Packard Company

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Error returns not handled correctly by sysfs.c:subsys_attr_store()

2008-01-07 Thread Andrew Patterson
On Thu, 2008-01-03 at 17:17 -0700, Andrew Patterson wrote:
 On Fri, 2008-01-04 at 09:07 +0900, Tejun Heo wrote:
  Hello,
  
  Andrew Patterson wrote:
   It looks like this is a shell issue.  After looking through the sysfs
   code, I realized that this problem seems to be driven from user-land.
   So I performed some experiments:
   
1. Wrote a simple program that just used write(2) to write to the
   sysfs entry. This works fine.
2. Used /bin/echo instead of the built-in echo command.  This too
   works fine.
3. Tried several shells.  Zsh and Bash both fail.  Csh works fine.
   
   I then ran strace on the following shell-script:
   
   #!/bin/bash
   
   echo x  allow_restart
   echo y  allow_restart
   echo z  allow_restart
   
   and got:
   
   # strace -e trace=write ~/tmp/tester.sh 
   write(1, x\n, 2)  = -1 EINVAL (Invalid argument)
   write(1, x\n, 2)  = -1 EINVAL (Invalid argument)
   write(2, /home/andrew/tmp/tester.sh: line..., 
   72/home/andrew/tmp/tester.sh: line 4: echo: write error: Invalid argument
   ) = 72
   write(1, x\ny\n, 4)   = -1 EINVAL (Invalid argument)
   write(1, x\ny\n, 4)   = -1 EINVAL (Invalid argument)
   write(2, /home/andrew/tmp/tester.sh: line..., 
   72/home/andrew/tmp/tester.sh: line 5: echo: write error: Invalid argument
   ) = 72
   write(1, x\ny\nz\n, 6)= -1 EINVAL (Invalid argument)
   write(1, x\ny\nz\n, 6)= -1 EINVAL (Invalid argument)
   write(2, /home/andrew/tmp/tester.sh: line..., 
   72/home/andrew/tmp/tester.sh: line 6: echo: write error: Invalid argument
   ) = 72
   write(1, x\ny\nz\n, 6x
   y
   z
   )= 6
   Process 3800 detached
  
  Eeee That's scary.  Which distro are you using and what does
  'bash --version' say?
 
 IA64 Debian lenny.  
 
 # bash --version
 GNU bash, version 3.1.17(1)-release (ia64-unknown-linux-gnu)
 
 # zsh --version 
 zsh 4.3.4 (ia64-unknown-linux-gnu)
 
 # csh --version
 tcsh 6.14.00 (Astron) 2005-03-25 (ia64-unknown-linux) options
 wide,nls,dl,al,kan,rh,nd,color,filec
 
 I suppose I should try this an ia32 box again, and perhaps with some
 other distros.  I am not sure what the kernel can do about this, but it
 might be nice to report it to the shell maintainers.
 

This looks like it might be the culprit. 

http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=459643

The fact that it works on SLES10 lends further evidence to glibc being
the problem.

-- 
Andrew Patterson
Hewlett-Packard Company

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Error returns not handled correctly by sysfs.c:subsys_attr_store()

2008-01-03 Thread Andrey Borzenkov
On Friday 04 January 2008, Andrew Patterson wrote:
> On Thu, 2008-01-03 at 17:17 -0700, Andrew Patterson wrote:
> > On Fri, 2008-01-04 at 09:07 +0900, Tejun Heo wrote:
> > > Hello,
> > > 
> > > Andrew Patterson wrote:
> > > > It looks like this is a shell issue.  After looking through the sysfs
> > > > code, I realized that this problem seems to be driven from user-land.
> > > > So I performed some experiments:
> > > > 
> > > >  1. Wrote a simple program that just used write(2) to write to the
> > > > sysfs entry. This works fine.
> > > >  2. Used /bin/echo instead of the built-in echo command.  This too
> > > > works fine.
> > > >  3. Tried several shells.  Zsh and Bash both fail.  Csh works fine.
> > > > 
> > > > I then ran strace on the following shell-script:
> > > > 
> > > > #!/bin/bash
> > > > 
> > > > echo x > allow_restart
> > > > echo y > allow_restart
> > > > echo z > allow_restart
> > > > 
> > > > and got:
> > > > 
> > > > # strace -e trace=write ~/tmp/tester.sh 
> > > > write(1, "x\n", 2)  = -1 EINVAL (Invalid argument)
> > > > write(1, "x\n", 2)  = -1 EINVAL (Invalid argument)
> > > > write(2, "/home/andrew/tmp/tester.sh: line"..., 
72/home/andrew/tmp/tester.sh: line 4: echo: write error: Invalid argument
> > > > ) = 72
> > > > write(1, "x\ny\n", 4)   = -1 EINVAL (Invalid argument)
> > > > write(1, "x\ny\n", 4)   = -1 EINVAL (Invalid argument)
> > > > write(2, "/home/andrew/tmp/tester.sh: line"..., 
72/home/andrew/tmp/tester.sh: line 5: echo: write error: Invalid argument
> > > > ) = 72
> > > > write(1, "x\ny\nz\n", 6)= -1 EINVAL (Invalid argument)
> > > > write(1, "x\ny\nz\n", 6)= -1 EINVAL (Invalid argument)
> > > > write(2, "/home/andrew/tmp/tester.sh: line"..., 
72/home/andrew/tmp/tester.sh: line 6: echo: write error: Invalid argument
> > > > ) = 72
> > > > write(1, "x\ny\nz\n", 6x
> > > > y
> > > > z
> > > > )= 6
> > > > Process 3800 detached
> > > 
> > > Eeee That's scary.  Which distro are you using and what does
> > > 'bash --version' say?
> > 
> > IA64 Debian lenny.  
> > 
> > # bash --version
> > GNU bash, version 3.1.17(1)-release (ia64-unknown-linux-gnu)
> > 
> > # zsh --version 
> > zsh 4.3.4 (ia64-unknown-linux-gnu)
> > 
> > # csh --version
> > tcsh 6.14.00 (Astron) 2005-03-25 (ia64-unknown-linux) options
> > wide,nls,dl,al,kan,rh,nd,color,filec
> > 
> > I suppose I should try this an ia32 box again, and perhaps with some
> > other distros.  I am not sure what the kernel can do about this, but it
> > might be nice to report it to the shell maintainers.
> 
> Some further tests:
> 
> AMD running Debian lenny with i686 kernel -- fails.  
> Bash version = 3.1.17(1)
> 
> Intel running Ubuntu/gutsy with i686 kernel -- fails.
> Bash version = 3.2.25(1)
> 
> Itanium running SLES10 with ia64 kernel -- succeeds.
> Bash version = 3.1.17(1)
> 
> BTW, I found a way to reproduce this without modifying the kernel.
> The /sys/class/scsi_host/*/state sysfs store routine returns EINVAL if
> an invalid state is written. So just echo 2 bad values to the the state
> sysfs entry while running strace.
> 

I can't reproduce it using zsh either 4.3.4 as shipped by Mandriva or zsh CVS 
head. In both cases it echoes correct argument. Nor do I see double writes's in 
strace.

{pts/0}% sudo strace -e trace=write /tmp/foo # zsh 4.3.4
write(1, "x\n", 2)  = -1 EINVAL (Invalid argument)
write(2, "/tmp/foo:echo:3: write error: \320\235"..., 72/tmp/foo:echo:3: write 
error: Недопустимый аргумент
) = 72
write(1, "y\n", 2)  = -1 EINVAL (Invalid argument)
write(2, "/tmp/foo:echo:4: write error: \320\235"..., 72/tmp/foo:echo:4: write 
error: Недопустимый аргумент
) = 72
write(1, "z\n", 2)  = -1 EINVAL (Invalid argument)
write(2, "/tmp/foo:echo:5: write error: \320\235"..., 72/tmp/foo:echo:5: write 
error: Недопустимый аргумент
) = 72
{pts/0}% sudo strace -e trace=write /tmp/foo # zsh CVS head
write(1, "x\n", 2)  = -1 EINVAL (Invalid argument)
write(2, "/tmp/foo:echo:3: write error: \320\235"..., 72/tmp/foo:echo:3: write 
error: Недопустимый аргумент
) = 72
write(1, "y\n", 2)  = -1 EINVAL (Invalid argument)
write(2, "/tmp/foo:echo:4: write error: \320\235"..., 72/tmp/foo:echo:4: write 
error: Недопустимый аргумент
) = 72
write(1, "z\n", 2)  = -1 EINVAL (Invalid argument)
write(2, "/tmp/foo:echo:5: write error: \320\235"..., 72/tmp/foo:echo:5: write 
error: Недопустимый аргумент
) = 72

{pts/0}% cat /tmp/foo
#!/home/bor/pkg/bin/zsh -f

echo x > state
echo y > state
echo z > state

where state is /sys/power/state


{pts/1}% zsh --version
zsh 4.3.4 (i586-mandriva-linux-gnu)
{pts/1}% ~/pkg/bin/zsh --version
zsh 4.3.4-dev-6 (i686-pc-linux-gnu)

-andrey


signature.asc
Description: This is a digitally signed message part.


Re: Error returns not handled correctly by sysfs.c:subsys_attr_store()

2008-01-03 Thread Andrew Patterson
On Thu, 2008-01-03 at 17:17 -0700, Andrew Patterson wrote:
> On Fri, 2008-01-04 at 09:07 +0900, Tejun Heo wrote:
> > Hello,
> > 
> > Andrew Patterson wrote:
> > > It looks like this is a shell issue.  After looking through the sysfs
> > > code, I realized that this problem seems to be driven from user-land.
> > > So I performed some experiments:
> > > 
> > >  1. Wrote a simple program that just used write(2) to write to the
> > > sysfs entry. This works fine.
> > >  2. Used /bin/echo instead of the built-in echo command.  This too
> > > works fine.
> > >  3. Tried several shells.  Zsh and Bash both fail.  Csh works fine.
> > > 
> > > I then ran strace on the following shell-script:
> > > 
> > > #!/bin/bash
> > > 
> > > echo x > allow_restart
> > > echo y > allow_restart
> > > echo z > allow_restart
> > > 
> > > and got:
> > > 
> > > # strace -e trace=write ~/tmp/tester.sh 
> > > write(1, "x\n", 2)  = -1 EINVAL (Invalid argument)
> > > write(1, "x\n", 2)  = -1 EINVAL (Invalid argument)
> > > write(2, "/home/andrew/tmp/tester.sh: line"..., 
> > > 72/home/andrew/tmp/tester.sh: line 4: echo: write error: Invalid argument
> > > ) = 72
> > > write(1, "x\ny\n", 4)   = -1 EINVAL (Invalid argument)
> > > write(1, "x\ny\n", 4)   = -1 EINVAL (Invalid argument)
> > > write(2, "/home/andrew/tmp/tester.sh: line"..., 
> > > 72/home/andrew/tmp/tester.sh: line 5: echo: write error: Invalid argument
> > > ) = 72
> > > write(1, "x\ny\nz\n", 6)= -1 EINVAL (Invalid argument)
> > > write(1, "x\ny\nz\n", 6)= -1 EINVAL (Invalid argument)
> > > write(2, "/home/andrew/tmp/tester.sh: line"..., 
> > > 72/home/andrew/tmp/tester.sh: line 6: echo: write error: Invalid argument
> > > ) = 72
> > > write(1, "x\ny\nz\n", 6x
> > > y
> > > z
> > > )= 6
> > > Process 3800 detached
> > 
> > Eeee That's scary.  Which distro are you using and what does
> > 'bash --version' say?
> 
> IA64 Debian lenny.  
> 
> # bash --version
> GNU bash, version 3.1.17(1)-release (ia64-unknown-linux-gnu)
> 
> # zsh --version 
> zsh 4.3.4 (ia64-unknown-linux-gnu)
> 
> # csh --version
> tcsh 6.14.00 (Astron) 2005-03-25 (ia64-unknown-linux) options
> wide,nls,dl,al,kan,rh,nd,color,filec
> 
> I suppose I should try this an ia32 box again, and perhaps with some
> other distros.  I am not sure what the kernel can do about this, but it
> might be nice to report it to the shell maintainers.

Some further tests:

AMD running Debian lenny with i686 kernel -- fails.  
Bash version = 3.1.17(1)

Intel running Ubuntu/gutsy with i686 kernel -- fails.
Bash version = 3.2.25(1)

Itanium running SLES10 with ia64 kernel -- succeeds.
Bash version = 3.1.17(1)

BTW, I found a way to reproduce this without modifying the kernel.
The /sys/class/scsi_host/*/state sysfs store routine returns EINVAL if
an invalid state is written. So just echo 2 bad values to the the state
sysfs entry while running strace.

Andrew

-- 
Andrew Patterson
Hewlett-Packard Company

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Error returns not handled correctly by sysfs.c:subsys_attr_store()

2008-01-03 Thread Andrew Patterson

On Fri, 2008-01-04 at 09:07 +0900, Tejun Heo wrote:
> Hello,
> 
> Andrew Patterson wrote:
> > It looks like this is a shell issue.  After looking through the sysfs
> > code, I realized that this problem seems to be driven from user-land.
> > So I performed some experiments:
> > 
> >  1. Wrote a simple program that just used write(2) to write to the
> > sysfs entry. This works fine.
> >  2. Used /bin/echo instead of the built-in echo command.  This too
> > works fine.
> >  3. Tried several shells.  Zsh and Bash both fail.  Csh works fine.
> > 
> > I then ran strace on the following shell-script:
> > 
> > #!/bin/bash
> > 
> > echo x > allow_restart
> > echo y > allow_restart
> > echo z > allow_restart
> > 
> > and got:
> > 
> > # strace -e trace=write ~/tmp/tester.sh 
> > write(1, "x\n", 2)  = -1 EINVAL (Invalid argument)
> > write(1, "x\n", 2)  = -1 EINVAL (Invalid argument)
> > write(2, "/home/andrew/tmp/tester.sh: line"..., 
> > 72/home/andrew/tmp/tester.sh: line 4: echo: write error: Invalid argument
> > ) = 72
> > write(1, "x\ny\n", 4)   = -1 EINVAL (Invalid argument)
> > write(1, "x\ny\n", 4)   = -1 EINVAL (Invalid argument)
> > write(2, "/home/andrew/tmp/tester.sh: line"..., 
> > 72/home/andrew/tmp/tester.sh: line 5: echo: write error: Invalid argument
> > ) = 72
> > write(1, "x\ny\nz\n", 6)= -1 EINVAL (Invalid argument)
> > write(1, "x\ny\nz\n", 6)= -1 EINVAL (Invalid argument)
> > write(2, "/home/andrew/tmp/tester.sh: line"..., 
> > 72/home/andrew/tmp/tester.sh: line 6: echo: write error: Invalid argument
> > ) = 72
> > write(1, "x\ny\nz\n", 6x
> > y
> > z
> > )= 6
> > Process 3800 detached
> 
> Eeee That's scary.  Which distro are you using and what does
> 'bash --version' say?

IA64 Debian lenny.  

# bash --version
GNU bash, version 3.1.17(1)-release (ia64-unknown-linux-gnu)

# zsh --version 
zsh 4.3.4 (ia64-unknown-linux-gnu)

# csh --version
tcsh 6.14.00 (Astron) 2005-03-25 (ia64-unknown-linux) options
wide,nls,dl,al,kan,rh,nd,color,filec

I suppose I should try this an ia32 box again, and perhaps with some
other distros.  I am not sure what the kernel can do about this, but it
might be nice to report it to the shell maintainers.

-- 
Andrew Patterson
Hewlett-Packard Company

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Error returns not handled correctly by sysfs.c:subsys_attr_store()

2008-01-03 Thread Tejun Heo
Hello,

Andrew Patterson wrote:
> It looks like this is a shell issue.  After looking through the sysfs
> code, I realized that this problem seems to be driven from user-land.
> So I performed some experiments:
> 
>  1. Wrote a simple program that just used write(2) to write to the
> sysfs entry. This works fine.
>  2. Used /bin/echo instead of the built-in echo command.  This too
> works fine.
>  3. Tried several shells.  Zsh and Bash both fail.  Csh works fine.
> 
> I then ran strace on the following shell-script:
> 
> #!/bin/bash
> 
> echo x > allow_restart
> echo y > allow_restart
> echo z > allow_restart
> 
> and got:
> 
> # strace -e trace=write ~/tmp/tester.sh 
> write(1, "x\n", 2)  = -1 EINVAL (Invalid argument)
> write(1, "x\n", 2)  = -1 EINVAL (Invalid argument)
> write(2, "/home/andrew/tmp/tester.sh: line"..., 72/home/andrew/tmp/tester.sh: 
> line 4: echo: write error: Invalid argument
> ) = 72
> write(1, "x\ny\n", 4)   = -1 EINVAL (Invalid argument)
> write(1, "x\ny\n", 4)   = -1 EINVAL (Invalid argument)
> write(2, "/home/andrew/tmp/tester.sh: line"..., 72/home/andrew/tmp/tester.sh: 
> line 5: echo: write error: Invalid argument
> ) = 72
> write(1, "x\ny\nz\n", 6)= -1 EINVAL (Invalid argument)
> write(1, "x\ny\nz\n", 6)= -1 EINVAL (Invalid argument)
> write(2, "/home/andrew/tmp/tester.sh: line"..., 72/home/andrew/tmp/tester.sh: 
> line 6: echo: write error: Invalid argument
> ) = 72
> write(1, "x\ny\nz\n", 6x
> y
> z
> )= 6
> Process 3800 detached

Eeee That's scary.  Which distro are you using and what does
'bash --version' say?

-- 
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Error returns not handled correctly by sysfs.c:subsys_attr_store()

2008-01-03 Thread Andrew Patterson
On Mon, 2007-12-03 at 14:15 -0700, Andrew Patterson wrote:
> On Thu, 2007-11-29 at 10:07 +0900, Tejun Heo wrote:
> > Andrew Patterson wrote:
> > > I tried with clean 2.6.24-rc3 and get the same bad behavior.  This is on
> > > an ia64 box, so maybe that is an issue. I can try on an x86 box as well.
> > > Oh, one other thing.  I tried a "uname -r" to make sure I had the
> > > correct kernel booted and got:
> > > 
> > > # uname -r
> > > 2.6.24-rc3
> > > x
> > > y
> > > z
> > > #
> > 
> > Yeah, please try it on another machine from clean tree.  sysfs code is
> > definitely not endian dependent and is 64 bit clean.  Heck, all my test
> > machines run 64 bit these days.  I would be surprised if it's something
> > architecture dependent but please try on a different machine with
> > different userland with kernel built from fresh source tree.
> > 
> > Thanks.
> 
> I tried this on a AMD system running an i386 kernel. I get the same bad
> behavior.  This is from a 2.6.24-rc3 kernel downloaded from kernel.org.
> I ran "make mrproper" followed by "make oldconfig" and accepted all the
> defaults for the config.
> 
> There is one slight change with this experiment.  Other nodes are not
> getting corrupted, i.e., uname -r is getting the correct value.

It looks like this is a shell issue.  After looking through the sysfs
code, I realized that this problem seems to be driven from user-land.
So I performed some experiments:

 1. Wrote a simple program that just used write(2) to write to the
sysfs entry. This works fine.
 2. Used /bin/echo instead of the built-in echo command.  This too
works fine.
 3. Tried several shells.  Zsh and Bash both fail.  Csh works fine.

I then ran strace on the following shell-script:

#!/bin/bash

echo x > allow_restart
echo y > allow_restart
echo z > allow_restart

and got:

# strace -e trace=write ~/tmp/tester.sh 
write(1, "x\n", 2)  = -1 EINVAL (Invalid argument)
write(1, "x\n", 2)  = -1 EINVAL (Invalid argument)
write(2, "/home/andrew/tmp/tester.sh: line"..., 72/home/andrew/tmp/tester.sh: 
line 4: echo: write error: Invalid argument
) = 72
write(1, "x\ny\n", 4)   = -1 EINVAL (Invalid argument)
write(1, "x\ny\n", 4)   = -1 EINVAL (Invalid argument)
write(2, "/home/andrew/tmp/tester.sh: line"..., 72/home/andrew/tmp/tester.sh: 
line 5: echo: write error: Invalid argument
) = 72
write(1, "x\ny\nz\n", 6)= -1 EINVAL (Invalid argument)
write(1, "x\ny\nz\n", 6)= -1 EINVAL (Invalid argument)
write(2, "/home/andrew/tmp/tester.sh: line"..., 72/home/andrew/tmp/tester.sh: 
line 6: echo: write error: Invalid argument
) = 72
write(1, "x\ny\nz\n", 6x
y
z
)= 6
Process 3800 detached

As you can see, subsequent echo commands have their arguments appended
to the previous command. So it seems that the shell is not resetting the
buffer between failed echo commands.  

-- 
Andrew Patterson
Hewlett-Packard Company

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Error returns not handled correctly by sysfs.c:subsys_attr_store()

2008-01-03 Thread Andrew Patterson
On Mon, 2007-12-03 at 14:15 -0700, Andrew Patterson wrote:
 On Thu, 2007-11-29 at 10:07 +0900, Tejun Heo wrote:
  Andrew Patterson wrote:
   I tried with clean 2.6.24-rc3 and get the same bad behavior.  This is on
   an ia64 box, so maybe that is an issue. I can try on an x86 box as well.
   Oh, one other thing.  I tried a uname -r to make sure I had the
   correct kernel booted and got:
   
   # uname -r
   2.6.24-rc3
   x
   y
   z
   #
  
  Yeah, please try it on another machine from clean tree.  sysfs code is
  definitely not endian dependent and is 64 bit clean.  Heck, all my test
  machines run 64 bit these days.  I would be surprised if it's something
  architecture dependent but please try on a different machine with
  different userland with kernel built from fresh source tree.
  
  Thanks.
 
 I tried this on a AMD system running an i386 kernel. I get the same bad
 behavior.  This is from a 2.6.24-rc3 kernel downloaded from kernel.org.
 I ran make mrproper followed by make oldconfig and accepted all the
 defaults for the config.
 
 There is one slight change with this experiment.  Other nodes are not
 getting corrupted, i.e., uname -r is getting the correct value.

It looks like this is a shell issue.  After looking through the sysfs
code, I realized that this problem seems to be driven from user-land.
So I performed some experiments:

 1. Wrote a simple program that just used write(2) to write to the
sysfs entry. This works fine.
 2. Used /bin/echo instead of the built-in echo command.  This too
works fine.
 3. Tried several shells.  Zsh and Bash both fail.  Csh works fine.

I then ran strace on the following shell-script:

#!/bin/bash

echo x  allow_restart
echo y  allow_restart
echo z  allow_restart

and got:

# strace -e trace=write ~/tmp/tester.sh 
write(1, x\n, 2)  = -1 EINVAL (Invalid argument)
write(1, x\n, 2)  = -1 EINVAL (Invalid argument)
write(2, /home/andrew/tmp/tester.sh: line..., 72/home/andrew/tmp/tester.sh: 
line 4: echo: write error: Invalid argument
) = 72
write(1, x\ny\n, 4)   = -1 EINVAL (Invalid argument)
write(1, x\ny\n, 4)   = -1 EINVAL (Invalid argument)
write(2, /home/andrew/tmp/tester.sh: line..., 72/home/andrew/tmp/tester.sh: 
line 5: echo: write error: Invalid argument
) = 72
write(1, x\ny\nz\n, 6)= -1 EINVAL (Invalid argument)
write(1, x\ny\nz\n, 6)= -1 EINVAL (Invalid argument)
write(2, /home/andrew/tmp/tester.sh: line..., 72/home/andrew/tmp/tester.sh: 
line 6: echo: write error: Invalid argument
) = 72
write(1, x\ny\nz\n, 6x
y
z
)= 6
Process 3800 detached

As you can see, subsequent echo commands have their arguments appended
to the previous command. So it seems that the shell is not resetting the
buffer between failed echo commands.  

-- 
Andrew Patterson
Hewlett-Packard Company

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Error returns not handled correctly by sysfs.c:subsys_attr_store()

2008-01-03 Thread Tejun Heo
Hello,

Andrew Patterson wrote:
 It looks like this is a shell issue.  After looking through the sysfs
 code, I realized that this problem seems to be driven from user-land.
 So I performed some experiments:
 
  1. Wrote a simple program that just used write(2) to write to the
 sysfs entry. This works fine.
  2. Used /bin/echo instead of the built-in echo command.  This too
 works fine.
  3. Tried several shells.  Zsh and Bash both fail.  Csh works fine.
 
 I then ran strace on the following shell-script:
 
 #!/bin/bash
 
 echo x  allow_restart
 echo y  allow_restart
 echo z  allow_restart
 
 and got:
 
 # strace -e trace=write ~/tmp/tester.sh 
 write(1, x\n, 2)  = -1 EINVAL (Invalid argument)
 write(1, x\n, 2)  = -1 EINVAL (Invalid argument)
 write(2, /home/andrew/tmp/tester.sh: line..., 72/home/andrew/tmp/tester.sh: 
 line 4: echo: write error: Invalid argument
 ) = 72
 write(1, x\ny\n, 4)   = -1 EINVAL (Invalid argument)
 write(1, x\ny\n, 4)   = -1 EINVAL (Invalid argument)
 write(2, /home/andrew/tmp/tester.sh: line..., 72/home/andrew/tmp/tester.sh: 
 line 5: echo: write error: Invalid argument
 ) = 72
 write(1, x\ny\nz\n, 6)= -1 EINVAL (Invalid argument)
 write(1, x\ny\nz\n, 6)= -1 EINVAL (Invalid argument)
 write(2, /home/andrew/tmp/tester.sh: line..., 72/home/andrew/tmp/tester.sh: 
 line 6: echo: write error: Invalid argument
 ) = 72
 write(1, x\ny\nz\n, 6x
 y
 z
 )= 6
 Process 3800 detached

Eeee That's scary.  Which distro are you using and what does
'bash --version' say?

-- 
tejun
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Error returns not handled correctly by sysfs.c:subsys_attr_store()

2008-01-03 Thread Andrew Patterson
On Thu, 2008-01-03 at 17:17 -0700, Andrew Patterson wrote:
 On Fri, 2008-01-04 at 09:07 +0900, Tejun Heo wrote:
  Hello,
  
  Andrew Patterson wrote:
   It looks like this is a shell issue.  After looking through the sysfs
   code, I realized that this problem seems to be driven from user-land.
   So I performed some experiments:
   
1. Wrote a simple program that just used write(2) to write to the
   sysfs entry. This works fine.
2. Used /bin/echo instead of the built-in echo command.  This too
   works fine.
3. Tried several shells.  Zsh and Bash both fail.  Csh works fine.
   
   I then ran strace on the following shell-script:
   
   #!/bin/bash
   
   echo x  allow_restart
   echo y  allow_restart
   echo z  allow_restart
   
   and got:
   
   # strace -e trace=write ~/tmp/tester.sh 
   write(1, x\n, 2)  = -1 EINVAL (Invalid argument)
   write(1, x\n, 2)  = -1 EINVAL (Invalid argument)
   write(2, /home/andrew/tmp/tester.sh: line..., 
   72/home/andrew/tmp/tester.sh: line 4: echo: write error: Invalid argument
   ) = 72
   write(1, x\ny\n, 4)   = -1 EINVAL (Invalid argument)
   write(1, x\ny\n, 4)   = -1 EINVAL (Invalid argument)
   write(2, /home/andrew/tmp/tester.sh: line..., 
   72/home/andrew/tmp/tester.sh: line 5: echo: write error: Invalid argument
   ) = 72
   write(1, x\ny\nz\n, 6)= -1 EINVAL (Invalid argument)
   write(1, x\ny\nz\n, 6)= -1 EINVAL (Invalid argument)
   write(2, /home/andrew/tmp/tester.sh: line..., 
   72/home/andrew/tmp/tester.sh: line 6: echo: write error: Invalid argument
   ) = 72
   write(1, x\ny\nz\n, 6x
   y
   z
   )= 6
   Process 3800 detached
  
  Eeee That's scary.  Which distro are you using and what does
  'bash --version' say?
 
 IA64 Debian lenny.  
 
 # bash --version
 GNU bash, version 3.1.17(1)-release (ia64-unknown-linux-gnu)
 
 # zsh --version 
 zsh 4.3.4 (ia64-unknown-linux-gnu)
 
 # csh --version
 tcsh 6.14.00 (Astron) 2005-03-25 (ia64-unknown-linux) options
 wide,nls,dl,al,kan,rh,nd,color,filec
 
 I suppose I should try this an ia32 box again, and perhaps with some
 other distros.  I am not sure what the kernel can do about this, but it
 might be nice to report it to the shell maintainers.

Some further tests:

AMD running Debian lenny with i686 kernel -- fails.  
Bash version = 3.1.17(1)

Intel running Ubuntu/gutsy with i686 kernel -- fails.
Bash version = 3.2.25(1)

Itanium running SLES10 with ia64 kernel -- succeeds.
Bash version = 3.1.17(1)

BTW, I found a way to reproduce this without modifying the kernel.
The /sys/class/scsi_host/*/state sysfs store routine returns EINVAL if
an invalid state is written. So just echo 2 bad values to the the state
sysfs entry while running strace.

Andrew

-- 
Andrew Patterson
Hewlett-Packard Company

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Error returns not handled correctly by sysfs.c:subsys_attr_store()

2008-01-03 Thread Andrey Borzenkov
On Friday 04 January 2008, Andrew Patterson wrote:
 On Thu, 2008-01-03 at 17:17 -0700, Andrew Patterson wrote:
  On Fri, 2008-01-04 at 09:07 +0900, Tejun Heo wrote:
   Hello,
   
   Andrew Patterson wrote:
It looks like this is a shell issue.  After looking through the sysfs
code, I realized that this problem seems to be driven from user-land.
So I performed some experiments:

 1. Wrote a simple program that just used write(2) to write to the
sysfs entry. This works fine.
 2. Used /bin/echo instead of the built-in echo command.  This too
works fine.
 3. Tried several shells.  Zsh and Bash both fail.  Csh works fine.

I then ran strace on the following shell-script:

#!/bin/bash

echo x  allow_restart
echo y  allow_restart
echo z  allow_restart

and got:

# strace -e trace=write ~/tmp/tester.sh 
write(1, x\n, 2)  = -1 EINVAL (Invalid argument)
write(1, x\n, 2)  = -1 EINVAL (Invalid argument)
write(2, /home/andrew/tmp/tester.sh: line..., 
72/home/andrew/tmp/tester.sh: line 4: echo: write error: Invalid argument
) = 72
write(1, x\ny\n, 4)   = -1 EINVAL (Invalid argument)
write(1, x\ny\n, 4)   = -1 EINVAL (Invalid argument)
write(2, /home/andrew/tmp/tester.sh: line..., 
72/home/andrew/tmp/tester.sh: line 5: echo: write error: Invalid argument
) = 72
write(1, x\ny\nz\n, 6)= -1 EINVAL (Invalid argument)
write(1, x\ny\nz\n, 6)= -1 EINVAL (Invalid argument)
write(2, /home/andrew/tmp/tester.sh: line..., 
72/home/andrew/tmp/tester.sh: line 6: echo: write error: Invalid argument
) = 72
write(1, x\ny\nz\n, 6x
y
z
)= 6
Process 3800 detached
   
   Eeee That's scary.  Which distro are you using and what does
   'bash --version' say?
  
  IA64 Debian lenny.  
  
  # bash --version
  GNU bash, version 3.1.17(1)-release (ia64-unknown-linux-gnu)
  
  # zsh --version 
  zsh 4.3.4 (ia64-unknown-linux-gnu)
  
  # csh --version
  tcsh 6.14.00 (Astron) 2005-03-25 (ia64-unknown-linux) options
  wide,nls,dl,al,kan,rh,nd,color,filec
  
  I suppose I should try this an ia32 box again, and perhaps with some
  other distros.  I am not sure what the kernel can do about this, but it
  might be nice to report it to the shell maintainers.
 
 Some further tests:
 
 AMD running Debian lenny with i686 kernel -- fails.  
 Bash version = 3.1.17(1)
 
 Intel running Ubuntu/gutsy with i686 kernel -- fails.
 Bash version = 3.2.25(1)
 
 Itanium running SLES10 with ia64 kernel -- succeeds.
 Bash version = 3.1.17(1)
 
 BTW, I found a way to reproduce this without modifying the kernel.
 The /sys/class/scsi_host/*/state sysfs store routine returns EINVAL if
 an invalid state is written. So just echo 2 bad values to the the state
 sysfs entry while running strace.
 

I can't reproduce it using zsh either 4.3.4 as shipped by Mandriva or zsh CVS 
head. In both cases it echoes correct argument. Nor do I see double writes's in 
strace.

{pts/0}% sudo strace -e trace=write /tmp/foo # zsh 4.3.4
write(1, x\n, 2)  = -1 EINVAL (Invalid argument)
write(2, /tmp/foo:echo:3: write error: \320\235..., 72/tmp/foo:echo:3: write 
error: Недопустимый аргумент
) = 72
write(1, y\n, 2)  = -1 EINVAL (Invalid argument)
write(2, /tmp/foo:echo:4: write error: \320\235..., 72/tmp/foo:echo:4: write 
error: Недопустимый аргумент
) = 72
write(1, z\n, 2)  = -1 EINVAL (Invalid argument)
write(2, /tmp/foo:echo:5: write error: \320\235..., 72/tmp/foo:echo:5: write 
error: Недопустимый аргумент
) = 72
{pts/0}% sudo strace -e trace=write /tmp/foo # zsh CVS head
write(1, x\n, 2)  = -1 EINVAL (Invalid argument)
write(2, /tmp/foo:echo:3: write error: \320\235..., 72/tmp/foo:echo:3: write 
error: Недопустимый аргумент
) = 72
write(1, y\n, 2)  = -1 EINVAL (Invalid argument)
write(2, /tmp/foo:echo:4: write error: \320\235..., 72/tmp/foo:echo:4: write 
error: Недопустимый аргумент
) = 72
write(1, z\n, 2)  = -1 EINVAL (Invalid argument)
write(2, /tmp/foo:echo:5: write error: \320\235..., 72/tmp/foo:echo:5: write 
error: Недопустимый аргумент
) = 72

{pts/0}% cat /tmp/foo
#!/home/bor/pkg/bin/zsh -f

echo x  state
echo y  state
echo z  state

where state is /sys/power/state


{pts/1}% zsh --version
zsh 4.3.4 (i586-mandriva-linux-gnu)
{pts/1}% ~/pkg/bin/zsh --version
zsh 4.3.4-dev-6 (i686-pc-linux-gnu)

-andrey


signature.asc
Description: This is a digitally signed message part.


Re: Error returns not handled correctly by sysfs.c:subsys_attr_store()

2007-12-21 Thread Greg KH
On Mon, Dec 03, 2007 at 02:15:58PM -0700, Andrew Patterson wrote:
> On Thu, 2007-11-29 at 10:07 +0900, Tejun Heo wrote:
> > Andrew Patterson wrote:
> > > I tried with clean 2.6.24-rc3 and get the same bad behavior.  This is on
> > > an ia64 box, so maybe that is an issue. I can try on an x86 box as well.
> > > Oh, one other thing.  I tried a "uname -r" to make sure I had the
> > > correct kernel booted and got:
> > > 
> > > # uname -r
> > > 2.6.24-rc3
> > > x
> > > y
> > > z
> > > #
> > 
> > Yeah, please try it on another machine from clean tree.  sysfs code is
> > definitely not endian dependent and is 64 bit clean.  Heck, all my test
> > machines run 64 bit these days.  I would be surprised if it's something
> > architecture dependent but please try on a different machine with
> > different userland with kernel built from fresh source tree.
> > 
> > Thanks.
> 
> I tried this on a AMD system running an i386 kernel. I get the same bad
> behavior.  This is from a 2.6.24-rc3 kernel downloaded from kernel.org.
> I ran "make mrproper" followed by "make oldconfig" and accepted all the
> defaults for the config.
> 
> There is one slight change with this experiment.  Other nodes are not
> getting corrupted, i.e., uname -r is getting the correct value.

Are you still seeing this on 2.6.24-rc6?

I still can not duplicate this here :(

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Error returns not handled correctly by sysfs.c:subsys_attr_store()

2007-12-21 Thread Greg KH
On Mon, Dec 03, 2007 at 02:15:58PM -0700, Andrew Patterson wrote:
 On Thu, 2007-11-29 at 10:07 +0900, Tejun Heo wrote:
  Andrew Patterson wrote:
   I tried with clean 2.6.24-rc3 and get the same bad behavior.  This is on
   an ia64 box, so maybe that is an issue. I can try on an x86 box as well.
   Oh, one other thing.  I tried a uname -r to make sure I had the
   correct kernel booted and got:
   
   # uname -r
   2.6.24-rc3
   x
   y
   z
   #
  
  Yeah, please try it on another machine from clean tree.  sysfs code is
  definitely not endian dependent and is 64 bit clean.  Heck, all my test
  machines run 64 bit these days.  I would be surprised if it's something
  architecture dependent but please try on a different machine with
  different userland with kernel built from fresh source tree.
  
  Thanks.
 
 I tried this on a AMD system running an i386 kernel. I get the same bad
 behavior.  This is from a 2.6.24-rc3 kernel downloaded from kernel.org.
 I ran make mrproper followed by make oldconfig and accepted all the
 defaults for the config.
 
 There is one slight change with this experiment.  Other nodes are not
 getting corrupted, i.e., uname -r is getting the correct value.

Are you still seeing this on 2.6.24-rc6?

I still can not duplicate this here :(

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Error returns not handled correctly by sysfs.c:subsys_attr_store()

2007-12-03 Thread Andrew Patterson
On Thu, 2007-11-29 at 10:07 +0900, Tejun Heo wrote:
> Andrew Patterson wrote:
> > I tried with clean 2.6.24-rc3 and get the same bad behavior.  This is on
> > an ia64 box, so maybe that is an issue. I can try on an x86 box as well.
> > Oh, one other thing.  I tried a "uname -r" to make sure I had the
> > correct kernel booted and got:
> > 
> > # uname -r
> > 2.6.24-rc3
> > x
> > y
> > z
> > #
> 
> Yeah, please try it on another machine from clean tree.  sysfs code is
> definitely not endian dependent and is 64 bit clean.  Heck, all my test
> machines run 64 bit these days.  I would be surprised if it's something
> architecture dependent but please try on a different machine with
> different userland with kernel built from fresh source tree.
> 
> Thanks.

I tried this on a AMD system running an i386 kernel. I get the same bad
behavior.  This is from a 2.6.24-rc3 kernel downloaded from kernel.org.
I ran "make mrproper" followed by "make oldconfig" and accepted all the
defaults for the config.

There is one slight change with this experiment.  Other nodes are not
getting corrupted, i.e., uname -r is getting the correct value.

-- 
Andrew Patterson
Hewlett-Packard Company

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Error returns not handled correctly by sysfs.c:subsys_attr_store()

2007-12-03 Thread Andrew Patterson
On Thu, 2007-11-29 at 10:07 +0900, Tejun Heo wrote:
 Andrew Patterson wrote:
  I tried with clean 2.6.24-rc3 and get the same bad behavior.  This is on
  an ia64 box, so maybe that is an issue. I can try on an x86 box as well.
  Oh, one other thing.  I tried a uname -r to make sure I had the
  correct kernel booted and got:
  
  # uname -r
  2.6.24-rc3
  x
  y
  z
  #
 
 Yeah, please try it on another machine from clean tree.  sysfs code is
 definitely not endian dependent and is 64 bit clean.  Heck, all my test
 machines run 64 bit these days.  I would be surprised if it's something
 architecture dependent but please try on a different machine with
 different userland with kernel built from fresh source tree.
 
 Thanks.

I tried this on a AMD system running an i386 kernel. I get the same bad
behavior.  This is from a 2.6.24-rc3 kernel downloaded from kernel.org.
I ran make mrproper followed by make oldconfig and accepted all the
defaults for the config.

There is one slight change with this experiment.  Other nodes are not
getting corrupted, i.e., uname -r is getting the correct value.

-- 
Andrew Patterson
Hewlett-Packard Company

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Error returns not handled correctly by sysfs.c:subsys_attr_store()

2007-11-28 Thread Tejun Heo
Andrew Patterson wrote:
> I tried with clean 2.6.24-rc3 and get the same bad behavior.  This is on
> an ia64 box, so maybe that is an issue. I can try on an x86 box as well.
> Oh, one other thing.  I tried a "uname -r" to make sure I had the
> correct kernel booted and got:
> 
> # uname -r
> 2.6.24-rc3
> x
> y
> z
> #

Yeah, please try it on another machine from clean tree.  sysfs code is
definitely not endian dependent and is 64 bit clean.  Heck, all my test
machines run 64 bit these days.  I would be surprised if it's something
architecture dependent but please try on a different machine with
different userland with kernel built from fresh source tree.

Thanks.

-- 
tejun
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Error returns not handled correctly by sysfs.c:subsys_attr_store()

2007-11-28 Thread Greg KH
On Wed, Nov 28, 2007 at 12:31:40PM -0700, Andrew Patterson wrote:
> On Wed, 2007-11-28 at 16:42 +0900, Tejun Heo wrote:
> > Greg KH wrote:
> > > On Mon, Nov 26, 2007 at 08:31:16PM -0800, Andrew Morton wrote:
> > >> On Wed, 21 Nov 2007 15:16:59 -0700 Andrew Patterson <[EMAIL PROTECTED]> 
> > >> wrote:
> > >>
> > >>> The buf in fs/sysfs.c:subsys_attr_store() does not seem to be updated
> > >>> correctly when returning a negative value (indicating that an error
> > >>> condition has occurred) is returned.  If a negative value is returned,
> > >>> the next subsequent call to subsys_attr_store will have the contents of
> > >>> buf appended to the previous call.
> > >> subsys_attr_store() gets deleted by
> > >> http://www.kernel.org/pub/linux/kernel/people/gregkh/gregkh-2.6/gregkh-01-driver/kset-kill-subsys-attr.patch
> > >>
> > >> So maybe we will soon accidentally fix whatever-this-is?  Or maybe we 
> > >> will
> > >> faithfully maintain it.
> > > 
> > > Yes, subsys attributes go away, but this is showing a bug in the sysfs
> > > core with attributes, not in the "middle" layers of attributes.
> > > 
> > > I bounced the original bug report to Tejun, who has been changing the
> > > logic around this area to see if he sees anything that might be
> > > different now.
> > > 
> > > Tejun?
> > 
> > Weird, the problem is not reproducible here.
> > 
> > # echo a > allow_restart
> > -bash: echo: write error: Invalid argument
> > [  437.518024] buf_ptr = 0x810005e2, buf = x
> > [  437.518027] , count = 2
> > # echo b > allow_restart
> > -bash: echo: write error: Invalid argument
> > [  438.972973] buf_ptr = 0x81001be6f000, buf = y
> > [  438.972976] , count = 2
> > # echo c > allow_restart
> > -bash: echo: write error: Invalid argument
> > [  440.539747] buf_ptr = 0x81001d4ba000, buf = z
> > [  440.539750] , count = 2
> > 
> > Which is expected.  On each open, sysfs_buffer is allocated with kzalloc
> > and the buffer is freed on close, so I don't see how it can happen.
> > Behavior for multiple write can be considered peculiar in that ppos is
> > essentially ignored and each write is passed just like brand new write
> > to ->store method but this too is the expected behavior.
> > 
> > # (echo a; echo b; echo c) > allow_restart
> > [  765.257132] buf_ptr = 0x81001be4f000, buf = a
> > [  765.257135] , count = 2
> > [  765.285474] buf_ptr = 0x81001be4f000, buf = b
> > [  765.285484] , count = 2
> > [  765.314002] buf_ptr = 0x81001be4f000, buf = c
> > [  765.314004] , count = 2
> > -bash: echo: write error: Invalid argument
> > -bash: echo: write error: Invalid argument
> > -bash: echo: write error: Invalid argument
> > 
> > Andrew Petterson, can you please build 2.6.24-rc3 from clean source tree
> > and retry?
> > 
> 
> I tried with clean 2.6.24-rc3 and get the same bad behavior.  This is on
> an ia64 box, so maybe that is an issue. I can try on an x86 box as well.

Please do so.

> Oh, one other thing.  I tried a "uname -r" to make sure I had the
> correct kernel booted and got:
> 
> # uname -r
> 2.6.24-rc3
> x
> y
> z

Heh, that's not good, try a clean tree :)

thanks,

greg k-h
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Error returns not handled correctly by sysfs.c:subsys_attr_store()

2007-11-28 Thread Andrew Patterson
On Wed, 2007-11-28 at 16:42 +0900, Tejun Heo wrote:
> Greg KH wrote:
> > On Mon, Nov 26, 2007 at 08:31:16PM -0800, Andrew Morton wrote:
> >> On Wed, 21 Nov 2007 15:16:59 -0700 Andrew Patterson <[EMAIL PROTECTED]> 
> >> wrote:
> >>
> >>> The buf in fs/sysfs.c:subsys_attr_store() does not seem to be updated
> >>> correctly when returning a negative value (indicating that an error
> >>> condition has occurred) is returned.  If a negative value is returned,
> >>> the next subsequent call to subsys_attr_store will have the contents of
> >>> buf appended to the previous call.
> >> subsys_attr_store() gets deleted by
> >> http://www.kernel.org/pub/linux/kernel/people/gregkh/gregkh-2.6/gregkh-01-driver/kset-kill-subsys-attr.patch
> >>
> >> So maybe we will soon accidentally fix whatever-this-is?  Or maybe we will
> >> faithfully maintain it.
> > 
> > Yes, subsys attributes go away, but this is showing a bug in the sysfs
> > core with attributes, not in the "middle" layers of attributes.
> > 
> > I bounced the original bug report to Tejun, who has been changing the
> > logic around this area to see if he sees anything that might be
> > different now.
> > 
> > Tejun?
> 
> Weird, the problem is not reproducible here.
> 
> # echo a > allow_restart
> -bash: echo: write error: Invalid argument
> [  437.518024] buf_ptr = 0x810005e2, buf = x
> [  437.518027] , count = 2
> # echo b > allow_restart
> -bash: echo: write error: Invalid argument
> [  438.972973] buf_ptr = 0x81001be6f000, buf = y
> [  438.972976] , count = 2
> # echo c > allow_restart
> -bash: echo: write error: Invalid argument
> [  440.539747] buf_ptr = 0x81001d4ba000, buf = z
> [  440.539750] , count = 2
> 
> Which is expected.  On each open, sysfs_buffer is allocated with kzalloc
> and the buffer is freed on close, so I don't see how it can happen.
> Behavior for multiple write can be considered peculiar in that ppos is
> essentially ignored and each write is passed just like brand new write
> to ->store method but this too is the expected behavior.
> 
> # (echo a; echo b; echo c) > allow_restart
> [  765.257132] buf_ptr = 0x81001be4f000, buf = a
> [  765.257135] , count = 2
> [  765.285474] buf_ptr = 0x81001be4f000, buf = b
> [  765.285484] , count = 2
> [  765.314002] buf_ptr = 0x81001be4f000, buf = c
> [  765.314004] , count = 2
> -bash: echo: write error: Invalid argument
> -bash: echo: write error: Invalid argument
> -bash: echo: write error: Invalid argument
> 
> Andrew Petterson, can you please build 2.6.24-rc3 from clean source tree
> and retry?
> 

I tried with clean 2.6.24-rc3 and get the same bad behavior.  This is on
an ia64 box, so maybe that is an issue. I can try on an x86 box as well.
Oh, one other thing.  I tried a "uname -r" to make sure I had the
correct kernel booted and got:

# uname -r
2.6.24-rc3
x
y
z
#

Andrew
-- 
Andrew Patterson
Hewlett-Packard

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Error returns not handled correctly by sysfs.c:subsys_attr_store()

2007-11-28 Thread Andrew Patterson
On Wed, 2007-11-28 at 16:42 +0900, Tejun Heo wrote:
 Greg KH wrote:
  On Mon, Nov 26, 2007 at 08:31:16PM -0800, Andrew Morton wrote:
  On Wed, 21 Nov 2007 15:16:59 -0700 Andrew Patterson [EMAIL PROTECTED] 
  wrote:
 
  The buf in fs/sysfs.c:subsys_attr_store() does not seem to be updated
  correctly when returning a negative value (indicating that an error
  condition has occurred) is returned.  If a negative value is returned,
  the next subsequent call to subsys_attr_store will have the contents of
  buf appended to the previous call.
  subsys_attr_store() gets deleted by
  http://www.kernel.org/pub/linux/kernel/people/gregkh/gregkh-2.6/gregkh-01-driver/kset-kill-subsys-attr.patch
 
  So maybe we will soon accidentally fix whatever-this-is?  Or maybe we will
  faithfully maintain it.
  
  Yes, subsys attributes go away, but this is showing a bug in the sysfs
  core with attributes, not in the middle layers of attributes.
  
  I bounced the original bug report to Tejun, who has been changing the
  logic around this area to see if he sees anything that might be
  different now.
  
  Tejun?
 
 Weird, the problem is not reproducible here.
 
 # echo a  allow_restart
 -bash: echo: write error: Invalid argument
 [  437.518024] buf_ptr = 0x810005e2, buf = x
 [  437.518027] , count = 2
 # echo b  allow_restart
 -bash: echo: write error: Invalid argument
 [  438.972973] buf_ptr = 0x81001be6f000, buf = y
 [  438.972976] , count = 2
 # echo c  allow_restart
 -bash: echo: write error: Invalid argument
 [  440.539747] buf_ptr = 0x81001d4ba000, buf = z
 [  440.539750] , count = 2
 
 Which is expected.  On each open, sysfs_buffer is allocated with kzalloc
 and the buffer is freed on close, so I don't see how it can happen.
 Behavior for multiple write can be considered peculiar in that ppos is
 essentially ignored and each write is passed just like brand new write
 to -store method but this too is the expected behavior.
 
 # (echo a; echo b; echo c)  allow_restart
 [  765.257132] buf_ptr = 0x81001be4f000, buf = a
 [  765.257135] , count = 2
 [  765.285474] buf_ptr = 0x81001be4f000, buf = b
 [  765.285484] , count = 2
 [  765.314002] buf_ptr = 0x81001be4f000, buf = c
 [  765.314004] , count = 2
 -bash: echo: write error: Invalid argument
 -bash: echo: write error: Invalid argument
 -bash: echo: write error: Invalid argument
 
 Andrew Petterson, can you please build 2.6.24-rc3 from clean source tree
 and retry?
 

I tried with clean 2.6.24-rc3 and get the same bad behavior.  This is on
an ia64 box, so maybe that is an issue. I can try on an x86 box as well.
Oh, one other thing.  I tried a uname -r to make sure I had the
correct kernel booted and got:

# uname -r
2.6.24-rc3
x
y
z
#

Andrew
-- 
Andrew Patterson
Hewlett-Packard

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Error returns not handled correctly by sysfs.c:subsys_attr_store()

2007-11-28 Thread Greg KH
On Wed, Nov 28, 2007 at 12:31:40PM -0700, Andrew Patterson wrote:
 On Wed, 2007-11-28 at 16:42 +0900, Tejun Heo wrote:
  Greg KH wrote:
   On Mon, Nov 26, 2007 at 08:31:16PM -0800, Andrew Morton wrote:
   On Wed, 21 Nov 2007 15:16:59 -0700 Andrew Patterson [EMAIL PROTECTED] 
   wrote:
  
   The buf in fs/sysfs.c:subsys_attr_store() does not seem to be updated
   correctly when returning a negative value (indicating that an error
   condition has occurred) is returned.  If a negative value is returned,
   the next subsequent call to subsys_attr_store will have the contents of
   buf appended to the previous call.
   subsys_attr_store() gets deleted by
   http://www.kernel.org/pub/linux/kernel/people/gregkh/gregkh-2.6/gregkh-01-driver/kset-kill-subsys-attr.patch
  
   So maybe we will soon accidentally fix whatever-this-is?  Or maybe we 
   will
   faithfully maintain it.
   
   Yes, subsys attributes go away, but this is showing a bug in the sysfs
   core with attributes, not in the middle layers of attributes.
   
   I bounced the original bug report to Tejun, who has been changing the
   logic around this area to see if he sees anything that might be
   different now.
   
   Tejun?
  
  Weird, the problem is not reproducible here.
  
  # echo a  allow_restart
  -bash: echo: write error: Invalid argument
  [  437.518024] buf_ptr = 0x810005e2, buf = x
  [  437.518027] , count = 2
  # echo b  allow_restart
  -bash: echo: write error: Invalid argument
  [  438.972973] buf_ptr = 0x81001be6f000, buf = y
  [  438.972976] , count = 2
  # echo c  allow_restart
  -bash: echo: write error: Invalid argument
  [  440.539747] buf_ptr = 0x81001d4ba000, buf = z
  [  440.539750] , count = 2
  
  Which is expected.  On each open, sysfs_buffer is allocated with kzalloc
  and the buffer is freed on close, so I don't see how it can happen.
  Behavior for multiple write can be considered peculiar in that ppos is
  essentially ignored and each write is passed just like brand new write
  to -store method but this too is the expected behavior.
  
  # (echo a; echo b; echo c)  allow_restart
  [  765.257132] buf_ptr = 0x81001be4f000, buf = a
  [  765.257135] , count = 2
  [  765.285474] buf_ptr = 0x81001be4f000, buf = b
  [  765.285484] , count = 2
  [  765.314002] buf_ptr = 0x81001be4f000, buf = c
  [  765.314004] , count = 2
  -bash: echo: write error: Invalid argument
  -bash: echo: write error: Invalid argument
  -bash: echo: write error: Invalid argument
  
  Andrew Petterson, can you please build 2.6.24-rc3 from clean source tree
  and retry?
  
 
 I tried with clean 2.6.24-rc3 and get the same bad behavior.  This is on
 an ia64 box, so maybe that is an issue. I can try on an x86 box as well.

Please do so.

 Oh, one other thing.  I tried a uname -r to make sure I had the
 correct kernel booted and got:
 
 # uname -r
 2.6.24-rc3
 x
 y
 z

Heh, that's not good, try a clean tree :)

thanks,

greg k-h
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Error returns not handled correctly by sysfs.c:subsys_attr_store()

2007-11-28 Thread Tejun Heo
Andrew Patterson wrote:
 I tried with clean 2.6.24-rc3 and get the same bad behavior.  This is on
 an ia64 box, so maybe that is an issue. I can try on an x86 box as well.
 Oh, one other thing.  I tried a uname -r to make sure I had the
 correct kernel booted and got:
 
 # uname -r
 2.6.24-rc3
 x
 y
 z
 #

Yeah, please try it on another machine from clean tree.  sysfs code is
definitely not endian dependent and is 64 bit clean.  Heck, all my test
machines run 64 bit these days.  I would be surprised if it's something
architecture dependent but please try on a different machine with
different userland with kernel built from fresh source tree.

Thanks.

-- 
tejun
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Error returns not handled correctly by sysfs.c:subsys_attr_store()

2007-11-27 Thread Tejun Heo
Greg KH wrote:
> On Mon, Nov 26, 2007 at 08:31:16PM -0800, Andrew Morton wrote:
>> On Wed, 21 Nov 2007 15:16:59 -0700 Andrew Patterson <[EMAIL PROTECTED]> 
>> wrote:
>>
>>> The buf in fs/sysfs.c:subsys_attr_store() does not seem to be updated
>>> correctly when returning a negative value (indicating that an error
>>> condition has occurred) is returned.  If a negative value is returned,
>>> the next subsequent call to subsys_attr_store will have the contents of
>>> buf appended to the previous call.
>> subsys_attr_store() gets deleted by
>> http://www.kernel.org/pub/linux/kernel/people/gregkh/gregkh-2.6/gregkh-01-driver/kset-kill-subsys-attr.patch
>>
>> So maybe we will soon accidentally fix whatever-this-is?  Or maybe we will
>> faithfully maintain it.
> 
> Yes, subsys attributes go away, but this is showing a bug in the sysfs
> core with attributes, not in the "middle" layers of attributes.
> 
> I bounced the original bug report to Tejun, who has been changing the
> logic around this area to see if he sees anything that might be
> different now.
> 
> Tejun?

Weird, the problem is not reproducible here.

# echo a > allow_restart
-bash: echo: write error: Invalid argument
[  437.518024] buf_ptr = 0x810005e2, buf = x
[  437.518027] , count = 2
# echo b > allow_restart
-bash: echo: write error: Invalid argument
[  438.972973] buf_ptr = 0x81001be6f000, buf = y
[  438.972976] , count = 2
# echo c > allow_restart
-bash: echo: write error: Invalid argument
[  440.539747] buf_ptr = 0x81001d4ba000, buf = z
[  440.539750] , count = 2

Which is expected.  On each open, sysfs_buffer is allocated with kzalloc
and the buffer is freed on close, so I don't see how it can happen.
Behavior for multiple write can be considered peculiar in that ppos is
essentially ignored and each write is passed just like brand new write
to ->store method but this too is the expected behavior.

# (echo a; echo b; echo c) > allow_restart
[  765.257132] buf_ptr = 0x81001be4f000, buf = a
[  765.257135] , count = 2
[  765.285474] buf_ptr = 0x81001be4f000, buf = b
[  765.285484] , count = 2
[  765.314002] buf_ptr = 0x81001be4f000, buf = c
[  765.314004] , count = 2
-bash: echo: write error: Invalid argument
-bash: echo: write error: Invalid argument
-bash: echo: write error: Invalid argument

Andrew Petterson, can you please build 2.6.24-rc3 from clean source tree
and retry?

-- 
tejun
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Error returns not handled correctly by sysfs.c:subsys_attr_store()

2007-11-27 Thread Tejun Heo
Greg KH wrote:
 On Mon, Nov 26, 2007 at 08:31:16PM -0800, Andrew Morton wrote:
 On Wed, 21 Nov 2007 15:16:59 -0700 Andrew Patterson [EMAIL PROTECTED] 
 wrote:

 The buf in fs/sysfs.c:subsys_attr_store() does not seem to be updated
 correctly when returning a negative value (indicating that an error
 condition has occurred) is returned.  If a negative value is returned,
 the next subsequent call to subsys_attr_store will have the contents of
 buf appended to the previous call.
 subsys_attr_store() gets deleted by
 http://www.kernel.org/pub/linux/kernel/people/gregkh/gregkh-2.6/gregkh-01-driver/kset-kill-subsys-attr.patch

 So maybe we will soon accidentally fix whatever-this-is?  Or maybe we will
 faithfully maintain it.
 
 Yes, subsys attributes go away, but this is showing a bug in the sysfs
 core with attributes, not in the middle layers of attributes.
 
 I bounced the original bug report to Tejun, who has been changing the
 logic around this area to see if he sees anything that might be
 different now.
 
 Tejun?

Weird, the problem is not reproducible here.

# echo a  allow_restart
-bash: echo: write error: Invalid argument
[  437.518024] buf_ptr = 0x810005e2, buf = x
[  437.518027] , count = 2
# echo b  allow_restart
-bash: echo: write error: Invalid argument
[  438.972973] buf_ptr = 0x81001be6f000, buf = y
[  438.972976] , count = 2
# echo c  allow_restart
-bash: echo: write error: Invalid argument
[  440.539747] buf_ptr = 0x81001d4ba000, buf = z
[  440.539750] , count = 2

Which is expected.  On each open, sysfs_buffer is allocated with kzalloc
and the buffer is freed on close, so I don't see how it can happen.
Behavior for multiple write can be considered peculiar in that ppos is
essentially ignored and each write is passed just like brand new write
to -store method but this too is the expected behavior.

# (echo a; echo b; echo c)  allow_restart
[  765.257132] buf_ptr = 0x81001be4f000, buf = a
[  765.257135] , count = 2
[  765.285474] buf_ptr = 0x81001be4f000, buf = b
[  765.285484] , count = 2
[  765.314002] buf_ptr = 0x81001be4f000, buf = c
[  765.314004] , count = 2
-bash: echo: write error: Invalid argument
-bash: echo: write error: Invalid argument
-bash: echo: write error: Invalid argument

Andrew Petterson, can you please build 2.6.24-rc3 from clean source tree
and retry?

-- 
tejun
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Error returns not handled correctly by sysfs.c:subsys_attr_store()

2007-11-26 Thread Tejun Heo
Greg KH wrote:
> On Mon, Nov 26, 2007 at 08:31:16PM -0800, Andrew Morton wrote:
>> On Wed, 21 Nov 2007 15:16:59 -0700 Andrew Patterson <[EMAIL PROTECTED]> 
>> wrote:
>>
>>> The buf in fs/sysfs.c:subsys_attr_store() does not seem to be updated
>>> correctly when returning a negative value (indicating that an error
>>> condition has occurred) is returned.  If a negative value is returned,
>>> the next subsequent call to subsys_attr_store will have the contents of
>>> buf appended to the previous call.
>> subsys_attr_store() gets deleted by
>> http://www.kernel.org/pub/linux/kernel/people/gregkh/gregkh-2.6/gregkh-01-driver/kset-kill-subsys-attr.patch
>>
>> So maybe we will soon accidentally fix whatever-this-is?  Or maybe we will
>> faithfully maintain it.
> 
> Yes, subsys attributes go away, but this is showing a bug in the sysfs
> core with attributes, not in the "middle" layers of attributes.
> 
> I bounced the original bug report to Tejun, who has been changing the
> logic around this area to see if he sees anything that might be
> different now.
> 
> Tejun?

(groaning buried under ATA bugs) Will take a look soon.

Thanks.

-- 
tejun
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Error returns not handled correctly by sysfs.c:subsys_attr_store()

2007-11-26 Thread Greg KH
On Mon, Nov 26, 2007 at 08:31:16PM -0800, Andrew Morton wrote:
> On Wed, 21 Nov 2007 15:16:59 -0700 Andrew Patterson <[EMAIL PROTECTED]> wrote:
> 
> > The buf in fs/sysfs.c:subsys_attr_store() does not seem to be updated
> > correctly when returning a negative value (indicating that an error
> > condition has occurred) is returned.  If a negative value is returned,
> > the next subsequent call to subsys_attr_store will have the contents of
> > buf appended to the previous call.
> 
> subsys_attr_store() gets deleted by
> http://www.kernel.org/pub/linux/kernel/people/gregkh/gregkh-2.6/gregkh-01-driver/kset-kill-subsys-attr.patch
> 
> So maybe we will soon accidentally fix whatever-this-is?  Or maybe we will
> faithfully maintain it.

Yes, subsys attributes go away, but this is showing a bug in the sysfs
core with attributes, not in the "middle" layers of attributes.

I bounced the original bug report to Tejun, who has been changing the
logic around this area to see if he sees anything that might be
different now.

Tejun?

thanks,

greg k-h
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Error returns not handled correctly by sysfs.c:subsys_attr_store()

2007-11-26 Thread Andrew Morton
On Wed, 21 Nov 2007 15:16:59 -0700 Andrew Patterson <[EMAIL PROTECTED]> wrote:

> The buf in fs/sysfs.c:subsys_attr_store() does not seem to be updated
> correctly when returning a negative value (indicating that an error
> condition has occurred) is returned.  If a negative value is returned,
> the next subsequent call to subsys_attr_store will have the contents of
> buf appended to the previous call.

subsys_attr_store() gets deleted by
http://www.kernel.org/pub/linux/kernel/people/gregkh/gregkh-2.6/gregkh-01-driver/kset-kill-subsys-attr.patch

So maybe we will soon accidentally fix whatever-this-is?  Or maybe we will
faithfully maintain it.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Error returns not handled correctly by sysfs.c:subsys_attr_store()

2007-11-26 Thread Andrew Morton
On Wed, 21 Nov 2007 15:16:59 -0700 Andrew Patterson [EMAIL PROTECTED] wrote:

 The buf in fs/sysfs.c:subsys_attr_store() does not seem to be updated
 correctly when returning a negative value (indicating that an error
 condition has occurred) is returned.  If a negative value is returned,
 the next subsequent call to subsys_attr_store will have the contents of
 buf appended to the previous call.

subsys_attr_store() gets deleted by
http://www.kernel.org/pub/linux/kernel/people/gregkh/gregkh-2.6/gregkh-01-driver/kset-kill-subsys-attr.patch

So maybe we will soon accidentally fix whatever-this-is?  Or maybe we will
faithfully maintain it.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Error returns not handled correctly by sysfs.c:subsys_attr_store()

2007-11-26 Thread Greg KH
On Mon, Nov 26, 2007 at 08:31:16PM -0800, Andrew Morton wrote:
 On Wed, 21 Nov 2007 15:16:59 -0700 Andrew Patterson [EMAIL PROTECTED] wrote:
 
  The buf in fs/sysfs.c:subsys_attr_store() does not seem to be updated
  correctly when returning a negative value (indicating that an error
  condition has occurred) is returned.  If a negative value is returned,
  the next subsequent call to subsys_attr_store will have the contents of
  buf appended to the previous call.
 
 subsys_attr_store() gets deleted by
 http://www.kernel.org/pub/linux/kernel/people/gregkh/gregkh-2.6/gregkh-01-driver/kset-kill-subsys-attr.patch
 
 So maybe we will soon accidentally fix whatever-this-is?  Or maybe we will
 faithfully maintain it.

Yes, subsys attributes go away, but this is showing a bug in the sysfs
core with attributes, not in the middle layers of attributes.

I bounced the original bug report to Tejun, who has been changing the
logic around this area to see if he sees anything that might be
different now.

Tejun?

thanks,

greg k-h
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Error returns not handled correctly by sysfs.c:subsys_attr_store()

2007-11-26 Thread Tejun Heo
Greg KH wrote:
 On Mon, Nov 26, 2007 at 08:31:16PM -0800, Andrew Morton wrote:
 On Wed, 21 Nov 2007 15:16:59 -0700 Andrew Patterson [EMAIL PROTECTED] 
 wrote:

 The buf in fs/sysfs.c:subsys_attr_store() does not seem to be updated
 correctly when returning a negative value (indicating that an error
 condition has occurred) is returned.  If a negative value is returned,
 the next subsequent call to subsys_attr_store will have the contents of
 buf appended to the previous call.
 subsys_attr_store() gets deleted by
 http://www.kernel.org/pub/linux/kernel/people/gregkh/gregkh-2.6/gregkh-01-driver/kset-kill-subsys-attr.patch

 So maybe we will soon accidentally fix whatever-this-is?  Or maybe we will
 faithfully maintain it.
 
 Yes, subsys attributes go away, but this is showing a bug in the sysfs
 core with attributes, not in the middle layers of attributes.
 
 I bounced the original bug report to Tejun, who has been changing the
 logic around this area to see if he sees anything that might be
 different now.
 
 Tejun?

(groaning buried under ATA bugs) Will take a look soon.

Thanks.

-- 
tejun
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Error returns not handled correctly by sysfs.c:subsys_attr_store()

2007-11-21 Thread Andrew Patterson
The buf in fs/sysfs.c:subsys_attr_store() does not seem to be updated
correctly when returning a negative value (indicating that an error
condition has occurred) is returned.  If a negative value is returned,
the next subsequent call to subsys_attr_store will have the contents of
buf appended to the previous call.  Example: I have modified the
sd.c:sd_store_allow_restart to always print out the contents of the buf
and return an error using the following patch:

--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -183,6 +183,9 @@ static ssize_t sd_store_allow_restart(struct class_device *c
struct scsi_disk *sdkp = to_scsi_disk(cdev);
struct scsi_device *sdp = sdkp->device;
 
+   printk(KERN_ERR "buf_ptr = 0x%p, buf = %s, count = %u\n", buf, buf, coun
+   return -EINVAL;
+
if (!capable(CAP_SYS_ADMIN))
return -EACCES;

I get the following output when writing invalid values to the
allow_restart sysfs file:

# echo x > /sys/class/scsi_disk/4:0:0:0/allow_restart
bash: echo: write error: Invalid argument
# echo y > /sys/class/scsi_disk/4:0:0:0/allow_restart
bash: echo: write error: Invalid argument
# echo z > /sys/class/scsi_disk/4:0:0:0/allow_restart
bash: echo: write error: Invalid argument

And the console output shows:

buf_ptr = 0xe1004bdb, buf = x
, count = 2
buf_ptr = 0xe1004bdb, buf = x
, count = 2
buf_ptr = 0xe1004bdb, buf = x
y
, count = 4
buf_ptr = 0xe1004bdb, buf = x
y
, count = 4
buf_ptr = 0xe1004caf, buf = x
y
z
, count = 6
buf_ptr = 0xe1004caf, buf = x
y
z
, count = 6

and the same append problem occurs when using another sysfs file:

# echo xyzzy > /sys/class/scsi_disk/4:0:1:0/allow_restart
bash: echo: write error: Invalid argument

buf_ptr = 0xe1004caf, buf = x
y
z
xyzzy
, count = 12

I found this problem in 2.6.24-rc3 and and an earlier version of 2.6.23.

This seems to work correctly on 2.6.18 (at least with the RHEL5 kernel I
did some testing with), i.e. the contents of buf from the previous
failed called are thrown away/overwritten. I looked through sysfs.c to
see if I could find anything obvious but could not see anything.
Perhaps this is handled at a higher level. 

-- 
Andrew Patterson
Hewlett-Packard

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Error returns not handled correctly by sysfs.c:subsys_attr_store()

2007-11-21 Thread Andrew Patterson
The buf in fs/sysfs.c:subsys_attr_store() does not seem to be updated
correctly when returning a negative value (indicating that an error
condition has occurred) is returned.  If a negative value is returned,
the next subsequent call to subsys_attr_store will have the contents of
buf appended to the previous call.  Example: I have modified the
sd.c:sd_store_allow_restart to always print out the contents of the buf
and return an error using the following patch:

--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -183,6 +183,9 @@ static ssize_t sd_store_allow_restart(struct class_device *c
struct scsi_disk *sdkp = to_scsi_disk(cdev);
struct scsi_device *sdp = sdkp-device;
 
+   printk(KERN_ERR buf_ptr = 0x%p, buf = %s, count = %u\n, buf, buf, coun
+   return -EINVAL;
+
if (!capable(CAP_SYS_ADMIN))
return -EACCES;

I get the following output when writing invalid values to the
allow_restart sysfs file:

# echo x  /sys/class/scsi_disk/4:0:0:0/allow_restart
bash: echo: write error: Invalid argument
# echo y  /sys/class/scsi_disk/4:0:0:0/allow_restart
bash: echo: write error: Invalid argument
# echo z  /sys/class/scsi_disk/4:0:0:0/allow_restart
bash: echo: write error: Invalid argument

And the console output shows:

buf_ptr = 0xe1004bdb, buf = x
, count = 2
buf_ptr = 0xe1004bdb, buf = x
, count = 2
buf_ptr = 0xe1004bdb, buf = x
y
, count = 4
buf_ptr = 0xe1004bdb, buf = x
y
, count = 4
buf_ptr = 0xe1004caf, buf = x
y
z
, count = 6
buf_ptr = 0xe1004caf, buf = x
y
z
, count = 6

and the same append problem occurs when using another sysfs file:

# echo xyzzy  /sys/class/scsi_disk/4:0:1:0/allow_restart
bash: echo: write error: Invalid argument

buf_ptr = 0xe1004caf, buf = x
y
z
xyzzy
, count = 12

I found this problem in 2.6.24-rc3 and and an earlier version of 2.6.23.

This seems to work correctly on 2.6.18 (at least with the RHEL5 kernel I
did some testing with), i.e. the contents of buf from the previous
failed called are thrown away/overwritten. I looked through sysfs.c to
see if I could find anything obvious but could not see anything.
Perhaps this is handled at a higher level. 

-- 
Andrew Patterson
Hewlett-Packard

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/