Messages in this thread | | | Date | Wed, 15 Apr 2015 11:56:53 -0700 | From | Andrew Morton <> | Subject | Re: [PATCH] direct-io: only inc/dec inode->i_dio_count for file systems |
| |
On Wed, 15 Apr 2015 12:22:56 -0600 Jens Axboe <axboe@fb.com> 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?
| |