Continuing the procfs cleanup (was Re: updates to procfs?)
Alexander Viro wrote: > > On Tue, 19 Oct 1999, Jeff Garzik wrote: > > > Comments on your procfs patch: I like it. :) > > > > It looks like many xxx_read_proc operation in fs/proc/proc_array.c > > contains the _exact_ same code for adjusting 'len', 'start', etc. > > Cleanup should take care of that. > > > > Putting that stuff into an inline or plain macro would save a lot of > > [source] code. But I think proc_array.c can be made even smaller. > > Maybe 80% of the functions could be condensed into something like: > > > > DECLARE_SIMPLE_READ_PROC(filesystems_read_proc, get_filesystem_list) > > DECLARE_SIMPLE_READ_PROC(dma_read_proc, get_dma_list) > > Yes, but it's _not_ a good thing. See comment in the beginning of > proc_misc.c - the thing you are talking about is the wrapper turning > ->info-ish stuff into ->read_proc one. It's OK for the small things, but > for anything large... Look into drivers/pci/proc.c - I did a proper > conversion there and IMO the same should be done for the rest of them. Ok, well my main complaint is that there is duplicate code no matter how you scatter or update things. In pci_read_proc there is logic to calculate 'count', 'eof', etc. which is duplicated many times. My idea for solving this is to create struct proc_print, and function proc_printk: int proc_printk (struct proc_print *p, const char *format, ...) 'struct proc_print' would contain all the args passed to xxx_read_proc, and proc_printk would update that information each time it is called. proc_printk can also be set up to handle overflows of the procfs output buffer, multiple output buffers, etc.
Re: updates to procfs?
On Tue, 19 Oct 1999, Jeff Garzik wrote: > Comments on your procfs patch: I like it. :) > > It looks like many xxx_read_proc operation in fs/proc/proc_array.c > contains the _exact_ same code for adjusting 'len', 'start', etc. > Cleanup should take care of that. > > Putting that stuff into an inline or plain macro would save a lot of > [source] code. But I think proc_array.c can be made even smaller. > Maybe 80% of the functions could be condensed into something like: > > DECLARE_SIMPLE_READ_PROC(filesystems_read_proc, get_filesystem_list) > DECLARE_SIMPLE_READ_PROC(dma_read_proc, get_dma_list) Yes, but it's _not_ a good thing. See comment in the beginning of proc_misc.c - the thing you are talking about is the wrapper turning ->info-ish stuff into ->read_proc one. It's OK for the small things, but for anything large... Look into drivers/pci/proc.c - I did a proper conversion there and IMO the same should be done for the rest of them. > Still other functions in proc_array look like they can be similarly > simplified: > > DECLARE_READ_PROC(ksyms_read_proc, get_ksyms_list) > > IMHO it would be even better if proc_array didn't have to exist at all, > and the /proc hook would be 100% declared in the module where > get_xxx_list() exists. I don't think so. proc_array is for per-process stuff. I've moved the rest into proc_misc to have them together when the conversion to ->read_proc will come. For anything that can be longer then one page get_foo_list() is not an option - check the code and you will see. There are several areas outside of proc that also need a cleanup - MCA and SCSI stuff. Could you look at them?
Re: updates to procfs?
Comments on your procfs patch: I like it. :) It looks like many xxx_read_proc operation in fs/proc/proc_array.c contains the _exact_ same code for adjusting 'len', 'start', etc. Cleanup should take care of that. Putting that stuff into an inline or plain macro would save a lot of [source] code. But I think proc_array.c can be made even smaller. Maybe 80% of the functions could be condensed into something like: DECLARE_SIMPLE_READ_PROC(filesystems_read_proc, get_filesystem_list) DECLARE_SIMPLE_READ_PROC(dma_read_proc, get_dma_list) Still other functions in proc_array look like they can be similarly simplified: DECLARE_READ_PROC(ksyms_read_proc, get_ksyms_list) IMHO it would be even better if proc_array didn't have to exist at all, and the /proc hook would be 100% declared in the module where get_xxx_list() exists. Jeff
Re: updates to procfs?
On Wed, 13 Oct 1999, Jeff Garzik wrote: > Some on linux-kernel mentioned that procfs needed cleanup. Is there a > TODO list somewhere? Even an initial variant of patch (ouch... porting the thing from 2.3.13-pre1 to 2.3.22-pre2 _did_ hurt; damn CVS...). I can't promise that in the current state it _builds_, let alone works - there were changes and testing is in order. Sore points (partially addressed in the patch): a) way too many places know about the layout of proc_dir_entry. In most cases they don't need it - dynamical creation works quite fine. I've added several functions (create_proc_*entry/remove_proc_*entry) and I think that they should cover almost everything. See the patch for examples of usage. b) many moons ago, when procfs was small it was using an array of in-core structures that imitated on-disk inodes. Some inumbers were used by per-process stuff, some - by (then very few) special files (a-la /proc/loadavg, etc.). Methods used to be switches. Well, it didn't scale, so we went for the dynamic inumbers. But the old code stayed around and some of the new procfs files went there too. Some even went into switches. Resulting mess was _not_ nice. I tried to clean that up. c) after the switches period the interface went through several changes. We'ld better unify this stuff. I almost didn't touch this thing. d) many drivers failed to unregister the entries. Each of those cases is oopsable bug. I've fixed several such animals, but I suspect that some are still lurking. e) we don't need constant inumbers in procfs. If we will finally get rid of them we will be able to simplify the permission tests in per-process part and close several nasty holes with stale dentries. f) proc_unregister() needs some form of revoke(). It even implements something, but I suspect that it's racey. g) proc/mem.c is a living horror. Look at it and you'll see. h) per-process part really ought to be separated (codewise) from the rest. Different needs, different races, etc... i) tons of additional fun. IMO the first thing that should be done is the exorcism - knowledge of procfs guts should be driven out of the rest of tree. Then we will be able to deal with procfs problems not causing the massive changes in the $BIGNUM drivers. Until that will be done we can't do much with procfs proper. I've dropped the patch (against 2.3.22-pre2) on the ftp.math.psu.edu/pub/viro/proc-patch-22-2.gz. Feel free to look it through/try to test it. Comments/questions/flames to [EMAIL PROTECTED] Cheers, Al
