Re: [systemd-devel] How to properly wait for udev?

2023-11-30 Thread Richard Weinberger
On Wed, Nov 29, 2023 at 9:48 AM Lennart Poettering
 wrote:
> > Why doesn't udev flock() every device it is probing?
> > Or asked differently, why is this feature opt-in instead of opt-out?
>
> Some software really doesn't like it if we take BSD locks on their
> devices, hence we don't take it blanket everywhere. And what's more
> important even: for various devices it simply isn't safe to just
> willy-nilly even open them (tape drivers and things, which might start
> to pull in a tape if we do). For others we might not be able to even
> open thing at all with the wrong flags (for example, because they are
> output only).
>
> Bock devices have relatively well defined semantics, there it's
> generally safe to do this, hence we do.

I see.

> Hence, it might be safe for UBI, but for the general case it might
> not be.
>
> That said, would BSD locking even address your issue? If you devices
> are exclusive access things and we first open() them and then flock()
> them, then that's not atomic. So if your test cases open the devices,
> then flock() them you might still get into conflict udev because it
> just open()ed the device, but didn#t get to call flock() yet.
>
> Doesn't UBI have something like O_EXCL-behaviour that grants true
> exclusive access?

It has in-kernel support for that but not from userspace, I'm
currently evaluating
whether it's worth exposing exclusive access to userspace.

Stay tuned. :-)

-- 
Thanks,
//richard


Re: [systemd-devel] How to properly wait for udev?

2023-11-29 Thread Lennart Poettering
On Mo, 27.11.23 21:32, Richard Weinberger (richard.weinber...@gmail.com) wrote:

> On Mon, Nov 27, 2023 at 9:29 AM Lennart Poettering
>  wrote:
> > If they conceptually should be considered block device equivalents, we
> > might want to extend the udev logic to such UBI devices too.  Patches
> > welcome.
>
> Why doesn't udev flock() every device it is probing?
> Or asked differently, why is this feature opt-in instead of opt-out?

Some software really doesn't like it if we take BSD locks on their
devices, hence we don't take it blanket everywhere. And what's more
important even: for various devices it simply isn't safe to just
willy-nilly even open them (tape drivers and things, which might start
to pull in a tape if we do). For others we might not be able to even
open thing at all with the wrong flags (for example, because they are
output only).

Bock devices have relatively well defined semantics, there it's
generally safe to do this, hence we do.

Hence, it might be safe for UBI, but for the general case it might
not be.

That said, would BSD locking even address your issue? If you devices
are exclusive access things and we first open() them and then flock()
them, then that's not atomic. So if your test cases open the devices,
then flock() them you might still get into conflict udev because it
just open()ed the device, but didn#t get to call flock() yet.

Doesn't UBI have something like O_EXCL-behaviour that grants true
exclusive access?

Lennart

--
Lennart Poettering, Berlin


Re: [systemd-devel] How to properly wait for udev?

2023-11-27 Thread Richard Weinberger
On Mon, Nov 27, 2023 at 9:29 AM Lennart Poettering
 wrote:
> If they conceptually should be considered block device equivalents, we
> might want to extend the udev logic to such UBI devices too.  Patches
> welcome.

Why doesn't udev flock() every device it is probing?
Or asked differently, why is this feature opt-in instead of opt-out?

-- 
Thanks,
//richard


Re: [systemd-devel] How to properly wait for udev?

2023-11-27 Thread Richard Weinberger
On Mon, Nov 27, 2023 at 9:29 AM Lennart Poettering
 wrote:
> On So, 26.11.23 00:39, Richard Weinberger (richard.weinber...@gmail.com) 
> wrote:
>
> > Hello!
> >
> > After upgrading my main test worker to a recent distribution, the UBI
> > test suite [0] fails at various places with -EBUSY.
> > The reason is that these tests create and remove UBI volumes rapidly.
> > A typical test sequence is as follows:
> > 1. creation of /dev/ubi0_0
> > 2. some exclusive operation, such as atomic update or volume resize on
> > /dev/ubi0_0
> > 3. removal of /dev/ubi0_0
> >
> > Both steps 2 and 3 can fail with -EBUSY because the udev worker still
> > holds a file descriptor to /dev/ubi0_0.
>
> Hmm, I have no experience with UBI, but are you sure we open that? why
> would we? are such devices analyzed by blkid? We generally don't open
> device nodes unless we have a reason to, such as doing blkid on it or
> so.

