Closed Bug 34920 Opened 25 years ago Closed 25 years ago

Provide an implementation of PR_AcceptRead, PR_TransmitFile, and PR_SendFile that can be used by I/O layer implementers

Categories

(NSPR :: NSPR, enhancement, P3)

enhancement

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: wtc, Assigned: wtc)

References

Details

NSPR defines three I/O methods, PR_AcceptRead, PR_TransmitFile, and PR_SendFile, that are intended to exploit fast system calls on some platforms, for example, the AcceptEx and TransmitFile functions on Windows NT. However, only the bottom layer (i.e., the NSPR layer) can take advantage of these fast system calls; all the upper layers need to emulate these functions. It turns out that the emulation of these functions is the same for most upper I/O layers: - PR_AcceptRead: PR_Accept followed by PR_Recv. - PR_TransmitFile and PR_SendFile: memory-map the file, then write the memory-mapped region to the socket. Instead of having each I/O layer emulate these I/O methods, NSPR should provide the common, emulated implementation of these I/O methods, which the I/O layer implementers can just plug into the PRIOMethods table of their layer. Because normal NSPR clients don't call these emulated functions directly, these emulated functions should be exported as private functions and declared in "private/pprio.h".
Severity: normal → enhancement
Target Milestone: --- → 4.1
Status: NEW → ASSIGNED
Reassigned the bug to myself.
Assignee: larryh → wtc
Status: ASSIGNED → NEW
I added two exported private functions (declared in pprio.h) PR_EmulateAcceptRead and PR_EmulateSendFile that I/O layers can use in their implementation of the acceptread and sendfile methods. PR_EmulateAcceptRead is based on the original internal function _PR_EmulateAcceptRead, and PR_EmulateSendFile is based on the original internal functions _PR_EmulateSendFile and _PR_UnixSendFile. I/O layers still need to define their transmitfile method in terms of the sendfile method like this: static PRInt32 foo_TransmitFile( PRFileDesc *sd, PRFileDesc *fd, const void *headers, PRInt32 hlen, PRTransmitFileFlags flags, PRIntervalTime timeout) { PRSendFileData sfd; sfd.fd = fd; sfd.file_offset = 0; sfd.file_nbytes = 0; sfd.header = headers; sfd.hlen = hlen; sfd.trailer = NULL; sfd.tlen = 0; return foo_SendFile(sd, &sfd, flags, timeout); } /* foo_TransmitFile */ We should make this the default transmitfile method, but I'm worried that it may break binary compatibility so I didn't do that. /cvsroot/mozilla/nsprpub/config/OSF1.mk, revision 3.10 /cvsroot/mozilla/nsprpub/pr/include/md/_hpux.h, revision 3.8 /cvsroot/mozilla/nsprpub/pr/include/private/pprio.h, revision 3.11 /cvsroot/mozilla/nsprpub/pr/include/private/primpl.h, revision 3.41 /cvsroot/mozilla/nsprpub/pr/src/io/priometh.c, revision 3.7 /cvsroot/mozilla/nsprpub/pr/src/io/prsocket.c, revision 3.33 /cvsroot/mozilla/nsprpub/pr/src/md/unix/unix.c, revision 3.35 /cvsroot/mozilla/nsprpub/pr/src/pthreads/ptio.c, revision 3.51
Status: NEW → RESOLVED
Closed: 25 years ago
Resolution: --- → FIXED
Found a bug during testing on NT. Reopened the bug.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
The bug I found has to do with the alignment of the offset argument to PR_MemMap. Right now we assume offset must be aligned on page boundaries. Turns out that this is true on Unix but not true on Windows. The Unix mmap man page says: pa=mmap(addr, len, prot, flags, fildes, off); [...] The off argument is constrained to be aligned and sized according to the value returned by sysconf() when passed _SC_PAGESIZE or _SC_PAGE_SIZE. [...] The implementation performs mapping operations over whole pages. Thus, while the argument len need not meet a size or alignment constraint, the implementation will include, in any mapping operation, any partial page specified by the range [pa, pa + len). (http://www.opengroup.org/onlinepubs/007908799/xsh/mmap.html) The Win32 MapViewOfFile documentation says: dwFileOffsetHigh [in] Specifies the high-order DWORD of the file offset where mapping is to begin. dwFileOffsetLow [in] Specifies the low-order DWORD of the file offset where mapping is to begin. The combination of the high and low offsets must specify an offset within the file that matches the system's memory allocation granularity, or the function fails. That is, the offset must be a multiple of the allocation granularity. Use the GetSystemInfo function, which fills in the members of a SYSTEM_INFO structure, to obtain the system's memory allocation granularity. (http://msdn.microsoft.com/library/psdk/winbase/filemap_8p9h.htm) On NT 4.0, the page size is 4K whereas the allocation granularity is 64K. If offset is a multiple of page size but not a multiple of the allocation granularity, MapViewOfFile fails with ERROR_MAPPED_ALIGNMENT (1132L). We should add a new function PR_GetMemMapAlignment that returns the alignment of the offset argument to PR_MemMap.
Status: REOPENED → ASSIGNED
Depends on: 38996
Fixed PR_EmulateSendFile to use the new PR_GetMemMapAlignment function to get the proper alignment of the offset argument to PR_MemMap. Also, the size argument to PR_CreateFileMap and the offset argument to PR_MemMap are 64-bit, so use 64-bit variables and LL macros. /cvsroot/mozilal/nsprpub/pr/src/io/priometh.c, revision 3.8
Modified PR_EmulateSendFile so that it doesn't depend on the mmap alignment being a power of 2. /cvsroot/mozilla/nsprpub/pr/src/io/priometh.c, revision 3.9
Modified the socket.c test to test PR_EmulateSendFile. /cvsroot/mozilla/nsprpub/pr/tests/socket.c, revision 3.15 Added the new test acceptreademu.c to test PR_EmulateAcceptRead. Marked the bug fixed.
Status: ASSIGNED → RESOLVED
Closed: 25 years ago25 years ago
Resolution: --- → FIXED
I found a bug in PR_EmulateAcceptRead() that is manifested on some 64-bit platforms. I define enum { AMASK = 7 }; and bitwise-AND an address with ~AMASK. If an enum is implemented as an unsigned int (32-bit), ~AMASK is still unsigned, so when extended to 64-bit (0x00000000fffffff8) and bitwise-ANDed with an address, the upper 32 bits of the address are zeroed. This bug won't be noticed if enum is implemented as a signed int, because ~AMASK is signed-extended to be 0xfffffffffffffff8. The fix is to explicitly define AMASK as a PRPtrdiff value: Index: mozilla/nsprpub/pr/src/io/priometh.c =================================================================== RCS file: /cvsroot/mozilla/nsprpub/pr/src/io/priometh.c,v retrieving revision 3.13 diff -u -r3.13 priometh.c --- priometh.c 2000/07/19 22:03:28 3.13 +++ priometh.c 2000/08/10 02:25:20 @@ -307,7 +307,7 @@ if (rv >= 0) { /* copy the new info out where caller can see it */ - enum { AMASK = 7 }; /* mask for alignment of PRNetAddr */ +#define AMASK ((PRPtrdiff)7) /* mask for alignment of PRNetAddr */ PRPtrdiff aligned = (PRPtrdiff)buf + amount + AMASK; *raddr = (PRNetAddr*)(aligned & ~AMASK); memcpy(*raddr, &remote, PR_NETADDR_SIZE(&remote)); I checked in the fix. /cvsroot/mozilla/nsprpub/pr/src/io/priometh.c, revision: 3.14 Larry, this explained the accept, acceptread, and acceptreademu test failures on 64-bit HP-UX and Solaris.
I said: > Larry, this explained the accept, acceptread, and acceptreademu > test failures on 64-bit HP-UX and Solaris. It should be just 64-bit HP-UX. The failure of those tests on 64-bit Solaris was caused by something else.
Good bug shooting! The Solaris problems, I believe, were a procedural error in running the tests concurrently. ... I'll mergeout and re-test.
You need to log in before you can comment on or make changes to this bug.