LinuxLists.cc - -mm merge plans for 2.6.21

[permalink] [raw]

Subject: Re: -mm merge plans for 2.6.21

On Fri, 9 Feb 2007 00:29:06 +0100 (MET)
Jan Engelhardt <[email protected]> wrote:

>
> On Feb 8 2007 15:07, Andrew Morton wrote:
>
> >scheduled-removal-of-sa_xxx-interrupt-flags-fixups.patch
> >scheduled-removal-of-sa_xxx-interrupt-flags-fixups-2.patch
> >scheduled-removal-of-sa_xxx-interrupt-flags.patch
> >scheduled-removal-of-sa_xxx-interrupt-flags-ata-fix.patch
> >
> > This removes SA_INTERRUPT and friends, so 10000000 external drivers won't
> > compile any more. I think I'd prefer to find a way to get usage of SA_* to
> > spit a deprecated warning.
>
> Here's an idea:
>
> #define SA_INTERRUPT sa_interrupt_with_warning()
> static inline int sa_interrupt_with_warning(void) {
> if(more_or_less_often)
> printk(fat_warning);
> return 0x123456; /* whatever numerical value it is */
> }
>

Yeah, this seems to work.

--- a/include/linux/interrupt.h~deprecate-sa_interrupt-and-friends
+++ a/include/linux/interrupt.h
@@ -54,17 +54,32 @@
* Migration helpers. Scheduled for removal in 1/2007
* Do not use for new code !
*/
-#define SA_INTERRUPT IRQF_DISABLED
-#define SA_SAMPLE_RANDOM IRQF_SAMPLE_RANDOM
-#define SA_SHIRQ IRQF_SHARED
-#define SA_PROBEIRQ IRQF_PROBE_SHARED
-#define SA_PERCPU IRQF_PERCPU
-
-#define SA_TRIGGER_LOW IRQF_TRIGGER_LOW
-#define SA_TRIGGER_HIGH IRQF_TRIGGER_HIGH
-#define SA_TRIGGER_FALLING IRQF_TRIGGER_FALLING
-#define SA_TRIGGER_RISING IRQF_TRIGGER_RISING
-#define SA_TRIGGER_MASK IRQF_TRIGGER_MASK
+#define emit_old_interrupt_name(old, new) \
+static inline unsigned __deprecated emit_old_interrupt_name##old(void) \
+{ \
+ return (new); \
+}
+
+emit_old_interrupt_name(SA_INTERRUPT, IRQF_DISABLED)
+#define SA_INTERRUPT emit_old_interrupt_nameSA_INTERRUPT()
+emit_old_interrupt_name(SA_SAMPLE_RANDOM, IRQF_SAMPLE_RANDOM)
+#define SA_SAMPLE_RANDOM emit_old_interrupt_nameSA_SAMPLE_RANDOM()
+emit_old_interrupt_name(SA_SHIRQ, IRQF_SHARED)
+#define SA_SHIRQ emit_old_interrupt_nameSA_SHIRQ()
+emit_old_interrupt_name(SA_PROBEIRQ, IRQF_PROBE_SHARED)
+#define SA_PROBEIRQ emit_old_interrupt_nameSA_PROBEIRQ()
+emit_old_interrupt_name(SA_PERCPU, IRQF_PERCPU)
+#define SA_PERCPU emit_old_interrupt_nameSA_PERCPU()
+emit_old_interrupt_name(SA_TRIGGER_LOW, IRQF_TRIGGER_LOW)
+#define SA_TRIGGER_LOW emit_old_interrupt_nameSA_TRIGGER_LOW()
+emit_old_interrupt_name(SA_TRIGGER_HIGH, IRQF_TRIGGER_HIGH)
+#define SA_TRIGGER_HIGH emit_old_interrupt_nameSA_TRIGGER_HIGH()
+emit_old_interrupt_name(SA_TRIGGER_FALLING, IRQF_TRIGGER_FALLING)
+#define SA_TRIGGER_FALLING emit_old_interrupt_nameSA_TRIGGER_FALLING()
+emit_old_interrupt_name(SA_TRIGGER_RISING, IRQF_TRIGGER_RISING)
+#define SA_TRIGGER_RISING emit_old_interrupt_nameSA_TRIGGER_RISING()
+emit_old_interrupt_name(SA_TRIGGER_MASK, IRQF_TRIGGER_MASK)
+#define SA_TRIGGER_MASK emit_old_interrupt_nameSA_TRIGGER_MASK()

typedef irqreturn_t (*irq_handler_t)(int, void *);

_

For some reason

enum foo {
...
} __deprecated;

doesn't work.

2007-02-08 23:53:06

[permalink] [raw]

Subject: Re: -mm merge plans for 2.6.21

On Thu, 8 Feb 2007 18:34:49 -0500
Kyle McMartin <[email protected]> wrote:

> On Thu, Feb 08, 2007 at 03:07:10PM -0800, Andrew Morton wrote:
> > I'm getting fed up of holding onto hundreds of patches against subsystem
> > trees, sending them over and over again seeing and nothing happen. I sent 242
> > patches out to subsystem maintainers on Monday and look at what's still here.
> >
> > parisc-fix-module_param-iommu-permission.patch
> > pa-risc-fix-bogus-warnings-from-modpost.patch
> > use-__u64-rather-than-u64-in-parisc-statfs-structs.patch
> >
> > -> kyle
> >
>
> Sorry, I thought the mails meant you were dumping to Linus. Will
> merge.

Once a subsystem has a subsystem tree (git or quilt) I basically never
merge anything which belongs to that tree. It's always

originator->mm->subsystemtree->Linus

If the subsystem tree maintainer wants to tell me "I can't be bothered
setting up a git pull for that, please merge it for me" then that's fine.

But unless I'm told that, or unless the maintainer is vacationing or totally
asleep or unless the fix has some sufficiently high obviousness*importance product,
I'll just keep buffering it up.

2007-02-09 00:55:47

by Paul Mackerras

[permalink] [raw]

Subject: Re: -mm merge plans for 2.6.21

Andrew Morton writes:

> Once a subsystem has a subsystem tree (git or quilt) I basically never
> merge anything which belongs to that tree. It's always
>
> originator->mm->subsystemtree->Linus
>
> If the subsystem tree maintainer wants to tell me "I can't be bothered
> setting up a git pull for that, please merge it for me" then that's fine.
>
> But unless I'm told that, or unless the maintainer is vacationing or totally
> asleep or unless the fix has some sufficiently high obviousness*importance product,
> I'll just keep buffering it up.

What about the sort of thing that crosses all archs? For example, the
local_t changes? Particularly in the case where the change has to be
made in generic code and in all archs at the same time, it makes sense
to me for you to send the whole batch to Linus at the same time,
rather than individual arch maintainers all sending their bit at
varying times.

Paul.

2007-02-09 01:01:09

[permalink] [raw]

Subject: Re: -mm merge plans for 2.6.21

On Fri, 9 Feb 2007 11:55:40 +1100
Paul Mackerras <[email protected]> wrote:

> Andrew Morton writes:
>
> > Once a subsystem has a subsystem tree (git or quilt) I basically never
> > merge anything which belongs to that tree. It's always
> >
> > originator->mm->subsystemtree->Linus
> >
> > If the subsystem tree maintainer wants to tell me "I can't be bothered
> > setting up a git pull for that, please merge it for me" then that's fine.
> >
> > But unless I'm told that, or unless the maintainer is vacationing or totally
> > asleep or unless the fix has some sufficiently high obviousness*importance product,
> > I'll just keep buffering it up.
>
> What about the sort of thing that crosses all archs? For example, the
> local_t changes? Particularly in the case where the change has to be
> made in generic code and in all archs at the same time, it makes sense
> to me for you to send the whole batch to Linus at the same time,
> rather than individual arch maintainers all sending their bit at
> varying times.
>

yup. It's better of course if the changes aren't both-way dependent and
often we do it that way. But if they really are that bound together then
I'll stage the patch in -mm, ensure that it doesn't conflict with any
queued-up arch patches and will merge it after the arch trees have gone in.

2007-02-09 05:32:30

by Bharata B Rao

[permalink] [raw]

Subject: Re: -mm merge plans for 2.6.21

Andrew,

On 2/9/07, Andrew Morton <[email protected]> wrote:
> fsaio-add-a-wait-queue-arg-to-the-wait_bit-action-routine.patch
> fsaio-add-a-wait-queue-arg-to-the-wait_bit-action-routine-gfs2-fix.patch
> fsaio-rename-__lock_page-to-lock_page_blocking.patch
> fsaio-interfaces-to-initialize-and-to-test-a-wait-bit-key.patch
> fsaio-add-a-default-io-wait-bit-field-in-task-struct.patch
> fsaio-enable-wait-bit-based-filtered-wakeups-to-work-for-aio.patch
> fsaio-enable-wait-bit-based-filtered-wakeups-to-work-for-aio-fix.patch
> fsaio-enable-wait-bit-based-filtered-wakeups-to-work-for-aio-fix-sparse-fix.patch
> fsaio-enable-asynchronous-wait-page-and-lock-page.patch
> fsaio-filesystem-aio-read.patch
> fsaio-aio-o_sync-filesystem-write.patch
>
> Will wait to see what happens with febrils.
>
> aio-is-unlikely.patch
>
> Will merge.
>
> rework-compat_sys_io_submit.patch
> fix-aioh-includes.patch
> fix-access_ok-checks.patch
> make-good_sigevent-non-static.patch
> make-good_sigevent-non-static-fix.patch
> make-__sigqueue_free-and.patch
> aio-completion-signal-notification.patch
> aio-completion-signal-notification-fix.patch
> aio-completion-signal-notification-fixes-and-cleanups.patch
> aio-completion-signal-notification-small-cleanup.patch
> add-listio-syscall-support.patch
>
> I guess these are dependent upon fsaio.

No. AIO completion signal notification patches and listio patch work
independently of fsaio patches. Just that for buffered AIO case, the
fsaio patches will make the behaviour truly asynchronous.

Regards,
Bharata.

2007-02-09 08:27:45

by Sébastien Dugué

[permalink] [raw]

Subject: Re: -mm merge plans for 2.6.21

On Fri, 9 Feb 2007 11:02:28 +0530 "Bharata B Rao" <[email protected]> wrote:

> Andrew,
>
> On 2/9/07, Andrew Morton <[email protected]> wrote:
> > fsaio-add-a-wait-queue-arg-to-the-wait_bit-action-routine.patch
> > fsaio-add-a-wait-queue-arg-to-the-wait_bit-action-routine-gfs2-fix.patch
> > fsaio-rename-__lock_page-to-lock_page_blocking.patch
> > fsaio-interfaces-to-initialize-and-to-test-a-wait-bit-key.patch
> > fsaio-add-a-default-io-wait-bit-field-in-task-struct.patch
> > fsaio-enable-wait-bit-based-filtered-wakeups-to-work-for-aio.patch
> > fsaio-enable-wait-bit-based-filtered-wakeups-to-work-for-aio-fix.patch
> > fsaio-enable-wait-bit-based-filtered-wakeups-to-work-for-aio-fix-sparse-fix.patch
> > fsaio-enable-asynchronous-wait-page-and-lock-page.patch
> > fsaio-filesystem-aio-read.patch
> > fsaio-aio-o_sync-filesystem-write.patch
> >
> > Will wait to see what happens with febrils.
> >
> > aio-is-unlikely.patch
> >
> > Will merge.
> >
> > rework-compat_sys_io_submit.patch
> > fix-aioh-includes.patch
> > fix-access_ok-checks.patch
> > make-good_sigevent-non-static.patch
> > make-good_sigevent-non-static-fix.patch
> > make-__sigqueue_free-and.patch
> > aio-completion-signal-notification.patch
> > aio-completion-signal-notification-fix.patch
> > aio-completion-signal-notification-fixes-and-cleanups.patch
> > aio-completion-signal-notification-small-cleanup.patch
> > add-listio-syscall-support.patch
> >
> > I guess these are dependent upon fsaio.
>
> No. AIO completion signal notification patches and listio patch work
> independently of fsaio patches. Just that for buffered AIO case, the
> fsaio patches will make the behaviour truly asynchronous.
>
> Regards,
> Bharata.

I concur.

Thanks,

S?bastien.

2007-02-09 09:06:10

[permalink] [raw]

Subject: Re: -mm merge plans for 2.6.21

On Fri, 9 Feb 2007 09:26:11 +0100 S?bastien Dugu? <[email protected]> wrote:

