On 31/05/17 06:06, Joe Perches wrote:
> On Wed, 2017-05-31 at 12:28 +0900, Hirokazu Honda wrote:
>> If I understand a bitmap correctly, it is necessary to change the log level
>> for each message.
>> I didn't mean a bitmap will take a long CPU time.
>> I mean the work to change so takes a long time.
>
> No, none of the messages or levels need change,
> only the >= test changes to & so that for instance,
> level 1 and level 3 messages could be emitted
> without also emitting level 2 messages.
>
> The patch suggested is all that would be required.
>
I prefer the solution that Joe proposed as well.
It's more useful, esp. with a complex beast like vb2.
Regards,
Hans
Hi,
I completely understand bitmask method now.
I agree to the idea, but it is necessary to change the specification of
a debug parameter.
(We probably need to change a document about that?)
For example, there is maybe a user who set a debug parameter 3.
The user assume that logs whose levels are less than 4 are shown.
However, after the bitmask method is adopted, someday the logs whose
level is 1 or 2 are only shown, not 3 level logs are not shown.
This will be confusing to users.
The function that users can select a log method is necessary (e.g.
implement dprintk_bitmask and dprintk_level)
The main purpose of my patch is to not output much log messages.
I think the current patch is enough to accomplish the purpose.
Changing the method is a related but different task from my patch.
Therefore, I think it should be done in another patch.
Best,
Hirokazu Honda
On Wed, Jun 7, 2017 at 6:01 PM, Hans Verkuil <[email protected]> wrote:
> On 31/05/17 06:06, Joe Perches wrote:
>> On Wed, 2017-05-31 at 12:28 +0900, Hirokazu Honda wrote:
>>> If I understand a bitmap correctly, it is necessary to change the log level
>>> for each message.
>>> I didn't mean a bitmap will take a long CPU time.
>>> I mean the work to change so takes a long time.
>>
>> No, none of the messages or levels need change,
>> only the >= test changes to & so that for instance,
>> level 1 and level 3 messages could be emitted
>> without also emitting level 2 messages.
>>
>> The patch suggested is all that would be required.
>>
>
> I prefer the solution that Joe proposed as well.
>
> It's more useful, esp. with a complex beast like vb2.
>
> Regards,
>
> Hans
On Thu, Jun 8, 2017 at 12:24 PM, Hirokazu Honda <[email protected]> wrote:
> Hi,
>
> I completely understand bitmask method now.
> I agree to the idea, but it is necessary to change the specification of
> a debug parameter.
> (We probably need to change a document about that?)
> For example, there is maybe a user who set a debug parameter 3.
> The user assume that logs whose levels are less than 4 are shown.
> However, after the bitmask method is adopted, someday the logs whose
> level is 1 or 2 are only shown, not 3 level logs are not shown.
> This will be confusing to users.
I think I have to agree with Hirokazu here. Even though it's only
about debugging, there might be some automatic testing systems that
actually rely on certain values here. It probably shouldn't be
considered hard ABI, but that still could be a significant annoyance
for everyone.
However, one could add this in an incremental way, i.e. add a new
debug_mask parameter that would be used by dprinkt(), while making the
original debug parameter simply update the debug_mask whenever it's
changed.
I still think that it should be made with a separate patch, though, as
adjusting the levels and changing the filtering method are orthogonal.
Best regards,
Tomasz
On Thu, 2017-06-08 at 13:39 +0900, Tomasz Figa wrote:
> On Thu, Jun 8, 2017 at 12:24 PM, Hirokazu Honda <[email protected]> wrote:
> > Hi,
> >
> > I completely understand bitmask method now.
> > I agree to the idea, but it is necessary to change the specification of
> > a debug parameter.
> > (We probably need to change a document about that?)
> > For example, there is maybe a user who set a debug parameter 3.
> > The user assume that logs whose levels are less than 4 are shown.
> > However, after the bitmask method is adopted, someday the logs whose
> > level is 1 or 2 are only shown, not 3 level logs are not shown.
> > This will be confusing to users.
>
> I think I have to agree with Hirokazu here. Even though it's only
> about debugging, there might be some automatic testing systems that
> actually rely on certain values here.
I think it's a non-argument.
If there automated systems that rely on specific levels, then
changing the levels of individual messages could also cause
those automated systems to fail.
On Thu, Jun 8, 2017 at 2:16 PM, Joe Perches <[email protected]> wrote:
> On Thu, 2017-06-08 at 13:39 +0900, Tomasz Figa wrote:
>> On Thu, Jun 8, 2017 at 12:24 PM, Hirokazu Honda <[email protected]> wrote:
>> > Hi,
>> >
>> > I completely understand bitmask method now.
>> > I agree to the idea, but it is necessary to change the specification of
>> > a debug parameter.
>> > (We probably need to change a document about that?)
>> > For example, there is maybe a user who set a debug parameter 3.
>> > The user assume that logs whose levels are less than 4 are shown.
>> > However, after the bitmask method is adopted, someday the logs whose
>> > level is 1 or 2 are only shown, not 3 level logs are not shown.
>> > This will be confusing to users.
>>
>> I think I have to agree with Hirokazu here. Even though it's only
>> about debugging, there might be some automatic testing systems that
>> actually rely on certain values here.
>
> I think it's a non-argument.
>
> If there automated systems that rely on specific levels, then
> changing the levels of individual messages could also cause
> those automated systems to fail.
Well, that might be true for some of them indeed. I was thinking about
our use case, which relies on particular numbers to get expected
verbosity levels not caring about particular messages. I guess the
break all or none rule is going to apply here, so we should do the
bitmap conversion indeed. :)
On the other hand, I think it would be still preferable to do the
conversion in a separate patch.
Best regards,
Tomasz
On Thu, 2017-06-08 at 14:24 +0900, Tomasz Figa wrote:
> On Thu, Jun 8, 2017 at 2:16 PM, Joe Perches <[email protected]> wrote:
[]
> > If there automated systems that rely on specific levels, then
> > changing the levels of individual messages could also cause
> > those automated systems to fail.
>
> Well, that might be true for some of them indeed. I was thinking about
> our use case, which relies on particular numbers to get expected
> verbosity levels not caring about particular messages. I guess the
> break all or none rule is going to apply here, so we should do the
> bitmap conversion indeed. :)
>
> On the other hand, I think it would be still preferable to do the
> conversion in a separate patch.
Right. No worries.
Hi,
According to patch work, this patch are not approved yet and its
status are "Changes Requested."
What changes are necessary actually?
If there is no necessary change, can you approve this patch?
Best,
Hirokazu Honda
On Thu, Jun 8, 2017 at 2:33 PM, Joe Perches <[email protected]> wrote:
> On Thu, 2017-06-08 at 14:24 +0900, Tomasz Figa wrote:
>> On Thu, Jun 8, 2017 at 2:16 PM, Joe Perches <[email protected]> wrote:
> []
>> > If there automated systems that rely on specific levels, then
>> > changing the levels of individual messages could also cause
>> > those automated systems to fail.
>>
>> Well, that might be true for some of them indeed. I was thinking about
>> our use case, which relies on particular numbers to get expected
>> verbosity levels not caring about particular messages. I guess the
>> break all or none rule is going to apply here, so we should do the
>> bitmap conversion indeed. :)
>>
>> On the other hand, I think it would be still preferable to do the
>> conversion in a separate patch.
>
> Right. No worries.
>
On 06/28/2017 12:57 AM, Hirokazu Honda wrote:
> Hi,
>
> According to patch work, this patch are not approved yet and its
> status are "Changes Requested."
> What changes are necessary actually?
> If there is no necessary change, can you approve this patch?
I was considering to have more fine grained control by changing
the debug parameter to a bitmask. But after thinking about it a
bit more I decided that this patch is OK after all.
I'll pick it up the next time I prepare a pull request.
Regards,
Hans
>
> Best,
> Hirokazu Honda
>
> On Thu, Jun 8, 2017 at 2:33 PM, Joe Perches <[email protected]> wrote:
>> On Thu, 2017-06-08 at 14:24 +0900, Tomasz Figa wrote:
>>> On Thu, Jun 8, 2017 at 2:16 PM, Joe Perches <[email protected]> wrote:
>> []
>>>> If there automated systems that rely on specific levels, then
>>>> changing the levels of individual messages could also cause
>>>> those automated systems to fail.
>>>
>>> Well, that might be true for some of them indeed. I was thinking about
>>> our use case, which relies on particular numbers to get expected
>>> verbosity levels not caring about particular messages. I guess the
>>> break all or none rule is going to apply here, so we should do the
>>> bitmap conversion indeed. :)
>>>
>>> On the other hand, I think it would be still preferable to do the
>>> conversion in a separate patch.
>>
>> Right. No worries.
>>