* [PATCH] remove unused line for mmap_region()
@ 2009-06-21 14:43 Huang Shijie
2009-06-21 18:30 ` Hugh Dickins
0 siblings, 1 reply; 6+ messages in thread
From: Huang Shijie @ 2009-06-21 14:43 UTC (permalink / raw)
To: akpm; +Cc: linux-mm, Huang Shijie
The variable pgoff is not used in the following codes.
So, just remove the line.
Signed-off-by: Huang Shijie <shijie8@gmail.com>
---
mm/mmap.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/mm/mmap.c b/mm/mmap.c
index 34579b2..1dd6aaa 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -1210,7 +1210,6 @@ munmap_back:
* f_op->mmap method. -DaveM
*/
addr = vma->vm_start;
- pgoff = vma->vm_pgoff;
vm_flags = vma->vm_flags;
if (vma_wants_writenotify(vma))
--
1.6.0.6
--
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 related [flat|nested] 6+ messages in thread
* Re: [PATCH] remove unused line for mmap_region()
2009-06-21 14:43 [PATCH] remove unused line for mmap_region() Huang Shijie
@ 2009-06-21 18:30 ` Hugh Dickins
2009-06-22 3:50 ` Huang Shijie
0 siblings, 1 reply; 6+ messages in thread
From: Hugh Dickins @ 2009-06-21 18:30 UTC (permalink / raw)
To: Huang Shijie; +Cc: akpm, linux-mm
On Sun, 21 Jun 2009, Huang Shijie wrote:
> The variable pgoff is not used in the following codes.
> So, just remove the line.
>
> Signed-off-by: Huang Shijie <shijie8@gmail.com>
Hmm, hmm, well, I suppose a grudging Ack, though I'd happily be overruled.
Of course you are right, so why am I so reluctant to acknowledge it?
Because it's exceptional for addr and pgoff to be different after
coming back from the file's ->mmap method, and if someone adds a
use for pgoff lower down (there was, of course, a use for pgoff
in vma_merge lower down, until Linus moved that higher in 2.6.29),
it's a fair bet that they'll forget to restore the update you're
removing, and a fair bet that nobody will notice that it's gone
wrong for a while.
However, what would be likely to need pgoff lower down, other than
another attempt at vma_merge? And given how we're now working with
the vma_merge higher up (before pgoff had a chance to be adjusted),
I think we can conclude that anything needing to meddle with pgoff
would be setting some vm_flags that prevent merging anyway.
So I can just about bring myself to Ack your patch, but...
Hugh
> ---
> mm/mmap.c | 1 -
> 1 files changed, 0 insertions(+), 1 deletions(-)
>
> diff --git a/mm/mmap.c b/mm/mmap.c
> index 34579b2..1dd6aaa 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -1210,7 +1210,6 @@ munmap_back:
> * f_op->mmap method. -DaveM
> */
> addr = vma->vm_start;
> - pgoff = vma->vm_pgoff;
> vm_flags = vma->vm_flags;
>
> if (vma_wants_writenotify(vma))
> --
> 1.6.0.6
--
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 [flat|nested] 6+ messages in thread
* Re: [PATCH] remove unused line for mmap_region()
2009-06-21 18:30 ` Hugh Dickins
@ 2009-06-22 3:50 ` Huang Shijie
2009-06-23 11:14 ` Hugh Dickins
0 siblings, 1 reply; 6+ messages in thread
From: Huang Shijie @ 2009-06-22 3:50 UTC (permalink / raw)
To: Hugh Dickins; +Cc: akpm, linux-mm
> On Sun, 21 Jun 2009, Huang Shijie wrote:
>
>
>> The variable pgoff is not used in the following codes.
>> So, just remove the line.
>>
>> Signed-off-by: Huang Shijie <shijie8@gmail.com>
>>
>
> Hmm, hmm, well, I suppose a grudging Ack, though I'd happily be overruled.
>
> Of course you are right, so why am I so reluctant to acknowledge it?
>
> Because it's exceptional for addr and pgoff to be different after
> coming back from the file's ->mmap method, and if someone adds a
> use for pgoff lower down (there was, of course, a use for pgoff
> in vma_merge lower down, until Linus moved that higher in 2.6.29),
> it's a fair bet that they'll forget to restore the update you're
> removing, and a fair bet that nobody will notice that it's gone
> wrong for a while.
>
>
I knew the file's ->mmap method can change addr and pgoff, but I think
the comment of
DavidM is enough to remind the person who wants to use pgoff lower down
that pgoff maybe
go wrong for a while.
But now, it (this line) looks like a big fly on 'Mona Lisa'. :)
> However, what would be likely to need pgoff lower down, other than
> another attempt at vma_merge? And given how we're now working with
> the vma_merge higher up (before pgoff had a chance to be adjusted),
> I think we can conclude that anything needing to meddle with pgoff
> would be setting some vm_flags that prevent merging anyway.
>
>
Could you tell some drivers which will meddle with pgoff?
If a driver changes pgoff,the work done by vma_merge higher up will be
invalidated.
The process gets a wrong memory map.
> So I can just about bring myself to Ack your patch, but...
>
> Hugh
>
>
>> ---
>> mm/mmap.c | 1 -
>> 1 files changed, 0 insertions(+), 1 deletions(-)
>>
>> diff --git a/mm/mmap.c b/mm/mmap.c
>> index 34579b2..1dd6aaa 100644
>> --- a/mm/mmap.c
>> +++ b/mm/mmap.c
>> @@ -1210,7 +1210,6 @@ munmap_back:
>> * f_op->mmap method. -DaveM
>> */
>> addr = vma->vm_start;
>> - pgoff = vma->vm_pgoff;
>> vm_flags = vma->vm_flags;
>>
>> if (vma_wants_writenotify(vma))
>> --
>> 1.6.0.6
>>
>
>
--
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 [flat|nested] 6+ messages in thread
* Re: [PATCH] remove unused line for mmap_region()
2009-06-22 3:50 ` Huang Shijie
@ 2009-06-23 11:14 ` Hugh Dickins
2009-06-24 3:16 ` Huang Shijie
0 siblings, 1 reply; 6+ messages in thread
From: Hugh Dickins @ 2009-06-23 11:14 UTC (permalink / raw)
To: Huang Shijie; +Cc: akpm, linux-mm
On Mon, 22 Jun 2009, Huang Shijie wrote:
> >
> I knew the file's ->mmap method can change addr and pgoff,
> but I think the comment of DavidM is enough to remind the
> person who wants to use pgoff lower down that pgoff maybe
> go wrong for a while.
Indeed it's a very helpful comment from DaveM there. Unfortunately,
we can't really repeat that comment on every line below where
somebody might in future want to insert a use of pgoff, and not all
of us are as good at actually reading nearby comments as you are!
>
> But now, it (this line) looks like a big fly on 'Mona Lisa'. :)
> > However, what would be likely to need pgoff lower down, other than
> > another attempt at vma_merge? And given how we're now working with
> > the vma_merge higher up (before pgoff had a chance to be adjusted),
> > I think we can conclude that anything needing to meddle with pgoff
> > would be setting some vm_flags that prevent merging anyway.
> >
> Could you tell some drivers which will meddle with pgoff?
I can't name a list of drivers offhand, no (but note VM_PFNMAP areas
have a particular use for vm_pgoff, so all those drivers are likely
to be on the list). May I please leave that investigation to you?
What I expect you to find in the end is that every driver which does
meddle with pgoff in its ->mmap, also has some other characteristic
(e.g. sets VM_IO or VM_DONTEXPAND or VM_RESERVED or VM_PFNMAP, or
even some other flag which the new vm_flags wouldn't have set),
which will prevent its vmas being merged anyway.
> If a driver changes pgoff,the work done by vma_merge higher
> up will be invalidated. The process gets a wrong memory map.
It's probably all okay; but I won't be astonished if you discover
one or two cases which ought to be fixed up, probably by adding
one of those flags.
Thanks,
Hugh
--
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 [flat|nested] 6+ messages in thread
* Re: [PATCH] remove unused line for mmap_region()
2009-06-23 11:14 ` Hugh Dickins
@ 2009-06-24 3:16 ` Huang Shijie
2009-06-24 4:22 ` Hugh Dickins
0 siblings, 1 reply; 6+ messages in thread
From: Huang Shijie @ 2009-06-24 3:16 UTC (permalink / raw)
To: Hugh Dickins; +Cc: akpm, linux-mm
> I can't name a list of drivers offhand, no (but note VM_PFNMAP areas
> have a particular use for vm_pgoff, so all those drivers are likely
> to be on the list). May I please leave that investigation to you?
>
>
ok.
> What I expect you to find in the end is that every driver which does
> meddle with pgoff in its ->mmap, also has some other characteristic
> (e.g. sets VM_IO or VM_DONTEXPAND or VM_RESERVED or VM_PFNMAP, or
> even some other flag which the new vm_flags wouldn't have set),
> which will prevent its vmas being merged anyway.
>
>
Unfortunately,the driver's -->mmap is called below the vma_merge(),
so even if the driver sets the VM_SPECIAL flag, it does not prevent the
vmas being merged actually.
--
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 [flat|nested] 6+ messages in thread
* Re: [PATCH] remove unused line for mmap_region()
2009-06-24 3:16 ` Huang Shijie
@ 2009-06-24 4:22 ` Hugh Dickins
0 siblings, 0 replies; 6+ messages in thread
From: Hugh Dickins @ 2009-06-24 4:22 UTC (permalink / raw)
To: Huang Shijie; +Cc: akpm, linux-mm
On Wed, 24 Jun 2009, Huang Shijie wrote:
> > What I expect you to find in the end is that every driver which does
> > meddle with pgoff in its ->mmap, also has some other characteristic
> > (e.g. sets VM_IO or VM_DONTEXPAND or VM_RESERVED or VM_PFNMAP, or
> > even some other flag which the new vm_flags wouldn't have set),
> > which will prevent its vmas being merged anyway.
> >
> Unfortunately,the driver's -->mmap is called below the vma_merge(),
> so even if the driver sets the VM_SPECIAL flag, it does not prevent the
> vmas being merged actually.
It does not prevent it by virtue of being VM_SPECIAL at that point,
you're right, but I believe it does prevent it by virtue of presenting
a different vm_flags. Collapsing a definition to make it clearer, see
if ((vma->vm_flags ^ vm_flags) & ~VM_CAN_NONLINEAR)
return 0;
at the beginning of is_mergeable_vma(). We have to make an exception
of VM_CAN_NONLINEAR precisely because it gets set by a normal ->mmap,
but cannot be predicted by mmap_region() before its vma_merge().
Hugh
--
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 [flat|nested] 6+ messages in thread
end of thread, other threads:[~2009-06-24 4:22 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-06-21 14:43 [PATCH] remove unused line for mmap_region() Huang Shijie
2009-06-21 18:30 ` Hugh Dickins
2009-06-22 3:50 ` Huang Shijie
2009-06-23 11:14 ` Hugh Dickins
2009-06-24 3:16 ` Huang Shijie
2009-06-24 4:22 ` Hugh Dickins
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.