public inbox for linux-ia64@vger.kernel.org
 help / color / mirror / Atom feed
* Re: [Discontig-devel] Re: [Linux-ia64] Re: 2.5.59 & mmap_sem
@ 2003-02-18 10:43 Andrew Morton
  2003-02-24 10:14 ` Xavier Bru
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Andrew Morton @ 2003-02-18 10:43 UTC (permalink / raw)
  To: linux-ia64

William Lee Irwin III <wli@holomorphy.com> wrote:
>
> On Tue, Feb 18, 2003 at 09:46:31AM +0100, Xavier Bru wrote:
> > Thanks for your answers.
> > You are right, we do not need a page structure for mapping /dev/mem in 
> > IO space (I am not a mm expert :-).
> > Here after a possible patch that allows the Xserver running on the NUMA
> > platform. (We had the same problem on Azusa).
> 
> Looks perfectly reasonable to me. Other discontig/NUMA implementations
> are likely to run into the same issue.
> 
> You might want to send this on to lkml and cc: akpm, he's pretty much
> the final destination wrt. VM patches and 2.5.
> 

I'd prefer to do this at the vma level if poss.  Given that mmap_mem()
has set VM_RESERVED against the VMA, it should be sufficient to do

diff -puN mm/memory.c~a mm/memory.c
--- 25/mm/memory.c~a	2003-02-18 02:40:01.000000000 -0800
+++ 25-akpm/mm/memory.c	2003-02-18 02:40:18.000000000 -0800
@@ -211,6 +211,9 @@ int copy_page_range(struct mm_struct *ds
 	unsigned long cow;
 	struct pte_chain *pte_chain = NULL;
 
+	if (vma->vm_flags & (VM_RESERVED|VM_IO))
+		return 0;
+
 	if (is_vm_hugetlb_page(vma))
 		return copy_hugetlb_page_range(dst, src, vma);
 
diff -puN fs/hugetlbfs/inode.c~a fs/hugetlbfs/inode.c
--- 25/fs/hugetlbfs/inode.c~a	2003-02-18 02:42:47.000000000 -0800
+++ 25-akpm/fs/hugetlbfs/inode.c	2003-02-18 02:42:55.000000000 -0800
@@ -63,7 +63,7 @@ static int hugetlbfs_file_mmap(struct fi
 	down(&inode->i_sem);
 
 	UPDATE_ATIME(inode);
-	vma->vm_flags |= VM_HUGETLB | VM_RESERVED;
+	vma->vm_flags |= VM_HUGETLB;
 	vma->vm_ops = &hugetlb_vm_ops;
 	ret = hugetlb_prefault(mapping, vma);
 	len = (loff_t)(vma->vm_end - vma->vm_start) +

_
 

The semantics (and usage!) of the VM_foo flags are rather vague.  It needs
definiton, and an audit.



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Discontig-devel] Re: [Linux-ia64] Re: 2.5.59 & mmap_sem
  2003-02-18 10:43 [Discontig-devel] Re: [Linux-ia64] Re: 2.5.59 & mmap_sem Andrew Morton
@ 2003-02-24 10:14 ` Xavier Bru
  2003-03-02  1:32 ` Andrew Morton
  2003-03-03 15:37 ` Xavier Bru
  2 siblings, 0 replies; 4+ messages in thread
From: Xavier Bru @ 2003-02-24 10:14 UTC (permalink / raw)
  To: linux-ia64

Works OK for us.
Thanks

-- 

 Sincères salutations.
_____________________________________________________________________
 
Xavier BRU                 BULL ISD/R&D/INTEL office:     FREC B1-422
tel : +33 (0)4 76 29 77 45                    http://www-frec.bull.fr
fax : +33 (0)4 76 29 77 70                 mailto:Xavier.Bru@bull.net
addr: BULL, 1 rue de Provence, BP 208, 38432 Echirolles Cedex, FRANCE
_____________________________________________________________________