> On Fri, 9 Feb 2007 11:02:28 +0530 "Bharata B Rao" <[email protected]> wrote:
>
> > Andrew,
> >
> > On 2/9/07, Andrew Morton <[email protected]> wrote:
> > > fsaio-add-a-wait-queue-arg-to-the-wait_bit-action-routine.patch
> > > fsaio-add-a-wait-queue-arg-to-the-wait_bit-action-routine-gfs2-fix.patch
> > > fsaio-rename-__lock_page-to-lock_page_blocking.patch
> > > fsaio-interfaces-to-initialize-and-to-test-a-wait-bit-key.patch
> > > fsaio-add-a-default-io-wait-bit-field-in-task-struct.patch
> > > fsaio-enable-wait-bit-based-filtered-wakeups-to-work-for-aio.patch
> > > fsaio-enable-wait-bit-based-filtered-wakeups-to-work-for-aio-fix.patch
> > > fsaio-enable-wait-bit-based-filtered-wakeups-to-work-for-aio-fix-sparse-fix.patch
> > > fsaio-enable-asynchronous-wait-page-and-lock-page.patch
> > > fsaio-filesystem-aio-read.patch
> > > fsaio-aio-o_sync-filesystem-write.patch
> > >
> > > Will wait to see what happens with febrils.
> > >
> > > aio-is-unlikely.patch
> > >
> > > Will merge.
> > >
> > > rework-compat_sys_io_submit.patch
> > > fix-aioh-includes.patch
> > > fix-access_ok-checks.patch
> > > make-good_sigevent-non-static.patch
> > > make-good_sigevent-non-static-fix.patch
> > > make-__sigqueue_free-and.patch
> > > aio-completion-signal-notification.patch
> > > aio-completion-signal-notification-fix.patch
> > > aio-completion-signal-notification-fixes-and-cleanups.patch
> > > aio-completion-signal-notification-small-cleanup.patch
> > > add-listio-syscall-support.patch
> > >
> > > I guess these are dependent upon fsaio.
> >
> > No. AIO completion signal notification patches and listio patch work
> > independently of fsaio patches. Just that for buffered AIO case, the
> > fsaio patches will make the behaviour truly asynchronous.
> >
> > Regards,
> > Bharata.
>
> I concur.
>

But is the proposed listio implementation still relevant if we go and add
the async syscalls?

2007-02-09 10:01:13

by Lenar Lõhmus

[permalink] [raw]

Subject: Re: -mm merge plans for 2.6.21

Hello,

Andrew Morton wrote:
> sched-add-above-background-load-function.patch
> mm-implement-swap-prefetching.patch
> mm-implement-swap-prefetching-vs-zvc-stuff.patch
> mm-implement-swap-prefetching-vs-zvc-stuff-2.patch
> mm-implement-swap-prefetching-use-ctl_unnumbered.patch
> swap_prefetch-vs-zoned-counters.patch
> add-include-linux-freezerh-and-move-definitions-from-prefetch.patch
> zvc-support-nr_slab_reclaimable--nr_slab_unreclaimable-swap_prefetch.patch
> reduce-max_nr_zones-swap_prefetch-remove-incorrect-use-of-zone_highmem.patch
> sched-cleanup-remove-task_t-convert-to-struct-task_struct-prefetch.patch
> numa-add-zone_to_nid-function-swap_prefetch.patch
> remove-uses-of-kmem_cache_t-from-mm-and-include-linux-slabh-prefetch.patch
>
> Hold.
Why hold?

It's been shown this patchset really helps desktop users. Why keep it in -mm
for so long?

L.

2007-02-09 10:11:47

by Sébastien Dugué

[permalink] [raw]

Subject: Re: -mm merge plans for 2.6.21

On Fri, 9 Feb 2007 01:05:57 -0800 Andrew Morton <[email protected]> wrote:

> On Fri, 9 Feb 2007 09:26:11 +0100 S?bastien Dugu? <[email protected]> wrote:
>
> > On Fri, 9 Feb 2007 11:02:28 +0530 "Bharata B Rao" <[email protected]> wrote:
> >
> > > Andrew,
> > >
> > > On 2/9/07, Andrew Morton <[email protected]> wrote:
> > > > fsaio-add-a-wait-queue-arg-to-the-wait_bit-action-routine.patch
> > > > fsaio-add-a-wait-queue-arg-to-the-wait_bit-action-routine-gfs2-fix.patch
> > > > fsaio-rename-__lock_page-to-lock_page_blocking.patch
> > > > fsaio-interfaces-to-initialize-and-to-test-a-wait-bit-key.patch
> > > > fsaio-add-a-default-io-wait-bit-field-in-task-struct.patch
> > > > fsaio-enable-wait-bit-based-filtered-wakeups-to-work-for-aio.patch
> > > > fsaio-enable-wait-bit-based-filtered-wakeups-to-work-for-aio-fix.patch
> > > > fsaio-enable-wait-bit-based-filtered-wakeups-to-work-for-aio-fix-sparse-fix.patch
> > > > fsaio-enable-asynchronous-wait-page-and-lock-page.patch
> > > > fsaio-filesystem-aio-read.patch
> > > > fsaio-aio-o_sync-filesystem-write.patch
> > > >
> > > > Will wait to see what happens with febrils.
> > > >
> > > > aio-is-unlikely.patch
> > > >
> > > > Will merge.
> > > >
> > > > rework-compat_sys_io_submit.patch
> > > > fix-aioh-includes.patch
> > > > fix-access_ok-checks.patch
> > > > make-good_sigevent-non-static.patch
> > > > make-good_sigevent-non-static-fix.patch
> > > > make-__sigqueue_free-and.patch
> > > > aio-completion-signal-notification.patch
> > > > aio-completion-signal-notification-fix.patch
> > > > aio-completion-signal-notification-fixes-and-cleanups.patch
> > > > aio-completion-signal-notification-small-cleanup.patch
> > > > add-listio-syscall-support.patch
> > > >
> > > > I guess these are dependent upon fsaio.
> > >
> > > No. AIO completion signal notification patches and listio patch work
> > > independently of fsaio patches. Just that for buffered AIO case, the
> > > fsaio patches will make the behaviour truly asynchronous.
> > >
> > > Regards,
> > > Bharata.
> >
> > I concur.
> >
>
> But is the proposed listio implementation still relevant if we go and add
> the async syscalls?

I think the listio syscall could be emulated in userspace using async
syscalls.

Even more, the whole POSIX AIO functions could benefit from this, rendering
aio.c obsolete.

S?bastien.

2007-02-09 10:12:46

[permalink] [raw]

Subject: Re: -mm merge plans for 2.6.21

On Fri, 09 Feb 2007 11:54:29 +0200 Lenar L?hmus <[email protected]> wrote:

> Hello,
>
> Andrew Morton wrote:
> > sched-add-above-background-load-function.patch
> > mm-implement-swap-prefetching.patch
> > mm-implement-swap-prefetching-vs-zvc-stuff.patch
> > mm-implement-swap-prefetching-vs-zvc-stuff-2.patch
> > mm-implement-swap-prefetching-use-ctl_unnumbered.patch
> > swap_prefetch-vs-zoned-counters.patch
> > add-include-linux-freezerh-and-move-definitions-from-prefetch.patch
> > zvc-support-nr_slab_reclaimable--nr_slab_unreclaimable-swap_prefetch.patch
> > reduce-max_nr_zones-swap_prefetch-remove-incorrect-use-of-zone_highmem.patch
> > sched-cleanup-remove-task_t-convert-to-struct-task_struct-prefetch.patch
> > numa-add-zone_to_nid-function-swap_prefetch.patch
> > remove-uses-of-kmem_cache_t-from-mm-and-include-linux-slabh-prefetch.patch
> >
> > Hold.
> Why hold?
>
> It's been shown this patchset really helps desktop users.

Has it? I don't think I've ever observed any benefits from it and I don't
think anyone has ever got down and worked out what its drawbacks might be,
and seen if they can be demonstrated in practice.

I also have vague memories of some serious-sounding review comments about
the code from Nick.

> Why keep it in -mm for so long?

I'm waiting to be shown that its benefits exceed its costs. Sometimes
that's hard.

2007-02-09 10:58:38

by Frederik Deweerdt

[permalink] [raw]

Subject: Re: -mm merge plans for 2.6.21

On Fri, Feb 09, 2007 at 12:29:06AM +0100, Jan Engelhardt wrote:
>
> On Feb 8 2007 15:07, Andrew Morton wrote:
>
> >scheduled-removal-of-sa_xxx-interrupt-flags-fixups.patch
> >scheduled-removal-of-sa_xxx-interrupt-flags-fixups-2.patch
> >scheduled-removal-of-sa_xxx-interrupt-flags.patch
> >scheduled-removal-of-sa_xxx-interrupt-flags-ata-fix.patch
> >
> > This removes SA_INTERRUPT and friends, so 10000000 external drivers won't
> > compile any more. I think I'd prefer to find a way to get usage of SA_* to
> > spit a deprecated warning.
>
> Here's an idea:
>
> #define SA_INTERRUPT sa_interrupt_with_warning()
> static inline int sa_interrupt_with_warning(void) {
> if(more_or_less_often)
> printk(fat_warning);
> return 0x123456; /* whatever numerical value it is */
> }
>
I'd say that we want to warn the developper, not the user, isn't it?
The attached patch marks the variables as __deprecated, and re-schedules
the removal for January 2008.

Regards,
Frederik

diff --git a/Documentation/feature-removal-schedule.txt b/Documentation/feature-removal-schedule.txt
index b7c642c..4a5aad3 100644
--- a/Documentation/feature-removal-schedule.txt
+++ b/Documentation/feature-removal-schedule.txt
@@ -308,3 +308,12 @@ Why: OSS drivers with ALSA replacements
Who: Adrian Bunk <[email protected]>

---------------------------
+
+What: Interrupt only SA_* flags
+When: January 2008
+Why: The interrupt related SA_* flags are replaced by IRQF_* to move them
+ out of the signal namespace.
+
+Who: Frederik Deweerdt <[email protected]>
+
+---------------------------
diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
index e66b458..472f2d2 100644
--- a/include/linux/interrupt.h
+++ b/include/linux/interrupt.h
@@ -52,6 +52,25 @@
#define IRQF_PERCPU 0x00000400
#define IRQF_NOBALANCING 0x00000800

+/*
+ * Migration helpers. Scheduled for removal in 1/2008
+ * SA_* flags are now deprecated, leaving time for out-of-the-tree drivers
+ * to catch up.
+ *
+ * Do not use for new code !
+ */
+static const int __deprecated SA_INTERRUPT = IRQF_DISABLED;
+static const int __deprecated SA_SAMPLE_RANDOM = IRQF_SAMPLE_RANDOM;
+static const int __deprecated SA_SHIRQ = IRQF_SHARED;
+static const int __deprecated SA_PROBEIRQ = IRQF_PROBE_SHARED;
+static const int __deprecated SA_PERCPU = IRQF_PERCPU;
+
+static const int __deprecated SA_TRIGGER_LOW = IRQF_TRIGGER_LOW;
+static const int __deprecated SA_TRIGGER_HIGH = IRQF_TRIGGER_HIGH;
+static const int __deprecated SA_TRIGGER_FALLING = IRQF_TRIGGER_FALLING;
+static const int __deprecated SA_TRIGGER_RISING = IRQF_TRIGGER_RISING;
+static const int __deprecated SA_TRIGGER_MASK = IRQF_TRIGGER_MASK;
+
typedef irqreturn_t (*irq_handler_t)(int, void *);

struct irqaction {

2007-02-09 11:25:00

by Arjan van de Ven

[permalink] [raw]

Subject: Re: -mm merge plans for 2.6.21

On Fri, 2007-02-09 at 10:57 +0000, Frederik Deweerdt wrote:
> +static const int __deprecated SA_INTERRUPT = IRQF_DISABLED;
> +static const int __deprecated SA_SAMPLE_RANDOM = IRQF_SAMPLE_RANDOM;
> +static const int __deprecated SA_SHIRQ = IRQF_SHARED;
> +static const int __deprecated SA_PROBEIRQ = IRQF_PROBE_SHARED;
> +static const int __deprecated SA_PERCPU = IRQF_PERCPU;
> +
> +static const int __deprecated SA_TRIGGER_LOW = IRQF_TRIGGER_LOW;
> +static const int __deprecated SA_TRIGGER_HIGH = IRQF_TRIGGER_HIGH;
> +static const int __deprecated SA_TRIGGER_FALLING = IRQF_TRIGGER_FALLING;
> +static const int __deprecated SA_TRIGGER_RISING = IRQF_TRIGGER_RISING;
> +static const int __deprecated SA_TRIGGER_MASK = IRQF_TRIGGER_MASK;

this will include these in every .o file for which the .c file includes
the header. NOT GOOD(tm)

why not just bite the bullet?
removing version.h also broke the same all external modules, and they
got fixed in days.. no big deal. kernel api change all the time, this
one has been around in "double mode" quite some time...

2007-02-09 11:40:09

[permalink] [raw]

Subject: Re: -mm merge plans for 2.6.21

On Fri, 09 Feb 2007 12:24:33 +0100 Arjan van de Ven <[email protected]> wrote:

> On Fri, 2007-02-09 at 10:57 +0000, Frederik Deweerdt wrote:
> > +static const int __deprecated SA_INTERRUPT = IRQF_DISABLED;
> > +static const int __deprecated SA_SAMPLE_RANDOM = IRQF_SAMPLE_RANDOM;
> > +static const int __deprecated SA_SHIRQ = IRQF_SHARED;
> > +static const int __deprecated SA_PROBEIRQ = IRQF_PROBE_SHARED;
> > +static const int __deprecated SA_PERCPU = IRQF_PERCPU;
> > +
> > +static const int __deprecated SA_TRIGGER_LOW = IRQF_TRIGGER_LOW;
> > +static const int __deprecated SA_TRIGGER_HIGH = IRQF_TRIGGER_HIGH;
> > +static const int __deprecated SA_TRIGGER_FALLING = IRQF_TRIGGER_FALLING;
> > +static const int __deprecated SA_TRIGGER_RISING = IRQF_TRIGGER_RISING;
> > +static const int __deprecated SA_TRIGGER_MASK = IRQF_TRIGGER_MASK;
>
> this will include these in every .o file for which the .c file includes
> the header. NOT GOOD(tm)

As long as nobody takes the address of them (which wouldn't compile today
anyway) then the compiler should be able to not allocate store for these.
That they're const might help too.

> why not just bite the bullet?
> removing version.h also broke the same all external modules, and they
> got fixed in days.. no big deal. kernel api change all the time, this
> one has been around in "double mode" quite some time...

Pretty much every driver in the world will want these symbols. I expect
we'll help some people by doing this, and the cost to us is very small.

Plus we're getting a bad reputation out there for breaking stuff and
screwing people around, which isn't good.

2007-02-09 12:04:09

[permalink] [raw]

Subject: Re: -mm merge plans for 2.6.21

Andrew Morton <[email protected]> writes:
>
> As long as nobody takes the address of them (which wouldn't compile today
> anyway) then the compiler should be able to not allocate store for these.

This would only work for unit-at-a-time compilers (if it works at all,
i'm not sure), but not older 3.x compilers

> That they're const might help too.

Don't think it does.

-Andi

2007-02-09 12:31:10

by Jan Engelhardt

[permalink] [raw]

Subject: Re: -mm merge plans for 2.6.21

On Feb 9 2007 14:04, Andi Kleen wrote:
>Andrew Morton <[email protected]> writes:
>>
>> As long as nobody takes the address of them (which wouldn't compile today
>> anyway) then the compiler should be able to not allocate store for these.
>
>This would only work for unit-at-a-time compilers (if it works at all,
>i'm not sure), but not older 3.x compilers
>
>> That they're const might help too.
>
>Don't think it does.

GCC 4.1 optimizes both Andrew's and Frederik Deweerdt's ideas
perfectly out. Even if the const was not there in Frederik's example,
gcc seems throw it out with -O2 (judging by `nm` output) since it is
1. static 2. unused. Gcc even gives out a warning that the item is
unused when not marked with const.

Jan
--
ft: http://freshmeat.net/p/chaostables/

2007-02-09 12:32:18

by Arjan van de Ven

[permalink] [raw]

Subject: Re: -mm merge plans for 2.6.21

>
> As long as nobody takes the address of them (which wouldn't compile today
> anyway) then the compiler should be able to not allocate store for these.
> That they're const might help too.

are you really sure?
>
> > why not just bite the bullet?
> > removing version.h also broke the same all external modules, and they
> > got fixed in days.. no big deal. kernel api change all the time, this
> > one has been around in "double mode" quite some time...
>
> Pretty much every driver in the world will want these symbols. I expect
> we'll help some people by doing this, and the cost to us is very small.

well the same people had to change for the request_irq prototype change
etc etc.

I suppose they'll all get fixed if some distro does this cleanup
anyway... otherwise no amount of deprecation will get things changed;
unless the compile actually breaks nobody will pay attention.

Also quite a few external drivers that are aiming for mainline inclusion
should already be using the new settings for a while anyway...

Another option is to back out the change again totally; having 2
constants for the same thing is just a bad idea, and old users will keep
sneaking in.

--
if you want to mail me at work (you don't), use arjan (at) linux.intel.com
Test the interaction between Linux and your BIOS via http://www.linuxfirmwarekit.org

2007-02-09 12:48:04

by James

[permalink] [raw]

Subject: Re: -mm merge plans for 2.6.21

On 2/9/07, Andrew Morton <[email protected]> wrote:
> On Fri, 09 Feb 2007 11:54:29 +0200 Lenar L?hmus <[email protected]> wrote:
>
> > Hello,
> >
> > Andrew Morton wrote:
> > > sched-add-above-background-load-function.patch
> > > mm-implement-swap-prefetching.patch
> > > mm-implement-swap-prefetching-vs-zvc-stuff.patch
> > > mm-implement-swap-prefetching-vs-zvc-stuff-2.patch
> > > mm-implement-swap-prefetching-use-ctl_unnumbered.patch
> > > swap_prefetch-vs-zoned-counters.patch
> > > add-include-linux-freezerh-and-move-definitions-from-prefetch.patch
> > > zvc-support-nr_slab_reclaimable--nr_slab_unreclaimable-swap_prefetch.patch
> > > reduce-max_nr_zones-swap_prefetch-remove-incorrect-use-of-zone_highmem.patch
> > > sched-cleanup-remove-task_t-convert-to-struct-task_struct-prefetch.patch
> > > numa-add-zone_to_nid-function-swap_prefetch.patch
> > > remove-uses-of-kmem_cache_t-from-mm-and-include-linux-slabh-prefetch.patch
> > >
> > > Hold.
> > Why hold?
> >
> > It's been shown this patchset really helps desktop users.
>
> Has it? I don't think I've ever observed any benefits from it and I don't
> think anyone has ever got down and worked out what its drawbacks might be,
> and seen if they can be demonstrated in practice.
>

Plenty of people replied with positive reviews when it was posted
earlier. I'd expect that you'd have a fair bit of ram, so you're less
likely to observe any benefit, as you wouldnt be digging into swap as
often as those on lesser systems.

http://lkml.org/lkml/2006/3/23/25

> I also have vague memories of some serious-sounding review comments about
> the code from Nick.

http://lkml.org/lkml/2006/3/23/55

Could that be it? Maybe repoll Con on those.

> > Why keep it in -mm for so long?
>
> I'm waiting to be shown that its benefits exceed its costs. Sometimes
> that's hard.
>

Con's patchset is used pretty widely now, with many distro's (Arch,
Mandriva, PCLinuxOS among others) providing prebuilt kernels and
nobody has reported any drawbacks. I don't think anyone in -mm has
reported any either.

James
--
iphitus // Arch Developer // kernel26beyond // iphitus.loudas.com

2007-02-09 12:59:46

by Lenar Lõhmus

[permalink] [raw]

Subject: Re: -mm merge plans for 2.6.21

Andrew Morton wrote:
> On Fri, 09 Feb 2007 11:54:29 +0200 Lenar Lõhmus <[email protected]> wrote:
> Has it? I don't think I've ever observed any benefits from it and I don't
> think anyone has ever got down and worked out what its drawbacks might be,
> and seen if they can be demonstrated in practice.
>
Who should get down? It seems no serious kernel developer cares enough
to get to it. It's just a desktop feature you know - no big iron -
uninteresting.
>> Why keep it in -mm for so long?
>>
>
> I'm waiting to be shown that its benefits exceed its costs. Sometimes
> that's hard.
>
Swap prefetch and those mystic _costs_. But nobody
hasn't communicated loud and clear what those costs are. I imagine
it must be really hard to fight to something you don't even exists.
Or prove that you are better than somebody you have never seen or heard of.

Please somebody qualified, take the time, dig that code, and show us
those costs.
With details. So Con can make the code even better and included in
mainline or
drop altogether if it's really bad and move on to other endeavours.

With best,
Lenar

2007-02-09 13:06:40

by Frederik Deweerdt

[permalink] [raw]

Subject: Re: -mm merge plans for 2.6.21

Quoting Arjan van de Ven <[email protected]>:

>
> >
> > As long as nobody takes the address of them (which wouldn't compile today
> > anyway) then the compiler should be able to not allocate store for these.
> > That they're const might help too.
>
> are you really sure?
I've run some tests, and the compiler seems to do the right thing -I must admit,
that I trusted the compiler to do it blindly on my first attempt :)- , see
below:
> cat flag.h
static const int __attribute__((__deprecated__)) SA_INTERRUPT = 0x123456;
> cat use_flag.c
#include <stdio.h>
#include "flag.h"

int main()
{
int flags = SA_INTERRUPT;
printf("%d", flags);
}
> cat dont_use_flag.c
#include <stdio.h>
#include "flag.h"

int main()
{
int flags = 0;
printf("%d", flags);
}
> gcc --version
gcc (GCC) 4.1.1 (Gentoo 4.1.1-r3)
> for i in *use_flag.c; do gcc -o $(echo $i | sed 's/.c//') -O2 -g $i; done
use_flag.c: In function 'main':
use_flag.c:6: warning: 'SA_INTERRUPT' is deprecated (declared at flag.h:1)
> size *use_flag
text data bss dec hex filename
897 260 4 1161 489 dont_use_flag
897 260 4 1161 489 use_flag

# The relevant parts of the compiled code, see how the flag is replaced with
# the const value in the code. It does not result in a code size increment.
> objdump -d {dont_,}use_flag | grep -A11 '<main>'
080483b0 <main>:
80483b0: 8d 4c 24 04 lea 0x4(%esp),%ecx
80483b4: 83 e4 f0 and $0xfffffff0,%esp
80483b7: ff 71 fc pushl 0xfffffffc(%ecx)
80483ba: 31 c0 xor %eax,%eax
80483bc: 55 push %ebp
80483bd: 89 e5 mov %esp,%ebp
80483bf: 51 push %ecx
80483c0: 83 ec 14 sub $0x14,%esp
80483c3: 89 44 24 04 mov %eax,0x4(%esp)
80483c7: c7 04 24 b8 84 04 08 movl $0x80484b8,(%esp)
80483ce: e8 05 ff ff ff call 80482d8 <printf@plt>
--
080483b0 <main>:
80483b0: 8d 4c 24 04 lea 0x4(%esp),%ecx
80483b4: 83 e4 f0 and $0xfffffff0,%esp
80483b7: ff 71 fc pushl 0xfffffffc(%ecx)
80483ba: b8 56 34 12 00 mov $0x123456,%eax
80483bf: 55 push %ebp
80483c0: 89 e5 mov %esp,%ebp
80483c2: 51 push %ecx
80483c3: 83 ec 14 sub $0x14,%esp
80483c6: 89 44 24 04 mov %eax,0x4(%esp)
80483ca: c7 04 24 b8 84 04 08 movl $0x80484b8,(%esp)
80483d1: e8 02 ff ff ff call 80482d8 <printf@plt>

======== With 3.4.3 ==========

$ gcc --version
gcc (GCC) 3.4.3 20050227 (Red Hat 3.4.3-22.fc3)
$ size {dont_,}use_flag
text data bss dec hex filename
851 256 4 1111 457 dont_use_flag
855 256 4 1115 45b use_flag
[def@bigboss ~]$ objdump -d {dont_,}use_flag | grep -A11 '<main>'
08048368 <main>:
8048368: 55 push %ebp
8048369: 89 e5 mov %esp,%ebp
804836b: 83 ec 08 sub $0x8,%esp
804836e: 83 e4 f0 and $0xfffffff0,%esp
8048371: 83 ec 18 sub $0x18,%esp
8048374: 6a 00 push $0x0
8048376: 68 64 84 04 08 push $0x8048464
804837b: e8 30 ff ff ff call 80482b0 <printf@plt>

--
08048368 <main>:
8048368: 55 push %ebp
8048369: 89 e5 mov %esp,%ebp
804836b: 83 ec 08 sub $0x8,%esp
804836e: 83 e4 f0 and $0xfffffff0,%esp
8048371: 83 ec 18 sub $0x18,%esp
8048374: 68 56 34 12 00 push $0x123456
8048379: 68 68 84 04 08 push $0x8048468
804837e: e8 2d ff ff ff call 80482b0 <printf@plt>

So I'd say that both in 3.4.3 and 4.1.1, no extra space is needed for the "const
static int" flag.

Regards,
Frederik

2007-02-09 15:03:01

by Thomas Gleixner

[permalink] [raw]

Subject: Re: -mm merge plans for 2.6.21

On Thu, 2007-02-08 at 15:44 -0800, Andrew Morton wrote:
> Yeah, this seems to work.
>
> +#define emit_old_interrupt_name(old, new) \
> +static inline unsigned __deprecated emit_old_interrupt_name##old(void) \
> +{ \
> + return (new); \
> +}
> +
> +emit_old_interrupt_name(SA_INTERRUPT, IRQF_DISABLED)
> +#define SA_INTERRUPT emit_old_interrupt_nameSA_INTERRUPT()

/me cringes, but if it makes people happy, so be it. Please update the
expiry date of this evil source of eye cancer.

tglx

diff --git a/Documentation/feature-removal-schedule.txt b/Documentation/feature-removal-schedule.txt
index 2dc5e5d..125a1c6 100644
--- a/Documentation/feature-removal-schedule.txt
+++ b/Documentation/feature-removal-schedule.txt
@@ -199,7 +199,7 @@ Who: Nick Piggin <[email protected]>
---------------------------

What: Interrupt only SA_* flags
-When: Januar 2007
+When: August 2007
Why: The interrupt related SA_* flags are replaced by IRQF_* to move them
out of the signal namespace.

2007-02-09 17:35:38

[permalink] [raw]

Subject: Re: -mm merge plans for 2.6.21

On Thu, 2007-02-08 at 15:07 -0800, Andrew Morton wrote:
> lutimesat-simplify-utime2.patch
> lutimesat-extend-do_utimes-with-flags.patch
> lutimesat-actual-syscall-and-wire-up-on-i386.patch
>
> Do we want this? Ulrich says so. Will merge, I guess.

I would strongly recommend that in the general case, you don't merge new
system calls unless the corresponding compat_ system call is
implemented.

This makes sure that people adding system calls will design the API for
the new system call appropriately, rather than trying to implement
compat support as an afterthought and only then realising that they wish
the original had been done differently. We've seen examples of this
where it would have been _trivial_ to adjust the API slightly to make
compat syscalls a non-issue, but the developer just didn't _think_ about
it until the syscall was set in stone.

This new system call seems to need compat_ support but lacks it, so I
would suggest you shouldn't merge it just yet.

--
dwmw2

2007-02-09 19:24:36

by Alan

[permalink] [raw]

Subject: Re: -mm merge plans for 2.6.21

Please just push the EDAC K8 stuff. Andi will say "no" from now until the
end of time, but end users want it, distributions want it, and Andi is
not the EDAC maintainer so should consider himself overruled on what
isn't a technical issue but a personal political viewpoint.

Alan

2007-02-09 21:45:31

[permalink] [raw]

Subject: Re: -mm merge plans for 2.6.21

On Fri, 09 Feb 2007 17:35:35 +0000
David Woodhouse <[email protected]> wrote:

> On Thu, 2007-02-08 at 15:07 -0800, Andrew Morton wrote:
> > lutimesat-simplify-utime2.patch
> > lutimesat-extend-do_utimes-with-flags.patch
> > lutimesat-actual-syscall-and-wire-up-on-i386.patch
> >
> > Do we want this? Ulrich says so. Will merge, I guess.
>
> I would strongly recommend that in the general case, you don't merge new
> system calls unless the corresponding compat_ system call is
> implemented.

Good point.

> This makes sure that people adding system calls will design the API for
> the new system call appropriately, rather than trying to implement
> compat support as an afterthought and only then realising that they wish
> the original had been done differently. We've seen examples of this
> where it would have been _trivial_ to adjust the API slightly to make
> compat syscalls a non-issue, but the developer just didn't _think_ about
> it until the syscall was set in stone.
>
> This new system call seems to need compat_ support but lacks it, so I
> would suggest you shouldn't merge it just yet.

OK, thanks.

2007-02-09 21:49:57

by Russell King

[permalink] [raw]

Subject: Re: -mm merge plans for 2.6.21

On Fri, Feb 09, 2007 at 01:45:16PM -0800, Andrew Morton wrote:
> On Fri, 09 Feb 2007 17:35:35 +0000
> David Woodhouse <[email protected]> wrote:
>
> > On Thu, 2007-02-08 at 15:07 -0800, Andrew Morton wrote:
> > > lutimesat-simplify-utime2.patch
> > > lutimesat-extend-do_utimes-with-flags.patch
> > > lutimesat-actual-syscall-and-wire-up-on-i386.patch
> > >
> > > Do we want this? Ulrich says so. Will merge, I guess.
> >
> > I would strongly recommend that in the general case, you don't merge new
> > system calls unless the corresponding compat_ system call is
> > implemented.
>
> Good point.
>
> > This makes sure that people adding system calls will design the API for
> > the new system call appropriately, rather than trying to implement
> > compat support as an afterthought and only then realising that they wish
> > the original had been done differently. We've seen examples of this
> > where it would have been _trivial_ to adjust the API slightly to make
> > compat syscalls a non-issue, but the developer just didn't _think_ about
> > it until the syscall was set in stone.
> >
> > This new system call seems to need compat_ support but lacks it, so I
> > would suggest you shouldn't merge it just yet.
>
> OK, thanks.

urgh, new system calls... wonder if they fit in the ARM ABI... Looks
fine.

Are there any other new syscalls sitting around in -mm ?

--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of:

2007-02-09 21:51:55

[permalink] [raw]

Subject: Re: -mm merge plans for 2.6.21

On Fri, 9 Feb 2007 19:37:53 +0000
Alan <[email protected]> wrote:

> Please just push the EDAC K8 stuff.

OK.

> Andi will say "no" from now until the
> end of time, but end users want it, distributions want it, and Andi is
> not the EDAC maintainer so should consider himself overruled on what
> isn't a technical issue but a personal political viewpoint.

I'll just tell him I sent it by accident.

2007-02-09 21:53:42

[permalink] [raw]

Subject: Re: -mm merge plans for 2.6.21

On Fri, 2007-02-09 at 21:49 +0000, Russell King wrote:
> urgh, new system calls... wonder if they fit in the ARM ABI... Looks
> fine.

Can you elucidate on _what_ you just checked for?

There was something about alignment of 64-bit arguments to syscalls
which affects MIPS and/or ARM which I can't quite remember-- something
about putting it them arguments in either an even-numbered or
odd-numbered position to make it work nicely. Is that actually written
anywhere, and does anyone bother to check?

--
dwmw2

2007-02-09 21:59:10

[permalink] [raw]

Subject: Re: -mm merge plans for 2.6.21

On Fri, 2007-02-09 at 13:45 -0800, Andrew Morton wrote:
> > I would strongly recommend that in the general case, you don't merge new
> > system calls unless the corresponding compat_ system call is
> > implemented.
>
> Good point.

It's a _damn_ good point, but I see we went ahead and merged
sys_epoll_pwait without it anyway -- despite the fact that it's
include/linux/eventpoll.h which contains the example of why we should
think first :)

I think I even threw together an untested implementation of
compat_sys_epoll_pwait() at one point to assist with that task, but it
didn't seem to help much.

--
dwmw2

2007-02-09 22:01:43

[permalink] [raw]

Subject: Re: -mm merge plans for 2.6.21

On Fri, 9 Feb 2007 21:49:44 +0000
Russell King <[email protected]> wrote:

> On Fri, Feb 09, 2007 at 01:45:16PM -0800, Andrew Morton wrote:
> > On Fri, 09 Feb 2007 17:35:35 +0000
> > David Woodhouse <[email protected]> wrote:
> >
> > > On Thu, 2007-02-08 at 15:07 -0800, Andrew Morton wrote:
> > > > lutimesat-simplify-utime2.patch
> > > > lutimesat-extend-do_utimes-with-flags.patch
> > > > lutimesat-actual-syscall-and-wire-up-on-i386.patch
> > > >
> > > > Do we want this? Ulrich says so. Will merge, I guess.
> > >
> > > I would strongly recommend that in the general case, you don't merge new
> > > system calls unless the corresponding compat_ system call is
> > > implemented.
> >
> > Good point.
> >
> > > This makes sure that people adding system calls will design the API for
> > > the new system call appropriately, rather than trying to implement
> > > compat support as an afterthought and only then realising that they wish
> > > the original had been done differently. We've seen examples of this
> > > where it would have been _trivial_ to adjust the API slightly to make
> > > compat syscalls a non-issue, but the developer just didn't _think_ about
> > > it until the syscall was set in stone.
> > >
> > > This new system call seems to need compat_ support but lacks it, so I
> > > would suggest you shouldn't merge it just yet.
> >
> > OK, thanks.
>
> urgh, new system calls... wonder if they fit in the ARM ABI... Looks
> fine.

What's the ARM ABI?

> Are there any other new syscalls sitting around in -mm ?

Yes, the AIO listio additions:

From: Bharata B Rao <[email protected]>

This patch provides POSIX listio support by means of a new system call.

long lio_submit(aio_context_t ctx_id, int mode, long nr,
struct iocb __user * __user *iocbpp, struct sigevent __user *event)

This system call is similar to the io_submit() system call, but takes
two more arguments.

'mode' argument can be LIO_WAIT or LIO_NOWAIT.

'event' argument specifies the signal notification mechanism.

This patch is built on support provided by the aio signal notification
patch by Sebastien. The following two structures together provide
the support for grouping iocbs belonging to a list (lio).

struct aio_notify {
struct task_struct *target;
__u16 signo;
__u16 notify;
sigval_t value;
struct sigqueue *sigq;
};

struct lio_event {
atomic_t lio_users;
struct aio_notify lio_notify;
};

A single lio_event struct is maintained for the list of iocbs. lio_users
holds the number of requests attached to this lio and lio_notify has the
necessary information for generating completion notification signal.

If the mode is LIO_WAIT, the event argument is ignored and the system call
waits until all the requests of the lio are completed.

If the mode is LIO_NOWAIT, the system call returns immediately after
submitting the io requests and may optionally notify the process on list io
completion depending on the event argument.

Signed-off-by: S_bastien Dugu_ <[email protected]>
Signed-off-by: Laurent Vivier <[email protected]>
Signed-off-by: Bharata B Rao <[email protected]>
Cc: Bharata B Rao <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Suparna Bhattacharya <[email protected]>
Cc: Zach Brown <[email protected]>
Cc: Oleg Nesterov <[email protected]>
Cc: Badari Pulavarty <[email protected]>
Cc: Benjamin LaHaise <[email protected]>
Cc: Jean Pierre Dion <[email protected]>
Cc: Andi Kleen <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
---

arch/i386/kernel/syscall_table.S | 1
arch/x86_64/ia32/ia32entry.S | 4
fs/aio.c | 188 +++++++++++++++++++++++++----
fs/compat.c | 119 +++++++++++++++---
include/asm-i386/unistd.h | 3
include/asm-x86_64/unistd.h | 4
include/linux/aio.h | 14 +-
include/linux/aio_abi.h | 5
include/linux/syscalls.h | 2
9 files changed, 295 insertions(+), 45 deletions(-)

diff -puN arch/i386/kernel/syscall_table.S~add-listio-syscall-support arch/i386/kernel/syscall_table.S
--- a/arch/i386/kernel/syscall_table.S~add-listio-syscall-support
+++ a/arch/i386/kernel/syscall_table.S
@@ -319,3 +319,4 @@ ENTRY(sys_call_table)
.long sys_move_pages
.long sys_getcpu
.long sys_epoll_pwait
+ .long sys_lio_submit /* 320 */
diff -puN arch/x86_64/ia32/ia32entry.S~add-listio-syscall-support arch/x86_64/ia32/ia32entry.S
--- a/arch/x86_64/ia32/ia32entry.S~add-listio-syscall-support
+++ a/arch/x86_64/ia32/ia32entry.S
@@ -726,8 +726,10 @@ ia32_sys_call_table:
.quad compat_sys_get_robust_list
.quad sys_splice
.quad sys_sync_file_range
- .quad sys_tee
+ .quad sys_tee /* 315 */
.quad compat_sys_vmsplice
.quad compat_sys_move_pages
.quad sys_getcpu
+ .quad quiet_ni_syscall /* sys_epoll_wait */
+ .quad compat_sys_lio_submit
ia32_syscall_end:
diff -puN fs/aio.c~add-listio-syscall-support fs/aio.c
--- a/fs/aio.c~add-listio-syscall-support
+++ a/fs/aio.c
@@ -416,6 +416,7 @@ static struct kiocb fastcall *__aio_get_
req->ki_ctx = ctx;
req->ki_cancel = NULL;
req->ki_retry = NULL;
+ req->ki_lio = NULL;
req->ki_dtor = NULL;
req->private = NULL;
req->ki_iovec = NULL;
@@ -997,6 +998,72 @@ out_unlock:
return -EINVAL;
}

+/*
+ * lio_notify
+ * When all iocbs belonging to an lio are complete, this initiates
+ * the completion notification for the lio.
+ *
+ * NOTE: lio is freed here after the last iocb completes, but only
+ * for LIO_NOWAIT case. In case of LIO_WAIT, the submitting thread
+ * waits for iocbs to complete and later frees the lio.
+ */
+void lio_notify(struct lio_event *lio)
+{
+ int ret;
+
+ ret = atomic_dec_and_test(&lio->lio_users);
+
+ if (unlikely(ret) && lio->lio_notify.notify != SIGEV_NONE) {
+ /* last one -> notify process */
+ if (aio_send_signal(&lio->lio_notify))
+ __sigqueue_free(lio->lio_notify.sigq);
+ kfree(lio);
+ }
+}
+
+/*
+ * lio_create
+ * Allocates a lio_event structure which groups the iocbs belonging
+ * to the lio and sets up the completion notification.
+ *
+ * Case 1: LIO_NOWAIT and NULL sigevent - No need to group the iocbs
+ * and hence lio_event is not created.
+ * Case 2: LIO_NOWAIT and sigvent - lio_event is created and notification
+ * mechanism is setup.
+ * Case 3: LIO_WAIT - lio_event is created and sigevent is ignored.
+ */
+struct lio_event *lio_create(struct sigevent __user *user_event,
+ int mode)
+{
+ int ret = 0;
+ struct lio_event *lio = NULL;
+
+ if (unlikely((mode == LIO_NOWAIT) && !user_event))
+ return lio;
+
+ lio = kzalloc(sizeof(*lio), GFP_KERNEL);
+
+ if (!lio)
+ return ERR_PTR(-EAGAIN);
+
+ /*
+ * Grab an initial ref on the lio to avoid races between
+ * submission and completion.
+ */
+ atomic_set(&lio->lio_users, 1);
+
+ lio->lio_notify.notify = SIGEV_NONE;
+
+ if (user_event && (mode == LIO_NOWAIT)) {
+ ret = aio_setup_sigevent(&lio->lio_notify, user_event);
+ if (ret) {
+ kfree(lio);
+ return ERR_PTR(ret);
+ }
+ }
+ return lio;
+}
+
/* aio_complete
* Called when the io request on the given iocb is complete.
* Returns true if this is the last user of the request. The
@@ -1045,6 +1112,9 @@ int fastcall aio_complete(struct kiocb *
* when the event got cancelled.
*/
if (kiocbIsCancelled(iocb)) {
+ if (iocb->ki_lio)
+ lio_notify(iocb->ki_lio);
+
if (iocb->ki_notify.sigq)
__sigqueue_free(iocb->ki_notify.sigq);
goto put_rq;
@@ -1087,6 +1157,8 @@ int fastcall aio_complete(struct kiocb *
__sigqueue_free(iocb->ki_notify.sigq);
}

+ if (iocb->ki_lio)
+ lio_notify(iocb->ki_lio);
put_rq:
/* everything turned out well, dispose of the aiocb. */
ret = __aio_put_req(ctx, iocb);
@@ -1631,7 +1703,7 @@ static int aio_wake_function(wait_queue_
}

int fastcall io_submit_one(struct kioctx *ctx, struct iocb __user *user_iocb,
- struct iocb *iocb)
+ struct iocb *iocb, struct lio_event *lio)
{
struct kiocb *req;
struct file *file;
@@ -1692,6 +1764,9 @@ int fastcall io_submit_one(struct kioctx
goto out_sigqfree;
}

+ /* Attach this iocb to its lio */
+ req->ki_lio = lio;
+
ret = aio_setup_iocb(req);

if (ret)
@@ -1719,6 +1794,48 @@ out_put_req:
return ret;
}

+static int io_submit_group(struct kioctx *ctx, long nr,
+ struct iocb __user * __user *iocbpp, struct lio_event *lio)
+{
+ int i;
+ long ret = 0;
+
+ /*
+ * AKPM: should this return a partial result if some of the IOs were
+ * successfully submitted?
+ */
+ for (i = 0; i < nr; i++) {
+ struct iocb __user *user_iocb;
+ struct iocb tmp;
+
+ if (unlikely(__get_user(user_iocb, iocbpp + i))) {
+ ret = -EFAULT;
+ break;
+ }
+
+ if (unlikely(copy_from_user(&tmp, user_iocb, sizeof(tmp)))) {
+ ret = -EFAULT;
+ break;
+ }
+
+ if (lio)
+ atomic_inc(&lio->lio_users);
+
+ ret = io_submit_one(ctx, user_iocb, &tmp, lio);
+ if (ret) {
+ if (lio) {
+ /*
+ * In case of listio, continue with
+ * the subsequent requests
+ */
+ atomic_dec(&lio->lio_users);
+ } else
+ break;
+ }
+ }
+ return i ? i : ret;
+}
+
/* sys_io_submit:
* Queue the nr iocbs pointed to by iocbpp for processing. Returns
* the number of iocbs queued. May return -EINVAL if the aio_context
@@ -1736,7 +1853,6 @@ asmlinkage long sys_io_submit(aio_contex
{
struct kioctx *ctx;
long ret = 0;
- int i;

if (unlikely((nr*sizeof(*iocbpp)) < 0))
return -EINVAL;
@@ -1750,31 +1866,61 @@ asmlinkage long sys_io_submit(aio_contex
return -EINVAL;
}

- /*
- * AKPM: should this return a partial result if some of the IOs were
- * successfully submitted?
- */
- for (i=0; i<nr; i++) {
- struct iocb __user *user_iocb;
- struct iocb tmp;
+ ret = io_submit_group(ctx, nr, iocbpp, NULL);

- if (unlikely(__get_user(user_iocb, iocbpp + i))) {
- ret = -EFAULT;
- break;
- }
+ put_ioctx(ctx);
+ return ret;
+}

- if (unlikely(copy_from_user(&tmp, user_iocb, sizeof(tmp)))) {
- ret = -EFAULT;
- break;
- }
+asmlinkage long sys_lio_submit(aio_context_t ctx_id, int mode, long nr,
+ struct iocb __user * __user *iocbpp, struct sigevent __user *event)
+{
+ struct kioctx *ctx;
+ struct lio_event *lio = NULL;
+ long ret = 0;

- ret = io_submit_one(ctx, user_iocb, &tmp);
- if (ret)
- break;
+ if (unlikely((nr*sizeof(*iocbpp)) < 0))
+ return -EINVAL;
+
+ if (unlikely(!access_ok(VERIFY_READ, iocbpp, (nr*sizeof(*iocbpp)))))
+ return -EFAULT;
+
+ ctx = lookup_ioctx(ctx_id);
+ if (unlikely(!ctx)) {
+ pr_debug("EINVAL: lio_submit: invalid context id\n");
+ return -EINVAL;
+ }
+
+ lio = lio_create(event, mode);
+
+ ret = PTR_ERR(lio);
+ if (IS_ERR(lio))
+ goto out_put_ctx;
+
+ ret = io_submit_group(ctx, nr, iocbpp, lio);
+
+ /* If we failed to submit even one request just return */
+ if (ret < 0 ) {
+ if (lio)
+ kfree(lio);
+ goto out_put_ctx;
+ }
+
+ /*
+ * Drop extra ref on the lio now that we're done submitting requests.
+ */
+ if (lio)
+ lio_notify(lio);
+
+ if (mode == LIO_WAIT) {
+ wait_event(ctx->wait, atomic_read(&lio->lio_users) == 0);
+ kfree(lio);
}

+out_put_ctx:
put_ioctx(ctx);
- return i ? i : ret;
+
+ return ret;
}

/* lookup_kiocb
diff -puN fs/compat.c~add-listio-syscall-support fs/compat.c
--- a/fs/compat.c~add-listio-syscall-support
+++ a/fs/compat.c
@@ -644,24 +644,13 @@ out:
return ret;
}

-asmlinkage long
-compat_sys_io_submit(aio_context_t ctx_id, int nr, u32 __user *iocb)
+static int compat_io_submit_group(struct kioctx *ctx, long nr,
+ u32 __user *iocb, struct lio_event *lio)
{
- struct kioctx *ctx;
- long ret = 0;
int i;
+ long ret = 0;

- if (unlikely((nr * sizeof(u32)) < 0))
- return -EINVAL;
-
- if (unlikely(!access_ok(VERIFY_READ, iocb, (nr * sizeof(u32)))))
- return -EFAULT;
-
- ctx = lookup_ioctx(ctx_id);
- if (unlikely(!ctx))
- return -EINVAL;
-
- for (i=0; i<nr; i++) {
+ for (i = 0; i < nr; i++) {
compat_uptr_t uptr;
struct iocb __user *user_iocb;
struct iocb tmp;
@@ -696,13 +685,105 @@ compat_sys_io_submit(aio_context_t ctx_i
tmp.aio_sigeventp = (__u64)event;
}

- ret = io_submit_one(ctx, user_iocb, &tmp);
- if (ret)
- break;
+ if (lio)
+ atomic_inc(&lio->lio_users);
+
+ ret = io_submit_one(ctx, user_iocb, &tmp, lio);
+ if (ret) {
+ if (lio) {
+ /*
+ * In case of listio, continue with
+ * the subsequent requests
+ */
+ atomic_dec(&lio->lio_users);
+ } else
+ break;
+ }
+
+
}
+ return i ? i : ret;
+}

+asmlinkage long
+compat_sys_io_submit(aio_context_t ctx_id, int nr, u32 __user *iocb)
+{
+ struct kioctx *ctx;
+ long ret = 0;
+
+ if (unlikely((nr*sizeof(u32)) < 0))
+ return -EINVAL;
+
+ if (unlikely(!access_ok(VERIFY_READ, iocb, (nr * sizeof(u32)))))
+ return -EFAULT;
+
+ ctx = lookup_ioctx(ctx_id);
+ if (unlikely(!ctx))
+ return -EINVAL;
+
+ ret = compat_io_submit_group(ctx, nr, iocb, NULL);
put_ioctx(ctx);
- return i ? i: ret;
+ return ret;
+}
+
+asmlinkage long
+compat_sys_lio_submit(aio_context_t ctx_id, int mode, int nr, u32 __user *iocb,
+ struct compat_sigevent __user *sig_user)
+{
+ struct kioctx *ctx;
+ struct lio_event *lio = NULL;
+ struct sigevent __user *event = NULL;
+ long ret = 0;
+
+ if (unlikely((nr*sizeof(u32)) < 0))
+ return -EINVAL;
+
+ if (unlikely(!access_ok(VERIFY_READ, iocb, (nr * sizeof(u32)))))
+ return -EFAULT;
+
+ ctx = lookup_ioctx(ctx_id);
+ if (unlikely(!ctx))
+ return -EINVAL;
+
+ if (sig_user) {
+ struct sigevent kevent;
+ event = compat_alloc_user_space(sizeof(struct sigevent));
+ if (get_compat_sigevent(&kevent, sig_user) ||
+ copy_to_user(event, &kevent, sizeof(struct sigevent))) {
+ ret = -EFAULT;
+ goto out_put_ctx;
+ }
+ }
+
+ lio = lio_create(event, mode);
+
+ ret = PTR_ERR(lio);
+ if (IS_ERR(lio))
+ goto out_put_ctx;
+
+ ret = compat_io_submit_group(ctx, nr, iocb, lio);
+
+ /* If we failed to submit even one request just return */
+ if (ret < 0) {
+ if (lio)
+ kfree(lio);
+ goto out_put_ctx;
+ }
+
+ /*
+ * Drop extra ref on the lio now that we're done submitting requests.
+ */
+ if (lio)
+ lio_notify(lio);
+
+ if (mode == LIO_WAIT) {
+ wait_event(ctx->wait, atomic_read(&lio->lio_users) == 0);
+ kfree(lio);
+ }
+
+out_put_ctx:
+ put_ioctx(ctx);
+ return ret;
}

struct compat_ncp_mount_data {
diff -puN include/asm-i386/unistd.h~add-listio-syscall-support include/asm-i386/unistd.h
--- a/include/asm-i386/unistd.h~add-listio-syscall-support
+++ a/include/asm-i386/unistd.h
@@ -325,10 +325,11 @@
#define __NR_move_pages 317
#define __NR_getcpu 318
#define __NR_epoll_pwait 319
+#define __NR_lio_submit 320

#ifdef __KERNEL__

-#define NR_syscalls 320
+#define NR_syscalls 321

#define __ARCH_WANT_IPC_PARSE_VERSION
#define __ARCH_WANT_OLD_READDIR
diff -puN include/asm-x86_64/unistd.h~add-listio-syscall-support include/asm-x86_64/unistd.h
--- a/include/asm-x86_64/unistd.h~add-listio-syscall-support
+++ a/include/asm-x86_64/unistd.h
@@ -619,8 +619,10 @@ __SYSCALL(__NR_sync_file_range, sys_sync
__SYSCALL(__NR_vmsplice, sys_vmsplice)
#define __NR_move_pages 279
__SYSCALL(__NR_move_pages, sys_move_pages)
+#define __NR_lio_submit 280
+__SYSCALL(__NR_lio_submit, sys_lio_submit)

-#define __NR_syscall_max __NR_move_pages
+#define __NR_syscall_max __NR_lio_submit

#ifndef __NO_STUBS
#define __ARCH_WANT_OLD_READDIR
diff -puN include/linux/aio.h~add-listio-syscall-support include/linux/aio.h
--- a/include/linux/aio.h~add-listio-syscall-support
+++ a/include/linux/aio.h
@@ -63,6 +63,11 @@ struct aio_notify {
struct sigqueue *sigq;
};

+struct lio_event {
+ atomic_t lio_users;
+ struct aio_notify lio_notify;
+};
+
/* is there a better place to document function pointer methods? */
/**
* ki_retry - iocb forward progress callback
@@ -117,6 +122,8 @@ struct kiocb {
__u64 ki_user_data; /* user's data for completion */
struct wait_bit_queue ki_wait;
loff_t ki_pos;
+ /* lio this iocb might be attached to */
+ struct lio_event *ki_lio;

atomic_t ki_bio_count; /* num bio used for this iocb */
void *private;
@@ -225,12 +232,15 @@ struct mm_struct;
extern void FASTCALL(exit_aio(struct mm_struct *mm));
extern struct kioctx *lookup_ioctx(unsigned long ctx_id);
extern int FASTCALL(io_submit_one(struct kioctx *ctx,
- struct iocb __user *user_iocb, struct iocb *iocb));
+ struct iocb __user *user_iocb, struct iocb *iocb,
+ struct lio_event *lio));
+struct lio_event *lio_create(struct sigevent __user *user_event, int mode);
+void lio_notify(struct lio_event *lio);

/* semi private, but used by the 32bit emulations: */
struct kioctx *lookup_ioctx(unsigned long ctx_id);
int FASTCALL(io_submit_one(struct kioctx *ctx, struct iocb __user *user_iocb,
- struct iocb *iocb));
+ struct iocb *iocb, struct lio_event *lio));

#define get_ioctx(kioctx) do { \
BUG_ON(atomic_read(&(kioctx)->users) <= 0); \
diff -puN include/linux/aio_abi.h~add-listio-syscall-support include/linux/aio_abi.h
--- a/include/linux/aio_abi.h~add-listio-syscall-support
+++ a/include/linux/aio_abi.h
@@ -45,6 +45,11 @@ enum {
IOCB_CMD_PWRITEV = 8,
};

+enum {
+ LIO_WAIT = 0,
+ LIO_NOWAIT = 1,
+};
+
/* read() from /dev/aio returns these structures. */
struct io_event {
__u64 data; /* the data field from the iocb */
diff -puN include/linux/syscalls.h~add-listio-syscall-support include/linux/syscalls.h
--- a/include/linux/syscalls.h~add-listio-syscall-support
+++ a/include/linux/syscalls.h
@@ -317,6 +317,8 @@ asmlinkage long sys_io_getevents(aio_con
struct timespec __user *timeout);
asmlinkage long sys_io_submit(aio_context_t, long,
struct iocb __user * __user *);
+asmlinkage long sys_lio_submit(aio_context_t, int, long,
+ struct iocb __user * __user *, struct sigevent __user *);
asmlinkage long sys_io_cancel(aio_context_t ctx_id, struct iocb __user *iocb,
struct io_event __user *result);
asmlinkage ssize_t sys_sendfile(int out_fd, int in_fd,
_

2007-02-09 22:03:36

by Russell King

[permalink] [raw]

Subject: Re: -mm merge plans for 2.6.21

On Fri, Feb 09, 2007 at 09:53:19PM +0000, David Woodhouse wrote:
> On Fri, 2007-02-09 at 21:49 +0000, Russell King wrote:
> > urgh, new system calls... wonder if they fit in the ARM ABI... Looks
> > fine.
>
> Can you elucidate on _what_ you just checked for?
>
> There was something about alignment of 64-bit arguments to syscalls
> which affects MIPS and/or ARM which I can't quite remember--

It's something like that.

> something
> about putting it them arguments in either an even-numbered or
> odd-numbered position to make it work nicely.

We have a maximum of 7 32-bit registers to pass system call arguments.
In EABI, 64-bit arguments must be aligned to an even-numbered register
(starting at zero). In OABI, 64-bit arguments do not have this
restriction.

So, for EABI:

sys_foo(int a, unsigned long long b, int c, unsigned long long d)

would allocate 'a' into r0, 'b' into r2,r3, 'c' into r4, and 'd' into
r6, and oops we're out of registers - so the above syscall prototype
can't be supported on ARM.

However:

sys_foo(int a, int c, unsigned long long b, unsigned long long d)

is entirely reasonable and leaves us with spare room for one additional
32-bit arg to be passed.

> Is that actually written anywhere, and does anyone bother to check?

Mostly mailing list archives I'd guess. As far as anyone bothering
to check, that's me when I'm aware of new syscalls... which typically
happens a long time after the syscalls have been introduced on x86
etc.

--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of:

2007-02-09 22:06:32

by Russell King

[permalink] [raw]

Subject: Re: -mm merge plans for 2.6.21

On Fri, Feb 09, 2007 at 02:00:49PM -0800, Andrew Morton wrote:
> +asmlinkage long sys_lio_submit(aio_context_t ctx_id, int mode, long nr,
> + struct iocb __user * __user *iocbpp, struct sigevent __user *event)

Not a problem. (r0 := ctx_id, r1 := mode, r2 := nr, r3 := iocbpp,
r4 := event) Thanks.

--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of:

2007-02-09 22:12:37

[permalink] [raw]

Subject: Re: -mm merge plans for 2.6.21

On Fri, 2007-02-09 at 22:03 +0000, Russell King wrote:
> > Is that actually written anywhere, and does anyone bother to check?
>
> Mostly mailing list archives I'd guess. As far as anyone bothering
> to check, that's me when I'm aware of new syscalls... which typically
> happens a long time after the syscalls have been introduced on x86
> etc.

I suspect we could do with a Documentation/syscalls.txt collecting such
rules from various architectures.

We could _also_ do with a way to warn about unimplemented syscalls on
any given architecture. I'm thinking about something along the lines of
a kernel/syscalls.c containing nothing but...

#include <asm/unistd.h>

#ifndef __NR_sys_foo
#warning The sys_foo system call is not implemented on this architecture
#endif

Ideally, that wants to be auto-generated from the union of all
<asm-*/unistd.h> files, but in practice I suspect we could do it just
from <asm-i386/unistd.h>. Even I usually manage to add new syscalls on
i386 after I've done PowerPC.

--
dwmw2

2007-02-09 22:18:32

by Guillaume Chazarain

[permalink] [raw]

Subject: Re: -mm merge plans for 2.6.21

Andrew Morton a ?crit :
> factor-outstanding-i-o-error-handling.patch
> factor-outstanding-i-o-error-handling-tidy.patch
>

I still like them. But the original problem is still present.

> sync_sb_inodes-propagate-errors.patch
>

You explained in http://lkml.org/lkml/2007/1/2/238 that the mapping
flags should be set at the lowest level, but with this change I have a
hard time choosing a place to stick it. I don't like when a function
both sets the mapping flags and returns an error code, I think it should
be mutually exclusive, so that we know what to do (propagate the return
code?) and what to expect (are the mapping flags up to date?), which
seemed to be the case before this patch. For instance there is no point
in propagating an error return code up to sys_sync() as it can only drop it.

The call trace that cleared the flags, the origin of the problem, is:

void do_sync(1)
void sync_inodes(1)
void __sync_inodes(1)
void sync_inodes_sb(sb, 1)
void sync_sb_inodes(sb, WB_SYNC_ALL)
int __writeback_single_inode(inode, WB_SYNC_ALL)
int __sync_single_inode(inode, WB_SYNC_ALL)
int filemap_fdatawait(mapping)
int wait_on_page_writeback_range(mapping)
int test_and_clear(...)

re-setting the flag at a too low level, would mean it is still set after
a msync() or fsync() that could return the status to its caller. My
interpretation is that low level functions up to
__writeback_single_inode() can be used by fsync() and the like that can
return the error to their caller, unlike high level functions starting
at sync_sb_inodes() that don't need a return value as their caller can
do nothing with it. So re-setting the flag in sync_sb_inodes() just
after __writeback_single_inode() looks right to me, and I propose the
exact same patch as the first time.

> block_write_full_page-handle-enospc.patch
>

It seems to me that __block_write_full_page is always called more or
less directly from __mpage_writepage, and the latter handles enospc in
the mapping flags. So I am not sure this patch is needed.

Thanks.

--
Guillaume

Attachments:

sync_sb_inodes_handle_error.patch (2.15 kB)

2007-02-09 22:42:59

[permalink] [raw]

Subject: Re: -mm merge plans for 2.6.21

On Fri, 2007-02-09 at 22:12 +0000, David Woodhouse wrote:
>
> We could _also_ do with a way to warn about unimplemented syscalls on
> any given architecture. I'm thinking about something along the lines
> of
> a kernel/syscalls.c containing nothing but...
>
> #include <asm/unistd.h>
>
> #ifndef __NR_sys_foo
> #warning The sys_foo system call is not implemented on this
> architecture
> #endif
>
> Ideally, that wants to be auto-generated from the union of all
> <asm-*/unistd.h> files, but in practice I suspect we could do it just
> from <asm-i386/unistd.h>. Even I usually manage to add new syscalls on
> i386 after I've done PowerPC.

Realistically speaking, I'm not going to have time to do anything useful
with this before I disappear for a week starting tomorrow. But in case
it helps, I'll throw this in to see if it inspires anyone...

( echo -e "#include <asm/unistd.h>\n#include
<asm-generic/optional-syscalls.h>" ; sed -n '/^#define/s/[^_]*__NR_
$[^[:space:]]*$.*/#if !defined (__NR_\1) \&\& !defined (__IGNORE_
\1)\n#warning System call \1 is not implemented\n#endif/p'
include/asm-i386/unistd.h ) > kernel/syscalls.c

Coupled with an include/optional-syscalls.h which defines __IGNORE_vm86,
__IGNORE_vm86old and then on 64-bit machines a bunch more
__IGNORE_stat64 et al., that should mostly do the trick...

--
dwmw2

2007-02-09 22:50:20

by Davide Libenzi

[permalink] [raw]

Subject: Re: -mm merge plans for 2.6.21

On Fri, 9 Feb 2007, David Woodhouse wrote:

> On Fri, 2007-02-09 at 13:45 -0800, Andrew Morton wrote:
> > > I would strongly recommend that in the general case, you don't merge new
> > > system calls unless the corresponding compat_ system call is
> > > implemented.
> >
> > Good point.
>
> It's a _damn_ good point, but I see we went ahead and merged
> sys_epoll_pwait without it anyway -- despite the fact that it's
> include/linux/eventpoll.h which contains the example of why we should
> think first :)
>
> I think I even threw together an untested implementation of
> compat_sys_epoll_pwait() at one point to assist with that task, but it
> didn't seem to help much.

Damn! I always forget. Doing it right now ...

- Davide

2007-02-10 01:14:23

by Carl-Daniel Hailfinger

[permalink] [raw]

Subject: Re: -mm merge plans for 2.6.21

Andrew Morton wrote:
> On Fri, 9 Feb 2007 19:37:53 +0000
> Alan <[email protected]> wrote:
>
>> Please just push the EDAC K8 stuff.
>
> OK.
>
>> Andi will say "no" from now until the
>> end of time, but end users want it, distributions want it, and Andi is
>> not the EDAC maintainer so should consider himself overruled on what
>> isn't a technical issue but a personal political viewpoint.
>
> I'll just tell him I sent it by accident.

Could you please merge ACPI-DSDT-in-initrd for the same reasons?

Regards,
Carl-Daniel

2007-02-10 01:29:42

[permalink] [raw]

Subject: Re: -mm merge plans for 2.6.21

On Sat, 10 Feb 2007 02:15:11 +0100
Carl-Daniel Hailfinger <[email protected]> wrote:

> Andrew Morton wrote:
> > On Fri, 9 Feb 2007 19:37:53 +0000
> > Alan <[email protected]> wrote:
> >
> >> Please just push the EDAC K8 stuff.
> >
> > OK.
> >
> >> Andi will say "no" from now until the
> >> end of time, but end users want it, distributions want it, and Andi is
> >> not the EDAC maintainer so should consider himself overruled on what
> >> isn't a technical issue but a personal political viewpoint.
> >
> > I'll just tell him I sent it by accident.
>
> Could you please merge ACPI-DSDT-in-initrd for the same reasons?
>

I don't know what that is.

2007-02-10 01:57:16

by Oleg Verych

[permalink] [raw]

Subject: Re: -mm merge plans for 2.6.21

> From: Russell King
> Newsgroups: gmane.linux.kernel
> Subject: Re: -mm merge plans for 2.6.21
> Date: Fri, 9 Feb 2007 22:03:27 +0000
[]
> However:
>
> sys_foo(int a, int c, unsigned long long b, unsigned long long d)
>
> is entirely reasonable and leaves us with spare room for one additional
> 32-bit arg to be passed.
>
>> Is that actually written anywhere, and does anyone bother to check?
>
> Mostly mailing list archives I'd guess. As far as anyone bothering
> to check, that's me when I'm aware of new syscalls... which typically
> happens a long time after the syscalls have been introduced on x86
> etc.

Why not to have "the most large argument first" rule here?

sys_bar(largest,..., larger,..., smaller,..., small);

Put it in Documentation/ABI/README and bother only, when compiller
will bark on -mm tree.

____

2007-02-10 10:00:12

by Heiko Carstens

[permalink] [raw]

Subject: Re: -mm merge plans for 2.6.21 -- md-dm-reduce-stack-usage-with-stacked-block-devices.patch

On Thu, Feb 08, 2007 at 03:07:10PM -0800, Andrew Morton wrote:
> drivers-mdc-use-array_size-macro-when-appropriate.patch
> md-dm-reduce-stack-usage-with-stacked-block-devices.patch
>
> -> neilb
>
> (The second one is getting idiotic. When are we going to fix this??)

Since it was me who asked to have the stack usage reduced and Neil was
kind enough to fix this, I'm wondering what the state of dm is.

I think more than a year ago we had this:

Alasdair G Kergon <[email protected]> said:

I can see nothing wrong with this in principle.

For device-mapper at the moment though it's essential that, while the bio
mappings may now get delayed, they still get processed in exactly
the same order as they were passed to generic_make_request().

My main concern is whether the timing changes implicit in this patch
will make the rare data-corrupting races in the existing snapshot code
more likely. (I'm working on a fix for these races, but the unfinished
patch is already several hundred lines long.)

It would be helpful if some people on this mailing list would test
this patch in various scenarios and report back.

Alasdair, is dm already fixed or is there any chance that it will
ever get fixed?

2007-02-10 10:23:17

by Heiko Carstens

[permalink] [raw]

Subject: Re: -mm merge plans for 2.6.21

On Fri, Feb 09, 2007 at 02:50:12PM -0800, Davide Libenzi wrote:
> On Fri, 9 Feb 2007, David Woodhouse wrote:
>
> > On Fri, 2007-02-09 at 13:45 -0800, Andrew Morton wrote:
> > > > I would strongly recommend that in the general case, you don't merge new
> > > > system calls unless the corresponding compat_ system call is
> > > > implemented.
> > >
> > > Good point.
> >
> > It's a _damn_ good point, but I see we went ahead and merged
> > sys_epoll_pwait without it anyway -- despite the fact that it's
> > include/linux/eventpoll.h which contains the example of why we should
> > think first :)
> >
> > I think I even threw together an untested implementation of
> > compat_sys_epoll_pwait() at one point to assist with that task, but it
> > didn't seem to help much.
>
> Damn! I always forget. Doing it right now ...

Which remembers me that I think that MIPS is using the non-compat version
of sys_epoll_pwait for compat syscalls. But maybe MIPS doesn't need a compat
syscall for some reason. Dunno.

2007-02-10 10:32:28

[permalink] [raw]

Subject: Re: -mm merge plans for 2.6.21