I think it came via commit:
dbbf424c8b77 ("rules: ubi mtd - add link to named partitions (#6750)")

Here is the bpftrace output of a failed mkvol_basic run.
The test created a new volume and tried to delete it via ioctl().
Right after creating the volume, udev started inspecting it and mkvol_basic
was unable to delete it because the delete operation needs exclusive ownership.

mkvol_basic(530):   open() = /dev/ubi0
mkvol_basic(530):   ioctl(cmd: 1074032385)
(udev-worker)(531): open UBI volume 0 = 0x96644533ac80
mkvol_basic(530):   open UBI volume 0 = 0xfff0
mkvol_basic(530):   failed ioctl() = -16
(udev-worker)(531): closing UBI volume 0x96644533ac80

> What precisely fails for you? the open()? or some operation on the
> opened fd?

All of that. I depends on the test.
Basically every test assumes that it has the full ownership of a
volume it has created.

> >
> > FWIW, the problem can also get triggered using UBI's shell utilities
> > if the system is fast enough, e.g.
> > # ubimkvol -N testv -S 50 -n 0 /dev/ubi0 && ubirmvol -n 0 /dev/ubi0
> > Volume ID 0, size 50 LEBs (793600 bytes, 775.0 KiB), LEB size 15872
> > bytes (15.5 KiB), dynamic, name "testv", alignment 1
> > ubirmvol: error!: cannot UBI remove volume
> >  error 16 (Device or resource busy)
> >
> > Instead of adding a retry loop around -EBUSY, I believe the best
> > solution is to add code to wait for udev.
> > For example, having a udev barrier in ubi_mkvol() and ubi_rmvol() [1]
> > seems like a good idea to me.
>
> For block devices we implement this:
>
> https://systemd.io/BLOCK_DEVICE_LOCKING
>
> I understand UBI aren't block devices though?

Exactly, UBI volumes are character devices, just like MTDs.

> If they conceptually should be considered block device equivalents, we
> might want to extend the udev logic to such UBI devices too.  Patches
> welcome.
>
> We provide "udevadm lock" to lock a block device according to this
> scheme from shell scripts.
>
> > What function from libsystemd do you suggest for waiting until udev is
> > done with rule processing?
> > My naive approach, using udev_queue_is_empty() and
> > sd_device_get_is_initialized(), does not resolve all failures so far.
> > Firstly, udev_queue_is_empty() doesn't seem to be exported by
> > libsystemd. I have open-coded it as:
> > static int udev_queue_is_empty(void) {
> >return access("/run/udev/queue", F_OK) < 0 ?
> >(errno == ENOENT ? true : -errno) : false;
> > }
>
> This doesn't really work. udev might still process the device in the
> background.

I see.

-- 
Thanks,
//richard


Re: [systemd-devel] How to properly wait for udev?

2023-11-27 Thread Mantas Mikulėnas
On Mon, Nov 27, 2023 at 10:30 AM Lennart Poettering 
wrote:

> On So, 26.11.23 00:39, Richard Weinberger (richard.weinber...@gmail.com)
> wrote:
>
> > Hello!
> >
> > After upgrading my main test worker to a recent distribution, the UBI
> > test suite [0] fails at various places with -EBUSY.
> > The reason is that these tests create and remove UBI volumes rapidly.
> > A typical test sequence is as follows:
> > 1. creation of /dev/ubi0_0
> > 2. some exclusive operation, such as atomic update or volume resize on
> > /dev/ubi0_0
> > 3. removal of /dev/ubi0_0
> >
> > Both steps 2 and 3 can fail with -EBUSY because the udev worker still
> > holds a file descriptor to /dev/ubi0_0.
>
> Hmm, I have no experience with UBI, but are you sure we open that? why
> would we? are such devices analyzed by blkid? We generally don't open
> device nodes unless we have a reason to, such as doing blkid on it or
> so.
>

