Date:   Tue, 24 Jan 2023 12:16:59 -0800
From:   Ira Weiny <ira.weiny@intel.com>
To:     "Fabio M. De Francesco" <fmdefrancesco@gmail.com>,
        Matthew Wilcox <willy@infradead.org>,
        Al Viro <viro@zeniv.linux.org.uk>
CC:     Christoph Hellwig <hch@infradead.org>,
        <linux-kernel@vger.kernel.org>, <linux-fsdevel@vger.kernel.org>,
        Ira Weiny <ira.weiny@intel.com>, <linux-parisc@vger.kernel.org>
Subject: Re: [PATCH v3 4/4] fs/sysv: Replace kmap() with kmap_local_page()
Message-ID: <63d03cbb5c6a8_9ed629449@iweiny-mobl.notmuch>
References: <20230119153232.29750-1-fmdefrancesco@gmail.com>
 <Y8ohpDtqI8bPAgRn@ZenIV>
 <Y8os8QR1pRXyu4N8@ZenIV>
 <3146373.5fSG56mABF@suse>
Content-Type: text/plain; charset="iso-8859-1"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <3146373.5fSG56mABF@suse>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?iso-8859-1?Q?NCU55otDfDFqQFopwOvNvKlY13Uyo+rratw//gYhoRPjF/354eIGIQ256k?=
 =?iso-8859-1?Q?p5IDZ9WoLkoDuo/GIu7ySWrEfNXHYW9TT/HgpyjKseWkS+c3r4NLadpVqG?=
 =?iso-8859-1?Q?7EBoSY+B5/PtfeWVoe59m2LbF6soB/rPOhlk2F2R5ccvL6RWf7Ltesy79L?=
 =?iso-8859-1?Q?OtjvhKiHmTSUue8G75Pmqy+3yI734s/qnjrA+YWCwUvYHImvQiO94GxLJl?=
 =?iso-8859-1?Q?CeYTxOBlQyFXZc+1wHnrjIR2Amr/0lg10z5WQvXsQgtYPsa1/bogs7f5uJ?=
 =?iso-8859-1?Q?Bll9bLVDzMjtfrs29w1oMqvRtywdvp/pOuszAD58g4VOLZHN1ygW+tTiP0?=
 =?iso-8859-1?Q?PBNKXZQqhB3tGQSR8BKBG9jxrdqs2NDswcKZHbsJ8FEFPT3kiO8TaD7ghb?=
 =?iso-8859-1?Q?Pa7y/FCT0GLCMsSTUslXQpv2ZN7T2QtxkHpKiehTcHiivR9GKHlI/4GjU5?=
 =?iso-8859-1?Q?1i4Mk9OxK4T5iijMBcdrlRonOj/9Ylg8VqBL2c+fB4L3zIT5sHvFBZuHYH?=
 =?iso-8859-1?Q?EokLnuO4GEZuRa372q+n8yrnZjE07z0zpgnFwKJV6jhjv8BzriwH9OKQmL?=
 =?iso-8859-1?Q?0N4yRL9nAWrPxTyxD5/qQIbRj0pBDs4963tUiEW10u/Jns6f793Kf1GAEv?=
 =?iso-8859-1?Q?Q/ws6jMlnJrLM8QNiz5+6TZjaw+GSIdq/VjOqpE07AN0sZYSMzoSf6UsZF?=
 =?iso-8859-1?Q?h5bQnfo+J1xZoS22E85sag9TPChBgCrYP9avPlnxciQ4YlccRD6ut+86qz?=
 =?iso-8859-1?Q?9sxfyXMSfUCnPHHvfHhU1NHeFcPrSP7hbfW91NSTVkTPJP6Sd5+DvFB/57?=
 =?iso-8859-1?Q?b9sFTogWV9SXnBkTfAWq6YQ57MZWVtw2ZTF+OAoPB0pLYtPhgsN/oBiNC6?=
 =?iso-8859-1?Q?j9XEVy70D3xxIDBnnjYU1QGqp2fXAnSiFBT5HdEopXsw229ysQnT09zaxt?=
 =?iso-8859-1?Q?P1EI3Z9LE2qBHferXNeVR8fprT1YAiUnYx7AlSSiG+vkUXZHJG4yQFK5kv?=
 =?iso-8859-1?Q?r1wdcMaYt3BZFzon0mx+hwggSd9q6Gvod4VFWrsx/Ah0YyKfxHVaoh8POF?=
 =?iso-8859-1?Q?zeSf2g8VSmWdJ/IQ4OClgKG0N9PAuIK/fq2Ks124r3A1RN9gWKcl3dKIP/?=
 =?iso-8859-1?Q?Y9+RXdaZiW762ss4D73OdxaNf3URP0dZ35ZhML7BPD04g7j/zrxWZO+7tq?=
 =?iso-8859-1?Q?tkYcQoZ2Kyl1NOJEMCWiNAc7AQg0vaazmwTlj8qkBQzXbneJ3A/kgNv2o0?=
 =?iso-8859-1?Q?sYZ0+dSLRYXGRh2LT635LTunBRLf2Jpd1B2VIbddxhRCRADnGhe8BQcgdd?=
 =?iso-8859-1?Q?MsLR6s6izKr/+28bKT9Hbf8t94I2H6H2/ftcd+Vb36LWxGEo18nc/8dgoa?=
 =?iso-8859-1?Q?6aZg6HuzreXHmK8xYDUPmyDlI8H7GPNsWJtQMQfYst/c1MB4jeGv0Pc+WQ?=
 =?iso-8859-1?Q?bL5xfLUPVMWDe6heHWRNi3mPq0vKSau+y6ZjQbGigwyoUqw8lNMrukT5Zw?=
 =?iso-8859-1?Q?5K6qlvM+/rdXt4NfTVm1+HHA833h4U5jgkv99WVJlgUOvzoZx5I7cuVHpJ?=
 =?iso-8859-1?Q?LDArF6shYIXj/iF0YLA2ZT45tEkcRdvGPtcfnPuUZOWuAP3AMcn8RxhGYw?=
 =?iso-8859-1?Q?d8282A7Z/OGxAbQwNp6zJ1G4gBAzzB4OUdd8SYtbrKbtTDVsHiph+vvw?=
 =?iso-8859-1?Q?=3D=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 55d0ba88-3ba7-4cbc-71e5-08dafe47f496
