Differential D30218 Diff 90116 sys/fs/nullfs/null_vfsops.c

Changeset View

Standalone View

sys/fs/nullfs/null_vfsops.c

Show First 20 Lines • Show All 288 Lines • ▼ Show 20 Lines	if (error == 0) {
if (error == 0) {		if (error == 0) {
*vpp = vp;		*vpp = vp;
}		}
}		}
return (error);		return (error);
}		}

static int		static int
nullfs_quotactl(mp, cmd, uid, arg)		nullfs_quotactl(mp, cmd, uid, arg, mp_busy)
struct mount *mp;		struct mount *mp;
int cmd;		int cmd;
uid_t uid;		uid_t uid;
void *arg;		void *arg;
		bool *mp_busy;
{		{
return VFS_QUOTACTL(MOUNTTONULLMOUNT(mp)->nullm_vfs, cmd, uid, arg);		struct mount *lowermp;
		struct null_mount *mntdata;
		markjUnsubmitted Not Done Inline Actions Doesn't `mp_busy` describe the state of the nullfs mount rather than the target mount? Ditto for unionfs. markj: Doesn't `mp_busy` describe the state of the nullfs mount rather than the target mount? Ditto…
		jahAuthorUnsubmitted Done Inline Actions Yes, that's right. For unionfs this should already be handled by D30152. It looks like nullfs will need something similar to the ref+busy helpers in D30152, as I don't see that it does anything to ensure the underlying mount sticks around. jah: Yes, that's right. For unionfs this should already be handled by D30152. It looks like nullfs…
		kibUnsubmitted Not Done Inline Actions May be, the right thing to do for stacked filesystems is to disable quota handling from upper at all. For instance, you cannot change async option from upper, why any other administrative setup of lower should be affected? kib: May be, the right thing to do for stacked filesystems is to disable quota handling from upper…
		jahAuthorUnsubmitted Done Inline Actions Maybe? From my naive perspective as a VFS newbie, this seems like a difference between something that's an attribute of the mount vs. an attribute of the filesystem. Since the async flag pertains to a specific mount, it makes sense to allow it to be changed only by directly operating on that mount. To me quotas seem more like an attribute of the filesystem. Since stacked filesystems are intended to subsume the attributes of their lower filesystems in order to provide additional functionality, it at least doesn't seem wrong to allow them to pass quota operations to the lower filesystems. In a hypothetical world where quotactl was more generic and less UFS-specific, I could imagine a stacked filesystem wanting to do its own quota management before passing the request to the lower FS. jah: Maybe? From my naive perspective as a VFS newbie, this seems like a difference between…
		kibUnsubmitted Not Done Inline Actions I have somewhat different perspective there. I would think that administrative configuration of the lower filesystem should be only done on lower mp. Upper mp should provide a view into lower, but otherwise it generally should be not allowed to configure it. This is why I provided the async option as an example of administratively controlled consistency configuration, and I think that quota controls are same. If anything, quota control on mount should control quotas on upper, which would be then strict union of lower and upper limits. But so far nobody asked for this (quotas are rarely used anyway). kib: I have somewhat different perspective there. I would think that administrative configuration…
		jahAuthorUnsubmitted Done Inline Actions I think we can always do that in a later change, right? It seems that nullfs needs some fixes to the way it handles the lower mount regardless of quotactl, so I plan to do that as a separate change to be committed at the same time as this and D30152. jah: I think we can always do that in a later change, right? It seems that nullfs needs some fixes…
		int error;
		bool unbusy;

		mntdata = MOUNTTONULLMOUNT(mp);
		lowermp = atomic_load_ptr(&mntdata->nullm_vfs);
		kibUnsubmitted Not Done Inline Actions All these empty lines are not needed/not allowed by style. kib: All these empty lines are not needed/not allowed by style.
		KASSERT(*mp_busy == true, ("upper mount not busy"));
		/*
		* See comment in sys_quotactl() for an explanation of why the
		* lower mount needs to be busied by the caller of VFS_QUOTACTL()
		* but may be unbusied by the implementation. We must unbusy
		kibUnsubmitted Not Done Inline Actions It is not safe to call into lower QUOTACTL while still having the upper mount busy due to namei()/open(). If you ref/unbusy upper, then you can stop using MBF_NOWAIT for busying lower. kib: It is not safe to call into lower QUOTACTL while still having the upper mount busy due to namei…
		markjUnsubmitted Not Done Inline Actions I would suggest adding a comment explaining why it's not ok to hold both mount points busy. markj: I would suggest adding a comment explaining why it's not ok to hold both mount points busy.
		* the upper mount for the same reason; otherwise a namei lookup
		* issued by the VFS_QUOTACTL() implementation could traverse the
		* upper mount and deadlock.
		*/
		vfs_unbusy(mp);
		*mp_busy = false;
		unbusy = true;
		error = vfs_busy(lowermp, 0);
		if (error == 0)
		error = VFS_QUOTACTL(lowermp, cmd, uid, arg, &unbusy);
		if (unbusy)
		vfs_unbusy(lowermp);

		return (error);
}		}

static int		static int
nullfs_statfs(mp, sbp)		nullfs_statfs(mp, sbp)
struct mount *mp;		struct mount *mp;
struct statfs *sbp;		struct statfs *sbp;
{		{
int error;		int error;
▲ Show 20 Lines • Show All 159 Lines • Show Last 20 Lines