blkid and 60-persistent-storage indeed analyze ubi devices, it seems.

-- 
Mantas Mikulėnas


Re: [systemd-devel] How to properly wait for udev?

2023-11-27 Thread Lennart Poettering
On So, 26.11.23 00:39, Richard Weinberger (richard.weinber...@gmail.com) wrote:

> Hello!
>
> After upgrading my main test worker to a recent distribution, the UBI
> test suite [0] fails at various places with -EBUSY.
> The reason is that these tests create and remove UBI volumes rapidly.
> A typical test sequence is as follows:
> 1. creation of /dev/ubi0_0
> 2. some exclusive operation, such as atomic update or volume resize on
> /dev/ubi0_0
> 3. removal of /dev/ubi0_0
>
> Both steps 2 and 3 can fail with -EBUSY because the udev worker still
> holds a file descriptor to /dev/ubi0_0.

Hmm, I have no experience with UBI, but are you sure we open that? why
would we? are such devices analyzed by blkid? We generally don't open
device nodes unless we have a reason to, such as doing blkid on it or
so.

What precisely fails for you? the open()? or some operation on the
opened fd?

>
> FWIW, the problem can also get triggered using UBI's shell utilities
> if the system is fast enough, e.g.
> # ubimkvol -N testv -S 50 -n 0 /dev/ubi0 && ubirmvol -n 0 /dev/ubi0
> Volume ID 0, size 50 LEBs (793600 bytes, 775.0 KiB), LEB size 15872
> bytes (15.5 KiB), dynamic, name "testv", alignment 1
> ubirmvol: error!: cannot UBI remove volume
>  error 16 (Device or resource busy)
>
> Instead of adding a retry loop around -EBUSY, I believe the best
> solution is to add code to wait for udev.
> For example, having a udev barrier in ubi_mkvol() and ubi_rmvol() [1]
> seems like a good idea to me.

For block devices we implement this:

https://systemd.io/BLOCK_DEVICE_LOCKING

I understand UBI aren't block devices though?

If they conceptually should be considered block device equivalents, we
might want to extend the udev logic to such UBI devices too.  Patches
welcome.

We provide "udevadm lock" to lock a block device according to this
scheme from shell scripts.

> What function from libsystemd do you suggest for waiting until udev is
> done with rule processing?
> My naive approach, using udev_queue_is_empty() and
> sd_device_get_is_initialized(), does not resolve all failures so far.
> Firstly, udev_queue_is_empty() doesn't seem to be exported by
> libsystemd. I have open-coded it as:
> static int udev_queue_is_empty(void) {
>return access("/run/udev/queue", F_OK) < 0 ?
>(errno == ENOENT ? true : -errno) : false;
> }

This doesn't really work. udev might still process the device in the
background.

Lennart

--
Lennart Poettering, Berlin


Re: [systemd-devel] How to properly wait for udev?

2023-11-26 Thread Richard Weinberger
On Sun, Nov 26, 2023 at 10:36 PM Mantas Mikulėnas  wrote:
>
> If I remember correctly, udev (recent versions) takes a BSD lock using 
> flock(2) while processing the device, and tools are supposed to do the same. 
> The flock() call can be set to wait until the lock can be taken.

Hmm, indeed. But it seems to do so only for block devices.
This explain also why none of my syscall tracing showed flock() calls so far.
UBI volumes are character devices.

--
Thanks,
//richard


Re: [systemd-devel] How to properly wait for udev?

2023-11-26 Thread Mantas Mikulėnas
If I remember correctly, udev (recent versions) takes a BSD lock using
flock(2) while processing the device, and tools are supposed to do the
same. The flock() call can be set to wait until the lock can be taken.

On Sun, Nov 26, 2023 at 1:40 AM Richard Weinberger <
richard.weinber...@gmail.com> wrote:

