2003-01-25 03:51:49

by Davide Libenzi

[permalink] [raw]
Subject: [patch] epoll for 2.4.20 updated ...


I updated the 2.4.20 patch with the changes posted today and I fixed a
little error about the wait queue function prototype :

http://www.xmailserver.org/linux-patches/sys_epoll-2.4.20-0.61.diff



- Davide


2003-01-25 21:49:34

by J.A. Magallon

[permalink] [raw]
Subject: Re: [patch] epoll for 2.4.20 updated ...


On 2003.01.25 Davide Libenzi wrote:
>
> I updated the 2.4.20 patch with the changes posted today and I fixed a
> little error about the wait queue function prototype :
>
> http://www.xmailserver.org/linux-patches/sys_epoll-2.4.20-0.61.diff
>

Mixing epoll ontop of current aa, I found this:

#define add_wait_queue_cond(q, wait, cond) \
({ \
unsigned long flags; \
int _raced = 0; \
wq_write_lock_irqsave(&(q)->lock, flags); \
(wait)->flags = 0; \
__add_wait_queue((q), (wait)); \
mb(); \
if (!(cond)) { \
_raced = 1; \
__remove_wait_queue((q), (wait)); \
} \
wq_write_unlock_irqrestore(&(q)->lock, flags); \
_raced; \
})

this is the -aa version. Version from epoll uses just a rmb() barrier
(afaik, just a _read_ barrier). In -aa they are just the same, but I also
use a patch that does this:


+#ifdef CONFIG_X86_MFENCE
+#define mb() __asm__ __volatile__ ("mfence": : :"memory")
+#else
#define mb() __asm__ __volatile__ ("lock; addl $0,0(%%esp)": : :"memory")
+#endif
+
+#ifdef CONFIG_X86_LFENCE
+#define rmb() __asm__ __volatile__ ("lfence": : :"memory")
+#else
#define rmb() mb()
+#endif

so for modern processors they are different, and can affect performance and
correctness. So which one it the correct one for the above code snipet ?

TIA

--
J.A. Magallon <[email protected]> \ Software is like sex:
werewolf.able.es \ It's better when it's free
Mandrake Linux release 9.1 (Cooker) for i586
Linux 2.4.21-pre3-jam3 (gcc 3.2.1 (Mandrake Linux 9.1 3.2.1-3mdk))

2003-01-25 23:30:16

by Davide Libenzi

[permalink] [raw]
Subject: Re: [patch] epoll for 2.4.20 updated ...

On Sat, 25 Jan 2003, J.A. Magallon wrote:

>
> On 2003.01.25 Davide Libenzi wrote:
> >
> > I updated the 2.4.20 patch with the changes posted today and I fixed a
> > little error about the wait queue function prototype :
> >
> > http://www.xmailserver.org/linux-patches/sys_epoll-2.4.20-0.61.diff
> >
>
> Mixing epoll ontop of current aa, I found this:
>
> #define add_wait_queue_cond(q, wait, cond) \
> ({ \
> unsigned long flags; \
> int _raced = 0; \
> wq_write_lock_irqsave(&(q)->lock, flags); \
> (wait)->flags = 0; \
> __add_wait_queue((q), (wait)); \
> mb(); \
> if (!(cond)) { \
> _raced = 1; \
> __remove_wait_queue((q), (wait)); \
> } \
> wq_write_unlock_irqrestore(&(q)->lock, flags); \
> _raced; \
> })
>
> this is the -aa version. Version from epoll uses just a rmb() barrier
> (afaik, just a _read_ barrier). In -aa they are just the same, but I also
> use a patch that does this:
>
>
> +#ifdef CONFIG_X86_MFENCE
> +#define mb() __asm__ __volatile__ ("mfence": : :"memory")
> +#else
> #define mb() __asm__ __volatile__ ("lock; addl $0,0(%%esp)": : :"memory")
> +#endif
> +
> +#ifdef CONFIG_X86_LFENCE
> +#define rmb() __asm__ __volatile__ ("lfence": : :"memory")
> +#else
> #define rmb() mb()
> +#endif
>
> so for modern processors they are different, and can affect performance and
> correctness. So which one it the correct one for the above code snipet ?

It depends on what "cond" does. Being it a macro I'd feel safer with an mb().



- Davide

2003-01-26 00:06:20

by J.A. Magallon

[permalink] [raw]
Subject: Re: [patch] epoll for 2.4.20 updated ...


On 2003.01.25 Davide Libenzi wrote:
>
> I updated the 2.4.20 patch with the changes posted today and I fixed a
> little error about the wait queue function prototype :
>
> http://www.xmailserver.org/linux-patches/sys_epoll-2.4.20-0.61.diff
>

I needed this to build smbfs:

--- linux-2.4.21-pre3-jam3/fs/smbfs/sock.c.orig 2003-01-26 01:02:32.000000000 +0100
+++ linux-2.4.21-pre3-jam3/fs/smbfs/sock.c 2003-01-26 01:03:11.000000000 +0100
@@ -314,7 +314,7 @@
smb_receive_poll(struct smb_sb_info *server)
{
struct file *file = server->sock_file;
- poll_table wait_table;
+ struct poll_wqueues wait_table;
int result = 0;
int timeout = server->mnt->timeo * HZ;
int mask;
@@ -323,7 +323,7 @@
poll_initwait(&wait_table);
set_current_state(TASK_INTERRUPTIBLE);

- mask = file->f_op->poll(file, &wait_table);
+ mask = file->f_op->poll(file, &wait_table.pt);
if (mask & POLLIN) {
poll_freewait(&wait_table);
current->state = TASK_RUNNING;

Is it correct ?

TIA

--
J.A. Magallon <[email protected]> \ Software is like sex:
werewolf.able.es \ It's better when it's free
Mandrake Linux release 9.1 (Cooker) for i586
Linux 2.4.21-pre3-jam3 (gcc 3.2.1 (Mandrake Linux 9.1 3.2.1-4mdk))

2003-01-26 00:13:53

by Davide Libenzi

[permalink] [raw]
Subject: Re: [patch] epoll for 2.4.20 updated ...

On Sun, 26 Jan 2003, J.A. Magallon wrote:

>
> On 2003.01.25 Davide Libenzi wrote:
> >
> > I updated the 2.4.20 patch with the changes posted today and I fixed a
> > little error about the wait queue function prototype :
> >
> > http://www.xmailserver.org/linux-patches/sys_epoll-2.4.20-0.61.diff
> >
>
> I needed this to build smbfs:
>
> --- linux-2.4.21-pre3-jam3/fs/smbfs/sock.c.orig 2003-01-26 01:02:32.000000000 +0100
> +++ linux-2.4.21-pre3-jam3/fs/smbfs/sock.c 2003-01-26 01:03:11.000000000 +0100
> @@ -314,7 +314,7 @@
> smb_receive_poll(struct smb_sb_info *server)
> {
> struct file *file = server->sock_file;
> - poll_table wait_table;
> + struct poll_wqueues wait_table;
> int result = 0;
> int timeout = server->mnt->timeo * HZ;
> int mask;
> @@ -323,7 +323,7 @@
> poll_initwait(&wait_table);
> set_current_state(TASK_INTERRUPTIBLE);
>
> - mask = file->f_op->poll(file, &wait_table);
> + mask = file->f_op->poll(file, &wait_table.pt);
> if (mask & POLLIN) {
> poll_freewait(&wait_table);
> current->state = TASK_RUNNING;
>
> Is it correct ?

I thought this was already been reported and fixed. Your fix is fine, I'll
make 0.62 ...



- Davide