On Sun, Aug 16, 2020 at 10:12:27AM +0200, Christian Brauner wrote:

On Fri, Aug 14, 2020 at 05:30:58PM -0700, Andrew Morton wrote:

quoted

From: Minchan Kim <minchan@kernel.org>
Subject: mm/madvise: introduce process_madvise() syscall: an external memory hinting API

<snip>

quoted

+SYSCALL_DEFINE5(process_madvise, int, pidfd, const struct iovec __user *, vec,
+		unsigned long, vlen, int, behavior, unsigned int, flags)
+{
+	ssize_t ret;
+	struct iovec iovstack[UIO_FASTIOV];
+	struct iovec *iov = iovstack;
+	struct iov_iter iter;
+
+	ret = import_iovec(READ, vec, vlen, ARRAY_SIZE(iovstack), &iov, &iter);
+	if (ret >= 0) {
+		ret = do_process_madvise(pidfd, &iter, behavior, flags);
+		kfree(iov);
+	}
+	return ret;
+}
+
+#ifdef CONFIG_COMPAT
+COMPAT_SYSCALL_DEFINE5(process_madvise, compat_int_t, pidfd,
+			const struct compat_iovec __user *, vec,
+			compat_ulong_t, vlen,
+			compat_int_t, behavior,
+			compat_uint_t, flags)
+
+{
+	ssize_t ret;
+	struct iovec iovstack[UIO_FASTIOV];
+	struct iovec *iov = iovstack;
+	struct iov_iter iter;
+
+	ret = compat_import_iovec(READ, vec, vlen, ARRAY_SIZE(iovstack),
+				&iov, &iter);
+	if (ret >= 0) {
+		ret = do_process_madvise(pidfd, &iter, behavior, flags);
+		kfree(iov);
+	}
+	return ret;
+}
+#endif

Note, I'm only commenting on this patch because it has already been
dropped for this merge window. Otherwise I wouldn't interfer with stuff
that has already been sent for inclusion.

I haven't noticed this before but why do you need this
COMPAT_SYSCALL_DEFINE5()? New code we add today tries pretty hard to
avoid the compat syscall definitions. (See what we did for
pidfd_send_signal(), seccomp, and in io_uring and in various other places.)

Afaict, this could just be sm like (__completely untested__):

static inline int madv_import_iovec(int type, const struct iovec __user *uvec, unsigned nr_segs,
				    unsigned fast_segs, struct iovec **iov, struct iov_iter *i)
{
#ifdef CONFIG_COMPAT
	if (in_compat_syscall())
		return compat_import_iovec(type, (struct compat_iovec __user *)uvec, nr_segs,
					   fast_segs, iov, i);
#endif

	return import_iovec(type, uvec, nr_segs, fast_segs, iov, i);
}

SYSCALL_DEFINE5(process_madvise, int, pidfd, const struct iovec __user *, vec,
		 unsigned long, vlen, int, behavior, unsigned int, flags)
{
	ssize_t ret;
	struct iovec iovstack[UIO_FASTIOV];
	struct iovec *iov = iovstack;
	struct iov_iter iter;

	ret = madv_import_iovec(READ, vec, vlen, ARRAY_SIZE(iovstack), &iov, &iter);
	if (ret < 0)
		return ret;

	ret = do_process_madvise(pidfd, &iter, behavior, flags);
	kfree(iov);
	return ret;
}

or is there are specific reason this wouldn't work here?

No, I just didn't know such trend to avoid compact syscall definitions.
Thanks for the information.

I think your suggestion will work. Let me have it at respin.
Thanks!