Linux-BTRFS Archive on lore.kernel.org
 help / color / Atom feed
From: Josef Bacik <josef@toxicpanda.com>
To: Filipe Manana <fdmanana@gmail.com>
Cc: Josef Bacik <josef@toxicpanda.com>,
	linux-btrfs <linux-btrfs@vger.kernel.org>,
	kernel-team@fb.com, stable@vger.kernel.org
Subject: Re: [PATCH] btrfs: save i_size in compress_file_range
Date: Fri, 11 Oct 2019 13:15:28 -0400
Message-ID: <20191011171527.v4rdw6ex3fcnqzmw@macbook-pro-91.dhcp.thefacebook.com> (raw)
In-Reply-To: <CAL3q7H79yipNYZgO2Es-Zn0mBKbWoH07Zdv4P473C5NQv9fEqA@mail.gmail.com>

On Fri, Oct 11, 2019 at 04:28:28PM +0100, Filipe Manana wrote:
> On Fri, Oct 11, 2019 at 2:05 PM Josef Bacik <josef@toxicpanda.com> wrote:
> >
> > We hit a regression while rolling out 5.2 internally where we were
> > hitting the following panic
> >
> > kernel BUG at mm/page-writeback.c:2659!
> > RIP: 0010:clear_page_dirty_for_io+0xe6/0x1f0
> > Call Trace:
> >  __process_pages_contig+0x25a/0x350
> >  ? extent_clear_unlock_delalloc+0x43/0x70
> >  submit_compressed_extents+0x359/0x4d0
> >  normal_work_helper+0x15a/0x330
> >  process_one_work+0x1f5/0x3f0
> >  worker_thread+0x2d/0x3d0
> >  ? rescuer_thread+0x340/0x340
> >  kthread+0x111/0x130
> >  ? kthread_create_on_node+0x60/0x60
> >  ret_from_fork+0x1f/0x30
> >
> > this is happening because the page is not locked when doing
> > clear_page_dirty_for_io.  Looking at the core dump it was because our
> > async_extent had a ram_size of 24576 but our async_chunk range only
> > spanned 20480, so we had a whole extra page in our ram_size for our
> > async_extent.
> >
> > This happened because we try not to compress pages outside of our
> > i_size, however a cleanup patch changed us to do
> >
> > actual_end = min_t(u64, i_size_read(inode), end + 1);
> >
> > which is problematic because i_size_read() can evaluate to different
> > values in between checking and assigning.  So either a expanding
> > truncate or a fallocate could increase our i_size while we're doing
> 
> Well, not just those operations but a write starting at or beyond eof
> could also do it,
> since at writeback time we don't hold the inode's lock and the
> buffered write path doesn't lock the range if it starts at or past
> current i_size.
>

Yeah it was just for example.

> > writeout and actual_end would end up being past the range we have
> > locked.
> >
> > I confirmed this was what was happening by installing a debug kernel
> > that had
> >
> > actual_end = min_t(u64, i_size_read(inode), end + 1);
> > if (actual_end > end + 1) {
> >         printk(KERN_ERR "WE GOT FUCKED\n");
> 
> I think we coud be more polite on changelogs and mailing lists, or we
> could add a tag in the subjects warning about improper content like
> video games and music records :)
> 

Hah, well that's what I put, but we can change it for the changelog if it's a
problem.  Thanks,

Josef

  reply index

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-11 13:03 Josef Bacik
2019-10-11 15:28 ` Filipe Manana
2019-10-11 17:15   ` Josef Bacik [this message]
2019-10-14 11:52 ` David Sterba

Reply instructions:

You may reply publically 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=20191011171527.v4rdw6ex3fcnqzmw@macbook-pro-91.dhcp.thefacebook.com \
    --to=josef@toxicpanda.com \
    --cc=fdmanana@gmail.com \
    --cc=kernel-team@fb.com \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    /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

Linux-BTRFS Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-btrfs/0 linux-btrfs/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-btrfs linux-btrfs/ https://lore.kernel.org/linux-btrfs \
		linux-btrfs@vger.kernel.org linux-btrfs@archiver.kernel.org
	public-inbox-index linux-btrfs

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-btrfs


AGPL code for this site: git clone https://public-inbox.org/ public-inbox