Received-SPF: pass (google.com: domain of linux-ext4-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Message-ID: <499043a0-b3d8-7a42-4aee-84b81f5b633f@oracle.com>
Date:   Tue, 19 Oct 2021 16:13:34 +0100
Subject: Re: [PATCH v1 2/2] mm: remove extra ZONE_DEVICE struct page refcount
Content-Language: en-US
To:     Jason Gunthorpe <jgg@ziepe.ca>,
        Dan Williams <dan.j.williams@intel.com>
Cc:     Matthew Wilcox <willy@infradead.org>,
        Alex Sierra <alex.sierra@amd.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        "Kuehling, Felix" <Felix.Kuehling@amd.com>,
        Linux MM <linux-mm@kvack.org>,
        Ralph Campbell <rcampbell@nvidia.com>,
        linux-ext4 <linux-ext4@vger.kernel.org>,
        linux-xfs <linux-xfs@vger.kernel.org>,
        amd-gfx list <amd-gfx@lists.freedesktop.org>,
        Maling list - DRI developers 
        <dri-devel@lists.freedesktop.org>, Christoph Hellwig <hch@lst.de>,
        =?UTF-8?B?SsOpcsO0bWUgR2xpc3Nl?= <jglisse@redhat.com>,
        Alistair Popple <apopple@nvidia.com>,
        Vishal Verma <vishal.l.verma@intel.com>,
        Dave Jiang <dave.jiang@intel.com>,
        Linux NVDIMM <nvdimm@lists.linux.dev>,
        David Hildenbrand <david@redhat.com>
References: <20211014153928.16805-3-alex.sierra@amd.com>
 <20211014170634.GV2744544@nvidia.com> <YWh6PL7nvh4DqXCI@casper.infradead.org>
 <CAPcyv4hBdSwdtG6Hnx9mDsRXiPMyhNH=4hDuv8JZ+U+Jj4RUWg@mail.gmail.com>
 <20211014230606.GZ2744544@nvidia.com>
 <CAPcyv4hC4qxbO46hp=XBpDaVbeh=qdY6TgvacXRprQ55Qwe-Dg@mail.gmail.com>
 <20211016154450.GJ2744544@nvidia.com>
 <CAPcyv4j0kHREAOG6_07E2foz6e4FP8D72mZXH6ivsiUBu_8c6g@mail.gmail.com>
 <20211018182559.GC3686969@ziepe.ca>
 <CAPcyv4jvZjeMcKLVuOEQ_gXRd87i3NUX5D=MmsJ++rWafnK-NQ@mail.gmail.com>
 <20211018230614.GF3686969@ziepe.ca>
From:   Joao Martins <joao.m.martins@oracle.com>
In-Reply-To: <20211018230614.GF3686969@ziepe.ca>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?Rjk3Q0tOUFlFcmdURkxHcFZIdzRCNnJaQ1dqNVBvTS94bHlXL01FeWdTdUhj?=
 =?utf-8?B?dEVNVWUxc2FMS0toUURja1pmL2FXcXYvN1JDbFJNSElPZXhuVFljREpUYzFk?=
 =?utf-8?B?QUQ3OVdMbXowUDZTd2pmUjBiZEFuUnNXU05VcVgrVHJ5UzE4c1ZUYk5lR2g4?=
 =?utf-8?B?MlpXdFBQN1M0ODA4N1FYYy9rMlBkSjVRblZjRVdRaXp0TmtaTXB0c1JiQVRo?=
 =?utf-8?B?d080YjJnTkM2STNUZ3FkZGJUNmI4TG1QQ2MzMStoeVNVZEx3YmtHakJmWlJ3?=
 =?utf-8?B?blpoeVFMVkZ5eXZEZ3VqUmVhdjRGZ1BNSTdHNThzRFJ3azY1TWI1eGtJcEY5?=
 =?utf-8?B?L1JFTjBnNktVRVREQTFoTytSd0I3M0N3dDIyUEo1T1owM0JqcFNaRW1jdXZk?=
 =?utf-8?B?RDRCSVJHNXNjU29rZkVLWWFwL2M1TFduL3drZitVZUpjNFAvWTNsYWlLZ0Rz?=
 =?utf-8?B?TVNRNHVTaU5OUmhKYktwaFlUNnl3aHVWdXJkTDdwcnIyemVJbmtENGY4ZkE0?=
 =?utf-8?B?RWRycmVtbkNOTjlmTldIajdoYmF6ejRvdGkxaFBHUFlxbVZhWFUxVUtZQjFZ?=
 =?utf-8?B?T1FuUVJjRjE0WlBBWDlsZUN1Umw0aXdySGVWN3ZpYUV2dkc4ZHpEcDBKZm1M?=
 =?utf-8?B?clMrZGJlZFBOREFmUlVBKy83UEJlUmZLMmQ4S1g1K21US0tEUjlRVDhIc2tS?=
 =?utf-8?B?RjhjcEFEczBxRDVkd0dhWjR3QkxSZGRyOU9nM1NRU3I2eEpMaGQwWDlDbnlD?=
 =?utf-8?B?NUxzUUVweFUvOEI0d1NEVnR3QXVZZWNvZ2NrY0ZtNmFVYTZNQ1hqTGNVVjQr?=
 =?utf-8?B?bko5ZmtmcWxtN0tPT1JNVDM5cFBJNWNrR1NVQlJjRnpCb3ozZzJ2L0xseGt4?=
 =?utf-8?B?eTBLdVI1NUVSZUdHckFwTmhHWlJhdko4cHZVcElUTjl3eXd5Q2tlQUUvKzdJ?=
 =?utf-8?B?RzFpWUhCbGkrV0tFNUJ5V1FjWllRSjMrdnR5UE1rOEZIdDlaTmUxOTVtMCtK?=
 =?utf-8?B?c0pBM2hWTDdWaVhUYUV2VXgwL0I1cEgyQVhaOUl5ZjF0NHlUVFBDNGx2VWhk?=
 =?utf-8?B?OE96bFZwWVhPSHB4ODY4WmtMdXFZaE95WTMwdzdsMGVFRTNVaGdVa2FpSjVN?=
 =?utf-8?B?RjREZm80TkY4US9UMGJkVkhjaEhXNVBUN3Y4VWdaaFpkbjRwR1FoOGNNc043?=
 =?utf-8?B?eUlIcWdMRDh6QkdqSmFWRUdJNlNZU2lmbjZ1ZnRNSnRTenpGcTdvOWp6YjRR?=
 =?utf-8?B?a0x1T2NQNzh5dmFzODIvRXVuaDQrTzZsN1p2dVlPSkZUU2ZSRlM5TEpQdDE1?=
 =?utf-8?B?M1hEamhrSm9aTzNnNVZYWTI0RldscnNiZ1l6a0h3c1RVT1UwenhZcTBRZVpP?=
 =?utf-8?B?ZmMrN1ZjbGJTWWdVMGx4dDdFaERnUzV2Zjh1c3J0SDhKcWQ2KzRjZWZYUzN3?=
 =?utf-8?B?UUlNc0RBVFVVWlhvYVZEM0xDVEJkWkJEbjltWGxCajVBWmlqSWVtNjJTZERR?=
 =?utf-8?B?dVk4UXg4WUlpVjlmbnhOQ1M4M2VvNy9NcVYyWDhWOEt4cm5OMzIrd1MyQkZk?=
 =?utf-8?B?dFRqWGU1c1dvS1lNeFJURU5qc1FtZUFPMDBwRzQ2d0dBdE5EaXgwVm5LREtu?=
 =?utf-8?B?ZnRXdmFaUGdjS2ljbnJSK1pydFRQRCtNeWVPVVlHVzdyYWt4V0NoYTY1aXQ2?=
 =?utf-8?B?bC8xb3V3eWFPczI3NVFZQnlwWGkvZStmaGhUN0dYeUlZbXRvU09IZk1lUDZx?=
 =?utf-8?Q?a+eUGqU9VN93l1e9ZlVY5lRhFxZzKywKfPSHaMy?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 5a1011eb-648a-4af4-b709-08d993130a1b
X-MS-Exchange-CrossTenant-AuthSource: BLAPR10MB4835.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 19 Oct 2021 15:13:43.6530
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: WxurfhG+UZjnlYJM96T2sPLWz12AKEs35qZua6AFg30kb2SVC5mR4TquMITNs30R+pkVt35PUPz8hIBddqBjpdQYQPtaby8CmORFl8iXzeM=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BLAPR10MB5169
X-Proofpoint-Virus-Version: vendor=nai engine=6300 definitions=10142 signatures=668683
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxlogscore=999 mlxscore=0 adultscore=0
 spamscore=0 phishscore=0 bulkscore=0 suspectscore=0 malwarescore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2109230001
 definitions=main-2110190091
X-Proofpoint-ORIG-GUID: f4UnzgvkAxrBcIy4aqzsMou_lUADvY1X
X-Proofpoint-GUID: f4UnzgvkAxrBcIy4aqzsMou_lUADvY1X
Precedence: bulk
List-ID: <linux-ext4.vger.kernel.org>
X-Mailing-List: linux-ext4@vger.kernel.org

On 10/19/21 00:06, Jason Gunthorpe wrote:
> On Mon, Oct 18, 2021 at 12:37:30PM -0700, Dan Williams wrote:
> 
>>> device-dax uses PUD, along with TTM, they are the only places. I'm not
>>> sure TTM is a real place though.
>>
>> I was setting device-dax aside because it can use Joao's changes to
>> get compound-page support.
> 
> Ideally, but that ideas in that patch series have been floating around
> for a long time now..
>  
The current status of the series misses a Rb on patches 6,7,10,12-14.
Well, patch 8 too should now drop its tag, considering the latest
discussion.

If it helps moving things forward I could split my series further into:

1) the compound page introduction (patches 1-7) of my aforementioned series
2) vmemmap deduplication for memory gains (patches 9-14)
3) gup improvements (patch 8 and gup-slow improvements)

