Received-SPF: pass (google.com: domain of linux-ext4-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 30 Sep 2022 10:56:27 -0700
From:   Dan Williams <dan.j.williams@intel.com>
To:     Jan Kara <jack@suse.cz>, Dan Williams <dan.j.williams@intel.com>
CC:     Jan Kara <jack@suse.cz>, Dave Chinner <david@fromorbit.com>,
        "Jason Gunthorpe" <jgg@nvidia.com>, <akpm@linux-foundation.org>,
        Matthew Wilcox <willy@infradead.org>,
        "Darrick J. Wong" <djwong@kernel.org>,
        "Christoph Hellwig" <hch@lst.de>,
        John Hubbard <jhubbard@nvidia.com>,
        <linux-fsdevel@vger.kernel.org>, <nvdimm@lists.linux.dev>,
        <linux-xfs@vger.kernel.org>, <linux-mm@kvack.org>,
        <linux-ext4@vger.kernel.org>
Subject: Re: [PATCH v2 05/18] xfs: Add xfs_break_layouts() to the inode
 eviction path
Message-ID: <63372dcbc7f13_739029490@dwillia2-mobl3.amr.corp.intel.com.notmuch>
References: <20220921221416.GT3600936@dread.disaster.area>
 <YyuQI08LManypG6u@nvidia.com>
 <20220923001846.GX3600936@dread.disaster.area>
 <632d00a491d0d_4a67429488@dwillia2-xfh.jf.intel.com.notmuch>
 <20220923021012.GZ3600936@dread.disaster.area>
 <20220923093803.nroajmvn7twuptez@quack3>
 <20220925235407.GA3600936@dread.disaster.area>
 <20220926141055.sdlm3hkfepa7azf2@quack3>
 <63362b4781294_795a6294f0@dwillia2-xfh.jf.intel.com.notmuch>
 <20220930134144.pd67rbgahzcb62mf@quack3>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20220930134144.pd67rbgahzcb62mf@quack3>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?xr11XnjvMxHGkDhJaVm5EeXNfTktrTZowZ1sediVkaK1BUEzOhuecHZaJ7T0?=
 =?us-ascii?Q?Ss34MZ79w3gZD0KDartx6+92l9BC12zzNEqcOIHLAlmyN1V9v6nBE1UXUKGb?=
 =?us-ascii?Q?5h0xyXcfvTeYwXWAIoupKkOcO70wML0Kj3cAb0OWtKTaRAwH4R+87AzVGMGc?=
 =?us-ascii?Q?U1kU4p8V1+yp+TLzN19BFyb5zmXc7+B70VeURpNzzYSRcbhvGNO/aCjQQnFn?=
 =?us-ascii?Q?YHoW3MGZApRWq1FDXVTQ0P92NkiQcHOjaFyrux0xg0pgv7pOxZykS3i6fEBN?=
 =?us-ascii?Q?jUpKXoQofgyOXLpTgSC5exp2tZ9GIGalkdtmaR1fJQ0W7rJEqYw+gomLb81q?=
 =?us-ascii?Q?Bzzt/MqchGGo7beV0Jrj420yF8GNB4gMCnSRWovavO/fbCZKzBTYJPxw0StT?=
 =?us-ascii?Q?vXrot/jM4liHydEArT24FCs46lGd3J66trS5TT7XRJPsmOoymayYwlZwn8Lh?=
 =?us-ascii?Q?NcXYc/jAnWAGk46H5J6pL6IrQAUYQTs0qu7j60WGPPrempuyKVf5T6jnW6t9?=
 =?us-ascii?Q?spW6J/w0n517MbD2+bvDPTbLYfTkTMR9c9RbxJRo3ITMSl8s5FQs6EvYRnzP?=
 =?us-ascii?Q?isBT1p5Lg96HyAW/Ekpdv3g57VijR1Aw5FAYkJ9AppnGeZWIpBOljNgX+W+O?=
 =?us-ascii?Q?bRUZ6Qpm8sbR58eY6MUwL2GnUrhbvmiaYnTHqiF2oOwjkoPwejCY/K4uk8TB?=
 =?us-ascii?Q?B5bMiZFMao+dNut2OF5GfAsab10YBj20avW6sw3Mqdx3cdZayLFTGJ9kYOvx?=
 =?us-ascii?Q?DSl9F/g/UYQBvAoSzZsIXnQIzQodemSW9zhzgP3lnJUDWH/0W+Z5EmKLlYWN?=
 =?us-ascii?Q?bzTEIHdxS8e98ZGM09vflPz1jJRiu3u0Vy4dYbmflrhM5hPnY710sM7a/gXq?=
 =?us-ascii?Q?jBn4iX0GX4CtVJdcQ5GD6wK9cWUOtJczOch1cnzoD8d8+BSD71K1s9jO8cWV?=
 =?us-ascii?Q?xGk6wy+ErhxTPrtoKD7YXXO/CFilwpAtxe7R1jnOVpUvsSmZX0tOeUHE632y?=
 =?us-ascii?Q?KgpvQUB++1nUkIfOGM0QZVWYhhQwQ3FG4Ar5HCMjzdi0tYtfyizW3ifCb+oa?=
 =?us-ascii?Q?D2Vxd/e61HreH4xZWQgkCDACFTjEnI0o619RFxFYRolPlt6hurSDSWvj8dD8?=
 =?us-ascii?Q?kAraDtiiaZC+RJTL4TICLbRyJ77g4GFoOtN4MpnpF83LZcCufs+zMyjoZI+5?=
 =?us-ascii?Q?Hskz7QnvQwR2YMQkI09qO235OWc43SbxqeWnEQVsQkxRWzbC7hiBMzJ03iIR?=
 =?us-ascii?Q?l/U5L8kBxxLieLK/tza1CbkLjfJSaVSvfJQlAFjSMUNQ0wur0Pv3wxuuhwg8?=
 =?us-ascii?Q?eXnYClD7ubhoK8k7GkbEu0zFZY+05/M+9Lgy1+B2d1MSWW4auHIic3xVFK02?=
 =?us-ascii?Q?ZqbMEBA6uYyxd+eTXuWIKuxl4+OoFL2Hs1YiZz5mI8G3/fEEXuf2MI5i4wCt?=
 =?us-ascii?Q?bxTkafJteYkfc+tfYWUx1iYCAnU77pXR8l4RuLhJBHhxLadMUEWCyIXrfASD?=
 =?us-ascii?Q?bfGfAf7gPwY2Yrw7HLhGX1A2tkrNRjVo+P5Nfoccw43XIylKd3t88l45CN4v?=
 =?us-ascii?Q?MEsqM5nJF0wzF56rKHTGLiWLiqoO/SHTNEHL6zP8Rqou3jqx3lhot25tKn9B?=
 =?us-ascii?Q?lg=3D=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: ee152f5c-2feb-4068-11f5-08daa30d1b06
X-MS-Exchange-CrossTenant-AuthSource: MWHPR1101MB2126.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 30 Sep 2022 17:56:31.1662
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: I6E7GUcrZ9kqA3xabFV3wKdUMMEQKU4myyZGYiQ+XYqOJqmZXFBlNuylHcrzyejIFV4c9fyzLw1jBt4cxJVxEjL0CypUgEh8xiyYGN6QvaA=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW5PR11MB5809
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.5 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,
        SPF_HELO_NONE,SPF_NONE autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-ext4.vger.kernel.org>
X-Mailing-List: linux-ext4@vger.kernel.org

Jan Kara wrote:
[..]
> I agree this is doable but there's the nasty sideeffect that inode reclaim
> may block for abitrary time waiting for page pinning. If the application
> that has pinned the page requires __GFP_FS memory allocation to get to a
> point where it releases the page, we even have a deadlock possibility.
> So it's better than the UAF issue but still not ideal.

I expect VMA pinning would have similar deadlock exposure if pinning a
VMA keeps the inode allocated. Anything that puts a page-pin release
dependency in the inode freeing path can potentially deadlock a reclaim
event that depends on that inode being freed.

As you say the UAF is worse. I am not too worried about the deadlock
case for a couple reasons:

1/ There are no reports I can find of iput_final() triggering the WARN
that validates that truncate_inode_pages_final() is called while all
associated pages are unpinned. That WARN has been in place since 2017:

d2c997c0f145 fs, dax: use page->mapping to warn if truncate collides with a busy page

2/ It is bad form for I/O drivers to perform __GFP_FS and __GFP_IO
allocations in their fast paths. So while the deadlock is not impossible
it is unlikely with the major producers of transient page pin events.

My hope, famous last words, is that this is only a theoretical deadlock,
or we can handle this with targeted driver fixes. Any driver that thinks
it wants to pin pages and then do more allocations that recurse into the
FS likely wants to get that out of its fast path anyway. I will also
take a look at a lockdep annotation for the wait event to see if that
can give an early warning versus fs_reclaim_acquire().