From: Andrew Morton <akpm@osdl.org>
To: Miklos Szeredi <miklos@szeredi.hu>
Cc: linux-kernel@vger.kernel.org
Subject: Re: [PATCH 11/17] fuse: add number of waiting requests attribute
Date: Sat, 14 Jan 2006 01:53:31 -0800 [thread overview]
Message-ID: <20060114015331.6c4a7618.akpm@osdl.org> (raw)
In-Reply-To: <E1ExhxA-0004Bz-00@dorka.pomaz.szeredi.hu>
Miklos Szeredi <miklos@szeredi.hu> wrote:
>
> > This doesn't get initialised anywhere.
> >
> > Presumably you're relying on a memset somewhere. That might work on all
> > architectures, AFAIK. But in theory it's wrong. If, for example, the
> > architecture implements atomic_t via a spinlock-plus-integer, and that
> > spinlock's unlocked state is not all-bits-zero, we're dead.
> >
> > So we should initialise it with
> >
> > foo->num_waiting = ATOMIC_INIT(0);
> >
>
> Is it correct to use a structure initializer this way?
Yes, if it's typecast to the right type.
ATOMIC_INIT is not. I had a brainfart.
> > nb: it is not correct to initialise an atomic_t with
> >
> > atomic_set(a, 0);
> >
> > because in the above theoretical case case where the arch uses a spinlock
> > in the atomic_t, that spinlock doesn't get initialised. I bet we've got code
> > in there which does this.
>
> According to Documentation/atomic_ops.txt, this is the correct usage
> of atomic_set():
>
> | The first operations to implement for atomic_t's are the
> | initializers and plain reads.
> |
> | #define ATOMIC_INIT(i) { (i) }
> | #define atomic_set(v, i) ((v)->counter = (i))
> |
> | The first macro is used in definitions, such as:
> |
> | static atomic_t my_counter = ATOMIC_INIT(1);
> |
> | The second interface can be used at runtime, as in:
> |
> | struct foo { atomic_t counter; };
> | ...
> |
> | struct foo *k;
> |
> | k = kmalloc(sizeof(*k), GFP_KERNEL);
> | if (!k)
> | return -ENOMEM;
> | atomic_set(&k->counter, 0);
>
> So in fact atomic_set() is an initializer, and should be named
> atomic_init() accordingly.
Yes, we're screwed. I don't think it's possible to implement atomic_t as
spinlock+int due to this.
> Is atomic_set() ever used as an atomic
> operation rather than an initializer?
>
Sure, lots of places. Lots of places where you _don't_ want your
atomic_t's spinlock to be reinitialised.
hmm.
next prev parent reply other threads:[~2006-01-14 9:53 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2006-01-14 0:39 [PATCH 00/17] fuse: fixes, cleanups and asynchronous read requests Miklos Szeredi
2006-01-14 0:39 ` [PATCH 01/17] add /sys/fs Miklos Szeredi
2006-01-17 12:47 ` Christoph Hellwig
2006-01-17 12:52 ` Miklos Szeredi
2006-01-14 0:39 ` [PATCH 02/17] fuse: fuse_copy_finish() order fix Miklos Szeredi
2006-01-14 0:39 ` [PATCH 03/17] fuse: fix request_end() Miklos Szeredi
2006-01-14 0:39 ` [PATCH 04/17] fuse: handle error INIT reply Miklos Szeredi
2006-01-14 0:39 ` [PATCH 05/17] fuse: uninline some functions Miklos Szeredi
2006-01-14 0:39 ` [PATCH 06/17] fuse: miscellaneous cleanup Miklos Szeredi
2006-01-14 0:39 ` [PATCH 07/17] fuse: introduce unified request state Miklos Szeredi
2006-01-14 0:39 ` [PATCH 08/17] fuse: introduce list for requests under I/O Miklos Szeredi
2006-01-14 0:39 ` [PATCH 09/17] fuse: extend semantics of connected flag Miklos Szeredi
2006-01-14 0:39 ` [PATCH 10/17] fuse: make fuse connection a kobject Miklos Szeredi
2006-01-14 0:39 ` [PATCH 11/17] fuse: add number of waiting requests attribute Miklos Szeredi
2006-01-14 1:28 ` Andrew Morton
2006-01-14 9:44 ` Miklos Szeredi
2006-01-14 9:53 ` Andrew Morton [this message]
2006-01-18 5:56 ` Eric Dumazet
2006-01-14 0:40 ` [PATCH 12/17] fuse: add connection aborting Miklos Szeredi
2006-01-14 1:31 ` Andrew Morton
2006-01-14 9:50 ` Miklos Szeredi
2006-01-14 0:40 ` [PATCH 13/17] fuse: add asynchronous request support Miklos Szeredi
2006-01-14 0:40 ` [PATCH 14/17] fuse: move INIT handling to inode.c Miklos Szeredi
2006-01-14 0:40 ` [PATCH 15/17] fuse: READ request initialization Miklos Szeredi
2006-01-14 0:40 ` [PATCH 16/17] fuse: use asynchronous READ requests for readpages Miklos Szeredi
2006-01-14 0:40 ` [PATCH 17/17] fuse: update documentation for sysfs Miklos Szeredi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20060114015331.6c4a7618.akpm@osdl.org \
--to=akpm@osdl.org \
--cc=linux-kernel@vger.kernel.org \
--cc=miklos@szeredi.hu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).