1 Subject: Use state_mutex for switch_log locking, and prevent multiple openers
2 From: Jeremy Kerr <jk@ozlabs.org>
3 References: 447133 - LTC50070
5 Currently, we use ctx->mapping_lock and ctx->switch_log->lock for the
6 context switch log. The mapping lock only prevents concurrent open()s,
7 so we require the switch_lock->lock for reads.
9 Since writes to the switch log buffer occur on context switches, we're
10 better off synchronising with the state_mutex, which is held during a
11 switch. Since we're serialised througout the buffer reads and writes,
12 we can use the state mutex to protect open and release too, and
13 can now kfree() the log buffer on release. This allows us to perform
14 the switch log notify without taking any extra locks.
16 Because the buffer is only present while the file is open, we can use
17 it to prevent multiple simultaneous openers.
19 Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
20 Signed-off-by: Olaf Hering <olh@suse.de>
22 arch/powerpc/platforms/cell/spufs/file.c | 133 ++++++++++++++++++------------
23 arch/powerpc/platforms/cell/spufs/run.c | 3
24 arch/powerpc/platforms/cell/spufs/spufs.h | 1
25 3 files changed, 81 insertions(+), 56 deletions(-)
27 --- a/arch/powerpc/platforms/cell/spufs/file.c
28 +++ b/arch/powerpc/platforms/cell/spufs/file.c
29 @@ -2429,38 +2429,48 @@ static inline int spufs_switch_log_avail
30 static int spufs_switch_log_open(struct inode *inode, struct file *file)
32 struct spu_context *ctx = SPUFS_I(inode)->i_ctx;
35 + rc = spu_acquire(ctx);
40 - * We (ab-)use the mapping_lock here because it serves the similar
41 - * purpose for synchronizing open/close elsewhere. Maybe it should
42 - * be renamed eventually.
44 - mutex_lock(&ctx->mapping_lock);
45 if (ctx->switch_log) {
46 - spin_lock(&ctx->switch_log->lock);
47 - ctx->switch_log->head = 0;
48 - ctx->switch_log->tail = 0;
49 - spin_unlock(&ctx->switch_log->lock);
52 - * We allocate the switch log data structures on first open.
53 - * They will never be free because we assume a context will
54 - * be traced until it goes away.
56 - ctx->switch_log = kzalloc(sizeof(struct switch_log) +
57 - SWITCH_LOG_BUFSIZE * sizeof(struct switch_log_entry),
59 - if (!ctx->switch_log)
61 - spin_lock_init(&ctx->switch_log->lock);
62 - init_waitqueue_head(&ctx->switch_log->wait);
67 + ctx->switch_log = kzalloc(sizeof(struct switch_log) +
68 + SWITCH_LOG_BUFSIZE * sizeof(struct switch_log_entry),
71 + if (!ctx->switch_log) {
75 - mutex_unlock(&ctx->mapping_lock);
77 + init_waitqueue_head(&ctx->switch_log->wait);
85 +static int spufs_switch_log_release(struct inode *inode, struct file *file)
87 + struct spu_context *ctx = SPUFS_I(inode)->i_ctx;
90 + rc = spu_acquire(ctx);
94 + kfree(ctx->switch_log);
95 + ctx->switch_log = NULL;
100 - mutex_unlock(&ctx->mapping_lock);
104 static int switch_log_sprint(struct spu_context *ctx, char *tbuf, int n)
105 @@ -2488,42 +2498,46 @@ static ssize_t spufs_switch_log_read(str
109 + error = spu_acquire(ctx);
117 if (file->f_flags & O_NONBLOCK) {
118 - if (spufs_switch_log_used(ctx) <= 0)
119 - return cnt ? cnt : -EAGAIN;
120 + if (spufs_switch_log_used(ctx) == 0) {
125 - /* Wait for data in buffer */
126 - error = wait_event_interruptible(ctx->switch_log->wait,
127 + /* spufs_wait will drop the mutex and re-acquire,
128 + * but since we're in read(), the file cannot be
129 + * _released (and so ctx->switch_log is stable).
131 + error = spufs_wait(ctx->switch_log->wait,
132 spufs_switch_log_used(ctx) > 0);
134 + /* On error, spufs_wait returns without the
135 + * state mutex held */
141 - spin_lock(&ctx->switch_log->lock);
142 - if (ctx->switch_log->head == ctx->switch_log->tail) {
143 - /* multiple readers race? */
144 - spin_unlock(&ctx->switch_log->lock);
145 + /* We may have had entries read from underneath us while we
146 + * dropped the mutex in spufs_wait, so re-check */
147 + if (ctx->switch_log->head == ctx->switch_log->tail)
151 width = switch_log_sprint(ctx, tbuf, sizeof(tbuf));
154 ctx->switch_log->tail =
155 (ctx->switch_log->tail + 1) %
159 - spin_unlock(&ctx->switch_log->lock);
162 - * If the record is greater than space available return
163 - * partial buffer (so far)
167 + /* If the record is greater than space available return
168 + * partial buffer (so far) */
171 error = copy_to_user(buf + cnt, tbuf, width);
172 @@ -2532,6 +2546,8 @@ static ssize_t spufs_switch_log_read(str
178 return cnt == 0 ? error : cnt;
181 @@ -2540,29 +2556,41 @@ static unsigned int spufs_switch_log_pol
182 struct inode *inode = file->f_path.dentry->d_inode;
183 struct spu_context *ctx = SPUFS_I(inode)->i_ctx;
184 unsigned int mask = 0;
187 poll_wait(file, &ctx->switch_log->wait, wait);
189 + rc = spu_acquire(ctx);
193 if (spufs_switch_log_used(ctx) > 0)
201 static const struct file_operations spufs_switch_log_fops = {
202 - .owner = THIS_MODULE,
203 - .open = spufs_switch_log_open,
204 - .read = spufs_switch_log_read,
205 - .poll = spufs_switch_log_poll,
206 + .owner = THIS_MODULE,
207 + .open = spufs_switch_log_open,
208 + .read = spufs_switch_log_read,
209 + .poll = spufs_switch_log_poll,
210 + .release = spufs_switch_log_release,
214 + * Log a context switch event to a switch log reader.
216 + * Must be called with ctx->state_mutex held.
218 void spu_switch_log_notify(struct spu *spu, struct spu_context *ctx,
221 if (!ctx->switch_log)
224 - spin_lock(&ctx->switch_log->lock);
225 if (spufs_switch_log_avail(ctx) > 1) {
226 struct switch_log_entry *p;
228 @@ -2576,7 +2604,6 @@ void spu_switch_log_notify(struct spu *s
229 ctx->switch_log->head =
230 (ctx->switch_log->head + 1) % SWITCH_LOG_BUFSIZE;
232 - spin_unlock(&ctx->switch_log->lock);
234 wake_up(&ctx->switch_log->wait);
236 --- a/arch/powerpc/platforms/cell/spufs/run.c
237 +++ b/arch/powerpc/platforms/cell/spufs/run.c
238 @@ -249,6 +249,7 @@ static int spu_run_fini(struct spu_conte
240 spuctx_switch_state(ctx, SPU_UTIL_IDLE_LOADED);
241 clear_bit(SPU_SCHED_SPU_RUN, &ctx->sched_flags);
242 + spu_switch_log_notify(NULL, ctx, SWITCH_LOG_EXIT, *status);
245 if (signal_pending(current))
246 @@ -417,8 +418,6 @@ long spufs_run_spu(struct spu_context *c
247 ret = spu_run_fini(ctx, npc, &status);
250 - spu_switch_log_notify(NULL, ctx, SWITCH_LOG_EXIT, status);
252 if ((status & SPU_STATUS_STOPPED_BY_STOP) &&
253 (((status >> SPU_STOP_STATUS_SHIFT) & 0x3f00) == 0x2100))
254 ctx->stats.libassist++;
255 --- a/arch/powerpc/platforms/cell/spufs/spufs.h
256 +++ b/arch/powerpc/platforms/cell/spufs/spufs.h
257 @@ -65,7 +65,6 @@ enum {
262 wait_queue_head_t wait;