From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mike Snitzer Subject: Re: [PATCH v2] dm-io: deal with wandering queue limits when handling REQ_DISCARD and REQ_WRITE_SAME Date: Fri, 27 Feb 2015 14:19:04 -0500 Message-ID: <20150227191904.GA6945@redhat.com> References: <20150227184438.GF5519@birch.djwong.org> 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: device-mapper development , Srinivas Eeda , "Martin K. Petersen" , agk@redhat.com, "Darrick J. Wong" List-Id: dm-devel.ids On Fri, Feb 27 2015 at 2:09pm -0500, Mikulas Patocka wrote: > > > On Fri, 27 Feb 2015, Darrick J. Wong wrote: > > > Since it's apparently possible that the queue limits for discard and > > write same can change while the upper level command is being sliced > > and diced, fix up both of them (a) to reject IO if the special command > > is unsupported at the start of the function and (b) read the limits > > once and let the commands error out on their own if the status happens > > to change. > > > > Signed-off-by: Darrick J. Wong > > > + unsigned int special_cmd_max_sectors; > > + > > + /* Reject unsupported discard and write same requests */ > > + if (rw & REQ_DISCARD) > > + special_cmd_max_sectors = q->limits.max_discard_sectors; > > + else if (rw & REQ_WRITE_SAME) > > + special_cmd_max_sectors = q->limits.max_write_same_sectors; > > + if ((rw & (REQ_DISCARD | REQ_WRITE_SAME)) && > > + special_cmd_max_sectors == 0) { > > That results in uninitialized variable warning (although the warning is > false positive). We need the macro uninitialized_var to suppress the > warning. > > It's better to use ACCESS_ONCE on variables that may be changing so that > the compiler doesn't load them multiple times. > > Here I'm sending the updated patch. > > Mikulas > > > From: Mikulas Patocka ... I'm reviewing this now, but just to be clear, this patch will still be attributed to Darrick.