2004-09-10 04:05:48

by Kohei KaiGai

[permalink] [raw]
Subject: PATCH] atomic_inc_return() [0/5] (Re: atomic_inc_return)

The first mail to LKML was returned to me because Triple-X was in subject.
I tried to send this one again.
---------------------------

Hello, Hugh Dickins

> KaiGai-San,
>
> I believe you have a patch adding those to i386 (including CONFIG_M386
> runtime check lest it's an actual i386 which cannot do "xadd") and x86_64.
> I'd be glad to see that go into the tree, would you be ready to submit it
> to Andrew or Linus based on the current 2.6 BK tree?

Indeed, I hope to do this.
atomic_inc_return() is necessary for the 'SELinux performance improvement
by RCU' patch.

See, http://lkml.org/lkml/2004/8/30/63
[PATCH]SELinux performance improvement by RCU (Re: RCU issue with SELinux)

These fundamental functions should be managed in up-stream.

> I think arm was missing the inc and dec, but has recently gained them.
> arm26 doesn't have any of the four, but it's not SMP, and just a matter
> of following the other examples in its atomic.h to add them. sparc64
> is missing the add and sub (which neither of us particularly need),
> I think just because nobody was using them: easily added - I think
> it'd be a good idea for all architectures to have the set of four.

I made patches for i386, x86_64, arm, arm26 and sparc64.
These patches apply to both linux-2.6.9-rc1 and 2.6.9-rc1-mm4.

[1/5] atomic_inc_return-linux-2.6.9-rc1.i386.patch
This patch implements atomic_inc_return() and so on for i386,
and includes runtime check whether CPU is legacy 386.
It is same as I posted to LKML and Andi Kleen at '04/09/01.

[2/5] atomic_inc_return-linux-2.6.9-rc1.x86_64.patch
This patch implements atomic_inc_return() and so on for x86_64.
It is same as I posted to LKML and Andi Kleen at '04/09/01.

[3/5] atomic_inc_return-linux-2.6.9-rc1.arm.patch
This patch declares atomic_inc_return() as the alias of atomic_add_return()
and atomic_dec_return() as an alias of atomic_dec_return().
This patch has not been tested, since we don't have ARM machine.
I want to let this reviewed by ARM specialists.

[4/5] atomic_inc_return-linux-2.6.9-rc1.arm26.patch
This patch implements atomic_inc_return() and so on for ARM26.
Because Hugh says that SMP is not supported in arm26, it is implemented
by normal operations between local_irq_save() and local_irq_restore()
like another atomic operations.
This patch has not been tested, since we don't have ARM26 machine.
I want to let this reviewed by ARM26 specialists.

[5/5] atomic_inc_return-linux-2.6.9-rc1.sparc64.patch
This patch declares atomic_add_return() as an alias of __atomic_add().
atomic64_add_return(),atomic_sub_return() and atomic64_sub_return() are same.
This patch has not been tested, since we don't have SPARC64 machine.
I want to let this reviewed by SPARC64 specialists.

--------
Kai Gai <[email protected]>


> Andrew,
>
> Sorry, I think I must ask you to back my lighten-mmlist_lock.patch
> out of -mm for the moment, and I'll resubmit a little later on.
>
> Reason being, though nobody is at all likely to hit the race,
> I've put a pathetic piece of self-delusion in try_to_unuse:
> if (!atomic_read(&mm->mm_users))
> continue;
> atomic_inc(&mm->mm_users);
>
> I've tried several ways of fixing it (including reworking that dance
> using marker mms instead of raised counts), but got bored or given up
> in disgust over most of them. Much the nicest fix would be:
> if (atomic_inc_return(&mm->mm_users) == 1) {
> atomic_dec(&mm->mm_users);
> continue;
> }
>
> (Since, if mm_users was 0, try_to_unuse is the only one which could
> be incrementing it; and though it's possible to do two swapoffs at
> once, we here have the mmlist_lock guarding against another.)
>
> But that suffers from the drawback that not all architectures currently
> support atomic_inc_return (nor do all support cmpxchg, which could have
> been used instead), and its friends atomic_add_return, atomic_sub_return,
> atomic_dec_return. Though most do: wouldn't it be nice if they all did?
>
> KaiGai-San,
>
> I believe you have a patch adding those to i386 (including CONFIG_M386
> runtime check lest it's an actual i386 which cannot do "xadd") and x86_64.
> I'd be glad to see that go into the tree, would you be ready to submit it
> to Andrew or Linus based on the current 2.6 BK tree?
>
> I think arm was missing the inc and dec, but has recently gained them.
> arm26 doesn't have any of the four, but it's not SMP, and just a matter
> of following the other examples in its atomic.h to add them. sparc64
> is missing the add and sub (which neither of us particularly need),
> I think just because nobody was using them: easily added - I think
> it'd be a good idea for all architectures to have the set of four.
>
> Hirokazu-San,
>
> That leaves your m32r architecture without them: are those functions
> which you could easily add to the m32r atomic.h, or would there be a
> problem with them? I'd hate to break your newly-arrived architecture!
>
> Thanks,
> Hugh
>


2004-09-10 06:38:28

by Hugh Dickins

[permalink] [raw]
Subject: Re: PATCH] atomic_inc_return() [0/5] (Re: atomic_inc_return)

On Fri, 10 Sep 2004, Kaigai Kohei wrote:
> >
> > I believe you have a patch adding those to i386 (including CONFIG_M386
> > runtime check lest it's an actual i386 which cannot do "xadd") and x86_64.
> > I'd be glad to see that go into the tree, would you be ready to submit it
> > to Andrew or Linus based on the current 2.6 BK tree?
>
> Indeed, I hope to do this.
> atomic_inc_return() is necessary for the 'SELinux performance improvement
> by RCU' patch.

Thanks a lot, the patches 1-5 you've posted look good to me (aside from
some spaces instead of tab in the asm-arm 3/5), and should save me from
my silly race.

If we'd had these earlier, we could have avoiding changing page->count
and page->mapcount over to start from -1 (though I dare say they may
be slightly more efficient as they now are, than if we converted them
back). Should save making such conversions in future, anyway.

> > I think arm was missing the inc and dec, but has recently gained them.

I was wrong when I said that: asm-arm/atomic.h has recently been reworked,
but does still needs your additions: sorry for the confusion.

Thanks,
Hugh