diff mbox series

[1/6] common: udev settle before _scratch_pool_mkfs

Message ID b8e39744f98bde6484d222c6e90240f074df1eaf.1715896529.git.dsterba@suse.com (mailing list archive)
State New, archived
Headers show
Series Btrfs test updates and fixups | expand

Commit Message

David Sterba May 16, 2024, 10:12 p.m. UTC
From: Josef Bacik <josef@toxicpanda.com>

There are some btrfs tests that do _scratch_pool_mkfs in a loop.
Sometimes this fails with EBUSY.  Tracing revealed that udevd will
sometimes write to /sys/block/device/uevent to make sure an event
triggers to rules get written.  However these events will not get sent
to user space until after an O_EXCL open as been closed.  The general
flow is something like

mkfs.btrfs /dev/sda /dev/sdb /dev/sdc /dev/sdd
mount /dev/sda /mnt/test
<things>
umount /mnt/test

in a loop.  The problem is udevd will add uevents for the devices and
they won't get delivered until after the umount.  If we're doing the
above sequence in a loop the next mkfs.btrfs will fail because udev is
touching the devices to consume the KOBJ_CHANGE event.

Fix this by doing a udev settle before _scratch_pool_mkfs.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: David Sterba <dsterba@suse.com>
---
 common/rc | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Anand Jain May 17, 2024, 9:57 p.m. UTC | #1
On 5/17/24 06:12, David Sterba wrote:
> From: Josef Bacik <josef@toxicpanda.com>
> 
> There are some btrfs tests that do _scratch_pool_mkfs in a loop.
> Sometimes this fails with EBUSY.  Tracing revealed that udevd will
> sometimes write to /sys/block/device/uevent to make sure an event
> triggers to rules get written.  However these events will not get sent
> to user space until after an O_EXCL open as been closed.  The general
> flow is something like
> 
> mkfs.btrfs /dev/sda /dev/sdb /dev/sdc /dev/sdd
> mount /dev/sda /mnt/test
> <things>
> umount /mnt/test
> 
> in a loop.  The problem is udevd will add uevents for the devices and
> they won't get delivered until after the umount.  If we're doing the
> above sequence in a loop the next mkfs.btrfs will fail because udev is
> touching the devices to consume the KOBJ_CHANGE event.
> 
> Fix this by doing a udev settle before _scratch_pool_mkfs.
> 



> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> Signed-off-by: David Sterba <dsterba@suse.com>
> ---
>   common/rc | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/common/rc b/common/rc
> index 0fe56382a6a497..5d38571ffe87eb 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -903,6 +903,12 @@ _scratch_pool_mkfs()
>   {
>       case $FSTYP in
>       btrfs)
> +	# For multi-disk file systems udev can queue up events on the device
> +	# when we mkfs the device, and thus tie up the device after we've
> +	# unmounted.  Tests that _scratch_pool_mkfs() in a loop can sometimes
> +	# trip over udev trying to do the updates after the umount, so make sure
> +	# we settle before we try mkfs'ing so we don't get an EBUSY
> +	$UDEV_SETTLE_PROG >/dev/null 2>&1
>           $MKFS_BTRFS_PROG $MKFS_OPTIONS $* $SCRATCH_DEV_POOL > /dev/null
>           ;;


Just curious: have we seen this issue even after the btrfs-progs commit 
below?

   e54514aaeab6 btrfs-progs: fix stray fd close in open_ctree_fs_info()

Thanks, Anand

>       *)
David Sterba May 20, 2024, 7:21 p.m. UTC | #2
On Sat, May 18, 2024 at 05:57:12AM +0800, Anand Jain wrote:
> On 5/17/24 06:12, David Sterba wrote:
> > From: Josef Bacik <josef@toxicpanda.com>
> > 
> > There are some btrfs tests that do _scratch_pool_mkfs in a loop.
> > Sometimes this fails with EBUSY.  Tracing revealed that udevd will
> > sometimes write to /sys/block/device/uevent to make sure an event
> > triggers to rules get written.  However these events will not get sent
> > to user space until after an O_EXCL open as been closed.  The general
> > flow is something like
> > 
> > mkfs.btrfs /dev/sda /dev/sdb /dev/sdc /dev/sdd
> > mount /dev/sda /mnt/test
> > <things>
> > umount /mnt/test
> > 
> > in a loop.  The problem is udevd will add uevents for the devices and
> > they won't get delivered until after the umount.  If we're doing the
> > above sequence in a loop the next mkfs.btrfs will fail because udev is
> > touching the devices to consume the KOBJ_CHANGE event.
> > 
> > Fix this by doing a udev settle before _scratch_pool_mkfs.
> > 
> 
> 
> 
> > Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> > Signed-off-by: David Sterba <dsterba@suse.com>
> > ---
> >   common/rc | 6 ++++++
> >   1 file changed, 6 insertions(+)
> > 
> > diff --git a/common/rc b/common/rc
> > index 0fe56382a6a497..5d38571ffe87eb 100644
> > --- a/common/rc
> > +++ b/common/rc
> > @@ -903,6 +903,12 @@ _scratch_pool_mkfs()
> >   {
> >       case $FSTYP in
> >       btrfs)
> > +	# For multi-disk file systems udev can queue up events on the device
> > +	# when we mkfs the device, and thus tie up the device after we've
> > +	# unmounted.  Tests that _scratch_pool_mkfs() in a loop can sometimes
> > +	# trip over udev trying to do the updates after the umount, so make sure
> > +	# we settle before we try mkfs'ing so we don't get an EBUSY
> > +	$UDEV_SETTLE_PROG >/dev/null 2>&1
> >           $MKFS_BTRFS_PROG $MKFS_OPTIONS $* $SCRATCH_DEV_POOL > /dev/null
> >           ;;
> 
> 
> Just curious: have we seen this issue even after the btrfs-progs commit 
> below?
> 
>    e54514aaeab6 btrfs-progs: fix stray fd close in open_ctree_fs_info()

