From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Morton Subject: Re: [PATCH] direct-io: only inc/dec inode->i_dio_count for file systems Date: Wed, 15 Apr 2015 11:56:53 -0700 Message-ID: <20150415115653.f69d970e4b155deea98a0829@linux-foundation.org> References: <20150415182256.GA30339@kernel.dk> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Cc: , , , To: Jens Axboe Return-path: Received: from mail.linuxfoundation.org ([140.211.169.12]:47710 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755050AbbDOS4y (ORCPT ); Wed, 15 Apr 2015 14:56:54 -0400 In-Reply-To: <20150415182256.GA30339@kernel.dk> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Wed, 15 Apr 2015 12:22:56 -0600 Jens Axboe wrote: > Hi, > > This is a reposting of a patch that was originally in the blk-mq series. > It has huge upside on shared access to a multiqueue device doing > O_DIRECT, it's basically the scaling block that ends up killing > performance. A quick test here reveals that we spend 30% of all system > time just incrementing and decremening inode->i_dio_count. For block > devices this isn't useful at all, as we don't need protection against > truncate. For that test case, performance increases about 3.6x (!!) by > getting rid of this inc/dec per IO. > > I've cleaned it up a bit since last time, integrating the checks in > inode_dio_done() and adding a inode_dio_begin() so that callers don't > need to know about this. > > We've been running a variant of this patch in the FB kernel for a while. > I'd like to finally get this upstream. 30% overhead for one atomic_inc+atomic_dec+wake_up_bit() per IO? That seems very high! Is there something else going on? Is there similar impact to direct-io-to-file? It would be nice to fix that up also. Many filesystems do something along the lines of atomic_inc(i_dio_count); wibble() atomic_dev(i_dio_count); __blockdev_direct_IO(...); and with your patch I think we could change them to atomic_inc(i_dio_count); wibble() __blockdev_direct_IO(..., flags|DIO_IGNORE_TRUNCATE); atomic_dev(i_dio_count); which would halve the atomic op load. But that's piling hack on top of hack. Can we change the do_blockdev_direct_IO() interface to "caller shall hold i_mutex, or increment i_dio_count"? ie: exclusion against truncate is wholly the caller's responsibility. That way, this awkward sharing of responsibility between caller and callee gets cleaned up and DIO_IGNORE_TRUNCATE goes away. inode_dio_begin() would be a good place to assert that i_mutex is held, btw. This whole i_dio_count thing is pretty nasty, really. If you stand back and squint, it's basically an rwsem. I wonder if we can use an rwsem... What's the reason for DIO_IGNORE_TRUNCATE rather than boring old !S_ISBLK?