Linus,
The raid5 xor checksums use MMX/SSE state and are not preempt-safe.
Attached patch disables preemption in FPU_SAVE and XMMS_SAVE and
restores it in FPU_RESTORE and XMMS_RESTORE - preventing preemption
while in fp mode.
Please, apply.
Robert Love
diff -urN linux-2.5.31/include/asm-i386/xor.h linux/include/asm-i386/xor.h
--- linux-2.5.31/include/asm-i386/xor.h Sat Aug 10 21:41:20 2002
+++ linux/include/asm-i386/xor.h Sat Aug 24 20:14:43 2002
@@ -20,6 +20,7 @@
#define FPU_SAVE \
do { \
+ preempt_disable(); \
if (!test_thread_flag(TIF_USEDFPU)) \
__asm__ __volatile__ (" clts;\n"); \
__asm__ __volatile__ ("fsave %0; fwait": "=m"(fpu_save[0])); \
@@ -30,6 +31,7 @@
__asm__ __volatile__ ("frstor %0": : "m"(fpu_save[0])); \
if (!test_thread_flag(TIF_USEDFPU)) \
stts(); \
+ preempt_enable(); \
} while (0)
#define LD(x,y) " movq 8*("#x")(%1), %%mm"#y" ;\n"
@@ -543,6 +545,7 @@
*/
#define XMMS_SAVE \
+ preempt_disable(); \
__asm__ __volatile__ ( \
"movl %%cr0,%0 ;\n\t" \
"clts ;\n\t" \
@@ -564,7 +567,8 @@
"movl %0,%%cr0 ;\n\t" \
: \
: "r" (cr0), "r" (xmm_save) \
- : "memory")
+ : "memory") \
+ preempt_enable();
#define ALIGN16 __attribute__((aligned(16)))
diff -urN linux-2.5.31/include/asm-x86_64/xor.h linux/include/asm-x86_64/xor.h
--- linux-2.5.31/include/asm-x86_64/xor.h Sat Aug 10 21:41:23 2002
+++ linux/include/asm-x86_64/xor.h Sat Aug 24 20:05:41 2002
@@ -38,7 +38,8 @@
/* Doesn't use gcc to save the XMM registers, because there is no easy way to
tell it to do a clts before the register saving. */
#define XMMS_SAVE \
- asm volatile ( \
+ preempt_disable(); \
+ asm volatile ( \
"movq %%cr0,%0 ;\n\t" \
"clts ;\n\t" \
"movups %%xmm0,(%1) ;\n\t" \
@@ -59,7 +60,8 @@
"movq %0,%%cr0 ;\n\t" \
: \
: "r" (cr0), "r" (xmm_save) \
- : "memory")
+ : "memory") \
+ preempt_enable();
#define OFFS(x) "16*("#x")"
#define PF_OFFS(x) "256+16*("#x")"
Hi,
On 26 Aug 2002, Robert Love wrote:
> @@ -543,6 +545,7 @@
> #define XMMS_SAVE \
> + preempt_disable(); \
> __asm__ __volatile__ ( \
> "movl %%cr0,%0 ;\n\t" \
> "clts ;\n\t" \
> @@ -564,7 +567,8 @@
> "movl %0,%%cr0 ;\n\t" \
> : \
> : "r" (cr0), "r" (xmm_save) \
> - : "memory")
> + : "memory") \
> + preempt_enable();
> @@ -38,7 +38,8 @@
> #define XMMS_SAVE \
> - asm volatile ( \
> + preempt_disable(); \
> + asm volatile ( \
> "movq %%cr0,%0 ;\n\t" \
> "clts ;\n\t" \
> "movups %%xmm0,(%1) ;\n\t" \
> @@ -59,7 +60,8 @@
> "movq %0,%%cr0 ;\n\t" \
> : \
> : "r" (cr0), "r" (xmm_save) \
> - : "memory")
> + : "memory") \
> + preempt_enable();
These will suck when on if, I guess... Anyway, will this compile at all?
There seems no semicolon after the asm volatile ()
Thunder
--
--./../...-/. -.--/---/..-/.-./..././.-../..-. .---/..-/.../- .-
--/../-./..-/-/./--..-- ../.----./.-../.-.. --./../...-/. -.--/---/..-
.- -/---/--/---/.-./.-./---/.--/.-.-.-
--./.-/-.../.-./.././.-../.-.-.-
On Mon, 2002-08-26 at 17:09, Thunder from the hill wrote:
> These will suck when on if, I guess...
hm?
> Anyway, will this compile at all? There seems no semicolon after the
> asm volatile ()
Ah yes, curses. Thanks.
Robert Love
diff -urN linux-2.5.31/include/asm-i386/xor.h linux/include/asm-i386/xor.h
--- linux-2.5.31/include/asm-i386/xor.h Sat Aug 10 21:41:20 2002
+++ linux/include/asm-i386/xor.h Sat Aug 24 20:14:43 2002
@@ -20,6 +20,7 @@
#define FPU_SAVE \
do { \
+ preempt_disable(); \
if (!test_thread_flag(TIF_USEDFPU)) \
__asm__ __volatile__ (" clts;\n"); \
__asm__ __volatile__ ("fsave %0; fwait": "=m"(fpu_save[0])); \
@@ -30,6 +31,7 @@
__asm__ __volatile__ ("frstor %0": : "m"(fpu_save[0])); \
if (!test_thread_flag(TIF_USEDFPU)) \
stts(); \
+ preempt_enable(); \
} while (0)
#define LD(x,y) " movq 8*("#x")(%1), %%mm"#y" ;\n"
@@ -543,6 +545,7 @@
*/
#define XMMS_SAVE \
+ preempt_disable(); \
__asm__ __volatile__ ( \
"movl %%cr0,%0 ;\n\t" \
"clts ;\n\t" \
@@ -564,7 +567,8 @@
"movl %0,%%cr0 ;\n\t" \
: \
: "r" (cr0), "r" (xmm_save) \
- : "memory")
+ : "memory"); \
+ preempt_enable()
#define ALIGN16 __attribute__((aligned(16)))
diff -urN linux-2.5.31/include/asm-x86_64/xor.h linux/include/asm-x86_64/xor.h
--- linux-2.5.31/include/asm-x86_64/xor.h Sat Aug 10 21:41:23 2002
+++ linux/include/asm-x86_64/xor.h Sat Aug 24 20:05:41 2002
@@ -38,7 +38,8 @@
/* Doesn't use gcc to save the XMM registers, because there is no easy way to
tell it to do a clts before the register saving. */
#define XMMS_SAVE \
- asm volatile ( \
+ preempt_disable(); \
+ asm volatile ( \
"movq %%cr0,%0 ;\n\t" \
"clts ;\n\t" \
"movups %%xmm0,(%1) ;\n\t" \
@@ -59,7 +60,8 @@
"movq %0,%%cr0 ;\n\t" \
: \
: "r" (cr0), "r" (xmm_save) \
- : "memory")
+ : "memory"); \
+ preempt_enable()
#define OFFS(x) "16*("#x")"
#define PF_OFFS(x) "256+16*("#x")"
Robert Love wrote:
> Linus,
>
> The raid5 xor checksums use MMX/SSE state and are not preempt-safe.
>
> Attached patch disables preemption in FPU_SAVE and XMMS_SAVE and
> restores it in FPU_RESTORE and XMMS_RESTORE - preventing preemption
> while in fp mode.
>
> Please, apply.
>
> Robert Love
Use kernel_fpu_begin() and kernel_fpu_end() instead of reinventing the
wheel. The current code is broken wrt SSE as well.
--
Brian Gerst
Robert Love <[email protected]> said:
> On Mon, 2002-08-26 at 17:09, Thunder from the hill wrote:
>
> > These will suck when on if, I guess...
>
> hm?
>
> > Anyway, will this compile at all? There seems no semicolon after the
> > asm volatile ()
>
> Ah yes, curses. Thanks.
Also, kindly place "do { ... } while(0)" around XMMS_SAVE, if only for
symmetry.
--
Dr. Horst H. von Brand User #22616 counter.li.org
Departamento de Informatica Fono: +56 32 654431
Universidad Tecnica Federico Santa Maria +56 32 654239
Casilla 110-V, Valparaiso, Chile Fax: +56 32 797513
Hi,
On Mon, 26 Aug 2002, Horst von Brand wrote:
> Also, kindly place "do { ... } while(0)" around XMMS_SAVE, if only for
> symmetry.
We've already had that talked about. Will appear in one of the following
patches.
Thunder
--
--./../...-/. -.--/---/..-/.-./..././.-../..-. .---/..-/.../- .-
--/../-./..-/-/./--..-- ../.----./.-../.-.. --./../...-/. -.--/---/..-
.- -/---/--/---/.-./.-./---/.--/.-.-.-
--./.-/-.../.-./.././.-../.-.-.-
I need to close all the none std[in|out|err] open fd's.
I've been told to do it like so:
{
int i;
for (i=3; i<OPEN_MAX; i++)
close(i);
}
This is very slow, plus I have discovered that I can have open fd's with
values greater than OPEN_MAX.
I thought about getting the max fd from rlimit, but that doesn't work
either. Say I have a rlimit of 1024 open fd's, and I open numbers 3
thru 1023, then I close 3 thru 1022, then I set the rlimit down to
16. rlimit then returns 16, but the largest open fd is still 1023.
So that doesn't work.
And I still have the problem that looping between 3 and whatever I pick
as the top and calling close on each in turn is very slow.
So what's the "right way" to do it?
I would *love* for there to be an ioctl or some syscall that I could
pass a pointer to an int and a pointer to an int array, and it would
come back telling me how many open fd's I've got, and fill in the
array with those fd's.
--
Mark Atwood | Well done is better than well said.
[email protected] |
http://www.pobox.com/~mra
tricky. You can use /proc/<getpid>/fd, and close all
handles listed here, but this has some caveats:
it's _very_ slow if you have many open files.
it's not portable.
it's not safe if you have a thread/signal handler running.
i never heard of a right way to do this.
-alex
int close_all_fd()
{
char fdpath[PATH_MAX];
DIR * dp;
struct dirent * de;
int fd;
sprintf(fdpath, "/proc/%d/fd", getpid());
dp = opendir(fdpath);
if ( !dp )
return -errno;
while ( (de = readdir(dp)) )
{
if ( !strcmp(de->d_name, ".") || !strcmp(de->d_name, "..") )
continue;
fd = strtol(de->d_name, 0, 10);
if ( fd == dirfd(dp) || fd == 0 || fd == 1 || fd == 2 )
continue;
if ( close(fd) < 0 )
fprintf(stderr, "%s: %s\n", de->d_name, strerror(errno));
}
closedir(dp);
return 0;
}
On Tue, Aug 27, 2002 at 08:34:04AM -0700, Mark Atwood wrote:
>
> I need to close all the none std[in|out|err] open fd's.
>
> I've been told to do it like so:
>
> {
> int i;
> for (i=3; i<OPEN_MAX; i++)
> close(i);
> }
>
> This is very slow, plus I have discovered that I can have open fd's with
> values greater than OPEN_MAX.
>
> I thought about getting the max fd from rlimit, but that doesn't work
> either. Say I have a rlimit of 1024 open fd's, and I open numbers 3
> thru 1023, then I close 3 thru 1022, then I set the rlimit down to
> 16. rlimit then returns 16, but the largest open fd is still 1023.
>
> So that doesn't work.
>
> And I still have the problem that looping between 3 and whatever I pick
> as the top and calling close on each in turn is very slow.
>
> So what's the "right way" to do it?
>
> I would *love* for there to be an ioctl or some syscall that I could
> pass a pointer to an int and a pointer to an int array, and it would
> come back telling me how many open fd's I've got, and fill in the
> array with those fd's.
>
> --
> Mark Atwood | Well done is better than well said.
> [email protected] |
> http://www.pobox.com/~mra
> -
On Tue, Aug 27, 2002 at 06:08:42PM +0200, Alex Riesen wrote:
> tricky. You can use /proc/<getpid>/fd, and close all
> handles listed here, but this has some caveats:
> it's _very_ slow if you have many open files.
> it's not portable.
> it's not safe if you have a thread/signal handler running.
>
> i never heard of a right way to do this.
>
> -alex
>
> int close_all_fd()
> {
> char fdpath[PATH_MAX];
> DIR * dp;
> struct dirent * de;
> int fd;
>
> sprintf(fdpath, "/proc/%d/fd", getpid());
> dp = opendir(fdpath);
This can just be:
dp = opendir("/proc/self/fd/");
then you don't need fdpath.
You can use sigprocmask() if you're worried about signals coming
in during this operation.
miket
Hi Mark :)
>So what's the "right way" to do it?
AFAIK, the for loop, with getdtablesize() instead of 'OPEN_MAX'.
I do it that way, but I don't really know if it is the 'right way'(tm).
>I would *love* for there to be an ioctl or some syscall that I could
>pass a pointer to an int and a pointer to an int array, and it would
>come back telling me how many open fd's I've got, and fill in the
>array with those fd's.
The array should be allocated by the kernel, or the syscall won't
work as expected ;) If you have 2000 fd open and the array whose
address you pass to the ioctl has an smaller size... Anyway, you can
call the ioctl a few times ;)
Your proposal seems reasonable (unless there is any other way of
doing this portably), but portability will be an issue...
Ra?l
On Tue, Aug 27, 2002 at 02:26:38PM -0700, Mike Touloumtzis wrote:
> On Tue, Aug 27, 2002 at 06:08:42PM +0200, Alex Riesen wrote:
> > tricky. You can use /proc/<getpid>/fd, and close all
> > handles listed here, but this has some caveats:
> > it's _very_ slow if you have many open files.
> > it's not portable.
> > it's not safe if you have a thread/signal handler running.
> >
> > i never heard of a right way to do this.
> >
> > -alex
> >
> > int close_all_fd()
> > {
> > char fdpath[PATH_MAX];
> > DIR * dp;
> > struct dirent * de;
> > int fd;
> >
> > sprintf(fdpath, "/proc/%d/fd", getpid());
> > dp = opendir(fdpath);
>
> This can just be:
>
> dp = opendir("/proc/self/fd/");
>
> then you don't need fdpath.
Oh, indeed.
> You can use sigprocmask() if you're worried about signals coming
> in during this operation.
I, personally, strongly dislike this option.
You never know which signals are to be blocked.
Besides, it's still not portable and not safe agains threads.
-alex