That's a good question, I don't know but I can revert the change and see
if things break. This may take time as triggering the udev/mkfs race is
not reliable.
Anand Jain May 20, 2024, 11:48 p.m. UTC | #3
On 5/21/24 03:21, David Sterba wrote:
> On Sat, May 18, 2024 at 05:57:12AM +0800, Anand Jain wrote:
>> On 5/17/24 06:12, David Sterba wrote:
>>> From: Josef Bacik <josef@toxicpanda.com>
>>>
>>> There are some btrfs tests that do _scratch_pool_mkfs in a loop.
>>> Sometimes this fails with EBUSY.  Tracing revealed that udevd will
>>> sometimes write to /sys/block/device/uevent to make sure an event
>>> triggers to rules get written.  However these events will not get sent
>>> to user space until after an O_EXCL open as been closed.  The general
>>> flow is something like
>>>
>>> mkfs.btrfs /dev/sda /dev/sdb /dev/sdc /dev/sdd
>>> mount /dev/sda /mnt/test
>>> <things>
>>> umount /mnt/test
>>>
>>> in a loop.  The problem is udevd will add uevents for the devices and
>>> they won't get delivered until after the umount.  If we're doing the
>>> above sequence in a loop the next mkfs.btrfs will fail because udev is
>>> touching the devices to consume the KOBJ_CHANGE event.
>>>
>>> Fix this by doing a udev settle before _scratch_pool_mkfs.
>>>
>>
>>
>>
>>> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
>>> Signed-off-by: David Sterba <dsterba@suse.com>
>>> ---
>>>    common/rc | 6 ++++++
>>>    1 file changed, 6 insertions(+)
>>>
>>> diff --git a/common/rc b/common/rc
>>> index 0fe56382a6a497..5d38571ffe87eb 100644
>>> --- a/common/rc
>>> +++ b/common/rc
>>> @@ -903,6 +903,12 @@ _scratch_pool_mkfs()
>>>    {
>>>        case $FSTYP in
>>>        btrfs)
>>> +	# For multi-disk file systems udev can queue up events on the device
>>> +	# when we mkfs the device, and thus tie up the device after we've
>>> +	# unmounted.  Tests that _scratch_pool_mkfs() in a loop can sometimes
>>> +	# trip over udev trying to do the updates after the umount, so make sure
>>> +	# we settle before we try mkfs'ing so we don't get an EBUSY
>>> +	$UDEV_SETTLE_PROG >/dev/null 2>&1
>>>            $MKFS_BTRFS_PROG $MKFS_OPTIONS $* $SCRATCH_DEV_POOL > /dev/null
>>>            ;;
>>
>>
>> Just curious: have we seen this issue even after the btrfs-progs commit
>> below?
>>
>>     e54514aaeab6 btrfs-progs: fix stray fd close in open_ctree_fs_info()
> 
> That's a good question, I don't know but I can revert the change and see
> if things break. This may take time as triggering the udev/mkfs race is
> not reliable.

Agreed. The udev/mkfs race is tricky and will take a while to report,
but it's good to experiment. IMO.

Thanks! Anand
diff mbox series

Patch

diff --git a/common/rc b/common/rc
index 0fe56382a6a497..5d38571ffe87eb 100644
--- a/common/rc
+++ b/common/rc
@@ -903,6 +903,12 @@  _scratch_pool_mkfs()
 {
     case $FSTYP in
     btrfs)
+	# For multi-disk file systems udev can queue up events on the device
+	# when we mkfs the device, and thus tie up the device after we've
+	# unmounted.  Tests that _scratch_pool_mkfs() in a loop can sometimes
+	# trip over udev trying to do the updates after the umount, so make sure
+	# we settle before we try mkfs'ing so we don't get an EBUSY
+	$UDEV_SETTLE_PROG >/dev/null 2>&1
         $MKFS_BTRFS_PROG $MKFS_OPTIONS $* $SCRATCH_DEV_POOL > /dev/null
         ;;
     *)