Re: [PATCH 1/5] Use mutex instead of semaphore in the Host AP driver
From: Satyam Sharma <hidden>
Date: 2007-07-30 19:59:39
Also in:
lkml
On Mon, 30 Jul 2007, Andrew Morton wrote:
On Mon, 30 Jul 2007 19:09:38 +0200 Michael Buesch [off-list ref] wrote:quoted
On Monday 30 July 2007, Satyam Sharma wrote:quoted
On Mon, 30 Jul 2007, Michael Buesch wrote:quoted
On Sunday 29 July 2007 23:34, Matthias Kaehlcke wrote:quoted
@@ -902,7 +902,7 @@ static int hfa384x_set_rid(struct net_device *dev, u16 rid, void *buf, int len) /* RID len in words and +1 for rec.rid */ rec.len = cpu_to_le16(len / 2 + len % 2 + 1); - res = down_interruptible(&local->rid_bap_sem); + res = mutex_lock_interruptible(&local->rid_bap_mtx); if (res) return res;Is res returned to userspace? If yes, that's not right.Yup, that's not right.quoted
On a interrupted mutex allocation you should return -ERESTARTSYS to userspace.Nope, userspace must not see ERESTARTSYS (I /think/ this is the third time I'm participating in this exact same discussion :-) If the return would be caught by a previous in-kernel caller in the call chain, ERESTARTSYS is okay and it could try to restart the operation. However, if the return goes unfiltered directly to userspace, EINTR is the correct choice.Last time I submitted a patch which returned EINTR to userspace, people came and said it was wrong. It was said that we must return ERESTARTSYS to the libc and the libc will convert that into either EINTR or automatically restart it immediately. (You can set that in the sigaction stuff). So either the one who told me that last time was wrong, or you. :) Andrew? I think you were involved in this discussion I mentioned. (It was about the HW-RNG chardev, but that doesn't matter here).who, me? signals? Danger. yes, I think ERESTARTSYS is correct. That gets converted to -EINTR by the arch's signal handling code (not by glibc) just before we return to userspace.
Hmm? But the question is whether which of EINTR or ERESTARTSYS is correct to be returned to userspace (so, EINTR). But now that you pointed to the automagic ERESTARTSYS-to-EINTR conversion in the arch/ code, I think there's no problem returning ERESTARTSYS from syscalls in generic kernel code when interrupted by a signal either (if all arch's do that conversion). Probably makes sense to change down_interruptible() and mutex_lock_interruptible() to return ERESTARTSYS (and not EINTR) in that case. wait_event_interruptible() is already ERESTARTSYS, I see ... Satyam