Page MenuHomeFreeBSD

VFS_QUOTACTL(9): allow implementation to indicate busy state changes
ClosedPublic

Authored by jah on May 30 2021, 6:26 PM.

Details

Summary

Instead of requiring all implementations of vfs_quotactl to unbusy
the mount for Q_QUOTAON and Q_QUOTAOFF, add an "mp_busy" in/out param
to VFS_QUOTACTL(9). The implementation may then indicate to the caller
whether it needed to unbusy the mount.

Workaround missing bool type for libprocstat

Diff Detail

Repository
rG FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

jah requested review of this revision.May 30 2021, 6:26 PM

This is the same change you've already reviewed, with 2 exceptions:
--explicit inclusion of sys/types.h for _KERNEL builds from sys/mount.h, to avoid relying on coincidentally getting the header from elsewhere.
--inclusion of stdbool.h for libprocstat modules whose use of kernel headers otherwise defeats the definition of type bool.

For the second issue, I would be willing to drop the libprocstat changes and instead use _Bool in VFS_QUOTACTL() if anyone insists on it. However, my personal preference is strongly for 'bool' over '_Bool' as a matter of style, and we seem to be slowly standardizing on 'bool' in the kernel. My take is that we shouldn't need to warp KPIs in order to accommodate userspace code that does sketchy things with kernel headers, especially when the code doing said sketchy things can employ a simple workaround.

Well _Bool is the official C99 name of the bool type. So I do not see much problem using it.

Or as alternative you can use int/int *.

I am fine with either the patch as it is in the review, or with one of the proposals above.

This revision is now accepted and ready to land.May 30 2021, 7:20 PM
In D30556#686194, @jah wrote:

This is the same change you've already reviewed, with 2 exceptions:
--explicit inclusion of sys/types.h for _KERNEL builds from sys/mount.h, to avoid relying on coincidentally getting the header from elsewhere.
--inclusion of stdbool.h for libprocstat modules whose use of kernel headers otherwise defeats the definition of type bool.

For the second issue, I would be willing to drop the libprocstat changes and instead use _Bool in VFS_QUOTACTL() if anyone insists on it. However, my personal preference is strongly for 'bool' over '_Bool' as a matter of style, and we seem to be slowly standardizing on 'bool' in the kernel. My take is that we shouldn't need to warp KPIs in order to accommodate userspace code that does sketchy things with kernel headers, especially when the code doing said sketchy things can employ a simple workaround.

I tend to agree that we should avoid mixing _Bool and bool in the kernel. It's just confusing. The change you proposed is ok with me, with the caveat that a similar modification may be needed in a small number of ports that do something like libprocstat (lsof is the main one that comes to mind). I'm ok with kib's suggestions as well.

In D30556#686194, @jah wrote:

This is the same change you've already reviewed, with 2 exceptions:
--explicit inclusion of sys/types.h for _KERNEL builds from sys/mount.h, to avoid relying on coincidentally getting the header from elsewhere.
--inclusion of stdbool.h for libprocstat modules whose use of kernel headers otherwise defeats the definition of type bool.

For the second issue, I would be willing to drop the libprocstat changes and instead use _Bool in VFS_QUOTACTL() if anyone insists on it. However, my personal preference is strongly for 'bool' over '_Bool' as a matter of style, and we seem to be slowly standardizing on 'bool' in the kernel. My take is that we shouldn't need to warp KPIs in order to accommodate userspace code that does sketchy things with kernel headers, especially when the code doing said sketchy things can employ a simple workaround.

I tend to agree that we should avoid mixing _Bool and bool in the kernel. It's just confusing. The change you proposed is ok with me, with the caveat that a similar modification may be needed in a small number of ports that do something like libprocstat (lsof is the main one that comes to mind). I'm ok with kib's suggestions as well.

Just grabbed the lsof distfile; it looks like the FreeBSD dialect already includes stdbool.h because of refcount(9), so hopefully that one is ok.

In D30556#686217, @jah wrote:
In D30556#686194, @jah wrote:

This is the same change you've already reviewed, with 2 exceptions:
--explicit inclusion of sys/types.h for _KERNEL builds from sys/mount.h, to avoid relying on coincidentally getting the header from elsewhere.
--inclusion of stdbool.h for libprocstat modules whose use of kernel headers otherwise defeats the definition of type bool.

For the second issue, I would be willing to drop the libprocstat changes and instead use _Bool in VFS_QUOTACTL() if anyone insists on it. However, my personal preference is strongly for 'bool' over '_Bool' as a matter of style, and we seem to be slowly standardizing on 'bool' in the kernel. My take is that we shouldn't need to warp KPIs in order to accommodate userspace code that does sketchy things with kernel headers, especially when the code doing said sketchy things can employ a simple workaround.

I tend to agree that we should avoid mixing _Bool and bool in the kernel. It's just confusing. The change you proposed is ok with me, with the caveat that a similar modification may be needed in a small number of ports that do something like libprocstat (lsof is the main one that comes to mind). I'm ok with kib's suggestions as well.

Just grabbed the lsof distfile; it looks like the FreeBSD dialect already includes stdbool.h because of refcount(9), so hopefully that one is ok.

I think this change is probably fine in that case. There may be some minor fallout in other ports, but it can easily be patched.