Re: [PATCH 12/13] drivers/staging/unisys/visorhba: convert stats to use seqnum_ops

From: Shuah Khan
Date: Tue Nov 10 2020 - 16:02:18 EST


On 11/10/20 1:42 PM, Greg KH wrote:
On Tue, Nov 10, 2020 at 12:53:38PM -0700, Shuah Khan wrote:
seqnum_ops api is introduced to be used when a variable is used as
a sequence/stat counter and doesn't guard object lifetimes. This
clearly differentiates atomic_t usages that guard object lifetimes.

seqnum32 variables wrap around to INT_MIN when it overflows and
should not be used to guard resource lifetimes, device usage and
open counts that control state changes, and pm states.

atomic_t variables used for error_count and ios_threshold are atomic
counters and guarded by max. values. No change to the behavior with
this change.

Signed-off-by: Shuah Khan <skhan@xxxxxxxxxxxxxxxxxxx>
---
.../staging/unisys/visorhba/visorhba_main.c | 37 ++++++++++---------
1 file changed, 19 insertions(+), 18 deletions(-)

diff --git a/drivers/staging/unisys/visorhba/visorhba_main.c b/drivers/staging/unisys/visorhba/visorhba_main.c
index 7ae5306b92fe..3209958b8aaa 100644
--- a/drivers/staging/unisys/visorhba/visorhba_main.c
+++ b/drivers/staging/unisys/visorhba/visorhba_main.c
@@ -10,6 +10,7 @@
#include <linux/module.h>
#include <linux/seq_file.h>
#include <linux/visorbus.h>
+#include <linux/seqnum_ops.h>
#include <scsi/scsi.h>
#include <scsi/scsi_host.h>
#include <scsi/scsi_cmnd.h>
@@ -41,8 +42,8 @@ MODULE_ALIAS("visorbus:" VISOR_VHBA_CHANNEL_GUID_STR);
struct visordisk_info {
struct scsi_device *sdev;
u32 valid;
- atomic_t ios_threshold;
- atomic_t error_count;
+ struct seqnum32 ios_threshold;
+ struct seqnum32 error_count;

Are you sure the threshold variable is a sequence number >
It goes up and down, not just up and up and up.

Right. I does go down. Turns out this is the only place seqnum32_dec()
is used. :)

I will fix. I noticed you made a comment about _dec() interfaces on
1/13. I can drop those as well. This way seqnum_ops can be just used
for up counters.

thanks,
-- Shuah