aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorJeff Dike <jdike@addtoit.com>2006-09-26 02:33:04 -0400
committerLinus Torvalds <torvalds@g5.osdl.org>2006-09-26 11:49:07 -0400
commit19bdf0409f25a85a45874a5a8da6f3e4edcf4a49 (patch)
tree82abcba44b8026b3dab0e6d26bdf3109bc6bb0cf
parent6edf428ed177e333863a8e5c37751a9ec176f241 (diff)
[PATCH] uml: SIGIO cleanups
- Various cleanups in the sigio code. - Removed explicit zero-initializations of a few structures. - Improved some error messages. - An API change - there was an asymmetry between reactivate_fd calling maybe_sigio_broken, which goes through all the machinery of figuring out if a file descriptor supports SIGIO and applying the workaround to it if not, and deactivate_fd, which just turns off the descriptor. This is changed so that only activate_fd calls maybe_sigio_broken, when the descriptor is first seen. reactivate_fd now calls add_sigio_fd, which is symmetric with ignore_sigio_fd. This removes a recursion which makes a critical section look more critical than it really was, obsoleting a big comment to that effect. This requires keeping track of all descriptors which are getting the SIGIO treatment, not just the ones being polled at any given moment, so that reactivate_fd, through add_sigio_fd, doesn't try to tell the SIGIO thread about descriptors it doesn't care about. Signed-off-by: Jeff Dike <jdike@addtoit.com> Signed-off-by: Andrew Morton <akpm@osdl.org> Signed-off-by: Linus Torvalds <torvalds@osdl.org>
-rw-r--r--arch/um/include/os.h1
-rw-r--r--arch/um/kernel/irq.c34
-rw-r--r--arch/um/os-Linux/sigio.c103
3 files changed, 67 insertions, 71 deletions
diff --git a/arch/um/include/os.h b/arch/um/include/os.h
index 5316e8a4a4fd..c73dfa78f943 100644
--- a/arch/um/include/os.h
+++ b/arch/um/include/os.h
@@ -329,6 +329,7 @@ extern void os_set_ioignore(void);
329extern void init_irq_signals(int on_sigstack); 329extern void init_irq_signals(int on_sigstack);
330 330
331/* sigio.c */ 331/* sigio.c */
332extern int add_sigio_fd(int fd);
332extern int ignore_sigio_fd(int fd); 333extern int ignore_sigio_fd(int fd);
333extern void maybe_sigio_broken(int fd, int read); 334extern void maybe_sigio_broken(int fd, int read);
334 335
diff --git a/arch/um/kernel/irq.c b/arch/um/kernel/irq.c
index 589c69a75043..ce7f233fc490 100644
--- a/arch/um/kernel/irq.c
+++ b/arch/um/kernel/irq.c
@@ -142,19 +142,6 @@ int activate_fd(int irq, int fd, int type, void *dev_id)
142 .events = events, 142 .events = events,
143 .current_events = 0 } ); 143 .current_events = 0 } );
144 144
145 /* Critical section - locked by a spinlock because this stuff can
146 * be changed from interrupt handlers. The stuff above is done
147 * outside the lock because it allocates memory.
148 */
149
150 /* Actually, it only looks like it can be called from interrupt
151 * context. The culprit is reactivate_fd, which calls
152 * maybe_sigio_broken, which calls write_sigio_workaround,
153 * which calls activate_fd. However, write_sigio_workaround should
154 * only be called once, at boot time. That would make it clear that
155 * this is called only from process context, and can be locked with
156 * a semaphore.
157 */
158 spin_lock_irqsave(&irq_lock, flags); 145 spin_lock_irqsave(&irq_lock, flags);
159 for (irq_fd = active_fds; irq_fd != NULL; irq_fd = irq_fd->next) { 146 for (irq_fd = active_fds; irq_fd != NULL; irq_fd = irq_fd->next) {
160 if ((irq_fd->fd == fd) && (irq_fd->type == type)) { 147 if ((irq_fd->fd == fd) && (irq_fd->type == type)) {
@@ -165,7 +152,6 @@ int activate_fd(int irq, int fd, int type, void *dev_id)
165 } 152 }
166 } 153 }
167 154
168 /*-------------*/
169 if (type == IRQ_WRITE) 155 if (type == IRQ_WRITE)
170 fd = -1; 156 fd = -1;
171 157
@@ -198,7 +184,6 @@ int activate_fd(int irq, int fd, int type, void *dev_id)
198 184
199 spin_lock_irqsave(&irq_lock, flags); 185 spin_lock_irqsave(&irq_lock, flags);
200 } 186 }
201 /*-------------*/
202 187
203 *last_irq_ptr = new_fd; 188 *last_irq_ptr = new_fd;
204 last_irq_ptr = &new_fd->next; 189 last_irq_ptr = &new_fd->next;
@@ -210,14 +195,14 @@ int activate_fd(int irq, int fd, int type, void *dev_id)
210 */ 195 */
211 maybe_sigio_broken(fd, (type == IRQ_READ)); 196 maybe_sigio_broken(fd, (type == IRQ_READ));
212 197
213 return(0); 198 return 0;
214 199
215 out_unlock: 200 out_unlock:
216 spin_unlock_irqrestore(&irq_lock, flags); 201 spin_unlock_irqrestore(&irq_lock, flags);
217 out_kfree: 202 out_kfree:
218 kfree(new_fd); 203 kfree(new_fd);
219 out: 204 out:
220 return(err); 205 return err;
221} 206}
222 207
223static void free_irq_by_cb(int (*test)(struct irq_fd *, void *), void *arg) 208static void free_irq_by_cb(int (*test)(struct irq_fd *, void *), void *arg)
@@ -302,10 +287,7 @@ void reactivate_fd(int fd, int irqnum)
302 os_set_pollfd(i, irq->fd); 287 os_set_pollfd(i, irq->fd);
303 spin_unlock_irqrestore(&irq_lock, flags); 288 spin_unlock_irqrestore(&irq_lock, flags);
304 289
305 /* This calls activate_fd, so it has to be outside the critical 290 add_sigio_fd(fd);
306 * section.
307 */
308 maybe_sigio_broken(fd, (irq->type == IRQ_READ));
309} 291}
310 292
311void deactivate_fd(int fd, int irqnum) 293void deactivate_fd(int fd, int irqnum)
@@ -316,11 +298,15 @@ void deactivate_fd(int fd, int irqnum)
316 298
317 spin_lock_irqsave(&irq_lock, flags); 299 spin_lock_irqsave(&irq_lock, flags);
318 irq = find_irq_by_fd(fd, irqnum, &i); 300 irq = find_irq_by_fd(fd, irqnum, &i);
319 if (irq == NULL) 301 if(irq == NULL){
320 goto out; 302 spin_unlock_irqrestore(&irq_lock, flags);
303 return;
304 }
305
321 os_set_pollfd(i, -1); 306 os_set_pollfd(i, -1);
322 out:
323 spin_unlock_irqrestore(&irq_lock, flags); 307 spin_unlock_irqrestore(&irq_lock, flags);
308
309 ignore_sigio_fd(fd);
324} 310}
325 311
326int deactivate_all_fds(void) 312int deactivate_all_fds(void)
diff --git a/arch/um/os-Linux/sigio.c b/arch/um/os-Linux/sigio.c
index 0ecac563c7b3..f6457765b17d 100644
--- a/arch/um/os-Linux/sigio.c
+++ b/arch/um/os-Linux/sigio.c
@@ -43,17 +43,9 @@ struct pollfds {
43/* Protected by sigio_lock(). Used by the sigio thread, but the UML thread 43/* Protected by sigio_lock(). Used by the sigio thread, but the UML thread
44 * synchronizes with it. 44 * synchronizes with it.
45 */ 45 */
46static struct pollfds current_poll = { 46static struct pollfds current_poll;
47 .poll = NULL, 47static struct pollfds next_poll;
48 .size = 0, 48static struct pollfds all_sigio_fds;
49 .used = 0
50};
51
52static struct pollfds next_poll = {
53 .poll = NULL,
54 .size = 0,
55 .used = 0
56};
57 49
58static int write_sigio_thread(void *unused) 50static int write_sigio_thread(void *unused)
59{ 51{
@@ -78,7 +70,8 @@ static int write_sigio_thread(void *unused)
78 n = os_read_file(sigio_private[1], &c, sizeof(c)); 70 n = os_read_file(sigio_private[1], &c, sizeof(c));
79 if(n != sizeof(c)) 71 if(n != sizeof(c))
80 printk("write_sigio_thread : " 72 printk("write_sigio_thread : "
81 "read failed, err = %d\n", -n); 73 "read on socket failed, "
74 "err = %d\n", -n);
82 tmp = current_poll; 75 tmp = current_poll;
83 current_poll = next_poll; 76 current_poll = next_poll;
84 next_poll = tmp; 77 next_poll = tmp;
@@ -93,35 +86,36 @@ static int write_sigio_thread(void *unused)
93 86
94 n = os_write_file(respond_fd, &c, sizeof(c)); 87 n = os_write_file(respond_fd, &c, sizeof(c));
95 if(n != sizeof(c)) 88 if(n != sizeof(c))
96 printk("write_sigio_thread : write failed, " 89 printk("write_sigio_thread : write on socket "
97 "err = %d\n", -n); 90 "failed, err = %d\n", -n);
98 } 91 }
99 } 92 }
100 93
101 return 0; 94 return 0;
102} 95}
103 96
104static int need_poll(int n) 97static int need_poll(struct pollfds *polls, int n)
105{ 98{
106 if(n <= next_poll.size){ 99 if(n <= polls->size){
107 next_poll.used = n; 100 polls->used = n;
108 return(0); 101 return 0;
109 } 102 }
110 kfree(next_poll.poll); 103 kfree(polls->poll);
111 next_poll.poll = um_kmalloc_atomic(n * sizeof(struct pollfd)); 104 polls->poll = um_kmalloc_atomic(n * sizeof(struct pollfd));
112 if(next_poll.poll == NULL){ 105 if(polls->poll == NULL){
113 printk("need_poll : failed to allocate new pollfds\n"); 106 printk("need_poll : failed to allocate new pollfds\n");
114 next_poll.size = 0; 107 polls->size = 0;
115 next_poll.used = 0; 108 polls->used = 0;
116 return(-1); 109 return -ENOMEM;
117 } 110 }
118 next_poll.size = n; 111 polls->size = n;
119 next_poll.used = n; 112 polls->used = n;
120 return(0); 113 return 0;
121} 114}
122 115
123/* Must be called with sigio_lock held, because it's needed by the marked 116/* Must be called with sigio_lock held, because it's needed by the marked
124 * critical section. */ 117 * critical section.
118 */
125static void update_thread(void) 119static void update_thread(void)
126{ 120{
127 unsigned long flags; 121 unsigned long flags;
@@ -156,34 +150,39 @@ static void update_thread(void)
156 set_signals(flags); 150 set_signals(flags);
157} 151}
158 152
159static int add_sigio_fd(int fd, int read) 153int add_sigio_fd(int fd)
160{ 154{
161 int err = 0, i, n, events; 155 struct pollfd *p;
156 int err = 0, i, n;
162 157
163 sigio_lock(); 158 sigio_lock();
159 for(i = 0; i < all_sigio_fds.used; i++){
160 if(all_sigio_fds.poll[i].fd == fd)
161 break;
162 }
163 if(i == all_sigio_fds.used)
164 goto out;
165
166 p = &all_sigio_fds.poll[i];
167
164 for(i = 0; i < current_poll.used; i++){ 168 for(i = 0; i < current_poll.used; i++){
165 if(current_poll.poll[i].fd == fd) 169 if(current_poll.poll[i].fd == fd)
166 goto out; 170 goto out;
167 } 171 }
168 172
169 n = current_poll.used + 1; 173 n = current_poll.used + 1;
170 err = need_poll(n); 174 err = need_poll(&next_poll, n);
171 if(err) 175 if(err)
172 goto out; 176 goto out;
173 177
174 for(i = 0; i < current_poll.used; i++) 178 for(i = 0; i < current_poll.used; i++)
175 next_poll.poll[i] = current_poll.poll[i]; 179 next_poll.poll[i] = current_poll.poll[i];
176 180
177 if(read) events = POLLIN; 181 next_poll.poll[n - 1] = *p;
178 else events = POLLOUT;
179
180 next_poll.poll[n - 1] = ((struct pollfd) { .fd = fd,
181 .events = events,
182 .revents = 0 });
183 update_thread(); 182 update_thread();
184 out: 183 out:
185 sigio_unlock(); 184 sigio_unlock();
186 return(err); 185 return err;
187} 186}
188 187
189int ignore_sigio_fd(int fd) 188int ignore_sigio_fd(int fd)
@@ -205,18 +204,14 @@ int ignore_sigio_fd(int fd)
205 if(i == current_poll.used) 204 if(i == current_poll.used)
206 goto out; 205 goto out;
207 206
208 err = need_poll(current_poll.used - 1); 207 err = need_poll(&next_poll, current_poll.used - 1);
209 if(err) 208 if(err)
210 goto out; 209 goto out;
211 210
212 for(i = 0; i < current_poll.used; i++){ 211 for(i = 0; i < current_poll.used; i++){
213 p = &current_poll.poll[i]; 212 p = &current_poll.poll[i];
214 if(p->fd != fd) next_poll.poll[n++] = current_poll.poll[i]; 213 if(p->fd != fd)
215 } 214 next_poll.poll[n++] = *p;
216 if(n == i){
217 printk("ignore_sigio_fd : fd %d not found\n", fd);
218 err = -1;
219 goto out;
220 } 215 }
221 216
222 update_thread(); 217 update_thread();
@@ -234,7 +229,7 @@ static struct pollfd *setup_initial_poll(int fd)
234 printk("setup_initial_poll : failed to allocate poll\n"); 229 printk("setup_initial_poll : failed to allocate poll\n");
235 return NULL; 230 return NULL;
236 } 231 }
237 *p = ((struct pollfd) { .fd = fd, 232 *p = ((struct pollfd) { .fd = fd,
238 .events = POLLIN, 233 .events = POLLIN,
239 .revents = 0 }); 234 .revents = 0 });
240 return p; 235 return p;
@@ -323,6 +318,8 @@ out_close1:
323 318
324void maybe_sigio_broken(int fd, int read) 319void maybe_sigio_broken(int fd, int read)
325{ 320{
321 int err;
322
326 if(!isatty(fd)) 323 if(!isatty(fd))
327 return; 324 return;
328 325
@@ -330,7 +327,19 @@ void maybe_sigio_broken(int fd, int read)
330 return; 327 return;
331 328
332 write_sigio_workaround(); 329 write_sigio_workaround();
333 add_sigio_fd(fd, read); 330
331 sigio_lock();
332 err = need_poll(&all_sigio_fds, all_sigio_fds.used + 1);
333 if(err){
334 printk("maybe_sigio_broken - failed to add pollfd\n");
335 goto out;
336 }
337 all_sigio_fds.poll[all_sigio_fds.used++] =
338 ((struct pollfd) { .fd = fd,
339 .events = read ? POLLIN : POLLOUT,
340 .revents = 0 });
341out:
342 sigio_unlock();
334} 343}
335 344
336static void sigio_cleanup(void) 345static void sigio_cleanup(void)