Date: Fri, 12 Apr 2002 18:17:21 -0500 (CDT) From: Alan Cox <alc@cs.rice.edu> To: Thomas Moestl <tmm@FreeBSD.org> Cc: <cvs-committers@FreeBSD.org>, <cvs-all@FreeBSD.org> Subject: Re: cvs commit: src/sys/kern sys_pipe.c Message-ID: <Pine.GSO.4.33.0204121754130.11123-100000@cs.rice.edu> In-Reply-To: <20020412210248.GB290@crow.dom2ip.de>
next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, 12 Apr 2002, Thomas Moestl wrote: > On Fri, 2002/04/12 at 15:42:37 -0500, Alan Cox wrote: > > > > > > On Fri, 12 Apr 2002, Thomas Moestl wrote: > > > > > tmm 2002/04/12 12:38:41 PDT > > > > > > Modified files: > > > sys/kern sys_pipe.c > > > Log: > > > Do not use pmap_kextract() to find out the physical address of a user > > > belong to a user virtual address; while this happens to work on some > > > architectures, it can't on sparc64, since user and kernel virtual > > > address spaces overlap there (the distinction between them is done via > > > separate address space identifiers). > > > > > > > Why not pmap_extract() on the map->pmap? pmap_extract() is the equivalent > > of pmap_kextract() for ordinary pmap's, i.e., not the kernel pmap. > > Since a pmap module is allowed to drop a mapping at any time, this > would unnecessarily open a race between the vm_fault_quick() and the > pmap_extract() which might bite us later SMPng-wise. > The same is in a way true for the code I committed (a pageout may > happen in between), but that should be easy to fix when it is required > (by just replacing the vm_fault_quick()/vm_page_wire() sequence with a > vm_fault_user_wire()). With Giant out of the picture, I agree that there is a race condition in both the old code and in your new code. The same race condition also exists in vmapbuf() that's used by both the physio and aio code. My concern is that vm_map_lookup(), etc. are not cheap operations ... to the point that copying the data would become cheaper in some cases. I think in the short term you should use pmap_extract() to resolve the issue that the address is in user-space and not kernel-space. This is without question a "bug" in the old code. pmap_kextract() is faster, so that's almost certainly why it's been used. And, in the long term, we need to find a way to make the combination of vm_fault_quick()/pmap_extract() atomic. Also, at a low-level, your new code has a bug: You assume that the page will be found in the top-level vm object. If the sending process has forked between the time that the buffer was initialized and sent down the pipe, the pages may not be in the top-level vm object. I'm afraid that you need to recur on the shadow chain. Alan To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe cvs-all" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?Pine.GSO.4.33.0204121754130.11123-100000>