----- Original Message ----- > > > > Date: Fri, 19 Jun 2015 12:23:39 -0400 > > From: anderson@xxxxxxxxxx > > To: crash-utility@xxxxxxxxxx > > Subject: Re: [PATCH v2] files: support dump file pages from > > its address space > > > > > > Here's what I suggest. Instead of using RADIX_TREE_DUMP, create a new > > > > flag > > > > RADIX_TREE_DUMP_CALLBACK, and you can store the callback function > > > > address inside > > > > the general purpose pc->curcmd_private member. > > > > > > I'm ok with use a different flag. Although I didn't find any source codes > > > are > > > using RADIX_TREE_DUMP flag. > > > > > > But I had some concerns for storing callback address in > > > pc->curcmd_private. > > > > > > I noticed that the pc->curcmd_private is already used by other crash code > > > at > > > many locations. > > > > Right, but only within the context of specific commands, such as "irq -u", "vm -M", > > "search", "struct -f" and "runq -c". Similar to using GETBUF(), the pc->curcmd_private > > is only in effect within the period of time a particular command is running, and > > is cleared by restore_sanity() prior to the next "crash> " prompt. > > > > > > If someday people want to use the RADIX_TREE_DUMP_CALLBACK at that context, > > > it may cause the troubles. > > > > How? If they want to do so within the context of a different command, they > > can set the pc->curcmd_private field to their own function. That's what > > it's designed to be used for. > > The do_radix_tree is a general purpose api, but if we wanted to use it in "irq -u" "search" > command, we would add the limitations here. The pc->curcmd_private already used in these > commands. OK, yes, in that case, point taken... > > > > > How about this: have your new RADIX_TREE_DUMP_CALLBACK option use the rtp->value > > (void *) member to store the callback function? As long as it's documented, I would > > have no problem accepting that as well. > > I think this is a good idea. > > Below is my plan, > > a. add the RADIX_TREE_DUMP_CB flag for do_radix_tree api. > b. Change the rtp definition, use union to replace original void * value definition. > > I think pre-build extension should be able to work with new code changes. > The union definition could make api easy to be understand. It may work with pre-existing extension modules, but if an extension module that uses a radix_tree_pair structure gets re-compiled, it fails like this: error: 'struct radix_tree_pair' has no member named 'value' I should have recommended using the radix_tree_pair.value as my first suggestion, but I was mistakenly thinking that your code was already using it. (although even if if you were using it, the value pointer could still be used, for example, to point to a data structure containing a array pointer and your callback function) So please just cast the value member to the callback function. Thanks, Dave -- Crash-utility mailing list Crash-utility@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/crash-utility
