March 2001 - Wine-Devel - list.winehq.org

Palette managing

by kenon＠go2.pl

Hi In file windows/painting.c in function RealizePalette16 I found that line 1713: if (hWnd) SendMessageA( HWND_BROADCAST, WM_PALETTECHANGED, hWnd, 0L); broke some applications . When I comment it out - every things work well, but when I leave it some child-windows are displayed in improper place. I think this message should be sent only when used entry in COLOR_sysPal is replaced by new one (function X11DRV_PALETTE_SetMapping in line 907 when index == -1). Maciek Kaliszewski

March 10, 2001

Re: Edit control notification fix

by Andreas Mohr

On Fri, Mar 09, 2001 at 03:52:56PM +0800, Dmitry Timoshkov wrote: > Hello. > > Andreas Mohr privately reported that some old 16-bit installer > started to not update its edit control after my patch entitled > "Do not notify parent of the Edit control on WM_SETTEXT when Edit > is part of the ComboBox." Investigation has shown that installer > has its main dialog window defined in resources, and DIALOG_GetControl16() > gets edit control with style ES_COMBO (0x200) defined there. > My first attempt to fix the trouble was to simply mask out not > allowed styles in the DIALOG_GetControl16(): i.e. for edit control > do style &= ~ES_COMBO. That hack cured the application, but Andreas Well, that's not exactly true. You did not do it "for edit control". You did it for *all* controls in DIALOG_GetControl16() (at least in the patch you sent me). That's why I objected. I just had to :) > didn't like that approach. After some thinking, I added yet another > test into my "Edit Control Test Suit". And here is the result of it. > > Changelog: > Dmitry Timoshkov <dmitry(a)codeweavers.com> > Do not send notification to parent of an edit control, > if parent is a child window. Remove old and not correct patch. OK. I can't really judge whether that's correct, as I'm not too much into these kinds of things. Hopefully it's correct :) Thanks again, Andreas Mohr

March 9, 2001

Re: Edit control notification fix

by Dmitry Timoshkov

"Andreas Mohr" <a.mohr(a)mailto.de> wrote: > > That hack cured the application, but Andreas > Well, that's not exactly true. > You did not do it "for edit control". You did it for *all* controls > in DIALOG_GetControl16() (at least in the patch you sent me). > That's why I objected. I just had to :) Sure :-) [...] > OK. I can't really judge whether that's correct, as I'm not too much into > these kinds of things. > > Hopefully it's correct :) Well, you can always write your own test app and prove or contest my patch :-) It is really not so hard as it seemed to be... First, using spy++ I discovered that ALL edit controls of that installer have 0x200 style under win2000: therefore style got not corrected or masked upon dialog creation. Second, I took some thought, and wrote test app. My test application sends WM_SETTEXT to the edit control (parent is dialog), then reparents that edit to button (!) in that dialog, makes edit size to be equal new parent client size, brings edit to front and then sends WM_SETTEXT to the edit again. Also I did editing of the text in that edit before and after change of parent, did change focus to and from edit using keyboard and mouse, did copy and paste... All messages to and from dialog and all its controls were spyed and analyzed. That's all.

March 9, 2001

Re: Speeding up wineserver syncronization objects with shared memory

by David Howells

> OK, this has apparently changed in kernel 2.4; in kernel 2.2 block > size alignment is enough. That's unfortunate, but I guess it was > necessary to support large files or memory sizes. Then yes, in 2.4 > your patch will be more useful. As far as I know, it's the case on most Unices, though it maybe not be so on older versions of Linux. It's part of the definition of mmap() too, I think. David

March 8, 2001

Re: Speeding up wineserver syncronization objects with shared memory

by Alexandre Julliard

David Howells <dhowells(a)cambridge.redhat.com> writes: > Not so... The file offset must also be page aligned! OK, this has apparently changed in kernel 2.4; in kernel 2.2 block size alignment is enough. That's unfortunate, but I guess it was necessary to support large files or memory sizes. Then yes, in 2.4 your patch will be more useful. -- Alexandre Julliard julliard(a)winehq.com

March 8, 2001

Re: Speeding up wineserver syncronization objects with shared memory

by David Howells

> I'll have to write a small program to collect some statistics:-) Okay, I did a quick and dirty program to do that. It looked through the following products that I've got installed: Win2000 sp1 MSDEV 98 MS Office 2000 Using the following command: find /fs/win2000/WINNT \ "/fs/win2000/Program Files/Microsoft Visual Studio" \ "/fs/win2000/Program Files/Microsoft Office" -type f | ./scandll And came up with the following statistics: files accessed : 3985 valid PE images : 1752 - libraries : 1280 - badly VM aligned : 20 - standard image base : 42 - specific image base : 1218 - file align by sector: 976 - file align by page : 284 - executables : 472 - badly VM aligned : 133 - standard image base : 70 - specific image base : 269 - file align by sector: 287 - file align by page : 52 I've attached the program I used. As you can see, most DLLs actually have their image base set manually to avoid conflicts. Also a lot of EXEs have it set too... presumably so that they can also be used as DLLs. However! the majority of DLLs and EXEs are sector-aligned (512) in the files, not page-aligned (4096). David /* scandll.c: scan DLLs and EXEs * * Copyright (c) 2001 David Howells (dhowells(a)redhat.com) */ #include <stdio.h> #include <string.h> #include <unistd.h> #include <fcntl.h> #include <errno.h> #include <linux/types.h> typedef __u8 BOOL, BYTE; typedef __u16 WORD; typedef __u32 DWORD, UINT; #define PAGE_SIZE 4096 #define _H_229A4E44_C7CC_11D4_8F92_0000C0005742 /* skip section.h */ #include "pefile.h" IMAGE_SECTION_HEADER sechdr; IMAGE_DOS_HEADER imghdr; IMAGE_NT_HEADERS nthdr; int files; int valid_image; int dlls; int exes; int bad_vm_align[2]; int file_align_sector[2]; int file_align_page[2]; int normal_image_base[2]; int specific_image_base[2]; /*****************************************************************************/ /* * */ int main(int argc, char **argv) { char fname[1024], *cp; int fd = -1, tmp, isexe; while (close(fd), fd = -1, fgets(fname,sizeof(fname),stdin) ) { cp = strpbrk(fname,"\r\n"); if (cp) *cp = 0; fd = open(fname,O_RDONLY); if (!fd) { if (errno==EISDIR) continue; perror("open"); return 1; } /* get the DOS image header */ tmp = read(fd,&imghdr,sizeof(imghdr)); if (tmp<0) { if (errno==EISDIR) continue; perror("read doshdr"); return 1; } if (tmp<sizeof(imghdr)) continue; files++; if (imghdr.e_magic != IMAGE_DOS_SIGNATURE) continue; /* get the NT image header */ lseek(fd,imghdr.e_lfanew,SEEK_SET); tmp = read(fd,&nthdr,sizeof(nthdr)); if (tmp<0) { perror("read nthdr"); return 1; } if (tmp<sizeof(nthdr)) continue; if (nthdr.Signature != IMAGE_NT_SIGNATURE || nthdr.FileHeader.Machine != IMAGE_FILE_MACHINE_I386 ) continue; if (!(nthdr.FileHeader.Characteristics & IMAGE_FILE_EXECUTABLE_IMAGE)) continue; if (nthdr.OptionalHeader.Magic != IMAGE_NT_OPTIONAL_HDR_MAGIC) continue; /* okay... it's a valid PE image */ valid_image++; /* is it an EXE file? */ if (nthdr.FileHeader.Characteristics & IMAGE_FILE_DLL) { dlls++; isexe = 0; } else { exes++; isexe = 1; } /* check the VM alignment */ if (!nthdr.OptionalHeader.SectionAlignment || nthdr.OptionalHeader.SectionAlignment&(PAGE_SIZE-1) ) { bad_vm_align[isexe]++; continue; } /* check the file alignment */ if (!nthdr.OptionalHeader.FileAlignment || nthdr.OptionalHeader.FileAlignment&(PAGE_SIZE-1) ) { file_align_sector[isexe]++; } else { file_align_page[isexe]++; } /* consider the image base */ printf("%08x %s\n",nthdr.OptionalHeader.ImageBase,fname); if (nthdr.OptionalHeader.ImageBase==0x10000000 || nthdr.OptionalHeader.ImageBase==0x00400000 ) { normal_image_base[isexe]++; } else { specific_image_base[isexe]++; } } printf("files accessed : %d\n",files); printf("valid PE images : %d\n",valid_image); printf("- libraries : %d\n",dlls); printf(" - badly VM aligned : %d\n",bad_vm_align[0]); printf(" - standard image base : %d\n",normal_image_base[0]); printf(" - specific image base : %d\n",specific_image_base[0]); printf(" - file align by sector: %d\n",file_align_sector[0]); printf(" - file align by page : %d\n",file_align_page[0]); printf("- executables : %d\n",exes); printf(" - badly VM aligned : %d\n",bad_vm_align[1]); printf(" - standard image base : %d\n",normal_image_base[1]); printf(" - specific image base : %d\n",specific_image_base[1]); printf(" - file align by sector: %d\n",file_align_sector[1]); printf(" - file align by page : %d\n",file_align_page[1]); return 0; } /* end main() */ \n",exes); printf(" - badly VM aligned : %d\n",bad_vm_align[1]); printf(" - standard image base : %d\n",normal_image_base[1]); printf(" - specific image base : %d\n",specific_image_base[1]); printf(" - file align by sector: %d\n",file_align_sector[1]); printf(" - file align by page : %d\n",file_align_page[1]); return 0; } /* end main() */

