Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1762969AbZCQJTx (ORCPT ); Tue, 17 Mar 2009 05:19:53 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1755698AbZCQJTp (ORCPT ); Tue, 17 Mar 2009 05:19:45 -0400 Received: from bombadil.infradead.org ([18.85.46.34]:44537 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755439AbZCQJTo (ORCPT ); Tue, 17 Mar 2009 05:19:44 -0400 Date: Tue, 17 Mar 2009 05:19:26 -0400 From: Christoph Hellwig To: Nikanth Karthikesan Cc: Jens Axboe , Gerd Hoffmann , Constantine Sapuntzakis , Miklos Szeredi , nikanth@gmail.com, linux-kernel@vger.kernel.org Subject: Re: [PATCH] Block - Honour barrier requests in loop driver Message-ID: <20090317091926.GA26016@infradead.org> References: <200903171417.16760.knikanth@suse.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <200903171417.16760.knikanth@suse.de> User-Agent: Mutt/1.5.18 (2008-05-17) X-SRS-Rewrite: SMTP reverse-path rewritten from by bombadil.infradead.org See http://www.infradead.org/rpr.html Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1907 Lines: 77 On Tue, Mar 17, 2009 at 02:17:16PM +0530, Nikanth Karthikesan wrote: > +static int sync_file(struct file *file) > +{ > + struct address_space *mapping; > + int ret; > + > + if (!file->f_op || !file->f_op->fsync) > + return -EOPNOTSUPP; > + > + mapping = file->f_mapping; > + > + ret = filemap_fdatawrite(mapping); > + if (!ret) { > + int ret2; > + > + mutex_lock(&mapping->host->i_mutex); > + ret = file->f_op->fsync(file, file->f_dentry, 1); > + mutex_unlock(&mapping->host->i_mutex); > + > + ret2 = filemap_fdatawait(mapping); > + if (!ret) > + ret = ret2; Please use vfs_fsync. > + int barrier = bio_barrier(bio); > + > + if (barrier) { > + ret = sync_file(lo->lo_backing_file); > + if (unlikely(ret)) > + goto out; > + } > > pos = ((loff_t) bio->bi_sector << 9) + lo->lo_offset; > + if (bio_rw(bio) == WRITE) { > ret = lo_send(lo, bio, pos); > + if (barrier && !ret) > + ret = sync_file(lo->lo_backing_file); > + } else > ret = lo_receive(lo, bio, lo->lo_blocksize, pos); > + > +out: > return ret; We only use barrier requests for reads, which this code relies on for the second sync, too. So just move the whole thing into one if block, ala: pos = ((loff_t) bio->bi_sector << 9) + lo->lo_offset; if (bio_rw(bio) == WRITE) { int barrier = bio_barrier(bio); if (barrier) { ret = sync_file(lo->lo_backing_file); if (unlikely(ret)) goto out; } ret = lo_send(lo, bio, pos); if (ret) goto out; if (barrier) ret = sync_file(lo->lo_backing_file); } else ret = lo_receive(lo, bio, lo->lo_blocksize, pos); out: return ret; You also should advertise the barrier capability with a queue flag. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/