From: Vamsi Krishna Attunuru <vattunuru@marvell.com>
To: David Marchand <david.marchand@redhat.com>,
Ferruh Yigit <ferruh.yigit@amd.com>,
Kiran Kumar Kokkilagadda <kirankumark@marvell.com>,
Jerin Jacob Kollanukkaran <jerinj@marvell.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>, Thomas Monjalon <thomas@monjalon.net>
Subject: RE: [EXT] Re: [PATCH] kni: fix build with Linux 6.3
Date: Fri, 24 Mar 2023 03:04:37 +0000 [thread overview]
Message-ID: <MWHPR18MB0942EFCA7A6BC50DD991EF81A6849@MWHPR18MB0942.namprd18.prod.outlook.com> (raw)
In-Reply-To: <CAJFAV8yiSPtwv0SGWXRi6kVDWonc-dJy0mc8xAJb_h1hOcWAuw@mail.gmail.com>
> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Monday, March 20, 2023 6:31 PM
> To: Ferruh Yigit <ferruh.yigit@amd.com>; Vamsi Krishna Attunuru
> <vattunuru@marvell.com>; Kiran Kumar Kokkilagadda
> <kirankumark@marvell.com>; Jerin Jacob Kollanukkaran
> <jerinj@marvell.com>
> Cc: dev@dpdk.org; Thomas Monjalon <thomas@monjalon.net>
> Subject: [EXT] Re: [PATCH] kni: fix build with Linux 6.3
>
> External Email
>
> ----------------------------------------------------------------------
> On Mon, Mar 20, 2023 at 1:10 PM David Marchand
> <david.marchand@redhat.com> wrote:
> >
> > On Tue, Feb 28, 2023 at 9:45 PM David Marchand
> > <david.marchand@redhat.com> wrote:
> > >
> > > On Tue, Feb 28, 2023 at 6:29 PM Ferruh Yigit <ferruh.yigit@amd.com>
> wrote:
> > > >
> > > > KNI calls `get_user_pages_remote()` API which is using
> > > > `FOLL_TOUCH` flag, but `FOLL_TOUCH` is no more in public headers
> > > > since v6.3, causing a build error.
> > >
> > > Something looks strange with what kni was doing.
> > >
> > > Looking at get_user_pages_remote implementation, I see it internally
> > > passes FOLL_TOUCH in addition to passed gup_flags.
> > > And looking at FOLL_TOUCH definition, it seems natural (to me) that
> > > this flag would be handled internally.
> > >
> > > Maybe it changed over time... but then the question is when did
> > > passing FOLL_TOUCH become unneeded?
> >
> > Here is some more info.
> >
> > get_user_pages_remote() was added in kernel commit 1e9877902dc7
> > ("mm/gup: Introduce get_user_pages_remote()").
> > At this time, it was passing the FOLL_TOUCH flag internally.
> >
> > +long get_user_pages_remote(struct task_struct *tsk, struct mm_struct
> *mm,
> > + unsigned long start, unsigned long nr_pages,
> > + int write, int force, struct page **pages,
> > + struct vm_area_struct **vmas)
> > {
> > return __get_user_pages_locked(tsk, mm, start, nr_pages, write,
> force,
> > - pages, vmas, NULL, false, FOLL_TOUCH);
> > + pages, vmas, NULL, false,
> > + FOLL_TOUCH | FOLL_REMOTE); }
> > +EXPORT_SYMBOL(get_user_pages_remote);
> >
> > get_user_pages_remote() later gained the ability to pass gup flags in
> > kernel commit 9beae1ea8930 ("mm: replace get_user_pages_remote()
> > write/force parameters with gup_flags").
> > But FOLL_TOUCH was still added internally.
> >
> > long get_user_pages_remote(struct task_struct *tsk, struct mm_struct
> *mm,
> > unsigned long start, unsigned long nr_pages,
> > - int write, int force, struct page **pages,
> > + unsigned int gup_flags, struct page **pages,
> > struct vm_area_struct **vmas) {
> > - unsigned int flags = FOLL_TOUCH | FOLL_REMOTE;
> > -
> > - if (write)
> > - flags |= FOLL_WRITE;
> > - if (force)
> > - flags |= FOLL_FORCE;
> > -
> > return __get_user_pages_locked(tsk, mm, start, nr_pages, pages,
> vmas,
> > - NULL, false, flags);
> > + NULL, false,
> > + gup_flags | FOLL_TOUCH |
> > + FOLL_REMOTE);
> > }
> >
> >
> > There were other changes in this area of the kernel code, but I did
> > not notice a change in relation with FOLL_TOUCH.
> >
> > So I think the dpdk commit e73831dc6c26 ("kni: support userspace VA")
> > uselessly introduced call to this flag and we can remove it.
> > Adding author and reviewers of this change.
>
> Alternatively, we could go with passing 0 in flags when FOLL_TOUCH is not
> exported.
> Something like:
Yes, this flag is useless, I vaguely remember like I added it from v1(in that patch series) itself along with multiple kernel version checks,
but by looking at the internals neither of them would not need it.
We could pass 0 in flags as suggested.
>
> diff --git a/kernel/linux/kni/compat.h b/kernel/linux/kni/compat.h index
> 7aa6cd9fca..3164a48971 100644
> --- a/kernel/linux/kni/compat.h
> +++ b/kernel/linux/kni/compat.h
> @@ -129,6 +129,14 @@
> */
> #if KERNEL_VERSION(4, 10, 0) <= LINUX_VERSION_CODE #define
> HAVE_IOVA_TO_KVA_MAPPING_SUPPORT
> +
> +/*
> + * get_user_pages_remote() should not require the flag FOLL_TOUCH to be
> passed.
> + * Simply pass it as 0 when this flag is internal and not exported anymore.
> + */
> +#ifndef FOLL_TOUCH
> +#define FOLL_TOUCH 0
> +#endif
> #endif
>
> #if KERNEL_VERSION(5, 6, 0) <= LINUX_VERSION_CODE || \
>
>
> --
> David Marchand
next prev parent reply other threads:[~2023-03-24 3:04 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-28 17:29 Ferruh Yigit
2023-02-28 20:45 ` David Marchand
2023-03-20 12:10 ` David Marchand
2023-03-20 13:01 ` David Marchand
2023-03-24 3:04 ` Vamsi Krishna Attunuru [this message]
2023-04-13 7:22 ` [EXT] " David Marchand
2023-04-14 15:29 ` Ferruh Yigit
2023-04-14 15:25 ` [PATCH v2] " Ferruh Yigit
2023-04-17 7:32 ` David Marchand
2023-04-19 14:38 ` David Marchand
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=MWHPR18MB0942EFCA7A6BC50DD991EF81A6849@MWHPR18MB0942.namprd18.prod.outlook.com \
--to=vattunuru@marvell.com \
--cc=david.marchand@redhat.com \
--cc=dev@dpdk.org \
--cc=ferruh.yigit@amd.com \
--cc=jerinj@marvell.com \
--cc=kirankumark@marvell.com \
--cc=thomas@monjalon.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).