Messages in this thread |  | | Date | Sat, 22 Sep 2001 16:30:12 -0700 | From | Dan Kegel <> | Subject | Re: [PATCH][RFC] Signal-per-fd for RT signals; write_lock_bh(file_lock)? |
| |
Vitaly Luban wrote: > Could you please try attached one? It's mostly untested, but my home site > will be down next week.
I just had time to read your new patch; have not yet run it. I think there may be some problems in send_signal:
1) might still have null pointer dereference. If files is NULL, following section + if( newsignal && q ) + filep->f_infoptr = q; + write_unlock( &files->file_lock); may crash due to NULL filep and unlocking an already unlocked lock. BTW, you might be able to use a read lock there.
2) You're using a kind of spinlock that compiles to a no-op on single processor, but the crash (see the old oops traceback in http://marc.theaimsgroup.com/?l=linux-kernel&m=100055931808169&w=2 ) happens on a single processor, so your locking shouldn't help. The conflict is between a bottom half and normal. Thus write_lock, besides not helping on uniprocessor, might cause a deadlock on smp. Might have to convert the write_lock in get_unused_fd() and friends to be a write_lock_bh() instead to prevent the bottom half from running until the normal part is done with the file lock!
This bottom-half-vs-normal issue worries me greatly. It may take a lot of thought to fix this. Then again, maybe I'm just confused; I've never dealt with kernel spinlocks before, and anyone who reads my posts regularly knows that I'm wrong most of the time. Anyone who actually *understands* this stuff, please speak up with corrections...
3) you still save info in the file even if you don't end up queuing a signal; your changes in send_signal should be in the default case of the switch, just to keep things in a good state.
4) collect_signal references the file table, so it needs to lock it; probably read_lock (but maybe write_lock, I dunno), and probably _bh, too, to avoid deadlock.
> I'm looking forward to see a test case, all I could come up with happily > runs on the old version.
I'll try to put it together today. - Dan - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
|  |