* [PATCH 0/2] IB/rdmavt: cq ktrhead worker fix and API update
From: Petr Mladek @ 2016-10-19 12:07 UTC (permalink / raw)
To: Doug Ledford, Sean Hefty
Cc: Dennis Dalessandro, Hal Rosenstock, linux-rdma, Tejun Heo,
linux-kernel, Petr Mladek
The kthread worker API has been improved in 4.9-rc1. The 2nd
patch uses the new functions and simplifies the kthread worker
creation and destroying.
I have found a possible race when working on the API conversion.
A proposed fix is in the 1st patch.
Both changes are compile tested only. I did not find an easy way
how to test them at runtime.
Petr Mladek (2):
IB/rdmavt: Avoid queuing work into a destroyed cq kthread worker
IB/rdmavt: Handle the kthread worker using the new API
drivers/infiniband/sw/rdmavt/cq.c | 64 +++++++++++++++++----------------------
1 file changed, 27 insertions(+), 37 deletions(-)
--
1.8.5.6
^ permalink raw reply
* Re: [PATCH 10/10] mm: replace access_process_vm() write parameter with gup_flags
From: Michael Ellerman @ 2016-10-19 11:10 UTC (permalink / raw)
To: linux-mm
Cc: linux-mips, linux-fbdev, Jan Kara, kvm, linux-sh, Dave Hansen,
dri-devel, netdev, sparclinux, linux-ia64, linux-s390,
linux-samsung-soc, linux-scsi, linux-rdma, x86, Hugh Dickins,
linux-media, Rik van Riel, intel-gfx, adi-buildroot-devel,
ceph-devel, linux-arm-kernel, Lorenzo Stoakes, linux-cris-kernel,
Linus Torvalds, linuxppc-dev, linux-kernel, linux-security-module,
linux-alpha
In-Reply-To: <20161013002020.3062-11-lstoakes@gmail.com>
Lorenzo Stoakes <lstoakes@gmail.com> writes:
> diff --git a/arch/powerpc/kernel/ptrace32.c b/arch/powerpc/kernel/ptrace32.c
> index f52b7db3..010b7b3 100644
> --- a/arch/powerpc/kernel/ptrace32.c
> +++ b/arch/powerpc/kernel/ptrace32.c
> @@ -74,7 +74,7 @@ long compat_arch_ptrace(struct task_struct *child, compat_long_t request,
> break;
>
> copied = access_process_vm(child, (u64)addrOthers, &tmp,
> - sizeof(tmp), 0);
> + sizeof(tmp), FOLL_FORCE);
> if (copied != sizeof(tmp))
> break;
> ret = put_user(tmp, (u32 __user *)data);
LGTM.
> @@ -179,7 +179,8 @@ long compat_arch_ptrace(struct task_struct *child, compat_long_t request,
> break;
> ret = 0;
> if (access_process_vm(child, (u64)addrOthers, &tmp,
> - sizeof(tmp), 1) == sizeof(tmp))
> + sizeof(tmp),
> + FOLL_FORCE | FOLL_WRITE) == sizeof(tmp))
> break;
> ret = -EIO;
> break;
If you're respinning this anyway, can you format that as:
if (access_process_vm(child, (u64)addrOthers, &tmp, sizeof(tmp),
FOLL_FORCE | FOLL_WRITE) == sizeof(tmp))
break;
I realise you probably deliberately didn't do that to make the diff clearer.
Either way:
Acked-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc)
cheers
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCHv12 0/3] rdmacg: IB/core: rdma controller support
From: Parav Pandit @ 2016-10-19 9:34 UTC (permalink / raw)
To: Tejun Heo
Cc: Leon Romanovsky, cgroups-u79uwXL29TY76Z2rM5mHXA, linux-rdma,
Li Zefan, Johannes Weiner, Doug Ledford, Christoph Hellwig,
Liran Liss, Hefty, Sean, Jason Gunthorpe, Haggai Eran,
james.l.morris-QHcLZuEGTsvQT0dZR+AlfA, Or Gerlitz, Matan Barak
In-Reply-To: <20161018215134.GB2761-piEFEHQLUPpN0TnZuCh8vA@public.gmane.org>
Hi Tejun,
On Wed, Oct 19, 2016 at 3:21 AM, Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> Hello,
>
> On Wed, Oct 19, 2016 at 01:32:01AM +0530, Parav Pandit wrote:
>> On Fri, Oct 14, 2016 at 4:44 AM, Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>> > I think what you want is just a way to specify absolute limits using
>> > percentages of what's available at the time of configuration -
>> > e.g. being able to say "allow upto 30% of what's available in the
>> > parent".
>>
>> Yes. I am concerned about how to configure < 1% value to avoid
>> floating point math in kernel as thats discouraged.
>> Configuring in range of 1 to 100% for a given resource limits to only
>> 100 or less cgroup instances which I think is not desired.
>
> Heh, we can go for per-mil and use %0 as the suffix if absolutely
> necessary but is this a real issue?
>
We need to provide current version to exceed the 100 containers limit first. :-)
Please let me know if such user interface already exist as reference point.
I prefer to avoid introducing such configuration.
>> > If so, the simplest way would be simply updating the
>> > existing knobs to accept % inputs in addition to absolute values on
>> > writes.
>>
>> I was not sure to overload rdma.max file for accepting % inputs as
>> thats not done in other cgroups. So I was thinking more of weights
>> interface which avoids floating point problem and also allows much
>> wider configuration range.
>
> I think it's a lot more consistent to implement all absoulte limits
> through max.
I agree. I will implement reporting actual max values instead of
reporting "max" string so that percentage configuration can be done by
the user space tools.
(post merging of v12).
Waiting for direction from Doug.
> weight is for actual proportional control which this
> isn't. It's just a fancy way of specifying absolute limits.
>
> Thanks.
>
> --
> tejun
^ permalink raw reply
* Re: [PATCH 08/10] mm: replace __access_remote_vm() write parameter with gup_flags
From: Michal Hocko @ 2016-10-19 9:23 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: Jan Kara, linux-mm, Linus Torvalds, Hugh Dickins, Dave Hansen,
Rik van Riel, Mel Gorman, Andrew Morton, adi-buildroot-devel,
ceph-devel, dri-devel, intel-gfx, kvm, linux-alpha,
linux-arm-kernel, linux-cris-kernel, linux-fbdev, linux-fsdevel,
linux-ia64, linux-kernel, linux-media, linux-mips, linux-rdma,
linux-s390, linux-samsung-soc, linux-scsi, linux-security-module,
linux-sh
In-Reply-To: <20161019090646.GA24243@lucifer>
On Wed 19-10-16 10:06:46, Lorenzo Stoakes wrote:
> On Wed, Oct 19, 2016 at 10:52:05AM +0200, Michal Hocko wrote:
> > yes this is the desirable and expected behavior.
> >
> > > wonder if this is desirable behaviour or whether this ought to be limited to
> > > ptrace system calls. Regardless, by making the flag more visible it makes it
> > > easier to see that this is happening.
> >
> > mem_open already enforces PTRACE_MODE_ATTACH
>
> Ah I missed this, that makes a lot of sense, thanks!
>
> I still wonder whether other invocations of access_remote_vm() in fs/proc/base.c
> (the principle caller of this function) need FOLL_FORCE, for example the various
> calls that simply read data from other processes, so I think the point stands
> about keeping this explicit.
I do agree. Making them explicit will help to clean them up later,
should there be a need.
--
Michal Hocko
SUSE Labs
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH 00/10] mm: adjust get_user_pages* functions to explicitly pass FOLL_* flags
From: Michal Hocko @ 2016-10-19 9:07 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: linux-mm, Linus Torvalds, Jan Kara, Hugh Dickins, Dave Hansen,
Rik van Riel, Mel Gorman, Andrew Morton, adi-buildroot-devel,
ceph-devel, dri-devel, intel-gfx, kvm, linux-alpha,
linux-arm-kernel, linux-cris-kernel, linux-fbdev, linux-fsdevel,
linux-ia64, linux-kernel, linux-media, linux-mips, linux-rdma,
linux-s390, linux-samsung-soc, linux-scsi, linux-security-module,
linux-sh@
In-Reply-To: <20161019085815.GA22239@lucifer>
On Wed 19-10-16 09:58:15, Lorenzo Stoakes wrote:
> On Tue, Oct 18, 2016 at 05:30:50PM +0200, Michal Hocko wrote:
> > I am wondering whether we can go further. E.g. it is not really clear to
> > me whether we need an explicit FOLL_REMOTE when we can in fact check
> > mm != current->mm and imply that. Maybe there are some contexts which
> > wouldn't work, I haven't checked.
>
> This flag is set even when /proc/self/mem is used. I've not looked deeply into
> this flag but perhaps accessing your own memory this way can be considered
> 'remote' since you're not accessing it directly. On the other hand, perhaps this
> is just mistaken in this case?
My understanding of the flag is quite limited as well. All I know it is
related to protection keys and it is needed to bypass protection check.
See arch_vma_access_permitted. See also 1b2ee1266ea6 ("mm/core: Do not
enforce PKEY permissions on remote mm access").
--
Michal Hocko
SUSE Labs
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH 08/10] mm: replace __access_remote_vm() write parameter with gup_flags
From: Lorenzo Stoakes @ 2016-10-19 9:06 UTC (permalink / raw)
To: Michal Hocko
Cc: Jan Kara, linux-mm, Linus Torvalds, Hugh Dickins, Dave Hansen,
Rik van Riel, Mel Gorman, Andrew Morton, adi-buildroot-devel,
ceph-devel, dri-devel, intel-gfx, kvm, linux-alpha,
linux-arm-kernel, linux-cris-kernel, linux-fbdev, linux-fsdevel,
linux-ia64, linux-kernel, linux-media, linux-mips, linux-rdma,
linux-s390, linux-samsung-soc, linux-scsi, linux-security-module,
linux-sh
In-Reply-To: <20161019085204.GD7517@dhcp22.suse.cz>
On Wed, Oct 19, 2016 at 10:52:05AM +0200, Michal Hocko wrote:
> yes this is the desirable and expected behavior.
>
> > wonder if this is desirable behaviour or whether this ought to be limited to
> > ptrace system calls. Regardless, by making the flag more visible it makes it
> > easier to see that this is happening.
>
> mem_open already enforces PTRACE_MODE_ATTACH
Ah I missed this, that makes a lot of sense, thanks!
I still wonder whether other invocations of access_remote_vm() in fs/proc/base.c
(the principle caller of this function) need FOLL_FORCE, for example the various
calls that simply read data from other processes, so I think the point stands
about keeping this explicit.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH 00/10] mm: adjust get_user_pages* functions to explicitly pass FOLL_* flags
From: Lorenzo Stoakes @ 2016-10-19 8:58 UTC (permalink / raw)
To: Michal Hocko
Cc: linux-mips, linux-fbdev, Jan Kara, kvm, linux-sh, Dave Hansen,
dri-devel, linux-mm, netdev, sparclinux, linux-ia64, linux-s390,
linux-samsung-soc, linux-scsi, linux-rdma, x86, Hugh Dickins,
linux-media, Rik van Riel, intel-gfx, adi-buildroot-devel,
ceph-devel, linux-arm-kernel, linux-cris-kernel, Linus Torvalds,
linuxppc-dev, linux-kernel, linux-security-module, linux-alpha
In-Reply-To: <20161018153050.GC13117@dhcp22.suse.cz>
On Tue, Oct 18, 2016 at 05:30:50PM +0200, Michal Hocko wrote:
> I am wondering whether we can go further. E.g. it is not really clear to
> me whether we need an explicit FOLL_REMOTE when we can in fact check
> mm != current->mm and imply that. Maybe there are some contexts which
> wouldn't work, I haven't checked.
This flag is set even when /proc/self/mem is used. I've not looked deeply into
this flag but perhaps accessing your own memory this way can be considered
'remote' since you're not accessing it directly. On the other hand, perhaps this
is just mistaken in this case?
> I guess there is more work in that area and I do not want to impose all
> that work on you, but I couldn't resist once I saw you playing in that
> area ;) Definitely a good start!
Thanks, I am more than happy to go as far down this rabbit hole as is helpful,
no imposition at all :)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply
* Re: [PATCH 08/10] mm: replace __access_remote_vm() write parameter with gup_flags
From: Michal Hocko @ 2016-10-19 8:52 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: Jan Kara, linux-mm, Linus Torvalds, Hugh Dickins, Dave Hansen,
Rik van Riel, Mel Gorman, Andrew Morton, adi-buildroot-devel,
ceph-devel, dri-devel, intel-gfx, kvm, linux-alpha,
linux-arm-kernel, linux-cris-kernel, linux-fbdev, linux-fsdevel,
linux-ia64, linux-kernel, linux-media, linux-mips, linux-rdma,
linux-s390, linux-samsung-soc, linux-scsi, linux-security-module,
linux-sh
In-Reply-To: <20161019084045.GA19441@lucifer>
On Wed 19-10-16 09:40:45, Lorenzo Stoakes wrote:
> On Wed, Oct 19, 2016 at 10:13:52AM +0200, Michal Hocko wrote:
> > On Wed 19-10-16 09:59:03, Jan Kara wrote:
> > > On Thu 13-10-16 01:20:18, Lorenzo Stoakes wrote:
> > > > This patch removes the write parameter from __access_remote_vm() and replaces it
> > > > with a gup_flags parameter as use of this function previously _implied_
> > > > FOLL_FORCE, whereas after this patch callers explicitly pass this flag.
> > > >
> > > > We make this explicit as use of FOLL_FORCE can result in surprising behaviour
> > > > (and hence bugs) within the mm subsystem.
> > > >
> > > > Signed-off-by: Lorenzo Stoakes <lstoakes@gmail.com>
> > >
> > > So I'm not convinced this (and the following two patches) is actually
> > > helping much. By grepping for FOLL_FORCE we will easily see that any caller
> > > of access_remote_vm() gets that semantics and can thus continue search
> >
> > I am really wondering. Is there anything inherent that would require
> > FOLL_FORCE for access_remote_vm? I mean FOLL_FORCE is a really
> > non-trivial thing. It doesn't obey vma permissions so we should really
> > minimize its usage. Do all of those users really need FOLL_FORCE?
>
> I wonder about this also, for example by accessing /proc/self/mem you trigger
> access_remote_vm() and consequently get_user_pages_remote() meaning FOLL_FORCE
> is implied and you can use /proc/self/mem to override any VMA permissions. I
yes this is the desirable and expected behavior.
> wonder if this is desirable behaviour or whether this ought to be limited to
> ptrace system calls. Regardless, by making the flag more visible it makes it
> easier to see that this is happening.
mem_open already enforces PTRACE_MODE_ATTACH
--
Michal Hocko
SUSE Labs
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH 08/10] mm: replace __access_remote_vm() write parameter with gup_flags
From: Lorenzo Stoakes @ 2016-10-19 8:40 UTC (permalink / raw)
To: Michal Hocko
Cc: Jan Kara, linux-mm, Linus Torvalds, Hugh Dickins, Dave Hansen,
Rik van Riel, Mel Gorman, Andrew Morton, adi-buildroot-devel,
ceph-devel, dri-devel, intel-gfx, kvm, linux-alpha,
linux-arm-kernel, linux-cris-kernel, linux-fbdev, linux-fsdevel,
linux-ia64, linux-kernel, linux-media, linux-mips, linux-rdma,
linux-s390, linux-samsung-soc, linux-scsi, linux-security-module,
linux-sh
In-Reply-To: <20161019081352.GB7562@dhcp22.suse.cz>
On Wed, Oct 19, 2016 at 10:13:52AM +0200, Michal Hocko wrote:
> On Wed 19-10-16 09:59:03, Jan Kara wrote:
> > On Thu 13-10-16 01:20:18, Lorenzo Stoakes wrote:
> > > This patch removes the write parameter from __access_remote_vm() and replaces it
> > > with a gup_flags parameter as use of this function previously _implied_
> > > FOLL_FORCE, whereas after this patch callers explicitly pass this flag.
> > >
> > > We make this explicit as use of FOLL_FORCE can result in surprising behaviour
> > > (and hence bugs) within the mm subsystem.
> > >
> > > Signed-off-by: Lorenzo Stoakes <lstoakes@gmail.com>
> >
> > So I'm not convinced this (and the following two patches) is actually
> > helping much. By grepping for FOLL_FORCE we will easily see that any caller
> > of access_remote_vm() gets that semantics and can thus continue search
>
> I am really wondering. Is there anything inherent that would require
> FOLL_FORCE for access_remote_vm? I mean FOLL_FORCE is a really
> non-trivial thing. It doesn't obey vma permissions so we should really
> minimize its usage. Do all of those users really need FOLL_FORCE?
I wonder about this also, for example by accessing /proc/self/mem you trigger
access_remote_vm() and consequently get_user_pages_remote() meaning FOLL_FORCE
is implied and you can use /proc/self/mem to override any VMA permissions. I
wonder if this is desirable behaviour or whether this ought to be limited to
ptrace system calls. Regardless, by making the flag more visible it makes it
easier to see that this is happening.
Note that this /proc/self/mem behaviour is what triggered the bug that brought
about this discussion in the first place -
https://marc.info/?l=linux-mm&m=147363447105059 - as using /proc/self/mem this
way on PROT_NONE memory broke some assumptions.
On the other hand I see your point Jan in that you know any caller of these
functions will have FOLL_FORCE implied, and you have to decide to stop passing
the flag at _some_ point in the stack, however it seems fairly sane to have that
stopping point exist outside of exported gup functions so the gup interface
forces explicitness but callers can wrap things as they need.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH 08/10] mm: replace __access_remote_vm() write parameter with gup_flags
From: Michal Hocko @ 2016-10-19 8:13 UTC (permalink / raw)
To: Jan Kara
Cc: linux-mips, linux-fbdev, linux-ia64, kvm, linux-sh, Dave Hansen,
dri-devel, linux-mm, netdev, sparclinux, linux-s390,
linux-samsung-soc, linux-scsi, linux-rdma, x86, Hugh Dickins,
linux-media, Rik van Riel, intel-gfx, adi-buildroot-devel,
ceph-devel, linux-arm-kernel, Lorenzo Stoakes, linux-cris-kernel,
Linus Torvalds, linuxppc-dev, linux-kernel, linux-security-module,
linux-alpha
In-Reply-To: <20161019075903.GP29967@quack2.suse.cz>
On Wed 19-10-16 09:59:03, Jan Kara wrote:
> On Thu 13-10-16 01:20:18, Lorenzo Stoakes wrote:
> > This patch removes the write parameter from __access_remote_vm() and replaces it
> > with a gup_flags parameter as use of this function previously _implied_
> > FOLL_FORCE, whereas after this patch callers explicitly pass this flag.
> >
> > We make this explicit as use of FOLL_FORCE can result in surprising behaviour
> > (and hence bugs) within the mm subsystem.
> >
> > Signed-off-by: Lorenzo Stoakes <lstoakes@gmail.com>
>
> So I'm not convinced this (and the following two patches) is actually
> helping much. By grepping for FOLL_FORCE we will easily see that any caller
> of access_remote_vm() gets that semantics and can thus continue search
I am really wondering. Is there anything inherent that would require
FOLL_FORCE for access_remote_vm? I mean FOLL_FORCE is a really
non-trivial thing. It doesn't obey vma permissions so we should really
minimize its usage. Do all of those users really need FOLL_FORCE?
Anyway I would rather see the flag explicit and used at more places than
hidden behind a helper function.
--
Michal Hocko
SUSE Labs
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply
* Re: [PATCH 08/10] mm: replace __access_remote_vm() write parameter with gup_flags
From: Jan Kara @ 2016-10-19 7:59 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: linux-mm, Linus Torvalds, Jan Kara, Hugh Dickins, Dave Hansen,
Rik van Riel, Mel Gorman, Andrew Morton, adi-buildroot-devel,
ceph-devel, dri-devel, intel-gfx, kvm, linux-alpha,
linux-arm-kernel, linux-cris-kernel, linux-fbdev, linux-fsdevel,
linux-ia64, linux-kernel, linux-media, linux-mips, linux-rdma,
linux-s390, linux-samsung-soc, linux-scsi, linux-security-module,
linux-sh@
In-Reply-To: <20161013002020.3062-9-lstoakes@gmail.com>
On Thu 13-10-16 01:20:18, Lorenzo Stoakes wrote:
> This patch removes the write parameter from __access_remote_vm() and replaces it
> with a gup_flags parameter as use of this function previously _implied_
> FOLL_FORCE, whereas after this patch callers explicitly pass this flag.
>
> We make this explicit as use of FOLL_FORCE can result in surprising behaviour
> (and hence bugs) within the mm subsystem.
>
> Signed-off-by: Lorenzo Stoakes <lstoakes@gmail.com>
So I'm not convinced this (and the following two patches) is actually
helping much. By grepping for FOLL_FORCE we will easily see that any caller
of access_remote_vm() gets that semantics and can thus continue search
accordingly (it is much simpler than searching for all get_user_pages()
users and extracting from parameter lists what they actually pass as
'force' argument). Sure it makes somewhat more visible to callers of
access_remote_vm() that they get FOLL_FORCE semantics but OTOH it also
opens a space for issues where a caller of access_remote_vm() actually
wants FOLL_FORCE (and currently all of them want it) and just mistakenly
does not set it. All in all I'd prefer to keep access_remote_vm() and
friends as is...
Honza
> ---
> mm/memory.c | 23 +++++++++++++++--------
> mm/nommu.c | 9 ++++++---
> 2 files changed, 21 insertions(+), 11 deletions(-)
>
> diff --git a/mm/memory.c b/mm/memory.c
> index 20a9adb..79ebed3 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -3869,14 +3869,11 @@ EXPORT_SYMBOL_GPL(generic_access_phys);
> * given task for page fault accounting.
> */
> static int __access_remote_vm(struct task_struct *tsk, struct mm_struct *mm,
> - unsigned long addr, void *buf, int len, int write)
> + unsigned long addr, void *buf, int len, unsigned int gup_flags)
> {
> struct vm_area_struct *vma;
> void *old_buf = buf;
> - unsigned int flags = FOLL_FORCE;
> -
> - if (write)
> - flags |= FOLL_WRITE;
> + int write = gup_flags & FOLL_WRITE;
>
> down_read(&mm->mmap_sem);
> /* ignore errors, just check how much was successfully transferred */
> @@ -3886,7 +3883,7 @@ static int __access_remote_vm(struct task_struct *tsk, struct mm_struct *mm,
> struct page *page = NULL;
>
> ret = get_user_pages_remote(tsk, mm, addr, 1,
> - flags, &page, &vma);
> + gup_flags, &page, &vma);
> if (ret <= 0) {
> #ifndef CONFIG_HAVE_IOREMAP_PROT
> break;
> @@ -3945,7 +3942,12 @@ static int __access_remote_vm(struct task_struct *tsk, struct mm_struct *mm,
> int access_remote_vm(struct mm_struct *mm, unsigned long addr,
> void *buf, int len, int write)
> {
> - return __access_remote_vm(NULL, mm, addr, buf, len, write);
> + unsigned int flags = FOLL_FORCE;
> +
> + if (write)
> + flags |= FOLL_WRITE;
> +
> + return __access_remote_vm(NULL, mm, addr, buf, len, flags);
> }
>
> /*
> @@ -3958,12 +3960,17 @@ int access_process_vm(struct task_struct *tsk, unsigned long addr,
> {
> struct mm_struct *mm;
> int ret;
> + unsigned int flags = FOLL_FORCE;
>
> mm = get_task_mm(tsk);
> if (!mm)
> return 0;
>
> - ret = __access_remote_vm(tsk, mm, addr, buf, len, write);
> + if (write)
> + flags |= FOLL_WRITE;
> +
> + ret = __access_remote_vm(tsk, mm, addr, buf, len, flags);
> +
> mmput(mm);
>
> return ret;
> diff --git a/mm/nommu.c b/mm/nommu.c
> index 70cb844..bde7df3 100644
> --- a/mm/nommu.c
> +++ b/mm/nommu.c
> @@ -1809,9 +1809,10 @@ void filemap_map_pages(struct fault_env *fe,
> EXPORT_SYMBOL(filemap_map_pages);
>
> static int __access_remote_vm(struct task_struct *tsk, struct mm_struct *mm,
> - unsigned long addr, void *buf, int len, int write)
> + unsigned long addr, void *buf, int len, unsigned int gup_flags)
> {
> struct vm_area_struct *vma;
> + int write = gup_flags & FOLL_WRITE;
>
> down_read(&mm->mmap_sem);
>
> @@ -1853,7 +1854,8 @@ static int __access_remote_vm(struct task_struct *tsk, struct mm_struct *mm,
> int access_remote_vm(struct mm_struct *mm, unsigned long addr,
> void *buf, int len, int write)
> {
> - return __access_remote_vm(NULL, mm, addr, buf, len, write);
> + return __access_remote_vm(NULL, mm, addr, buf, len,
> + write ? FOLL_WRITE : 0);
> }
>
> /*
> @@ -1871,7 +1873,8 @@ int access_process_vm(struct task_struct *tsk, unsigned long addr, void *buf, in
> if (!mm)
> return 0;
>
> - len = __access_remote_vm(tsk, mm, addr, buf, len, write);
> + len = __access_remote_vm(tsk, mm, addr, buf, len,
> + write ? FOLL_WRITE : 0);
>
> mmput(mm);
> return len;
> --
> 2.10.0
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH 07/10] mm: replace get_user_pages_remote() write/force parameters with gup_flags
From: Jan Kara @ 2016-10-19 7:47 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: linux-mips, linux-fbdev, Jan Kara, kvm, linux-sh, Dave Hansen,
dri-devel, linux-mm, netdev, sparclinux, linux-ia64, linux-s390,
linux-samsung-soc, linux-scsi, linux-rdma, x86, Hugh Dickins,
linux-media, Rik van Riel, intel-gfx, adi-buildroot-devel,
ceph-devel, linux-arm-kernel, linux-cris-kernel, Linus Torvalds,
linuxppc-dev, linux-kernel, linux-security-module, linux-alpha
In-Reply-To: <20161013002020.3062-8-lstoakes@gmail.com>
On Thu 13-10-16 01:20:17, Lorenzo Stoakes wrote:
> This patch removes the write and force parameters from get_user_pages_remote()
> and replaces them with a gup_flags parameter to make the use of FOLL_FORCE
> explicit in callers as use of this flag can result in surprising behaviour (and
> hence bugs) within the mm subsystem.
>
> Signed-off-by: Lorenzo Stoakes <lstoakes@gmail.com>
Looks good. You can add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> drivers/gpu/drm/etnaviv/etnaviv_gem.c | 7 +++++--
> drivers/gpu/drm/i915/i915_gem_userptr.c | 6 +++++-
> drivers/infiniband/core/umem_odp.c | 7 +++++--
> fs/exec.c | 9 +++++++--
> include/linux/mm.h | 2 +-
> kernel/events/uprobes.c | 6 ++++--
> mm/gup.c | 22 +++++++---------------
> mm/memory.c | 6 +++++-
> security/tomoyo/domain.c | 2 +-
> 9 files changed, 40 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> index 5ce3603..0370b84 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> @@ -748,19 +748,22 @@ static struct page **etnaviv_gem_userptr_do_get_pages(
> int ret = 0, pinned, npages = etnaviv_obj->base.size >> PAGE_SHIFT;
> struct page **pvec;
> uintptr_t ptr;
> + unsigned int flags = 0;
>
> pvec = drm_malloc_ab(npages, sizeof(struct page *));
> if (!pvec)
> return ERR_PTR(-ENOMEM);
>
> + if (!etnaviv_obj->userptr.ro)
> + flags |= FOLL_WRITE;
> +
> pinned = 0;
> ptr = etnaviv_obj->userptr.ptr;
>
> down_read(&mm->mmap_sem);
> while (pinned < npages) {
> ret = get_user_pages_remote(task, mm, ptr, npages - pinned,
> - !etnaviv_obj->userptr.ro, 0,
> - pvec + pinned, NULL);
> + flags, pvec + pinned, NULL);
> if (ret < 0)
> break;
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c b/drivers/gpu/drm/i915/i915_gem_userptr.c
> index e537930..c6f780f 100644
> --- a/drivers/gpu/drm/i915/i915_gem_userptr.c
> +++ b/drivers/gpu/drm/i915/i915_gem_userptr.c
> @@ -508,6 +508,10 @@ __i915_gem_userptr_get_pages_worker(struct work_struct *_work)
> pvec = drm_malloc_gfp(npages, sizeof(struct page *), GFP_TEMPORARY);
> if (pvec != NULL) {
> struct mm_struct *mm = obj->userptr.mm->mm;
> + unsigned int flags = 0;
> +
> + if (!obj->userptr.read_only)
> + flags |= FOLL_WRITE;
>
> ret = -EFAULT;
> if (atomic_inc_not_zero(&mm->mm_users)) {
> @@ -517,7 +521,7 @@ __i915_gem_userptr_get_pages_worker(struct work_struct *_work)
> (work->task, mm,
> obj->userptr.ptr + pinned * PAGE_SIZE,
> npages - pinned,
> - !obj->userptr.read_only, 0,
> + flags,
> pvec + pinned, NULL);
> if (ret < 0)
> break;
> diff --git a/drivers/infiniband/core/umem_odp.c b/drivers/infiniband/core/umem_odp.c
> index 75077a0..1f0fe32 100644
> --- a/drivers/infiniband/core/umem_odp.c
> +++ b/drivers/infiniband/core/umem_odp.c
> @@ -527,6 +527,7 @@ int ib_umem_odp_map_dma_pages(struct ib_umem *umem, u64 user_virt, u64 bcnt,
> u64 off;
> int j, k, ret = 0, start_idx, npages = 0;
> u64 base_virt_addr;
> + unsigned int flags = 0;
>
> if (access_mask == 0)
> return -EINVAL;
> @@ -556,6 +557,9 @@ int ib_umem_odp_map_dma_pages(struct ib_umem *umem, u64 user_virt, u64 bcnt,
> goto out_put_task;
> }
>
> + if (access_mask & ODP_WRITE_ALLOWED_BIT)
> + flags |= FOLL_WRITE;
> +
> start_idx = (user_virt - ib_umem_start(umem)) >> PAGE_SHIFT;
> k = start_idx;
>
> @@ -574,8 +578,7 @@ int ib_umem_odp_map_dma_pages(struct ib_umem *umem, u64 user_virt, u64 bcnt,
> */
> npages = get_user_pages_remote(owning_process, owning_mm,
> user_virt, gup_num_pages,
> - access_mask & ODP_WRITE_ALLOWED_BIT,
> - 0, local_page_list, NULL);
> + flags, local_page_list, NULL);
> up_read(&owning_mm->mmap_sem);
>
> if (npages < 0)
> diff --git a/fs/exec.c b/fs/exec.c
> index 6fcfb3f..4e497b9 100644
> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -191,6 +191,7 @@ static struct page *get_arg_page(struct linux_binprm *bprm, unsigned long pos,
> {
> struct page *page;
> int ret;
> + unsigned int gup_flags = FOLL_FORCE;
>
> #ifdef CONFIG_STACK_GROWSUP
> if (write) {
> @@ -199,12 +200,16 @@ static struct page *get_arg_page(struct linux_binprm *bprm, unsigned long pos,
> return NULL;
> }
> #endif
> +
> + if (write)
> + gup_flags |= FOLL_WRITE;
> +
> /*
> * We are doing an exec(). 'current' is the process
> * doing the exec and bprm->mm is the new process's mm.
> */
> - ret = get_user_pages_remote(current, bprm->mm, pos, 1, write,
> - 1, &page, NULL);
> + ret = get_user_pages_remote(current, bprm->mm, pos, 1, gup_flags,
> + &page, NULL);
> if (ret <= 0)
> return NULL;
>
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 686a477..2a481d3 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -1276,7 +1276,7 @@ long __get_user_pages(struct task_struct *tsk, struct mm_struct *mm,
> struct vm_area_struct **vmas, int *nonblocking);
> 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);
> long get_user_pages(unsigned long start, unsigned long nr_pages,
> unsigned int gup_flags, struct page **pages,
> diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
> index d4129bb..f9ec9ad 100644
> --- a/kernel/events/uprobes.c
> +++ b/kernel/events/uprobes.c
> @@ -300,7 +300,8 @@ int uprobe_write_opcode(struct mm_struct *mm, unsigned long vaddr,
>
> retry:
> /* Read the page with vaddr into memory */
> - ret = get_user_pages_remote(NULL, mm, vaddr, 1, 0, 1, &old_page, &vma);
> + ret = get_user_pages_remote(NULL, mm, vaddr, 1, FOLL_FORCE, &old_page,
> + &vma);
> if (ret <= 0)
> return ret;
>
> @@ -1710,7 +1711,8 @@ static int is_trap_at_addr(struct mm_struct *mm, unsigned long vaddr)
> * but we treat this as a 'remote' access since it is
> * essentially a kernel access to the memory.
> */
> - result = get_user_pages_remote(NULL, mm, vaddr, 1, 0, 1, &page, NULL);
> + result = get_user_pages_remote(NULL, mm, vaddr, 1, FOLL_FORCE, &page,
> + NULL);
> if (result < 0)
> return result;
>
> diff --git a/mm/gup.c b/mm/gup.c
> index dc91303..0deecf3 100644
> --- a/mm/gup.c
> +++ b/mm/gup.c
> @@ -905,9 +905,7 @@ EXPORT_SYMBOL(get_user_pages_unlocked);
> * @mm: mm_struct of target mm
> * @start: starting user address
> * @nr_pages: number of pages from start to pin
> - * @write: whether pages will be written to by the caller
> - * @force: whether to force access even when user mapping is currently
> - * protected (but never forces write access to shared mapping).
> + * @gup_flags: flags modifying lookup behaviour
> * @pages: array that receives pointers to the pages pinned.
> * Should be at least nr_pages long. Or NULL, if caller
> * only intends to ensure the pages are faulted in.
> @@ -936,9 +934,9 @@ EXPORT_SYMBOL(get_user_pages_unlocked);
> * or similar operation cannot guarantee anything stronger anyway because
> * locks can't be held over the syscall boundary.
> *
> - * If write=0, the page must not be written to. If the page is written to,
> - * set_page_dirty (or set_page_dirty_lock, as appropriate) must be called
> - * after the page is finished with, and before put_page is called.
> + * If gup_flags & FOLL_WRITE == 0, the page must not be written to. If the page
> + * is written to, set_page_dirty (or set_page_dirty_lock, as appropriate) must
> + * be called after the page is finished with, and before put_page is called.
> *
> * get_user_pages is typically used for fewer-copy IO operations, to get a
> * handle on the memory by some means other than accesses via the user virtual
> @@ -955,18 +953,12 @@ EXPORT_SYMBOL(get_user_pages_unlocked);
> */
> 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);
> }
> EXPORT_SYMBOL(get_user_pages_remote);
>
> diff --git a/mm/memory.c b/mm/memory.c
> index fc1987d..20a9adb 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -3873,6 +3873,10 @@ static int __access_remote_vm(struct task_struct *tsk, struct mm_struct *mm,
> {
> struct vm_area_struct *vma;
> void *old_buf = buf;
> + unsigned int flags = FOLL_FORCE;
> +
> + if (write)
> + flags |= FOLL_WRITE;
>
> down_read(&mm->mmap_sem);
> /* ignore errors, just check how much was successfully transferred */
> @@ -3882,7 +3886,7 @@ static int __access_remote_vm(struct task_struct *tsk, struct mm_struct *mm,
> struct page *page = NULL;
>
> ret = get_user_pages_remote(tsk, mm, addr, 1,
> - write, 1, &page, &vma);
> + flags, &page, &vma);
> if (ret <= 0) {
> #ifndef CONFIG_HAVE_IOREMAP_PROT
> break;
> diff --git a/security/tomoyo/domain.c b/security/tomoyo/domain.c
> index ade7c6c..682b73a 100644
> --- a/security/tomoyo/domain.c
> +++ b/security/tomoyo/domain.c
> @@ -881,7 +881,7 @@ bool tomoyo_dump_page(struct linux_binprm *bprm, unsigned long pos,
> * the execve().
> */
> if (get_user_pages_remote(current, bprm->mm, pos, 1,
> - 0, 1, &page, NULL) <= 0)
> + FOLL_FORCE, &page, NULL) <= 0)
> return false;
> #else
> page = bprm->page[pos / PAGE_SIZE];
> --
> 2.10.0
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply
* Re: [PATCH 06/10] mm: replace get_user_pages() write/force parameters with gup_flags
From: Jan Kara @ 2016-10-19 7:44 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: linux-mips, linux-fbdev, Jan Kara, kvm, linux-sh, Dave Hansen,
dri-devel, linux-mm, netdev, sparclinux, linux-ia64, linux-s390,
linux-samsung-soc, linux-scsi, linux-rdma, x86, Hugh Dickins,
linux-media, Rik van Riel, intel-gfx, adi-buildroot-devel,
ceph-devel, linux-arm-kernel, linux-cris-kernel, Linus Torvalds,
linuxppc-dev, linux-kernel, linux-security-module, linux-alpha
In-Reply-To: <20161013002020.3062-7-lstoakes@gmail.com>
On Thu 13-10-16 01:20:16, Lorenzo Stoakes wrote:
> This patch removes the write and force parameters from get_user_pages() and
> replaces them with a gup_flags parameter to make the use of FOLL_FORCE explicit
> in callers as use of this flag can result in surprising behaviour (and hence
> bugs) within the mm subsystem.
>
> Signed-off-by: Lorenzo Stoakes <lstoakes@gmail.com>
The patch looks good. You can add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> arch/cris/arch-v32/drivers/cryptocop.c | 4 +---
> arch/ia64/kernel/err_inject.c | 2 +-
> arch/x86/mm/mpx.c | 5 ++---
> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 7 +++++--
> drivers/gpu/drm/radeon/radeon_ttm.c | 3 ++-
> drivers/gpu/drm/via/via_dmablit.c | 4 ++--
> drivers/infiniband/core/umem.c | 6 +++++-
> drivers/infiniband/hw/mthca/mthca_memfree.c | 2 +-
> drivers/infiniband/hw/qib/qib_user_pages.c | 3 ++-
> drivers/infiniband/hw/usnic/usnic_uiom.c | 5 ++++-
> drivers/media/v4l2-core/videobuf-dma-sg.c | 7 +++++--
> drivers/misc/mic/scif/scif_rma.c | 3 +--
> drivers/misc/sgi-gru/grufault.c | 2 +-
> drivers/platform/goldfish/goldfish_pipe.c | 3 ++-
> drivers/rapidio/devices/rio_mport_cdev.c | 3 ++-
> .../vc04_services/interface/vchiq_arm/vchiq_2835_arm.c | 3 +--
> .../vc04_services/interface/vchiq_arm/vchiq_arm.c | 3 +--
> drivers/virt/fsl_hypervisor.c | 4 ++--
> include/linux/mm.h | 2 +-
> mm/gup.c | 12 +++---------
> mm/mempolicy.c | 2 +-
> mm/nommu.c | 18 ++++--------------
> 22 files changed, 49 insertions(+), 54 deletions(-)
>
> diff --git a/arch/cris/arch-v32/drivers/cryptocop.c b/arch/cris/arch-v32/drivers/cryptocop.c
> index b5698c8..099e170 100644
> --- a/arch/cris/arch-v32/drivers/cryptocop.c
> +++ b/arch/cris/arch-v32/drivers/cryptocop.c
> @@ -2722,7 +2722,6 @@ static int cryptocop_ioctl_process(struct inode *inode, struct file *filp, unsig
> err = get_user_pages((unsigned long int)(oper.indata + prev_ix),
> noinpages,
> 0, /* read access only for in data */
> - 0, /* no force */
> inpages,
> NULL);
>
> @@ -2736,8 +2735,7 @@ static int cryptocop_ioctl_process(struct inode *inode, struct file *filp, unsig
> if (oper.do_cipher){
> err = get_user_pages((unsigned long int)oper.cipher_outdata,
> nooutpages,
> - 1, /* write access for out data */
> - 0, /* no force */
> + FOLL_WRITE, /* write access for out data */
> outpages,
> NULL);
> up_read(¤t->mm->mmap_sem);
> diff --git a/arch/ia64/kernel/err_inject.c b/arch/ia64/kernel/err_inject.c
> index 09f8457..5ed0ea9 100644
> --- a/arch/ia64/kernel/err_inject.c
> +++ b/arch/ia64/kernel/err_inject.c
> @@ -142,7 +142,7 @@ store_virtual_to_phys(struct device *dev, struct device_attribute *attr,
> u64 virt_addr=simple_strtoull(buf, NULL, 16);
> int ret;
>
> - ret = get_user_pages(virt_addr, 1, VM_READ, 0, NULL, NULL);
> + ret = get_user_pages(virt_addr, 1, FOLL_WRITE, NULL, NULL);
> if (ret<=0) {
> #ifdef ERR_INJ_DEBUG
> printk("Virtual address %lx is not existing.\n",virt_addr);
> diff --git a/arch/x86/mm/mpx.c b/arch/x86/mm/mpx.c
> index 8047687..e4f8009 100644
> --- a/arch/x86/mm/mpx.c
> +++ b/arch/x86/mm/mpx.c
> @@ -544,10 +544,9 @@ static int mpx_resolve_fault(long __user *addr, int write)
> {
> long gup_ret;
> int nr_pages = 1;
> - int force = 0;
>
> - gup_ret = get_user_pages((unsigned long)addr, nr_pages, write,
> - force, NULL, NULL);
> + gup_ret = get_user_pages((unsigned long)addr, nr_pages,
> + write ? FOLL_WRITE : 0, NULL, NULL);
> /*
> * get_user_pages() returns number of pages gotten.
> * 0 means we failed to fault in and get anything,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index 887483b..dcaf691 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -555,10 +555,13 @@ struct amdgpu_ttm_tt {
> int amdgpu_ttm_tt_get_user_pages(struct ttm_tt *ttm, struct page **pages)
> {
> struct amdgpu_ttm_tt *gtt = (void *)ttm;
> - int write = !(gtt->userflags & AMDGPU_GEM_USERPTR_READONLY);
> + unsigned int flags = 0;
> unsigned pinned = 0;
> int r;
>
> + if (!(gtt->userflags & AMDGPU_GEM_USERPTR_READONLY))
> + flags |= FOLL_WRITE;
> +
> if (gtt->userflags & AMDGPU_GEM_USERPTR_ANONONLY) {
> /* check that we only use anonymous memory
> to prevent problems with writeback */
> @@ -581,7 +584,7 @@ int amdgpu_ttm_tt_get_user_pages(struct ttm_tt *ttm, struct page **pages)
> list_add(&guptask.list, >t->guptasks);
> spin_unlock(>t->guptasklock);
>
> - r = get_user_pages(userptr, num_pages, write, 0, p, NULL);
> + r = get_user_pages(userptr, num_pages, flags, p, NULL);
>
> spin_lock(>t->guptasklock);
> list_del(&guptask.list);
> diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
> index 4552682..3de5e6e 100644
> --- a/drivers/gpu/drm/radeon/radeon_ttm.c
> +++ b/drivers/gpu/drm/radeon/radeon_ttm.c
> @@ -566,7 +566,8 @@ static int radeon_ttm_tt_pin_userptr(struct ttm_tt *ttm)
> uint64_t userptr = gtt->userptr + pinned * PAGE_SIZE;
> struct page **pages = ttm->pages + pinned;
>
> - r = get_user_pages(userptr, num_pages, write, 0, pages, NULL);
> + r = get_user_pages(userptr, num_pages, write ? FOLL_WRITE : 0,
> + pages, NULL);
> if (r < 0)
> goto release_pages;
>
> diff --git a/drivers/gpu/drm/via/via_dmablit.c b/drivers/gpu/drm/via/via_dmablit.c
> index 7e2a12c..1a3ad76 100644
> --- a/drivers/gpu/drm/via/via_dmablit.c
> +++ b/drivers/gpu/drm/via/via_dmablit.c
> @@ -241,8 +241,8 @@ via_lock_all_dma_pages(drm_via_sg_info_t *vsg, drm_via_dmablit_t *xfer)
> down_read(¤t->mm->mmap_sem);
> ret = get_user_pages((unsigned long)xfer->mem_addr,
> vsg->num_pages,
> - (vsg->direction == DMA_FROM_DEVICE),
> - 0, vsg->pages, NULL);
> + (vsg->direction == DMA_FROM_DEVICE) ? FOLL_WRITE : 0,
> + vsg->pages, NULL);
>
> up_read(¤t->mm->mmap_sem);
> if (ret != vsg->num_pages) {
> diff --git a/drivers/infiniband/core/umem.c b/drivers/infiniband/core/umem.c
> index c68746c..224ad27 100644
> --- a/drivers/infiniband/core/umem.c
> +++ b/drivers/infiniband/core/umem.c
> @@ -94,6 +94,7 @@ struct ib_umem *ib_umem_get(struct ib_ucontext *context, unsigned long addr,
> unsigned long dma_attrs = 0;
> struct scatterlist *sg, *sg_list_start;
> int need_release = 0;
> + unsigned int gup_flags = FOLL_WRITE;
>
> if (dmasync)
> dma_attrs |= DMA_ATTR_WRITE_BARRIER;
> @@ -183,6 +184,9 @@ struct ib_umem *ib_umem_get(struct ib_ucontext *context, unsigned long addr,
> if (ret)
> goto out;
>
> + if (!umem->writable)
> + gup_flags |= FOLL_FORCE;
> +
> need_release = 1;
> sg_list_start = umem->sg_head.sgl;
>
> @@ -190,7 +194,7 @@ struct ib_umem *ib_umem_get(struct ib_ucontext *context, unsigned long addr,
> ret = get_user_pages(cur_base,
> min_t(unsigned long, npages,
> PAGE_SIZE / sizeof (struct page *)),
> - 1, !umem->writable, page_list, vma_list);
> + gup_flags, page_list, vma_list);
>
> if (ret < 0)
> goto out;
> diff --git a/drivers/infiniband/hw/mthca/mthca_memfree.c b/drivers/infiniband/hw/mthca/mthca_memfree.c
> index 6c00d04..c6fe89d 100644
> --- a/drivers/infiniband/hw/mthca/mthca_memfree.c
> +++ b/drivers/infiniband/hw/mthca/mthca_memfree.c
> @@ -472,7 +472,7 @@ int mthca_map_user_db(struct mthca_dev *dev, struct mthca_uar *uar,
> goto out;
> }
>
> - ret = get_user_pages(uaddr & PAGE_MASK, 1, 1, 0, pages, NULL);
> + ret = get_user_pages(uaddr & PAGE_MASK, 1, FOLL_WRITE, pages, NULL);
> if (ret < 0)
> goto out;
>
> diff --git a/drivers/infiniband/hw/qib/qib_user_pages.c b/drivers/infiniband/hw/qib/qib_user_pages.c
> index 2d2b94f..75f0862 100644
> --- a/drivers/infiniband/hw/qib/qib_user_pages.c
> +++ b/drivers/infiniband/hw/qib/qib_user_pages.c
> @@ -67,7 +67,8 @@ static int __qib_get_user_pages(unsigned long start_page, size_t num_pages,
>
> for (got = 0; got < num_pages; got += ret) {
> ret = get_user_pages(start_page + got * PAGE_SIZE,
> - num_pages - got, 1, 1,
> + num_pages - got,
> + FOLL_WRITE | FOLL_FORCE,
> p + got, NULL);
> if (ret < 0)
> goto bail_release;
> diff --git a/drivers/infiniband/hw/usnic/usnic_uiom.c b/drivers/infiniband/hw/usnic/usnic_uiom.c
> index a0b6ebe..1ccee6e 100644
> --- a/drivers/infiniband/hw/usnic/usnic_uiom.c
> +++ b/drivers/infiniband/hw/usnic/usnic_uiom.c
> @@ -111,6 +111,7 @@ static int usnic_uiom_get_pages(unsigned long addr, size_t size, int writable,
> int i;
> int flags;
> dma_addr_t pa;
> + unsigned int gup_flags;
>
> if (!can_do_mlock())
> return -EPERM;
> @@ -135,6 +136,8 @@ static int usnic_uiom_get_pages(unsigned long addr, size_t size, int writable,
>
> flags = IOMMU_READ | IOMMU_CACHE;
> flags |= (writable) ? IOMMU_WRITE : 0;
> + gup_flags = FOLL_WRITE;
> + gup_flags |= (writable) ? 0 : FOLL_FORCE;
> cur_base = addr & PAGE_MASK;
> ret = 0;
>
> @@ -142,7 +145,7 @@ static int usnic_uiom_get_pages(unsigned long addr, size_t size, int writable,
> ret = get_user_pages(cur_base,
> min_t(unsigned long, npages,
> PAGE_SIZE / sizeof(struct page *)),
> - 1, !writable, page_list, NULL);
> + gup_flags, page_list, NULL);
>
> if (ret < 0)
> goto out;
> diff --git a/drivers/media/v4l2-core/videobuf-dma-sg.c b/drivers/media/v4l2-core/videobuf-dma-sg.c
> index f300f06..1db0af6 100644
> --- a/drivers/media/v4l2-core/videobuf-dma-sg.c
> +++ b/drivers/media/v4l2-core/videobuf-dma-sg.c
> @@ -156,6 +156,7 @@ static int videobuf_dma_init_user_locked(struct videobuf_dmabuf *dma,
> {
> unsigned long first, last;
> int err, rw = 0;
> + unsigned int flags = FOLL_FORCE;
>
> dma->direction = direction;
> switch (dma->direction) {
> @@ -178,12 +179,14 @@ static int videobuf_dma_init_user_locked(struct videobuf_dmabuf *dma,
> if (NULL == dma->pages)
> return -ENOMEM;
>
> + if (rw == READ)
> + flags |= FOLL_WRITE;
> +
> dprintk(1, "init user [0x%lx+0x%lx => %d pages]\n",
> data, size, dma->nr_pages);
>
> err = get_user_pages(data & PAGE_MASK, dma->nr_pages,
> - rw == READ, 1, /* force */
> - dma->pages, NULL);
> + flags, dma->pages, NULL);
>
> if (err != dma->nr_pages) {
> dma->nr_pages = (err >= 0) ? err : 0;
> diff --git a/drivers/misc/mic/scif/scif_rma.c b/drivers/misc/mic/scif/scif_rma.c
> index e0203b1..f806a44 100644
> --- a/drivers/misc/mic/scif/scif_rma.c
> +++ b/drivers/misc/mic/scif/scif_rma.c
> @@ -1396,8 +1396,7 @@ int __scif_pin_pages(void *addr, size_t len, int *out_prot,
> pinned_pages->nr_pages = get_user_pages(
> (u64)addr,
> nr_pages,
> - !!(prot & SCIF_PROT_WRITE),
> - 0,
> + (prot & SCIF_PROT_WRITE) ? FOLL_WRITE : 0,
> pinned_pages->pages,
> NULL);
> up_write(&mm->mmap_sem);
> diff --git a/drivers/misc/sgi-gru/grufault.c b/drivers/misc/sgi-gru/grufault.c
> index a2d97b9..6fb773d 100644
> --- a/drivers/misc/sgi-gru/grufault.c
> +++ b/drivers/misc/sgi-gru/grufault.c
> @@ -198,7 +198,7 @@ static int non_atomic_pte_lookup(struct vm_area_struct *vma,
> #else
> *pageshift = PAGE_SHIFT;
> #endif
> - if (get_user_pages(vaddr, 1, write, 0, &page, NULL) <= 0)
> + if (get_user_pages(vaddr, 1, write ? FOLL_WRITE : 0, &page, NULL) <= 0)
> return -EFAULT;
> *paddr = page_to_phys(page);
> put_page(page);
> diff --git a/drivers/platform/goldfish/goldfish_pipe.c b/drivers/platform/goldfish/goldfish_pipe.c
> index 07462d7..1aba2c7 100644
> --- a/drivers/platform/goldfish/goldfish_pipe.c
> +++ b/drivers/platform/goldfish/goldfish_pipe.c
> @@ -309,7 +309,8 @@ static ssize_t goldfish_pipe_read_write(struct file *filp, char __user *buffer,
> * much memory to the process.
> */
> down_read(¤t->mm->mmap_sem);
> - ret = get_user_pages(address, 1, !is_write, 0, &page, NULL);
> + ret = get_user_pages(address, 1, is_write ? 0 : FOLL_WRITE,
> + &page, NULL);
> up_read(¤t->mm->mmap_sem);
> if (ret < 0)
> break;
> diff --git a/drivers/rapidio/devices/rio_mport_cdev.c b/drivers/rapidio/devices/rio_mport_cdev.c
> index 436dfe8..9013a58 100644
> --- a/drivers/rapidio/devices/rio_mport_cdev.c
> +++ b/drivers/rapidio/devices/rio_mport_cdev.c
> @@ -892,7 +892,8 @@ rio_dma_transfer(struct file *filp, u32 transfer_mode,
> down_read(¤t->mm->mmap_sem);
> pinned = get_user_pages(
> (unsigned long)xfer->loc_addr & PAGE_MASK,
> - nr_pages, dir == DMA_FROM_DEVICE, 0,
> + nr_pages,
> + dir == DMA_FROM_DEVICE ? FOLL_WRITE : 0,
> page_list, NULL);
> up_read(¤t->mm->mmap_sem);
>
> diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
> index c29040f..1091b9f 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
> @@ -423,8 +423,7 @@ create_pagelist(char __user *buf, size_t count, unsigned short type,
> actual_pages = get_user_pages(task, task->mm,
> (unsigned long)buf & ~(PAGE_SIZE - 1),
> num_pages,
> - (type == PAGELIST_READ) /*Write */ ,
> - 0 /*Force */ ,
> + (type == PAGELIST_READ) ? FOLL_WRITE : 0,
> pages,
> NULL /*vmas */);
> up_read(&task->mm->mmap_sem);
> diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> index e11c0e0..7b6cd4d 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> @@ -1477,8 +1477,7 @@ dump_phys_mem(void *virt_addr, uint32_t num_bytes)
> current->mm, /* mm */
> (unsigned long)virt_addr, /* start */
> num_pages, /* len */
> - 0, /* write */
> - 0, /* force */
> + 0, /* gup_flags */
> pages, /* pages (array of page pointers) */
> NULL); /* vmas */
> up_read(¤t->mm->mmap_sem);
> diff --git a/drivers/virt/fsl_hypervisor.c b/drivers/virt/fsl_hypervisor.c
> index 60bdad3..150ce2a 100644
> --- a/drivers/virt/fsl_hypervisor.c
> +++ b/drivers/virt/fsl_hypervisor.c
> @@ -245,8 +245,8 @@ static long ioctl_memcpy(struct fsl_hv_ioctl_memcpy __user *p)
> /* Get the physical addresses of the source buffer */
> down_read(¤t->mm->mmap_sem);
> num_pinned = get_user_pages(param.local_vaddr - lb_offset,
> - num_pages, (param.source == -1) ? READ : WRITE,
> - 0, pages, NULL);
> + num_pages, (param.source == -1) ? 0 : FOLL_WRITE,
> + pages, NULL);
> up_read(¤t->mm->mmap_sem);
>
> if (num_pinned != num_pages) {
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 5ff084f6..686a477 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -1279,7 +1279,7 @@ long get_user_pages_remote(struct task_struct *tsk, struct mm_struct *mm,
> int write, int force, struct page **pages,
> struct vm_area_struct **vmas);
> long get_user_pages(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);
> long get_user_pages_locked(unsigned long start, unsigned long nr_pages,
> unsigned int gup_flags, struct page **pages, int *locked);
> diff --git a/mm/gup.c b/mm/gup.c
> index 7a0d033..dc91303 100644
> --- a/mm/gup.c
> +++ b/mm/gup.c
> @@ -977,18 +977,12 @@ EXPORT_SYMBOL(get_user_pages_remote);
> * obviously don't pass FOLL_REMOTE in here.
> */
> long get_user_pages(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;
> -
> - if (write)
> - flags |= FOLL_WRITE;
> - if (force)
> - flags |= FOLL_FORCE;
> -
> return __get_user_pages_locked(current, current->mm, start, nr_pages,
> - pages, vmas, NULL, false, flags);
> + pages, vmas, NULL, false,
> + gup_flags | FOLL_TOUCH);
> }
> EXPORT_SYMBOL(get_user_pages);
>
> diff --git a/mm/mempolicy.c b/mm/mempolicy.c
> index ad1c96a..0b859af 100644
> --- a/mm/mempolicy.c
> +++ b/mm/mempolicy.c
> @@ -850,7 +850,7 @@ static int lookup_node(unsigned long addr)
> struct page *p;
> int err;
>
> - err = get_user_pages(addr & PAGE_MASK, 1, 0, 0, &p, NULL);
> + err = get_user_pages(addr & PAGE_MASK, 1, 0, &p, NULL);
> if (err >= 0) {
> err = page_to_nid(p);
> put_page(p);
> diff --git a/mm/nommu.c b/mm/nommu.c
> index 842cfdd..70cb844 100644
> --- a/mm/nommu.c
> +++ b/mm/nommu.c
> @@ -160,18 +160,11 @@ long __get_user_pages(struct task_struct *tsk, struct mm_struct *mm,
> * - don't permit access to VMAs that don't support it, such as I/O mappings
> */
> long get_user_pages(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)
> {
> - int flags = 0;
> -
> - if (write)
> - flags |= FOLL_WRITE;
> - if (force)
> - flags |= FOLL_FORCE;
> -
> - return __get_user_pages(current, current->mm, start, nr_pages, flags,
> - pages, vmas, NULL);
> + return __get_user_pages(current, current->mm, start, nr_pages,
> + gup_flags, pages, vmas, NULL);
> }
> EXPORT_SYMBOL(get_user_pages);
>
> @@ -179,10 +172,7 @@ long get_user_pages_locked(unsigned long start, unsigned long nr_pages,
> unsigned int gup_flags, struct page **pages,
> int *locked)
> {
> - int write = gup_flags & FOLL_WRITE;
> - int force = gup_flags & FOLL_FORCE;
> -
> - return get_user_pages(start, nr_pages, write, force, pages, NULL);
> + return get_user_pages(start, nr_pages, gup_flags, pages, NULL);
> }
> EXPORT_SYMBOL(get_user_pages_locked);
>
> --
> 2.10.0
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply
* Re: [PATCH 05/10] mm: replace get_vaddr_frames() write/force parameters with gup_flags
From: Jan Kara @ 2016-10-19 7:34 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: linux-mips, linux-fbdev, Jan Kara, kvm, linux-sh, Dave Hansen,
dri-devel, linux-mm, netdev, sparclinux, linux-ia64, linux-s390,
linux-samsung-soc, linux-scsi, linux-rdma, x86, Hugh Dickins,
linux-media, Rik van Riel, intel-gfx, adi-buildroot-devel,
ceph-devel, linux-arm-kernel, linux-cris-kernel, Linus Torvalds,
linuxppc-dev, linux-kernel, linux-security-module, linux-alpha
In-Reply-To: <20161013002020.3062-6-lstoakes@gmail.com>
On Thu 13-10-16 01:20:15, Lorenzo Stoakes wrote:
> This patch removes the write and force parameters from get_vaddr_frames() and
> replaces them with a gup_flags parameter to make the use of FOLL_FORCE explicit
> in callers as use of this flag can result in surprising behaviour (and hence
> bugs) within the mm subsystem.
>
> Signed-off-by: Lorenzo Stoakes <lstoakes@gmail.com>
Looks good. You can add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> drivers/gpu/drm/exynos/exynos_drm_g2d.c | 3 ++-
> drivers/media/platform/omap/omap_vout.c | 2 +-
> drivers/media/v4l2-core/videobuf2-memops.c | 6 +++++-
> include/linux/mm.h | 2 +-
> mm/frame_vector.c | 13 ++-----------
> 5 files changed, 11 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_g2d.c b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
> index aa92dec..fbd13fa 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_g2d.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
> @@ -488,7 +488,8 @@ static dma_addr_t *g2d_userptr_get_dma_addr(struct drm_device *drm_dev,
> goto err_free;
> }
>
> - ret = get_vaddr_frames(start, npages, true, true, g2d_userptr->vec);
> + ret = get_vaddr_frames(start, npages, FOLL_FORCE | FOLL_WRITE,
> + g2d_userptr->vec);
> if (ret != npages) {
> DRM_ERROR("failed to get user pages from userptr.\n");
> if (ret < 0)
> diff --git a/drivers/media/platform/omap/omap_vout.c b/drivers/media/platform/omap/omap_vout.c
> index e668dde..a31b95c 100644
> --- a/drivers/media/platform/omap/omap_vout.c
> +++ b/drivers/media/platform/omap/omap_vout.c
> @@ -214,7 +214,7 @@ static int omap_vout_get_userptr(struct videobuf_buffer *vb, u32 virtp,
> if (!vec)
> return -ENOMEM;
>
> - ret = get_vaddr_frames(virtp, 1, true, false, vec);
> + ret = get_vaddr_frames(virtp, 1, FOLL_WRITE, vec);
> if (ret != 1) {
> frame_vector_destroy(vec);
> return -EINVAL;
> diff --git a/drivers/media/v4l2-core/videobuf2-memops.c b/drivers/media/v4l2-core/videobuf2-memops.c
> index 3c3b517..1cd322e 100644
> --- a/drivers/media/v4l2-core/videobuf2-memops.c
> +++ b/drivers/media/v4l2-core/videobuf2-memops.c
> @@ -42,6 +42,10 @@ struct frame_vector *vb2_create_framevec(unsigned long start,
> unsigned long first, last;
> unsigned long nr;
> struct frame_vector *vec;
> + unsigned int flags = FOLL_FORCE;
> +
> + if (write)
> + flags |= FOLL_WRITE;
>
> first = start >> PAGE_SHIFT;
> last = (start + length - 1) >> PAGE_SHIFT;
> @@ -49,7 +53,7 @@ struct frame_vector *vb2_create_framevec(unsigned long start,
> vec = frame_vector_create(nr);
> if (!vec)
> return ERR_PTR(-ENOMEM);
> - ret = get_vaddr_frames(start & PAGE_MASK, nr, write, true, vec);
> + ret = get_vaddr_frames(start & PAGE_MASK, nr, flags, vec);
> if (ret < 0)
> goto out_destroy;
> /* We accept only complete set of PFNs */
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 27ab538..5ff084f6 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -1305,7 +1305,7 @@ struct frame_vector {
> struct frame_vector *frame_vector_create(unsigned int nr_frames);
> void frame_vector_destroy(struct frame_vector *vec);
> int get_vaddr_frames(unsigned long start, unsigned int nr_pfns,
> - bool write, bool force, struct frame_vector *vec);
> + unsigned int gup_flags, struct frame_vector *vec);
> void put_vaddr_frames(struct frame_vector *vec);
> int frame_vector_to_pages(struct frame_vector *vec);
> void frame_vector_to_pfns(struct frame_vector *vec);
> diff --git a/mm/frame_vector.c b/mm/frame_vector.c
> index 81b6749..db77dcb 100644
> --- a/mm/frame_vector.c
> +++ b/mm/frame_vector.c
> @@ -11,10 +11,7 @@
> * get_vaddr_frames() - map virtual addresses to pfns
> * @start: starting user address
> * @nr_frames: number of pages / pfns from start to map
> - * @write: whether pages will be written to by the caller
> - * @force: whether to force write access even if user mapping is
> - * readonly. See description of the same argument of
> - get_user_pages().
> + * @gup_flags: flags modifying lookup behaviour
> * @vec: structure which receives pages / pfns of the addresses mapped.
> * It should have space for at least nr_frames entries.
> *
> @@ -34,23 +31,17 @@
> * This function takes care of grabbing mmap_sem as necessary.
> */
> int get_vaddr_frames(unsigned long start, unsigned int nr_frames,
> - bool write, bool force, struct frame_vector *vec)
> + unsigned int gup_flags, struct frame_vector *vec)
> {
> struct mm_struct *mm = current->mm;
> struct vm_area_struct *vma;
> int ret = 0;
> int err;
> int locked;
> - unsigned int gup_flags = 0;
>
> if (nr_frames == 0)
> return 0;
>
> - if (write)
> - gup_flags |= FOLL_WRITE;
> - if (force)
> - gup_flags |= FOLL_FORCE;
> -
> if (WARN_ON_ONCE(nr_frames > vec->nr_allocated))
> nr_frames = vec->nr_allocated;
>
> --
> 2.10.0
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply
* Re: [PATCH 04/10] mm: replace get_user_pages_locked() write/force parameters with gup_flags
From: Jan Kara @ 2016-10-19 7:33 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: linux-mm, Linus Torvalds, Jan Kara, Hugh Dickins, Dave Hansen,
Rik van Riel, Mel Gorman, Andrew Morton, adi-buildroot-devel,
ceph-devel, dri-devel, intel-gfx, kvm, linux-alpha,
linux-arm-kernel, linux-cris-kernel, linux-fbdev, linux-fsdevel,
linux-ia64, linux-kernel, linux-media, linux-mips, linux-rdma,
linux-s390, linux-samsung-soc, linux-scsi, linux-security-module,
linux-sh@
In-Reply-To: <20161013002020.3062-5-lstoakes@gmail.com>
On Thu 13-10-16 01:20:14, Lorenzo Stoakes wrote:
> This patch removes the write and force parameters from get_user_pages_locked()
> and replaces them with a gup_flags parameter to make the use of FOLL_FORCE
> explicit in callers as use of this flag can result in surprising behaviour (and
> hence bugs) within the mm subsystem.
>
> Signed-off-by: Lorenzo Stoakes <lstoakes@gmail.com>
After our discussion the patch looks good to me. You can add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH 04/10] mm: replace get_user_pages_locked() write/force parameters with gup_flags
From: Jan Kara @ 2016-10-19 7:32 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: Jan Kara, linux-mm, Linus Torvalds, Hugh Dickins, Dave Hansen,
Rik van Riel, Mel Gorman, Andrew Morton, adi-buildroot-devel,
ceph-devel, dri-devel, intel-gfx, kvm, linux-alpha,
linux-arm-kernel, linux-cris-kernel, linux-fbdev, linux-fsdevel,
linux-ia64, linux-kernel, linux-media, linux-mips, linux-rdma,
linux-s390, linux-samsung-soc, linux-scsi, linux-security-module,
linux-sh
In-Reply-To: <20161018135609.GA30025@lucifer>
On Tue 18-10-16 14:56:09, Lorenzo Stoakes wrote:
> On Tue, Oct 18, 2016 at 02:54:25PM +0200, Jan Kara wrote:
> > > @@ -1282,7 +1282,7 @@ long get_user_pages(unsigned long start, unsigned long nr_pages,
> > > int write, int force, struct page **pages,
> > > struct vm_area_struct **vmas);
> > > long get_user_pages_locked(unsigned long start, unsigned long nr_pages,
> > > - int write, int force, struct page **pages, int *locked);
> > > + unsigned int gup_flags, struct page **pages, int *locked);
> >
> > Hum, the prototype is inconsistent with e.g. __get_user_pages_unlocked()
> > where gup_flags come after **pages argument. Actually it makes more sense
> > to have it before **pages so that input arguments come first and output
> > arguments second but I don't care that much. But it definitely should be
> > consistent...
>
> It was difficult to decide quite how to arrange parameters as there was
> inconsitency with regards to parameter ordering already - for example
> __get_user_pages() places its flags argument before pages whereas, as you note,
> __get_user_pages_unlocked() puts them afterwards.
>
> I ended up compromising by trying to match the existing ordering of the function
> as much as I could by replacing write, force pairs with gup_flags in the same
> location (with the exception of get_user_pages_unlocked() which I felt should
> match __get_user_pages_unlocked() in signature) or if there was already a
> gup_flags parameter as in the case of __get_user_pages_unlocked() I simply
> removed the write, force pair and left the flags as the last parameter.
>
> I am happy to rearrange parameters as needed, however I am not sure if it'd be
> worthwhile for me to do so (I am keen to try to avoid adding too much noise here
> :)
>
> If we were to rearrange parameters for consistency I'd suggest adjusting
> __get_user_pages_unlocked() to put gup_flags before pages and do the same with
> get_user_pages_unlocked(), let me know what you think.
Yeah, ok. If the inconsistency is already there, just leave it for now.
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: iscsi_trx going into D state
From: Nicholas A. Bellinger @ 2016-10-19 6:25 UTC (permalink / raw)
To: Robert LeBlanc
Cc: Zhu Lingshan, linux-rdma-u79uwXL29TY76Z2rM5mHXA,
linux-scsi-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <CAANLjFqXt5r=c9F75vjeK=_zLa8zCS1priLuZo=A1ZSHKZ=1Bw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Tue, 2016-10-18 at 16:13 -0600, Robert LeBlanc wrote:
> Nicholas,
>
> We patched this in and for the first time in many reboots, we didn't
> have iSCSI going straight into D state. We have had to work on a
> couple of other things, so we don't know if this is just a coincidence
> or not. We will reboot back into the old kernel and back a few times
> and do some more testing, but so far it has given us a little bit of
> hope that we may be narrowing down on the root cause. We will report
> back once we have some more info.
>
> Thank you,
> Robert LeBlanc
> ----------------
> Robert LeBlanc
> PGP Fingerprint 79A2 9CA4 6CC4 45DD A904 C70E E654 3BB2 FA62 B9F1
>
Hello Robert,
Thanks for the update. Btw, if the original /var/log/messages
reproduction logs for iser-target are still handy, I'm happy to have
a look to confirm. Feel free to send them along here, or off-list if
necessary.
For further reference, you can also enable Linux kernel crash dump
(LKCD) at build time using CONFIG_CRASH_DUMP=y, so it's possible to
manually generate a vmcore dumpfile of the running system via 'echo c
> /proc/sysrq-trigger', once the bug occurs.
http://cateee.net/lkddb/web-lkddb/CRASH_DUMP.html
Note in order to fully debug within this in a LKCD environment, it
requires the vmcore dump from /var/crash/, unstripped vmlinux,
target_core_mod, iscsi_target_mod and ib_isert modules matching the
specific particular x86_64 build setup of the running system.
Also, can you share a bit more about the details of your particular
iser-target + backend setup..?
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: some test question//Re: [For help] configure crossbar build tool in CMakelist.txt
From: Jason Gunthorpe @ 2016-10-19 4:11 UTC (permalink / raw)
To: oulijun; +Cc: linux-rdma, Linuxarm
In-Reply-To: <5806E02E.7030400-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
On Wed, Oct 19, 2016 at 10:53:34AM +0800, oulijun wrote:
> when use the cmd: CC=aarch64-linux-gnu-gcc cmake -GNinja -DENABLE_RESOLVE_NEIGH=0 ..
>
> the provider/hns/CMakelist.txt can be called correctly and generate
> the library libhns-rdmav2.so(not be tested), but it will generate
> warning with mlx5
>
> the print log as follows:
>
> Building C object providers/mlx5/CMakeFiles/mlx5-rdmav2.dir/cq.c.o
> ../providers/mlx5/cq.c:414:13: warning: function declaration isn't a prototype [-Wstrict-prototypes]
> static void mlx5_stall_poll_cq()
Thanks, you may be the first person to compile on ARM64, so I'm glad
to hear that is your only warning, I will get it fixed.
> In summary, I have two questions:
>
> 1. if don't use libnl(use -DENABLE_RESOLVE_NEIGH=0), it will not
> confluent the function of library?
Correct, RoCEE will not work entirely as expected if you try and run
the libibverbs from such a build.
> 2. the rdma-core.git use the default build method by used build.sh,
> and i need modify it by added the building item or modify the
> README.md for hns, is the right approach?
No.
If you native compile on an actual ARM64 machine you will not have
problems, simply follow the README.md directions to install the
required packages on the actual machine.
Your problem is cross compiling. Cross compiling is hard. You need to
make libnl3 available in your cross compiler's 'sysroot'. I can't
really give you generic good directions for that..
One option is to cross compile libnl and 'cross-install' it into the
compiler tree.
Another option is to download the libnl3 ARM64 packages from Debian and
then unpack and rename directories 'just so' to make it work.
But if you don't intend to run the build binaries (eg when you go to
test, you will build natively on ARM64), then don't worry about it.
Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 0/3] iopmem : A block device for PCIe memory
From: Dan Williams @ 2016-10-19 3:51 UTC (permalink / raw)
To: Stephen Bates
Cc: linux-kernel@vger.kernel.org, linux-nvdimm@lists.01.org,
linux-rdma, linux-block, Linux MM, Ross Zwisler, Matthew Wilcox,
jgunthorpe, haggaie, Christoph Hellwig, Jens Axboe,
Jonathan Corbet, jim.macdonald, sbates, Logan Gunthorpe,
David Woodhouse, Raj, Ashok
In-Reply-To: <1476826937-20665-1-git-send-email-sbates@raithlin.com>
[ adding Ashok and David for potential iommu comments ]
On Tue, Oct 18, 2016 at 2:42 PM, Stephen Bates <sbates@raithlin.com> wrote:
> This patch follows from an RFC we did earlier this year [1]. This
> patchset applies cleanly to v4.9-rc1.
>
> Updates since RFC
> -----------------
> Rebased.
> Included the iopmem driver in the submission.
>
> History
> -------
>
> There have been several attempts to upstream patchsets that enable
> DMAs between PCIe peers. These include Peer-Direct [2] and DMA-Buf
> style patches [3]. None have been successful to date. Haggai Eran
> gives a nice overview of the prior art in this space in his cover
> letter [3].
>
> Motivation and Use Cases
> ------------------------
>
> PCIe IO devices are getting faster. It is not uncommon now to find PCIe
> network and storage devices that can generate and consume several GB/s.
> Almost always these devices have either a high performance DMA engine, a
> number of exposed PCIe BARs or both.
>
> Until this patch, any high-performance transfer of information between
> two PICe devices has required the use of a staging buffer in system
> memory. With this patch the bandwidth to system memory is not compromised
> when high-throughput transfers occurs between PCIe devices. This means
> that more system memory bandwidth is available to the CPU cores for data
> processing and manipulation. In addition, in systems where the two PCIe
> devices reside behind a PCIe switch the datapath avoids the CPU
> entirely.
I agree with the motivation and the need for a solution, but I have
some questions about this implementation.
>
> Consumers
> ---------
>
> We provide a PCIe device driver in an accompanying patch that can be
> used to map any PCIe BAR into a DAX capable block device. For
> non-persistent BARs this simply serves as an alternative to using
> system memory bounce buffers. For persistent BARs this can serve as an
> additional storage device in the system.
Why block devices? I wonder if iopmem was initially designed back
when we were considering enabling DAX for raw block devices. However,
that support has since been ripped out / abandoned. You currently
need a filesystem on top of a block-device to get DAX operation.
Putting xfs or ext4 on top of PCI-E memory mapped range seems awkward
if all you want is a way to map the bar for another PCI-E device in
the topology.
If you're only using the block-device as a entry-point to create
dax-mappings then a device-dax (drivers/dax/) character-device might
be a better fit.
>
> Testing and Performance
> -----------------------
>
> We have done a moderate about of testing of this patch on a QEMU
> environment and on real hardware. On real hardware we have observed
> peer-to-peer writes of up to 4GB/s and reads of up to 1.2 GB/s. In
> both cases these numbers are limitations of our consumer hardware. In
> addition, we have observed that the CPU DRAM bandwidth is not impacted
> when using IOPMEM which is not the case when a traditional path
> through system memory is taken.
>
> For more information on the testing and performance results see the
> GitHub site [4].
>
> Known Issues
> ------------
>
> 1. Address Translation. Suggestions have been made that in certain
> architectures and topologies the dma_addr_t passed to the DMA master
> in a peer-2-peer transfer will not correctly route to the IO memory
> intended. However in our testing to date we have not seen this to be
> an issue, even in systems with IOMMUs and PCIe switches. It is our
> understanding that an IOMMU only maps system memory and would not
> interfere with device memory regions. (It certainly has no opportunity
> to do so if the transfer gets routed through a switch).
>
There may still be platforms where peer-to-peer cycles are routed up
through the root bridge and then back down to target device, but we
can address that when / if it happens. I wonder if we could (ab)use a
software-defined 'pasid' as the requester id for a peer-to-peer
mapping that needs address translation.
> 2. Memory Segment Spacing. This patch has the same limitations that
> ZONE_DEVICE does in that memory regions must be spaces at least
> SECTION_SIZE bytes part. On x86 this is 128MB and there are cases where
> BARs can be placed closer together than this. Thus ZONE_DEVICE would not
> be usable on neighboring BARs. For our purposes, this is not an issue as
> we'd only be looking at enabling a single BAR in a given PCIe device.
> More exotic use cases may have problems with this.
I'm working on patches for 4.10 to allow mixing multiple
devm_memremap_pages() allocations within the same physical section.
Hopefully this won't be a problem going forward.
> 3. Coherency Issues. When IOMEM is written from both the CPU and a PCIe
> peer there is potential for coherency issues and for writes to occur out
> of order. This is something that users of this feature need to be
> cognizant of. Though really, this isn't much different than the
> existing situation with things like RDMA: if userspace sets up an MR
> for remote use, they need to be careful about using that memory region
> themselves.
We might be able to mitigate this by indicating that the mapping is
busy for device-to-device transfers. But you're right we could wait
to see how much of a problem this is in practice.
>
> 4. Architecture. Currently this patch is applicable only to x86_64
> architectures. The same is true for much of the code pertaining to
> PMEM and ZONE_DEVICE. It is hoped that the work will be extended to other
> ARCH over time.
>
> References
> ----------
> [1] https://patchwork.kernel.org/patch/8583221/
> [2] http://comments.gmane.org/gmane.linux.drivers.rdma/21849
> [3] http://www.spinics.net/lists/linux-rdma/msg38748.html
> [4] https://github.com/sbates130272/zone-device
>
> Logan Gunthorpe (1):
> memremap.c : Add support for ZONE_DEVICE IO memory with struct pages.
I haven't yet grokked the motivation for this, but I'll go comment on
that separately.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* some test question//Re: [For help] configure crossbar build tool in CMakelist.txt
From: oulijun @ 2016-10-19 2:53 UTC (permalink / raw)
To: Jason Gunthorpe; +Cc: linux-rdma, Linuxarm
In-Reply-To: <20161018155004.GA24189-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
Hi, Jason
Indeed, you are right! when use the cmd: CC= aarch64-linux-gnu-gcc cmake -GNinja ..
the libnl doesn't work
the print log as follows:
ubuntu@62fe1357a077:~/rdma-core/build$ ninja
[27/164] Linking C executable bin/iwpmd
FAILED: : && /opt/gcc-linaro-aarch64-linux-gnu-4.9-2014.09_linux/bin/aarch64-linux-gnu-gcc -std=gnu99 -Wall -Wextra -Wno-sign-compare -Wno-unused-parameter -Wmissing-prototypes -Wmissing-declarations -Wwrite-strings -Wformat=2 -Wshadow -Wno-missing-field-initializers -Wstrict-prototypes -Wold-style-definition -O2 -g -Wl,--as-needed -Wl,--no-undefined iwpmd/CMakeFiles/iwpmd.dir/iwarp_pm_common.c.o iwpmd/CMakeFiles/iwpmd.dir/iwarp_pm_helper.c.o iwpmd/CMakeFiles/iwpmd.dir/iwarp_pm_server.c.o -o bin/iwpmd ccan/libccan.a -lnl-route-3 -lnl-3 -lpthread && :
/opt/gcc-linaro-aarch64-linux-gnu-4.9-2014.09_linux/bin/../lib/gcc/aarch64-linux-gnu/4.9.2/../../../../aarch64-linux-gnu/bin/ld: cannot find -lnl-route-3
/opt/gcc-linaro-aarch64-linux-gnu-4.9-2014.09_linux/bin/../lib/gcc/aarch64-linux-gnu/4.9.2/../../../../aarch64-linux-gnu/bin/ld: cannot find -lnl-3
collect2: error: ld returned 1 exit status
[27/164] Building C object libibverbs/CMakeFiles/ibverbs.dir/cmd.c.o
ninja: build stopped: subcommand failed.
when use the cmd: CC=aarch64-linux-gnu-gcc cmake -GNinja -DENABLE_RESOLVE_NEIGH=0 ..
the provider/hns/CMakelist.txt can be called correctly and generate the library libhns-rdmav2.so(not be tested), but it will generate warning with mlx5
the print log as follows:
Building C object providers/mlx5/CMakeFiles/mlx5-rdmav2.dir/cq.c.o
../providers/mlx5/cq.c:414:13: warning: function declaration isn't a prototype [-Wstrict-prototypes]
static void mlx5_stall_poll_cq()
^
../providers/mlx5/cq.c: In function 'mlx5_stall_poll_cq':
../providers/mlx5/cq.c:414:13: warning: old-style function definition [-Wold-style-definition]
In summary, I have two questions:
1. if don't use libnl(use -DENABLE_RESOLVE_NEIGH=0), it will not confluent the function of library?
2. the rdma-core.git use the default build method by used build.sh, and i need modify it by added the building item or modify the README.md for hns, is the right approach?
Thanks
在 2016/10/18 23:50, Jason Gunthorpe 写道:
> On Tue, Oct 18, 2016 at 06:55:08PM +0800, oulijun wrote:
>
>> if I use crossbar build tool aarch64-linux-gnu-gcc for building
>> the directory(provider/hns), what i should do it ?
>
> You cannot cross compile only a part of the project, you must cross
> compile everything.
>
> $ mkdir build; cd build
> $ CC=aarch64-linux-gnu-gcc cmake -GNinja
> $ ninja
>
> I haven't extensively tested cross compiling, you may run into
> problems, in particular I know pkgconfig for libnl doesn't work
> reliably when cross compiling.
>
> You may need to build with -DENABLE_RESOLVE_NEIGH=0 if you don't have
> a cross compiled libnl3 available.
>
>> My modification currently according to the others as fllows:
>>
>> in the file : provider/hns/CMakelist.txt
>>
>> set(CMAKE_C_COMPILER /opt/gcc-linaro-aarch64-linux-gnu-4.9-2014.09_linux/bin/aarch64-linux-gnu-gcc)
>
> This is not the right approach, the hns provider CMakelist should
> detect the compiler is not ARM64 and just do nothing.
>
> Perhaps something like this:
>
> CHECK_C_SOURCE_COMPILES("
> #ifndef __ARM64__
> #error Failed
> #endif
> int main(int argc,const char *argv[]) { return 1; }"
> HAVE_ARCH_ARM64)
>
> if (HAVE_ARCH_ARM64)
> [..]
> endif()
>
> Jason
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
> .
>
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH net-next 6/6] net: use core MTU range checking in misc drivers
From: Jarod Wilson @ 2016-10-19 2:33 UTC (permalink / raw)
To: linux-kernel
Cc: Jarod Wilson, netdev, Stefan Richter, Faisal Latif, linux-rdma,
Cliff Whickman, Robin Holt, Jes Sorensen, Marek Lindner,
Simon Wunderlich, Antonio Quartulli
In-Reply-To: <20161019023333.15760-1-jarod@redhat.com>
firewire-net:
- set min/max_mtu
- remove fwnet_change_mtu
nes:
- set max_mtu
- clean up nes_netdev_change_mtu
xpnet:
- set min/max_mtu
- remove xpnet_dev_change_mtu
hippi:
- set min/max_mtu
- remove hippi_change_mtu
batman-adv:
- set max_mtu
- remove batadv_interface_change_mtu
- initialization is a little async, not 100% certain that max_mtu is set
in the optimal place, don't have hardware to test with
rionet:
- set min/max_mtu
- remove rionet_change_mtu
slip:
- set min/max_mtu
- streamline sl_change_mtu
CC: netdev@vger.kernel.org
CC: Stefan Richter <stefanr@s5r6.in-berlin.de>
CC: Faisal Latif <faisal.latif@intel.com>
CC: linux-rdma@vger.kernel.org
CC: Cliff Whickman <cpw@sgi.com>
CC: Robin Holt <robinmholt@gmail.com>
CC: Jes Sorensen <jes@trained-monkey.org>
CC: Marek Lindner <mareklindner@neomailbox.ch>
CC: Simon Wunderlich <sw@simonwunderlich.de>
CC: Antonio Quartulli <a@unstable.cc>
Signed-off-by: Jarod Wilson <jarod@redhat.com>
---
drivers/firewire/net.c | 12 ++----------
drivers/infiniband/hw/nes/nes.c | 1 -
drivers/infiniband/hw/nes/nes.h | 4 ++--
drivers/infiniband/hw/nes/nes_nic.c | 10 +++-------
drivers/misc/sgi-xp/xpnet.c | 21 ++++-----------------
drivers/net/hippi/rrunner.c | 1 -
drivers/net/rionet.c | 15 +++------------
drivers/net/slip/slip.c | 11 +++++------
include/linux/hippidevice.h | 1 -
net/802/hippi.c | 14 ++------------
net/batman-adv/soft-interface.c | 13 +------------
11 files changed, 22 insertions(+), 81 deletions(-)
diff --git a/drivers/firewire/net.c b/drivers/firewire/net.c
index 309311b..b5f125c 100644
--- a/drivers/firewire/net.c
+++ b/drivers/firewire/net.c
@@ -1349,15 +1349,6 @@ static netdev_tx_t fwnet_tx(struct sk_buff *skb, struct net_device *net)
return NETDEV_TX_OK;
}
-static int fwnet_change_mtu(struct net_device *net, int new_mtu)
-{
- if (new_mtu < 68)
- return -EINVAL;
-
- net->mtu = new_mtu;
- return 0;
-}
-
static const struct ethtool_ops fwnet_ethtool_ops = {
.get_link = ethtool_op_get_link,
};
@@ -1366,7 +1357,6 @@ static const struct net_device_ops fwnet_netdev_ops = {
.ndo_open = fwnet_open,
.ndo_stop = fwnet_stop,
.ndo_start_xmit = fwnet_tx,
- .ndo_change_mtu = fwnet_change_mtu,
};
static void fwnet_init_dev(struct net_device *net)
@@ -1481,6 +1471,8 @@ static int fwnet_probe(struct fw_unit *unit,
max_mtu = (1 << (card->max_receive + 1))
- sizeof(struct rfc2734_header) - IEEE1394_GASP_HDR_SIZE;
net->mtu = min(1500U, max_mtu);
+ net->min_mtu = ETH_MIN_MTU;
+ net->max_mtu = net->mtu;
/* Set our hardware address while we're at it */
ha = (union fwnet_hwaddr *)net->dev_addr;
diff --git a/drivers/infiniband/hw/nes/nes.c b/drivers/infiniband/hw/nes/nes.c
index 35cbb17..2baa45a 100644
--- a/drivers/infiniband/hw/nes/nes.c
+++ b/drivers/infiniband/hw/nes/nes.c
@@ -65,7 +65,6 @@ MODULE_DESCRIPTION("NetEffect RNIC Low-level iWARP Driver");
MODULE_LICENSE("Dual BSD/GPL");
MODULE_VERSION(DRV_VERSION);
-int max_mtu = 9000;
int interrupt_mod_interval = 0;
/* Interoperability */
diff --git a/drivers/infiniband/hw/nes/nes.h b/drivers/infiniband/hw/nes/nes.h
index e7430c9..85acd08 100644
--- a/drivers/infiniband/hw/nes/nes.h
+++ b/drivers/infiniband/hw/nes/nes.h
@@ -83,6 +83,8 @@
#define NES_FIRST_QPN 64
#define NES_SW_CONTEXT_ALIGN 1024
+#define NES_MAX_MTU 9000
+
#define NES_NIC_MAX_NICS 16
#define NES_MAX_ARP_TABLE_SIZE 4096
@@ -169,8 +171,6 @@ do { \
#include "nes_cm.h"
#include "nes_mgt.h"
-extern int max_mtu;
-#define max_frame_len (max_mtu+ETH_HLEN)
extern int interrupt_mod_interval;
extern int nes_if_count;
extern int mpa_version;
diff --git a/drivers/infiniband/hw/nes/nes_nic.c b/drivers/infiniband/hw/nes/nes_nic.c
index 2b27d13..7f8597d 100644
--- a/drivers/infiniband/hw/nes/nes_nic.c
+++ b/drivers/infiniband/hw/nes/nes_nic.c
@@ -981,20 +981,16 @@ static int nes_netdev_change_mtu(struct net_device *netdev, int new_mtu)
{
struct nes_vnic *nesvnic = netdev_priv(netdev);
struct nes_device *nesdev = nesvnic->nesdev;
- int ret = 0;
u8 jumbomode = 0;
u32 nic_active;
u32 nic_active_bit;
u32 uc_all_active;
u32 mc_all_active;
- if ((new_mtu < ETH_ZLEN) || (new_mtu > max_mtu))
- return -EINVAL;
-
netdev->mtu = new_mtu;
nesvnic->max_frame_size = new_mtu + VLAN_ETH_HLEN;
- if (netdev->mtu > 1500) {
+ if (netdev->mtu > ETH_DATA_LEN) {
jumbomode=1;
}
nes_nic_init_timer_defaults(nesdev, jumbomode);
@@ -1020,7 +1016,7 @@ static int nes_netdev_change_mtu(struct net_device *netdev, int new_mtu)
nes_write_indexed(nesdev, NES_IDX_NIC_UNICAST_ALL, nic_active);
}
- return ret;
+ return 0;
}
@@ -1658,7 +1654,7 @@ struct net_device *nes_netdev_init(struct nes_device *nesdev,
netdev->watchdog_timeo = NES_TX_TIMEOUT;
netdev->irq = nesdev->pcidev->irq;
- netdev->mtu = ETH_DATA_LEN;
+ netdev->max_mtu = NES_MAX_MTU;
netdev->hard_header_len = ETH_HLEN;
netdev->addr_len = ETH_ALEN;
netdev->type = ARPHRD_ETHER;
diff --git a/drivers/misc/sgi-xp/xpnet.c b/drivers/misc/sgi-xp/xpnet.c
index 557f978..0c26eaf 100644
--- a/drivers/misc/sgi-xp/xpnet.c
+++ b/drivers/misc/sgi-xp/xpnet.c
@@ -118,6 +118,8 @@ static DEFINE_SPINLOCK(xpnet_broadcast_lock);
* now, the default is 64KB.
*/
#define XPNET_MAX_MTU (0x800000UL - L1_CACHE_BYTES)
+/* 68 comes from min TCP+IP+MAC header */
+#define XPNET_MIN_MTU 68
/* 32KB has been determined to be the ideal */
#define XPNET_DEF_MTU (0x8000UL)
@@ -330,22 +332,6 @@ xpnet_dev_stop(struct net_device *dev)
return 0;
}
-static int
-xpnet_dev_change_mtu(struct net_device *dev, int new_mtu)
-{
- /* 68 comes from min TCP+IP+MAC header */
- if ((new_mtu < 68) || (new_mtu > XPNET_MAX_MTU)) {
- dev_err(xpnet, "ifconfig %s mtu %d failed; value must be "
- "between 68 and %ld\n", dev->name, new_mtu,
- XPNET_MAX_MTU);
- return -EINVAL;
- }
-
- dev->mtu = new_mtu;
- dev_dbg(xpnet, "ifconfig %s mtu set to %d\n", dev->name, new_mtu);
- return 0;
-}
-
/*
* Notification that the other end has received the message and
* DMA'd the skb information. At this point, they are done with
@@ -519,7 +505,6 @@ static const struct net_device_ops xpnet_netdev_ops = {
.ndo_open = xpnet_dev_open,
.ndo_stop = xpnet_dev_stop,
.ndo_start_xmit = xpnet_dev_hard_start_xmit,
- .ndo_change_mtu = xpnet_dev_change_mtu,
.ndo_tx_timeout = xpnet_dev_tx_timeout,
.ndo_set_mac_address = eth_mac_addr,
.ndo_validate_addr = eth_validate_addr,
@@ -555,6 +540,8 @@ xpnet_init(void)
xpnet_device->netdev_ops = &xpnet_netdev_ops;
xpnet_device->mtu = XPNET_DEF_MTU;
+ xpnet_device->min_mtu = XPNET_MIN_MTU;
+ xpnet_device->max_mtu = XPNET_MAX_MTU;
/*
* Multicast assumes the LSB of the first octet is set for multicast
diff --git a/drivers/net/hippi/rrunner.c b/drivers/net/hippi/rrunner.c
index 95c0b45..f5a9728 100644
--- a/drivers/net/hippi/rrunner.c
+++ b/drivers/net/hippi/rrunner.c
@@ -68,7 +68,6 @@ static const struct net_device_ops rr_netdev_ops = {
.ndo_stop = rr_close,
.ndo_do_ioctl = rr_ioctl,
.ndo_start_xmit = rr_start_xmit,
- .ndo_change_mtu = hippi_change_mtu,
.ndo_set_mac_address = hippi_mac_addr,
};
diff --git a/drivers/net/rionet.c b/drivers/net/rionet.c
index a31f461..300bb14 100644
--- a/drivers/net/rionet.c
+++ b/drivers/net/rionet.c
@@ -466,17 +466,6 @@ static void rionet_set_msglevel(struct net_device *ndev, u32 value)
rnet->msg_enable = value;
}
-static int rionet_change_mtu(struct net_device *ndev, int new_mtu)
-{
- if ((new_mtu < 68) || (new_mtu > RIONET_MAX_MTU)) {
- printk(KERN_ERR "%s: Invalid MTU size %d\n",
- ndev->name, new_mtu);
- return -EINVAL;
- }
- ndev->mtu = new_mtu;
- return 0;
-}
-
static const struct ethtool_ops rionet_ethtool_ops = {
.get_drvinfo = rionet_get_drvinfo,
.get_msglevel = rionet_get_msglevel,
@@ -488,7 +477,6 @@ static const struct net_device_ops rionet_netdev_ops = {
.ndo_open = rionet_open,
.ndo_stop = rionet_close,
.ndo_start_xmit = rionet_start_xmit,
- .ndo_change_mtu = rionet_change_mtu,
.ndo_validate_addr = eth_validate_addr,
.ndo_set_mac_address = eth_mac_addr,
};
@@ -525,6 +513,9 @@ static int rionet_setup_netdev(struct rio_mport *mport, struct net_device *ndev)
ndev->netdev_ops = &rionet_netdev_ops;
ndev->mtu = RIONET_MAX_MTU;
+ /* MTU range: 68 - 4082 */
+ ndev->min_mtu = ETH_MIN_MTU;
+ ndev->max_mtu = RIONET_MAX_MTU;
ndev->features = NETIF_F_LLTX;
SET_NETDEV_DEV(ndev, &mport->dev);
ndev->ethtool_ops = &rionet_ethtool_ops;
diff --git a/drivers/net/slip/slip.c b/drivers/net/slip/slip.c
index 9ed6d1c..7e933d8 100644
--- a/drivers/net/slip/slip.c
+++ b/drivers/net/slip/slip.c
@@ -561,12 +561,7 @@ static int sl_change_mtu(struct net_device *dev, int new_mtu)
{
struct slip *sl = netdev_priv(dev);
- if (new_mtu < 68 || new_mtu > 65534)
- return -EINVAL;
-
- if (new_mtu != dev->mtu)
- return sl_realloc_bufs(sl, new_mtu);
- return 0;
+ return sl_realloc_bufs(sl, new_mtu);
}
/* Netdevice get statistics request */
@@ -663,6 +658,10 @@ static void sl_setup(struct net_device *dev)
dev->addr_len = 0;
dev->tx_queue_len = 10;
+ /* MTU range: 68 - 65534 */
+ dev->min_mtu = 68;
+ dev->max_mtu = 65534;
+
/* New-style flags. */
dev->flags = IFF_NOARP|IFF_POINTOPOINT|IFF_MULTICAST;
}
diff --git a/include/linux/hippidevice.h b/include/linux/hippidevice.h
index 8ec23fb..402f99e 100644
--- a/include/linux/hippidevice.h
+++ b/include/linux/hippidevice.h
@@ -32,7 +32,6 @@ struct hippi_cb {
};
__be16 hippi_type_trans(struct sk_buff *skb, struct net_device *dev);
-int hippi_change_mtu(struct net_device *dev, int new_mtu);
int hippi_mac_addr(struct net_device *dev, void *p);
int hippi_neigh_setup_dev(struct net_device *dev, struct neigh_parms *p);
struct net_device *alloc_hippi_dev(int sizeof_priv);
diff --git a/net/802/hippi.c b/net/802/hippi.c
index ade1a52..5e4427b 100644
--- a/net/802/hippi.c
+++ b/net/802/hippi.c
@@ -116,18 +116,6 @@ __be16 hippi_type_trans(struct sk_buff *skb, struct net_device *dev)
EXPORT_SYMBOL(hippi_type_trans);
-int hippi_change_mtu(struct net_device *dev, int new_mtu)
-{
- /*
- * HIPPI's got these nice large MTUs.
- */
- if ((new_mtu < 68) || (new_mtu > 65280))
- return -EINVAL;
- dev->mtu = new_mtu;
- return 0;
-}
-EXPORT_SYMBOL(hippi_change_mtu);
-
/*
* For HIPPI we will actually use the lower 4 bytes of the hardware
* address as the I-FIELD rather than the actual hardware address.
@@ -174,6 +162,8 @@ static void hippi_setup(struct net_device *dev)
dev->type = ARPHRD_HIPPI;
dev->hard_header_len = HIPPI_HLEN;
dev->mtu = 65280;
+ dev->min_mtu = 68;
+ dev->max_mtu = 65280;
dev->addr_len = HIPPI_ALEN;
dev->tx_queue_len = 25 /* 5 */;
memset(dev->broadcast, 0xFF, HIPPI_ALEN);
diff --git a/net/batman-adv/soft-interface.c b/net/batman-adv/soft-interface.c
index 49e16b6..112679d 100644
--- a/net/batman-adv/soft-interface.c
+++ b/net/batman-adv/soft-interface.c
@@ -158,17 +158,6 @@ static int batadv_interface_set_mac_addr(struct net_device *dev, void *p)
return 0;
}
-static int batadv_interface_change_mtu(struct net_device *dev, int new_mtu)
-{
- /* check ranges */
- if ((new_mtu < 68) || (new_mtu > batadv_hardif_min_mtu(dev)))
- return -EINVAL;
-
- dev->mtu = new_mtu;
-
- return 0;
-}
-
/**
* batadv_interface_set_rx_mode - set the rx mode of a device
* @dev: registered network device to modify
@@ -920,7 +909,6 @@ static const struct net_device_ops batadv_netdev_ops = {
.ndo_vlan_rx_add_vid = batadv_interface_add_vid,
.ndo_vlan_rx_kill_vid = batadv_interface_kill_vid,
.ndo_set_mac_address = batadv_interface_set_mac_addr,
- .ndo_change_mtu = batadv_interface_change_mtu,
.ndo_set_rx_mode = batadv_interface_set_rx_mode,
.ndo_start_xmit = batadv_interface_tx,
.ndo_validate_addr = eth_validate_addr,
@@ -987,6 +975,7 @@ struct net_device *batadv_softif_create(struct net *net, const char *name)
dev_net_set(soft_iface, net);
soft_iface->rtnl_link_ops = &batadv_link_ops;
+ soft_iface->max_mtu = batadv_hardif_min_mtu(soft_iface);
ret = register_netdevice(soft_iface);
if (ret < 0) {
--
2.10.0
^ permalink raw reply related
* Re: iscsi_trx going into D state
From: Robert LeBlanc @ 2016-10-18 22:13 UTC (permalink / raw)
To: Nicholas A. Bellinger
Cc: Zhu Lingshan, linux-rdma-u79uwXL29TY76Z2rM5mHXA,
linux-scsi-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1476774332.8490.43.camel-XoQW25Eq2zviZyQQd+hFbcojREIfoBdhmpATvIKMPHk@public.gmane.org>
Nicholas,
We patched this in and for the first time in many reboots, we didn't
have iSCSI going straight into D state. We have had to work on a
couple of other things, so we don't know if this is just a coincidence
or not. We will reboot back into the old kernel and back a few times
and do some more testing, but so far it has given us a little bit of
hope that we may be narrowing down on the root cause. We will report
back once we have some more info.
Thank you,
Robert LeBlanc
----------------
Robert LeBlanc
PGP Fingerprint 79A2 9CA4 6CC4 45DD A904 C70E E654 3BB2 FA62 B9F1
On Tue, Oct 18, 2016 at 1:05 AM, Nicholas A. Bellinger
<nab-IzHhD5pYlfBP7FQvKIMDCQ@public.gmane.org> wrote:
> Hello Robert, Zhu & Co,
>
> Thanks for your detailed bug report. Comments inline below.
>
> On Mon, 2016-10-17 at 22:42 -0600, Robert LeBlanc wrote:
>> Sorry I forget that Android has an aversion to plain text emails.
>>
>> If we can provide any information to help, let us know. We are willing
>> to patch in more debug statements or whatever you think might help.
>> Today has been a difficult day. Thanks for looking into it, I tried
>> looking at it, but it is way over my head.
>>
>> ----------------
>> Robert LeBlanc
>> PGP Fingerprint 79A2 9CA4 6CC4 45DD A904 C70E E654 3BB2 FA62 B9F1
>>
>>
>> On Mon, Oct 17, 2016 at 9:06 PM, Zhu Lingshan <lszhu-IBi9RG/b67k@public.gmane.org> wrote:
>> > Hi Robert,
>> >
>> > I think the reason why you can not logout the targets is that iscsi_np in D
>> > status. I think the patches fixed something, but it seems to be more than
>> > one code path can trigger these similar issues. as you can see, there are
>> > several call stacks, I am still working on it. Actually in my environment I
>> > see there is another call stack not listed in your mail....
>> >
>> > Thanks,
>> > BR
>> > Zhu Lingshan
>> >
>> >
>> > On 10/18/2016 03:11 AM, Robert LeBlanc wrote:
>> >>
>> >> Sorry hit send too soon.
>> >>
>> >> In addition, on the client we see:
>> >> # ps -aux | grep D | grep kworker
>> >> root 5583 0.0 0.0 0 0 ? D 11:55 0:03
>> >> [kworker/11:0]
>> >> root 7721 0.1 0.0 0 0 ? D 12:00 0:04
>> >> [kworker/4:25]
>> >> root 10877 0.0 0.0 0 0 ? D 09:27 0:00
>> >> [kworker/22:1]
>> >> root 11246 0.0 0.0 0 0 ? D 10:28 0:00
>> >> [kworker/30:2]
>> >> root 14034 0.0 0.0 0 0 ? D 12:20 0:02
>> >> [kworker/19:15]
>> >> root 14048 0.0 0.0 0 0 ? D 12:20 0:00
>> >> [kworker/16:0]
>> >> root 15871 0.0 0.0 0 0 ? D 12:25 0:00
>> >> [kworker/13:0]
>> >> root 17442 0.0 0.0 0 0 ? D 12:28 0:00
>> >> [kworker/9:1]
>> >> root 17816 0.0 0.0 0 0 ? D 12:30 0:00
>> >> [kworker/11:1]
>> >> root 18744 0.0 0.0 0 0 ? D 12:32 0:00
>> >> [kworker/10:2]
>> >> root 19060 0.0 0.0 0 0 ? D 12:32 0:00
>> >> [kworker/29:0]
>> >> root 21748 0.0 0.0 0 0 ? D 12:40 0:00
>> >> [kworker/21:0]
>> >> root 21967 0.0 0.0 0 0 ? D 12:40 0:00
>> >> [kworker/22:0]
>> >> root 21978 0.0 0.0 0 0 ? D 12:40 0:00
>> >> [kworker/22:2]
>> >> root 22024 0.0 0.0 0 0 ? D 12:40 0:00
>> >> [kworker/22:4]
>> >> root 22035 0.0 0.0 0 0 ? D 12:40 0:00
>> >> [kworker/22:5]
>> >> root 22060 0.0 0.0 0 0 ? D 12:40 0:00
>> >> [kworker/16:1]
>> >> root 22282 0.0 0.0 0 0 ? D 12:41 0:00
>> >> [kworker/26:0]
>> >> root 22362 0.0 0.0 0 0 ? D 12:42 0:00
>> >> [kworker/18:9]
>> >> root 22426 0.0 0.0 0 0 ? D 12:42 0:00
>> >> [kworker/16:3]
>> >> root 23298 0.0 0.0 0 0 ? D 12:43 0:00
>> >> [kworker/12:1]
>> >> root 23302 0.0 0.0 0 0 ? D 12:43 0:00
>> >> [kworker/12:5]
>> >> root 24264 0.0 0.0 0 0 ? D 12:46 0:00
>> >> [kworker/30:1]
>> >> root 24271 0.0 0.0 0 0 ? D 12:46 0:00
>> >> [kworker/14:8]
>> >> root 24441 0.0 0.0 0 0 ? D 12:47 0:00
>> >> [kworker/9:7]
>> >> root 24443 0.0 0.0 0 0 ? D 12:47 0:00
>> >> [kworker/9:9]
>> >> root 25005 0.0 0.0 0 0 ? D 12:48 0:00
>> >> [kworker/30:3]
>> >> root 25158 0.0 0.0 0 0 ? D 12:49 0:00
>> >> [kworker/9:12]
>> >> root 26382 0.0 0.0 0 0 ? D 12:52 0:00
>> >> [kworker/13:2]
>> >> root 26453 0.0 0.0 0 0 ? D 12:52 0:00
>> >> [kworker/21:2]
>> >> root 26724 0.0 0.0 0 0 ? D 12:53 0:00
>> >> [kworker/19:1]
>> >> root 28400 0.0 0.0 0 0 ? D 05:20 0:00
>> >> [kworker/25:1]
>> >> root 29552 0.0 0.0 0 0 ? D 11:40 0:00
>> >> [kworker/17:1]
>> >> root 29811 0.0 0.0 0 0 ? D 11:40 0:00
>> >> [kworker/7:10]
>> >> root 31903 0.0 0.0 0 0 ? D 11:43 0:00
>> >> [kworker/26:1]
>> >>
>> >> And all of the processes have this stack:
>> >> [<ffffffffa0727ed5>] iser_release_work+0x25/0x60 [ib_iser]
>> >> [<ffffffff8109633f>] process_one_work+0x14f/0x400
>> >> [<ffffffff81096bb4>] worker_thread+0x114/0x470
>> >> [<ffffffff8109c6f8>] kthread+0xd8/0xf0
>> >> [<ffffffff8172004f>] ret_from_fork+0x3f/0x70
>> >> [<ffffffffffffffff>] 0xffffffffffffffff
>> >>
>> >> We are not able to log out of the sessions in all cases. And have to
>> >> restart the box.
>> >>
>> >> iscsiadm -m session will show messages like:
>> >> iscsiadm: could not read session targetname: 5
>> >> iscsiadm: could not find session info for session100
>> >> iscsiadm: could not read session targetname: 5
>> >> iscsiadm: could not find session info for session101
>> >> iscsiadm: could not read session targetname: 5
>> >> iscsiadm: could not find session info for session103
>> >> ...
>> >>
>> >> I can't find any way to force iscsiadm to clean up these sessions
>> >> possibly due to tasks in D state.
>> >> ----------------
>> >> Robert LeBlanc
>> >> PGP Fingerprint 79A2 9CA4 6CC4 45DD A904 C70E E654 3BB2 FA62 B9F1
>> >>
>> >>
>> >> On Mon, Oct 17, 2016 at 10:32 AM, Robert LeBlanc <robert-4JaGZRWAfWbajFs6igw21g@public.gmane.org>
>> >> wrote:
>> >>>
>> >>> Some more info as we hit this this morning. We have volumes mirrored
>> >>> between two targets and we had one target on the kernel with the three
>> >>> patches mentioned in this thread [0][1][2] and the other was on a
>> >>> kernel without the patches. We decided that after a week and a half we
>> >>> wanted to get both targets on the same kernel so we rebooted the
>> >>> non-patched target. Within an hour we saw iSCSI in D state with the
>> >>> same stack trace so it seems that we are not hitting any of the
>> >>> WARN_ON lines. We are getting both iscsi_trx and iscsi_np both in D
>> >>> state, this time we have two iscsi_trx processes in D state. I don't
>> >>> know if stale sessions on the clients could be contributing to this
>> >>> issue (the target trying to close non-existent sessions??). This is on
>> >>> 4.4.23. Any more debug info we can throw at this problem to help?
>> >>>
>> >>> Thank you,
>> >>> Robert LeBlanc
>> >>>
>> >>> # ps aux | grep D | grep iscsi
>> >>> root 16525 0.0 0.0 0 0 ? D 08:50 0:00
>> >>> [iscsi_np]
>> >>> root 16614 0.0 0.0 0 0 ? D 08:50 0:00
>> >>> [iscsi_trx]
>> >>> root 16674 0.0 0.0 0 0 ? D 08:50 0:00
>> >>> [iscsi_trx]
>> >>>
>> >>> # for i in 16525 16614 16674; do echo $i; cat /proc/$i/stack; done
>> >>> 16525
>> >>> [<ffffffff814f0d5f>] iscsit_stop_session+0x19f/0x1d0
>> >>> [<ffffffff814e2516>] iscsi_check_for_session_reinstatement+0x1e6/0x270
>> >>> [<ffffffff814e4ed0>] iscsi_target_check_for_existing_instances+0x30/0x40
>> >>> [<ffffffff814e5020>] iscsi_target_do_login+0x140/0x640
>> >>> [<ffffffff814e63bc>] iscsi_target_start_negotiation+0x1c/0xb0
>> >>> [<ffffffff814e410b>] iscsi_target_login_thread+0xa9b/0xfc0
>> >>> [<ffffffff8109c748>] kthread+0xd8/0xf0
>> >>> [<ffffffff8172018f>] ret_from_fork+0x3f/0x70
>> >>> [<ffffffffffffffff>] 0xffffffffffffffff
>> >>> 16614
>> >>> [<ffffffff814cca79>] target_wait_for_sess_cmds+0x49/0x1a0
>> >>> [<ffffffffa064692b>] isert_wait_conn+0x1ab/0x2f0 [ib_isert]
>> >>> [<ffffffff814f0ef2>] iscsit_close_connection+0x162/0x870
>> >>> [<ffffffff814df9bf>] iscsit_take_action_for_connection_exit+0x7f/0x100
>> >>> [<ffffffff814f00a0>] iscsi_target_rx_thread+0x5a0/0xe80
>> >>> [<ffffffff8109c748>] kthread+0xd8/0xf0
>> >>> [<ffffffff8172018f>] ret_from_fork+0x3f/0x70
>> >>> [<ffffffffffffffff>] 0xffffffffffffffff
>> >>> 16674
>> >>> [<ffffffff814cca79>] target_wait_for_sess_cmds+0x49/0x1a0
>> >>> [<ffffffffa064692b>] isert_wait_conn+0x1ab/0x2f0 [ib_isert]
>> >>> [<ffffffff814f0ef2>] iscsit_close_connection+0x162/0x870
>> >>> [<ffffffff814df9bf>] iscsit_take_action_for_connection_exit+0x7f/0x100
>> >>> [<ffffffff814f00a0>] iscsi_target_rx_thread+0x5a0/0xe80
>> >>> [<ffffffff8109c748>] kthread+0xd8/0xf0
>> >>> [<ffffffff8172018f>] ret_from_fork+0x3f/0x70
>> >>> [<ffffffffffffffff>] 0xffffffffffffffff
>> >>>
>> >>>
>> >>> [0] https://www.spinics.net/lists/target-devel/msg13463.html
>> >>> [1] http://marc.info/?l=linux-scsi&m=147282568910535&w=2
>> >>> [2] http://www.spinics.net/lists/linux-scsi/msg100221.html
>
> The call chain above is iscsi session reinstatement driven by
> open-iscsi/iser resulting in target-core to sleep indefinitely, waiting
> for outstanding target-core backend driver se_cmd I/O to complete in
> order to make forward progress.
>
> Note, there is a v4.1+ se_cmd->cmd_kref reference leak bug for
> TMR ABORT_TASK during simultaneous target back-end I/O completion
> timeouts here:
>
> http://www.spinics.net/lists/target-devel/msg13530.html
>
> If you are actively observing TMR ABORT_TASK preceding the hung task
> timeout warnings above with v4.4.y + v4.2.y iser-target exports, then
> it's likely the same bug. Please apply the patch on your v4.x setup to
> verify.
>
> If no TMR ABORT_TASK timeouts + session reinstatements are occurring on
> your iser-target setup, then it is a separate bug.
>
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v3 0/11] Fix race conditions related to stopping block layer queues
From: Bart Van Assche @ 2016-10-18 21:56 UTC (permalink / raw)
To: Jens Axboe
Cc: Christoph Hellwig, James Bottomley, Martin K. Petersen,
Mike Snitzer, Doug Ledford, Keith Busch, Ming Lin,
Laurence Oberman,
linux-block-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-scsi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-nvme-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
In-Reply-To: <b39eb0e7-1007-eb63-8e7f-9a7f08508379-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
On 10/18/2016 02:48 PM, Bart Van Assche wrote:
> - blk_mq_quiesce_queue() has been reworked (thanks to Ming Lin and Sagi
> for their feedback).
(replying to my own e-mail)
A correction: Ming Lei provided feedback on v2 of this patch series
instead of Ming Lin.
Bart.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH v3 11/11] nvme: Fix a race condition
From: Bart Van Assche @ 2016-10-18 21:53 UTC (permalink / raw)
To: Jens Axboe
Cc: Christoph Hellwig, James Bottomley, Martin K. Petersen,
Mike Snitzer, Doug Ledford, Keith Busch, Ming Lin,
Laurence Oberman, linux-block@vger.kernel.org,
linux-scsi@vger.kernel.org, linux-rdma@vger.kernel.org,
linux-nvme@lists.infradead.org
In-Reply-To: <b39eb0e7-1007-eb63-8e7f-9a7f08508379@sandisk.com>
Avoid that nvme_queue_rq() is still running when nvme_stop_queues()
returns.
Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: Keith Busch <keith.busch@intel.com>
Cc: Sagi Grimberg <sagi@grimberg.me>
Cc: Christoph Hellwig <hch@lst.de>
---
drivers/nvme/host/core.c | 15 ++++++---------
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 18a265d..96f00c7 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -201,13 +201,7 @@ static struct nvme_ns *nvme_get_ns_from_disk(struct gendisk *disk)
void nvme_requeue_req(struct request *req)
{
- unsigned long flags;
-
- blk_mq_requeue_request(req, false);
- spin_lock_irqsave(req->q->queue_lock, flags);
- if (!blk_mq_queue_stopped(req->q))
- blk_mq_kick_requeue_list(req->q);
- spin_unlock_irqrestore(req->q->queue_lock, flags);
+ blk_mq_requeue_request(req, true);
}
EXPORT_SYMBOL_GPL(nvme_requeue_req);
@@ -2074,11 +2068,14 @@ EXPORT_SYMBOL_GPL(nvme_kill_queues);
void nvme_stop_queues(struct nvme_ctrl *ctrl)
{
struct nvme_ns *ns;
+ struct request_queue *q;
mutex_lock(&ctrl->namespaces_mutex);
list_for_each_entry(ns, &ctrl->namespaces, list) {
- blk_mq_cancel_requeue_work(ns->queue);
- blk_mq_stop_hw_queues(ns->queue);
+ q = ns->queue;
+ blk_mq_cancel_requeue_work(q);
+ blk_mq_stop_hw_queues(q);
+ blk_mq_quiesce_queue(q);
}
mutex_unlock(&ctrl->namespaces_mutex);
}
--
2.10.1
^ permalink raw reply related
* [PATCH v3 10/11] nvme: Use BLK_MQ_S_STOPPED instead of QUEUE_FLAG_STOPPED in blk-mq code
From: Bart Van Assche @ 2016-10-18 21:53 UTC (permalink / raw)
To: Jens Axboe
Cc: Christoph Hellwig, James Bottomley, Martin K. Petersen,
Mike Snitzer, Doug Ledford, Keith Busch, Ming Lin,
Laurence Oberman, linux-block@vger.kernel.org,
linux-scsi@vger.kernel.org, linux-rdma@vger.kernel.org,
linux-nvme@lists.infradead.org
In-Reply-To: <b39eb0e7-1007-eb63-8e7f-9a7f08508379@sandisk.com>
Make nvme_requeue_req() check BLK_MQ_S_STOPPED instead of
QUEUE_FLAG_STOPPED. Remove the QUEUE_FLAG_STOPPED manipulations
that became superfluous because of this change. This patch fixes
a race condition: using queue_flag_clear_unlocked() is not safe
if any other function that manipulates the queue flags can be
called concurrently, e.g. blk_cleanup_queue().
Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: Keith Busch <keith.busch@intel.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Sagi Grimberg <sagi@grimberg.me>
---
drivers/nvme/host/core.c | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index e4a6f2d..18a265d 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -205,7 +205,7 @@ void nvme_requeue_req(struct request *req)
blk_mq_requeue_request(req, false);
spin_lock_irqsave(req->q->queue_lock, flags);
- if (!blk_queue_stopped(req->q))
+ if (!blk_mq_queue_stopped(req->q))
blk_mq_kick_requeue_list(req->q);
spin_unlock_irqrestore(req->q->queue_lock, flags);
}
@@ -2077,10 +2077,6 @@ void nvme_stop_queues(struct nvme_ctrl *ctrl)
mutex_lock(&ctrl->namespaces_mutex);
list_for_each_entry(ns, &ctrl->namespaces, list) {
- spin_lock_irq(ns->queue->queue_lock);
- queue_flag_set(QUEUE_FLAG_STOPPED, ns->queue);
- spin_unlock_irq(ns->queue->queue_lock);
-
blk_mq_cancel_requeue_work(ns->queue);
blk_mq_stop_hw_queues(ns->queue);
}
@@ -2094,7 +2090,6 @@ void nvme_start_queues(struct nvme_ctrl *ctrl)
mutex_lock(&ctrl->namespaces_mutex);
list_for_each_entry(ns, &ctrl->namespaces, list) {
- queue_flag_clear_unlocked(QUEUE_FLAG_STOPPED, ns->queue);
blk_mq_start_stopped_hw_queues(ns->queue, true);
blk_mq_kick_requeue_list(ns->queue);
}
--
2.10.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox