From: Andrew Morton <[email protected]>
Apparently FUTEX_FD is unfixably racy and nothing uses it (or if it does, it
shouldn't).
Add a warning printk, give any remaining users six months to migrate off it.
Cc: Ulrich Drepper <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Rusty Russell <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
---
kernel/futex.c | 8 ++++++++
1 files changed, 8 insertions(+)
diff -puN kernel/futex.c~schedule-removal-of-futex_fd kernel/futex.c
--- a/kernel/futex.c~schedule-removal-of-futex_fd
+++ a/kernel/futex.c
@@ -1507,6 +1507,14 @@ static int futex_fd(u32 __user *uaddr, i
struct futex_q *q;
struct file *filp;
int ret, err;
+ static int warn_count;
+
+ if (warn_count < 10) {
+ printk(KERN_WARNING "Process `%s' used FUTEX_FD, which "
+ "will be removed from the kernel in June 2007\n",
+ current->comm);
+ warn_count++;
+ }
ret = -EINVAL;
if (!valid_signal(signal))
_
Ar Maw, 2006-10-31 am 15:09 -0800, ysgrifennodd [email protected]:
> From: Andrew Morton <[email protected]>
>
> Apparently FUTEX_FD is unfixably racy and nothing uses it (or if it does, it
> shouldn't).
>
> Add a warning printk, give any remaining users six months to migrate off it.
Andrew - please use time based rate limits for this sort of thing, that
way you actually get to see who is actually using it. Probably doesn't
matter for the FUTEX_FD case as nobody does, but in general the 'ten
times during boot' approach is not as good as ratelimit(): Perhaps
net_ratelimit() ought to become more general ?
On Tue, 31 Oct 2006 23:48:11 +0000
Alan Cox <[email protected]> wrote:
> Ar Maw, 2006-10-31 am 15:09 -0800, ysgrifennodd [email protected]:
> > From: Andrew Morton <[email protected]>
> >
> > Apparently FUTEX_FD is unfixably racy and nothing uses it (or if it does, it
> > shouldn't).
> >
> > Add a warning printk, give any remaining users six months to migrate off it.
>
> Andrew - please use time based rate limits for this sort of thing, that
> way you actually get to see who is actually using it. Probably doesn't
> matter for the FUTEX_FD case as nobody does, but in general the 'ten
> times during boot' approach is not as good as ratelimit(): Perhaps
> net_ratelimit() ought to become more general ?
I don't think either works very well, really. How frequently do we
rate-limit it? If it's faster than once-per-hour then we'll irritate
people.
Maybe that's the answer. Once-per-hour.
umm, I think we'd need something like this:
From: Andrew Morton <[email protected]>
printk_ratelimit() has global state which makes it not useful for callers
which wish to perform ratelimiting at a particular frequency.
Add a printk_timed_ratelimit() which utilises caller-provided state storage to
permit more flexibility.
This function can in fact be used for things other than printk ratelimiting
and is perhaps poorly named.
Signed-off-by: Andrew Morton <[email protected]>
---
include/linux/kernel.h | 2 ++
kernel/printk.c | 21 +++++++++++++++++++++
2 files changed, 23 insertions(+)
diff -puN kernel/printk.c~printk-timed-ratelimit kernel/printk.c
--- a/kernel/printk.c~printk-timed-ratelimit
+++ a/kernel/printk.c
@@ -31,6 +31,7 @@
#include <linux/security.h>
#include <linux/bootmem.h>
#include <linux/syscalls.h>
+#include <linux/jiffies.h>
#include <asm/uaccess.h>
@@ -1101,3 +1102,23 @@ int printk_ratelimit(void)
printk_ratelimit_burst);
}
EXPORT_SYMBOL(printk_ratelimit);
+
+/**
+ * printk_timed_ratelimit - caller-controlled printk ratelimiting
+ * @caller_jiffies: pointer to caller's state
+ * @interval_msecs: minimum interval between prints
+ *
+ * printk_timed_ratelimit() returns true if more than @interval_msecs
+ * milliseconds have elapsed since the last time printk_timed_ratelimit()
+ * returned true.
+ */
+bool printk_timed_ratelimit(unsigned long *caller_jiffies,
+ unsigned int interval_msecs)
+{
+ if (*caller_jiffies == 0 || time_after(jiffies, *caller_jiffies)) {
+ *caller_jiffies = jiffies + msecs_to_jiffies(interval_msecs);
+ return true;
+ }
+ return false;
+}
+EXPORT_SYMBOL(printk_timed_ratelimit);
diff -puN include/linux/kernel.h~printk-timed-ratelimit include/linux/kernel.h
--- a/include/linux/kernel.h~printk-timed-ratelimit
+++ a/include/linux/kernel.h
@@ -171,6 +171,8 @@ __attribute_const__ roundup_pow_of_two(u
extern int printk_ratelimit(void);
extern int __printk_ratelimit(int ratelimit_jiffies, int ratelimit_burst);
+extern bool printk_timed_ratelimit(unsigned long *caller_jiffies,
+ unsigned int interval_msec);
static inline void console_silent(void)
{
_
On Wed, 01 Nov 2006 12:19:05 +1100
Rusty Russell <[email protected]> wrote:
> On Tue, 2006-10-31 at 15:09 -0800, [email protected] wrote:
> > From: Andrew Morton <[email protected]>
> >
> > Apparently FUTEX_FD is unfixably racy and nothing uses it (or if it does, it
> > shouldn't).
> >
> > Add a warning printk, give any remaining users six months to migrate off it.
>
> This makes sense. FUTEX_FD was for the NGPT project which did userspace
> threading, and hence couldn't block. It was always kind of a hack
> (although unfixably racy isn't quite right, it depends on usage).
>
> However, the existence of FUTEX_FD is what made Ingo complain that we
> couldn't simply pin the futex page in memory, because now a process
> could pin one page per fd. Removing it would seem to indicate that we
> can return to a much simpler scheme of (1) pinning a page when someone
> does futex_wait, and (2) simply comparing futexes by physical address.
>
> Now, I realize with some dismay that simplicity is no longer a futex
> feature, but it might be worth considering?
Sure. Perhaps we could accelerate the removal schedule if we want to do
this. Let's see how many 2.6.19 users squeak first.
> Cheers,
> Rusty.
> PS. I used to have a patch for "ratelim_printk()" which hashed on the
> format string to reduce the chance that one message limit would clobber
> other messages. I'll dig it out...
I think the caller-provided-state thing will work OK?
On Tue, 2006-10-31 at 15:09 -0800, [email protected] wrote:
> From: Andrew Morton <[email protected]>
>
> Apparently FUTEX_FD is unfixably racy and nothing uses it (or if it does, it
> shouldn't).
>
> Add a warning printk, give any remaining users six months to migrate off it.
>
> Cc: Ulrich Drepper <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Rusty Russell <[email protected]>
> Signed-off-by: Andrew Morton <[email protected]>
ACK.
Can you add it to Documentation/feature-removal-schedule.txt too ?
tglx
On Tue, 2006-10-31 at 23:48 +0000, Alan Cox wrote:
> Ar Maw, 2006-10-31 am 15:09 -0800, ysgrifennodd [email protected]:
> > From: Andrew Morton <[email protected]>
> >
> > Apparently FUTEX_FD is unfixably racy and nothing uses it (or if it does, it
> > shouldn't).
> >
> > Add a warning printk, give any remaining users six months to migrate off it.
>
> Andrew - please use time based rate limits for this sort of thing, that
> way you actually get to see who is actually using it.
Not really: It might be one time calls, so a time based rate limit might
exclude apps which get executed in a row.
Maybe some thread->deprecated bitfield which rate limits the deprecated
warning per caller would be an alternative solution. We should not have
more than 32 of such deprecated functions at once. Of cource it would
still spam the logs when high frequency forking servers use such
interfaces, but I think the probability is low.
tglx
On Tue, 2006-10-31 at 15:09 -0800, [email protected] wrote:
> From: Andrew Morton <[email protected]>
>
> Apparently FUTEX_FD is unfixably racy and nothing uses it (or if it does, it
> shouldn't).
>
> Add a warning printk, give any remaining users six months to migrate off it.
This makes sense. FUTEX_FD was for the NGPT project which did userspace
threading, and hence couldn't block. It was always kind of a hack
(although unfixably racy isn't quite right, it depends on usage).
However, the existence of FUTEX_FD is what made Ingo complain that we
couldn't simply pin the futex page in memory, because now a process
could pin one page per fd. Removing it would seem to indicate that we
can return to a much simpler scheme of (1) pinning a page when someone
does futex_wait, and (2) simply comparing futexes by physical address.
Now, I realize with some dismay that simplicity is no longer a futex
feature, but it might be worth considering?
Cheers,
Rusty.
PS. I used to have a patch for "ratelim_printk()" which hashed on the
format string to reduce the chance that one message limit would clobber
other messages. I'll dig it out...
On Tue, Oct 31, 2006 at 05:23:12PM -0800, Andrew Morton wrote:
> > Now, I realize with some dismay that simplicity is no longer a futex
> > feature, but it might be worth considering?
>
> Sure. Perhaps we could accelerate the removal schedule if we want to do
> this. Let's see how many 2.6.19 users squeak first.
I must admit to never having used FUTEX_FD, but the idea of waiting on a
FUTEX by way of epoll appealed to me. Should I resort to pipe tricks if I
want that ability henceforth?
Thanks.
--
http://www.PowerDNS.com Open source, database driven DNS Software
http://netherlabs.nl Open and Closed source services
On Wed, 1 Nov 2006 10:11:35 +0100
bert hubert <[email protected]> wrote:
> On Tue, Oct 31, 2006 at 05:23:12PM -0800, Andrew Morton wrote:
> > > Now, I realize with some dismay that simplicity is no longer a futex
> > > feature, but it might be worth considering?
> >
> > Sure. Perhaps we could accelerate the removal schedule if we want to do
> > this. Let's see how many 2.6.19 users squeak first.
>
> I must admit to never having used FUTEX_FD, but the idea of waiting on a
> FUTEX by way of epoll appealed to me. Should I resort to pipe tricks if I
> want that ability henceforth?
>
I guess so. Until the grand unified kernel event framework is done.
On Wed, Nov 01, 2006 at 01:18:46AM -0800, Andrew Morton wrote:
> > I must admit to never having used FUTEX_FD, but the idea of waiting on a
> > FUTEX by way of epoll appealed to me. Should I resort to pipe tricks if I
> > want that ability henceforth?
>
> I guess so. Until the grand unified kernel event framework is done.
It might be polite to not entirely remove FUTEX_FD until said 'GUKE'
framework is done.
Bert
--
http://www.PowerDNS.com Open source, database driven DNS Software
http://netherlabs.nl Open and Closed source services
Rusty Russell a ?crit :
> On Tue, 2006-10-31 at 15:09 -0800, [email protected] wrote:
>> From: Andrew Morton <[email protected]>
>>
>> Apparently FUTEX_FD is unfixably racy and nothing uses it (or if it does, it
>> shouldn't).
>>
>> Add a warning printk, give any remaining users six months to migrate off it.
>
> This makes sense. FUTEX_FD was for the NGPT project which did userspace
> threading, and hence couldn't block. It was always kind of a hack
> (although unfixably racy isn't quite right, it depends on usage).
>
> However, the existence of FUTEX_FD is what made Ingo complain that we
> couldn't simply pin the futex page in memory, because now a process
> could pin one page per fd. Removing it would seem to indicate that we
> can return to a much simpler scheme of (1) pinning a page when someone
> does futex_wait, and (2) simply comparing futexes by physical address.
>
> Now, I realize with some dismay that simplicity is no longer a futex
> feature, but it might be worth considering?
Hum... I am not sure playing MM games is good... really...
This discussion reminds me I posted a patch some time ago that got no comments
from the community.
http://lkml.org/lkml/2006/8/9/26
Eric