March 8, 2001

Re: Speeding up wineserver syncronization objects with shared memory

by David Howells

> Page alignment is needed for the address in memory, not for the offset > inside the file on disk; since section virtual addresses in PE files > are always page-aligned the memory address is never a problem. The > only problem comes from the alignment of the data inside the PE file, > and this is where we only need block-size alignment to make mmap > possible. Not so... The file offset must also be page aligned! Try the following program: #include <stdio.h> #include <unistd.h> #include <fcntl.h> #include <sys/mman.h> int main(int argc, char **argv) { void *addr; int fd; fd = open("/bin/sh",O_RDONLY); if (fd<0) { perror("mmap"); return 1; } addr = mmap(NULL,512,PROT_READ,MAP_PRIVATE,fd,argc==1?512:4096); if (addr==MAP_FAILED) { perror("mmap"); return 1; } return 0; } If you don't run it with an argument, it fails (Invalid Argument), and if you do give it an argument, it works. If you look at the kernel: [arch/i386/kernel/sys_i386.c] asmlinkage int old_mmap(struct mmap_arg_struct *arg) { struct mmap_arg_struct a; int err = -EFAULT; if (copy_from_user(&a, arg, sizeof(a))) goto out; err = -EINVAL; if (a.offset & ~PAGE_MASK) goto out; err = do_mmap2(a.addr, a.len, a.prot, a.flags, a.fd, a.offset >> PAGE_SHIFT); out: return err; } Note that "a.offset" (the file offset) is checked for page alignment. Note also that the internal kernel routine do_mmap2() deals with the offset in terms of pages, not bytes. David

March 8, 2001

Re: Speeding up wineserver syncronization objects with shared memory

by Alexandre Julliard

Gavriel State <gav(a)transgaming.com> writes: > Looking at the kernel sources, it appears to be page-alignment, not > filesystem block size that mmap needs to match. Rereading old wine-devel > archives, it appears that the notion that the filesystem block size is > involved is either from a very old kernel, or a misreading of some kernel > comments. Page alignment is needed for the address in memory, not for the offset inside the file on disk; since section virtual addresses in PE files are always page-aligned the memory address is never a problem. The only problem comes from the alignment of the data inside the PE file, and this is where we only need block-size alignment to make mmap possible. -- Alexandre Julliard julliard(a)winehq.com

March 8, 2001

Re: Speeding up wineserver syncronization objects with shared memory

by Gavriel State