The reason being that item 1) is the the main dependency listed below.
And allows 2) and 3) to be parallelized. FWIW, it is almost fully reviewed
by Dan (as of v3->v4). For (1) patches 6 & 7 are on changes to
device-dax subsystem (drivers/dax/*) which still needs his Ack.

>>> Here I imagine the thing that creates the pgmap would specify the
>>> policy it wants. In most cases the policy is tightly coupled to what
>>> the free function in the the provided dev_pagemap_ops does..
>>
>> The thing that creates the pgmap is the device-driver, and
>> device-driver does not implement truncate or reclaim. It's not until
>> the FS mounts that the pgmap needs to start enforcing pin lifetime
>> guarantees.
> 
> I am explaining this wrong, the immediate need is really 'should
> foll_longterm fail fast-gup to the slow path' and something like the
> nvdimm driver can just set that to 1 and rely on VMA flags to control
> what the slow path does - as is today.
> 
> It is not as elegant as more flags in the pgmap, but it would get the
> job done with minimal fuss.
> 
> Might be nice to either rely fully on VMA flags or fully on pgmap
> holder flags for FOLL_LONGTERM?
>

Whats the benefit between preventing longterm at start
versus only after mounting the filesystem? Or is the intended future purpose
to pass more context into an holder potential future callback e.g. nack longterm
pins on a page basis?

Maybe we can start by at least not add any flags and just prevent
FOLL_LONGTERM on fsdax -- which I guess was the original purpose of
commit 7af75561e171 ("mm/gup: add FOLL_LONGTERM capability to GUP fast").
This patch (which I can formally send) has a sketch of that (below scissors mark):

https://lore.kernel.org/linux-mm/6a18179e-65f7-367d-89a9-d5162f10fef0@oracle.com/

It uses pgmap->type rather than adding further fields into pgmap, given this
restriction applies only to fsdax.

... and then we could improve devmap_longterm_available(pgmap) to look at the
holder::flags or pgmap::flags should we decide that an explicit flags is required
from holder/pgmap .. as a further improvement?

	Joao