Greg KH, on 10/22/2010 10:54 PM wrote: > On Fri, Oct 22, 2010 at 10:40:34PM +0400, Vladislav Bolkhovitin wrote: >>>> + unsigned int tgt_kobj_initialized:1; >>> >>> It's the middle of the merge window, and I'm about to go on vacation, so >>> I didn't read this patch after this line. >>> >>> It's obvious that this patch is wrong, you shouldn't need to worry about >>> this. And even if you did, you don't need this flag. >>> >>> Why are you trying to do something that no one else needs? Why make >>> things harder than they have to be. >> >> I tried to explain that to you in http://lkml.org/lkml/2010/10/14/291 >> and mentioned there the need to create this flag to track >> half-initialized kobjects. You agreed >> (http://lkml.org/lkml/2010/10/14/299) that don't return half-initialized >> objects is a regular kernel practice, but then requested to strictly >> bound the larger object freeing to its kobject release(), which means >> that all SYSFS creating functions now have to return half-initialized >> SYSFS hierarchy in case of any error. Hence the flag to track it. > > I agreed that you needed to do something about it, not that this is the > correct way to do it. > > Think for a second as to why your code path looks different from EVERY > other kobject user in the kernel. Perhaps it is incorrect? You don't > need all this completion mess, in fact, it's wrong. > > Just do what everyone else does please, as that is the simpler, and > correct, way to do it. Hello Greg, Why SCST objects are different from most other kernel objects and why they can't be implemented the same way as them is exactly what I'm trying to explain you. Let me try again from a bit different angle. SCST objects are different from the most other kernel objects, because they are very complex, hence complex to initialize and delete with dependencies in order how initialization and delete actions should be performed. Particularly, SCST objects have a lot of attributes and sub-objects, so their kobjects can't be inited and exposed to SYSFS or removed from it until they reach some point during initialization or delete correspondingly, otherwise their attributes' reads and writes can crash. I can elaborate all those on examples, if you request. I understand you are very busy and appreciate very much your review and comments, so be glad to implement what you are requesting. But I don't see how to implement that in an acceptable manner with neither the completion-based delete as now, nor with x_initialized flag as in the previous patch. Note, SCST's kobjects are not the only kobjects in the kernel using the completion based delete. See, for instance, ktype_cpufreq, ktype_cpuidle or ext4_ktype. Also, elevator (struct elevator_queue) uses "registered" flag to see if it was added to the SYSFS. Thus, you can see, both the completion and the flag approaches already used in the kernel. Hence, I believe, the way how SCST is doing it should also be acceptable. I'm not a person who, as you, probably, guessed, at first doing, only then thinking and reading docs. I before writing a code line at first read all the available docs and carefully consider possible alternatives, so there isn't a well thought bit in SCST. I believe, the completions aren't mess, they are refinement. > Oh, and why are you using a kobject at all anyway? Shouldn't you be > using a 'struct device'? We need only to represent our internal SCST objects on the SYSFS. The objects are not devices, but special entities for special purposes. For instance, struct scst_session is a representation of SCSI I_T nexuses. Struct scst_tgt_dev - I_T_L nexuses. Another example is struct scst_acg, which is a representation of per initiator access control groups to determine which initiators can see which LUNs. Hence, for such purpose kobjects are fully sufficient. Thanks, Vlad -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