Andrew Morton writes:
 > William Lee Irwin III <wli@holomorphy.com> wrote:
 > >
 > > On Tue, Feb 18, 2003 at 09:46:31AM +0100, Xavier Bru wrote:
 > > > Thanks for your answers.
 > > > You are right, we do not need a page structure for mapping /dev/mem in 
 > > > IO space (I am not a mm expert :-).
 > > > Here after a possible patch that allows the Xserver running on the NUMA
 > > > platform. (We had the same problem on Azusa).
 > > 
 > > Looks perfectly reasonable to me. Other discontig/NUMA implementations
 > > are likely to run into the same issue.
 > > 
 > > You might want to send this on to lkml and cc: akpm, he's pretty much
 > > the final destination wrt. VM patches and 2.5.
 > > 
 > 
 > I'd prefer to do this at the vma level if poss.  Given that mmap_mem()
 > has set VM_RESERVED against the VMA, it should be sufficient to do
 > 
 > diff -puN mm/memory.c~a mm/memory.c
 > --- 25/mm/memory.c~a	2003-02-18 02:40:01.000000000 -0800
 > +++ 25-akpm/mm/memory.c	2003-02-18 02:40:18.000000000 -0800
 > @@ -211,6 +211,9 @@ int copy_page_range(struct mm_struct *ds
 >  	unsigned long cow;
 >  	struct pte_chain *pte_chain = NULL;
 >  
 > +	if (vma->vm_flags & (VM_RESERVED|VM_IO))
 > +		return 0;
 > +
 >  	if (is_vm_hugetlb_page(vma))
 >  		return copy_hugetlb_page_range(dst, src, vma);
 >  
 > diff -puN fs/hugetlbfs/inode.c~a fs/hugetlbfs/inode.c
 > --- 25/fs/hugetlbfs/inode.c~a	2003-02-18 02:42:47.000000000 -0800
 > +++ 25-akpm/fs/hugetlbfs/inode.c	2003-02-18 02:42:55.000000000 -0800
 > @@ -63,7 +63,7 @@ static int hugetlbfs_file_mmap(struct fi
 >  	down(&inode->i_sem);
 >  
 >  	UPDATE_ATIME(inode);
 > -	vma->vm_flags |= VM_HUGETLB | VM_RESERVED;
 > +	vma->vm_flags |= VM_HUGETLB;
 >  	vma->vm_ops = &hugetlb_vm_ops;
 >  	ret = hugetlb_prefault(mapping, vma);
 >  	len = (loff_t)(vma->vm_end - vma->vm_start) +
 > 
 > _
 >  
 > 
 > The semantics (and usage!) of the VM_foo flags are rather vague.  It needs
 > definiton, and an audit.
 > 


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Discontig-devel] Re: [Linux-ia64] Re: 2.5.59 & mmap_sem
  2003-02-18 10:43 [Discontig-devel] Re: [Linux-ia64] Re: 2.5.59 & mmap_sem Andrew Morton
  2003-02-24 10:14 ` Xavier Bru
@ 2003-03-02  1:32 ` Andrew Morton
  2003-03-03 15:37 ` Xavier Bru
  2 siblings, 0 replies; 4+ messages in thread
From: Andrew Morton @ 2003-03-02  1:32 UTC (permalink / raw)
  To: linux-ia64

On reflection, I think your approach is best - there is (some) value in being
able to inherit a /dev/mem mapping across fork(), and ceasing to do so may
break applications.

I recast your patch a little.  Saves a few instructions and prevents a bogus
uninitialised variable warning.


diff -puN mm/memory.c~copy_page_range-invalid-page-fix mm/memory.c
--- 25/mm/memory.c~copy_page_range-invalid-page-fix	2003-03-01 17:26:03.000000000 -0800
+++ 25-akpm/mm/memory.c	2003-03-01 17:26:29.000000000 -0800
@@ -286,9 +286,11 @@ skip_copy_pte_range:
 					goto cont_copy_pte_range_noset;
 				}
 				pfn = pte_pfn(pte);
+				if (!pfn_valid(pfn)) {
+					set_pte(dst_pte, pte);
+					goto cont_copy_pte_range_noset;
+				}
 				page = pfn_to_page(pfn);
-				if (!pfn_valid(pfn))
-					goto cont_copy_pte_range;
 				if (PageReserved(page))
 					goto cont_copy_pte_range;
 

_

Xavier Bru  <Xavier.Bru@bull.net> wrote:
>
> Works OK for us.
> Thanks
> 
> -- 
> 
> _____________________________________________________________________
>  
> Xavier BRU                 BULL ISD/R&D/INTEL office:     FREC B1-422
> tel : +33 (0)4 76 29 77 45                    http://www-frec.bull.fr
> fax : +33 (0)4 76 29 77 70                 mailto:Xavier.Bru@bull.net
> addr: BULL, 1 rue de Provence, BP 208, 38432 Echirolles Cedex, FRANCE
> _____________________________________________________________________
> 
> Andrew Morton writes:
>  > William Lee Irwin III <wli@holomorphy.com> wrote:
>  > >
>  > > On Tue, Feb 18, 2003 at 09:46:31AM +0100, Xavier Bru wrote:
>  > > > Thanks for your answers.
>  > > > You are right, we do not need a page structure for mapping /dev/mem in 
>  > > > IO space (I am not a mm expert :-).
>  > > > Here after a possible patch that allows the Xserver running on the NUMA
>  > > > platform. (We had the same problem on Azusa).
>  > > 
>  > > Looks perfectly reasonable to me. Other discontig/NUMA implementations
>  > > are likely to run into the same issue.
>  > > 
>  > > You might want to send this on to lkml and cc: akpm, he's pretty much
>  > > the final destination wrt. VM patches and 2.5.
>  > > 
>  > 
>  > I'd prefer to do this at the vma level if poss.  Given that mmap_mem()
>  > has set VM_RESERVED against the VMA, it should be sufficient to do
>  > 
>  > diff -puN mm/memory.c~a mm/memory.c
>  > --- 25/mm/memory.c~a	2003-02-18 02:40:01.000000000 -0800
>  > +++ 25-akpm/mm/memory.c	2003-02-18 02:40:18.000000000 -0800
>  > @@ -211,6 +211,9 @@ int copy_page_range(struct mm_struct *ds
>  >  	unsigned long cow;
>  >  	struct pte_chain *pte_chain = NULL;
>  >  
>  > +	if (vma->vm_flags & (VM_RESERVED|VM_IO))
>  > +		return 0;
>  > +
>  >  	if (is_vm_hugetlb_page(vma))
>  >  		return copy_hugetlb_page_range(dst, src, vma);
>  >  
>  > diff -puN fs/hugetlbfs/inode.c~a fs/hugetlbfs/inode.c
>  > --- 25/fs/hugetlbfs/inode.c~a	2003-02-18 02:42:47.000000000 -0800
>  > +++ 25-akpm/fs/hugetlbfs/inode.c	2003-02-18 02:42:55.000000000 -0800
>  > @@ -63,7 +63,7 @@ static int hugetlbfs_file_mmap(struct fi
>  >  	down(&inode->i_sem);
>  >  
>  >  	UPDATE_ATIME(inode);
>  > -	vma->vm_flags |= VM_HUGETLB | VM_RESERVED;
>  > +	vma->vm_flags |= VM_HUGETLB;
>  >  	vma->vm_ops = &hugetlb_vm_ops;
>  >  	ret = hugetlb_prefault(mapping, vma);
>  >  	len = (loff_t)(vma->vm_end - vma->vm_start) +
>  > 
>  > _
>  >  
>  > 
>  > The semantics (and usage!) of the VM_foo flags are rather vague.  It needs
>  > definiton, and an audit.
>  > 
> 
> 
> -------------------------------------------------------
> This sf.net email is sponsored by:ThinkGeek
> Welcome to geek heaven.
> http://thinkgeek.com/sf
> _______________________________________________
> Discontig-devel mailing list
> Discontig-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/discontig-devel


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Discontig-devel] Re: [Linux-ia64] Re: 2.5.59 & mmap_sem
  2003-02-18 10:43 [Discontig-devel] Re: [Linux-ia64] Re: 2.5.59 & mmap_sem Andrew Morton
  2003-02-24 10:14 ` Xavier Bru
  2003-03-02  1:32 ` Andrew Morton
@ 2003-03-03 15:37 ` Xavier Bru
  2 siblings, 0 replies; 4+ messages in thread
From: Xavier Bru @ 2003-03-03 15:37 UTC (permalink / raw)
  To: linux-ia64

I tested this one OK on 2.5.60.
Thanks.
Xavier

-- 

 Sincères salutations.
_____________________________________________________________________
 
Xavier BRU                 BULL ISD/R&D/INTEL office:     FREC B1-422
tel : +33 (0)4 76 29 77 45                    http://www-frec.bull.fr
fax : +33 (0)4 76 29 77 70                 mailto:Xavier.Bru@bull.net
addr: BULL, 1 rue de Provence, BP 208, 38432 Echirolles Cedex, FRANCE
_____________________________________________________________________


Andrew Morton writes:
 > 
 > On reflection, I think your approach is best - there is (some) value in being
 > able to inherit a /dev/mem mapping across fork(), and ceasing to do so may
 > break applications.
 > 
 > I recast your patch a little.  Saves a few instructions and prevents a bogus
 > uninitialised variable warning.
 > 
 > 
 > diff -puN mm/memory.c~copy_page_range-invalid-page-fix mm/memory.c
 > --- 25/mm/memory.c~copy_page_range-invalid-page-fix	2003-03-01 17:26:03.000000000 -0800
 > +++ 25-akpm/mm/memory.c	2003-03-01 17:26:29.000000000 -0800
 > @@ -286,9 +286,11 @@ skip_copy_pte_range:
 >  					goto cont_copy_pte_range_noset;
 >  				}
 >  				pfn = pte_pfn(pte);
 > +				if (!pfn_valid(pfn)) {
 > +					set_pte(dst_pte, pte);
 > +					goto cont_copy_pte_range_noset;
 > +				}
 >  				page = pfn_to_page(pfn);
 > -				if (!pfn_valid(pfn))
 > -					goto cont_copy_pte_range;
 >  				if (PageReserved(page))
 >  					goto cont_copy_pte_range;
 >  
 > 
 > _
 > 
 > Xavier Bru  <Xavier.Bru@bull.net> wrote:
 > >
 > > Works OK for us.
 > > Thanks
 > > 
 > > -- 
 > > 
 > > _____________________________________________________________________
 > >  
 > > Xavier BRU                 BULL ISD/R&D/INTEL office:     FREC B1-422
 > > tel : +33 (0)4 76 29 77 45                    http://www-frec.bull.fr
 > > fax : +33 (0)4 76 29 77 70                 mailto:Xavier.Bru@bull.net
 > > addr: BULL, 1 rue de Provence, BP 208, 38432 Echirolles Cedex, FRANCE
 > > _____________________________________________________________________
 > > 
 > > Andrew Morton writes:
 > >  > William Lee Irwin III <wli@holomorphy.com> wrote:
 > >  > >
 > >  > > On Tue, Feb 18, 2003 at 09:46:31AM +0100, Xavier Bru wrote:
 > >  > > > Thanks for your answers.
 > >  > > > You are right, we do not need a page structure for mapping /dev/mem in 
 > >  > > > IO space (I am not a mm expert :-).
 > >  > > > Here after a possible patch that allows the Xserver running on the NUMA
 > >  > > > platform. (We had the same problem on Azusa).
 > >  > > 
 > >  > > Looks perfectly reasonable to me. Other discontig/NUMA implementations
 > >  > > are likely to run into the same issue.
 > >  > > 
 > >  > > You might want to send this on to lkml and cc: akpm, he's pretty much
 > >  > > the final destination wrt. VM patches and 2.5.
 > >  > > 
 > >  > 
 > >  > I'd prefer to do this at the vma level if poss.  Given that mmap_mem()
 > >  > has set VM_RESERVED against the VMA, it should be sufficient to do
 > >  > 
 > >  > diff -puN mm/memory.c~a mm/memory.c
 > >  > --- 25/mm/memory.c~a	2003-02-18 02:40:01.000000000 -0800
 > >  > +++ 25-akpm/mm/memory.c	2003-02-18 02:40:18.000000000 -0800
 > >  > @@ -211,6 +211,9 @@ int copy_page_range(struct mm_struct *ds
 > >  >  	unsigned long cow;
 > >  >  	struct pte_chain *pte_chain = NULL;
 > >  >  
 > >  > +	if (vma->vm_flags & (VM_RESERVED|VM_IO))
 > >  > +		return 0;
 > >  > +
 > >  >  	if (is_vm_hugetlb_page(vma))
 > >  >  		return copy_hugetlb_page_range(dst, src, vma);
 > >  >  
 > >  > diff -puN fs/hugetlbfs/inode.c~a fs/hugetlbfs/inode.c
 > >  > --- 25/fs/hugetlbfs/inode.c~a	2003-02-18 02:42:47.000000000 -0800
 > >  > +++ 25-akpm/fs/hugetlbfs/inode.c	2003-02-18 02:42:55.000000000 -0800
 > >  > @@ -63,7 +63,7 @@ static int hugetlbfs_file_mmap(struct fi
 > >  >  	down(&inode->i_sem);
 > >  >  
 > >  >  	UPDATE_ATIME(inode);
 > >  > -	vma->vm_flags |= VM_HUGETLB | VM_RESERVED;
 > >  > +	vma->vm_flags |= VM_HUGETLB;
 > >  >  	vma->vm_ops = &hugetlb_vm_ops;
 > >  >  	ret = hugetlb_prefault(mapping, vma);
 > >  >  	len = (loff_t)(vma->vm_end - vma->vm_start) +
 > >  > 
 > >  > _
 > >  >  
 > >  > 
 > >  > The semantics (and usage!) of the VM_foo flags are rather vague.  It needs
 > >  > definiton, and an audit.
 > >  > 
 > > 
 > > 
 > > -------------------------------------------------------
 > > This sf.net email is sponsored by:ThinkGeek
 > > Welcome to geek heaven.
 > > http://thinkgeek.com/sf
 > > _______________________________________________
 > > Discontig-devel mailing list
 > > Discontig-devel@lists.sourceforge.net
 > > https://lists.sourceforge.net/lists/listinfo/discontig-devel


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2003-03-03 15:37 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2003-02-18 10:43 [Discontig-devel] Re: [Linux-ia64] Re: 2.5.59 & mmap_sem Andrew Morton
2003-02-24 10:14 ` Xavier Bru
2003-03-02  1:32 ` Andrew Morton
2003-03-03 15:37 ` Xavier Bru

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox