[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Pine.LNX.4.64.0705021159340.7176@alien.or.mcafeemobile.com>
Date: Wed, 2 May 2007 12:42:45 -0700 (PDT)
From: Davide Libenzi <davidel@...ilserver.org>
To: Davi Arnaut <davi@...ent.com.br>
cc: Andrew Morton <akpm@...ux-foundation.org>,
Linus Torvalds <torvalds@...ux-foundation.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [patch 00/22] pollfs: filesystem abstraction for pollable objects
On Wed, 2 May 2007, Davi Arnaut wrote:
> So in this case I may borrow some signalfd code :-) I really like the
> signalfd approach, but IMHO the code is quite ugly and duplicates
> a lot of hairy code.
Ugly, really? Please ...
+ while (!mutex_trylock(&evs->mutex))
+ cpu_relax();
So, let's see. The whole things adds an unneeded (read "empty") extra
layer of indirections over the existing file_operations. Timerfd added the
return of the structure (because "developer may need it"), that wants
extra compat too. Your signal code does not handle things that is supposed
to be handling (easy to write clean code when the code does not actually
do what is supposed to be doing), and you'll be sucking code from
signalfd. On top of that, it returns a structure that needs compat too.
AIO code added an extra unnecessary wrapper over AIO native calls (by
literally calling sys_io_*). And the compat on sys_read/sys_write I
guess will be funny to watch. So what does your code effectively adds that
is useful in some way? Undocumented and testcase-less code?
As I see it, your code badly copies exiting one, and adds uneeded
infrastrcture/functionality.
See, I really wouldn't care, if you'd keep the "ugly" word for yourself.
Actually, I really care only about the functionality, so I leave the whole
ball into Andrew's hands ;)
- Davide
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists