swapon: Don't honour "trimonce" for active swap devices

[markj]

Summary

Running "swapon -a" on a system where some of the /etc/fstab swap
entries are already enabled causes us to erase the contents of active
swap devices, with predictable effects.

Try to avoid this problem by calling swapon() before erasing the swap
device, and not erasing the device if an error occurs. This is not an
elegant solution, but I can't see how to reliably determine whether a
device is already a swap device. vm.swap_info gives the device number
for all in-use swap devices, but this is not sufficient to identify
vnode-backed swap devices.

Suggestions for other approaches are welcome, I will try to implement
them.

PR: 260793

[kib]

Shouldn't swapon request exclusive+write on swap geom instead of read+write? What I like about it is that it fixes the obvious race between check and trim.

For regular files, might be some scheme with advisory locking both from command and swapon(2) internals would work. It is not immediately obvious, because in-kernel owner must be swapper or similar long-living process.

[markj]

In D33846#765409, @kib wrote:

Shouldn't swapon request exclusive+write on swap geom instead of read+write? What I like about it is that it fixes the obvious race between check and trim.

Indeed, but: https://github.com/freebsd/freebsd-src/blob/main/sys/vm/swap_pager.c#L3002

For regular files, might be some scheme with advisory locking both from command and swapon(2) internals would work. It is not immediately obvious, because in-kernel owner must be swapper or similar long-living process.

Could be, it would be nice to use the same mechanism for devfs- and vnode-backed swap devices though...

[kib]

In D33846#765414, @markj wrote:

In D33846#765409, @kib wrote:

Shouldn't swapon request exclusive+write on swap geom instead of read+write? What I like about it is that it fixes the obvious race between check and trim.

Indeed, but: https://github.com/freebsd/freebsd-src/blob/main/sys/vm/swap_pager.c#L3002

May be it is too silly, but e.g. init_dumpdev() could stop doing g_access(). It is ignored for actual writes anyway, from what I understand.

For regular files, might be some scheme with advisory locking both from command and swapon(2) internals would work. It is not immediately obvious, because in-kernel owner must be swapper or similar long-living process.

Could be, it would be nice to use the same mechanism for devfs- and vnode-backed swap devices though...

[markj]

In D33846#765418, @kib wrote:

In D33846#765414, @markj wrote:

In D33846#765409, @kib wrote:

Shouldn't swapon request exclusive+write on swap geom instead of read+write? What I like about it is that it fixes the obvious race between check and trim.

Indeed, but: https://github.com/freebsd/freebsd-src/blob/main/sys/vm/swap_pager.c#L3002

May be it is too silly, but e.g. init_dumpdev() could stop doing g_access(). It is ignored for actual writes anyway, from what I understand.

This is not enough, for instance savecore -c will also write directly to the device.

Maybe it would make sense to extend swapon() like was done to swapoff(), so a flag can be specified and trimming is done entirely in the kernel. The intent at one point was to also have some ability to perform online trimming, which must be handled by the kernel anyway.