From mboxrd@z Thu Jan 1 00:00:00 1970 From: Abramo Bagnara Subject: Re: [alsa-cvslog] CVS: alsa-kernel/core rawmidi.c,1.40,1.41 Date: Mon, 02 Feb 2004 11:12:26 +0100 Sender: alsa-devel-admin@lists.sourceforge.net Message-ID: <401E228A.7010104@tin.it> References: <401D318E.4020100@tin.it> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: Errors-To: alsa-devel-admin@lists.sourceforge.net List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , List-Archive: To: Jaroslav Kysela Cc: alsa-devel List-Id: alsa-devel@alsa-project.org Jaroslav Kysela ha scritto: > On Sun, 1 Feb 2004, Abramo Bagnara wrote: > > >>Jaroslav Kysela ha scritto: >> >>>Modified Files: >>> rawmidi.c >>>Log Message: >>>copy_*_user() function cannot be called from spinlock context >>> >>>Index: rawmidi.c >>>=================================================================== >>>RCS file: /cvsroot/alsa/alsa-kernel/core/rawmidi.c,v >>>retrieving revision 1.40 >>>retrieving revision 1.41 >>>diff -u -r1.40 -r1.41 >>>--- rawmidi.c 23 Oct 2003 14:34:52 -0000 1.40 >>>+++ rawmidi.c 1 Feb 2004 16:34:28 -0000 1.41 >>>@@ -909,10 +909,11 @@ >>> if (kernel) { >>> memcpy(buf + result, runtime->buffer + runtime->appl_ptr, count1); >>> } else { >>>+ spin_unlock_irqrestore(&runtime->lock, flags); >>> if (copy_to_user(buf + result, runtime->buffer + runtime->appl_ptr, count1)) { >>>- spin_unlock_irqrestore(&runtime->lock, flags); >>> return result > 0 ? result : -EFAULT; >>> } >>>+ spin_lock_irqsave(&runtime->lock, flags); >>> } >>> runtime->appl_ptr += count1; >>> runtime->appl_ptr %= runtime->buffer_size; >>>@@ -1133,10 +1134,13 @@ >>> if (kernel) { >>> memcpy(runtime->buffer + runtime->appl_ptr, buf, count1); >>> } else { >>>+ spin_unlock_irqrestore(&runtime->lock, flags); >>> if (copy_from_user(runtime->buffer + runtime->appl_ptr, buf, count1)) { >>>+ spin_lock_irqsave(&runtime->lock, flags); >>> result = result > 0 ? result : -EFAULT; >>> goto __end; >>> } >>>+ spin_lock_irqsave(&runtime->lock, flags); >>> } >>> runtime->appl_ptr += count1; >>> runtime->appl_ptr %= runtime->buffer_size; >> >> >>With this patch you break write and read atomicity, you should: >> >>1) read and save appl_ptr >>2) update appl_ptr and avail >>3) leave critical area >>4) copy from/to user using saved appl_ptr > > > But hardware can overwrite data (read) or send data which are not yet > available (write) when we are not in the spinlock context and data are not > copied. I see. > If we need atomic read() and write() we must probably add a semaphore. Too heavyweight, I propose: 1) size_t ptr = appl_ptr_next 2) update appl_ptr_next etc. 3) pending++ 4) spin_unlock 5) copy from/to user 6) spin_lock 7) if (--pending) appl_ptr = appl_ptr_next -- Abramo Bagnara mailto:abramo.bagnara@tin.it Opera Unica Phone: +39.0546.656023 Via Emilia Interna, 140 48014 Castel Bolognese (RA) - Italy ------------------------------------------------------- The SF.Net email is sponsored by EclipseCon 2004 Premiere Conference on Open Tools Development and Integration See the breadth of Eclipse activity. February 3-5 in Anaheim, CA. http://www.eclipsecon.org/osdn