[KJ] replacing old-style "non-seekable" errors with "no_llseek"

[KJ] replacing old-style "non-seekable" errors with "no_llseek"

[Robert P. J. Day]

  based on a discussion of lseek() here:

http://lwn.net/Articles/96662/

it appears that an old-style way of specifying that a device was
non-seekable:

    loff_t my_llseek(struct file *file, loff_t offset, int whence)
    {
        return -ESPIPE;    /* Not seekable */
    }

can be rewritten more cleanly using the no_llseek() helper routine,
and just assigning no_llseek to the llseek member of the
file_operations struct, as in:

    .llseek = no_llseek

and deleting the old routine.  am i reading that correctly?  or is
there more to it than that?

rday
-- 
====================================
Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA

http://fsdev.net/wiki/index.php?title=Main_Page
====================================
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/kernel-janitors

Re: [KJ] replacing old-style "non-seekable" errors with "no_llseek"

[Arnd Bergmann]

On Sunday 27 May 2007, Robert P. J. Day wrote:
> can be rewritten more cleanly using the no_llseek() helper routine,
> and just assigning no_llseek to the llseek member of the
> file_operations struct, as in:
> 
>     .llseek = no_llseek
> 
> and deleting the old routine.  am i reading that correctly?  or is
> there more to it than that?
 
Actually, that is outdated as well.

If the file descriptor cannot be seeked, you should set the open
operation to nonseekable_open, or call that function from the
driver's own open function.

	Arnd <><

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/kernel-janitors

Re: [KJ] replacing old-style "non-seekable" errors with "no_llseek"

[Robert P. J. Day]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1470 bytes --]

On Mon, 28 May 2007, Arnd Bergmann wrote:

> On Sunday 27 May 2007, Robert P. J. Day wrote:
> > can be rewritten more cleanly using the no_llseek() helper routine,
> > and just assigning no_llseek to the llseek member of the
> > file_operations struct, as in:
> >
> >     .llseek = no_llseek
> >
> > and deleting the old routine.  am i reading that correctly?  or is
> > there more to it than that?
>
> Actually, that is outdated as well.
>
> If the file descriptor cannot be seeked, you should set the open
> operation to nonseekable_open, or call that function from the
> driver's own open function.

ah, yes, i see that further down in the very article i linked to:

  http://lwn.net/Articles/97154/

so, in a nutshell, is whether something is seekable a yes/no
proposition?  or are there shades of gray to worry about?

that is, if it's obvious that some device is being set to be
non-seekable using one of the older techniques, is it appropriate to
just make it non-seekable using nonseekable_open()?

if not, is there some example in the kernel tree you can point at that
shows cases that deviate from the norm?  thanks.

rday
-- 
========================================================================
Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA

http://fsdev.net/wiki/index.php?title=Main_Page
========================================================================

[-- Attachment #2: Type: text/plain, Size: 187 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/kernel-janitors

Re: [KJ] replacing old-style "non-seekable" errors with "no_llseek"

[Robert P. J. Day]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2273 bytes --]

On Mon, 28 May 2007, Arnd Bergmann wrote:

> On Sunday 27 May 2007, Robert P. J. Day wrote:
> > can be rewritten more cleanly using the no_llseek() helper routine,
> > and just assigning no_llseek to the llseek member of the
> > file_operations struct, as in:
> >
> >     .llseek = no_llseek
> >
> > and deleting the old routine.  am i reading that correctly?  or is
> > there more to it than that?
>
> Actually, that is outdated as well.
>
> If the file descriptor cannot be seeked, you should set the open
> operation to nonseekable_open, or call that function from the
> driver's own open function.

  ok, after poking around in the code and submitting a few cleanup
patches, i want to make sure i'm doing this correctly.  once the
open() routine for a device invokes nonseekable_open(), does that mean
that *all* of those old-style tests for seekability can be removed?

  first, it seems obvious that declaring bogus llseek methods like
this can be deleted:

=================================
loff_t my_llseek(...)
{
	return -ESPIPE;  /* Not seekable. */
}
=================================

  next, there were explicit tests in the device read() and write()
methods that checked if pread() or pwrite() had been invoked:

=================================
...
if (ppos != &filp->f_pos)
    return -ESPIPE;
}
...
=================================

as i read it, those can also be removed.  and, finally, i notice a few
read() (and perhaps write()) implementations that contain the check:

=================================
...
if (*ppos)
    return -ESPIPE;
...
=================================

  that appears to simply be checking if you're trying to read from
anywhere but the beginning of the file.  is that also covered under
the nonseekable_open() call?  what exactly does that test represent?
should it be removed once nonseekable_open() is being invoked?

  thanks for any clarification.

rday
-- 
========================================================================
Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA

http://fsdev.net/wiki/index.php?title=Main_Page
========================================================================

[-- Attachment #2: Type: text/plain, Size: 187 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/kernel-janitors