From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:43592)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <paolo.bonzini@gmail.com>) id 1ZCpYc-0006Ba-J3
	for qemu-devel@nongnu.org; Wed, 08 Jul 2015 09:38:43 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <paolo.bonzini@gmail.com>) id 1ZCpYZ-0006bK-Ah
	for qemu-devel@nongnu.org; Wed, 08 Jul 2015 09:38:38 -0400
Sender: Paolo Bonzini <paolo.bonzini@gmail.com>
References: <1436359843-15612-1-git-send-email-rjones@redhat.com>
	<1436359843-15612-2-git-send-email-rjones@redhat.com>
	<20150708132642.GL4117@noname.redhat.com>
From: Paolo Bonzini <pbonzini@redhat.com>
Message-ID: <559D27D6.4090309@redhat.com>
Date: Wed, 8 Jul 2015 15:38:30 +0200
MIME-Version: 1.0
In-Reply-To: <20150708132642.GL4117@noname.redhat.com>
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: 8bit
Subject: Re: [Qemu-devel] [PATCH v3 1/2] Add a simple mechanism to protect
 against error message floods.
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Kevin Wolf <kwolf@redhat.com>, "Richard W.M. Jones" <rjones@redhat.com>
Cc: qemu-devel@nongnu.org, agraf@suse.de, qemu-block@nongnu.org, armbru@redhat.com



On 08/07/2015 15:26, Kevin Wolf wrote:
>> +/* Suggested initial value is 100 */
>> +#define FLOOD_COUNTER(counter_name, initial) \
>> +  static int counter_name = (initial)

No "static" here.

>> +#define NO_FLOOD(counter_name, code, suppress_message)         \
>> +  do {                                                         \
>> +    int t_##counter_name = atomic_fetch_add(&counter_name, 0); \
>> +    if (t_##counter_name > 0) {                                \
>> +        code;                                                  \
>> +        if (t_##counter_name == 1) {                           \
>> +            suppress_message;                                  \
>> +        }                                                      \
>> +        atomic_dec(&counter_name);                             \
>> +    }                                                          \
>> +} while (0)

What you want is a cmpxchg loop like:

   int old;
   do
       old = atomic_read(&counter_name);
   while (old > 0 && atomic_cmpxchg(&counter_name, old, old - 1) != old);
   if (old > 0) {
       code;
       if (old == 1) {
           suppress_message;
       }
   }

but I would wrap it with a simple API like

   void error_report_limited(int *counter, const char *s, ...);
   void error_report_err_limited(int *counter, Error *err);

instead of your complex NO_FLOOD macro.

Paolo