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

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

Details

Reviewers

kib
markj

Group Reviewers

manpages

Commits

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

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

rS FreeBSD src repository - subversion

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 39574
Build 36463: arc lint + arc unit

Event Timeline

jah created this revision.May 30 2021, 6:26 PM

Herald added a reviewer: manpages. · View Herald TranscriptMay 30 2021, 6:26 PM

Herald added subscribers: delphij, imp. · View Herald Transcript

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

Harbormaster completed remote builds in B39574: Diff 90135.May 30 2021, 6:27 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#686216, @markj 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.

In D30556#686217, @jah wrote:

In D30556#686216, @markj 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.