From mboxrd@z Thu Jan 1 00:00:00 1970 From: Josef Bacik Subject: Re: [PATCH] fs: kill default_llseek Date: Mon, 09 May 2011 09:23:35 -0400 Message-ID: <4DC7EAD7.9070606@redhat.com> References: <1304605677-1882-1-git-send-email-josef@redhat.com> <201105081623.39876.arnd@arndb.de> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-15; format=flowed Content-Transfer-Encoding: 7bit Cc: linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org, viro@zeniv.linux.org.uk, hch@infradead.org To: Arnd Bergmann Return-path: In-Reply-To: <201105081623.39876.arnd@arndb.de> Sender: linux-kernel-owner@vger.kernel.org List-Id: linux-fsdevel.vger.kernel.org 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 > > 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