All of lore.kernel.org
 help / color / mirror / Atom feed
From: Arnd Bergmann <arnd@arndb.de>
To: Josef Bacik <josef@redhat.com>
Cc: linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	viro@zeniv.linux.org.uk, hch@infradead.org
Subject: Re: [PATCH] fs: kill default_llseek
Date: Sun, 8 May 2011 16:23:39 +0200	[thread overview]
Message-ID: <201105081623.39876.arnd@arndb.de> (raw)
In-Reply-To: <1304605677-1882-1-git-send-email-josef@redhat.com>

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.

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

  reply	other threads:[~2011-05-08 14:23 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-05-05 14:27 [PATCH] fs: kill default_llseek Josef Bacik
2011-05-08 14:23 ` Arnd Bergmann [this message]
2011-05-09 13:23   ` Josef Bacik
2011-05-09 14:06     ` Christoph Hellwig
  -- strict thread matches above, loose matches on Subject: below --
2011-05-09 18:03 Dave Anderson
2011-05-09 18:11 ` Josef Bacik
2011-05-09 18:30   ` Dave Anderson
2011-05-09 18:47     ` Josef Bacik

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=201105081623.39876.arnd@arndb.de \
    --to=arnd@arndb.de \
    --cc=hch@infradead.org \
    --cc=josef@redhat.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.