* [PATCH] btrfs: resume qgroup rescan on rw remount
@ 2017-07-04 11:49 Aleksa Sarai
2017-07-10 13:12 ` Nikolay Borisov
0 siblings, 1 reply; 6+ messages in thread
From: Aleksa Sarai @ 2017-07-04 11:49 UTC (permalink / raw)
To: Chris Mason, Josef Bacik, David Sterba
Cc: linux-btrfs, linux-kernel, Aleksa Sarai, stable, Jeff Mahoney
Several distributions mount the "proper root" as ro during initrd and
then remount it as rw before pivot_root(2). Thus, if a rescan had been
aborted by a previous shutdown, the rescan would never be resumed.
This issue would manifest itself as several btrfs ioctl(2)s causing the
entire machine to hang when btrfs_qgroup_wait_for_completion was hit
(due to the fs_info->qgroup_rescan_running flag being set but the rescan
itself not being resumed). Notably, Docker's btrfs storage driver makes
regular use of BTRFS_QUOTA_CTL_DISABLE and BTRFS_IOC_QUOTA_RESCAN_WAIT
(causing this problem to be manifested on boot for some machines).
Cc: <stable@vger.kernel.org> # v3.11+
Cc: Jeff Mahoney <jeffm@suse.com>
Fixes: b382a324b60f ("Btrfs: fix qgroup rescan resume on mount")
Signed-off-by: Aleksa Sarai <asarai@suse.de>
---
fs/btrfs/super.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 6346876c97ea..ff6690389343 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -1821,6 +1821,8 @@ static int btrfs_remount(struct super_block *sb, int *flags, char *data)
goto restore;
}
+ btrfs_qgroup_rescan_resume(fs_info);
+
if (!fs_info->uuid_root) {
btrfs_info(fs_info, "creating UUID tree");
ret = btrfs_create_uuid_tree(fs_info);
--
2.13.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] btrfs: resume qgroup rescan on rw remount
2017-07-04 11:49 [PATCH] btrfs: resume qgroup rescan on rw remount Aleksa Sarai
@ 2017-07-10 13:12 ` Nikolay Borisov
2017-07-10 13:56 ` Nikolay Borisov
0 siblings, 1 reply; 6+ messages in thread
From: Nikolay Borisov @ 2017-07-10 13:12 UTC (permalink / raw)
To: Aleksa Sarai, Chris Mason, Josef Bacik, David Sterba
Cc: linux-btrfs, linux-kernel, stable, Jeff Mahoney
On 4.07.2017 14:49, Aleksa Sarai wrote:
> Several distributions mount the "proper root" as ro during initrd and
> then remount it as rw before pivot_root(2). Thus, if a rescan had been
> aborted by a previous shutdown, the rescan would never be resumed.
>
> This issue would manifest itself as several btrfs ioctl(2)s causing the
> entire machine to hang when btrfs_qgroup_wait_for_completion was hit
> (due to the fs_info->qgroup_rescan_running flag being set but the rescan
> itself not being resumed). Notably, Docker's btrfs storage driver makes
> regular use of BTRFS_QUOTA_CTL_DISABLE and BTRFS_IOC_QUOTA_RESCAN_WAIT
> (causing this problem to be manifested on boot for some machines).
>
> Cc: <stable@vger.kernel.org> # v3.11+
> Cc: Jeff Mahoney <jeffm@suse.com>
> Fixes: b382a324b60f ("Btrfs: fix qgroup rescan resume on mount")
> Signed-off-by: Aleksa Sarai <asarai@suse.de>
Indeed, looking at the code it seems that b382a324b60f ("Btrfs: fix
qgroup rescan resume on mount") missed adding the qgroup_rescan_resume
in the remount path. One thing which I couldn't verify though is whether
reading fs_info->qgroup_flags without any locking is safe from remount
context.
During remount I don't see any locks taken that prevent operations which
can modify qgroup_flags.
> ---
> fs/btrfs/super.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> index 6346876c97ea..ff6690389343 100644
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -1821,6 +1821,8 @@ static int btrfs_remount(struct super_block *sb, int *flags, char *data)
> goto restore;
> }
>
> + btrfs_qgroup_rescan_resume(fs_info);
> +
> if (!fs_info->uuid_root) {
> btrfs_info(fs_info, "creating UUID tree");
> ret = btrfs_create_uuid_tree(fs_info);
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] btrfs: resume qgroup rescan on rw remount
2017-07-10 13:12 ` Nikolay Borisov
@ 2017-07-10 13:56 ` Nikolay Borisov
2017-07-11 17:03 ` David Sterba
0 siblings, 1 reply; 6+ messages in thread
From: Nikolay Borisov @ 2017-07-10 13:56 UTC (permalink / raw)
To: Aleksa Sarai, Chris Mason, Josef Bacik, David Sterba
Cc: linux-btrfs, linux-kernel, stable, Jeff Mahoney
On 10.07.2017 16:12, Nikolay Borisov wrote:
>
>
> On 4.07.2017 14:49, Aleksa Sarai wrote:
>> Several distributions mount the "proper root" as ro during initrd and
>> then remount it as rw before pivot_root(2). Thus, if a rescan had been
>> aborted by a previous shutdown, the rescan would never be resumed.
>>
>> This issue would manifest itself as several btrfs ioctl(2)s causing the
>> entire machine to hang when btrfs_qgroup_wait_for_completion was hit
>> (due to the fs_info->qgroup_rescan_running flag being set but the rescan
>> itself not being resumed). Notably, Docker's btrfs storage driver makes
>> regular use of BTRFS_QUOTA_CTL_DISABLE and BTRFS_IOC_QUOTA_RESCAN_WAIT
>> (causing this problem to be manifested on boot for some machines).
>>
>> Cc: <stable@vger.kernel.org> # v3.11+
>> Cc: Jeff Mahoney <jeffm@suse.com>
>> Fixes: b382a324b60f ("Btrfs: fix qgroup rescan resume on mount")
>> Signed-off-by: Aleksa Sarai <asarai@suse.de>
>
> Indeed, looking at the code it seems that b382a324b60f ("Btrfs: fix
> qgroup rescan resume on mount") missed adding the qgroup_rescan_resume
> in the remount path. One thing which I couldn't verify though is whether
> reading fs_info->qgroup_flags without any locking is safe from remount
> context.
>
> During remount I don't see any locks taken that prevent operations which
> can modify qgroup_flags.
>
>
Further inspection reveals that the access rules to qgroup_flags are
somewhat broken so this patch doesn't really make things any worse than
they are. As such:
Reviewed-by: Nikolay Borisov <nborisov@suse.com>
Tested-by: Nikolay Borisov <nborisov@suse.com>
>
>> ---
>> fs/btrfs/super.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
>> index 6346876c97ea..ff6690389343 100644
>> --- a/fs/btrfs/super.c
>> +++ b/fs/btrfs/super.c
>> @@ -1821,6 +1821,8 @@ static int btrfs_remount(struct super_block *sb, int *flags, char *data)
>> goto restore;
>> }
>>
>> + btrfs_qgroup_rescan_resume(fs_info);
>> +
>> if (!fs_info->uuid_root) {
>> btrfs_info(fs_info, "creating UUID tree");
>> ret = btrfs_create_uuid_tree(fs_info);
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] btrfs: resume qgroup rescan on rw remount
2017-07-10 13:56 ` Nikolay Borisov
@ 2017-07-11 17:03 ` David Sterba
2017-08-26 6:39 ` Aleksa Sarai
0 siblings, 1 reply; 6+ messages in thread
From: David Sterba @ 2017-07-11 17:03 UTC (permalink / raw)
To: Nikolay Borisov
Cc: Aleksa Sarai, Chris Mason, Josef Bacik, David Sterba, linux-btrfs,
linux-kernel, stable, Jeff Mahoney
On Mon, Jul 10, 2017 at 04:56:36PM +0300, Nikolay Borisov wrote:
> On 10.07.2017 16:12, Nikolay Borisov wrote:
> > On 4.07.2017 14:49, Aleksa Sarai wrote:
> >> Several distributions mount the "proper root" as ro during initrd and
> >> then remount it as rw before pivot_root(2). Thus, if a rescan had been
> >> aborted by a previous shutdown, the rescan would never be resumed.
> >>
> >> This issue would manifest itself as several btrfs ioctl(2)s causing the
> >> entire machine to hang when btrfs_qgroup_wait_for_completion was hit
> >> (due to the fs_info->qgroup_rescan_running flag being set but the rescan
> >> itself not being resumed). Notably, Docker's btrfs storage driver makes
> >> regular use of BTRFS_QUOTA_CTL_DISABLE and BTRFS_IOC_QUOTA_RESCAN_WAIT
> >> (causing this problem to be manifested on boot for some machines).
> >>
> >> Cc: <stable@vger.kernel.org> # v3.11+
> >> Cc: Jeff Mahoney <jeffm@suse.com>
> >> Fixes: b382a324b60f ("Btrfs: fix qgroup rescan resume on mount")
> >> Signed-off-by: Aleksa Sarai <asarai@suse.de>
> >
> > Indeed, looking at the code it seems that b382a324b60f ("Btrfs: fix
> > qgroup rescan resume on mount") missed adding the qgroup_rescan_resume
> > in the remount path. One thing which I couldn't verify though is whether
> > reading fs_info->qgroup_flags without any locking is safe from remount
> > context.
> >
> > During remount I don't see any locks taken that prevent operations which
> > can modify qgroup_flags.
>
> Further inspection reveals that the access rules to qgroup_flags are
> somewhat broken so this patch doesn't really make things any worse than
> they are.
The usage follows a pattern for a bitfield, updated by set_bit/clear_bit
etc. The updates to the state or inconsistency is not safe, so some
updates could get lost under some circumstances.
Patch added to devel queue, possibly will be submitted to 4.13 so stable
can pick it.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] btrfs: resume qgroup rescan on rw remount
2017-07-11 17:03 ` David Sterba
@ 2017-08-26 6:39 ` Aleksa Sarai
2017-08-30 10:48 ` David Sterba
0 siblings, 1 reply; 6+ messages in thread
From: Aleksa Sarai @ 2017-08-26 6:39 UTC (permalink / raw)
To: dsterba, Nikolay Borisov, Chris Mason, Josef Bacik, David Sterba,
linux-btrfs, linux-kernel, stable
On 07/12/2017 03:03 AM, David Sterba wrote:
> On Mon, Jul 10, 2017 at 04:56:36PM +0300, Nikolay Borisov wrote:
>> On 10.07.2017 16:12, Nikolay Borisov wrote:
>>> On 4.07.2017 14:49, Aleksa Sarai wrote:
>>>> Several distributions mount the "proper root" as ro during initrd and
>>>> then remount it as rw before pivot_root(2). Thus, if a rescan had been
>>>> aborted by a previous shutdown, the rescan would never be resumed.
>>>>
>>>> This issue would manifest itself as several btrfs ioctl(2)s causing the
>>>> entire machine to hang when btrfs_qgroup_wait_for_completion was hit
>>>> (due to the fs_info->qgroup_rescan_running flag being set but the rescan
>>>> itself not being resumed). Notably, Docker's btrfs storage driver makes
>>>> regular use of BTRFS_QUOTA_CTL_DISABLE and BTRFS_IOC_QUOTA_RESCAN_WAIT
>>>> (causing this problem to be manifested on boot for some machines).
>>>>
>>>> Cc: <stable@vger.kernel.org> # v3.11+
>>>> Cc: Jeff Mahoney <jeffm@suse.com>
>>>> Fixes: b382a324b60f ("Btrfs: fix qgroup rescan resume on mount")
>>>> Signed-off-by: Aleksa Sarai <asarai@suse.de>
>>>
>>> Indeed, looking at the code it seems that b382a324b60f ("Btrfs: fix
>>> qgroup rescan resume on mount") missed adding the qgroup_rescan_resume
>>> in the remount path. One thing which I couldn't verify though is whether
>>> reading fs_info->qgroup_flags without any locking is safe from remount
>>> context.
>>>
>>> During remount I don't see any locks taken that prevent operations which
>>> can modify qgroup_flags.
>>
>> Further inspection reveals that the access rules to qgroup_flags are
>> somewhat broken so this patch doesn't really make things any worse than
>> they are.
>
> The usage follows a pattern for a bitfield, updated by set_bit/clear_bit
> etc. The updates to the state or inconsistency is not safe, so some
> updates could get lost under some circumstances.
>
> Patch added to devel queue, possibly will be submitted to 4.13 so stable
> can pick it.
Looks like it wasn't merged in the 4.13 window (so stable hasn't picked
it), will this be submitted for 4.14? Thanks.
--
Aleksa Sarai
Software Engineer (Containers)
SUSE Linux GmbH
https://www.cyphar.com/
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] btrfs: resume qgroup rescan on rw remount
2017-08-26 6:39 ` Aleksa Sarai
@ 2017-08-30 10:48 ` David Sterba
0 siblings, 0 replies; 6+ messages in thread
From: David Sterba @ 2017-08-30 10:48 UTC (permalink / raw)
To: Aleksa Sarai
Cc: Nikolay Borisov, Chris Mason, Josef Bacik, David Sterba,
linux-btrfs, linux-kernel, stable
On Sat, Aug 26, 2017 at 04:39:02PM +1000, Aleksa Sarai wrote:
> On 07/12/2017 03:03 AM, David Sterba wrote:
> > On Mon, Jul 10, 2017 at 04:56:36PM +0300, Nikolay Borisov wrote:
> >> On 10.07.2017 16:12, Nikolay Borisov wrote:
> >>> On 4.07.2017 14:49, Aleksa Sarai wrote:
> >>>> Several distributions mount the "proper root" as ro during initrd and
> >>>> then remount it as rw before pivot_root(2). Thus, if a rescan had been
> >>>> aborted by a previous shutdown, the rescan would never be resumed.
> >>>>
> >>>> This issue would manifest itself as several btrfs ioctl(2)s causing the
> >>>> entire machine to hang when btrfs_qgroup_wait_for_completion was hit
> >>>> (due to the fs_info->qgroup_rescan_running flag being set but the rescan
> >>>> itself not being resumed). Notably, Docker's btrfs storage driver makes
> >>>> regular use of BTRFS_QUOTA_CTL_DISABLE and BTRFS_IOC_QUOTA_RESCAN_WAIT
> >>>> (causing this problem to be manifested on boot for some machines).
> >>>>
> >>>> Cc: <stable@vger.kernel.org> # v3.11+
> >>>> Cc: Jeff Mahoney <jeffm@suse.com>
> >>>> Fixes: b382a324b60f ("Btrfs: fix qgroup rescan resume on mount")
> >>>> Signed-off-by: Aleksa Sarai <asarai@suse.de>
> >>>
> >>> Indeed, looking at the code it seems that b382a324b60f ("Btrfs: fix
> >>> qgroup rescan resume on mount") missed adding the qgroup_rescan_resume
> >>> in the remount path. One thing which I couldn't verify though is whether
> >>> reading fs_info->qgroup_flags without any locking is safe from remount
> >>> context.
> >>>
> >>> During remount I don't see any locks taken that prevent operations which
> >>> can modify qgroup_flags.
> >>
> >> Further inspection reveals that the access rules to qgroup_flags are
> >> somewhat broken so this patch doesn't really make things any worse than
> >> they are.
> >
> > The usage follows a pattern for a bitfield, updated by set_bit/clear_bit
> > etc. The updates to the state or inconsistency is not safe, so some
> > updates could get lost under some circumstances.
> >
> > Patch added to devel queue, possibly will be submitted to 4.13 so stable
> > can pick it.
>
> Looks like it wasn't merged in the 4.13 window (so stable hasn't picked
> it), will this be submitted for 4.14? Thanks.
Sorry this didn't make it to 4.13, the patch is in the queue for 4.14.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-08-30 10:50 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-07-04 11:49 [PATCH] btrfs: resume qgroup rescan on rw remount Aleksa Sarai
2017-07-10 13:12 ` Nikolay Borisov
2017-07-10 13:56 ` Nikolay Borisov
2017-07-11 17:03 ` David Sterba
2017-08-26 6:39 ` Aleksa Sarai
2017-08-30 10:48 ` David Sterba
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).