Hi Matthias, Did you get a chance to find a patch you mentioned?
I've tried to implement the code using your approach and it turned out such implementation is by far not trivial. The problem is that it is not safe to execute the queue in dap_cmd_new(). DAP queue is not fully 'assembled' and internal state is not coherent when dap_cmd_new() is called (for instance, dap->last_read still holds previous value). Running the queue results in incorrect data been transmitted/received. Moreover, dap_run() itself calls dap_cmd_new() so I had to use soft-limit for the number of allocated commands (execute the queue when number of allocated commands reaches 90% of the hard-limit). This makes the code quite complicated. What do you think about easier way: allocate dap_cmd from the pool, but check the limits in jtag_dp_q_read, jtag_dp_q_write, jtag_ap_q_read and jtag_ap_q_write. This guarantees that internal state is coherent and it is safe to run the queue. Bohdan ср, 13 лют. 2019 о 15:14 Matthias Welwarsky <[email protected]> пише: > On Mittwoch, 13. Februar 2019 13:48:21 CET Bohdan Tymkiv wrote: > > > Hi all, > > > > > > I've found an interesting issue while working with 64 MiB external QSPI > > > flash bank. Bank is memory mapped, so 'default_flash_read()' is used in > the > > > flash driver. OpenOCD consumes as much as 6.8 GiB (!!!) of RAM when I am > > > trying to read (flash read_bank) or verify (flash verify_bank) the > contents > > > of this bank. This is reproducible with JTAG transport only. > > > > > > That was surprising so I've made small investigation and found that most > of > > > the memory is allocated in: > > > cmd_queue_alloc (commands.c) - 4.2 GiB > > > dap_cmd_new (adi_v5_jtag.c) - 2.25GiB > > > > > > This happens because JTAG queue size is not limited in any way. OpenOCD > > > queues 16 million of AP reads allocating all corresponding data > structures. > > > Full valgrind log is available on pastebin: > > > https://pastebin.com/raw/0vjHXxk6 > > > > > > Some of the possible solutions to the problem are: > > > [1] Check the number of queued commands in adi_v5_jtag.c within > > > jtag_(dp|ap)_q_(read|write) functions and forcibly execute the queue by > > > calling dap_run() when number of queued commands exceeds some limit. I am > > > currently testing this approach and it seems to work correctly, but this > > > change affects all targets so I am not sure if it will not make things > > > broken. > > > > > > [2] Read data in small chunks (e.g. 64 KiB) in > > > handle_flash_read_bank_command etc. This is more safe but it does not > cover > > > all possible cases. > > > > > > Any suggestion on this? I am ready to submit the patch [1] to gerrit but > I > > > would like to hear the opinion of the community. > > > > I like the idea [1]. I remember I had a patch around myself to counter the > excessive memory usage. But I wasn't sure about the performance impact and > so I never submitted it. I'm trying to recall where and in which context I > did that patch and if it's still around in some branch. > > > > If you craft such a patch, I suggest you do the following: > > - make a pool of "struct dap_cmd". limit the pool size to a sane number > > - dap_cmd_new() checks the pool for available objects first > > * allocates a new one if none available and limit not reached > > * call dap_run() if limit reached. Mind you, error handling is tricky! > > - create a dap_cmd_free() that returns dap_cmd objects to the pool instead > of freeing them (modified version of flush_journal()). > > - maybe add some garbage collection to flush the pool after a while > > > > BR, > > Matthias > > > > > > > > Thanks, > > > Bohdan Tymkiv > > > > > > -- > > Mit freundlichen Grüßen/Best regards, > > > > Matthias Welwarsky > > Project Engineer > > > > SYSGO AG > > Office Mainz > > Am Pfaffenstein 14 / D-55270 Klein-Winternheim / Germany > > Phone: +49-6136-9948-0 / Fax: +49-6136-9948-10 > > VoIP: SIP:[email protected] > > E-mail: [email protected] / Web: http://www.sysgo.com > > _________________________________________________________________________________ > Web: https://www.sysgo.com > > Blog: https://www.sysgo.com/blog > > Events: https://www.sysgo.com/events > > Newsletter: https://www.sysgo.com/newsletter > _________________________________________________________________________________ > Handelsregister/Commercial Registry: HRB Mainz 90 HRB 8066 > > Vorstand/Executive Board: Etienne Butery (CEO), Kai Sablotny (COO) > > Aufsichtsratsvorsitzender/Supervisory Board Chairman: Marc Darmon > > USt-Id-Nr./VAT-Id-No.: DE 149062328 >
_______________________________________________ OpenOCD-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/openocd-devel
