All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jan Kara <jack@suse.cz>
To: Theodore Ts'o <tytso@mit.edu>
Cc: Ext4 Developers List <linux-ext4@vger.kernel.org>,
	bagasdotme@gmail.com, nikolas.kraetzschmar@sap.com, jack@suse.cz
Subject: Re: [PATCH 1/2] Revert "ext4: don't clear SB_RDONLY when remounting r/w until quota is re-enabled"
Date: Thu, 15 Jun 2023 12:01:26 +0200	[thread overview]
Message-ID: <20230615100126.othz2jtof4av4pur@quack3> (raw)
In-Reply-To: <20230608141805.1434230-1-tytso@mit.edu>

On Thu 08-06-23 10:18:04, Theodore Ts'o wrote:
> This reverts commit a44be64bbecb15a452496f60db6eacfee2b59c79.
> 
> Link: https://lore.kernel.org/r/653b3359-2005-21b1-039d-c55ca4cffdcc@gmail.com
> Signed-off-by: Theodore Ts'o <tytso@mit.edu>

So I was looking more into how the warning in xattr code can trigger. Sadly
the syzbot reproducer does not seem to reproduce the issue for me when I
enable the warnings in xattr code. Anyway, after staring for some time into
the code I think the problem has actually been introduced by the transition
to the new mount API. Because in the old API, userspace could not start
writes to the filesystem until we called set_mount_attributes() in
do_remount() which cleared the MNT_READONLY flag on the mount (which
happens after reconfigure_super(). In the new API, fsconfig(2) syscall
allows you to toggle SB_RDONLY flag without touching the mount flags and so
we can have userspace writing the filesystem as soon as we clear SB_RDONLY
flag.

I have checked and the other direction (i.e., remount read-only) is
properly serialized in the VFS by setting sb->s_readonly_remount (and
making sure we either return EBUSY or all writers are going to see
s_readonly_remount set) before calling into filesystem's reconfigure code.
I have actually a fixup within ext4 ready but I think it may be better to
fix this within VFS and provide similar serialization like for the rw-ro
transition there... Let's see what VFS people say to this.

								Honza

-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

  parent reply	other threads:[~2023-06-15 10:01 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-07  5:51 Fwd: Remounting ext4 filesystem from ro to rw fails when quotas are enabled Bagas Sanjaya
2023-06-08  4:40 ` Theodore Ts'o
2023-06-08 14:18   ` [PATCH 1/2] Revert "ext4: don't clear SB_RDONLY when remounting r/w until quota is re-enabled" Theodore Ts'o
2023-06-08 14:18     ` [PATCH 2/2] ext4: only check dquot_initialize_needed() when debugging Theodore Ts'o
2023-06-15 10:01     ` Jan Kara [this message]
2023-06-08 17:52   ` Fwd: Remounting ext4 filesystem from ro to rw fails when quotas are enabled Jan Kara

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=20230615100126.othz2jtof4av4pur@quack3 \
    --to=jack@suse.cz \
    --cc=bagasdotme@gmail.com \
    --cc=linux-ext4@vger.kernel.org \
    --cc=nikolas.kraetzschmar@sap.com \
    --cc=tytso@mit.edu \
    /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.