mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Vincent Whitchurch <Vincent.Whitchurch@axis.com>
To: Vincent Whitchurch <Vincent.Whitchurch@axis.com>,
	"phillip@squashfs.org.uk" <phillip@squashfs.org.uk>,
	"akpm@linux-foundation.org" <akpm@linux-foundation.org>
Cc: "hch@lst.de" <hch@lst.de>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	kernel <kernel@axis.com>
Subject: Re: [PATCH mm-nonmm-unstable v2 1/2] squashfs: fix page update race
Date: Thu, 29 Jun 2023 10:57:37 +0000	[thread overview]
Message-ID: <d7e032358b10a1f472f452e5170aa5cc8cbb9a81.camel@axis.com> (raw)
In-Reply-To: <20230526-squashfs-cache-fixup-v2-1-6fb7723c3647@axis.com>

On Fri, 2023-05-26 at 15:57 +0200, Vincent Whitchurch wrote:
> We only put the page into the cache after we've read it, so the
> PageUptodate() check should not be necessary.  In fact, it's actively
> harmful since the check could fail (since we used find_get_page() and
> not find_lock_page()) and we could end up submitting a page for I/O
> after it has been read and while it's actively being used, which could
> lead to corruption depending on what the block driver does with it.

It turns out that removing the PageUptodate() check entirely wasn't
correct.

While it's true that the squashfs code only puts the page into the cache
after it's been read as I wrote above, migration on the other hand
replaces the page in the mapping _before_ copying the contents over, so
a PageUptodate() check is still needed.

The original problem can be fixed by moving the PageUptodate() check to
squashfs_bio_read() and ignoring the cached page entirely if it's not up
to date.

I'll post a fix shortly.

  parent reply	other threads:[~2023-06-29 10:57 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-26 13:57 [PATCH mm-nonmm-unstable v2 0/2] squashfs: fixups for caching Vincent Whitchurch
2023-05-26 13:57 ` [PATCH mm-nonmm-unstable v2 1/2] squashfs: fix page update race Vincent Whitchurch
2023-05-26 17:59   ` Phillip Lougher
2023-06-29 10:57   ` Vincent Whitchurch [this message]
2023-05-26 13:57 ` [PATCH mm-nonmm-unstable v2 2/2] squashfs: fix page indices Vincent Whitchurch
2023-05-26 14:02   ` Christoph Hellwig
2023-05-26 17:59   ` Phillip Lougher

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=d7e032358b10a1f472f452e5170aa5cc8cbb9a81.camel@axis.com \
    --to=vincent.whitchurch@axis.com \
    --cc=akpm@linux-foundation.org \
    --cc=hch@lst.de \
    --cc=kernel@axis.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=phillip@squashfs.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

all inboxes | Powered by JetHome®