Hi Alain, Marcel, On Thu, Jan 23, 2020 at 10:20 AM Alain Michaud <alainmichaud@xxxxxxxxxx> wrote: > > Hi Marcel, > > That makes sense. Adding +Archie Pusaka as well who may have input into this. > > Thanks, > Alain > > On Thu, Jan 23, 2020 at 1:16 PM Marcel Holtmann <marcel@xxxxxxxxxxxx> wrote: > > > > Hi Alain, > > > > > From a high level, this looks good for me although I agree, this is an > > > order of magnitude bigger in terms of scope. Can you suggest perhaps > > > an interactive way to deliver this over a period of time, perhaps > > > prioritizing the BT_DEBUG kernel messages first? :) > > > > I am always in favor of increasing the ability to debug things, but we need to do this in a clean fashion and not some short term hacks (since they will come back and haunt us). I like to get some review on my idea first. > > > > What we could do is work on the BT_DBG etc infrastructure to allow switching when dynamic_debug is not available. Then you would use some debugfs toggle in /sys/kernel/debug/bluetooth since that is no stable API for us (and of course the clear understanding that this toggle is temporary). > > Not sure if I fully understood the problem, so I guess we are looking to a solution to replace dynamic_debug since that is not normally enabled on production? Not sure if this should be discussed with kernel community as whole because it does lead to each subsystem reinventing their own mechanism of logging. Now logging the kernel message into btmon I thing would be very useful, regardless on what the mechanism would be used to enable them, so perhaps we should start with that. I fill that enabling the exact same granularity as the dynamic_debug has would be a bit overkill so Id would suggest sticking with the current categories that we have for the monitor which are: #define BTSNOOP_PRIORITY_EMERG 0 #define BTSNOOP_PRIORITY_ALERT 1 #define BTSNOOP_PRIORITY_CRIT 2 #define BTSNOOP_PRIORITY_ERR 3 #define BTSNOOP_PRIORITY_WARNING 4 #define BTSNOOP_PRIORITY_NOTICE 5 #define BTSNOOP_PRIORITY_INFO 6 #define BTSNOOP_PRIORITY_DEBUG 7 Though I see Marcel's point that if we go this way enabling DEBUG level would simple flood the trace, but I believe the problem can be solved with a minimal change which is to split data (above L2CAP/RFCOMM) and signalling logs, obviously this would require a spit on the way BT_DBG works so we can actually say dump the data path or just the signalling (this should probably be the default), which I think would benefit us even in case of using dynamic_debug because depending what files are enabled (hci_core.c, l2cap_core.c, etc) that logs way too much and it is not uncommon to lose the logs because the terminal buffer is not big enough just because the data is intermixed with some signalling, that said I think we would have to prefix data and signalling with some string and then use format option to match them. -- Luiz Augusto von Dentz
