linux-kernel - Re: [PATCH 0/5] staging: comedi: split bus support into separate modules

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 31 Oct 2014 22:53:39 +0000
From:	Ian Abbott <abbotti@....co.uk>
To:	Hartley Sweeten <HartleyS@...ionengravers.com>,
	"driverdev-devel@...uxdriverproject.org" 
	<driverdev-devel@...uxdriverproject.org>
CC:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 0/5] staging: comedi: split bus support into separate
 modules

On 31/10/14 22:18, Hartley Sweeten wrote:
> On Friday, October 31, 2014 3:09 PM, Ian Abbott wrote:
>> On 31/10/14 18:19, Hartley Sweeten wrote:
>>> On Friday, October 31, 2014 10:48 AM, Ian Abbott wrote:
>>>> The Comedi core module doesn't need support for PCI, USB or PCMCIA.
>>>> Only the low-level Comedi drivers need it.  Split the support for these
>>>> bus types out of the core "comedi" module and into new modules,
>>>> "comedi_pci", "comedi_usb", and "comedi_pcmcia".
>>>>
>>>> 1) staging: comedi: comedidev.h: remove dummy PCI support functions
>>>> 2) staging: comedi: comedidev.h: remove some #ifdefs
>>>> 3) staging: comedi: split out PCMCIA support into new module
>>>> 4) staging: comedi: split out USB support into new module
>>>> 5) staging: comedi: split out PCI support into new module
>>>
>>> Ian,
>>>
>>> Is this really necessary?
>>>
>>> The pci, usb, and pcmcia support is already conditionally compiled in.
>>> The support does get added to the main comedi module instead of
>>> as separate modules but that shouldn't be a problem.
>>
>> Well a lot of potentially unused module space could get pulled in if
>> using a stock distro kernel. For example, the USB dependencies amount to
>> over 200k. Not so bad for the others, especially PCI where the code is
>> built in anyway.
>
> But its only pulled in if the user selected the usb, pci, and/or pcmcia
> comedi drivers. And if they selected any of those options they will
> need the that code anyway.

Yes, if the user configured and compiled the kernel his or herself.  I 
was more concerned with using distro kernel binaries.

> Splitting the bus specific code out of the comdi core module doesn't
> lower the total module space used it just spreads it from one to possibly
> four modules. And it increases the module dependancies for all the
> usb, pci, and pcmcia comedi drivers.

Yes, you might have to load one more module to load the modules for a 
particular card, but modprobe can take care of that. It's only a pain if 
you're loading the modules individually.  On the other hand, to load the 
"comedi_test.ko" module using insmod on a desktop PC on Debian for 
example (assuming the USB modules are already loaded), you'd first have 
to load the "pcmcia_core" and "pcmcia" modules before the "comedi" and 
"comedi_test" modules. But with the PCMCIA support separated out, you'd 
just have to load the "comedi" and "comedi_test" modules.

> To be clear, I'm not against this series I just want to make sure it's worth
> applying.

I like the idea of the core comedi module being separated from all the 
various buses. And other buses can be added easily without pulling extra 
dependencies into the core (e.g. for platform or spi devices).

-- 
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti@....co.uk> )=-
-=(                          Web: http://www.mev.co.uk/  )=-
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/