linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: Jeff Layton <jlayton@poochiereds.net>
Cc: Hugh Dickins <hughd@google.com>, angelo <angelo70@gmail.com>,
	Al Viro <viro@zeniv.linux.org.uk>,
	Andrew Morton <akpm@linux-foundation.org>,
	Christoph Hellwig <hch@infradead.org>,
	Andi Kleen <andi@firstfloor.org>, Eryu Guan <eguan@redhat.com>,
	linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	linux-mm@kvack.org
Subject: Re: [PATCH] mm: fix cpu hangs on truncating last page of a 16t sparse file
Date: Mon, 28 Sep 2015 11:06:29 +1000	[thread overview]
Message-ID: <20150928010629.GZ19114@dastard> (raw)
In-Reply-To: <20150927195655.18e20003@tlielax.poochiereds.net>

On Sun, Sep 27, 2015 at 07:56:55PM -0400, Jeff Layton wrote:
> On Mon, 28 Sep 2015 09:26:45 +1000 Dave Chinner <david@fromorbit.com> wrote:
> > On Sun, Sep 27, 2015 at 10:59:33AM -0700, Hugh Dickins wrote:
> > > > But if s_maxbytes doesn't have to be greater than MAX_LFS_FILESIZE,
> > > > i agree the issue should be fixed in layers above.
> > > 
> > > There is a "filesystems should never set s_maxbytes larger than
> > > MAX_LFS_FILESIZE" comment in fs/super.c, but unfortunately its
> > > warning is written with just 64-bit in mind (testing for negative).
> > 
> > Yup, introduced here:
> > 
> > commit 42cb56ae2ab67390da34906b27bedc3f2ff1393b
> > Author: Jeff Layton <jlayton@redhat.com>
> > Date:   Fri Sep 18 13:05:53 2009 -0700
> > 
> >     vfs: change sb->s_maxbytes to a loff_t
> >     
> >     sb->s_maxbytes is supposed to indicate the maximum size of a file that can
> >     exist on the filesystem.  It's declared as an unsigned long long.
> > 
> > And yes, that will never fire on a 32bit filesystem, because loff_t
> > is a "long long" type....
> > 
> 
> Hmm...should we change that to something like this instead?
> 
>     WARN(((unsigned long long)sb->s_maxbytes > (unsigned long long)MAX_LFS_FILESIZE,
> 	"%s set sb->s_maxbytes to too large a value (0x%llx)\n", type->name, sb->s_maxbytes);

Well, it doesn't change the fact that we've actually been supporting
sb->s_maxbytes > MAX_LFS_FILESIZE for a long time on 32 bit systems.
And it's pretty unfriendly to start issuing warnings on every mount
of every XFS filesystem on every 32 bit system in existence for
something we've explicitly supported since 2.4 kernels...

I suspect the warning should have been removed back in 2.6.34 like
was originally intended. :)

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

  reply	other threads:[~2015-09-28  1:06 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <560723F8.3010909@gmail.com>
2015-09-27  1:36 ` [PATCH] mm: fix cpu hangs on truncating last page of a 16t sparse file Hugh Dickins
2015-09-27  2:14   ` angelo
2015-09-27  2:21   ` angelo
2015-09-27 17:59     ` Hugh Dickins
2015-09-27 23:26       ` Dave Chinner
2015-09-27 23:56         ` Jeff Layton
2015-09-28  1:06           ` Dave Chinner [this message]
2015-09-28 17:03       ` Andi Kleen
2015-09-28 18:08         ` Hugh Dickins

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=20150928010629.GZ19114@dastard \
    --to=david@fromorbit.com \
    --cc=akpm@linux-foundation.org \
    --cc=andi@firstfloor.org \
    --cc=angelo70@gmail.com \
    --cc=eguan@redhat.com \
    --cc=hch@infradead.org \
    --cc=hughd@google.com \
    --cc=jlayton@poochiereds.net \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).