Alexandre Julliard wrote: > > > As far as I've observed (I've got Win2000 available), most Windows DLL's have > > 512-byte (sector) alignment internally, _not_ 4096-byte (page) alignment for > > the sections. This means that the separate sections can't be mmap'd (or else > > they'd lose their required relative relationships): > > Actually the file alignment doesn't need to be 4096, it needs to match > the filesystem block size. On a FAT filesystem the block size is 512 > so Linux will happily mmap every section. On a 1k-block ext2 fs it > will be able to mmap about 50% of them. Looking at the kernel sources, it appears to be page-alignment, not filesystem block size that mmap needs to match. Rereading old wine-devel archives, it appears that the notion that the filesystem block size is involved is either from a very old kernel, or a misreading of some kernel comments. From linux/mm/mmap.c: unsigned long do_mmap(struct file * file, unsigned long addr, unsigned long len, unsigned long prot, unsigned long flags, unsigned long off) . . . /* Obtain the address to map to. we verify (or select) it and ensure * that it represents a valid section of the address space. */ if (flags & MAP_FIXED) { if (addr & ~PAGE_MASK) return -EINVAL; } else { > > Also, since DLLs and EXEs are not compiled as PIC (the MSDEV compiler not > > having such an option as far as I can recall), the fixup tables usually seem > > to apply to just about every page in the code section. > > Only if the dll cannot be loaded at the preferred address, which > shouldn't happen too often. I'm not saying your patch is useless, but > I doubt the gain is as large as you seem to think. It's worthwhile to point out that while many Win2K system DLLs may be 512-byte aligned, almost any app written in the past several years is probably using 4k alignment, since that was made the default with MSVC 5.0. It makes one wonders why MS would be using old compilers for the OS... -Gav -- Gavriel State, CEO TransGaming Technologies Inc. http://www.transgaming.com gav(a)transgaming.com

March 7, 2001

Re: Speeding up wineserver syncronization objects with shared memory

by Alexandre Julliard

David Howells <dhowells(a)cambridge.redhat.com> writes: > So this saves you the cost of the fd transfer net packet. Though you still > have to do the two context switches, which is my main contention. I suspect we are doing more than two switches (though I haven't proved it), which is why I think there is a margin for improvement. You'll obviously always have the context switch cost unless everything is in the kernel. > True, but I'd have thought that the context switches involved are still a cost > you can't get rid of so easily. Out of interest, how do you plan on doing the > locking stuff for Read/WriteFile? Cache it locally? It is unfortunate, but you > can't really make use of UNIX file locking, since this is mostly advisory and > as such doesn't actively stop read/write calls. Yes, we'll need to store the locks in the server and check them before each read/write (and probably also release them afterwards if necessary). There may be some optimisations possible, but we should probably do it the easy way first. > Seriously, though, whilst this'd be a lot easier in many ways (and it would > allow you to avoid the context-switch penalties), you wouldn't be able to take > full advantage of the available support in the kernel, which is more capable > than the standard UNIX userspace API suggests. I don't see why. I'm not suggesting keeping the current socket stuff, just reusing the structures. So basically instead of passing the address of the stack arguments (which is really ugly IMO) to your ioctl, you pass one of the server request structures. This allows your changes to be localized to wine_server_call and doesn't require changing any of the routines that make server calls. Obviously you'd need some more changes for a few calls like ReadFile/WriteFile, but most operations could switch to your mechanism without needing any change. You simply cannot require people to recompile all of Wine to use your module. > > I still think that it should be possible to improve that by a small > > kernel hack. It will never be as fast as doing everything in the > > kernel of course, but it may just be fast enough to avoid the need to > > reimplement the whole server. > > If you want to suggest exactly what you'd like to see as a hack... I don't know exactly, there are many ways of doing it; you can have a specialized fifo, a network protocol, an ioctl, etc. Basically any mechanism that ensures that we do the strict mimimum number of context switches and schedule() calls for a server call. And probably also a way to transfer chunks of memory from the client address space so that we don't need the shared memory area. > As far as I've observed (I've got Win2000 available), most Windows DLL's have > 512-byte (sector) alignment internally, _not_ 4096-byte (page) alignment for > the sections. This means that the separate sections can't be mmap'd (or else > they'd lose their required relative relationships): Actually the file alignment doesn't need to be 4096, it needs to match the filesystem block size. On a FAT filesystem the block size is 512 so Linux will happily mmap every section. On a 1k-block ext2 fs it will be able to mmap about 50% of them. > Also, since DLLs and EXEs are not compiled as PIC (the MSDEV compiler not > having such an option as far as I can recall), the fixup tables usually seem > to apply to just about every page in the code section. Only if the dll cannot be loaded at the preferred address, which shouldn't happen too often. I'm not saying your patch is useless, but I doubt the gain is as large as you seem to think. -- Alexandre Julliard julliard(a)winehq.com

March 7, 2001