Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751645AbWJEKny (ORCPT ); Thu, 5 Oct 2006 06:43:54 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1751646AbWJEKnx (ORCPT ); Thu, 5 Oct 2006 06:43:53 -0400 Received: from mx2.suse.de ([195.135.220.15]:42407 "EHLO mx2.suse.de") by vger.kernel.org with ESMTP id S1751641AbWJEKnw (ORCPT ); Thu, 5 Oct 2006 06:43:52 -0400 Date: Thu, 05 Oct 2006 12:43:50 +0200 Message-ID: From: Takashi Iwai To: Karsten Wiese Cc: mingo@elte.hu, alsa-devel@lists.sourceforge.net, linux-kernel@vger.kernel.org Subject: Re: [Alsa-devel] [PATCH] Reset file->f_op in snd_card_file_remove(). Take 2 In-Reply-To: <200610050141.47847.annabellesgarden@yahoo.de> References: <200609282228.02611.annabellesgarden@yahoo.de> <200610042201.53337.annabellesgarden@yahoo.de> <200610050141.47847.annabellesgarden@yahoo.de> User-Agent: Wanderlust/2.12.0 (Your Wildest Dreams) SEMI/1.14.6 (Maruoka) FLIM/1.14.7 (=?ISO-8859-4?Q?Sanj=F2?=) APEL/10.6 MULE XEmacs/21.5 (beta25) (eggplant) (+CVS-20060326) (i386-suse-linux) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2615 Lines: 92 At Thu, 5 Oct 2006 01:41:47 +0200, Karsten Wiese wrote: > > Am Mittwoch, 4. Oktober 2006 22:15 schrieb Takashi Iwai: > > > > This looks like a good optoin. But one thing we have to be careful > > about is the module counter since the owner is different between the > > old f_op and disconnect_f_op... > > > here is rc1, will test later. > Feel free to pick it apart ;-) Any special reason to make it separate instead of patching init.c? Most of codes (e.g. dummy callbacks) are already in init.c. > struct snd_disconnected_file { > struct file *file; > int (*release) (struct inode *, struct file *); > struct snd_disconnected_file *next; We can use a standard list here. > }; > > static struct snd_disconnected_file *disconnecting_files; > static struct file_operations snd_disconnect_f_ops; > static DEFINE_MUTEX(mutex); > > void snd_disconnect_file(struct file *file, int (*release) (struct inode *, struct file *)) > { > struct snd_disconnected_file *df, **_dfs; > df = kmalloc(sizeof(struct snd_disconnected_file), GFP_ATOMIC); > if (df == NULL) > panic("Atomic allocation failed for snd_disconnected_file!"); IIRC, the reason that snd_card_disconnect() uses GFP_ATOMIC is that (usb-)disconnection was atomic in the earlier time. You're using mutex here, hence no reason to allocate with GFP_ATOMIC. > df->file = file; > df->release = release; > df->next = NULL; > > mutex_lock(&mutex); > _dfs = &disconnecting_files; > while (*_dfs != NULL) > _dfs = &(*_dfs)->next; > *_dfs = df; You can add to the item to head :) The order doesn't matter. > mutex_unlock(&mutex); > > { > const struct file_operations *old_f_op = file->f_op; > fops_get(&snd_disconnect_f_ops); > file->f_op = &snd_disconnect_f_ops; > fops_put(old_f_op); I wonder whether the old release might be called during this operation. Then df won't be freed. > static int snd_disconnect_release(struct inode *inode, struct file *file) > { > struct snd_disconnected_file *df, **_dfs, **__dfs; > int err = 0; > __dfs = _dfs = &disconnecting_files; > > mutex_lock(&mutex); > while ((df = *_dfs)) > if (df->file == file) { > *__dfs = df->next; > break; > } else { > __dfs = _dfs; > _dfs = &df->next; > } > mutex_unlock(&mutex); A standard list would make the code more readable (unless you use too many underscores ;) Thanks, Takashi - 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/