lseek() on debugfs entries in 2.6.37

lseek() on debugfs entries in 2.6.37

[Alexey Mikhailov]

Hello!

I use simple debugfs entries for user-space <-> kernel-space
interaction. Basically I read unsigned integers from debugfs
files like this:

   ...
   char buf[64];
   lseek(timesync_fd, 0, SEEK_SET);
   read(timesync_fd, buf, sizeof(buf));
   ...

It works perfectly with 2.6.32 kernel. But with 2.6.37 kernel,
lseek() fails with errno=29(Illegal seek). So second read()
call just fails or returns garbage. Can someone please shed
some light on it?

Thank you,
-- Alexey.

Re: lseek() on debugfs entries in 2.6.37

[Arnd Bergmann]

On Tuesday 15 March 2011, Alexey Mikhailov wrote:
> I use simple debugfs entries for user-space <-> kernel-space
> interaction. Basically I read unsigned integers from debugfs
> files like this:
> 
>    ...
>    char buf[64];
>    lseek(timesync_fd, 0, SEEK_SET);
>    read(timesync_fd, buf, sizeof(buf));
>    ...
> 
> It works perfectly with 2.6.32 kernel. But with 2.6.37 kernel,
> lseek() fails with errno=29(Illegal seek). So second read()
> call just fails or returns garbage. Can someone please shed
> some light on it?

This is probably a result of the changes I made as part of the
BKL removal. Which file specifically are you talking about?

In older kernels, having no .llseek function meant that you
implicitly get default_llseek. New kernels now require 
that the driver explicitly chooses one llseek variant.

I did a patch to automatically convert all drivers, but
this patch may have missed some that got moved around
while I was patching them.

	Arnd

Re: lseek() on debugfs entries in 2.6.37

[Steven Rostedt]

On Tue, Mar 15, 2011 at 03:54:14PM +0300, Alexey Mikhailov wrote:
> Hello!
> 
> I use simple debugfs entries for user-space <-> kernel-space
> interaction. Basically I read unsigned integers from debugfs
> files like this:

What debugfs file? Is this something you created?

> 
>    ...
>    char buf[64];
>    lseek(timesync_fd, 0, SEEK_SET);
>    read(timesync_fd, buf, sizeof(buf));
>    ...
> 
> It works perfectly with 2.6.32 kernel. But with 2.6.37 kernel,
> lseek() fails with errno=29(Illegal seek). So second read()
> call just fails or returns garbage. Can someone please shed
> some light on it?
> 

There's been a lot of clean ups in the debugfs to patch lseek in files
that just didn't support it. Could be one of the files that got the
clean up. The lseek may have failed silently before, but now it
produceses an error.

-- Steve

Re: lseek() on debugfs entries in 2.6.37

[Alexey Mikhailov]

At Tue, 15 Mar 2011 15:01:41 +0100, Arnd Bergmann wrote:
Hello!

> 
> On Tuesday 15 March 2011, Alexey Mikhailov wrote:
> > I use simple debugfs entries for user-space <-> kernel-space
> > interaction. Basically I read unsigned integers from debugfs
> > files like this:
> > 
> >    ...
> >    char buf[64];
> >    lseek(timesync_fd, 0, SEEK_SET);
> >    read(timesync_fd, buf, sizeof(buf));
> >    ...
> > 
> > It works perfectly with 2.6.32 kernel. But with 2.6.37 kernel,
> > lseek() fails with errno=29(Illegal seek). So second read()
> > call just fails or returns garbage. Can someone please shed
> > some light on it?
> 
> This is probably a result of the changes I made as part of the
> BKL removal. Which file specifically are you talking about?
> 
> In older kernels, having no .llseek function meant that you
> implicitly get default_llseek. New kernels now require 
> that the driver explicitly chooses one llseek variant.

Thank you for reply. Obvious patches like this one fixed it

   static struct file_operations fops_timesync = {
      .owner = THIS_MODULE,
      .open = fop_open_timesync,
      .read = fop_read_timesync,
  +   .llseek = default_llseek,
   };

I assume there is the reason you need to specify this
explicitly as it will break much out-of-kernel code.

-- Alexey

Re: lseek() on debugfs entries in 2.6.37

[Arnd Bergmann]

On Wednesday 16 March 2011, Alexey Mikhailov wrote:
> Thank you for reply. Obvious patches like this one fixed it
> 
>    static struct file_operations fops_timesync = {
>       .owner = THIS_MODULE,
>       .open = fop_open_timesync,
>       .read = fop_read_timesync,
>   +   .llseek = default_llseek,
>    };
> 
> I assume there is the reason you need to specify this
> explicitly as it will break much out-of-kernel code.

Yes, this was required as a prerequisite to removing the big
kernel lock. Generally speaking, there is very little care taken
to prevent out of tree modules from breaking. If you have device
drivers that you care about, I recommend submitting them for
inclusion in drivers/staging or as a proper driver in the mainline
kernel.

	Arnd