Messages in this thread | | | Date | Mon, 09 May 2011 09:23:35 -0400 | From | Josef Bacik <> | Subject | Re: [PATCH] fs: kill default_llseek |
| |
On 05/08/2011 10:23 AM, Arnd Bergmann wrote: > 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@redhat.com> > > 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. >
Right, I didn't miss it, I probably should have said this in my commit, but anybody who gets a super gets it from sget, which does alloc_super, which automatically sets s_maxbytes to MAX_NON_LFS, which is ((1UL<<31) - 1), plenty of size.
> 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. >
So every sb has s_maxbytes set to something, is this not acceptable for character devices? I guess some can let you seek well past this point so we should just do some if (S_ISCHR()) return or whatever? Thanks,
Josef
| |