X-MS-Exchange-CrossTenant-AuthSource: SA1PR11MB6733.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 24 Jan 2023 20:17:02.8010
 (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: w7Ixne5PH/OYW1o98dByWhK5rQdFNuCbf+P3VgO7sT9K74z/BxQdrGRsC0iEjExBfEkLOeuoSi7qJvzrkLTp3w==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA1PR11MB8150
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Fabio M. De Francesco wrote:
> On venerd? 20 gennaio 2023 06:56:01 CET Al Viro wrote:
> > On Fri, Jan 20, 2023 at 05:07:48AM +0000, Al Viro wrote:
> > > On Fri, Jan 20, 2023 at 04:54:51AM +0000, Matthew Wilcox wrote:
> > > > > Sure, but... there's also this:
> > > > > 
> > > > > static inline void __kunmap_local(const void *addr)
> > > > > {
> > > > > #ifdef ARCH_HAS_FLUSH_ON_KUNMAP
> > > > > 
> > > > >         kunmap_flush_on_unmap(addr);
> > > > > 
> > > > > #endif
> > > > > }
> > > > > 
> > > > > Are you sure that the guts of that thing will be happy with address 
> that
> > > > > is not page-aligned?  I've looked there at some point, got scared of
> > > > > parisc (IIRC) MMU details and decided not to rely upon that...
> > > > 
> > > > Ugh, PA-RISC (the only implementer) definitely will flush the wrong
> > > > addresses.  I think we should do this, as having bugs that only manifest
> > > > on one not-well-tested architecture seems Bad.
> > > > 
> > > >  static inline void __kunmap_local(const void *addr)
> > > >  {
> > > >  #ifdef ARCH_HAS_FLUSH_ON_KUNMAP
> > > > 
> > > > -       kunmap_flush_on_unmap(addr);
> > > > +       kunmap_flush_on_unmap(PAGE_ALIGN_DOWN(addr));
> > > > 
> > > >  #endif
> > > >  }
> > > 
> > > PTR_ALIGN_DOWN(addr, PAGE_SIZE), perhaps?
> > 
> > 	Anyway, that's a question to parisc folks; I _think_ pdtlb
> > quietly ignores the lower bits of address, so that part seems
> > to be safe, but I wouldn't bet upon that.  And when I got to
> > flush_kernel_dcache_page_asm I gave up - it's been a long time
> > since I've dealt with parisc assembler.
> 
> There seems to be consensus that __kunmap_local() needs to be fixed for the 
> parisc case (ARCH_HAS_FLUSH_ON_KUNMAP).
> 
> Is anyone doing this task?

I'm not looking at it.

> 
> If you agree, I could make this change and give proper credits for the tip.

I think that would be great.

Thanks!
Ira

> 
> Thank you,
> 
> Fabio
> 
> 
>