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:
> > From: Minchan Kim <minchan@xxxxxxxxxx>
> > Subject: mm/madvise: introduce process_madvise() syscall: an external memory hinting API
> >
>
> <snip>
>
> > +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!
