All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Theodore Ts'o" <tytso@mit.edu>
To: Christian Brauner <brauner@kernel.org>
Cc: Jan Kara <jack@suse.cz>, Al Viro <viro@zeniv.linux.org.uk>,
	linux-fsdevel@vger.kernel.org,
	David Howells <dhowells@redhat.com>
Subject: Re: [PATCH] fs: Protect reconfiguration of sb read-write from racing writes
Date: Thu, 15 Jun 2023 10:10:40 -0400	[thread overview]
Message-ID: <20230615141040.GG51259@mit.edu> (raw)
In-Reply-To: <20230615-zarte-locher-075323828cd1@brauner>

On Thu, Jun 15, 2023 at 02:53:53PM +0200, Christian Brauner wrote:
> 
> So looking at the ext4 code this can only happen when you clear
> SB_RDONLY in ->reconfigure() too early (and the mount isn't
> MNT_READONLY). Afaict, this was fixed in:
> 
> a44be64bbecb ("ext4: don't clear SB_RDONLY when remounting r/w until quota is re-enabled")
> 
> by clearing SB_RDONLY late, right before returning from ->reconfigure()
> when everything's ready. So your change is not about fixing that bug in
> [1] it's about making the vfs give the guarantee that an fs is free to
> clear SB_RDONLY because any ro<->rw transitions are protected via
> s_readonly_remount. Correct? It seems ok to me just making sure.

Unfortunately we had to revert that commit because that broke
r/o->r/w writes when quota was enabled.  The problem is we need a way
of enabling file system writes for internal purposes (e.g., because
quota needs to set up quota inodes) but *not* allow userspace file
system writes to occur until we are fully done with the remount process.

See the discussion here:

	https://lore.kernel.org/all/20230608044056.GA1418535@mit.edu/

The problem with the current state of the tree is commit dea9d8f7643f
("ext4: only check dquot_initialize_needed() when debugging") has
caught real bugs in the past where the caller of
ext4_xattr_block_set() failed to call dquot_initialize(inode).  In
addition, shutting up the warning doesn't fix the problem that while
we hit this race where we have started remounting r/w, quota hasn't
been initialized, quota tracking will get silently dropped, leading to
the quota usage tracking no longer reflecting reality.

Jan's patch will fix this problem.

Cheers,

						- Ted

  reply	other threads:[~2023-06-15 14:11 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-15 11:38 [PATCH] fs: Protect reconfiguration of sb read-write from racing writes Jan Kara
2023-06-15 12:53 ` Christian Brauner
2023-06-15 14:10   ` Theodore Ts'o [this message]
2023-06-15 14:48     ` Jan Kara
2023-06-15 15:01 ` Christian Brauner
2023-06-15 22:36 ` Dave Chinner
2023-06-16 16:37   ` Jan Kara
2023-06-16 22:48     ` Dave Chinner

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=20230615141040.GG51259@mit.edu \
    --to=tytso@mit.edu \
    --cc=brauner@kernel.org \
    --cc=dhowells@redhat.com \
    --cc=jack@suse.cz \
    --cc=linux-fsdevel@vger.kernel.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 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.