Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1753281AbdLEPuJ (ORCPT <rfc822;w@1wt.eu>);
        Tue, 5 Dec 2017 10:50:09 -0500
Received: from imap1.codethink.co.uk ([176.9.8.82]:44739 "EHLO
        imap1.codethink.co.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1752500AbdLEPuF (ORCPT
        <rfc822;linux-kernel@vger.kernel.org>);
        Tue, 5 Dec 2017 10:50:05 -0500
Message-ID: <1512488994.18523.173.camel@codethink.co.uk>
Subject: Re: [PATCH 4.4 13/16] ocfs2: should wait dio before inode lock in
 ocfs2_setattr()
From: Ben Hutchings <ben.hutchings@codethink.co.uk>
To: Alex Chen <alex.chen@huawei.com>, Jun Piao <piaojun@huawei.com>,
        Joseph Qi <jiangqi903@gmail.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: stable@vger.kernel.org, Changwei Ge <ge.changwei@h3c.com>,
        Mark Fasheh <mfasheh@versity.com>, Joel Becker <jlbec@evilplan.org>,
        Junxiao Bi <junxiao.bi@oracle.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        Linus Torvalds <torvalds@linux-foundation.org>,
        LKML <linux-kernel@vger.kernel.org>
Date: Tue, 05 Dec 2017 15:49:54 +0000
In-Reply-To: <20171122101111.411869812@linuxfoundation.org>
References: <20171122101110.784746358@linuxfoundation.org>
         <20171122101111.411869812@linuxfoundation.org>
Organization: Codethink Ltd.
Content-Type: text/plain; charset="UTF-8"
X-Mailer: Evolution 3.22.6-1+deb9u1 
Mime-Version: 1.0
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Length: 1209
Lines: 39

On Wed, 2017-11-22 at 11:12 +0100, Greg Kroah-Hartman wrote:
> 4.4-stable review patch.  If anyone has any objections, please let me know.
> 
> ------------------
> 
> From: alex chen <alex.chen@huawei.com>
> 
> commit 28f5a8a7c033cbf3e32277f4cc9c6afd74f05300 upstream.
> 
> we should wait dio requests to finish before inode lock in
> ocfs2_setattr(), otherwise the following deadlock will happen:
[...]

I looked at the kernel-doc for inode_dio_wait():

/**
 * inode_dio_wait - wait for outstanding DIO requests to finish
 * @inode: inode to wait for
 *
 * Waits for all pending direct I/O requests to finish so that we can
 * proceed with a truncate or equivalent operation.
 *
 * Must be called under a lock that serializes taking new references
 * to i_dio_count, usually by inode->i_mutex.
 */

Now that ocfs2_setattr() calls this outside of the inode locked region,
what prevents another task adding a new dio request immediately
afterward?

Also, ocfs2_dio_end_io_write() was introduced in 4.6 and it looks like
the dio completion path didn't previously take the inode lock.  So it
doesn't look this fix is needed in 3.18 or 4.4.

Ben.

-- 
Ben Hutchings
Software Developer, Codethink Ltd.