Greg KH wrote: >On Fri, Feb 22, 2002 at 08:38:10PM -0800, Mark McClelland wrote: > >>Here's a treediff between your tree and mine (which also doubles as a >>patch). As you can see, my tree is OK, but its metadata must be corrupt. >> > >No, it means that you didn't send me all of the changesets that you have >added to my tree since you last pulled it. >
Ever since I created the ov511-1.49 changeset, I get a revision to my ChangeSet file whenever I do a pull: [EMAIL PROTECTED], 2002-02-25 19:10:39-08:00, [EMAIL PROTECTED] Merge bk://linuxusb.bkbits.net/usb-2.5 into hal9000.dyndns.org:/mnt/home2/mark/tmp/bk/usb-2.5 Should I be sending them as well? Those are the only other changesets attributed to myself. I tried to see what needed to be sent with "bk send -ubk://linuxusb.bkbits.net/usb-2.5 - | less" but it didn't work (bk says "ERROR-BAD CMD: synckeys"). Since I'm not having much luck with bk, I guess I'll just stick with trusty old diff and patch for now :) <snip> >>+ * This program is free software; you can redistribute it and/or modify it >>+ * under the terms of the GNU General Public License as published by the >>+ * Free Software Foundation; version 2 of the License. You may also, at your >>+ * option, follow the terms under the heading "BSD LICENSE", below. >> > >I don't see such a heading below :) > >And if this is a dual licensed module, please label it as such in the >MODULE_LICENSE() macro. > It isn't; that sentence shouldn't be there. Thanks for noticing that. >I'd recommend removing the decomp_lock and decomp_unlock fields from >your interface and just using a simple, owner: THIS_MODULE, type field, >like the file_ops and usb core uses. > Good idea. Since that would change the driver, I'd like to sync the (2.5) kernel to my latest version before doing this. I assume I should send you the seven patches that do that separately, versus one big version 1.49 -> 1.56 patch. >Then when you would normally call decomp_lock, you would do something >like: > if (decomp_ops->owner) > __MOD_INC_USE_COUNT(decomp_ops->owner); > >which will probably enable you to get rid of lock_kernel() in >request_decompressor() and release_decompressor(). > I think it's probably safe to replace lock_kernel() with a global semaphore. There does seem to be a race that I never noticed until now though. The sequence of events is: 1. request_decompressor() is called. 2. User rmmod's the decompressor, which calls ov511_deregister_decomp_module() from its __exit function. 3. request_decompressor() gets the BKL first, and increases the decompressor's module use count. 4. ov511_deregister_decomp_module() gets the BKL and completes, and the decompressor unloads (while it is being used!) If a module's use count increases while it is in its __exit function, it still unloads regardless (it's true; I tested). I guess I need to check the count in ov511_deregister_decomp_module() and block until it hits zero. It seems that this race could foil the protections we use against module unload races. For example, if a USB driver is in the middle of its __exit function when a device is plugged in, it doesn't matter whether the driver's use count is increased before or during probe(); it will still be unloaded anyway. Maybe there's something obvious I'm missing here, though. I should probably be worrying about ov511's horrible usb_free_urb() SMP races instead anyway. :) -- Mark McClelland [EMAIL PROTECTED] _______________________________________________ [EMAIL PROTECTED] To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
