From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx2.suse.de ([195.135.220.15]:46475 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750732AbeBTNzh (ORCPT ); Tue, 20 Feb 2018 08:55:37 -0500 Date: Tue, 20 Feb 2018 14:55:36 +0100 From: Jan Kara To: Nikolay Borisov Cc: linux-fsdevel , Jens Axboe , Goldwyn Rodrigues Subject: Re: Correctness of inode_dio_end in generic DIO code Message-ID: <20180220135536.fr2dvcloxpvsbnzn@quack2.suse.cz> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Sender: linux-fsdevel-owner@vger.kernel.org List-ID: Hello, On Tue 20-02-18 10:59:46, Nikolay Borisov wrote: > Currently the generic DIO code calls inode_dio_begin/inode_dio_end if > DIO_SKIP_DIO_COUNT is not set. However, te generic ode doesn't really > know if there is a lock synchronizing all the various inode_dio_* > operations. As per inode_dio_wait comment : > > > Must be called under a lock that serializes taking new references to > i_dio_count, usually by inode->i_mutex. > > So is it at all correct to increment i_dio_count in generic dio code > without imposing strict locking requirement? Currently, most major > filesystems (Ext4/xfs/btrfs) do modify i_dio_count under their own > locks. Perhaps it's best if i_dio_count modification are removed from > the generic code, what do people think about that? Currently the onus is on inode_dio_wait() callers to make sure they cannot livelock (usually by calling that function in a context which blocks submission of new direct IO). So in this sense I don't see anything wrong with calling inode_dio_begin() from do_blockdev_direct_IO(). Whether calling these functions directly from fs code instead of from do_blockdev_direct_IO() to make things clearer is worth the additional code in quite a few filesystems is IMHO a matter of taste. I'm fine with the current state but then I admit I might have just got used to it :) Honza -- Jan Kara SUSE Labs, CR