On Sat, 2007-02-10 at 11:22 +0100, Heiko Carstens wrote:
> Which remembers me that I think that MIPS is using the non-compat version
> of sys_epoll_pwait for compat syscalls. But maybe MIPS doesn't need a compat
> syscall for some reason. Dunno.

It's OK as long as the 64-bit kernel, N32 and O32 userspace all agree
there there's 32 bits of padding between the fields of this structure:

struct epoll_event {
__u32 events;
__u64 data;
};

I suspect it's a fairly safe bet that N32 userspace agrees; if the O32
ABI is different then it would need the compat syscall.

--
dwmw2

2007-02-10 11:43:04

by Arnd Bergmann

[permalink] [raw]

Subject: Re: -mm merge plans for 2.6.21

On Friday 09 February 2007 12:24, Arjan van de Ven wrote:
>
> On Fri, 2007-02-09 at 10:57 +0000, Frederik Deweerdt wrote:
> > +static const int __deprecated SA_INTERRUPT = IRQF_DISABLED;
> > +static const int __deprecated SA_SAMPLE_RANDOM = IRQF_SAMPLE_RANDOM;
> > +static const int __deprecated SA_SHIRQ = IRQF_SHARED;
> > +static const int __deprecated SA_PROBEIRQ = IRQF_PROBE_SHARED;
> > +static const int __deprecated SA_PERCPU = IRQF_PERCPU;
> > +
> > +static const int __deprecated SA_TRIGGER_LOW = IRQF_TRIGGER_LOW;
> > +static const int __deprecated SA_TRIGGER_HIGH = IRQF_TRIGGER_HIGH;
> > +static const int __deprecated SA_TRIGGER_FALLING = IRQF_TRIGGER_FALLING;
> > +static const int __deprecated SA_TRIGGER_RISING = IRQF_TRIGGER_RISING;
> > +static const int __deprecated SA_TRIGGER_MASK = IRQF_TRIGGER_MASK;
>
> this will include these in every .o file for which the .c file includes
> the header. NOT GOOD(tm)
>

How about this one instead then:

Mark SA_* constants as deprecated

Signed-off-by: Arnd Bergmann <[email protected]>

--- a/include/linux/interrupt.h
+++ b/include/linux/interrupt.h
@@ -53,17 +53,19 @@
* Migration helpers. Scheduled for removal in 1/2007
* Do not use for new code !
*/
-#define SA_INTERRUPT IRQF_DISABLED
-#define SA_SAMPLE_RANDOM IRQF_SAMPLE_RANDOM
-#define SA_SHIRQ IRQF_SHARED
-#define SA_PROBEIRQ IRQF_PROBE_SHARED
-#define SA_PERCPU IRQF_PERCPU
-
-#define SA_TRIGGER_LOW IRQF_TRIGGER_LOW
-#define SA_TRIGGER_HIGH IRQF_TRIGGER_HIGH
-#define SA_TRIGGER_FALLING IRQF_TRIGGER_FALLING
-#define SA_TRIGGER_RISING IRQF_TRIGGER_RISING
-#define SA_TRIGGER_MASK IRQF_TRIGGER_MASK
+typedef unsigned int __deprecated deprecated_irqf;
+
+#define SA_INTERRUPT ((deprecated_irqf)IRQF_DISABLED)
+#define SA_SAMPLE_RANDOM ((deprecated_irqf)IRQF_SAMPLE_RANDOM)
+#define SA_SHIRQ ((deprecated_irqf)IRQF_SHARED)
+#define SA_PROBEIRQ ((deprecated_irqf)IRQF_PROBE_SHARED)
+#define SA_PERCPU ((deprecated_irqf)IRQF_PERCPU)
+
+#define SA_TRIGGER_LOW ((deprecated_irqf)IRQF_TRIGGER_LOW)
+#define SA_TRIGGER_HIGH ((deprecated_irqf)IRQF_TRIGGER_HIGH)
+#define SA_TRIGGER_FALLING ((deprecated_irqf)IRQF_TRIGGER_FALLING)
+#define SA_TRIGGER_RISING ((deprecated_irqf)IRQF_TRIGGER_RISING)
+#define SA_TRIGGER_MASK ((deprecated_irqf)IRQF_TRIGGER_MASK)

typedef irqreturn_t (*irq_handler_t)(int, void *);

2007-02-10 12:03:55

[permalink] [raw]

Subject: Re: -mm merge plans for 2.6.21

David Woodhouse <[email protected]> writes:

> On Thu, 2007-02-08 at 15:07 -0800, Andrew Morton wrote:
> > lutimesat-simplify-utime2.patch
> > lutimesat-extend-do_utimes-with-flags.patch
> > lutimesat-actual-syscall-and-wire-up-on-i386.patch
> >
> > Do we want this? Ulrich says so. Will merge, I guess.
>
> I would strongly recommend that in the general case, you don't merge new
> system calls unless the corresponding compat_ system call is
> implemented.

... and there is a manpage ready to submit to manpages.

-Andi

2007-02-10 12:06:23

[permalink] [raw]

Subject: Re: -mm merge plans for 2.6.21

Alan <[email protected]> writes:

> Please just push the EDAC K8 stuff. Andi will say "no" from now until the
> end of time, but end users want it, distributions want it, and Andi is
> not the EDAC maintainer so should consider himself overruled on what
> isn't a technical issue but a personal political viewpoint.

Well it's a technical issue -- it conflicts with the machine check
code that is already implemented by stealing away its events.
You would first need a CONFIG_MCE=n on x86-64 at least, otherwise
one of them will be unhappy.

The other issue is that the existing code does everything EDAC
K8 does already perfectly fine, just in a small fraction of
the code.

-Andi

2007-02-10 12:21:24

by Frederik Deweerdt

[permalink] [raw]

Subject: Re: -mm merge plans for 2.6.21

On Sat, Feb 10, 2007 at 12:42:41PM +0100, Arnd Bergmann wrote:
> On Friday 09 February 2007 12:24, Arjan van de Ven wrote:
> >
> > On Fri, 2007-02-09 at 10:57 +0000, Frederik Deweerdt wrote:
> > > +static const int __deprecated SA_INTERRUPT = IRQF_DISABLED;
> > > +static const int __deprecated SA_SAMPLE_RANDOM = IRQF_SAMPLE_RANDOM;
> > > +static const int __deprecated SA_SHIRQ = IRQF_SHARED;
> > > +static const int __deprecated SA_PROBEIRQ = IRQF_PROBE_SHARED;
> > > +static const int __deprecated SA_PERCPU = IRQF_PERCPU;
> > > +
> > > +static const int __deprecated SA_TRIGGER_LOW = IRQF_TRIGGER_LOW;
> > > +static const int __deprecated SA_TRIGGER_HIGH = IRQF_TRIGGER_HIGH;
> > > +static const int __deprecated SA_TRIGGER_FALLING = IRQF_TRIGGER_FALLING;
> > > +static const int __deprecated SA_TRIGGER_RISING = IRQF_TRIGGER_RISING;
> > > +static const int __deprecated SA_TRIGGER_MASK = IRQF_TRIGGER_MASK;
> >
> > this will include these in every .o file for which the .c file includes
> > the header. NOT GOOD(tm)
> >
>
> How about this one instead then:
Well, the warning you get is not that obvious:

test.c: In function 'main':
test.c:11: warning: 'deprecated_irqf' is deprecated

And as far as I could test (gcc 4.1.1 and gcc 3.4.3), Arjan's comment is
not true, the "static const int" don't use extra space, they get
optimized away by the compiler (see http://lkml.org/lkml/2007/2/9/106).

Regards,
Frederik
>
> Mark SA_* constants as deprecated
>
> Signed-off-by: Arnd Bergmann <[email protected]>
>
> --- a/include/linux/interrupt.h
> +++ b/include/linux/interrupt.h
> @@ -53,17 +53,19 @@
> * Migration helpers. Scheduled for removal in 1/2007
> * Do not use for new code !
> */
> -#define SA_INTERRUPT IRQF_DISABLED
> -#define SA_SAMPLE_RANDOM IRQF_SAMPLE_RANDOM
> -#define SA_SHIRQ IRQF_SHARED
> -#define SA_PROBEIRQ IRQF_PROBE_SHARED
> -#define SA_PERCPU IRQF_PERCPU
> -
> -#define SA_TRIGGER_LOW IRQF_TRIGGER_LOW
> -#define SA_TRIGGER_HIGH IRQF_TRIGGER_HIGH
> -#define SA_TRIGGER_FALLING IRQF_TRIGGER_FALLING
> -#define SA_TRIGGER_RISING IRQF_TRIGGER_RISING
> -#define SA_TRIGGER_MASK IRQF_TRIGGER_MASK
> +typedef unsigned int __deprecated deprecated_irqf;
> +
> +#define SA_INTERRUPT ((deprecated_irqf)IRQF_DISABLED)
> +#define SA_SAMPLE_RANDOM ((deprecated_irqf)IRQF_SAMPLE_RANDOM)
> +#define SA_SHIRQ ((deprecated_irqf)IRQF_SHARED)
> +#define SA_PROBEIRQ ((deprecated_irqf)IRQF_PROBE_SHARED)
> +#define SA_PERCPU ((deprecated_irqf)IRQF_PERCPU)
> +
> +#define SA_TRIGGER_LOW ((deprecated_irqf)IRQF_TRIGGER_LOW)
> +#define SA_TRIGGER_HIGH ((deprecated_irqf)IRQF_TRIGGER_HIGH)
> +#define SA_TRIGGER_FALLING ((deprecated_irqf)IRQF_TRIGGER_FALLING)
> +#define SA_TRIGGER_RISING ((deprecated_irqf)IRQF_TRIGGER_RISING)
> +#define SA_TRIGGER_MASK ((deprecated_irqf)IRQF_TRIGGER_MASK)
>
> typedef irqreturn_t (*irq_handler_t)(int, void *);
>
>

2007-02-10 13:35:41

by Alan

[permalink] [raw]

Subject: Re: -mm merge plans for 2.6.21

> Well it's a technical issue -- it conflicts with the machine check
> code that is already implemented by stealing away its events.
> You would first need a CONFIG_MCE=n on x86-64 at least, otherwise
> one of them will be unhappy.

That is a fair point, albeit after far too long sitting stuck in the tree.
I'll have a look into this a bit further, probably MCE_K8 should feed off
the output of the MCE driver.

> The other issue is that the existing code does everything EDAC
> K8 does already perfectly fine, just in a small fraction of
> the code.

Which is false. It provided a totally inconsistent interface to user
space, while the EDAC layer provides the consistency many people need and
want. Also unless it has changed remarkably the MCE driver doesn't do
scrubbing which is needed in software on some chip revisions.

The MCE driver is small, neat, incomplete for some cases and different
to all the other platforms as they use off CPU memory controllers. Its
ideal for a lot of uses but not big enterprise systems. Thats why we need
both.

Alan

2007-02-10 14:43:19

[permalink] [raw]

Subject: Re: -mm merge plans for 2.6.21

On Saturday 10 February 2007 14:48, Alan wrote:
> > Well it's a technical issue -- it conflicts with the machine check
> > code that is already implemented by stealing away its events.
> > You would first need a CONFIG_MCE=n on x86-64 at least, otherwise
> > one of them will be unhappy.
>
> That is a fair point, albeit after far too long sitting stuck in the tree.

I'm pretty sure I mentioned it earlier already.

> I'll have a look into this a bit further, probably MCE_K8 should feed off
> the output of the MCE driver.

You could probably write a simple edac driver that just feeds of the events
mce.c generates and presents that as the EDAC drivers. Currently
the user space device is the only consumer, but it wouldn't be hard
to full it off to other kernel users.

Possibly keeping the existing code that reads the DIMMs and
then reads the address map of the NB and match that, then you
get the same output.

[However see below -- i think matching it to SMBIOS using the address
is a lot more useful for the user in the end]

>
> > The other issue is that the existing code does everything EDAC
> > K8 does already perfectly fine, just in a small fraction of
> > the code.
>
> Which is false. It provided a totally inconsistent interface to user
> space, while the EDAC layer provides the consistency many people need and
> want.

I still don't get that argument because unlike mce.c+mcelog EDAC cannot
actually tell you where the DIMM that failed is located on your motherboard.

As far as i can make it out it's only useful for people who either
have full schematics of their motherboard and know how to read them
or did a full try'n'error of all combinations run once to
figure out which channel is located where.

Somehow I cannot imagine either of that is very common in "enterprises" ;-)

Ok one argument was that some LinuxBIOS users don't seem to
set up these tables and have the necessary information at hand
for their custom cluster hardware, but that's still a weird uncommon
case and it would be far better if they just fixed LB.

> Also unless it has changed remarkably the MCE driver doesn't do
> scrubbing which is needed in software on some chip revisions.

True, adding that might be a good idea.

[Although I guess that could be done with a small user utility
using /dev/mem though if you really wanted it?]

One thing EDAC also does that mcelog doesn't is decode the ECC
syndromes -- but I haven't figured out a use case yet where that
is actually useful to know afterwards.

-Andi

2007-02-10 22:35:37

by Alasdair G Kergon

[permalink] [raw]

Subject: Re: -mm merge plans for 2.6.21 -- md-dm-reduce-stack-usage-with-stacked-block-devices.patch

On Sat, Feb 10, 2007 at 10:58:58AM +0100, Heiko Carstens wrote:
> Alasdair, is dm already fixed or is there any chance that it will
> ever get fixed?

