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/
This archive was generated by hypermail 2b29 : Sun Sep 23 2001 - 21:00:51 EST