From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jens Axboe Subject: Re: [PATCH 0/7] Discard requests, v2 Date: Thu, 14 Aug 2008 09:49:01 +0200 Message-ID: <20080814074901.GG20055@kernel.dk> References: <20080812105428.GG20055@kernel.dk> <1218539797.2977.147.camel@pmac.infradead.org> <1218543544.2977.158.camel@pmac.infradead.org> <20080812125315.GH20055@kernel.dk> <1218546257.2977.167.camel@pmac.infradead.org> <1218556054.2977.188.camel@pmac.infradead.org> <20080813111547.GZ20055@kernel.dk> <1218626628.2977.266.camel@pmac.infradead.org> <20080813113220.GC20055@kernel.dk> <1218627286.2977.269.camel@pmac.infradead.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Andrew Morton , Ric Wheeler , linux-fsdevel@vger.kernel.org, gilad@codefidence.com, matthew@wil.cx To: David Woodhouse Return-path: Received: from brick.kernel.dk ([87.55.233.238]:5307 "EHLO kernel.dk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752440AbYHNHtW (ORCPT ); Thu, 14 Aug 2008 03:49:22 -0400 Content-Disposition: inline In-Reply-To: <1218627286.2977.269.camel@pmac.infradead.org> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Wed, Aug 13 2008, David Woodhouse wrote: > On Wed, 2008-08-13 at 13:32 +0200, Jens Axboe wrote: > > On Wed, Aug 13 2008, David Woodhouse wrote: > > > On Wed, 2008-08-13 at 13:15 +0200, Jens Axboe wrote: > > > > On Tue, Aug 12 2008, David Woodhouse wrote: > > > > > On Tue, 2008-08-12 at 14:04 +0100, David Woodhouse wrote: > > > > > > > Or just match the check before -EOPNOTSUPP with bio_has_data(), > > > > > > > since it only applies to a barrier that carries data. > > > > > > > > > > > > Like this, you mean? Empty barriers don't get to there? > > > > > > > > > > Seems to work, although I'm somewhat dubious about it. Still, if you're > > > > > happy that it's correct and you really prefer it that way, then I can > > > > > commit it. > > > > > > > > Looks ok to me. > > > > > > OK, that version is now in the git tree. > > > > Alright, I'll pull it it. > > > > > > > Anything else I need to address before you pull the tree? Any comment on > > > > > the BLKDISCARD ioctl? I've left that one using the non-barrier version > > > > > since it's waiting for it anyway, and shouldn't be happening > > > > > concurrently with anything else. > > > > > > > > > > (http://,git://} git.infradead.org/users/dwmw2/discard-2.6.git > > > > > > > > I'm with Jamie on using the safer version for the ioctl, unless you > > > > ensure that the block device isn't mounted before allowing it. > > > > > > We don't ensure that the block device isn't mounted before we allow > > > reads/writes -- why should we do so before we allow discard? > > > > > > If the userspace tool 'owns' the block device, that's a different story > > > -- and that's OK too, because the BLKDISCARD ioctl is synchronous. It > > > won't return until it's actually _complete_, and userspace really > > > shouldn't be trying to write to the same sectors until that happens. > > > > Still seems a little unsafe. I guess you could make a case for making > > the ioctl privileged. We should at least ensure that the user has write > > access to the device before allowing a discard operation. > > Er, yes. That would be a good idea, wouldn't it? :) I'd think so :-) > Hold off on pulling the tree for a moment; I'll do that. I've pulled it this morning. -- Jens Axboe