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@xxxxxxxxxxxxxx> 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
