> + uint cmds, type;
> + struct super_block *sb = NULL;
I don't think sb needs the NULL initialization.
> + struct path path, *pathp = NULL;
> + struct path mountpath;
> + bool excl = false, thawed = false;
> + int ret;
> +
> + cmds = cmd >> SUBCMDSHIFT;
> + type = cmd & SUBCMDMASK;
Personal pet peeve: it would be nice to just initialize cmds and
type on their declaration line, or while we're at it declutter
this a bit and remove the separate cmds variable:
unsigned int type = cmd & SUBCMDMASK;
cmd >>= SUBCMDSHIFT;
> + /*
> + * Path for quotaon has to be resolved before grabbing superblock
> + * because that gets s_umount sem which is also possibly needed by path
> + * resolution (think about autofs) and thus deadlocks could arise.
> + */
> + if (cmds == Q_QUOTAON) {
> + ret = user_path_at(AT_FDCWD, addr,
> + LOOKUP_FOLLOW | LOOKUP_AUTOMOUNT, &path);
> + if (ret)
> + pathp = ERR_PTR(ret);
> + else
> + pathp = &path;
> + }
> +
> + ret = user_path_at(AT_FDCWD, mountpoint,
> + LOOKUP_FOLLOW | LOOKUP_AUTOMOUNT, &mountpath);
> + if (ret)
> + goto out;
I don't think we need two path lookups here, we can path the same path
to the command for quotaon.
> + if (quotactl_cmd_onoff(cmds)) {
> + excl = true;
> + thawed = true;
> + } else if (quotactl_cmd_write(cmds)) {
> + thawed = true;
> + }
> +
> + if (thawed) {
> + ret = mnt_want_write(mountpath.mnt);
> + if (ret)
> + goto out1;
> + }
> +
> + sb = mountpath.dentry->d_inode->i_sb;
> +
> + if (excl)
> + down_write(&sb->s_umount);
> + else
> + down_read(&sb->s_umount);
Given how cheap quotactl_cmd_onoff and quotactl_cmd_write are we
could probably simplify this down do:
if (quotactl_cmd_write(cmd)) {
ret = mnt_want_write(path.mnt);
if (ret)
goto out1;
}
if (quotactl_cmd_onoff(cmd))
down_write(&sb->s_umount);
else
down_read(&sb->s_umount);
and duplicate the checks after the do_quotactl call.
