lkml.org 
[lkml]   [2001]   [Sep]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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/

\
 
 \ /
  Last update: 2005-03-22 13:03    [W:0.034 / U:3.308 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site