I still expect us to get this changed within the next few months. We've
dealt with the changes necessary to the crypt target for this. The final
problem remaining is the core dm bio splitting code, and there's a related
locking problem in that code to tackle at the same time.

Alasdair
--
[email protected]

2007-02-11 00:31:35

by Dave Jones

[permalink] [raw]

Subject: Re: -mm merge plans for 2.6.21

On Thu, Feb 08, 2007 at 03:07:10PM -0800, Andrew Morton wrote:
>
> I'm getting fed up of holding onto hundreds of patches against subsystem
> trees, sending them over and over again seeing and nothing happen. I sent 242
> patches out to subsystem maintainers on Monday and look at what's still here.
>
> agpgart-allow-drm-populated-agp-memory-types-tidy.patch
> remove-hotplug-cpu-crap-from-cpufreq.patch
> rewrite-lock-in-cpufreq-to-eliminate-cpufreq-hotplug-related-issues.patch
> ondemand-governor-restructure-the-work-callback.patch
> ondemand-governor-use-new-cpufreq-rwsem-locking-in-work-callback.patch
> cpu_freq_table-shouldnt-be-a-def_tristate.patch
>
> -> davej

Apologies, I suck.
I'll dispose of these appropriately tonight.

Dave

--
http://www.codemonkey.org.uk

2007-02-11 02:50:51

[permalink] [raw]

Subject: Re: -mm merge plans for 2.6.21

On Sat, Feb 10, 2007 at 11:22:05AM +0100, Heiko Carstens wrote:

> Which remembers me that I think that MIPS is using the non-compat version
> of sys_epoll_pwait for compat syscalls. But maybe MIPS doesn't need a compat
> syscall for some reason. Dunno.

Which reminds me that x86_64 i386 compat doesn't wire up sys_epoll_pwait ;-)

Ralf

Signed-off-by: Ralf Baechle <[email protected]>

diff --git a/arch/x86_64/ia32/ia32entry.S b/arch/x86_64/ia32/ia32entry.S
index b4aa875..5993262 100644
--- a/arch/x86_64/ia32/ia32entry.S
+++ b/arch/x86_64/ia32/ia32entry.S
@@ -718,4 +718,5 @@ ia32_sys_call_table:
.quad compat_sys_vmsplice
.quad compat_sys_move_pages
.quad sys_getcpu
+ .quad sys_epoll_pwait
ia32_syscall_end:

2007-02-11 02:51:17

[permalink] [raw]

Subject: Re: -mm merge plans for 2.6.21

On Sat, Feb 10, 2007 at 10:32:07AM +0000, David Woodhouse wrote:

> On Sat, 2007-02-10 at 11:22 +0100, Heiko Carstens wrote:
> > Which remembers me that I think that MIPS is using the non-compat version
> > of sys_epoll_pwait for compat syscalls. But maybe MIPS doesn't need a compat
> > syscall for some reason. Dunno.
>
> It's OK as long as the 64-bit kernel, N32 and O32 userspace all agree
> there there's 32 bits of padding between the fields of this structure:
>
> struct epoll_event {
> __u32 events;
> __u64 data;
> };
>
> I suspect it's a fairly safe bet that N32 userspace agrees; if the O32
> ABI is different then it would need the compat syscall.

That is correct - and apparently for all ABIs because I wasn't able to find
a compat_sys_epoll_pwait at all.

Unfortunately struct epoll_event contains a gap so it bets on identical
padding rules between native and compat ABI and anyway, padding is wasted
space so the struct members should have been swapped when this structure
was created. Oh well, too late.

Ralf

2007-02-11 04:54:00

by Davide Libenzi

[permalink] [raw]

Subject: Re: -mm merge plans for 2.6.21

On Sat, 10 Feb 2007, Ralf Baechle wrote:

> Unfortunately struct epoll_event contains a gap so it bets on identical
> padding rules between native and compat ABI and anyway, padding is wasted
> space so the struct members should have been swapped when this structure
> was created. Oh well, too late.

You really should have needed padding in there, since even if you swapped
the members, sizeof(struct epoll_event) would still need to be 16, if
alignof(u64) == 8. Either adding an extra auxilliary u32, or make the two
members be u64, would have been ok.
I'll be posting a patch that adds the compat_ layer for epoll in
kernel/compat.c. Architectures that needs it (currently only ARM-EABI and
IA64 use a compat code for epoll), should wire them up.

- Davide

2007-02-11 10:42:39

[permalink] [raw]

Subject: Re: -mm merge plans for 2.6.21

On Saturday 10 February 2007 22:05, Ralf Baechle wrote:
> On Sat, Feb 10, 2007 at 11:22:05AM +0100, Heiko Carstens wrote:
>
> > Which remembers me that I think that MIPS is using the non-compat version
> > of sys_epoll_pwait for compat syscalls. But maybe MIPS doesn't need a compat
> > syscall for some reason. Dunno.
>
> Which reminds me that x86_64 i386 compat doesn't wire up sys_epoll_pwait ;-)

Added thanks.

-Andi

2007-02-11 15:34:25

[permalink] [raw]

Subject: Re: -mm merge plans for 2.6.21

On Sat, 2007-02-10 at 21:34 +0000, Ralf Baechle wrote:
> Unfortunately struct epoll_event contains a gap so it bets on identical
> padding rules between native and compat ABI and anyway, padding is wasted
> space so the struct members should have been swapped when this structure
> was created. Oh well, too late.

Indeed. That was the example I was thinking of when I suggested the "no
new syscalls without _simultaneous_ compat version" rule.

--
dwmw2

2007-02-11 16:15:57

by Heiko Carstens

[permalink] [raw]

Subject: Re: -mm merge plans for 2.6.21

On Sat, Feb 10, 2007 at 09:34:47PM +0000, Ralf Baechle wrote:
> On Sat, Feb 10, 2007 at 10:32:07AM +0000, David Woodhouse wrote:
>
> > On Sat, 2007-02-10 at 11:22 +0100, Heiko Carstens wrote:
> > > Which remembers me that I think that MIPS is using the non-compat version
> > > of sys_epoll_pwait for compat syscalls. But maybe MIPS doesn't need a compat
> > > syscall for some reason. Dunno.
> >
> > It's OK as long as the 64-bit kernel, N32 and O32 userspace all agree
> > there there's 32 bits of padding between the fields of this structure:
> >
> > struct epoll_event {
> > __u32 events;
> > __u64 data;
> > };
> >
> > I suspect it's a fairly safe bet that N32 userspace agrees; if the O32
> > ABI is different then it would need the compat syscall.
>
> That is correct - and apparently for all ABIs because I wasn't able to find
> a compat_sys_epoll_pwait at all.

Hmm.. so you don't need to do some fancy compat conversion for the sigset_t
that gets passed? Why is that? I don't get it...

2007-02-11 16:34:55

by Davide Libenzi

[permalink] [raw]

Subject: Re: -mm merge plans for 2.6.21

On Sun, 11 Feb 2007, Heiko Carstens wrote:

> On Sat, Feb 10, 2007 at 09:34:47PM +0000, Ralf Baechle wrote:
> > On Sat, Feb 10, 2007 at 10:32:07AM +0000, David Woodhouse wrote:
> >
> > > On Sat, 2007-02-10 at 11:22 +0100, Heiko Carstens wrote:
> > > > Which remembers me that I think that MIPS is using the non-compat version
> > > > of sys_epoll_pwait for compat syscalls. But maybe MIPS doesn't need a compat
> > > > syscall for some reason. Dunno.
> > >
> > > It's OK as long as the 64-bit kernel, N32 and O32 userspace all agree
> > > there there's 32 bits of padding between the fields of this structure:
> > >
> > > struct epoll_event {
> > > __u32 events;
> > > __u64 data;
> > > };
> > >
> > > I suspect it's a fairly safe bet that N32 userspace agrees; if the O32
> > > ABI is different then it would need the compat syscall.
> >
> > That is correct - and apparently for all ABIs because I wasn't able to find
> > a compat_sys_epoll_pwait at all.
>
> Hmm.. so you don't need to do some fancy compat conversion for the sigset_t
> that gets passed? Why is that? I don't get it...

The compat_sys_epoll_pwait function has two sources of compat. One is the
sigset_t and the other one is the struct epoll_event. The sigset_t compat
is always needed, while the struct epoll_event may be needed. The code of
(upcoming) compat_sys_epoll_pwait takes care of both, with a smart
compile-time check to wire sys_epoll_wait or compat_sys_epoll_wait,
depending on the need of the struct epoll_event compat handling.
So yes, compat_sys_epoll_pwait is needed.

- Davide

2007-02-11 17:13:57

[permalink] [raw]

Subject: Re: -mm merge plans for 2.6.21

On Sun, Feb 11, 2007 at 04:33:41PM +0100, David Woodhouse wrote:

> On Sat, 2007-02-10 at 21:34 +0000, Ralf Baechle wrote:
> > Unfortunately struct epoll_event contains a gap so it bets on identical
> > padding rules between native and compat ABI and anyway, padding is wasted
> > space so the struct members should have been swapped when this structure
> > was created. Oh well, too late.
>
> Indeed. That was the example I was thinking of when I suggested the "no
> new syscalls without _simultaneous_ compat version" rule.

The need for a compat ABI is not always obvious.

Ralf

2007-02-11 18:04:48

[permalink] [raw]

Subject: Re: -mm merge plans for 2.6.21

On Sun, Feb 11, 2007 at 05:14:46PM +0100, Heiko Carstens wrote:

> Hmm.. so you don't need to do some fancy compat conversion for the sigset_t
> that gets passed? Why is that? I don't get it...

Ah, I finally get your point. Yes, that needs conversion.

Ralf

2007-02-12 21:03:28

by Doug Thompson

[permalink] [raw]

Subject: Re: -mm merge plans for 2.6.21

--- Andi Kleen <[email protected]> wrote:

> Alan <[email protected]> writes:
>
> > Please just push the EDAC K8 stuff. Andi will say "no" from now
> until the
> > end of time, but end users want it, distributions want it, and Andi
> is
> > not the EDAC maintainer so should consider himself overruled on
> what
> > isn't a technical issue but a personal political viewpoint.
>
> Well it's a technical issue -- it conflicts with the machine check
> code that is already implemented by stealing away its events.
> You would first need a CONFIG_MCE=n on x86-64 at least, otherwise
> one of them will be unhappy.
>
> The other issue is that the existing code does everything EDAC
> K8 does already perfectly fine, just in a small fraction of
> the code.
>
> -Andi

I assert that there is a greater than just a technical implementation
issue. I maintain that it is a design pattern issue.

The MCE hardware mechanism is a good mechanism for reporting Hardware
events. The problem comes in as to WHERE those events should be
handled.

EDAC was designed to be a 'stack' of drivers. The upper CORE module is
to provide an abstraction, whose presentation to user space is to be
uniform across processes and architectures. The individual lower
drivers, which register with the core, handle the machine and/or
architecture specifics of 'driving' the hardware and funnel events to
the core.

EDAC is operational now on MIPS and ARM architectures (phone device) as
well as on x86 and x86_64.

Additional features of a given arch/processor that can be utilized in
harvesting hardware events should be captured and then funneled to the
'low' level EDAC driver. That driver can then pass the event onto the
CORE module for further processings and presentation, as determined by
the controls into the CORE.

MCE event processing should be funneled to the EDAC K8 driver and not
directly to userspace.

The design pattern I have been trying to utilize in EDAC, is the same
one used by the Network stack and the SCSI stack.

If I have a new NIC, which has some great new hardware feature, should
the driver export that feature outside of the network stack, which does
not CURRENTLY support the processing of the feature. OR should the
network stack control paths be extended to provide a mechanism whereby
that new feature could be utilized?

Similiar design pattern exists on the SCSI stack, with device features
and hardware abilities.

I am currently developing enhancements to EDAC that provide for L1, L2,
etc cache ECC event harvesting, DMA ECC event harvest, interconnect
harvesting, and other chipset/processing ECC event harvesting. Some
events are polled others are software interupt delivered. This is on an
arch OTHER than x86 or x86_64. But these EDAC features can be used on
the x86/x86_64 as well.

As there IS an ECC event consumption race between MCE and EDAC, then at
least a 'config' mechanism can be utilized to switch between the two.

Better yet, I would like to be able to capture the MCE events and
funnel them to EDAC, when loaded, as a downstream consumer of the MCE
event stream. Then EDAC would process the information and then proceed
to inform whether the event was handled or not by EDAC. Additionally,
it would inform whether a panic should occur or not.

When EDAC is not loaded, then MCE would act as it does now.

The downside with both EDAC and MCE being in place, is that there would
be ONE MCE processing pattern for the AMD x86_64 processors and the
EDAC pattern for all the other archs/processors. As far as I can tell,
other MCE processors don't generate the events as advertized. Maybe I
am wrong on that, but I can't find them. How much hardware specific
detail should be exported?

I assert the better solution is to have the ability to hook into the
MCE event stream by the EDAC K8 driver, then provide action rules (EDAC
controls) in processing those events, which EDAC would return to the
MCE stream.

As to the size of the MCE code doing everything EDAC K8 does, that is
mostly true. BUT then the x86_64 MCE mechanism becomes the exception
path.

Even the company using EDAC on the phone device doesn't mind the 60
kilobytes.

doug t

2007-02-12 21:46:51