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:   Wed, 21 Sep 2022 14:38:56 -0700
From:   Dan Williams <dan.j.williams@intel.com>
To:     Dan Williams <dan.j.williams@intel.com>,
        Jason Gunthorpe <jgg@nvidia.com>
CC:     <akpm@linux-foundation.org>, Matthew Wilcox <willy@infradead.org>,
        "Jan Kara" <jack@suse.cz>, "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 10/18] fsdax: Manage pgmap references at entry
 insertion and deletion
Message-ID: <632b8470d34a6_34962946d@dwillia2-xfh.jf.intel.com.notmuch>
References: <166329930818.2786261.6086109734008025807.stgit@dwillia2-xfh.jf.intel.com>
 <166329936739.2786261.14035402420254589047.stgit@dwillia2-xfh.jf.intel.com>
 <YysZrdF/BSQhjWZs@nvidia.com>
 <632b2b4edd803_66d1a2941a@dwillia2-xfh.jf.intel.com.notmuch>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <632b2b4edd803_66d1a2941a@dwillia2-xfh.jf.intel.com.notmuch>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?bitLZW/9fII9MZrM46gzpU+odIMqR9zkie7EWHIqiK79919pvf5Iraht635d?=
 =?us-ascii?Q?UZ+U+Fy0dV8JtzD1U1cp9WXh24+UleBZ9ABWy/rrtf4HqXrLZLeFCAgNAbGB?=
 =?us-ascii?Q?Z5SJ/gTBn5KZgN+OuLgLUVbrJyJ0IqpWKigIr1y4wx3SD0I0ljrCc9oJCmeq?=
 =?us-ascii?Q?RW15D3SGjkY0q8nxLofHgpcrUlVRPw2YW7jgw9Ht0mMMIdNIKcI4Kni/b1jK?=
 =?us-ascii?Q?EkH6vqaf74JDvWr9qZWoTZJLejq07IoNXEi1iyWot/MeigYfx0ryYT8UWLBs?=
 =?us-ascii?Q?2u5nY33VIDMO2tHt10WGK5z3BfOupjbRt38RT93NJhHQ/xu4ZC0Aw6TriAc6?=
 =?us-ascii?Q?YXzzSZDXD2Cx5zfqcQWnzDaN578FhavzwOxhAp1N6g8SNbyopJ1+0UqAfVhF?=
 =?us-ascii?Q?Gf//ODN8ovuRhqzuY4nOTH5K4S19OwEbnUUfxe+YxTo8oX9hH+c/Ndo30YbL?=
 =?us-ascii?Q?G8pHayfC1Z/jmUbG2AgJ37Dnaucb+s6taFy859ndpqYAtCHcKp+PhwZOX8a1?=
 =?us-ascii?Q?9uWdJZM/ddbk6WaDpjMY43ESuBHeQEP5bXiA6ghjYMZyh1ZBxk+N+KYpu0wF?=
 =?us-ascii?Q?cggWzu7BxyWXRAqr88wqpKvZOfqPcR2o2K6ufDRa5+3BB2TF7YeOxz0tdFxx?=
 =?us-ascii?Q?rMA06O+Aw5fAG7fRv/vVEK0D6E2gLE7lAK4VsnYiRdNo4g/EjyftCXzFCLtN?=
 =?us-ascii?Q?qfO9zuprfiGVLj2G7iXzqO7aCmZQacvgyjN8KR7X5wPX8vMeJAy0TGn2wIyk?=
 =?us-ascii?Q?h1ko0qHa2S3gCdRl1p7jvdibrvrcsNr3MBDUdmIfQNPfpOIpM/mnPSty3W27?=
 =?us-ascii?Q?k2K1GBshTNhqx4XlDT5oeIBvYvF2se1IonSMZ3q1Kfxa/kfCqKhGD5GEtUDb?=
 =?us-ascii?Q?QZqcu/bTkPYppjKgJXIKWMDSboAHrhU5REWtJ9XROS63GeQyjtJ317/a9ihy?=
 =?us-ascii?Q?uFt7NSWMqQ8k51Z1ejMttJ5wtRxlWS6GS4x53jOqQJFpwPAWK2t6O676EEfi?=
 =?us-ascii?Q?38W52APzXoObCSJN6+I0iLgsIHYdZkK896RSCrCudHuNpklH7C9CsjlFQj3r?=
 =?us-ascii?Q?I8Ubox1V3R0G32DxAwoojyoTnFA1RqTqjF/XF2MTZCayOBoL6aMZbaWbjqVj?=
 =?us-ascii?Q?3fwkS/mbp5QYeoC2RB9NwdBwJhVELMvmcnpOtPAXZSSBY0l/preg5Q/7qcyH?=
 =?us-ascii?Q?+kxhbmCgBzssR/T8o21HAPcgc5+g/qfauK7JygYpZBGAf3OgFbbikY9Q5VMb?=
 =?us-ascii?Q?pEMXQAHz+/dLMxek0Y/q2dU/sC/ek2h8ERGCJTxiaT1sQAn/BG7cucdiSnJn?=
 =?us-ascii?Q?LCJIAXqOalgklKp3/RyuHgri+SgUa2oe+HtvKXMdWvqTHx3T8YEPgJ7mSJ43?=
 =?us-ascii?Q?aBfvpH9h7lvx786wPhNTdaddIFzw7rtNWYucuFwImxgUfrXi0pbxQfKiIIli?=
 =?us-ascii?Q?uQOzlX6Ad7Av1ipNy/Pjv3aiNxak1jEOuF6lAHRKbtO7QaNBU4++SkTkPXUW?=
 =?us-ascii?Q?NTjRLZFBrnowbAVpSvkQFfhskaqxA9X+b9aAUegJHVtKidaakkYgmDXfkSys?=
 =?us-ascii?Q?oz8UtyArLmiFRfNzlFji4OchbK7tB/kFEhMHTMP+P0o3m9bEg7vgCs45E7GI?=
 =?us-ascii?Q?AA=3D=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 4ebfdb64-0743-4230-a81e-08da9c19b198