> Hello!
>
> After upgrading my main test worker to a recent distribution, the UBI
> test suite [0] fails at various places with -EBUSY.
> The reason is that these tests create and remove UBI volumes rapidly.
> A typical test sequence is as follows:
> 1. creation of /dev/ubi0_0
> 2. some exclusive operation, such as atomic update or volume resize on
> /dev/ubi0_0
> 3. removal of /dev/ubi0_0
>
> Both steps 2 and 3 can fail with -EBUSY because the udev worker still
> holds a file descriptor to /dev/ubi0_0.
>
> FWIW, the problem can also get triggered using UBI's shell utilities
> if the system is fast enough, e.g.
> # ubimkvol -N testv -S 50 -n 0 /dev/ubi0 && ubirmvol -n 0 /dev/ubi0
> Volume ID 0, size 50 LEBs (793600 bytes, 775.0 KiB), LEB size 15872
> bytes (15.5 KiB), dynamic, name "testv", alignment 1
> ubirmvol: error!: cannot UBI remove volume
>  error 16 (Device or resource busy)
>
> Instead of adding a retry loop around -EBUSY, I believe the best
> solution is to add code to wait for udev.
> For example, having a udev barrier in ubi_mkvol() and ubi_rmvol() [1]
> seems like a good idea to me.
>
> What function from libsystemd do you suggest for waiting until udev is
> done with rule processing?
> My naive approach, using udev_queue_is_empty() and
> sd_device_get_is_initialized(), does not resolve all failures so far.
> Firstly, udev_queue_is_empty() doesn't seem to be exported by
> libsystemd. I have open-coded it as:
> static int udev_queue_is_empty(void) {
>return access("/run/udev/queue", F_OK) < 0 ?
>(errno == ENOENT ? true : -errno) : false;
> }
>
> Additionally, sd_device_get_is_initialized() seems to return sometimes
> true even if the udev worker still has the volume open.
> In short, which API do you recommend to ensure that the device my
> thread has created is actually usable?
>
> [0]: http://git.infradead.org/mtd-utils.git/tree/HEAD:/tests/ubi-tests
> [1]: http://git.infradead.org/mtd-utils.git/blob/HEAD:/lib/libubi.c#l994
>
> --
> Thanks,
> //richard
>


-- 
Mantas Mikulėnas


[systemd-devel] How to properly wait for udev?

2023-11-25 Thread Richard Weinberger
Hello!

After upgrading my main test worker to a recent distribution, the UBI
test suite [0] fails at various places with -EBUSY.
The reason is that these tests create and remove UBI volumes rapidly.
A typical test sequence is as follows:
1. creation of /dev/ubi0_0
2. some exclusive operation, such as atomic update or volume resize on
/dev/ubi0_0
3. removal of /dev/ubi0_0

Both steps 2 and 3 can fail with -EBUSY because the udev worker still
holds a file descriptor to /dev/ubi0_0.

FWIW, the problem can also get triggered using UBI's shell utilities
if the system is fast enough, e.g.
# ubimkvol -N testv -S 50 -n 0 /dev/ubi0 && ubirmvol -n 0 /dev/ubi0
Volume ID 0, size 50 LEBs (793600 bytes, 775.0 KiB), LEB size 15872
bytes (15.5 KiB), dynamic, name "testv", alignment 1
ubirmvol: error!: cannot UBI remove volume
 error 16 (Device or resource busy)

Instead of adding a retry loop around -EBUSY, I believe the best
solution is to add code to wait for udev.
For example, having a udev barrier in ubi_mkvol() and ubi_rmvol() [1]
seems like a good idea to me.

What function from libsystemd do you suggest for waiting until udev is
done with rule processing?
My naive approach, using udev_queue_is_empty() and
sd_device_get_is_initialized(), does not resolve all failures so far.
Firstly, udev_queue_is_empty() doesn't seem to be exported by
libsystemd. I have open-coded it as:
static int udev_queue_is_empty(void) {
   return access("/run/udev/queue", F_OK) < 0 ?
   (errno == ENOENT ? true : -errno) : false;
}

Additionally, sd_device_get_is_initialized() seems to return sometimes
true even if the udev worker still has the volume open.
In short, which API do you recommend to ensure that the device my
thread has created is actually usable?

[0]: http://git.infradead.org/mtd-utils.git/tree/HEAD:/tests/ubi-tests
[1]: http://git.infradead.org/mtd-utils.git/blob/HEAD:/lib/libubi.c#l994

-- 
Thanks,
//richard