All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: "Darrick J. Wong" <darrick.wong@oracle.com>
Cc: Brian Foster <bfoster@redhat.com>,
	Eric Sandeen <sandeen@sandeen.net>,
	Eric Sandeen <sandeen@redhat.com>,
	linux-xfs <linux-xfs@vger.kernel.org>
Subject: Re: [PATCH 2/2 V2] xfs: toggle readonly state around xfs_log_mount_finish
Date: Fri, 17 Mar 2017 10:42:49 +1100	[thread overview]
Message-ID: <20170316234249.GW17542@dastard> (raw)
In-Reply-To: <20170316191500.GL5280@birch.djwong.org>

On Thu, Mar 16, 2017 at 12:15:00PM -0700, Darrick J. Wong wrote:
> On Wed, Mar 15, 2017 at 07:36:29AM -0400, Brian Foster wrote:
> > On Tue, Mar 14, 2017 at 06:23:57PM -0500, Eric Sandeen wrote:
> > > When we do log recovery on a readonly mount, unlinked inode
> > > processing does not happen due to the readonly checks in
> > > xfs_inactive(), which are trying to prevent any I/O on a
> > > readonly mount.
> > > 
> > > This is misguided - we do I/O on readonly mounts all the time,
> > > for consistency; for example, log recovery.  So do the same
> > > RDONLY flag twiddling around xfs_log_mount_finish() as we
> > > do around xfs_log_mount(), for the same reason.
> > > 
> > > This all cries out for a big rework but for now this is a
> > > simple fix to an obvious problem.
> > > 
> > > Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> > > ---
> > > 
> 
> Both patches look ok, so I'll put them on the test queue for -rc4.
> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

FWIW, I don't think this is a -rc candidate. Making log recovery
process unlinked inode transactions on read-only mounts is a pretty
major change in behaviour. Who knows exactly what dragons are
lurking at lower layers that have never been run in this context
until now.

Also, it's not urgent - we've lived with this behaviour for years -
so waiting a month for the next merge window is not going to hurt
anyone and it gives us a chance to test it - XFS developers are the
people who should be burnt by the lurking dragons, not users who
updated to a late -rcX kernel....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

  reply	other threads:[~2017-03-16 23:44 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-09 19:40 [PATCH 0/2] xfs: readonly handling changes Eric Sandeen
2017-03-09 20:15 ` [PATCH 1/2] xfs: write unmount record for ro mounts Eric Sandeen
2017-03-15 15:18   ` Brian Foster
2017-03-09 20:24 ` [PATCH 2/2] xfs: remove readonly checks from xfs_release & xfs_inactive Eric Sandeen
2017-03-09 20:39   ` Eric Sandeen
2017-03-13 13:23     ` Brian Foster
2017-03-13 22:16       ` Eric Sandeen
2017-03-14 11:35         ` Brian Foster
2017-03-14 23:23   ` [PATCH 2/2 V2] xfs: toggle readonly state around xfs_log_mount_finish Eric Sandeen
2017-03-15 11:36     ` Brian Foster
2017-03-16 19:15       ` Darrick J. Wong
2017-03-16 23:42         ` Dave Chinner [this message]
2017-03-16 23:52           ` Eric Sandeen
2017-03-18  7:38             ` Dave Chinner
2017-03-27 17:16               ` Darrick J. Wong
  -- strict thread matches above, loose matches on Subject: below --
2017-07-21 15:10 [PATCH 0/2 resend] xfs: readonly handling changes Eric Sandeen
2017-07-21 15:25 ` [PATCH 2/2] xfs: toggle readonly state around xfs_log_mount_finish Eric Sandeen
2017-08-11 19:45   ` [PATCH 2/2 V2] " Eric Sandeen
2017-08-11 19:47     ` Darrick J. Wong

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=20170316234249.GW17542@dastard \
    --to=david@fromorbit.com \
    --cc=bfoster@redhat.com \
    --cc=darrick.wong@oracle.com \
    --cc=linux-xfs@vger.kernel.org \
    --cc=sandeen@redhat.com \
    --cc=sandeen@sandeen.net \
    /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.