From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mike Snitzer Subject: Re: [PATCH 6/7] dm: track the maximum number of bios in a cloned request Date: Thu, 12 Sep 2013 19:09:11 -0400 Message-ID: <20130912230911.GB31577@redhat.com> References: <1379024698-10487-1-git-send-email-snitzer@redhat.com> <1379024698-10487-7-git-send-email-snitzer@redhat.com> Reply-To: device-mapper development Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dm-devel-bounces@redhat.com Errors-To: dm-devel-bounces@redhat.com To: Mikulas Patocka Cc: dm-devel@redhat.com, Frank Mayhar List-Id: dm-devel.ids On Thu, Sep 12 2013 at 6:55pm -0400, Mikulas Patocka wrote: > There's a race condition (it's not serious): > > + if (num_bios > ACCESS_ONCE(peak_rq_based_ios)) > + ACCESS_ONCE(peak_rq_based_ios) = num_bios; > > You can double-check it using code like this: > if (num_bios > ACCESS_ONCE(peak_rq_based_ios)) { > spin_lock_irqsave(&peak_rq_based_ios_lock, flags); > if (num_bios > peak_rq_based_ios) > peak_rq_based_ios = num_bios; > spin_unlock_irqrestore(&peak_rq_based_ios_lock, flags); > } Yeah I thought about using double checked locking but didn't want to incur the extra locking. Figured precise answer for peak_rq_based_ios wasn't of utmost importance. But I'll think about doing it. > Maybe - reset the value peak_rq_based_ios if the user clears > track_peak_rq_based_ios? Well until patch 7 there isn't a convenient place to trap the change. And you already took issue with how I did it in that patch.