Re: [PATCH] fs: kill default_llseek

[Arnd Bergmann]

On Thursday 05 May 2011 16:27:57 Josef Bacik wrote:
> Looking at this llseek stuff I noticed that default_llseek is the exact same as
> generic_file_llseek, so kill default_llseek.  I patched this using spatch with
> just a simple
> 
> @@
> @@
> 
> - default_llseek
> + generic_file_llseek
> 
> and then I manually removed the default_llseek definitions.  I've built and
> booted this and it works fine.  Thanks,
> 
> Signed-off-by: Josef Bacik <josef@xxxxxxxxxx>

Good idea in principle, but you missed the fact that generic_file_llseek
checks (offset > inode->i_sb->s_maxbytes), which default_llseek
does not.

This makes a huge difference on file systems that do not initialize
s_maxbytes, leaving it at zero. In particular, it doesn't work for
character devices, IIRC they will use whatever s_maxbytes is set for
the file system that stores the dentry and for debugfs and other
virtual file systems, it is not set at all.

The proper fix should be to make sure all file systems set an appropriate
s_maxbytes, and then add a special case for character devices to
generic_file_llseek. The main reason I didn't do it last year was to
avoid possible regressions when missing some other special cases, but
please continue to push this once you are sure to have a correct version
of the patch.

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