From: Michael Ellerman <michael@ellerman.id.au>
To: Arnd Bergmann <arnd@arndb.de>
Cc: linuxppc-dev@ozlabs.org, Jeremy Kerr <jk@ozlabs.org>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 15/15] Add DEFINE_SPUFS_ATTRIBUTE()
Date: Thu, 13 Sep 2007 12:05:41 +1000 [thread overview]
Message-ID: <1189649141.19087.13.camel@concordia.ozlabs.ibm.com> (raw)
In-Reply-To: <200709121047.05471.arnd@arndb.de>
[-- Attachment #1: Type: text/plain, Size: 2021 bytes --]
On Wed, 2007-09-12 at 10:47 +0200, Arnd Bergmann wrote:
> On Wednesday 12 September 2007, Michael Ellerman wrote:
> > On Wed, 2007-09-12 at 17:43 +1000, Michael Ellerman wrote:
> > > This patch adds DEFINE_SPUFS_ATTRIBUTE(), a wraper around
> > > DEFINE_SIMPLE_ATTRIBUTE which does the specified locking for the get
> > > routine for us.
> > >
> > > Unfortunately we need two get routines (a locked and unlocked version) to
> > > support the coredump code. This patch hides one of those (the locked version)
> > > inside the macro foo.
>
> >
> > jk said:
> > > "Good god man!"
> >
> > Yeah, I'm a bit lukewarm on this one. But the diffstat is nice, 50% code
> > reduction ain't bad :)
>
> Have you looked at the change in object code size? I would expect the
> object code to actually become bigger. I also think that it hurts
> readability rather than help it.
Yeah I did, it's smaller actually:
text data bss dec hex filename
44898 17804 120 62822 f566 spufs-before.o
44886 17804 120 62810 f55a spufs-after.o
> Maybe a better solution is to change the core dump code to not
> require the mutex to be held in the first place. By the time
> we get to call the get functions, it should already be in
> saved state and no longer be able to get scheduled, so we might
> not actually need all the extra tricks with avoiding the
> mutex to be taken again.
Well that'd be nice, but I don't see anywhere that that happens. AFAICT
the acquire we do in the first coredump callback is the first the SPU
contexts know about their PPE process dying. And spufs is still live, so
I think we definitely need to grab the mutex, or we might race with
userspace accessing spufs files.
cheers
--
Michael Ellerman
OzLabs, IBM Australia Development Lab
wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)
We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]
next prev parent reply other threads:[~2007-09-13 2:05 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-09-12 7:43 [PATCH 01/15] Extract the file descriptor search logic in SPU coredump code Michael Ellerman
2007-09-12 7:43 ` [PATCH 02/15] Remove ctx_info and ctx_info_list Michael Ellerman
2007-09-12 7:43 ` [PATCH 03/15] Call spu_acquire_saved() before calculating the SPU note sizes Michael Ellerman
2007-09-12 7:43 ` [PATCH 04/15] Use computed sizes/#defines rather than literals in SPU coredump code Michael Ellerman
2007-09-12 7:43 ` [PATCH 05/15] Write some SPU coredump values as ASCII Michael Ellerman
2007-09-12 7:43 ` [PATCH 06/15] Correctly calculate the size of the local-store to dump Michael Ellerman
2007-09-12 11:31 ` Arnd Bergmann
2007-09-12 7:43 ` [PATCH 07/15] Don't return -ENOSYS as extra notes size if spufs is not loaded Michael Ellerman
2007-09-12 11:04 ` Arnd Bergmann
2007-09-12 7:43 ` [PATCH 08/15] Use spufs_coredump_num_notes everywhere, and don't NULL terminate Michael Ellerman
2007-09-12 8:55 ` Arnd Bergmann
2007-09-12 7:43 ` [PATCH 09/15] Internal __spufs_get_foo() routines should take a spu_context * Michael Ellerman
2007-09-12 8:53 ` Arnd Bergmann
2007-09-12 7:43 ` [PATCH 10/15] Add contents of npc file to SPU coredumps Michael Ellerman
2007-09-12 8:52 ` Arnd Bergmann
2007-09-12 7:43 ` [PATCH 11/15] Combine spufs_coredump_calls with spufs_calls Michael Ellerman
2007-09-12 8:51 ` Arnd Bergmann
2007-09-12 7:43 ` [PATCH 12/15] Cleanup ELF coredump extra notes logic Michael Ellerman
2007-09-12 7:43 ` [PATCH 13/15] Handle errors in SPU coredump code, and support coredump to a pipe Michael Ellerman
2007-09-12 7:43 ` [PATCH 14/15] Respect RLIMIT_CORE in spu coredump code Michael Ellerman
2007-09-12 7:43 ` [PATCH 15/15] Add DEFINE_SPUFS_ATTRIBUTE() Michael Ellerman
2007-09-12 7:49 ` Michael Ellerman
2007-09-12 8:47 ` Arnd Bergmann
2007-09-13 2:05 ` Michael Ellerman [this message]
2007-09-13 12:17 ` Arnd Bergmann
2007-09-12 8:17 ` [PATCH 01/15] Extract the file descriptor search logic in SPU coredump code Jeremy Kerr
2007-09-12 8:35 ` Andrew Morton
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=1189649141.19087.13.camel@concordia.ozlabs.ibm.com \
--to=michael@ellerman.id.au \
--cc=arnd@arndb.de \
--cc=jk@ozlabs.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linuxppc-dev@ozlabs.org \
/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).