Re: [PATCH 1/3] Compile iser driver as a loadable module
|[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
On 10/14/2011 01:09 AM, Andy Grover wrote:
With iser being built in, the tgtd package pulls in two more libraries, libibverbs and librdmacm, is the motivation here to eliminate that as of the overhead?
Yes. Admittedly not a huge issue, but tgt code is already modularized, so the entire change is to the Makefile, except for one line.
So, any objections to this change? Going once... Thanks -- Andy
Hi NB: Sorry for a late catch-up. I was not following the list for a while. I think that in general the idea of modularizaton is a good and a very appropriate one. Still, i have a few reservations about the suggested implementation. It singles out iser as the only module, which is "modularized-out". This is ok for now, and this is surely what a multitude of users would like to do, but i would opt for a change which is framework-wide. I suggest making ALL drivers loadable, in the following manner: 1) all LLDs are compiled as shared objects 2) any LLD can be loaded and unloaded (only when unused) administratively using a new tgtadm command; proposed format: # tgtadm --lld <lld-name> --mode system --op lldload # tgtadm --lld <lld-name> --mode system --op lldunload 3) some of the drivers (notably: iscsi) should be loaded (and not compiled in) by default 4) the list of default drivers may be changed by a new command line parameter, proposed format: # tgtd ... --lld <lld-name list> example: # tgtd --lldiscsi,iser,fc_over_pigeon_mail but they can be also unloaded afterward, if necessary, using the above (see 2) commands 5) All the above is relevant not only for LLDs but also for backing-stores. Thus the following additions are due: # tgtadm --bstype <bs-name> --mode system --op bsload # tgtadm --bstype <bs-name> --mode system --op bsunload # tgtd ... --bstype <bs-type list> The obvious current candidate for default BS is of course "rdwr". --- Some further notes: * If this proposal is adapted, an RPM packager can pick a set of drivers and backing-stores that he or she sees as the most appropriate (some official, some perhaps custom) and install a service script that starts tgtd with the default drivers sub-set that he deems right for the system/distro etc. * Just as most of the users would like to have only iscsi in their setup, some may want to run in iser-only environment. Obviously, iscsi does not require any special modules, but this could be useful to guarantee that all configured targets are iser only and no administrative mistake has happened. * An example for BS modularization was a recent mail by Kevin Baughman <curb_pks@xxxxxxxxx> who asked for a very specialized type of BS. Modularization would allow him and others to develop this BS-type independently (with or without pushing upstream), and be able to load it into any running tgtd, be it official or custom one (provided the API is fixed).* Browsing the LLD-related code we have identified a problematic scenario.
Suppose a LLD's init() fails. In case of iser this can be due to RDMA hardware absent or its drivers not loaded, in case of others this could happen because of allocation failures or other rare conditions. Then the LLD's descriptor is left on the list not initialized properly, so that the user may add targets and luns to it, although the LLD is clearly dysfunctional. I will send some patches, that fix this (marking the LLD in error), shortly. Suppose that we have recognized this situation and reloaded the missing modules, or freed some memory or otherwise fixed the problems. Currently we have to restart tgtd, in order to get a chance to re-init the LLD(s). If we had the abovementioned functionality, then # tgtadm --lld <lld-name> --mode system --op lldload would fix the problem dynamically. Of course something like # tgtadm --lld <lld-name> --mode system --op lldinit would do even for static implementation, but "--op lldload" is more general and probably cleaner: if we always unload the LLD on error we don't leave any allocated memory or state behind. * I am willing to co-operate actively in implementation of these features, when all agree on their design. Alexander Nezhinsky -- To unsubscribe from this list: send the line "unsubscribe stgt" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html