X-MS-Exchange-CrossTenant-AuthSource: MWHPR1101MB2126.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 21 Sep 2022 21:38:59.5127
 (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: g5mjBbbLw0wIP+bHr2Jm5Z64K+Mg6i37kKh/rbW3fSWLO/0+cPtpidxluu0tyhacqWgWknKNVHB8dJmyOMrM+VBAEqvP5LlovlKazupGSrc=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW3PR11MB4620
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.4 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_PASS,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

Dan Williams wrote:
> Jason Gunthorpe wrote:
> > On Thu, Sep 15, 2022 at 08:36:07PM -0700, Dan Williams wrote:
> > > The percpu_ref in 'struct dev_pagemap' is used to coordinate active
> > > mappings of device-memory with the device-removal / unbind path. It
> > > enables the semantic that initiating device-removal (or
> > > device-driver-unbind) blocks new mapping and DMA attempts, and waits for
> > > mapping revocation or inflight DMA to complete.
> > 
> > This seems strange to me
> > 
> > The pagemap should be ref'd as long as the filesystem is mounted over
> > the dax. The ref should be incrd when the filesystem is mounted and
> > decrd when it is unmounted.
> > 
> > When the filesystem unmounts it should zap all the mappings (actually
> > I don't think you can even unmount a filesystem while mappings are
> > open) and wait for all page references to go to zero, then put the
> > final pagemap back.
> > 
> > The rule is nothing can touch page->pgmap while page->refcount == 0,
> > and if page->refcount != 0 then page->pgmap must be valid, without any
> > refcounting on the page map itself.
> > 
> > So, why do we need pgmap refcounting all over the place? It seems like
> > it only existed before because of the abuse of the page->refcount?
> 
> Recall that this percpu_ref is mirroring the same function as
> blk_queue_enter() whereby every new request is checking to make sure the
> device is still alive, or whether it has started exiting.
> 
> So pgmap 'live' reference taking in fs/dax.c allows the core to start
> failing fault requests once device teardown has started. It is a 'block
> new, and drain old' semantic.

However this line of questioning has me realizing that I have the
put_dev_pagemap() in the wrong place. It needs to go in
free_zone_device_page(), so that gup extends the lifetime of the device.