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 11:46:31 -0700
From:   Dan Williams <dan.j.williams@intel.com>
To:     Jason Gunthorpe <jgg@nvidia.com>,
        Dan Williams <dan.j.williams@intel.com>
CC:     Jan Kara <jack@suse.cz>, Dave Chinner <david@fromorbit.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: <633739872f981_795a629425@dwillia2-xfh.jf.intel.com.notmuch>
References: <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>
 <63372dcbc7f13_739029490@dwillia2-mobl3.amr.corp.intel.com.notmuch>
 <YzcwN67+QOqXpvfg@nvidia.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <YzcwN67+QOqXpvfg@nvidia.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?GxL4siko6LY9k89ybLj7P1jbIjgLlJtlv+rO6fH9YrUd0ITXAQmAIqLoGUiI?=
 =?us-ascii?Q?2aqxZxuVRm92MsCV/tfbrZek/+8c3dKd5Pq2V/kGhTYDsSwj8SZ6JnW3wFNV?=
 =?us-ascii?Q?vgY3QmAFEAFAmrgL16YwRrIEy1cfyYrEqfMNV866vcoxGbmu2V7JK6VVZMOM?=
 =?us-ascii?Q?1C1S65sXxiBKjoSRK2x6ZGA/jY9ZXqs4bCGf8JYpUFUqG/Pp1OHPt+FUDV7S?=
 =?us-ascii?Q?XzIZ4D7sTfJA4rcCAzpdBE+SlpMnYgsktTUF+fzLemiHuPtHxsCog0MY1zJO?=
 =?us-ascii?Q?QV7j5DmXiZ0F4ZxveiUI22uDsrdAtnjlbQ5L+X46Y1fKUPFB3+KdW9LsARsU?=
 =?us-ascii?Q?STy6mecpKOPcun4f+HQNe+wJAsfL8MEc8XqadXFDmjGdggU2TDX6zOCM4XgA?=
 =?us-ascii?Q?rqQMw+cLhXebqYoLiIrHtOP3pO4vKmUYRq/9D0F9xt0Bbd4myZE6hQPucLPq?=
 =?us-ascii?Q?kHRgKQ0/0RemrsAH23CfFrqcuTyZ4XrGKDxwyCbpQ9OAwOO5rhYpVDMjYYle?=
 =?us-ascii?Q?JvdT3BBM692sHEPnEHy8UJF5RH10tgu5t0Urh7VnOlZF/1Zw+XvJwBQ2vau9?=
 =?us-ascii?Q?/xHOEd5cX0MCyZ0uMH1l3VOqU1qlREPi1zWJImncVK8I0s+HvRogHP5ccn1Y?=
 =?us-ascii?Q?2w+JceTVQeIWDhhlCOotlHhjCnPgEdirkGBco9eLW7vrM3lBFS2pynb1Mlhd?=
 =?us-ascii?Q?RWKd/ePzyLi1cdEj8lb4S2WHASIsej3kfpaTF+zcXHUHJK4ygfIHUzwoay7U?=
 =?us-ascii?Q?GhMaL1r3NKS4GfGHktCOUnnDitfQkvv55Ee7NC6LNMvAt/AJk7DqgHWNAosF?=
 =?us-ascii?Q?JcVja/Mp40KshFxdqTedHNoC+1DEq/gjGNShAKsT9+nwH8t7pi+ArjvCG9Jv?=
 =?us-ascii?Q?CkWBAw1dVsqPiQJB/cEcx5ruw2n7f/DJcv0SzhC3BUlWz8P4a2vxzP1EHb3f?=
 =?us-ascii?Q?+tezwD6yXjhJPa6zBAuT7iBgf8JBZVa08zO0KlCKBb2SvgTae22/G4Mb2k3X?=
 =?us-ascii?Q?DJQVyFGdCdfkaH6vfLEUg1h3BFH5+vG6ZBxEhwU81KyHLi/xTgCREo669BYu?=
 =?us-ascii?Q?i6O2tLEd+mUeHxGfd0qT3acaTAsXax/s4opvbt2YcxEuXQXn+OHUyKiy1gnQ?=
 =?us-ascii?Q?5AQL66X94HHS5fb0aU33alDO2NNHtY6o0jpC82Tm/ONjEWlZW2cHnxoIEhCi?=
 =?us-ascii?Q?zxNI/wWZcM1endBycjV0nPbtaAnKDJUcEIUfQdWimYxhbmN9sSvlVbftWJGz?=
 =?us-ascii?Q?czMK1cqHOPFyXK7lvtpmsg/d1sTQStw6UVwpPhFCHZl5Gd0YzLCdzF0kRcfG?=
 =?us-ascii?Q?+rHuNlWt7c6aZOXSq2Ye9+2YbGTfsaExCMV4PDZo4VBgAvXdH0FZJ09hhQkq?=
 =?us-ascii?Q?0J5LIatKYN3PKf58WoVhGkGbbuDFJdbWsmUJEH7nXAtB+szPt6mZG2Q8iCc9?=
 =?us-ascii?Q?rdhNKuNAfd4NVCRSH02LpAzZQmroXU6W2JbyI5fqTnKyVh4JjGTkeXoCE+2W?=
 =?us-ascii?Q?cAG42FJXUmWLzBQ/+glXSQejHHWPFlRi0TRooNiFRZvCp78AxwiBWFmXW6PI?=
 =?us-ascii?Q?JFDHJ64Vw/QkgfpPc6lJIaCEzz8fQxzNEy/hiJ5Mjof+vIMV7BXuAHUJRVt8?=
 =?us-ascii?Q?mQ=3D=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 85a14e92-5805-440a-3f51-08daa31418e6
X-MS-Exchange-CrossTenant-AuthSource: MWHPR1101MB2126.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 30 Sep 2022 18:46:33.9856
 (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: LdOskhCBNoU36pZVbCgSgtoaI1S0EjJ8sm9mqFkjCVbKUr5f09iuoU9FYge52Bz3NCa04b11gUFY0QUOxjPAibAK/BWlxArwoTh1e2JyBuY=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN9PR11MB5401
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-7.2 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_HI,
        RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,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

Jason Gunthorpe wrote:
> On Fri, Sep 30, 2022 at 10:56:27AM -0700, Dan Williams wrote:
> > 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.
> 
> I think the desire would be to go from the VMA to an inode_get and
> hold the inode reference for the from the pin_user_pages() to the
> unpin_user_page(), ie prevent it from being freed in the first place.
> 
> It is a fine idea, the trouble is just the high complexity to get
> there.
> 
> However, I wonder if the trucate/hole punch paths have the same
> deadlock problem?

If the deadlock is waiting for inode reclaim to complete then I can see
why the VMA pin proposal and the current truncate paths do not trigger
that deadlock because the inode is kept out of the reclaim path.

> I agree with you though, given the limited options we should convert
> the UAF into an unlikely deadlock.

I think this approach makes the implementation incrementally better, and
that the need to plumb VMA pinning can await evidence that a driver
actually does this *and* the driver can not be fixed.